linux-kernel - Re: [PATCH] HID: wacom: Correct power

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANRwn3SVcqFtv0BTTtqpM7M4WRN4sMKVo7_-9t5M8itt-rEttQ@mail.gmail.com>
Date:   Wed, 13 Apr 2022 07:59:14 -0700
From:   Jason Gerecke <killertofu@...il.com>
To:     Jiri Kosina <jikos@...nel.org>
Cc:     Bastien Nocera <hadess@...ess.net>,
        Linux Input <linux-input@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Benjamin Tissoires <benjamin.tissoires@...hat.com>,
        Ping Cheng <ping.cheng@...om.com>,
        Jason Gerecke <jason.gerecke@...om.com>
Subject: Re: [PATCH] HID: wacom: Correct power_supply type

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@...om.com>

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

On Tue, Apr 12, 2022 at 1:53 AM Jiri Kosina <jikos@...nel.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@...ess.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 wacom *wacom,
> >       bat_desc->get_property = wacom_battery_get_property;
> >       sprintf(battery->bat_name, "wacom_battery_%ld", n);
> >       bat_desc->name = battery->bat_name;
> > -     bat_desc->type = POWER_SUPPLY_TYPE_USB;
> > +     bat_desc->type = POWER_SUPPLY_TYPE_BATTERY;
> >       bat_desc->use_for_apm = 0;
> >
> >       ps_bat = 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
>