linux-kernel - Re: [PATCH v2] HID: uclogic: Add support for XP-PEN Deco LW

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: <20221029145511.GA7941@elementary>
Date:   Sat, 29 Oct 2022 16:55:11 +0200
From:   José Expósito <jose.exposito89@...il.com>
To:     Mia Kanashi <chad@...pilled.dev>
Cc:     jikos@...nel.org, benjamin.tissoires@...hat.com, spbnick@...il.com,
        pobrn@...tonmail.com, linux-input@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Andreas Grosse <andig.mail@...nline.de>
Subject: Re: [PATCH v2] HID: uclogic: Add support for XP-PEN Deco LW

Hi!

On Sat, Oct 29, 2022 at 04:55:21PM +0300, Mia Kanashi wrote:
> >>[1] Actually it should be set to discharging until this gets merged:
> >>    https://lore.kernel.org/linux-input/20221028181849.23157-1-jose.exposito89@gmail.com/T/
> >
> >But i also currently applied this ^ patch, i will try testing without it and then report.
> 
> Tested without it, same issue.
> So yeah it seems that hid-input driver can set supply status to discharging before setting a battery capacity? 

Very good catch. I managed to reproduce it using the USB dongle. I
didn't notice it before because I was running upower after connecting
the device, which isn't fast enough. However, using watch as you
suggested makes the issue pretty noticeable.

The problem is that the battery is fetched when the USB dongle is
connected. However, the tablet might not be paired at that point.

To explain it with the actual code:

  if (dev->battery_status != HID_BATTERY_REPORTED &&
      !dev->battery_avoid_query) {
	value = hidinput_query_battery_capacity(dev);
                ^ Here the battery is fetched, but because the tabled
                  is not paired and this function returns garbage
	if (value < 0)
		return value;

	dev->battery_capacity = value;
	dev->battery_status = HID_BATTERY_QUERIED;
                            ^ Now the battery is set as queried
  }

  if (dev->battery_status == HID_BATTERY_UNKNOWN)
	val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
  else
	val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
                    ^ And therefore the battery is reported

Thankfully, there is already a flag (battery_avoid_query) used to solve
the same issue on stylus. The battery percentage is unknown until the
stylus is in proximity.

So I guess I could use the same flag to avoid this problem.

I'll add a fix in a second revision of this patch.

Thanks!!