Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220407115406.115112-1-hadess@hadess.net> <nycvar.YFH.7.76.2204111708230.30217@cbobk.fhfr.pm>
In-Reply-To: <nycvar.YFH.7.76.2204111708230.30217@cbobk.fhfr.pm>
From:   Jason Gerecke <killertofu@gmail.com>
Date:   Wed, 13 Apr 2022 07:59:14 -0700
Message-ID: <CANRwn3SVcqFtv0BTTtqpM7M4WRN4sMKVo7_-9t5M8itt-rEttQ@mail.gmail.com>
Subject: Re: [PATCH] HID: wacom: Correct power_supply type
To:     Jiri Kosina <jikos@kernel.org>
Cc:     Bastien Nocera <hadess@hadess.net>,
        Linux Input <linux-input@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Benjamin Tissoires <benjamin.tissoires@redhat.com>,
        Ping Cheng <ping.cheng@wacom.com>,
        Jason Gerecke <jason.gerecke@wacom.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Following up on my previous comment. I've been able to test this patch
with both flavors of wireless interface. Both Bluetooth (Intuos Pro)
and dongle-based (Intuos5) appear to have mostly-correct behavior
while charging and discharging, even when the battery level gradually
drops to zero. The misbehaviors I see appear to be limited to upower
mis-categorizing the devices as an e.g. keyboard or generic battery
rather than as a tablet. This leads to some slightly confusing UI
issues (e.g. GNOME and KDE referring to the device incorrectly), but
nothing too annoying. If upower is taught to recognize tablets under
more circumstances those issues should disappear.

Ping tells me you may have an Intuos4 Wireless, Bastien? Any
additional testing you can do with that device would be appreciated,
though even without it I'm personally comfortable enough to provide an
ack:

Acked-by: Jason Gerecke <jason.gerecke@wacom.com>

Jason
---
Now instead of four in the eights place /
you=E2=80=99ve got three, =E2=80=98Cause you added one  /
(That is to say, eight) to the two,     /
But you can=E2=80=99t take seven from three,    /
So you look at the sixty-fours....

On Tue, Apr 12, 2022 at 1:53 AM Jiri Kosina <jikos@kernel.org> wrote:
>
> On Thu, 7 Apr 2022, Bastien Nocera wrote:
>
> > POWER_SUPPLY_TYPE_USB seems to only ever be used by USB ports that are
> > used to charge the machine itself (so a "system" scope), like the
> > single USB port on a phone, rather than devices.
> >
> > The wacom_sys driver is the only driver that sets its device battery as
> > being a USB type, which doesn't seem correct based on its usage, so
> > switch it to be a battery type like all the other USB-connected devices=
.
> >
> > Signed-off-by: Bastien Nocera <hadess@hadess.net>
> > ---
> >  drivers/hid/wacom_sys.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> > index 066c567dbaa2..620fe74f5676 100644
> > --- a/drivers/hid/wacom_sys.c
> > +++ b/drivers/hid/wacom_sys.c
> > @@ -1777,7 +1777,7 @@ static int __wacom_initialize_battery(struct waco=
m *wacom,
> >       bat_desc->get_property =3D wacom_battery_get_property;
> >       sprintf(battery->bat_name, "wacom_battery_%ld", n);
> >       bat_desc->name =3D battery->bat_name;
> > -     bat_desc->type =3D POWER_SUPPLY_TYPE_USB;
> > +     bat_desc->type =3D POWER_SUPPLY_TYPE_BATTERY;
> >       bat_desc->use_for_apm =3D 0;
> >
> >       ps_bat =3D devm_power_supply_register(dev, bat_desc, &psy_cfg);
>
> Thanks Bastien, makes sense. CCing Jason and Ping (the Wacom driver
> maintainers) to get their Ack.
>
> --
> Jiri Kosina
> SUSE Labs
>