[PATCH] HID: logitech-hidpp: battery: provide...

[PATCH] HID: logitech-hidpp: battery: provide CAPACITY property for newer devices

From: Hamza Mahfooz <hidden>
Date: 2021-07-23 06:24:42

For devices that only support the BATTERY_VOLTAGE (0x1001) feature, UPower
requires the additional information provided by this patch, to set them up.

Signed-off-by: Hamza Mahfooz <redacted>
---
 drivers/hid/hid-logitech-hidpp.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 61635e629469..662c335e9c17 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c

@@ -1331,6 +1331,32 @@ static int hidpp20_battery_get_battery_voltage(struct hidpp_device *hidpp,
 	return 0;
 }
 
+static int hidpp20_map_battery_capacity(int voltage)
+{
+	static const int voltages[] = {
+		4186, 4156, 4143, 4133, 4122, 4113, 4103, 4094, 4086, 4075,
+		4067, 4059, 4051, 4043, 4035, 4027, 4019, 4011, 4003, 3997,
+		3989, 3983, 3976, 3969, 3961, 3955, 3949, 3942, 3935, 3929,
+		3922, 3916, 3909, 3902, 3896, 3890, 3883, 3877, 3870, 3865,
+		3859, 3853, 3848, 3842, 3837, 3833, 3828, 3824, 3819, 3815,
+		3811, 3808, 3804, 3800, 3797, 3793, 3790, 3787, 3784, 3781,
+		3778, 3775, 3772, 3770, 3767, 3764, 3762, 3759, 3757, 3754,
+		3751, 3748, 3744, 3741, 3737, 3734, 3730, 3726, 3724, 3720,
+		3717, 3714, 3710, 3706, 3702, 3697, 3693, 3688, 3683, 3677,
+		3671, 3666, 3662, 3658, 3654, 3646, 3633, 3612, 3579, 3537,
+		3500
+	};
+
+	int i;
+
+	for (i = 0; i < (sizeof(voltages) / sizeof(int)); i++) {
+		if (voltage >= voltages[i])
+			return 100 - i;
+	}
+
+	return 0;
+}
+
 static int hidpp20_query_battery_voltage_info(struct hidpp_device *hidpp)
 {
 	u8 feature_type;

@@ -1354,6 +1380,7 @@ static int hidpp20_query_battery_voltage_info(struct hidpp_device *hidpp)
 
 	hidpp->battery.status = status;
 	hidpp->battery.voltage = voltage;
+	hidpp->battery.capacity = hidpp20_map_battery_capacity(voltage);
 	hidpp->battery.level = level;
 	hidpp->battery.charge_type = charge_type;
 	hidpp->battery.online = status != POWER_SUPPLY_STATUS_NOT_CHARGING;

@@ -1378,6 +1405,7 @@ static int hidpp20_battery_voltage_event(struct hidpp_device *hidpp,
 
 	if (voltage != hidpp->battery.voltage || status != hidpp->battery.status) {
 		hidpp->battery.voltage = voltage;
+		hidpp->battery.capacity = hidpp20_map_battery_capacity(voltage);
 		hidpp->battery.status = status;
 		hidpp->battery.level = level;
 		hidpp->battery.charge_type = charge_type;

@@ -3717,7 +3745,8 @@ static int hidpp_initialize_battery(struct hidpp_device *hidpp)
 	num_battery_props = ARRAY_SIZE(hidpp_battery_props) - 3;
 
 	if (hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_MILEAGE ||
-	    hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_PERCENTAGE)
+	    hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_PERCENTAGE ||
+	    hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_VOLTAGE)
 		battery_props[num_battery_props++] =
 				POWER_SUPPLY_PROP_CAPACITY;

-- 
2.32.0

Re: [PATCH] HID: logitech-hidpp: battery: provide CAPACITY property for newer devices

From: Bastien Nocera <hadess@hadess.net>
Date: 2021-07-23 09:31:36

Hey Hamza,

On Fri, 2021-07-23 at 02:23 -0400, Hamza Mahfooz wrote:

quoted hunk

For devices that only support the BATTERY_VOLTAGE (0x1001) feature,
UPower
requires the additional information provided by this patch, to set
them up.

Signed-off-by: Hamza Mahfooz <redacted>
---
 drivers/hid/hid-logitech-hidpp.c | 31
++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-

logitech-hidpp.c
index 61635e629469..662c335e9c17 100644

--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c

@@ -1331,6 +1331,32 @@ static int

hidpp20_battery_get_battery_voltage(struct hidpp_device *hidpp,
        return 0;
 }
 
+static int hidpp20_map_battery_capacity(int voltage)
+{
+       static const int voltages[] = {
+               4186, 4156, 4143, 4133, 4122, 4113, 4103, 4094, 4086,
4075,
+               4067, 4059, 4051, 4043, 4035, 4027, 4019, 4011, 4003,
3997,
+               3989, 3983, 3976, 3969, 3961, 3955, 3949, 3942, 3935,
3929,
+               3922, 3916, 3909, 3902, 3896, 3890, 3883, 3877, 3870,
3865,
+               3859, 3853, 3848, 3842, 3837, 3833, 3828, 3824, 3819,
3815,
+               3811, 3808, 3804, 3800, 3797, 3793, 3790, 3787, 3784,
3781,
+               3778, 3775, 3772, 3770, 3767, 3764, 3762, 3759, 3757,
3754,
+               3751, 3748, 3744, 3741, 3737, 3734, 3730, 3726, 3724,
3720,
+               3717, 3714, 3710, 3706, 3702, 3697, 3693, 3688, 3683,
3677,
+               3671, 3666, 3662, 3658, 3654, 3646, 3633, 3612, 3579,
3537,
+               3500
+       };
+
+       int i;
+
+       for (i = 0; i < (sizeof(voltages) / sizeof(int)); i++) {

Use ARRAY_SIZE()

+               if (voltage >= voltages[i])
+                       return 100 - i;

I don't really like this part, which seems to rely on things which
aren't asserted. Is there a way to make sure that voltages[] is 100
items and they're all initialised?

If you tested this, could you also mention which devices you tested
this on in the commit message?

Thanks for picking this up!

quoted hunk

+       }
+
+       return 0;
+}
+
 static int hidpp20_query_battery_voltage_info(struct hidpp_device
*hidpp)
 {
        u8 feature_type;

@@ -1354,6 +1380,7 @@ static int

hidpp20_query_battery_voltage_info(struct hidpp_device *hidpp)
 
        hidpp->battery.status = status;
        hidpp->battery.voltage = voltage;
+       hidpp->battery.capacity =
hidpp20_map_battery_capacity(voltage);
        hidpp->battery.level = level;
        hidpp->battery.charge_type = charge_type;
        hidpp->battery.online = status !=
POWER_SUPPLY_STATUS_NOT_CHARGING;

@@ -1378,6 +1405,7 @@ static int hidpp20_battery_voltage_event(struct

hidpp_device *hidpp,
 
        if (voltage != hidpp->battery.voltage || status != hidpp-

quoted

battery.status) {

                hidpp->battery.voltage = voltage;
+               hidpp->battery.capacity =
hidpp20_map_battery_capacity(voltage);
                hidpp->battery.status = status;
                hidpp->battery.level = level;
                hidpp->battery.charge_type = charge_type;

@@ -3717,7 +3745,8 @@ static int hidpp_initialize_battery(struct

hidpp_device *hidpp)
        num_battery_props = ARRAY_SIZE(hidpp_battery_props) - 3;
 
        if (hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_MILEAGE ||
-           hidpp->capabilities &
HIDPP_CAPABILITY_BATTERY_PERCENTAGE)
+           hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_PERCENTAGE
||
+           hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_VOLTAGE)
                battery_props[num_battery_props++] =
                                POWER_SUPPLY_PROP_CAPACITY;

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help