Re: 答复: [External Mail][PATCH 2/2] HID: input: report battery status changes immediately

José Expósito
2025-08-05T08:42:44Z

Hi!

On Tue, Aug 05, 2025 at 01:43:30AM +0000, 卢国宏 wrote:
> ________________________________
> 发件人: José Expósito <jose.exposito89@gmail.com>
> 发送时间: 2025年8月4日 17:11
> 收件人: jikos@kernel.org
> 抄送: bentiss@kernel.org; 卢国宏; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; José Expósito
> 主题: [External Mail][PATCH 2/2] HID: input: report battery status changes immediately
>
> [外部邮件] 此邮件来源于小米公司外部，请谨慎处理。若对邮件安全性存疑，请将邮件转发给misec@xiaomi.com进行反馈
>
> When the battery status changes, report the change immediately to user
> space.
>
> Fixes: a608dc1c0639 ("HID: input: map battery system charging")
> Reported-by: 卢国宏 <luguohong@xiaomi.com>
> Closes: https://lore.kernel.org/linux-input/aI49Im0sGb6fpgc8@fedora/T/
> Signed-off-by: José Expósito <jose.exposito89@gmail.com>
> ---
> drivers/hid/hid-input.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 262787e6eb20..277538a17b57 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -609,13 +609,19 @@ static bool hidinput_update_battery_charge_status(struct hid_device *dev,
> return false;
> }
>
> -static void hidinput_update_battery(struct hid_device *dev, int value)
> +static void hidinput_update_battery(struct hid_device *dev, unsigned int usage,
> + int value)
> {
> int capacity;
>
> if (!dev->battery)
> return;
>
> + if (hidinput_update_battery_charge_status(dev, usage, value)) {
> + power_supply_changed(dev->battery);
> + return;
> + }
> +
>
> > Hi, José. Shouldn't the return statement in this code be removed?
> > Otherwise, if both the battery level and the charging status change
> > simultaneously, the code following the if statement that updates the
> > battery level won't run, and the battery level won't be updated.
> > Thanks!

At least on the hardware I have access to, changes are reported independently.

I added a log at the beginning of this function to illustrate it.
This is the output when the battery of my device goes from 99% to 95%:

New EV_PWR report:
usage = 8716389
value = 99
New EV_PWR report:
usage = 8716356 (HID_BAT_CHARGING)
value = 0 (POWER_SUPPLY_STATUS_DISCHARGING)
[...]
New EV_PWR report:
usage = 8716389
value = 95
New EV_PWR report:
usage = 8716356 (HID_BAT_CHARGING)
value = 0 (POWER_SUPPLY_STATUS_DISCHARGING)

If we remove that return, then "value" (0 or 1) would be used as battery level,
reporting wrong battery levels to user-space.

Isn't your device reporting its battery information in a similar way?

Jose

PS - I'll fix the warning in v2 and add a Tested-by: 卢国宏 tag once
this is confirmed to work on the affected hardware.

>
>
> if (value == 0 || value < dev->battery_min || value > dev->battery_max)
> return;
>
> @@ -648,7 +654,8 @@ static bool hidinput_update_battery_charge_status(struct hid_device *dev,
> return false;
> }
>
> -static void hidinput_update_battery(struct hid_device *dev, int value)
> +static void hidinput_update_battery(struct hid_device *dev, unsigned int usage,
> + int value)
> {
> }
> #endif /* CONFIG_HID_BATTERY_STRENGTH */
> @@ -1515,11 +1522,7 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
> return;
>
> if (usage->type == EV_PWR) {
> - bool handled = hidinput_update_battery_charge_status(hid, usage->hid, value);
> -
> - if (!handled)
> - hidinput_update_battery(hid, value);
> -
> + hidinput_update_battery(hid, usage->hid, value);
> return;
> }
>
> --
> 2.50.1