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

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

[José Expósito]

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！

Re: 答复: [External Mail]Re: 转发: commit?a608dc1c06397dc50ab773498433432fb5938f92 (patch) has a bug

[José Expósito]

Hi 卢国宏,

Forwarding your email to the mailing list and answering after it.

The mailing list won't accept your emails unless you send them in
plain text:

On Mon, Aug 04, 2025 at 01:55:37AM +0000, 卢国宏 wrote:
> ________________________________
> 发件人: José Expósito <jose.exposito89@gmail.com>
> 发送时间: 2025年8月3日 0:30
> 收件人: 卢国宏
> 抄送: jikos@kernel.org; bentiss@kernel.org; linux-input@vger.kernel.org; torvalds@linux-foundation.org; linus@linuxfoundation.org; jkosina@suse.cz
> 主题: [External Mail]Re: 转发: commit?a608dc1c06397dc50ab773498433432fb5938f92 (patch) has a bug
> 
> [外部邮件] 此邮件来源于小米公司外部，请谨慎处理。若对邮件安全性存疑，请将邮件转发给misec@xiaomi.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?
> 
> --->>> Our device is a pure USB-connected controller with a battery inside. The controller has two USB ports, one for connecting to a mobile phone's data port and the other for connecting to a charger. After the controller is connected to the mobile phone, and then the charger is connected to the controller's charging port, the controller will report the controller's charging status to the mobile phone through the HID protocol (Usage Page: 0x85 - Battery System Page, Usage ID: 0x44).
> 
> 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.
> 
> --->>>  When I debugged this issue, I checked the charging status of the USB controller device by using the Android uevents bin file in the Linux command line. For example, I executed the command "./uevents | grep AAA", where AAA is the device name. Then, when you unplug the charger, it will print "AAA POWER_SUPPLY_STATUS=Discharging", and when you plug in the charger, it will print "AAA POWER_SUPPLY_STATUS=charging".
> --->>> Because I am not familiar with the use of "hid-recorder" and "hid-replay" for the time being, if it is really necessary later, I will help you capture the reproduction records through these two tools.
> --->>> By the way, we previously approached Google directly to resolve the issue by adding the line "power_supply_changed(dev->battery);" mentioned in my original email to the Google Linux kernel code. We've already built two generations of this product, and this solution has proven to be feasible. However, when we requested this solution again for our third-generation product, Google disagreed. They said we should address the issue directly upstream! In other words, we should approach the Linux community to resolve the issue once and for all. They have a point, so I'm coming to you for help.
> 
> > 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;
>         }
> 
> --->>> I am sure that your patch can also solve our problem, because we have used your method to solve this problem in our previous internal code. If you can submit this patch, our problem will be solved. Thank you very much!
> 
> 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！
> #/******本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!******/#


I sent a couple of patches that I think will solve your problem:
https://lore.kernel.org/linux-input/20250804091215.6637-1-jose.exposito89@gmail.com/T/#md9f4924a4a5817fe6c0ff9474f8a5c0e36c9ee3b

Please compile and test them and let us know if the solution works
so we can merge the fix upstream.

If you find any problems, please provide a reproducer so I can debug it.

Thanks a lot in advance,
Jose