Re: [PATCH v2 14/33] HID: logitech-hidpp: expose charge_type property for battery voltage feature

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] HID: logitech-hidpp: wrong charge_type in
  hidpp20_battery_map_status_voltage()
--

commit 17c7938d8f03bfe4c24215899bd92a24290ce9f0
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

HID: logitech-hidpp: expose charge_type property for battery voltage feature

This commit registers POWER_SUPPLY_PROP_CHARGE_TYPE in
hidpp_initialize_battery() for devices with the 0x1001 battery voltage
feature and checks level and charge_type for changes in
hidpp20_battery_voltage_event().

> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index cdeb1b3c2f219..fb9ad4cfc5832 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
[ ... ]
> @@ -4416,6 +4420,10 @@ static int hidpp_initialize_battery(struct hidpp_device *hidpp)
>  		battery_props[num_battery_props++] =
>  			POWER_SUPPLY_PROP_VOLTAGE_NOW;
>  
> +	if (hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_VOLTAGE)
> +		battery_props[num_battery_props++] =
> +				POWER_SUPPLY_PROP_CHARGE_TYPE;

[Severity: Medium]
Does exposing POWER_SUPPLY_PROP_CHARGE_TYPE here cause sysfs to report an
active charging rate even when the device is unplugged and discharging?

hidpp20_battery_map_status_voltage() unconditionally sets *charge_type to
POWER_SUPPLY_CHARGE_TYPE_STANDARD (or FAST / TRICKLE if bits 3 or 4 are
set) regardless of whether external power (flags & 0x80) is present or
status is POWER_SUPPLY_STATUS_CHARGING:

drivers/hid/hid-logitech-hidpp.c:hidpp20_battery_map_status_voltage() {
    ...
	} else
		status = POWER_SUPPLY_STATUS_DISCHARGING;

	*charge_type = POWER_SUPPLY_CHARGE_TYPE_STANDARD;
	if (test_bit(3, &flags)) {
		*charge_type = POWER_SUPPLY_CHARGE_TYPE_FAST;
	}
	if (test_bit(4, &flags)) {
		*charge_type = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
	}
    ...
}

When a device operates wirelessly on battery power, is full, or is not
charging, reading charge_type from sysfs returns Standard (or Fast/Trickle)
instead of N/A (POWER_SUPPLY_CHARGE_TYPE_NONE).  Also, when a wireless
device disconnects, hidpp_connect_event() resets online, status, and level
to unknown, but leaves hidpp->battery.charge_type at its previous active
value instead of POWER_SUPPLY_CHARGE_TYPE_UNKNOWN.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-hid-logitech-hidpp-v2-0-30743e91d83b@gmail.com?part=14