Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1609923 > unrolled thread

[PATCH v3 00/19] Report power supply from hid-logitech-hidpp

Started byBenjamin Tissoires <benjamin.tissoires@redhat.com>
First post2017-03-27 17:10 +0200
Last post2017-03-27 18:30 +0200
Articles 14 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 00/19] Report power supply from hid-logitech-hidpp Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-03-27 17:10 +0200
    [PATCH v3 12/19] HID: logitech-hidpp: return an error if the queried feature is not present Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-03-27 17:10 +0200
    [PATCH v3 11/19] HID: logitech-hidpp: create the battery for all types of HID++ devices Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-03-27 17:10 +0200
    [PATCH v3 08/19] HID: logitech-hidpp: rework hidpp_connect_event() Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-03-27 17:10 +0200
    [PATCH v3 10/19] HID: logitech-hidpp: forward device info in power_supply Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-03-27 17:10 +0200
    [PATCH v3 14/19] HID: logitech-hidpp: battery: provide ONLINE property Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-03-27 17:10 +0200
    [PATCH v3 17/19] HID: logitech-hidpp: add support for battery status for the K750 Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-03-27 17:10 +0200
    [PATCH v3 15/19] HID: logitech-hidpp: rename battery level into capacity Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-03-27 17:10 +0200
    [PATCH v3 18/19] HID: logitech-hidpp: enable HID++ 1.0 battery reporting Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-03-27 17:10 +0200
    [PATCH v3 03/19] HID: logitech-hidpp: make sure we only register one battery per device Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-03-27 17:10 +0200
    [PATCH v3 06/19] HID: logitech-hidpp: rework probe path for unifying devices Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-03-27 17:10 +0200
    [PATCH v3 09/19] HID: logitech-hidpp: handle battery events in hidpp_raw_hidpp_event() Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-03-27 17:10 +0200
    [PATCH v3 13/19] HID: logitech-hidpp: notify battery on connect Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-03-27 17:10 +0200
    Re: [PATCH v3 00/19] Report power supply from hid-logitech-hidpp Bastien Nocera <hadess@hadess.net> - 2017-03-27 18:30 +0200

#1609923 — [PATCH v3 00/19] Report power supply from hid-logitech-hidpp

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2017-03-27 17:10 +0200
Subject[PATCH v3 00/19] Report power supply from hid-logitech-hidpp
Message-ID<tpErn-1wA-7@gated-at.bofh.it>
Hi,

this is finally a rework of the series that provides kernel power_supply
for hidpp devices.

This will allow upower to not handle those devices anymore and to have more
immediate reportng of the device to the system.

I have splitted the series to not support non unifying receivers here. I got
the non Unifying (DJ) receivers working locally, but given I am not so sure
it will not break the G920, I'd rather send a shorter series later (this one
is big enough already).

Cheers,
Benjamin

Bastien Nocera (1):
  HID: logitech-hidpp: Add scope to battery

Benjamin Tissoires (18):
  HID: logitech-dj: allow devices to request full pairing information
  HID: logitech-hidpp: make sure we only register one battery per device
  HID: logitech-hidpp: do not query the name through HID++ for 1.0
    devices
  HID: logitech-hidpp: create a capabilities bits field
  HID: logitech-hidpp: rework probe path for unifying devices
  HID: logitech-hidpp: retrieve the HID++ device name when available
  HID: logitech-hidpp: rework hidpp_connect_event()
  HID: logitech-hidpp: handle battery events in hidpp_raw_hidpp_event()
  HID: logitech-hidpp: forward device info in power_supply
  HID: logitech-hidpp: create the battery for all types of HID++ devices
  HID: logitech-hidpp: return an error if the queried feature is not
    present
  HID: logitech-hidpp: notify battery on connect
  HID: logitech-hidpp: battery: provide ONLINE property
  HID: logitech-hidpp: rename battery level into capacity
  HID: logitech-hidpp: battery: provide CAPACITY_LEVEL
  HID: logitech-hidpp: add support for battery status for the K750
  HID: logitech-hidpp: enable HID++ 1.0 battery reporting
  HID: logitech-hidpp: add a sysfs file to tell we support power_supply

 drivers/hid/hid-logitech-dj.c    |  19 +-
 drivers/hid/hid-logitech-hidpp.c | 741 +++++++++++++++++++++++++++++++++------
 2 files changed, 641 insertions(+), 119 deletions(-)

-- 
2.9.3

[toc] | [next] | [standalone]

#1609924 — [PATCH v3 12/19] HID: logitech-hidpp: return an error if the queried feature is not present

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2017-03-27 17:10 +0200
Subject[PATCH v3 12/19] HID: logitech-hidpp: return an error if the queried feature is not present
Message-ID<tpEro-1wA-21@gated-at.bofh.it>
In reply to#1609923
Or the device just answers a valid feature '0'.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

changes in v3:
- moved up in the series

no changes in v2
---
 drivers/hid/hid-logitech-hidpp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 9a9771a..22129dd 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -503,6 +503,9 @@ static int hidpp_root_get_feature(struct hidpp_device *hidpp, u16 feature,
 	if (ret)
 		return ret;
 
+	if (response.fap.params[0] == 0)
+		return -ENOENT;
+
 	*feature_index = response.fap.params[0];
 	*feature_type = response.fap.params[1];
 
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1609925 — [PATCH v3 11/19] HID: logitech-hidpp: create the battery for all types of HID++ devices

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2017-03-27 17:10 +0200
Subject[PATCH v3 11/19] HID: logitech-hidpp: create the battery for all types of HID++ devices
Message-ID<tpEro-1wA-31@gated-at.bofh.it>
In reply to#1609923
The creation of the power_supply should not be in a HID++ 2.0 specific
function.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

changes in v3:
- moved up in the series

no changes in v2
---
 drivers/hid/hid-logitech-hidpp.c | 94 ++++++++++++++++++----------------------
 1 file changed, 43 insertions(+), 51 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 421c374..9a9771a 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -855,57 +855,6 @@ static int hidpp_battery_get_property(struct power_supply *psy,
 	return ret;
 }
 
-static int hidpp20_initialize_battery(struct hidpp_device *hidpp)
-{
-	static atomic_t battery_no = ATOMIC_INIT(0);
-	struct power_supply_config cfg = { .drv_data = hidpp };
-	struct power_supply_desc *desc = &hidpp->battery.desc;
-	struct hidpp_battery *battery;
-	unsigned long n;
-	int ret;
-
-	ret = hidpp20_query_battery_info(hidpp);
-	if (ret)
-		return ret;
-
-	battery = &hidpp->battery;
-
-	n = atomic_inc_return(&battery_no) - 1;
-	desc->properties = hidpp_battery_props;
-	desc->num_properties = ARRAY_SIZE(hidpp_battery_props);
-	desc->get_property = hidpp_battery_get_property;
-	sprintf(battery->name, "hidpp_battery_%ld", n);
-	desc->name = battery->name;
-	desc->type = POWER_SUPPLY_TYPE_BATTERY;
-	desc->use_for_apm = 0;
-
-	battery->ps = devm_power_supply_register(&hidpp->hid_dev->dev,
-						 &battery->desc,
-						 &cfg);
-	if (IS_ERR(battery->ps))
-		return PTR_ERR(battery->ps);
-
-	power_supply_powers(battery->ps, &hidpp->hid_dev->dev);
-
-	return 0;
-}
-
-static int hidpp_initialize_battery(struct hidpp_device *hidpp)
-{
-	int ret;
-
-	if (hidpp->battery.ps)
-		return 0;
-
-	if (hidpp->protocol_major >= 2) {
-		ret = hidpp20_initialize_battery(hidpp);
-		if (ret == 0)
-			hidpp->capabilities |= HIDPP_CAPABILITY_HIDPP20_BATTERY;
-	}
-
-	return ret;
-}
-
 /* -------------------------------------------------------------------------- */
 /* 0x6010: Touchpad FW items                                                  */
 /* -------------------------------------------------------------------------- */
@@ -2355,6 +2304,49 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report,
 	return 0;
 }
 
+static int hidpp_initialize_battery(struct hidpp_device *hidpp)
+{
+	static atomic_t battery_no = ATOMIC_INIT(0);
+	struct power_supply_config cfg = { .drv_data = hidpp };
+	struct power_supply_desc *desc = &hidpp->battery.desc;
+	struct hidpp_battery *battery;
+	unsigned long n;
+	int ret;
+
+	if (hidpp->battery.ps)
+		return 0;
+
+	if (hidpp->protocol_major >= 2) {
+		ret = hidpp20_query_battery_info(hidpp);
+		if (ret)
+			return ret;
+		hidpp->capabilities |= HIDPP_CAPABILITY_HIDPP20_BATTERY;
+	} else {
+		return -ENOENT;
+	}
+
+	battery = &hidpp->battery;
+
+	n = atomic_inc_return(&battery_no) - 1;
+	desc->properties = hidpp_battery_props;
+	desc->num_properties = ARRAY_SIZE(hidpp_battery_props);
+	desc->get_property = hidpp_battery_get_property;
+	sprintf(battery->name, "hidpp_battery_%ld", n);
+	desc->name = battery->name;
+	desc->type = POWER_SUPPLY_TYPE_BATTERY;
+	desc->use_for_apm = 0;
+
+	battery->ps = devm_power_supply_register(&hidpp->hid_dev->dev,
+						 &battery->desc,
+						 &cfg);
+	if (IS_ERR(battery->ps))
+		return PTR_ERR(battery->ps);
+
+	power_supply_powers(battery->ps, &hidpp->hid_dev->dev);
+
+	return ret;
+}
+
 static void hidpp_overwrite_name(struct hid_device *hdev)
 {
 	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1609926 — [PATCH v3 08/19] HID: logitech-hidpp: rework hidpp_connect_event()

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2017-03-27 17:10 +0200
Subject[PATCH v3 08/19] HID: logitech-hidpp: rework hidpp_connect_event()
Message-ID<tpEro-1wA-27@gated-at.bofh.it>
In reply to#1609923
Looks like all users don't care about a disconnect.
Simplify the various variant_connect() and put the connect state check
at the beginning.

For delayed input devices, make sure we go through all other connect
values (protocol, battery) before bailing out.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

changes in v3:
- moved up in the series

new in v2
---
 drivers/hid/hid-logitech-hidpp.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index b0d2fea..81ebded 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -1884,9 +1884,6 @@ static int wtp_connect(struct hid_device *hdev, bool connected)
 	struct wtp_data *wd = hidpp->private_data;
 	int ret;
 
-	if (!connected)
-		return 0;
-
 	if (!wd->x_size) {
 		ret = wtp_get_config(hidpp);
 		if (ret) {
@@ -1954,9 +1951,6 @@ static int m560_send_config_command(struct hid_device *hdev, bool connected)
 
 	hidpp_dev = hid_get_drvdata(hdev);
 
-	if (!connected)
-		return -ENODEV;
-
 	return hidpp_send_rap_command_sync(
 		hidpp_dev,
 		REPORT_ID_HIDPP_SHORT,
@@ -2160,9 +2154,6 @@ static int k400_connect(struct hid_device *hdev, bool connected)
 {
 	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
 
-	if (!connected)
-		return 0;
-
 	if (!disable_tap_to_click)
 		return 0;
 
@@ -2414,6 +2405,9 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
 	struct input_dev *input;
 	char *name, *devm_name;
 
+	if (!connected)
+		return;
+
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
 		ret = wtp_connect(hdev, connected);
 		if (ret)
@@ -2428,9 +2422,6 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
 			return;
 	}
 
-	if (!connected || hidpp->delayed_input)
-		return;
-
 	/* the device is already connected, we can ask for its name and
 	 * protocol */
 	if (!hidpp->protocol_major) {
@@ -2461,8 +2452,8 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
 
 	hidpp_initialize_battery(hidpp);
 
-	if (!(hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT))
-		/* if HID created the input nodes for us, we can stop now */
+	if (!(hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT) || hidpp->delayed_input)
+		/* if the input nodes are already created, we can stop now */
 		return;
 
 	input = hidpp_allocate_input(hdev);
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1609927 — [PATCH v3 10/19] HID: logitech-hidpp: forward device info in power_supply

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2017-03-27 17:10 +0200
Subject[PATCH v3 10/19] HID: logitech-hidpp: forward device info in power_supply
Message-ID<tpEro-1wA-33@gated-at.bofh.it>
In reply to#1609923
Better forwarding the device name, manufacturer and serial to upower.
Note that serial is still empty, it will be filled in a later patch
in this series.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

changes in v3:
- moved up in the series

changes in v2:
* model stripped of Logitech
* vendor Logitech -> Logitech, Inc.
* use hidpp->name for the power_supply name (so that it can be overloaded)
---
 drivers/hid/hid-logitech-hidpp.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 3e2f716..421c374 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -813,6 +813,9 @@ static enum power_supply_property hidpp_battery_props[] = {
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_CAPACITY,
 	POWER_SUPPLY_PROP_SCOPE,
+	POWER_SUPPLY_PROP_MODEL_NAME,
+	POWER_SUPPLY_PROP_MANUFACTURER,
+	POWER_SUPPLY_PROP_SERIAL_NUMBER,
 };
 
 static int hidpp_battery_get_property(struct power_supply *psy,
@@ -832,6 +835,18 @@ static int hidpp_battery_get_property(struct power_supply *psy,
 		case POWER_SUPPLY_PROP_SCOPE:
 			val->intval = POWER_SUPPLY_SCOPE_DEVICE;
 			break;
+		case POWER_SUPPLY_PROP_MODEL_NAME:
+			if (!strncmp(hidpp->name, "Logitech ", 9))
+				val->strval = hidpp->name + 9;
+			else
+				val->strval = hidpp->name;
+			break;
+		case POWER_SUPPLY_PROP_MANUFACTURER:
+			val->strval = "Logitech";
+			break;
+		case POWER_SUPPLY_PROP_SERIAL_NUMBER:
+			val->strval = hidpp->hid_dev->uniq;
+			break;
 		default:
 			ret = -EINVAL;
 			break;
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1609928 — [PATCH v3 14/19] HID: logitech-hidpp: battery: provide ONLINE property

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2017-03-27 17:10 +0200
Subject[PATCH v3 14/19] HID: logitech-hidpp: battery: provide ONLINE property
Message-ID<tpErp-1wA-37@gated-at.bofh.it>
In reply to#1609923
When ONLINE isn't set, upower should ignore the battery capacity,
so there is no need to overload it with some random values.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

completely reworked in v3:
- store a online field in hidpp->battery to be able to fine control
  the value
- provide POWER_SUPPLY_STATUS_UNKNOWN to the status when the device
  is disconnected
---
 drivers/hid/hid-logitech-hidpp.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 0781d2b..7e4445a 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -120,6 +120,7 @@ struct hidpp_battery {
 	char name[64];
 	int status;
 	int level;
+	bool online;
 };
 
 struct hidpp_device {
@@ -686,7 +687,6 @@ static int hidpp20_batterylevel_map_status_level(u8 data[3], int *level,
 						 int *next_level)
 {
 	int status;
-	int level_override;
 
 	*level = data[0];
 	*next_level = data[1];
@@ -698,36 +698,28 @@ static int hidpp20_batterylevel_map_status_level(u8 data[3], int *level,
 	switch (data[2]) {
 		case 0: /* discharging (in use) */
 			status = POWER_SUPPLY_STATUS_DISCHARGING;
-			level_override = 0;
 			break;
 		case 1: /* recharging */
 			status = POWER_SUPPLY_STATUS_CHARGING;
-			level_override = 80;
 			break;
 		case 2: /* charge in final stage */
 			status = POWER_SUPPLY_STATUS_CHARGING;
-			level_override = 90;
 			break;
 		case 3: /* charge complete */
 			status = POWER_SUPPLY_STATUS_FULL;
-			level_override = 100;
+			*level = 100;
 			break;
 		case 4: /* recharging below optimal speed */
 			status = POWER_SUPPLY_STATUS_CHARGING;
-			level_override = 50;
 			break;
 		/* 5 = invalid battery type
 		   6 = thermal error
 		   7 = other charging error */
 		default:
 			status = POWER_SUPPLY_STATUS_NOT_CHARGING;
-			level_override = 0;
 			break;
 	}
 
-	if (level_override != 0 && *level == 0)
-		*level = level_override;
-
 	return status;
 }
 
@@ -781,6 +773,9 @@ static int hidpp20_query_battery_info(struct hidpp_device *hidpp)
 
 	hidpp->battery.status = status;
 	hidpp->battery.level = level;
+	/* the capacity is only available when discharging or full */
+	hidpp->battery.online = status == POWER_SUPPLY_STATUS_DISCHARGING ||
+				status == POWER_SUPPLY_STATUS_FULL;
 
 	return 0;
 }
@@ -799,6 +794,10 @@ static int hidpp20_battery_event(struct hidpp_device *hidpp,
 	status = hidpp20_batterylevel_map_status_level(report->fap.params,
 						       &level, &next_level);
 
+	/* the capacity is only available when discharging or full */
+	hidpp->battery.online = status == POWER_SUPPLY_STATUS_DISCHARGING ||
+				status == POWER_SUPPLY_STATUS_FULL;
+
 	changed = level != hidpp->battery.level ||
 		  status != hidpp->battery.status;
 
@@ -813,6 +812,7 @@ static int hidpp20_battery_event(struct hidpp_device *hidpp,
 }
 
 static enum power_supply_property hidpp_battery_props[] = {
+	POWER_SUPPLY_PROP_ONLINE,
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_CAPACITY,
 	POWER_SUPPLY_PROP_SCOPE,
@@ -838,6 +838,9 @@ static int hidpp_battery_get_property(struct power_supply *psy,
 		case POWER_SUPPLY_PROP_SCOPE:
 			val->intval = POWER_SUPPLY_SCOPE_DEVICE;
 			break;
+		case POWER_SUPPLY_PROP_ONLINE:
+			val->intval = hidpp->battery.online;
+			break;
 		case POWER_SUPPLY_PROP_MODEL_NAME:
 			if (!strncmp(hidpp->name, "Logitech ", 9))
 				val->strval = hidpp->name + 9;
@@ -2416,8 +2419,14 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
 	struct input_dev *input;
 	char *name, *devm_name;
 
-	if (!connected)
+	if (!connected) {
+		if (hidpp->battery.ps) {
+			hidpp->battery.online = false;
+			hidpp->battery.status = POWER_SUPPLY_STATUS_UNKNOWN;
+			power_supply_changed(hidpp->battery.ps);
+		}
 		return;
+	}
 
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
 		ret = wtp_connect(hdev, connected);
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1609929 — [PATCH v3 17/19] HID: logitech-hidpp: add support for battery status for the K750

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2017-03-27 17:10 +0200
Subject[PATCH v3 17/19] HID: logitech-hidpp: add support for battery status for the K750
Message-ID<tpErp-1wA-43@gated-at.bofh.it>
In reply to#1609923
The Solar Keyboard uses a different feature to report the battery level.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

changes in v3:
- fixed online status

changes in v2:
* update state according to lux information
* do not update Lux if the event does not contain lux information
---
 drivers/hid/hid-logitech-hidpp.c | 115 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 113 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index d7dc458..bb40e5d 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -56,6 +56,7 @@ MODULE_PARM_DESC(disable_tap_to_click,
 #define HIDPP_QUIRK_CLASS_M560			BIT(1)
 #define HIDPP_QUIRK_CLASS_K400			BIT(2)
 #define HIDPP_QUIRK_CLASS_G920			BIT(3)
+#define HIDPP_QUIRK_CLASS_K750			BIT(4)
 
 /* bits 2..20 are reserved for classes */
 /* #define HIDPP_QUIRK_CONNECT_EVENTS		BIT(21) disabled */
@@ -117,6 +118,7 @@ struct hidpp_report {
 
 struct hidpp_battery {
 	u8 feature_index;
+	u8 solar_feature_index;
 	struct power_supply_desc desc;
 	struct power_supply *ps;
 	char name[64];
@@ -809,7 +811,7 @@ static int hidpp20_query_battery_info(struct hidpp_device *hidpp)
 	int ret;
 	int status, capacity, next_capacity, level;
 
-	if (hidpp->battery.feature_index == 0) {
+	if (hidpp->battery.feature_index == 0xff) {
 		ret = hidpp_root_get_feature(hidpp,
 					     HIDPP_PAGE_BATTERY_LEVEL_STATUS,
 					     &hidpp->battery.feature_index,
@@ -930,6 +932,101 @@ static int hidpp_battery_get_property(struct power_supply *psy,
 }
 
 /* -------------------------------------------------------------------------- */
+/* 0x4301: Solar Keyboard                                                     */
+/* -------------------------------------------------------------------------- */
+
+#define HIDPP_PAGE_SOLAR_KEYBOARD			0x4301
+
+#define CMD_SOLAR_SET_LIGHT_MEASURE			0x00
+
+#define EVENT_SOLAR_BATTERY_BROADCAST			0x00
+#define EVENT_SOLAR_BATTERY_LIGHT_MEASURE		0x10
+#define EVENT_SOLAR_CHECK_LIGHT_BUTTON			0x20
+
+static int hidpp_solar_request_battery_event(struct hidpp_device *hidpp)
+{
+	struct hidpp_report response;
+	u8 params[2] = { 1, 1 };
+	u8 feature_type;
+	int ret;
+
+	if (hidpp->battery.feature_index == 0xff) {
+		ret = hidpp_root_get_feature(hidpp,
+					     HIDPP_PAGE_SOLAR_KEYBOARD,
+					     &hidpp->battery.solar_feature_index,
+					     &feature_type);
+		if (ret)
+			return ret;
+	}
+
+	ret = hidpp_send_fap_command_sync(hidpp,
+					  hidpp->battery.solar_feature_index,
+					  CMD_SOLAR_SET_LIGHT_MEASURE,
+					  params, 2, &response);
+	if (ret > 0) {
+		hid_err(hidpp->hid_dev, "%s: received protocol error 0x%02x\n",
+			__func__, ret);
+		return -EPROTO;
+	}
+	if (ret)
+		return ret;
+
+	hidpp->capabilities |= HIDPP_CAPABILITY_BATTERY_MILEAGE;
+
+	return 0;
+}
+
+static int hidpp_solar_battery_event(struct hidpp_device *hidpp,
+				     u8 *data, int size)
+{
+	struct hidpp_report *report = (struct hidpp_report *)data;
+	int capacity, lux, status;
+	u8 function;
+
+	function = report->fap.funcindex_clientid;
+
+
+	if (report->fap.feature_index != hidpp->battery.solar_feature_index ||
+	    !(function == EVENT_SOLAR_BATTERY_BROADCAST ||
+	      function == EVENT_SOLAR_BATTERY_LIGHT_MEASURE ||
+	      function == EVENT_SOLAR_CHECK_LIGHT_BUTTON))
+		return 0;
+
+	capacity = report->fap.params[0];
+
+	switch (function) {
+	case EVENT_SOLAR_BATTERY_LIGHT_MEASURE:
+		lux = (report->fap.params[1] << 8) | report->fap.params[2];
+		if (lux > 200)
+			status = POWER_SUPPLY_STATUS_CHARGING;
+		else
+			status = POWER_SUPPLY_STATUS_DISCHARGING;
+		break;
+	case EVENT_SOLAR_CHECK_LIGHT_BUTTON:
+	default:
+		if (capacity < hidpp->battery.capacity)
+			status = POWER_SUPPLY_STATUS_DISCHARGING;
+		else
+			status = POWER_SUPPLY_STATUS_CHARGING;
+
+	}
+
+	if (capacity == 100)
+		status = POWER_SUPPLY_STATUS_FULL;
+
+	hidpp->battery.online = true;
+	if (capacity != hidpp->battery.capacity ||
+	    status != hidpp->battery.status) {
+		hidpp->battery.capacity = capacity;
+		hidpp->battery.status = status;
+		if (hidpp->battery.ps)
+			power_supply_changed(hidpp->battery.ps);
+	}
+
+	return 0;
+}
+
+/* -------------------------------------------------------------------------- */
 /* 0x6010: Touchpad FW items                                                  */
 /* -------------------------------------------------------------------------- */
 
@@ -2326,6 +2423,9 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
 		ret = hidpp20_battery_event(hidpp, data, size);
 		if (ret != 0)
 			return ret;
+		ret = hidpp_solar_battery_event(hidpp, data, size);
+		if (ret != 0)
+			return ret;
 	}
 
 	return 0;
@@ -2392,8 +2492,15 @@ static int hidpp_initialize_battery(struct hidpp_device *hidpp)
 	if (hidpp->battery.ps)
 		return 0;
 
+	hidpp->battery.feature_index = 0xff;
+	hidpp->battery.solar_feature_index = 0xff;
+
 	if (hidpp->protocol_major >= 2) {
-		ret = hidpp20_query_battery_info(hidpp);
+		if (hidpp->quirks & HIDPP_QUIRK_CLASS_K750)
+			ret = hidpp_solar_request_battery_event(hidpp);
+		else
+			ret = hidpp20_query_battery_info(hidpp);
+
 		if (ret)
 			return ret;
 		hidpp->capabilities |= HIDPP_CAPABILITY_HIDPP20_BATTERY;
@@ -2751,6 +2858,10 @@ static const struct hid_device_id hidpp_devices[] = {
 	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
 		USB_VENDOR_ID_LOGITECH, 0x4024),
 	  .driver_data = HIDPP_QUIRK_CLASS_K400 },
+	{ /* Solar Keyboard Logitech K750 */
+	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
+		USB_VENDOR_ID_LOGITECH, 0x4002),
+	  .driver_data = HIDPP_QUIRK_CLASS_K750 },
 
 	{ HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
 		USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1609930 — [PATCH v3 15/19] HID: logitech-hidpp: rename battery level into capacity

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2017-03-27 17:10 +0200
Subject[PATCH v3 15/19] HID: logitech-hidpp: rename battery level into capacity
Message-ID<tpEro-1wA-35@gated-at.bofh.it>
In reply to#1609923
The power_supply term for the percentage is capacity. Capacity level
can be given when non accurate mileage is provided by the device, so
better stick to the terms used in power_supply.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

new in v3
---
 drivers/hid/hid-logitech-hidpp.c | 55 ++++++++++++++++++++--------------------
 1 file changed, 28 insertions(+), 27 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 7e4445a..c59f7e5e 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -119,7 +119,7 @@ struct hidpp_battery {
 	struct power_supply *ps;
 	char name[64];
 	int status;
-	int level;
+	int capacity;
 	bool online;
 };
 
@@ -683,17 +683,16 @@ static char *hidpp_get_device_name(struct hidpp_device *hidpp)
 
 #define EVENT_BATTERY_LEVEL_STATUS_BROADCAST			0x00
 
-static int hidpp20_batterylevel_map_status_level(u8 data[3], int *level,
-						 int *next_level)
+static int hidpp20_batterylevel_map_status_capacity(u8 data[3], int *capacity,
+						    int *next_capacity)
 {
 	int status;
 
-	*level = data[0];
-	*next_level = data[1];
+	*capacity = data[0];
+	*next_capacity = data[1];
 
-	/* When discharging, we can rely on the device reported level.
-	 * For all other states the device reports level 0 (unknown). Make up
-	 * a number instead
+	/* When discharging, we can rely on the device reported capacity.
+	 * For all other states the device reports 0 (unknown).
 	 */
 	switch (data[2]) {
 		case 0: /* discharging (in use) */
@@ -707,7 +706,7 @@ static int hidpp20_batterylevel_map_status_level(u8 data[3], int *level,
 			break;
 		case 3: /* charge complete */
 			status = POWER_SUPPLY_STATUS_FULL;
-			*level = 100;
+			*capacity = 100;
 			break;
 		case 4: /* recharging below optimal speed */
 			status = POWER_SUPPLY_STATUS_CHARGING;
@@ -723,11 +722,11 @@ static int hidpp20_batterylevel_map_status_level(u8 data[3], int *level,
 	return status;
 }
 
-static int hidpp20_batterylevel_get_battery_level(struct hidpp_device *hidpp,
-						  u8 feature_index,
-						  int *status,
-						  int *level,
-						  int *next_level)
+static int hidpp20_batterylevel_get_battery_capacity(struct hidpp_device *hidpp,
+						     u8 feature_index,
+						     int *status,
+						     int *capacity,
+						     int *next_capacity)
 {
 	struct hidpp_report response;
 	int ret;
@@ -744,8 +743,8 @@ static int hidpp20_batterylevel_get_battery_level(struct hidpp_device *hidpp,
 	if (ret)
 		return ret;
 
-	*status = hidpp20_batterylevel_map_status_level(params, level,
-							next_level);
+	*status = hidpp20_batterylevel_map_status_capacity(params, capacity,
+							   next_capacity);
 
 	return 0;
 }
@@ -754,7 +753,7 @@ static int hidpp20_query_battery_info(struct hidpp_device *hidpp)
 {
 	u8 feature_type;
 	int ret;
-	int status, level, next_level;
+	int status, capacity, next_capacity;
 
 	if (hidpp->battery.feature_index == 0) {
 		ret = hidpp_root_get_feature(hidpp,
@@ -765,14 +764,15 @@ static int hidpp20_query_battery_info(struct hidpp_device *hidpp)
 			return ret;
 	}
 
-	ret = hidpp20_batterylevel_get_battery_level(hidpp,
-						     hidpp->battery.feature_index,
-						     &status, &level, &next_level);
+	ret = hidpp20_batterylevel_get_battery_capacity(hidpp,
+						hidpp->battery.feature_index,
+						&status, &capacity,
+						&next_capacity);
 	if (ret)
 		return ret;
 
 	hidpp->battery.status = status;
-	hidpp->battery.level = level;
+	hidpp->battery.capacity = capacity;
 	/* the capacity is only available when discharging or full */
 	hidpp->battery.online = status == POWER_SUPPLY_STATUS_DISCHARGING ||
 				status == POWER_SUPPLY_STATUS_FULL;
@@ -784,25 +784,26 @@ static int hidpp20_battery_event(struct hidpp_device *hidpp,
 				 u8 *data, int size)
 {
 	struct hidpp_report *report = (struct hidpp_report *)data;
-	int status, level, next_level;
+	int status, capacity, next_capacity;
 	bool changed;
 
 	if (report->fap.feature_index != hidpp->battery.feature_index ||
 	    report->fap.funcindex_clientid != EVENT_BATTERY_LEVEL_STATUS_BROADCAST)
 		return 0;
 
-	status = hidpp20_batterylevel_map_status_level(report->fap.params,
-						       &level, &next_level);
+	status = hidpp20_batterylevel_map_status_capacity(report->fap.params,
+							  &capacity,
+							  &next_capacity);
 
 	/* the capacity is only available when discharging or full */
 	hidpp->battery.online = status == POWER_SUPPLY_STATUS_DISCHARGING ||
 				status == POWER_SUPPLY_STATUS_FULL;
 
-	changed = level != hidpp->battery.level ||
+	changed = capacity != hidpp->battery.capacity ||
 		  status != hidpp->battery.status;
 
 	if (changed) {
-		hidpp->battery.level = level;
+		hidpp->battery.capacity = capacity;
 		hidpp->battery.status = status;
 		if (hidpp->battery.ps)
 			power_supply_changed(hidpp->battery.ps);
@@ -833,7 +834,7 @@ static int hidpp_battery_get_property(struct power_supply *psy,
 			val->intval = hidpp->battery.status;
 			break;
 		case POWER_SUPPLY_PROP_CAPACITY:
-			val->intval = hidpp->battery.level;
+			val->intval = hidpp->battery.capacity;
 			break;
 		case POWER_SUPPLY_PROP_SCOPE:
 			val->intval = POWER_SUPPLY_SCOPE_DEVICE;
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1609931 — [PATCH v3 18/19] HID: logitech-hidpp: enable HID++ 1.0 battery reporting

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2017-03-27 17:10 +0200
Subject[PATCH v3 18/19] HID: logitech-hidpp: enable HID++ 1.0 battery reporting
Message-ID<tpErp-1wA-41@gated-at.bofh.it>
In reply to#1609923
Also enable battery reporting for HID++ 1.0 devices through 2 registers:
0x07: battery status -> reports only 4 levels (critical, low, good, full)
0x0D: battery mileage -> reports true pourcentage

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

changes in v3:
- differentiate between level and mileage
- better online reporting

no changes in v2
---
 drivers/hid/hid-logitech-hidpp.c | 234 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 230 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index bb40e5d..eb4339e 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -400,6 +400,211 @@ static void hidpp_prefix_name(char **name, int name_length)
 #define HIDPP_SET_LONG_REGISTER				0x82
 #define HIDPP_GET_LONG_REGISTER				0x83
 
+#define HIDPP_REG_GENERAL				0x00
+
+static int hidpp10_enable_battery_reporting(struct hidpp_device *hidpp_dev)
+{
+	struct hidpp_report response;
+	int ret;
+	u8 params[3] = { 0 };
+
+	ret = hidpp_send_rap_command_sync(hidpp_dev,
+					REPORT_ID_HIDPP_SHORT,
+					HIDPP_GET_REGISTER,
+					HIDPP_REG_GENERAL,
+					NULL, 0, &response);
+	if (ret)
+		return ret;
+
+	memcpy(params, response.rap.params, 3);
+
+	/* Set the battery bit */
+	params[0] |= BIT(4);
+
+	return hidpp_send_rap_command_sync(hidpp_dev,
+					REPORT_ID_HIDPP_SHORT,
+					HIDPP_SET_REGISTER,
+					HIDPP_REG_GENERAL,
+					params, 3, &response);
+}
+
+#define HIDPP_REG_BATTERY_STATUS			0x07
+
+static int hidpp10_battery_status_map_level(u8 param)
+{
+	int level;
+
+	switch (param) {
+	case 1 ... 2:
+		level = POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL;
+		break;
+	case 3 ... 4:
+		level = POWER_SUPPLY_CAPACITY_LEVEL_LOW;
+		break;
+	case 5 ... 6:
+		level = POWER_SUPPLY_CAPACITY_LEVEL_NORMAL;
+		break;
+	case 7:
+		level = POWER_SUPPLY_CAPACITY_LEVEL_HIGH;
+		break;
+	default:
+		level = POWER_SUPPLY_CAPACITY_LEVEL_UNKNOWN;
+	}
+
+	return level;
+}
+
+static int hidpp10_battery_status_map_status(u8 param)
+{
+	int status;
+
+	switch (param) {
+	case 0x00:
+		/* discharging (in use) */
+		status = POWER_SUPPLY_STATUS_DISCHARGING;
+		break;
+	case 0x21: /* (standard) charging */
+	case 0x24: /* fast charging */
+	case 0x25: /* slow charging */
+		status = POWER_SUPPLY_STATUS_CHARGING;
+		break;
+	case 0x26: /* topping charge */
+	case 0x22: /* charge complete */
+		status = POWER_SUPPLY_STATUS_FULL;
+		break;
+	case 0x20: /* unknown */
+		status = POWER_SUPPLY_STATUS_UNKNOWN;
+		break;
+	/*
+	 * 0x01...0x1F = reserved (not charging)
+	 * 0x23 = charging error
+	 * 0x27..0xff = reserved
+	 */
+	default:
+		status = POWER_SUPPLY_STATUS_NOT_CHARGING;
+		break;
+	}
+
+	return status;
+}
+
+static int hidpp10_query_battery_status(struct hidpp_device *hidpp)
+{
+	struct hidpp_report response;
+	int ret, status;
+
+	ret = hidpp_send_rap_command_sync(hidpp,
+					REPORT_ID_HIDPP_SHORT,
+					HIDPP_GET_REGISTER,
+					HIDPP_REG_BATTERY_STATUS,
+					NULL, 0, &response);
+	if (ret)
+		return ret;
+
+	hidpp->battery.level =
+		hidpp10_battery_status_map_level(response.rap.params[0]);
+	status = hidpp10_battery_status_map_status(response.rap.params[1]);
+	hidpp->battery.status = status;
+	/* the capacity is only available when discharging or full */
+	hidpp->battery.online = status == POWER_SUPPLY_STATUS_DISCHARGING ||
+				status == POWER_SUPPLY_STATUS_FULL;
+
+	return 0;
+}
+
+#define HIDPP_REG_BATTERY_MILEAGE			0x0D
+
+static int hidpp10_battery_mileage_map_status(u8 param)
+{
+	int status;
+
+	switch (param >> 6) {
+	case 0x00:
+		/* discharging (in use) */
+		status = POWER_SUPPLY_STATUS_DISCHARGING;
+		break;
+	case 0x01: /* charging */
+		status = POWER_SUPPLY_STATUS_CHARGING;
+		break;
+	case 0x02: /* charge complete */
+		status = POWER_SUPPLY_STATUS_FULL;
+		break;
+	/*
+	 * 0x03 = charging error
+	 */
+	default:
+		status = POWER_SUPPLY_STATUS_NOT_CHARGING;
+		break;
+	}
+
+	return status;
+}
+
+static int hidpp10_query_battery_mileage(struct hidpp_device *hidpp)
+{
+	struct hidpp_report response;
+	int ret, status;
+
+	ret = hidpp_send_rap_command_sync(hidpp,
+					REPORT_ID_HIDPP_SHORT,
+					HIDPP_GET_REGISTER,
+					HIDPP_REG_BATTERY_MILEAGE,
+					NULL, 0, &response);
+	if (ret)
+		return ret;
+
+	hidpp->battery.capacity = response.rap.params[0];
+	status = hidpp10_battery_mileage_map_status(response.rap.params[2]);
+	hidpp->battery.status = status;
+	/* the capacity is only available when discharging or full */
+	hidpp->battery.online = status == POWER_SUPPLY_STATUS_DISCHARGING ||
+				status == POWER_SUPPLY_STATUS_FULL;
+
+	return 0;
+}
+
+static int hidpp10_battery_event(struct hidpp_device *hidpp, u8 *data, int size)
+{
+	struct hidpp_report *report = (struct hidpp_report *)data;
+	int status, capacity, level;
+	bool changed;
+
+	if (report->report_id != REPORT_ID_HIDPP_SHORT)
+		return 0;
+
+	switch (report->rap.sub_id) {
+	case HIDPP_REG_BATTERY_STATUS:
+		capacity = hidpp->battery.capacity;
+		level = hidpp10_battery_status_map_level(report->rawbytes[1]);
+		status = hidpp10_battery_status_map_status(report->rawbytes[2]);
+		break;
+	case HIDPP_REG_BATTERY_MILEAGE:
+		capacity = report->rap.params[0];
+		level = hidpp->battery.level;
+		status = hidpp10_battery_mileage_map_status(report->rawbytes[3]);
+		break;
+	default:
+		return 0;
+	}
+
+	changed = capacity != hidpp->battery.capacity ||
+		  level != hidpp->battery.level ||
+		  status != hidpp->battery.status;
+
+	/* the capacity is only available when discharging or full */
+	hidpp->battery.online = status == POWER_SUPPLY_STATUS_DISCHARGING ||
+				status == POWER_SUPPLY_STATUS_FULL;
+
+	if (changed) {
+		hidpp->battery.level = level;
+		hidpp->battery.status = status;
+		if (hidpp->battery.ps)
+			power_supply_changed(hidpp->battery.ps);
+	}
+
+	return 0;
+}
+
 #define HIDPP_REG_PAIRING_INFORMATION			0xB5
 #define HIDPP_EXTENDED_PAIRING				0x30
 #define HIDPP_DEVICE_NAME				0x40
@@ -2428,6 +2633,12 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
 			return ret;
 	}
 
+	if (hidpp->capabilities & HIDPP_CAPABILITY_HIDPP10_BATTERY) {
+		ret = hidpp10_battery_event(hidpp, data, size);
+		if (ret != 0)
+			return ret;
+	}
+
 	return 0;
 }
 
@@ -2505,7 +2716,16 @@ static int hidpp_initialize_battery(struct hidpp_device *hidpp)
 			return ret;
 		hidpp->capabilities |= HIDPP_CAPABILITY_HIDPP20_BATTERY;
 	} else {
-		return -ENOENT;
+		ret = hidpp10_query_battery_status(hidpp);
+		if (ret) {
+			ret = hidpp10_query_battery_mileage(hidpp);
+			if (ret)
+				return -ENOENT;
+			hidpp->capabilities |= HIDPP_CAPABILITY_BATTERY_MILEAGE;
+		} else {
+			hidpp->capabilities |= HIDPP_CAPABILITY_BATTERY_LEVEL_STATUS;
+		}
+		hidpp->capabilities |= HIDPP_CAPABILITY_HIDPP10_BATTERY;
 	}
 
 	battery_props = devm_kmemdup(&hidpp->hid_dev->dev,
@@ -2665,11 +2885,17 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
 	hidpp_initialize_battery(hidpp);
 
 	/* forward current battery state */
-	if (hidpp->capabilities & HIDPP_CAPABILITY_HIDPP20_BATTERY) {
+	if (hidpp->capabilities & HIDPP_CAPABILITY_HIDPP10_BATTERY) {
+		hidpp10_enable_battery_reporting(hidpp);
+		if (hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_MILEAGE)
+			hidpp10_query_battery_mileage(hidpp);
+		else
+			hidpp10_query_battery_status(hidpp);
+	} else if (hidpp->capabilities & HIDPP_CAPABILITY_HIDPP20_BATTERY) {
 		hidpp20_query_battery_info(hidpp);
-		if (hidpp->battery.ps)
-			power_supply_changed(hidpp->battery.ps);
 	}
+	if (hidpp->battery.ps)
+		power_supply_changed(hidpp->battery.ps);
 
 	if (!(hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT) || hidpp->delayed_input)
 		/* if the input nodes are already created, we can stop now */
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1609932 — [PATCH v3 03/19] HID: logitech-hidpp: make sure we only register one battery per device

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2017-03-27 17:10 +0200
Subject[PATCH v3 03/19] HID: logitech-hidpp: make sure we only register one battery per device
Message-ID<tpErp-1wA-47@gated-at.bofh.it>
In reply to#1609923
Simple check to add, huge improvement :)

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

no changes in v3

no changes in v2
---
 drivers/hid/hid-logitech-hidpp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 4aaf237..1cda29e 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -828,6 +828,9 @@ static int hidpp_initialize_battery(struct hidpp_device *hidpp)
 {
 	int ret;
 
+	if (hidpp->battery.ps)
+		return 0;
+
 	if (hidpp->protocol_major >= 2) {
 		ret = hidpp20_initialize_battery(hidpp);
 		if (ret == 0)
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1609933 — [PATCH v3 06/19] HID: logitech-hidpp: rework probe path for unifying devices

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2017-03-27 17:10 +0200
Subject[PATCH v3 06/19] HID: logitech-hidpp: rework probe path for unifying devices
Message-ID<tpErq-1wA-49@gated-at.bofh.it>
In reply to#1609923
Unifying devices are different from others because they can probed
while not connected. So we need to talk to the receiver to get some
extra information like the device name and the serial.

Instead of having conditionals while attempting to read the device name
from HID++ 2.0, have a special init path for them.

Store the retrieved serial in hdev->uniq.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

changes in v3:
- moved up in the series

changes in v2:
* only overwrite the name when the input is delayed (was causing different
  name depending of the connect stat of the device while probing)
---
 drivers/hid/hid-logitech-hidpp.c | 86 +++++++++++++++++++++++++++++++---------
 1 file changed, 67 insertions(+), 19 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 4ee466c..db15cfc 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -62,6 +62,7 @@ MODULE_PARM_DESC(disable_tap_to_click,
 #define HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS	BIT(22)
 #define HIDPP_QUIRK_NO_HIDINPUT			BIT(23)
 #define HIDPP_QUIRK_FORCE_OUTPUT_REPORTS	BIT(24)
+#define HIDPP_QUIRK_UNIFYING			BIT(25)
 
 #define HIDPP_QUIRK_DELAYED_INIT		HIDPP_QUIRK_NO_HIDINPUT
 
@@ -394,14 +395,14 @@ static void hidpp_prefix_name(char **name, int name_length)
 #define HIDPP_GET_LONG_REGISTER				0x83
 
 #define HIDPP_REG_PAIRING_INFORMATION			0xB5
-#define DEVICE_NAME					0x40
+#define HIDPP_EXTENDED_PAIRING				0x30
+#define HIDPP_DEVICE_NAME				0x40
 
-static char *hidpp_get_unifying_name(struct hidpp_device *hidpp_dev)
+static char *hidpp_unifying_get_name(struct hidpp_device *hidpp_dev)
 {
 	struct hidpp_report response;
 	int ret;
-	/* hid-logitech-dj is in charge of setting the right device index */
-	u8 params[1] = { DEVICE_NAME };
+	u8 params[1] = { HIDPP_DEVICE_NAME };
 	char *name;
 	int len;
 
@@ -430,6 +431,54 @@ static char *hidpp_get_unifying_name(struct hidpp_device *hidpp_dev)
 	return name;
 }
 
+static int hidpp_unifying_get_serial(struct hidpp_device *hidpp, u32 *serial)
+{
+	struct hidpp_report response;
+	int ret;
+	u8 params[1] = { HIDPP_EXTENDED_PAIRING };
+
+	ret = hidpp_send_rap_command_sync(hidpp,
+					REPORT_ID_HIDPP_SHORT,
+					HIDPP_GET_LONG_REGISTER,
+					HIDPP_REG_PAIRING_INFORMATION,
+					params, 1, &response);
+	if (ret)
+		return ret;
+
+	/*
+	 * We don't care about LE or BE, we will output it as a string
+	 * with %4phD, so we need to keep the order.
+	 */
+	*serial = *((u32 *)&response.rap.params[1]);
+	return 0;
+}
+
+static int hidpp_unifying_init(struct hidpp_device *hidpp)
+{
+	struct hid_device *hdev = hidpp->hid_dev;
+	const char *name;
+	u32 serial;
+	int ret;
+
+	ret = hidpp_unifying_get_serial(hidpp, &serial);
+	if (ret)
+		return ret;
+
+	snprintf(hdev->uniq, sizeof(hdev->uniq), "%04x-%4phD",
+		 hdev->product, &serial);
+	dbg_hid("HID++ Unifying: Got serial: %s\n", hdev->uniq);
+
+	name = hidpp_unifying_get_name(hidpp);
+	if (!name)
+		return -EIO;
+
+	snprintf(hdev->name, sizeof(hdev->name), "%s", name);
+	dbg_hid("HID++ Unifying: Got name: %s\n", name);
+
+	kfree(name);
+	return 0;
+}
+
 /* -------------------------------------------------------------------------- */
 /* 0x0000: Root                                                               */
 /* -------------------------------------------------------------------------- */
@@ -2299,22 +2348,15 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report,
 	return 0;
 }
 
-static void hidpp_overwrite_name(struct hid_device *hdev, bool use_unifying)
+static void hidpp_overwrite_name(struct hid_device *hdev)
 {
 	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
 	char *name;
 
-	if (use_unifying)
-		/*
-		 * the device is connected through an Unifying receiver, and
-		 * might not be already connected.
-		 * Ask the receiver for its name.
-		 */
-		name = hidpp_get_unifying_name(hidpp);
-	else if (hidpp->protocol_major < 2)
+	if (hidpp->protocol_major < 2)
 		return;
-	else
-		name = hidpp_get_device_name(hidpp);
+
+	name = hidpp_get_device_name(hidpp);
 
 	if (!name) {
 		hid_err(hdev, "unable to retrieve the name of the device");
@@ -2456,6 +2498,9 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 	hidpp->quirks = id->driver_data;
 
+	if (id->group == HID_GROUP_LOGITECH_DJ_DEVICE)
+		hidpp->quirks |= HIDPP_QUIRK_UNIFYING;
+
 	if (disable_raw_mode) {
 		hidpp->quirks &= ~HIDPP_QUIRK_CLASS_WTP;
 		hidpp->quirks &= ~HIDPP_QUIRK_NO_HIDINPUT;
@@ -2507,8 +2552,12 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	/* Allow incoming packets */
 	hid_device_io_start(hdev);
 
+	if (hidpp->quirks & HIDPP_QUIRK_UNIFYING)
+		hidpp_unifying_init(hidpp);
+
 	connected = hidpp_is_connected(hidpp);
-	if (id->group != HID_GROUP_LOGITECH_DJ_DEVICE) {
+	atomic_set(&hidpp->connected, connected);
+	if (!(hidpp->quirks & HIDPP_QUIRK_UNIFYING)) {
 		if (!connected) {
 			ret = -ENODEV;
 			hid_err(hdev, "Device not connected");
@@ -2517,10 +2566,9 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 		hid_info(hdev, "HID++ %u.%u device connected.\n",
 			 hidpp->protocol_major, hidpp->protocol_minor);
-	}
 
-	hidpp_overwrite_name(hdev, id->group == HID_GROUP_LOGITECH_DJ_DEVICE);
-	atomic_set(&hidpp->connected, connected);
+		hidpp_overwrite_name(hdev);
+	}
 
 	if (connected && (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)) {
 		ret = wtp_get_config(hidpp);
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1609934 — [PATCH v3 09/19] HID: logitech-hidpp: handle battery events in hidpp_raw_hidpp_event()

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2017-03-27 17:10 +0200
Subject[PATCH v3 09/19] HID: logitech-hidpp: handle battery events in hidpp_raw_hidpp_event()
Message-ID<tpErq-1wA-61@gated-at.bofh.it>
In reply to#1609923
Battery events are reported through HID++, so we need to be sure
the report ID is the HID++ one.

Without this, we might receive keyboard events that looks just like
battery events with wrong data and which will confuse user space.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

new in v3
---
 drivers/hid/hid-logitech-hidpp.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 81ebded..3e2f716 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -2250,6 +2250,7 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
 	struct hidpp_report *question = hidpp->send_receive_buf;
 	struct hidpp_report *answer = hidpp->send_receive_buf;
 	struct hidpp_report *report = (struct hidpp_report *)data;
+	int ret;
 
 	/*
 	 * If the mutex is locked then we have a pending answer from a
@@ -2283,6 +2284,12 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
 		return 1;
 	}
 
+	if (hidpp->capabilities & HIDPP_CAPABILITY_HIDPP20_BATTERY) {
+		ret = hidpp20_battery_event(hidpp, data, size);
+		if (ret != 0)
+			return ret;
+	}
+
 	return 0;
 }
 
@@ -2325,12 +2332,6 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report,
 	if (ret != 0)
 		return ret;
 
-	if (hidpp->capabilities & HIDPP_CAPABILITY_HIDPP20_BATTERY) {
-		ret = hidpp20_battery_event(hidpp, data, size);
-		if (ret != 0)
-			return ret;
-	}
-
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)
 		return wtp_raw_event(hdev, data, size);
 	else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1609936 — [PATCH v3 13/19] HID: logitech-hidpp: notify battery on connect

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2017-03-27 17:10 +0200
Subject[PATCH v3 13/19] HID: logitech-hidpp: notify battery on connect
Message-ID<tpErq-1wA-57@gated-at.bofh.it>
In reply to#1609923
When a device reconnects, there is a high chance its power supply has
been changed (for a battery replacement for instance). Just forward
the battery state here.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

changes in v3:
- moved up in the series

no changes in v2
---
 drivers/hid/hid-logitech-hidpp.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 22129dd..0781d2b 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -2463,6 +2463,13 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
 
 	hidpp_initialize_battery(hidpp);
 
+	/* forward current battery state */
+	if (hidpp->capabilities & HIDPP_CAPABILITY_HIDPP20_BATTERY) {
+		hidpp20_query_battery_info(hidpp);
+		if (hidpp->battery.ps)
+			power_supply_changed(hidpp->battery.ps);
+	}
+
 	if (!(hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT) || hidpp->delayed_input)
 		/* if the input nodes are already created, we can stop now */
 		return;
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1609972

FromBastien Nocera <hadess@hadess.net>
Date2017-03-27 18:30 +0200
Message-ID<tpFGN-2pv-3@gated-at.bofh.it>
In reply to#1609923
On Mon, 2017-03-27 at 16:59 +0200, Benjamin Tissoires wrote:
> Hi,
> 
> this is finally a rework of the series that provides kernel
> power_supply
> for hidpp devices.
> 
> This will allow upower to not handle those devices anymore and to
> have more
> immediate reportng of the device to the system.

You can add my Tested-by to all the commits in this patchset, using a
K750 solar keyboard and a T650 touchpad.

Note that user-space support work is still ongoing, on top of the
current upower master:
https://bugs.freedesktop.org/show_bug.cgi?id=100359

Cheers

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web