linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/1] HID: Add support for multiple batteries per device
@ 2025-11-11 10:56 Lucas Zampieri
  2025-11-11 10:56 ` [RFC PATCH 1/1] HID: input: " Lucas Zampieri
  0 siblings, 1 reply; 4+ messages in thread
From: Lucas Zampieri @ 2025-11-11 10:56 UTC (permalink / raw)
  To: linux-input
  Cc: Lucas Zampieri, linux-kernel, Jiri Kosina, Benjamin Tissoires,
	Sebastian Reichel, linux-pm

This RFC introduces support for multiple batteries per HID device, addressing
a long-standing architectural limitation in the HID battery reporting subsystem.

## Background

The current HID implementation explicitly prevents multiple batteries per device
through an early return in hidinput_setup_battery() that enforces a single-battery
assumption. Linux treats peripheral batteries (scope=Device) differently from system
batteries, with desktop environments often displaying them separately or ignoring
them entirely. However, this design doesn't account for modern multi-battery hardware patterns.

## Problem Statement

Multiple battery scenarios that cannot be properly reported today:

1. Gaming headsets with charging docks (e.g., SteelSeries Arctis Nova Pro
   Wireless) - headset battery reported, dock battery invisible
2. Graphics tablets with stylus batteries (Wacom) - requires driver-specific
   workarounds
3. Wireless earbuds with per-earbud batteries plus charging case
4. Multi-device receivers (Logitech Unifying) - requires proprietary HID++
   protocol parsing

This forces manufacturers to use proprietary protocols and vendor-specific
software. Community projects parse USB packets directly because standard HID
battery reporting cannot handle multi-battery scenarios.

## Why This Matters

The current limitation creates a cycle: OS lacks support, so manufacturers
implement proprietary protocols, which makes vendor software necessary, which
reduces pressure to fix the OS limitation. Improving HID core support for
multiple batteries would enable standardized reporting, reduce the need for
vendor software, improve OS integration, reduce driver duplication, and provide
a foundation for future multi-battery devices.

## Proposed Solution

This patch introduces struct hid_battery to encapsulate individual battery
state, adds a batteries list to struct hid_device for tracking multiple
batteries, and uses report ID-based identification. The implementation maintains
full backwards compatibility with existing single-battery code.

## Testing

Tested with split keyboard hardware. Each battery reports independently
through the power supply interface.

## Request for Comments

Is list-based storage appropriate or would another structure work better?
Should we support usage-based identification in addition to report ID for
devices using the same report ID? Is sequential naming (battery-N) sufficient
or should batteries have semantic role identifiers like "main", "stylus", "dock"?

To HID maintainers (Jiri Kosina, Benjamin Tissoires): Does this belong in
hid-input.c or should it be separate? Any concerns about the backwards
compatibility approach? Meaning, should I have removed the whole
dev->bat legacy mapping and use the new struct?

To power supply maintainers (Sebastian Reichel): Any issues with multiple
power_supply devices from a single HID device?

Related commits:
- c6838eeef2fb: HID: hid-input: occasionally report stylus battery
- a608dc1c0639: HID: input: map battery system charging
- fd2a9b29dc9c: HID: wacom: Remove AES power_supply after inactivity

Community projects demonstrating the need:
- HeadsetControl: https://github.com/Sapd/HeadsetControl
- Solaar: https://github.com/pwr-Solaar/Solaar
- OpenRazer: https://github.com/openrazer/openrazer

Lucas Zampieri (1):
  HID: input: Add support for multiple batteries per device

 drivers/hid/hid-core.c  |   4 +
 drivers/hid/hid-input.c | 193 +++++++++++++++++++++++++++-------------
 include/linux/hid.h     |  42 ++++++++-
 3 files changed, 176 insertions(+), 63 deletions(-)

--
2.51.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [RFC PATCH 1/1] HID: input: Add support for multiple batteries per device
  2025-11-11 10:56 [RFC PATCH 0/1] HID: Add support for multiple batteries per device Lucas Zampieri
@ 2025-11-11 10:56 ` Lucas Zampieri
  2025-11-12 14:15   ` Bastien Nocera
  0 siblings, 1 reply; 4+ messages in thread
From: Lucas Zampieri @ 2025-11-11 10:56 UTC (permalink / raw)
  To: linux-input
  Cc: Lucas Zampieri, linux-kernel, Jiri Kosina, Benjamin Tissoires,
	Sebastian Reichel, linux-pm

Add support for multiple batteries per HID device by introducing
struct hid_battery to encapsulate individual battery state and using
a list to track multiple batteries identified by report ID. The legacy
dev->battery field is maintained for backwards compatibility.

Signed-off-by: Lucas Zampieri <lzampier@redhat.com>
---
 drivers/hid/hid-core.c  |   4 +
 drivers/hid/hid-input.c | 193 +++++++++++++++++++++++++++-------------
 include/linux/hid.h     |  42 ++++++++-
 3 files changed, 176 insertions(+), 63 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index a5b3a8ca2fcb..76d628547e9a 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2990,6 +2990,10 @@ struct hid_device *hid_allocate_device(void)
 	mutex_init(&hdev->ll_open_lock);
 	kref_init(&hdev->ref);
 
+#ifdef CONFIG_HID_BATTERY_STRENGTH
+	INIT_LIST_HEAD(&hdev->batteries);
+#endif
+
 	ret = hid_bpf_device_init(hdev);
 	if (ret)
 		goto out_err;
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index e56e7de53279..071df319775b 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -454,7 +454,8 @@ static int hidinput_get_battery_property(struct power_supply *psy,
 					 enum power_supply_property prop,
 					 union power_supply_propval *val)
 {
-	struct hid_device *dev = power_supply_get_drvdata(psy);
+	struct hid_battery *bat = power_supply_get_drvdata(psy);
+	struct hid_device *dev = bat->dev;
 	int value;
 	int ret = 0;
 
@@ -465,13 +466,13 @@ static int hidinput_get_battery_property(struct power_supply *psy,
 		break;
 
 	case POWER_SUPPLY_PROP_CAPACITY:
-		if (dev->battery_status != HID_BATTERY_REPORTED &&
-		    !dev->battery_avoid_query) {
+		if (bat->status != HID_BATTERY_REPORTED &&
+		    !bat->avoid_query) {
 			value = hidinput_query_battery_capacity(dev);
 			if (value < 0)
 				return value;
 		} else  {
-			value = dev->battery_capacity;
+			value = bat->capacity;
 		}
 
 		val->intval = value;
@@ -482,20 +483,20 @@ static int hidinput_get_battery_property(struct power_supply *psy,
 		break;
 
 	case POWER_SUPPLY_PROP_STATUS:
-		if (dev->battery_status != HID_BATTERY_REPORTED &&
-		    !dev->battery_avoid_query) {
+		if (bat->status != HID_BATTERY_REPORTED &&
+		    !bat->avoid_query) {
 			value = hidinput_query_battery_capacity(dev);
 			if (value < 0)
 				return value;
 
-			dev->battery_capacity = value;
-			dev->battery_status = HID_BATTERY_QUERIED;
+			bat->capacity = value;
+			bat->status = HID_BATTERY_QUERIED;
 		}
 
-		if (dev->battery_status == HID_BATTERY_UNKNOWN)
+		if (bat->status == HID_BATTERY_UNKNOWN)
 			val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
 		else
-			val->intval = dev->battery_charge_status;
+			val->intval = bat->charge_status;
 		break;
 
 	case POWER_SUPPLY_PROP_SCOPE:
@@ -513,33 +514,53 @@ static int hidinput_get_battery_property(struct power_supply *psy,
 static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
 				  struct hid_field *field, bool is_percentage)
 {
+	struct hid_battery *bat;
 	struct power_supply_desc *psy_desc;
-	struct power_supply_config psy_cfg = { .drv_data = dev, };
+	struct power_supply_config psy_cfg;
 	unsigned quirks;
 	s32 min, max;
 	int error;
+	int battery_num = 0;
 
-	if (dev->battery)
-		return 0;	/* already initialized? */
+	list_for_each_entry(bat, &dev->batteries, list) {
+		if (bat->report_id == field->report->id)
+			return 0;	/* already initialized */
+		battery_num++;
+	}
 
 	quirks = find_battery_quirk(dev);
 
-	hid_dbg(dev, "device %x:%x:%x %d quirks %d\n",
-		dev->bus, dev->vendor, dev->product, dev->version, quirks);
+	hid_dbg(dev, "device %x:%x:%x %d quirks %d report_id %d\n",
+		dev->bus, dev->vendor, dev->product, dev->version, quirks,
+		field->report->id);
 
 	if (quirks & HID_BATTERY_QUIRK_IGNORE)
 		return 0;
 
-	psy_desc = kzalloc(sizeof(*psy_desc), GFP_KERNEL);
-	if (!psy_desc)
+	bat = kzalloc(sizeof(*bat), GFP_KERNEL);
+	if (!bat)
 		return -ENOMEM;
 
-	psy_desc->name = kasprintf(GFP_KERNEL, "hid-%s-battery",
-				   strlen(dev->uniq) ?
-					dev->uniq : dev_name(&dev->dev));
+	psy_desc = kzalloc(sizeof(*psy_desc), GFP_KERNEL);
+	if (!psy_desc) {
+		error = -ENOMEM;
+		goto err_free_bat;
+	}
+
+	/* Create unique name for each battery based on report ID */
+	if (battery_num == 0) {
+		psy_desc->name = kasprintf(GFP_KERNEL, "hid-%s-battery",
+					   strlen(dev->uniq) ?
+						dev->uniq : dev_name(&dev->dev));
+	} else {
+		psy_desc->name = kasprintf(GFP_KERNEL, "hid-%s-battery-%d",
+					   strlen(dev->uniq) ?
+						dev->uniq : dev_name(&dev->dev),
+					   battery_num);
+	}
 	if (!psy_desc->name) {
 		error = -ENOMEM;
-		goto err_free_mem;
+		goto err_free_desc;
 	}
 
 	psy_desc->type = POWER_SUPPLY_TYPE_BATTERY;
@@ -559,98 +580,148 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
 	if (quirks & HID_BATTERY_QUIRK_FEATURE)
 		report_type = HID_FEATURE_REPORT;
 
-	dev->battery_min = min;
-	dev->battery_max = max;
-	dev->battery_report_type = report_type;
-	dev->battery_report_id = field->report->id;
-	dev->battery_charge_status = POWER_SUPPLY_STATUS_DISCHARGING;
+	/* Initialize battery structure */
+	bat->dev = dev;
+	bat->min = min;
+	bat->max = max;
+	bat->report_type = report_type;
+	bat->report_id = field->report->id;
+	bat->charge_status = POWER_SUPPLY_STATUS_DISCHARGING;
+	bat->status = HID_BATTERY_UNKNOWN;
 
 	/*
 	 * Stylus is normally not connected to the device and thus we
 	 * can't query the device and get meaningful battery strength.
 	 * We have to wait for the device to report it on its own.
 	 */
-	dev->battery_avoid_query = report_type == HID_INPUT_REPORT &&
-				   field->physical == HID_DG_STYLUS;
+	bat->avoid_query = report_type == HID_INPUT_REPORT &&
+			   field->physical == HID_DG_STYLUS;
 
 	if (quirks & HID_BATTERY_QUIRK_AVOID_QUERY)
-		dev->battery_avoid_query = true;
+		bat->avoid_query = true;
 
-	dev->battery = power_supply_register(&dev->dev, psy_desc, &psy_cfg);
-	if (IS_ERR(dev->battery)) {
-		error = PTR_ERR(dev->battery);
+	psy_cfg.drv_data = bat;
+	bat->ps = power_supply_register(&dev->dev, psy_desc, &psy_cfg);
+	if (IS_ERR(bat->ps)) {
+		error = PTR_ERR(bat->ps);
 		hid_warn(dev, "can't register power supply: %d\n", error);
 		goto err_free_name;
 	}
 
-	power_supply_powers(dev->battery, &dev->dev);
+	power_supply_powers(bat->ps, &dev->dev);
+
+	list_add_tail(&bat->list, &dev->batteries);
+
+	/*
+	 * The legacy single battery API is preserved by exposing the first
+	 * discovered battery. Systems relying on a single battery view maintain
+	 * unchanged behavior.
+	 */
+	if (battery_num == 0) {
+		dev->battery = bat->ps;
+		dev->battery_min = bat->min;
+		dev->battery_max = bat->max;
+		dev->battery_report_type = bat->report_type;
+		dev->battery_report_id = bat->report_id;
+		dev->battery_charge_status = bat->charge_status;
+		dev->battery_status = bat->status;
+		dev->battery_avoid_query = bat->avoid_query;
+	}
+
 	return 0;
 
 err_free_name:
 	kfree(psy_desc->name);
-err_free_mem:
+err_free_desc:
 	kfree(psy_desc);
-	dev->battery = NULL;
+err_free_bat:
+	kfree(bat);
 	return error;
 }
 
 static void hidinput_cleanup_battery(struct hid_device *dev)
 {
+	struct hid_battery *bat, *next;
 	const struct power_supply_desc *psy_desc;
 
-	if (!dev->battery)
-		return;
+	list_for_each_entry_safe(bat, next, &dev->batteries, list) {
+		psy_desc = bat->ps->desc;
+		power_supply_unregister(bat->ps);
+		kfree(psy_desc->name);
+		kfree(psy_desc);
+		list_del(&bat->list);
+		kfree(bat);
+	}
 
-	psy_desc = dev->battery->desc;
-	power_supply_unregister(dev->battery);
-	kfree(psy_desc->name);
-	kfree(psy_desc);
 	dev->battery = NULL;
 }
 
-static bool hidinput_update_battery_charge_status(struct hid_device *dev,
+static struct hid_battery *hidinput_find_battery(struct hid_device *dev,
+						 int report_id)
+{
+	struct hid_battery *bat;
+
+	list_for_each_entry(bat, &dev->batteries, list) {
+		if (bat->report_id == report_id)
+			return bat;
+	}
+	return NULL;
+}
+
+static bool hidinput_update_battery_charge_status(struct hid_battery *bat,
 						  unsigned int usage, int value)
 {
 	switch (usage) {
 	case HID_BAT_CHARGING:
-		dev->battery_charge_status = value ?
-					     POWER_SUPPLY_STATUS_CHARGING :
-					     POWER_SUPPLY_STATUS_DISCHARGING;
+		bat->charge_status = value ?
+				     POWER_SUPPLY_STATUS_CHARGING :
+				     POWER_SUPPLY_STATUS_DISCHARGING;
+		if (bat->dev->battery == bat->ps)
+			bat->dev->battery_charge_status = bat->charge_status;
 		return true;
 	}
 
 	return false;
 }
 
-static void hidinput_update_battery(struct hid_device *dev, unsigned int usage,
-				    int value)
+static void hidinput_update_battery(struct hid_device *dev, int report_id,
+				    unsigned int usage, int value)
 {
+	struct hid_battery *bat;
 	int capacity;
 
-	if (!dev->battery)
+	bat = hidinput_find_battery(dev, report_id);
+	if (!bat)
 		return;
 
-	if (hidinput_update_battery_charge_status(dev, usage, value)) {
-		power_supply_changed(dev->battery);
+	if (hidinput_update_battery_charge_status(bat, usage, value)) {
+		power_supply_changed(bat->ps);
 		return;
 	}
 
 	if ((usage & HID_USAGE_PAGE) == HID_UP_DIGITIZER && value == 0)
 		return;
 
-	if (value < dev->battery_min || value > dev->battery_max)
+	if (value < bat->min || value > bat->max)
 		return;
 
 	capacity = hidinput_scale_battery_capacity(dev, value);
 
-	if (dev->battery_status != HID_BATTERY_REPORTED ||
-	    capacity != dev->battery_capacity ||
-	    ktime_after(ktime_get_coarse(), dev->battery_ratelimit_time)) {
-		dev->battery_capacity = capacity;
-		dev->battery_status = HID_BATTERY_REPORTED;
-		dev->battery_ratelimit_time =
+	if (bat->status != HID_BATTERY_REPORTED ||
+	    capacity != bat->capacity ||
+	    ktime_after(ktime_get_coarse(), bat->ratelimit_time)) {
+		bat->capacity = capacity;
+		bat->status = HID_BATTERY_REPORTED;
+		bat->ratelimit_time =
 			ktime_add_ms(ktime_get_coarse(), 30 * 1000);
-		power_supply_changed(dev->battery);
+
+		if (dev->battery == bat->ps) {
+			dev->battery_capacity = bat->capacity;
+			dev->battery_status = bat->status;
+			dev->battery_ratelimit_time = bat->ratelimit_time;
+		}
+
+		power_supply_changed(bat->ps);
 	}
 }
 #else  /* !CONFIG_HID_BATTERY_STRENGTH */
@@ -664,8 +735,8 @@ static void hidinput_cleanup_battery(struct hid_device *dev)
 {
 }
 
-static void hidinput_update_battery(struct hid_device *dev, unsigned int usage,
-				    int value)
+static void hidinput_update_battery(struct hid_device *dev, int report_id,
+				    unsigned int usage, int value)
 {
 }
 #endif	/* CONFIG_HID_BATTERY_STRENGTH */
@@ -1533,7 +1604,7 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
 		return;
 
 	if (usage->type == EV_PWR) {
-		hidinput_update_battery(hid, usage->hid, value);
+		hidinput_update_battery(hid, report->id, usage->hid, value);
 		return;
 	}
 
diff --git a/include/linux/hid.h b/include/linux/hid.h
index a4ddb94e3ee5..a6e36835fb3c 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -634,6 +634,36 @@ enum hid_battery_status {
 	HID_BATTERY_REPORTED,		/* Device sent unsolicited battery strength report */
 };
 
+/**
+ * struct hid_battery - represents a single battery power supply
+ * @list: list node for linking into hid_device's battery list
+ * @dev: pointer to the parent hid_device
+ * @ps: the power supply device
+ * @capacity: current battery capacity
+ * @min: minimum battery value
+ * @max: maximum battery value
+ * @report_type: type of report (HID_INPUT_REPORT, HID_FEATURE_REPORT)
+ * @report_id: report ID for this battery
+ * @charge_status: current charge status
+ * @status: battery status (unknown, queried, reported)
+ * @avoid_query: if true, don't query battery (wait for device reports)
+ * @ratelimit_time: time for rate limiting battery updates
+ */
+struct hid_battery {
+	struct list_head list;
+	struct hid_device *dev;
+	struct power_supply *ps;
+	__s32 capacity;
+	__s32 min;
+	__s32 max;
+	__s32 report_type;
+	__s32 report_id;
+	__s32 charge_status;
+	enum hid_battery_status status;
+	bool avoid_query;
+	ktime_t ratelimit_time;
+};
+
 struct hid_driver;
 struct hid_ll_driver;
 
@@ -670,8 +700,16 @@ struct hid_device {
 #ifdef CONFIG_HID_BATTERY_STRENGTH
 	/*
 	 * Power supply information for HID devices which report
-	 * battery strength. power_supply was successfully registered if
-	 * battery is non-NULL.
+	 * battery strength. Each battery is tracked separately in the
+	 * batteries list.
+	 */
+	struct list_head batteries;		/* List of hid_battery structures */
+
+	/*
+	 * Legacy single battery support - kept for backwards compatibility.
+	 * Points to the first battery in the list if any exists.
+	 * power_supply was successfully registered if battery is non-NULL.
+	 * DEPRECATED: New code should iterate through batteries list instead.
 	 */
 	struct power_supply *battery;
 	__s32 battery_capacity;
-- 
2.51.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH 1/1] HID: input: Add support for multiple batteries per device
  2025-11-11 10:56 ` [RFC PATCH 1/1] HID: input: " Lucas Zampieri
@ 2025-11-12 14:15   ` Bastien Nocera
  2025-11-12 21:36     ` Lucas Zampieri
  0 siblings, 1 reply; 4+ messages in thread
From: Bastien Nocera @ 2025-11-12 14:15 UTC (permalink / raw)
  To: Lucas Zampieri, linux-input
  Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Sebastian Reichel,
	linux-pm

Hey Lucas,

(Follow-up to a chat we had about this patch privately)

On Tue, 2025-11-11 at 10:56 +0000, Lucas Zampieri wrote:
> Add support for multiple batteries per HID device by introducing
> struct hid_battery to encapsulate individual battery state and using
> a list to track multiple batteries identified by report ID. The
> legacy
> dev->battery field is maintained for backwards compatibility.

The cover letter mentions specific hardware, you probably want to
mention this in the commit message itself, as the cover letter will be
disconnected from this commit once this gets merged. Don't hesitate to
link to product pages directly if you want to show specific products as
potential users of that capability.

You mentioned that you tested this patchset with a custom firmware for
a split keyboard. It would be great if the firmware could be made
available to show how this was tested and mention that in the commit
message.

bentiss will also likely want a hid-recorder output for the device that
shows the batteries being instantiated. This would also likely be used
to test whether upower continues working as expected.

Talking of upower, I think we'll need an systemd/hwdb + upower changes
to differentiate batteries within a single device, as I don't think we
can have enough metadata in the HID report to differentiate them.

Last comment about the patch itself, do you think it would be feasible
to split this in 2 or 3? One to introduce the hid_battery struct,
another to use it to replace direct power_supply access, and finally
one to allow a list of hid_batteries?

Don't hesitate to CC: on future versions.

Cheers

> 
> Signed-off-by: Lucas Zampieri <lzampier@redhat.com>
> ---
>  drivers/hid/hid-core.c  |   4 +
>  drivers/hid/hid-input.c | 193 +++++++++++++++++++++++++++-----------
> --
>  include/linux/hid.h     |  42 ++++++++-
>  3 files changed, 176 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index a5b3a8ca2fcb..76d628547e9a 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -2990,6 +2990,10 @@ struct hid_device *hid_allocate_device(void)
>  	mutex_init(&hdev->ll_open_lock);
>  	kref_init(&hdev->ref);
>  
> +#ifdef CONFIG_HID_BATTERY_STRENGTH
> +	INIT_LIST_HEAD(&hdev->batteries);
> +#endif
> +
>  	ret = hid_bpf_device_init(hdev);
>  	if (ret)
>  		goto out_err;
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index e56e7de53279..071df319775b 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -454,7 +454,8 @@ static int hidinput_get_battery_property(struct
> power_supply *psy,
>  					 enum power_supply_property
> prop,
>  					 union power_supply_propval
> *val)
>  {
> -	struct hid_device *dev = power_supply_get_drvdata(psy);
> +	struct hid_battery *bat = power_supply_get_drvdata(psy);
> +	struct hid_device *dev = bat->dev;
>  	int value;
>  	int ret = 0;
>  
> @@ -465,13 +466,13 @@ static int hidinput_get_battery_property(struct
> power_supply *psy,
>  		break;
>  
>  	case POWER_SUPPLY_PROP_CAPACITY:
> -		if (dev->battery_status != HID_BATTERY_REPORTED &&
> -		    !dev->battery_avoid_query) {
> +		if (bat->status != HID_BATTERY_REPORTED &&
> +		    !bat->avoid_query) {
>  			value =
> hidinput_query_battery_capacity(dev);
>  			if (value < 0)
>  				return value;
>  		} else  {
> -			value = dev->battery_capacity;
> +			value = bat->capacity;
>  		}
>  
>  		val->intval = value;
> @@ -482,20 +483,20 @@ static int hidinput_get_battery_property(struct
> power_supply *psy,
>  		break;
>  
>  	case POWER_SUPPLY_PROP_STATUS:
> -		if (dev->battery_status != HID_BATTERY_REPORTED &&
> -		    !dev->battery_avoid_query) {
> +		if (bat->status != HID_BATTERY_REPORTED &&
> +		    !bat->avoid_query) {
>  			value =
> hidinput_query_battery_capacity(dev);
>  			if (value < 0)
>  				return value;
>  
> -			dev->battery_capacity = value;
> -			dev->battery_status = HID_BATTERY_QUERIED;
> +			bat->capacity = value;
> +			bat->status = HID_BATTERY_QUERIED;
>  		}
>  
> -		if (dev->battery_status == HID_BATTERY_UNKNOWN)
> +		if (bat->status == HID_BATTERY_UNKNOWN)
>  			val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
>  		else
> -			val->intval = dev->battery_charge_status;
> +			val->intval = bat->charge_status;
>  		break;
>  
>  	case POWER_SUPPLY_PROP_SCOPE:
> @@ -513,33 +514,53 @@ static int hidinput_get_battery_property(struct
> power_supply *psy,
>  static int hidinput_setup_battery(struct hid_device *dev, unsigned
> report_type,
>  				  struct hid_field *field, bool
> is_percentage)
>  {
> +	struct hid_battery *bat;
>  	struct power_supply_desc *psy_desc;
> -	struct power_supply_config psy_cfg = { .drv_data = dev, };
> +	struct power_supply_config psy_cfg;
>  	unsigned quirks;
>  	s32 min, max;
>  	int error;
> +	int battery_num = 0;
>  
> -	if (dev->battery)
> -		return 0;	/* already initialized? */
> +	list_for_each_entry(bat, &dev->batteries, list) {
> +		if (bat->report_id == field->report->id)
> +			return 0;	/* already initialized */
> +		battery_num++;
> +	}
>  
>  	quirks = find_battery_quirk(dev);
>  
> -	hid_dbg(dev, "device %x:%x:%x %d quirks %d\n",
> -		dev->bus, dev->vendor, dev->product, dev->version,
> quirks);
> +	hid_dbg(dev, "device %x:%x:%x %d quirks %d report_id %d\n",
> +		dev->bus, dev->vendor, dev->product, dev->version,
> quirks,
> +		field->report->id);
>  
>  	if (quirks & HID_BATTERY_QUIRK_IGNORE)
>  		return 0;
>  
> -	psy_desc = kzalloc(sizeof(*psy_desc), GFP_KERNEL);
> -	if (!psy_desc)
> +	bat = kzalloc(sizeof(*bat), GFP_KERNEL);
> +	if (!bat)
>  		return -ENOMEM;
>  
> -	psy_desc->name = kasprintf(GFP_KERNEL, "hid-%s-battery",
> -				   strlen(dev->uniq) ?
> -					dev->uniq : dev_name(&dev-
> >dev));
> +	psy_desc = kzalloc(sizeof(*psy_desc), GFP_KERNEL);
> +	if (!psy_desc) {
> +		error = -ENOMEM;
> +		goto err_free_bat;
> +	}
> +
> +	/* Create unique name for each battery based on report ID */
> +	if (battery_num == 0) {
> +		psy_desc->name = kasprintf(GFP_KERNEL, "hid-%s-
> battery",
> +					   strlen(dev->uniq) ?
> +						dev->uniq :
> dev_name(&dev->dev));
> +	} else {
> +		psy_desc->name = kasprintf(GFP_KERNEL, "hid-%s-
> battery-%d",
> +					   strlen(dev->uniq) ?
> +						dev->uniq :
> dev_name(&dev->dev),
> +					   battery_num);
> +	}
>  	if (!psy_desc->name) {
>  		error = -ENOMEM;
> -		goto err_free_mem;
> +		goto err_free_desc;
>  	}
>  
>  	psy_desc->type = POWER_SUPPLY_TYPE_BATTERY;
> @@ -559,98 +580,148 @@ static int hidinput_setup_battery(struct
> hid_device *dev, unsigned report_type,
>  	if (quirks & HID_BATTERY_QUIRK_FEATURE)
>  		report_type = HID_FEATURE_REPORT;
>  
> -	dev->battery_min = min;
> -	dev->battery_max = max;
> -	dev->battery_report_type = report_type;
> -	dev->battery_report_id = field->report->id;
> -	dev->battery_charge_status =
> POWER_SUPPLY_STATUS_DISCHARGING;
> +	/* Initialize battery structure */
> +	bat->dev = dev;
> +	bat->min = min;
> +	bat->max = max;
> +	bat->report_type = report_type;
> +	bat->report_id = field->report->id;
> +	bat->charge_status = POWER_SUPPLY_STATUS_DISCHARGING;
> +	bat->status = HID_BATTERY_UNKNOWN;
>  
>  	/*
>  	 * Stylus is normally not connected to the device and thus
> we
>  	 * can't query the device and get meaningful battery
> strength.
>  	 * We have to wait for the device to report it on its own.
>  	 */
> -	dev->battery_avoid_query = report_type == HID_INPUT_REPORT
> &&
> -				   field->physical == HID_DG_STYLUS;
> +	bat->avoid_query = report_type == HID_INPUT_REPORT &&
> +			   field->physical == HID_DG_STYLUS;
>  
>  	if (quirks & HID_BATTERY_QUIRK_AVOID_QUERY)
> -		dev->battery_avoid_query = true;
> +		bat->avoid_query = true;
>  
> -	dev->battery = power_supply_register(&dev->dev, psy_desc,
> &psy_cfg);
> -	if (IS_ERR(dev->battery)) {
> -		error = PTR_ERR(dev->battery);
> +	psy_cfg.drv_data = bat;
> +	bat->ps = power_supply_register(&dev->dev, psy_desc,
> &psy_cfg);
> +	if (IS_ERR(bat->ps)) {
> +		error = PTR_ERR(bat->ps);
>  		hid_warn(dev, "can't register power supply: %d\n",
> error);
>  		goto err_free_name;
>  	}
>  
> -	power_supply_powers(dev->battery, &dev->dev);
> +	power_supply_powers(bat->ps, &dev->dev);
> +
> +	list_add_tail(&bat->list, &dev->batteries);
> +
> +	/*
> +	 * The legacy single battery API is preserved by exposing
> the first
> +	 * discovered battery. Systems relying on a single battery
> view maintain
> +	 * unchanged behavior.
> +	 */
> +	if (battery_num == 0) {
> +		dev->battery = bat->ps;
> +		dev->battery_min = bat->min;
> +		dev->battery_max = bat->max;
> +		dev->battery_report_type = bat->report_type;
> +		dev->battery_report_id = bat->report_id;
> +		dev->battery_charge_status = bat->charge_status;
> +		dev->battery_status = bat->status;
> +		dev->battery_avoid_query = bat->avoid_query;
> +	}
> +
>  	return 0;
>  
>  err_free_name:
>  	kfree(psy_desc->name);
> -err_free_mem:
> +err_free_desc:
>  	kfree(psy_desc);
> -	dev->battery = NULL;
> +err_free_bat:
> +	kfree(bat);
>  	return error;
>  }
>  
>  static void hidinput_cleanup_battery(struct hid_device *dev)
>  {
> +	struct hid_battery *bat, *next;
>  	const struct power_supply_desc *psy_desc;
>  
> -	if (!dev->battery)
> -		return;
> +	list_for_each_entry_safe(bat, next, &dev->batteries, list) {
> +		psy_desc = bat->ps->desc;
> +		power_supply_unregister(bat->ps);
> +		kfree(psy_desc->name);
> +		kfree(psy_desc);
> +		list_del(&bat->list);
> +		kfree(bat);
> +	}
>  
> -	psy_desc = dev->battery->desc;
> -	power_supply_unregister(dev->battery);
> -	kfree(psy_desc->name);
> -	kfree(psy_desc);
>  	dev->battery = NULL;
>  }
>  
> -static bool hidinput_update_battery_charge_status(struct hid_device
> *dev,
> +static struct hid_battery *hidinput_find_battery(struct hid_device
> *dev,
> +						 int report_id)
> +{
> +	struct hid_battery *bat;
> +
> +	list_for_each_entry(bat, &dev->batteries, list) {
> +		if (bat->report_id == report_id)
> +			return bat;
> +	}
> +	return NULL;
> +}
> +
> +static bool hidinput_update_battery_charge_status(struct hid_battery
> *bat,
>  						  unsigned int
> usage, int value)
>  {
>  	switch (usage) {
>  	case HID_BAT_CHARGING:
> -		dev->battery_charge_status = value ?
> -					    
> POWER_SUPPLY_STATUS_CHARGING :
> -					    
> POWER_SUPPLY_STATUS_DISCHARGING;
> +		bat->charge_status = value ?
> +				     POWER_SUPPLY_STATUS_CHARGING :
> +				    
> POWER_SUPPLY_STATUS_DISCHARGING;
> +		if (bat->dev->battery == bat->ps)
> +			bat->dev->battery_charge_status = bat-
> >charge_status;
>  		return true;
>  	}
>  
>  	return false;
>  }
>  
> -static void hidinput_update_battery(struct hid_device *dev, unsigned
> int usage,
> -				    int value)
> +static void hidinput_update_battery(struct hid_device *dev, int
> report_id,
> +				    unsigned int usage, int value)
>  {
> +	struct hid_battery *bat;
>  	int capacity;
>  
> -	if (!dev->battery)
> +	bat = hidinput_find_battery(dev, report_id);
> +	if (!bat)
>  		return;
>  
> -	if (hidinput_update_battery_charge_status(dev, usage,
> value)) {
> -		power_supply_changed(dev->battery);
> +	if (hidinput_update_battery_charge_status(bat, usage,
> value)) {
> +		power_supply_changed(bat->ps);
>  		return;
>  	}
>  
>  	if ((usage & HID_USAGE_PAGE) == HID_UP_DIGITIZER && value ==
> 0)
>  		return;
>  
> -	if (value < dev->battery_min || value > dev->battery_max)
> +	if (value < bat->min || value > bat->max)
>  		return;
>  
>  	capacity = hidinput_scale_battery_capacity(dev, value);
>  
> -	if (dev->battery_status != HID_BATTERY_REPORTED ||
> -	    capacity != dev->battery_capacity ||
> -	    ktime_after(ktime_get_coarse(), dev-
> >battery_ratelimit_time)) {
> -		dev->battery_capacity = capacity;
> -		dev->battery_status = HID_BATTERY_REPORTED;
> -		dev->battery_ratelimit_time =
> +	if (bat->status != HID_BATTERY_REPORTED ||
> +	    capacity != bat->capacity ||
> +	    ktime_after(ktime_get_coarse(), bat->ratelimit_time)) {
> +		bat->capacity = capacity;
> +		bat->status = HID_BATTERY_REPORTED;
> +		bat->ratelimit_time =
>  			ktime_add_ms(ktime_get_coarse(), 30 * 1000);
> -		power_supply_changed(dev->battery);
> +
> +		if (dev->battery == bat->ps) {
> +			dev->battery_capacity = bat->capacity;
> +			dev->battery_status = bat->status;
> +			dev->battery_ratelimit_time = bat-
> >ratelimit_time;
> +		}
> +
> +		power_supply_changed(bat->ps);
>  	}
>  }
>  #else  /* !CONFIG_HID_BATTERY_STRENGTH */
> @@ -664,8 +735,8 @@ static void hidinput_cleanup_battery(struct
> hid_device *dev)
>  {
>  }
>  
> -static void hidinput_update_battery(struct hid_device *dev, unsigned
> int usage,
> -				    int value)
> +static void hidinput_update_battery(struct hid_device *dev, int
> report_id,
> +				    unsigned int usage, int value)
>  {
>  }
>  #endif	/* CONFIG_HID_BATTERY_STRENGTH */
> @@ -1533,7 +1604,7 @@ void hidinput_hid_event(struct hid_device *hid,
> struct hid_field *field, struct
>  		return;
>  
>  	if (usage->type == EV_PWR) {
> -		hidinput_update_battery(hid, usage->hid, value);
> +		hidinput_update_battery(hid, report->id, usage->hid,
> value);
>  		return;
>  	}
>  
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index a4ddb94e3ee5..a6e36835fb3c 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -634,6 +634,36 @@ enum hid_battery_status {
>  	HID_BATTERY_REPORTED,		/* Device sent unsolicited
> battery strength report */
>  };
>  
> +/**
> + * struct hid_battery - represents a single battery power supply
> + * @list: list node for linking into hid_device's battery list
> + * @dev: pointer to the parent hid_device
> + * @ps: the power supply device
> + * @capacity: current battery capacity
> + * @min: minimum battery value
> + * @max: maximum battery value
> + * @report_type: type of report (HID_INPUT_REPORT,
> HID_FEATURE_REPORT)
> + * @report_id: report ID for this battery
> + * @charge_status: current charge status
> + * @status: battery status (unknown, queried, reported)
> + * @avoid_query: if true, don't query battery (wait for device
> reports)
> + * @ratelimit_time: time for rate limiting battery updates
> + */
> +struct hid_battery {
> +	struct list_head list;
> +	struct hid_device *dev;
> +	struct power_supply *ps;
> +	__s32 capacity;
> +	__s32 min;
> +	__s32 max;
> +	__s32 report_type;
> +	__s32 report_id;
> +	__s32 charge_status;
> +	enum hid_battery_status status;
> +	bool avoid_query;
> +	ktime_t ratelimit_time;
> +};
> +
>  struct hid_driver;
>  struct hid_ll_driver;
>  
> @@ -670,8 +700,16 @@ struct hid_device {
>  #ifdef CONFIG_HID_BATTERY_STRENGTH
>  	/*
>  	 * Power supply information for HID devices which report
> -	 * battery strength. power_supply was successfully
> registered if
> -	 * battery is non-NULL.
> +	 * battery strength. Each battery is tracked separately in
> the
> +	 * batteries list.
> +	 */
> +	struct list_head batteries;		/* List of
> hid_battery structures */
> +
> +	/*
> +	 * Legacy single battery support - kept for backwards
> compatibility.
> +	 * Points to the first battery in the list if any exists.
> +	 * power_supply was successfully registered if battery is
> non-NULL.
> +	 * DEPRECATED: New code should iterate through batteries
> list instead.
>  	 */
>  	struct power_supply *battery;
>  	__s32 battery_capacity;

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH 1/1] HID: input: Add support for multiple batteries per device
  2025-11-12 14:15   ` Bastien Nocera
@ 2025-11-12 21:36     ` Lucas Zampieri
  0 siblings, 0 replies; 4+ messages in thread
From: Lucas Zampieri @ 2025-11-12 21:36 UTC (permalink / raw)
  To: Bastien Nocera
  Cc: linux-input, linux-kernel, Jiri Kosina, Benjamin Tissoires,
	Sebastian Reichel, linux-pm

Hey Bastien,

On Wed, Nov 12, 2025 at 2:46 PM Bastien Nocera <hadess@hadess.net> wrote:
>
> Hey Lucas,
>
> (Follow-up to a chat we had about this patch privately)
>
> On Tue, 2025-11-11 at 10:56 +0000, Lucas Zampieri wrote:
> > Add support for multiple batteries per HID device by introducing
> > struct hid_battery to encapsulate individual battery state and using
> > a list to track multiple batteries identified by report ID. The
> > legacy
> > dev->battery field is maintained for backwards compatibility.
>
> The cover letter mentions specific hardware, you probably want to
> mention this in the commit message itself, as the cover letter will be
> disconnected from this commit once this gets merged. Don't hesitate to
> link to product pages directly if you want to show specific products as
> potential users of that capability.
>
Got it, I'll update the commits in the v2 with that in mind.

> You mentioned that you tested this patchset with a custom firmware for
> a split keyboard. It would be great if the firmware could be made
> available to show how this was tested and mention that in the commit
> message.
>
I've pushed my custom firmware to
https://github.com/zampierilucas/zmk/tree/feat/individual-hid-battery-reporting,
if this series gets merged, I'll also propose that change to upstream
zmk project.
I'll also add links to in the v2 of the cover-letter

> bentiss will also likely want a hid-recorder output for the device that
> shows the batteries being instantiated. This would also likely be used
> to test whether upower continues working as expected.
>
Ack, I'll get the hid-recorder output and add to the testing section
of my cover letter.

> Talking of upower, I think we'll need an systemd/hwdb + upower changes
> to differentiate batteries within a single device, as I don't think we
> can have enough metadata in the HID report to differentiate them.
>
> Last comment about the patch itself, do you think it would be feasible
> to split this in 2 or 3? One to introduce the hid_battery struct,
> another to use it to replace direct power_supply access, and finally
> one to allow a list of hid_batteries?
>
> Don't hesitate to CC: on future versions.
>
For sure, thanks for the feedback

> Cheers
>
> >
> > Signed-off-by: Lucas Zampieri <lzampier@redhat.com>
> > ---
> >  drivers/hid/hid-core.c  |   4 +
> >  drivers/hid/hid-input.c | 193 +++++++++++++++++++++++++++-----------
> > --
> >  include/linux/hid.h     |  42 ++++++++-
> >  3 files changed, 176 insertions(+), 63 deletions(-)
> >
> > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> > index a5b3a8ca2fcb..76d628547e9a 100644
> > --- a/drivers/hid/hid-core.c
> > +++ b/drivers/hid/hid-core.c
> > @@ -2990,6 +2990,10 @@ struct hid_device *hid_allocate_device(void)
> >       mutex_init(&hdev->ll_open_lock);
> >       kref_init(&hdev->ref);
> >
> > +#ifdef CONFIG_HID_BATTERY_STRENGTH
> > +     INIT_LIST_HEAD(&hdev->batteries);
> > +#endif
> > +
> >       ret = hid_bpf_device_init(hdev);
> >       if (ret)
> >               goto out_err;
> > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> > index e56e7de53279..071df319775b 100644
> > --- a/drivers/hid/hid-input.c
> > +++ b/drivers/hid/hid-input.c
> > @@ -454,7 +454,8 @@ static int hidinput_get_battery_property(struct
> > power_supply *psy,
> >                                        enum power_supply_property
> > prop,
> >                                        union power_supply_propval
> > *val)
> >  {
> > -     struct hid_device *dev = power_supply_get_drvdata(psy);
> > +     struct hid_battery *bat = power_supply_get_drvdata(psy);
> > +     struct hid_device *dev = bat->dev;
> >       int value;
> >       int ret = 0;
> >
> > @@ -465,13 +466,13 @@ static int hidinput_get_battery_property(struct
> > power_supply *psy,
> >               break;
> >
> >       case POWER_SUPPLY_PROP_CAPACITY:
> > -             if (dev->battery_status != HID_BATTERY_REPORTED &&
> > -                 !dev->battery_avoid_query) {
> > +             if (bat->status != HID_BATTERY_REPORTED &&
> > +                 !bat->avoid_query) {
> >                       value =
> > hidinput_query_battery_capacity(dev);
> >                       if (value < 0)
> >                               return value;
> >               } else  {
> > -                     value = dev->battery_capacity;
> > +                     value = bat->capacity;
> >               }
> >
> >               val->intval = value;
> > @@ -482,20 +483,20 @@ static int hidinput_get_battery_property(struct
> > power_supply *psy,
> >               break;
> >
> >       case POWER_SUPPLY_PROP_STATUS:
> > -             if (dev->battery_status != HID_BATTERY_REPORTED &&
> > -                 !dev->battery_avoid_query) {
> > +             if (bat->status != HID_BATTERY_REPORTED &&
> > +                 !bat->avoid_query) {
> >                       value =
> > hidinput_query_battery_capacity(dev);
> >                       if (value < 0)
> >                               return value;
> >
> > -                     dev->battery_capacity = value;
> > -                     dev->battery_status = HID_BATTERY_QUERIED;
> > +                     bat->capacity = value;
> > +                     bat->status = HID_BATTERY_QUERIED;
> >               }
> >
> > -             if (dev->battery_status == HID_BATTERY_UNKNOWN)
> > +             if (bat->status == HID_BATTERY_UNKNOWN)
> >                       val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
> >               else
> > -                     val->intval = dev->battery_charge_status;
> > +                     val->intval = bat->charge_status;
> >               break;
> >
> >       case POWER_SUPPLY_PROP_SCOPE:
> > @@ -513,33 +514,53 @@ static int hidinput_get_battery_property(struct
> > power_supply *psy,
> >  static int hidinput_setup_battery(struct hid_device *dev, unsigned
> > report_type,
> >                                 struct hid_field *field, bool
> > is_percentage)
> >  {
> > +     struct hid_battery *bat;
> >       struct power_supply_desc *psy_desc;
> > -     struct power_supply_config psy_cfg = { .drv_data = dev, };
> > +     struct power_supply_config psy_cfg;
> >       unsigned quirks;
> >       s32 min, max;
> >       int error;
> > +     int battery_num = 0;
> >
> > -     if (dev->battery)
> > -             return 0;       /* already initialized? */
> > +     list_for_each_entry(bat, &dev->batteries, list) {
> > +             if (bat->report_id == field->report->id)
> > +                     return 0;       /* already initialized */
> > +             battery_num++;
> > +     }
> >
> >       quirks = find_battery_quirk(dev);
> >
> > -     hid_dbg(dev, "device %x:%x:%x %d quirks %d\n",
> > -             dev->bus, dev->vendor, dev->product, dev->version,
> > quirks);
> > +     hid_dbg(dev, "device %x:%x:%x %d quirks %d report_id %d\n",
> > +             dev->bus, dev->vendor, dev->product, dev->version,
> > quirks,
> > +             field->report->id);
> >
> >       if (quirks & HID_BATTERY_QUIRK_IGNORE)
> >               return 0;
> >
> > -     psy_desc = kzalloc(sizeof(*psy_desc), GFP_KERNEL);
> > -     if (!psy_desc)
> > +     bat = kzalloc(sizeof(*bat), GFP_KERNEL);
> > +     if (!bat)
> >               return -ENOMEM;
> >
> > -     psy_desc->name = kasprintf(GFP_KERNEL, "hid-%s-battery",
> > -                                strlen(dev->uniq) ?
> > -                                     dev->uniq : dev_name(&dev-
> > >dev));
> > +     psy_desc = kzalloc(sizeof(*psy_desc), GFP_KERNEL);
> > +     if (!psy_desc) {
> > +             error = -ENOMEM;
> > +             goto err_free_bat;
> > +     }
> > +
> > +     /* Create unique name for each battery based on report ID */
> > +     if (battery_num == 0) {
> > +             psy_desc->name = kasprintf(GFP_KERNEL, "hid-%s-
> > battery",
> > +                                        strlen(dev->uniq) ?
> > +                                             dev->uniq :
> > dev_name(&dev->dev));
> > +     } else {
> > +             psy_desc->name = kasprintf(GFP_KERNEL, "hid-%s-
> > battery-%d",
> > +                                        strlen(dev->uniq) ?
> > +                                             dev->uniq :
> > dev_name(&dev->dev),
> > +                                        battery_num);
> > +     }
> >       if (!psy_desc->name) {
> >               error = -ENOMEM;
> > -             goto err_free_mem;
> > +             goto err_free_desc;
> >       }
> >
> >       psy_desc->type = POWER_SUPPLY_TYPE_BATTERY;
> > @@ -559,98 +580,148 @@ static int hidinput_setup_battery(struct
> > hid_device *dev, unsigned report_type,
> >       if (quirks & HID_BATTERY_QUIRK_FEATURE)
> >               report_type = HID_FEATURE_REPORT;
> >
> > -     dev->battery_min = min;
> > -     dev->battery_max = max;
> > -     dev->battery_report_type = report_type;
> > -     dev->battery_report_id = field->report->id;
> > -     dev->battery_charge_status =
> > POWER_SUPPLY_STATUS_DISCHARGING;
> > +     /* Initialize battery structure */
> > +     bat->dev = dev;
> > +     bat->min = min;
> > +     bat->max = max;
> > +     bat->report_type = report_type;
> > +     bat->report_id = field->report->id;
> > +     bat->charge_status = POWER_SUPPLY_STATUS_DISCHARGING;
> > +     bat->status = HID_BATTERY_UNKNOWN;
> >
> >       /*
> >        * Stylus is normally not connected to the device and thus
> > we
> >        * can't query the device and get meaningful battery
> > strength.
> >        * We have to wait for the device to report it on its own.
> >        */
> > -     dev->battery_avoid_query = report_type == HID_INPUT_REPORT
> > &&
> > -                                field->physical == HID_DG_STYLUS;
> > +     bat->avoid_query = report_type == HID_INPUT_REPORT &&
> > +                        field->physical == HID_DG_STYLUS;
> >
> >       if (quirks & HID_BATTERY_QUIRK_AVOID_QUERY)
> > -             dev->battery_avoid_query = true;
> > +             bat->avoid_query = true;
> >
> > -     dev->battery = power_supply_register(&dev->dev, psy_desc,
> > &psy_cfg);
> > -     if (IS_ERR(dev->battery)) {
> > -             error = PTR_ERR(dev->battery);
> > +     psy_cfg.drv_data = bat;
> > +     bat->ps = power_supply_register(&dev->dev, psy_desc,
> > &psy_cfg);
> > +     if (IS_ERR(bat->ps)) {
> > +             error = PTR_ERR(bat->ps);
> >               hid_warn(dev, "can't register power supply: %d\n",
> > error);
> >               goto err_free_name;
> >       }
> >
> > -     power_supply_powers(dev->battery, &dev->dev);
> > +     power_supply_powers(bat->ps, &dev->dev);
> > +
> > +     list_add_tail(&bat->list, &dev->batteries);
> > +
> > +     /*
> > +      * The legacy single battery API is preserved by exposing
> > the first
> > +      * discovered battery. Systems relying on a single battery
> > view maintain
> > +      * unchanged behavior.
> > +      */
> > +     if (battery_num == 0) {
> > +             dev->battery = bat->ps;
> > +             dev->battery_min = bat->min;
> > +             dev->battery_max = bat->max;
> > +             dev->battery_report_type = bat->report_type;
> > +             dev->battery_report_id = bat->report_id;
> > +             dev->battery_charge_status = bat->charge_status;
> > +             dev->battery_status = bat->status;
> > +             dev->battery_avoid_query = bat->avoid_query;
> > +     }
> > +
> >       return 0;
> >
> >  err_free_name:
> >       kfree(psy_desc->name);
> > -err_free_mem:
> > +err_free_desc:
> >       kfree(psy_desc);
> > -     dev->battery = NULL;
> > +err_free_bat:
> > +     kfree(bat);
> >       return error;
> >  }
> >
> >  static void hidinput_cleanup_battery(struct hid_device *dev)
> >  {
> > +     struct hid_battery *bat, *next;
> >       const struct power_supply_desc *psy_desc;
> >
> > -     if (!dev->battery)
> > -             return;
> > +     list_for_each_entry_safe(bat, next, &dev->batteries, list) {
> > +             psy_desc = bat->ps->desc;
> > +             power_supply_unregister(bat->ps);
> > +             kfree(psy_desc->name);
> > +             kfree(psy_desc);
> > +             list_del(&bat->list);
> > +             kfree(bat);
> > +     }
> >
> > -     psy_desc = dev->battery->desc;
> > -     power_supply_unregister(dev->battery);
> > -     kfree(psy_desc->name);
> > -     kfree(psy_desc);
> >       dev->battery = NULL;
> >  }
> >
> > -static bool hidinput_update_battery_charge_status(struct hid_device
> > *dev,
> > +static struct hid_battery *hidinput_find_battery(struct hid_device
> > *dev,
> > +                                              int report_id)
> > +{
> > +     struct hid_battery *bat;
> > +
> > +     list_for_each_entry(bat, &dev->batteries, list) {
> > +             if (bat->report_id == report_id)
> > +                     return bat;
> > +     }Tested with Dactyl 5x6 split keyboard usin
> > +     return NULL;
> > +}
> > +
> > +static bool hidinput_update_battery_charge_status(struct hid_battery
> > *bat,
> >                                                 unsigned int
> > usage, int value)
> >  {
> >       switch (usage) {
> >       case HID_BAT_CHARGING:
> > -             dev->battery_charge_status = value ?
> > -
> > POWER_SUPPLY_STATUS_CHARGING :
> > -
> > POWER_SUPPLY_STATUS_DISCHARGING;
> > +             bat->charge_status = value ?
> > +                                  POWER_SUPPLY_STATUS_CHARGING :
> > +
> > POWER_SUPPLY_STATUS_DISCHARGING;
> > +             if (bat->dev->battery == bat->ps)
> > +                     bat->dev->battery_charge_status = bat-
> > >charge_status;
> >               return true;
> >       }
> >
> >       return false;
> >  }
> >
> > -static void hidinput_update_battery(struct hid_device *dev, unsigned
> > int usage,
> > -                                 int value)
> > +static void hidinput_update_battery(struct hid_device *dev, int
> > report_id,
> > +                                 unsigned int usage, int value)
> >  {
> > +     struct hid_battery *bat;
> >       int capacity;
> >
> > -     if (!dev->battery)
> > +     bat = hidinput_find_battery(dev, report_id);
> > +     if (!bat)
> >               return;
> >
> > -     if (hidinput_update_battery_charge_status(dev, usage,
> > value)) {
> > -             power_supply_changed(dev->battery);
> > +     if (hidinput_update_battery_charge_status(bat, usage,
> > value)) {
> > +             power_supply_changed(bat->ps);
> >               return;
> >       }
> >
> >       if ((usage & HID_USAGE_PAGE) == HID_UP_DIGITIZER && value ==
> > 0)
> >               return;
> >
> > -     if (value < dev->battery_min || value > dev->battery_max)
> > +     if (value < bat->min || value > bat->max)
> >               return;
> >
> >       capacity = hidinput_scale_battery_capacity(dev, value);
> >
> > -     if (dev->battery_status != HID_BATTERY_REPORTED ||
> > -         capacity != dev->battery_capacity ||
> > -         ktime_after(ktime_get_coarse(), dev-
> > >battery_ratelimit_time)) {
> > -             dev->battery_capacity = capacity;
> > -             dev->battery_status = HID_BATTERY_REPORTED;
> > -             dev->battery_ratelimit_time =
> > +     if (bat->status != HID_BATTERY_REPORTED ||
> > +         capacity != bat->capacity ||
> > +         ktime_after(ktime_get_coarse(), bat->ratelimit_time)) {
> > +             bat->capacity = capacity;
> > +             bat->status = HID_BATTERY_REPORTED;
> > +             bat->ratelimit_time =
> >                       ktime_add_ms(ktime_get_coarse(), 30 * 1000);
> > -             power_supply_changed(dev->battery);
> > +
> > +             if (dev->battery == bat->ps) {
> > +                     dev->battery_capacity = bat->capacity;
> > +                     dev->battery_status = bat->status;
> > +                     dev->battery_ratelimit_time = bat-
> > >ratelimit_time;
> > +             }
> > +
> > +             power_supply_changed(bat->ps);
> >       }
> >  }
> >  #else  /* !CONFIG_HID_BATTERY_STRENGTH */
> > @@ -664,8 +735,8 @@ static void hidinput_cleanup_battery(struct
> > hid_device *dev)
> >  {
> >  }
> >
> > -static void hidinput_update_battery(struct hid_device *dev, unsigned
> > int usage,
> > -                                 int value)
> > +static void hidinput_update_battery(struct hid_device *dev, int
> > report_id,
> > +                                 unsigned int usage, int value)
> >  {
> >  }
> >  #endif       /* CONFIG_HID_BATTERY_STRENGTH */
> > @@ -1533,7 +1604,7 @@ void hidinput_hid_event(struct hid_device *hid,
> > struct hid_field *field, struct
> >               return;
> >
> >       if (usage->type == EV_PWR) {
> > -             hidinput_update_battery(hid, usage->hid, value);
> > +             hidinput_update_battery(hid, report->id, usage->hid,
> > value);
> >               return;
> >       }
> >
> > diff --git a/include/linux/hid.h b/include/linux/hid.h
> > index a4ddb94e3ee5..a6e36835fb3c 100644
> > --- a/include/linux/hid.h
> > +++ b/include/linux/hid.h
> > @@ -634,6 +634,36 @@ enum hid_battery_status {
> >       HID_BATTERY_REPORTED,           /* Device sent unsolicited
> > battery strength report */
> >  };
> >
> > +/**
> > + * struct hid_battery - represents a single battery power supply
> > + * @list: list node for linking into hid_device's battery list
> > + * @dev: pointer to the parent hid_device
> > + * @ps: the power supply device
> > + * @capacity: current battery capacity
> > + * @min: minimum battery value
> > + * @max: maximum battery value
> > + * @report_type: type of report (HID_INPUT_REPORT,
> > HID_FEATURE_REPORT)
> > + * @report_id: report ID for this battery
> > + * @charge_status: current charge status
> > + * @status: battery status (unknown, queried, reported)
> > + * @avoid_query: if true, don't query battery (wait for device
> > reports)
> > + * @ratelimit_time: time for rate limiting battery updates
> > + */
> > +struct hid_battery {
> > +     struct list_head list;
> > +     struct hid_device *dev;
> > +     struct power_supply *ps;
> > +     __s32 capacity;
> > +     __s32 min;
> > +     __s32 max;
> > +     __s32 report_type;
> > +     __s32 report_id;
> > +     __s32 charge_status;
> > +     enum hid_battery_status status;
> > +     bool avoid_query;
> > +     ktime_t ratelimit_time;
> > +};
> > +
> >  struct hid_driver;
> >  struct hid_ll_driver;
> >
> > @@ -670,8 +700,16 @@ struct hid_device {
> >  #ifdef CONFIG_HID_BATTERY_STRENGTH
> >       /*
> >        * Power supply information for HID devices which report
> > -      * battery strength. power_supply was successfully
> > registered if
> > -      * battery is non-NULL.
> > +      * battery strength. Each battery is tracked separately in
> > the
> > +      * batteries list.
> > +      */
> > +     struct list_head batteries;             /* List of
> > hid_battery structures */
> > +
> > +     /*
> > +      * Legacy single battery support - kept for backwards
> > compatibility.
> > +      * Points to the first battery in the list if any exists.
> > +      * power_supply was successfully registered if battery is
> > non-NULL.
> > +      * DEPRECATED: New code should iterate through batteries
> > list instead.
> >        */
> >       struct power_supply *battery;
> >       __s32 battery_capacity;
>
Best,


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-11-12 21:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-11 10:56 [RFC PATCH 0/1] HID: Add support for multiple batteries per device Lucas Zampieri
2025-11-11 10:56 ` [RFC PATCH 1/1] HID: input: " Lucas Zampieri
2025-11-12 14:15   ` Bastien Nocera
2025-11-12 21:36     ` Lucas Zampieri

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).