Re: [PATCH] HID: logitech-hidpp: BatteryVoltage: only read chargeStatus if extPower is active

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/11/20 4:24 PM, Filipe Laíns wrote:
In the HID++ 2.0 function getBatteryInfo() from the BatteryVoltage
(0x1001) feature, chargeStatus is only valid if extPower is active.

Previously we were ignoring extPower, which resulted in wrong values.

Nice catch. Sorry for missing that the first time around.


Example:
     With an unplugged mouse

     $ cat /sys/class/power_supply/hidpp_battery_0/status
     Charging

Tested and it works as expected now.


This patch makes fixes that, it also renames charge_sts to flags as
charge_sts can be confused with chargeStatus from the spec.

Spec:
+--------+-------------------------------------------------------------------------+
|  byte  |                                    2                                    |
+--------+--------------+------------+------------+----------+----------+----------+
|   bit  |     0..2     |      3     |      4     |     5    |     6    |     7    |
+--------+--------------+------------+------------+----------+----------+----------+
| buffer | chargeStatus | fastCharge | slowCharge | critical | (unused) | extPower |
+--------+--------------+------------+------------+----------+----------+----------+
Table 1 - battery voltage (0x1001), getBatteryInfo() (ASE 0), 3rd byte

+-------+--------------------------------------+
| value |                meaning               |
+-------+--------------------------------------+
|   0   | Charging                             |
+-------+--------------------------------------+
|   1   | End of charge (100% charged)         |
+-------+--------------------------------------+
|   2   | Charge stopped (any "normal" reason) |
+-------+--------------------------------------+
|   7   | Hardware error                       |
+-------+--------------------------------------+
Table 2 - chargeStatus value

Signed-off-by: Filipe Laíns <lains@xxxxxxxxxxxxx>
---
  drivers/hid/hid-logitech-hidpp.c | 43 ++++++++++++++++----------------
  1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index bb063e7d48df..39a5ee0aaab0 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -1256,36 +1256,35 @@ static int hidpp20_battery_map_status_voltage(u8 data[3], int *voltage,
  {
  	int status;
- long charge_sts = (long)data[2];
+	long flags = (long) data[2];
- *level = POWER_SUPPLY_CAPACITY_LEVEL_UNKNOWN;
-	switch (data[2] & 0xe0) {
-	case 0x00:
-		status = POWER_SUPPLY_STATUS_CHARGING;
-		break;
-	case 0x20:
-		status = POWER_SUPPLY_STATUS_FULL;
-		*level = POWER_SUPPLY_CAPACITY_LEVEL_FULL;
-		break;
-	case 0x40:
+	if (flags & 0x80)
+		switch (flags & 0x07) {
+		case 0:
+			status = POWER_SUPPLY_STATUS_CHARGING;
+			break;
+		case 1:
+			status = POWER_SUPPLY_STATUS_FULL;
+			*level = POWER_SUPPLY_CAPACITY_LEVEL_FULL;
+			break;
+		case 2:
+			status = POWER_SUPPLY_STATUS_NOT_CHARGING;
+			break;
+		default:
+			status = POWER_SUPPLY_STATUS_UNKNOWN;
+			break;
+		}
+	else
  		status = POWER_SUPPLY_STATUS_DISCHARGING;
-		break;
-	case 0xe0:
-		status = POWER_SUPPLY_STATUS_NOT_CHARGING;
-		break;
-	default:
-		status = POWER_SUPPLY_STATUS_UNKNOWN;
-	}
*charge_type = POWER_SUPPLY_CHARGE_TYPE_STANDARD;
-	if (test_bit(3, &charge_sts)) {
+	if (test_bit(3, &flags)) {
  		*charge_type = POWER_SUPPLY_CHARGE_TYPE_FAST;
  	}
-	if (test_bit(4, &charge_sts)) {
+	if (test_bit(4, &flags)) {
  		*charge_type = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
  	}
-
-	if (test_bit(5, &charge_sts)) {
+	if (test_bit(5, &flags)) {
  		*level = POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL;
  	}

Tested-by: Pedro Vanzella <pedro@xxxxxxxxxxxxxxxxx>
Reviewed-by: Pedro Vanzella <pedro@xxxxxxxxxxxxxxxxx>



[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux