Re: 转发: commit?a608dc1c06397dc50ab773498433432fb5938f92 (patch) has a bug

["José Expósito" <jose.exposito89@gmail.com>]

Hi 卢国宏,

Thanks a lot for reporting this issue.

On Sat, Aug 02, 2025 at 12:23:54PM +0000, 卢国宏 wrote:
> Hello, José and Jiri!
> I've discovered that the patch you submitted to the Linux community,
> "https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-6.3.y&id=a608dc1c06397dc50ab773498433432fb5938f92"
> contains a bug. Even with your patch, the charging status of HID
> devices is still not reported to upper layers in a timely manner.
> The reasons are as follows:
>
> [...]
> 
> if function hidinput_set_battery_charge_status() return true, That is,
> the charging status of the HID device has changed，This charging status
> will not be reported，Because, only when handled is false,
> "hid input update battery(hid, value);" will be called.

I wrote this patch a while ago and I can't remember the exact reason
why hidinput_update_battery() is only called by hidinput_hid_event()
when hidinput_set_battery_charge_status() returns false.

The devices I have change their status, for example, discharging to
charging, when they are connected via Bluetooth and I switch them to
USB. This reconnection reports the status to user-space, so there is
no need for additional synchronization.
Does your device work in the same manner?

However, before we try to fix the problem, it would be nice if you
could provide a reproducer.

What device is affected by this bug?

By using "sudo hid-recorder" and selecting your device, you will be
able to capture and replay (with "hid-replay") the HID events sent
by your device.
Could you share a recording of your device changing from the charging
to discharging status and back to charging so we can reproduce the
issue, please?

If the recording is too long, please upload it to a server.

> Therefore, the function "hidinput set battery charge status" can be
> changed to the following:
> 
> static bool hidinput_set_battery_charge_status(struct hid_device *dev,
> + unsigned int usage, int value)
> +{
> + switch (usage) {
> + case HID_BAT_CHARGING:
> + dev->battery_charge_status = value ?
> + POWER_SUPPLY_STATUS_CHARGING :
> + POWER_SUPPLY_STATUS_DISCHARGING;
> + power_supply_changed(dev->battery);
> + return true;
> + }
> +
> + return false;
> +}

Could you test if this patch also solves your problem, please?

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 9d80635a91eb..bce580beb5c6 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1515,11 +1515,8 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
                return;

        if (usage->type == EV_PWR) {
-               bool handled = hidinput_set_battery_charge_status(hid, usage->hid, value);
-
-               if (!handled)
-                       hidinput_update_battery(hid, value);
-
+               hidinput_set_battery_charge_status(hid, usage->hid, value);
+               hidinput_update_battery(hid, value);
                return;
        }

Thanks a lot in advance,
José Expósito

> Because we have encountered this problem in our project, and this
> method can solve it.
> I hope you can solve this problem as soon as possible, otherwise,
> we will encounter this problem again in our future projects.
>
> Thank you so much！