* [RFC PATCH v2 1/3] HID: input: Introduce struct hid_battery
2025-11-13 0:15 [RFC PATCH v2 0/3] HID: Add support for multiple batteries per device Lucas Zampieri
@ 2025-11-13 0:15 ` Lucas Zampieri
2025-11-13 10:47 ` Benjamin Tissoires
2025-11-13 0:15 ` [RFC PATCH v2 2/3] HID: input: Refactor battery code to use " Lucas Zampieri
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Lucas Zampieri @ 2025-11-13 0:15 UTC (permalink / raw)
To: linux-input
Cc: Lucas Zampieri, linux-kernel, Jiri Kosina, Benjamin Tissoires,
Sebastian Reichel, Bastien Nocera, linux-pm
Add struct hid_battery to encapsulate battery state per HID device.
This structure will allow tracking individual battery properties
including capacity, min/max values, report information, and status.
The structure includes a list node to enable support for multiple
batteries per device in subsequent patches.
This is a preparation step for transitioning from direct power_supply
access to a more flexible battery management system.
Signed-off-by: Lucas Zampieri <lzampier@redhat.com>
---
include/linux/hid.h | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/include/linux/hid.h b/include/linux/hid.h
index a4ddb94e3ee5..63422130de20 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;
--
2.51.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [RFC PATCH v2 1/3] HID: input: Introduce struct hid_battery
2025-11-13 0:15 ` [RFC PATCH v2 1/3] HID: input: Introduce struct hid_battery Lucas Zampieri
@ 2025-11-13 10:47 ` Benjamin Tissoires
2025-11-13 11:56 ` Bastien Nocera
0 siblings, 1 reply; 13+ messages in thread
From: Benjamin Tissoires @ 2025-11-13 10:47 UTC (permalink / raw)
To: Lucas Zampieri
Cc: linux-input, linux-kernel, Jiri Kosina, Sebastian Reichel,
Bastien Nocera, linux-pm
On Nov 13 2025, Lucas Zampieri wrote:
> Add struct hid_battery to encapsulate battery state per HID device.
> This structure will allow tracking individual battery properties
> including capacity, min/max values, report information, and status.
>
> The structure includes a list node to enable support for multiple
> batteries per device in subsequent patches.
>
> This is a preparation step for transitioning from direct power_supply
> access to a more flexible battery management system.
>
> Signed-off-by: Lucas Zampieri <lzampier@redhat.com>
> ---
> include/linux/hid.h | 30 ++++++++++++++++++++++++++++++
I know Bastien asked you to split out this into a separate commit, but I
hate having a header change when noone uses it. It is painful for people
needing to backport the further changes (imagine you are fixing a CVE
without noticing it), as they also need to pull this one.
> 1 file changed, 30 insertions(+)
>
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index a4ddb94e3ee5..63422130de20 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
For the first inclusion of the new struct, please drop the list field.
This should go into the last patch when you actually make use of it.
> + * @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;
>
> --
> 2.51.1
>
Cheers,
Benjamin
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [RFC PATCH v2 1/3] HID: input: Introduce struct hid_battery
2025-11-13 10:47 ` Benjamin Tissoires
@ 2025-11-13 11:56 ` Bastien Nocera
0 siblings, 0 replies; 13+ messages in thread
From: Bastien Nocera @ 2025-11-13 11:56 UTC (permalink / raw)
To: Benjamin Tissoires, Lucas Zampieri
Cc: linux-input, linux-kernel, Jiri Kosina, Sebastian Reichel,
linux-pm
On Thu, 2025-11-13 at 11:47 +0100, Benjamin Tissoires wrote:
> On Nov 13 2025, Lucas Zampieri wrote:
> > Add struct hid_battery to encapsulate battery state per HID device.
> > This structure will allow tracking individual battery properties
> > including capacity, min/max values, report information, and status.
> >
> > The structure includes a list node to enable support for multiple
> > batteries per device in subsequent patches.
> >
> > This is a preparation step for transitioning from direct
> > power_supply
> > access to a more flexible battery management system.
> >
> > Signed-off-by: Lucas Zampieri <lzampier@redhat.com>
> > ---
> > include/linux/hid.h | 30 ++++++++++++++++++++++++++++++
>
> I know Bastien asked you to split out this into a separate commit,
> but I
> hate having a header change when noone uses it. It is painful for
> people
> needing to backport the further changes (imagine you are fixing a CVE
> without noticing it), as they also need to pull this one.
I usually find that a stand-alone "this can't possibly introduce bugs"
commit to be better (and it makes it easier to concentrate on smaller
amounts of content).
For the CVE use case, I don't really understand the problem. Either you
forget to backport the commit that added the type, in which case it
just doesn't compile, or you can backport the whole series (I would
hope that there's enough metadata in the kernel git to figure that out,
isn't there?).
In any case, your subsystem, your rules, just thought I'd mention why I
asked Lucas to split this up.
Cheers
>
> > 1 file changed, 30 insertions(+)
> >
> > diff --git a/include/linux/hid.h b/include/linux/hid.h
> > index a4ddb94e3ee5..63422130de20 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
>
> For the first inclusion of the new struct, please drop the list
> field.
> This should go into the last patch when you actually make use of it.
>
> > + * @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;
> >
> > --
> > 2.51.1
> >
>
> Cheers,
> Benjamin
^ permalink raw reply [flat|nested] 13+ messages in thread
* [RFC PATCH v2 2/3] HID: input: Refactor battery code to use struct hid_battery
2025-11-13 0:15 [RFC PATCH v2 0/3] HID: Add support for multiple batteries per device Lucas Zampieri
2025-11-13 0:15 ` [RFC PATCH v2 1/3] HID: input: Introduce struct hid_battery Lucas Zampieri
@ 2025-11-13 0:15 ` Lucas Zampieri
2025-11-13 10:54 ` Benjamin Tissoires
2025-11-13 0:15 ` [RFC PATCH v2 3/3] HID: input: Add support for multiple batteries per device Lucas Zampieri
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Lucas Zampieri @ 2025-11-13 0:15 UTC (permalink / raw)
To: linux-input
Cc: Lucas Zampieri, linux-kernel, Jiri Kosina, Benjamin Tissoires,
Sebastian Reichel, Bastien Nocera, linux-pm
Refactor the battery handling code to use the newly introduced
struct hid_battery internally, replacing direct access to individual
power_supply and state fields.
The legacy dev->battery and dev->battery_* fields are maintained and
synchronized for backward compatibility. This refactoring prepares
the code for supporting multiple batteries per device in a subsequent
patch.
Signed-off-by: Lucas Zampieri <lzampier@redhat.com>
---
drivers/hid/hid-input.c | 125 ++++++++++++++++++++++++++--------------
1 file changed, 83 insertions(+), 42 deletions(-)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index e56e7de53279..0e71efea9da3 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,8 +514,9 @@ 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;
@@ -530,16 +532,22 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
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 = kzalloc(sizeof(*psy_desc), GFP_KERNEL);
+ if (!psy_desc) {
+ error = -ENOMEM;
+ goto err_free_bat;
+ }
+
psy_desc->name = kasprintf(GFP_KERNEL, "hid-%s-battery",
strlen(dev->uniq) ?
dev->uniq : dev_name(&dev->dev));
if (!psy_desc->name) {
error = -ENOMEM;
- goto err_free_mem;
+ goto err_free_desc;
}
psy_desc->type = POWER_SUPPLY_TYPE_BATTERY;
@@ -559,63 +567,85 @@ 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);
+
+ /* Maintain legacy single battery fields for backward compatibility */
+ 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;
const struct power_supply_desc *psy_desc;
if (!dev->battery)
return;
+ bat = power_supply_get_drvdata(dev->battery);
psy_desc = dev->battery->desc;
power_supply_unregister(dev->battery);
kfree(psy_desc->name);
kfree(psy_desc);
+ kfree(bat);
dev->battery = NULL;
}
-static bool hidinput_update_battery_charge_status(struct hid_device *dev,
+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;
+ /* Update legacy field for backward compatibility */
+ if (bat->dev->battery == bat->ps)
+ bat->dev->battery_charge_status = bat->charge_status;
return true;
}
@@ -625,32 +655,43 @@ static bool hidinput_update_battery_charge_status(struct hid_device *dev,
static void hidinput_update_battery(struct hid_device *dev, unsigned int usage,
int value)
{
+ struct hid_battery *bat;
int capacity;
if (!dev->battery)
return;
- if (hidinput_update_battery_charge_status(dev, usage, value)) {
- power_supply_changed(dev->battery);
+ bat = power_supply_get_drvdata(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);
+
+ /* Update legacy fields for backward compatibility */
+ 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 */
--
2.51.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [RFC PATCH v2 2/3] HID: input: Refactor battery code to use struct hid_battery
2025-11-13 0:15 ` [RFC PATCH v2 2/3] HID: input: Refactor battery code to use " Lucas Zampieri
@ 2025-11-13 10:54 ` Benjamin Tissoires
2025-11-16 11:40 ` Lucas Zampieri
0 siblings, 1 reply; 13+ messages in thread
From: Benjamin Tissoires @ 2025-11-13 10:54 UTC (permalink / raw)
To: Lucas Zampieri
Cc: linux-input, linux-kernel, Jiri Kosina, Sebastian Reichel,
Bastien Nocera, linux-pm
On Nov 13 2025, Lucas Zampieri wrote:
> Refactor the battery handling code to use the newly introduced
> struct hid_battery internally, replacing direct access to individual
> power_supply and state fields.
>
> The legacy dev->battery and dev->battery_* fields are maintained and
> synchronized for backward compatibility. This refactoring prepares
> the code for supporting multiple batteries per device in a subsequent
> patch.
>
> Signed-off-by: Lucas Zampieri <lzampier@redhat.com>
> ---
> drivers/hid/hid-input.c | 125 ++++++++++++++++++++++++++--------------
> 1 file changed, 83 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index e56e7de53279..0e71efea9da3 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,8 +514,9 @@ 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;
> @@ -530,16 +532,22 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
> 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 = kzalloc(sizeof(*psy_desc), GFP_KERNEL);
> + if (!psy_desc) {
> + error = -ENOMEM;
> + goto err_free_bat;
> + }
> +
> psy_desc->name = kasprintf(GFP_KERNEL, "hid-%s-battery",
> strlen(dev->uniq) ?
> dev->uniq : dev_name(&dev->dev));
> if (!psy_desc->name) {
> error = -ENOMEM;
> - goto err_free_mem;
> + goto err_free_desc;
I wonder if a devm conversion of the whole battery support handling
would not be beneficial here.
> }
>
> psy_desc->type = POWER_SUPPLY_TYPE_BATTERY;
> @@ -559,63 +567,85 @@ 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);
> +
> + /* Maintain legacy single battery fields for backward compatibility */
> + 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;
Already mentioned in the cover letter, but what's the point of keeping
those legacy fields when the exact same data is stored in bat?
> +
> 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;
> const struct power_supply_desc *psy_desc;
>
> if (!dev->battery)
> return;
>
> + bat = power_supply_get_drvdata(dev->battery);
> psy_desc = dev->battery->desc;
> power_supply_unregister(dev->battery);
> kfree(psy_desc->name);
> kfree(psy_desc);
> + kfree(bat);
> dev->battery = NULL;
> }
>
> -static bool hidinput_update_battery_charge_status(struct hid_device *dev,
> +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;
> + /* Update legacy field for backward compatibility */
> + if (bat->dev->battery == bat->ps)
> + bat->dev->battery_charge_status = bat->charge_status;
> return true;
> }
>
> @@ -625,32 +655,43 @@ static bool hidinput_update_battery_charge_status(struct hid_device *dev,
> static void hidinput_update_battery(struct hid_device *dev, unsigned int usage,
> int value)
> {
> + struct hid_battery *bat;
> int capacity;
>
> if (!dev->battery)
> return;
>
> - if (hidinput_update_battery_charge_status(dev, usage, value)) {
> - power_supply_changed(dev->battery);
> + bat = power_supply_get_drvdata(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);
> +
> + /* Update legacy fields for backward compatibility */
> + 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 */
> --
> 2.51.1
>
Cheers,
Benjamin
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [RFC PATCH v2 2/3] HID: input: Refactor battery code to use struct hid_battery
2025-11-13 10:54 ` Benjamin Tissoires
@ 2025-11-16 11:40 ` Lucas Zampieri
0 siblings, 0 replies; 13+ messages in thread
From: Lucas Zampieri @ 2025-11-16 11:40 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: linux-input, linux-kernel, Jiri Kosina, Sebastian Reichel,
Bastien Nocera, linux-pm
On Thu, Nov 13, 2025 at 10:54 AM Benjamin Tissoires <bentiss@kernel.org> wrote:
>
> On Nov 13 2025, Lucas Zampieri wrote:
> > Refactor the battery handling code to use the newly introduced
> > struct hid_battery internally, replacing direct access to individual
> > power_supply and state fields.
> >
> > The legacy dev->battery and dev->battery_* fields are maintained and
> > synchronized for backward compatibility. This refactoring prepares
> > the code for supporting multiple batteries per device in a subsequent
> > patch.
> >
> > Signed-off-by: Lucas Zampieri <lzampier@redhat.com>
> > ---
> > drivers/hid/hid-input.c | 125 ++++++++++++++++++++++++++--------------
> > 1 file changed, 83 insertions(+), 42 deletions(-)
> >
> > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> > index e56e7de53279..0e71efea9da3 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,8 +514,9 @@ 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;
> > @@ -530,16 +532,22 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
> > 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 = kzalloc(sizeof(*psy_desc), GFP_KERNEL);
> > + if (!psy_desc) {
> > + error = -ENOMEM;
> > + goto err_free_bat;
> > + }
> > +
> > psy_desc->name = kasprintf(GFP_KERNEL, "hid-%s-battery",
> > strlen(dev->uniq) ?
> > dev->uniq : dev_name(&dev->dev));
> > if (!psy_desc->name) {
> > error = -ENOMEM;
> > - goto err_free_mem;
> > + goto err_free_desc;
>
> I wonder if a devm conversion of the whole battery support handling
> would not be beneficial here.
>
I'll give it a try and see how it looks.
> > }
> >
> > psy_desc->type = POWER_SUPPLY_TYPE_BATTERY;
> > @@ -559,63 +567,85 @@ 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);
> > +
> > + /* Maintain legacy single battery fields for backward compatibility */
> > + 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;
>
> Already mentioned in the cover letter, but what's the point of keeping
> those legacy fields when the exact same data is stored in bat?
>
Yep, now that I know that is better to have cohereand-API instead of
maintaining backwards compatiliby to a single battery structure that
was only internal, I'll update those as well.
> > +
> > 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;
> > const struct power_supply_desc *psy_desc;
> >
> > if (!dev->battery)
> > return;
> >
> > + bat = power_supply_get_drvdata(dev->battery);
> > psy_desc = dev->battery->desc;
> > power_supply_unregister(dev->battery);
> > kfree(psy_desc->name);
> > kfree(psy_desc);
> > + kfree(bat);
> > dev->battery = NULL;
> > }
> >
> > -static bool hidinput_update_battery_charge_status(struct hid_device *dev,
> > +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;
> > + /* Update legacy field for backward compatibility */
> > + if (bat->dev->battery == bat->ps)
> > + bat->dev->battery_charge_status = bat->charge_status;
> > return true;
> > }
> >
> > @@ -625,32 +655,43 @@ static bool hidinput_update_battery_charge_status(struct hid_device *dev,
> > static void hidinput_update_battery(struct hid_device *dev, unsigned int usage,
> > int value)
> > {
> > + struct hid_battery *bat;
> > int capacity;
> >
> > if (!dev->battery)
> > return;
> >
> > - if (hidinput_update_battery_charge_status(dev, usage, value)) {
> > - power_supply_changed(dev->battery);
> > + bat = power_supply_get_drvdata(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);
> > +
> > + /* Update legacy fields for backward compatibility */
> > + 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 */
> > --
> > 2.51.1
> >
>
> Cheers,
> Benjamin
>
Best,
Lucas Zampieri
^ permalink raw reply [flat|nested] 13+ messages in thread
* [RFC PATCH v2 3/3] HID: input: Add support for multiple batteries per device
2025-11-13 0:15 [RFC PATCH v2 0/3] HID: Add support for multiple batteries per device Lucas Zampieri
2025-11-13 0:15 ` [RFC PATCH v2 1/3] HID: input: Introduce struct hid_battery Lucas Zampieri
2025-11-13 0:15 ` [RFC PATCH v2 2/3] HID: input: Refactor battery code to use " Lucas Zampieri
@ 2025-11-13 0:15 ` Lucas Zampieri
2025-11-13 11:08 ` Benjamin Tissoires
2025-11-13 10:44 ` [RFC PATCH v2 0/3] HID: " Benjamin Tissoires
2025-11-13 11:16 ` Sebastian Reichel
4 siblings, 1 reply; 13+ messages in thread
From: Lucas Zampieri @ 2025-11-13 0:15 UTC (permalink / raw)
To: linux-input
Cc: Lucas Zampieri, linux-kernel, Jiri Kosina, Benjamin Tissoires,
Sebastian Reichel, Bastien Nocera, linux-pm
Enable HID devices to register and manage multiple batteries by
maintaining a list of hid_battery structures, each identified by
its report ID.
The legacy dev->battery field and related fields are maintained for
backward compatibility, pointing to the first battery in the list.
This allows existing code to continue working unchanged while
enabling new functionality for multi-battery devices.
Example hardware that can benefit from this:
- Gaming headsets with charging docks (e.g., SteelSeries Arctis Nova Pro
Wireless)
- Graphics tablets with stylus batteries (Wacom)
- Wireless earbuds with per-earbud batteries plus charging case
- Split keyboards with independent battery per side
Signed-off-by: Lucas Zampieri <lzampier@redhat.com>
---
drivers/hid/hid-core.c | 4 ++
drivers/hid/hid-input.c | 99 +++++++++++++++++++++++++++--------------
include/linux/hid.h | 12 ++++-
3 files changed, 80 insertions(+), 35 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 0e71efea9da3..9d0be3d4ce04 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -520,14 +520,20 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
unsigned quirks;
s32 min, max;
int error;
+ int battery_num = 0;
- if (dev->battery)
- return 0; /* already initialized? */
+ /* Check if battery with this report_id already exists */
+ 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;
@@ -542,9 +548,17 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
goto err_free_bat;
}
- psy_desc->name = kasprintf(GFP_KERNEL, "hid-%s-battery",
- strlen(dev->uniq) ?
- dev->uniq : dev_name(&dev->dev));
+ /* 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_desc;
@@ -597,15 +611,23 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
power_supply_powers(bat->ps, &dev->dev);
- /* Maintain legacy single battery fields for backward compatibility */
- 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;
+ 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;
@@ -620,21 +642,33 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
static void hidinput_cleanup_battery(struct hid_device *dev)
{
- struct hid_battery *bat;
+ 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);
+ }
- bat = power_supply_get_drvdata(dev->battery);
- psy_desc = dev->battery->desc;
- power_supply_unregister(dev->battery);
- kfree(psy_desc->name);
- kfree(psy_desc);
- kfree(bat);
dev->battery = NULL;
}
+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)
{
@@ -652,17 +686,16 @@ static bool hidinput_update_battery_charge_status(struct hid_battery *bat,
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;
- bat = power_supply_get_drvdata(dev->battery);
-
if (hidinput_update_battery_charge_status(bat, usage, value)) {
power_supply_changed(bat->ps);
return;
@@ -705,8 +738,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 */
@@ -1574,7 +1607,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 63422130de20..a6e36835fb3c 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -700,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] 13+ messages in thread* Re: [RFC PATCH v2 3/3] HID: input: Add support for multiple batteries per device
2025-11-13 0:15 ` [RFC PATCH v2 3/3] HID: input: Add support for multiple batteries per device Lucas Zampieri
@ 2025-11-13 11:08 ` Benjamin Tissoires
2025-11-16 11:54 ` Lucas Zampieri
0 siblings, 1 reply; 13+ messages in thread
From: Benjamin Tissoires @ 2025-11-13 11:08 UTC (permalink / raw)
To: Lucas Zampieri
Cc: linux-input, linux-kernel, Jiri Kosina, Sebastian Reichel,
Bastien Nocera, linux-pm
On Nov 13 2025, Lucas Zampieri wrote:
> Enable HID devices to register and manage multiple batteries by
> maintaining a list of hid_battery structures, each identified by
> its report ID.
>
> The legacy dev->battery field and related fields are maintained for
> backward compatibility, pointing to the first battery in the list.
> This allows existing code to continue working unchanged while
> enabling new functionality for multi-battery devices.
>
> Example hardware that can benefit from this:
> - Gaming headsets with charging docks (e.g., SteelSeries Arctis Nova Pro
> Wireless)
> - Graphics tablets with stylus batteries (Wacom)
> - Wireless earbuds with per-earbud batteries plus charging case
> - Split keyboards with independent battery per side
>
> Signed-off-by: Lucas Zampieri <lzampier@redhat.com>
> ---
> drivers/hid/hid-core.c | 4 ++
> drivers/hid/hid-input.c | 99 +++++++++++++++++++++++++++--------------
> include/linux/hid.h | 12 ++++-
> 3 files changed, 80 insertions(+), 35 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 0e71efea9da3..9d0be3d4ce04 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -520,14 +520,20 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
> unsigned quirks;
> s32 min, max;
> int error;
> + int battery_num = 0;
>
> - if (dev->battery)
> - return 0; /* already initialized? */
> + /* Check if battery with this report_id already exists */
> + list_for_each_entry(bat, &dev->batteries, list) {
> + if (bat->report_id == field->report->id)
> + return 0; /* already initialized */
I wonder if we should not make this test stick out a little bit more.
Something like "get_battery(field)" which would return a battery if the
field matches. This way, if we ever encounter multiple batteries
reported on the same report ID, we can always split them by logical
collection, physical, or something else.
> + 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;
> @@ -542,9 +548,17 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
> goto err_free_bat;
> }
>
> - psy_desc->name = kasprintf(GFP_KERNEL, "hid-%s-battery",
> - strlen(dev->uniq) ?
> - dev->uniq : dev_name(&dev->dev));
> + /* 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);
> + }
Not sure how much conservative you need to be here, but I would prefer
we stick to the same naming pattern whether this is the first or second
battery.
> if (!psy_desc->name) {
> error = -ENOMEM;
> goto err_free_desc;
> @@ -597,15 +611,23 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
>
> power_supply_powers(bat->ps, &dev->dev);
>
> - /* Maintain legacy single battery fields for backward compatibility */
> - 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;
> + 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;
> + }
Again, why keeping the old fields? Are they used anywhere?
>
> return 0;
>
> @@ -620,21 +642,33 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
>
> static void hidinput_cleanup_battery(struct hid_device *dev)
> {
> - struct hid_battery *bat;
> + 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);
> + }
Really, switching to devm the batteries would greatly help not making
mistakes here.
>
> - bat = power_supply_get_drvdata(dev->battery);
> - psy_desc = dev->battery->desc;
> - power_supply_unregister(dev->battery);
> - kfree(psy_desc->name);
> - kfree(psy_desc);
> - kfree(bat);
> dev->battery = NULL;
> }
>
> +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;
> +}
Oh, so you already have this function. So why not making use of it in
hidinput_setup_battery()?
> +
> static bool hidinput_update_battery_charge_status(struct hid_battery *bat,
> unsigned int usage, int value)
> {
> @@ -652,17 +686,16 @@ static bool hidinput_update_battery_charge_status(struct hid_battery *bat,
> 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;
>
> - bat = power_supply_get_drvdata(dev->battery);
> -
> if (hidinput_update_battery_charge_status(bat, usage, value)) {
> power_supply_changed(bat->ps);
> return;
> @@ -705,8 +738,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)
Why not hidinput_update_battery(struct hid_battery *bat, unsigned int usage, int value)?
> {
> }
> #endif /* CONFIG_HID_BATTERY_STRENGTH */
> @@ -1574,7 +1607,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);
With the suggested change in hidinput_update_battery, first query the
battery and then change it.
> return;
> }
>
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 63422130de20..a6e36835fb3c 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -700,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;
Nah. hid_device is pure internal interface. So change all the users if
needed, but don't keep something around in the hope that others will do
the work for you.
For references there has been a very long discussion with Linus about
API changes, and the result was that any API change that introduced a
duplicate API was probably a bad design ;)
Cheers,
Benjamin
> __s32 battery_capacity;
> --
> 2.51.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [RFC PATCH v2 3/3] HID: input: Add support for multiple batteries per device
2025-11-13 11:08 ` Benjamin Tissoires
@ 2025-11-16 11:54 ` Lucas Zampieri
0 siblings, 0 replies; 13+ messages in thread
From: Lucas Zampieri @ 2025-11-16 11:54 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: linux-input, linux-kernel, Jiri Kosina, Sebastian Reichel,
Bastien Nocera, linux-pm
On Thu, Nov 13, 2025 at 11:08 AM Benjamin Tissoires <bentiss@kernel.org> wrote:
>
> On Nov 13 2025, Lucas Zampieri wrote:
> > Enable HID devices to register and manage multiple batteries by
> > maintaining a list of hid_battery structures, each identified by
> > its report ID.
> >
> > The legacy dev->battery field and related fields are maintained for
> > backward compatibility, pointing to the first battery in the list.
> > This allows existing code to continue working unchanged while
> > enabling new functionality for multi-battery devices.
> >
> > Example hardware that can benefit from this:
> > - Gaming headsets with charging docks (e.g., SteelSeries Arctis Nova Pro
> > Wireless)
> > - Graphics tablets with stylus batteries (Wacom)
> > - Wireless earbuds with per-earbud batteries plus charging case
> > - Split keyboards with independent battery per side
> >
> > Signed-off-by: Lucas Zampieri <lzampier@redhat.com>
> > ---
> > drivers/hid/hid-core.c | 4 ++
> > drivers/hid/hid-input.c | 99 +++++++++++++++++++++++++++--------------
> > include/linux/hid.h | 12 ++++-
> > 3 files changed, 80 insertions(+), 35 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 0e71efea9da3..9d0be3d4ce04 100644
> > --- a/drivers/hid/hid-input.c
> > +++ b/drivers/hid/hid-input.c
> > @@ -520,14 +520,20 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
> > unsigned quirks;
> > s32 min, max;
> > int error;
> > + int battery_num = 0;
> >
> > - if (dev->battery)
> > - return 0; /* already initialized? */
> > + /* Check if battery with this report_id already exists */
> > + list_for_each_entry(bat, &dev->batteries, list) {
> > + if (bat->report_id == field->report->id)
> > + return 0; /* already initialized */
>
> I wonder if we should not make this test stick out a little bit more.
>
> Something like "get_battery(field)" which would return a battery if the
> field matches. This way, if we ever encounter multiple batteries
> reported on the same report ID, we can always split them by logical
> collection, physical, or something else.
>
I'll add that, and test it in my custom fw.
> > + 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;
> > @@ -542,9 +548,17 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
> > goto err_free_bat;
> > }
> >
> > - psy_desc->name = kasprintf(GFP_KERNEL, "hid-%s-battery",
> > - strlen(dev->uniq) ?
> > - dev->uniq : dev_name(&dev->dev));
> > + /* 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);
> > + }
>
> Not sure how much conservative you need to be here, but I would prefer
> we stick to the same naming pattern whether this is the first or second
> battery.
>
Agree completely, honestly that was the ugly bit for me, and the only
reason I keep that was for 'backwards compatibility' but now I know
better, and Ill made those consistent.
> > if (!psy_desc->name) {
> > error = -ENOMEM;
> > goto err_free_desc;
> > @@ -597,15 +611,23 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
> >
> > power_supply_powers(bat->ps, &dev->dev);
> >
> > - /* Maintain legacy single battery fields for backward compatibility */
> > - 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;
> > + 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;
> > + }
>
> Again, why keeping the old fields? Are they used anywhere?
>
Ack, and updating those on v3.
> >
> > return 0;
> >
> > @@ -620,21 +642,33 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
> >
> > static void hidinput_cleanup_battery(struct hid_device *dev)
> > {
> > - struct hid_battery *bat;
> > + 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);
> > + }
>
> Really, switching to devm the batteries would greatly help not making
> mistakes here.
>
> >
> > - bat = power_supply_get_drvdata(dev->battery);
> > - psy_desc = dev->battery->desc;
> > - power_supply_unregister(dev->battery);
> > - kfree(psy_desc->name);
> > - kfree(psy_desc);
> > - kfree(bat);
> > dev->battery = NULL;
> > }
> >
> > +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;
> > +}
>
> Oh, so you already have this function. So why not making use of it in
> hidinput_setup_battery()?
>
"backwards compatibility" lol, but yeah in v2 I implemented it
half-way, in v3 I'll make a proper use of it with the full list
conversion.
> > +
> > static bool hidinput_update_battery_charge_status(struct hid_battery *bat,
> > unsigned int usage, int value)
> > {
> > @@ -652,17 +686,16 @@ static bool hidinput_update_battery_charge_status(struct hid_battery *bat,
> > 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;
> >
> > - bat = power_supply_get_drvdata(dev->battery);
> > -
> > if (hidinput_update_battery_charge_status(bat, usage, value)) {
> > power_supply_changed(bat->ps);
> > return;
> > @@ -705,8 +738,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)
>
> Why not hidinput_update_battery(struct hid_battery *bat, unsigned int usage, int value)?
>
I see, this is much more elegant, as I can avoid the lookup inside the
funcion that way, let me use that on the v3.
> > {
> > }
> > #endif /* CONFIG_HID_BATTERY_STRENGTH */
> > @@ -1574,7 +1607,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);
>
> With the suggested change in hidinput_update_battery, first query the
> battery and then change it.
>
Ack
> > return;
> > }
> >
> > diff --git a/include/linux/hid.h b/include/linux/hid.h
> > index 63422130de20..a6e36835fb3c 100644
> > --- a/include/linux/hid.h
> > +++ b/include/linux/hid.h
> > @@ -700,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;
>
> Nah. hid_device is pure internal interface. So change all the users if
> needed, but don't keep something around in the hope that others will do
> the work for you.
>
> For references there has been a very long discussion with Linus about
> API changes, and the result was that any API change that introduced a
> duplicate API was probably a bad design ;)
>
Ack, and that was the knowledge that I was missing and why I stuck to
much to making it backwards compatible, thanks for that really.
> Cheers,
> Benjamin
>
> > __s32 battery_capacity;
> > --
> > 2.51.1
> >
>
Best,
Lucas Zampieri
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v2 0/3] HID: Add support for multiple batteries per device
2025-11-13 0:15 [RFC PATCH v2 0/3] HID: Add support for multiple batteries per device Lucas Zampieri
` (2 preceding siblings ...)
2025-11-13 0:15 ` [RFC PATCH v2 3/3] HID: input: Add support for multiple batteries per device Lucas Zampieri
@ 2025-11-13 10:44 ` Benjamin Tissoires
2025-11-16 11:38 ` Lucas Zampieri
2025-11-13 11:16 ` Sebastian Reichel
4 siblings, 1 reply; 13+ messages in thread
From: Benjamin Tissoires @ 2025-11-13 10:44 UTC (permalink / raw)
To: Lucas Zampieri
Cc: linux-input, linux-kernel, Jiri Kosina, Sebastian Reichel,
Bastien Nocera, linux-pm
Hi Lucas,
On Nov 13 2025, Lucas Zampieri wrote:
> This RFC introduces support for multiple batteries per HID device, addressing
> a long-standing architectural limitation in the HID battery reporting subsystem.
(TBH, that cover letter screams AI :-P)
Stating a "long-standing architectural limitation" is a bit rough. The
need wasn't there or nobody took the time and effort to plug this hole
:)
General comment on this cover letter actually: it is way too detailed,
and try to iron out every corner case the AI bot thought would be fought
against.
Which leads to people actually nitpicking on the details below:
for example, I immediately thought the Wacom bit down below was wrong.
But really, all you need is multiple battery support on devices.
And this is fine. A couple of example would be good enough, and we can
move on.
>
> ## 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
Technically, most Wacom styluses are battery-less for their pencils. The
battery issue that was related below is for a AES device, which is an
integrated wacom tablet in a laptop, so the only battery we have is on
the stylus itself. Arguably we can have several styluses, but only one
can be used at the same time, so reusing the same power supply device is
fine.
(see, I told you, that's the "someone is wrong on the internet effect")
> 3. Wireless earbuds with per-earbud batteries plus charging case
> 4. Multi-device receivers (Logitech Unifying) - requires proprietary HID++
> protocol parsing
That's an entire different reason why there is HID++, and the fact that
Solaar handles batteries itself is lack of support in the kernel for
some devices. (and because sometimes it's much simpler to update a user
space client than a kernel driver)
>
> 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.
I don't think Linux ever forced any manufacturer to use proprietary
protocols or vendor-specific software. For Logitech (or any gaming mouse
FWIW), those protocols were in place well before Linux ever had support
for them.
>
> ## 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.
This is completely BS and AI generated. There is no such cycle.
But again, the need is there, so we need to push this series forward.
The packaging of the cover letter is wrong. We are not salesmen, there
is no need to pitch this like that.
>
> ## Proposed Solution
>
> This series introduces struct hid_battery to encapsulate individual battery
> state, refactors the code to use this structure internally, and adds support
> for multiple batteries tracked in a list within struct hid_device. Batteries
> are identified by report ID. The implementation maintains full backwards
> compatibility with existing single-battery code.
That paragraph is good. Keep it please :)
>
> ## Testing
>
> Tested with split keyboard hardware (Dactyl 5x6) using custom ZMK firmware
> that implements per-side HID battery reporting. Each battery (left and right
> keyboard halves) reports independently through the power supply interface with
> distinct report IDs (0x05 and 0x06).
>
> Test firmware available on my personal fork at:
> https://github.com/zampierilucas/zmk/tree/feat/individual-hid-battery-reporting
> If this series gets merged, these changes will be proposed to upstream ZMK.
>
> HID descriptor and recording captured with hid-recorder:
>
> D: 0
> R: 162 05 01 09 06 a1 01 85 01 05 07 19 e0 29 e7 15 00 25 01 75 01 95 08 81 02 05 07 75 08 95 01 81 03 05 07 15 00 25 01 19 00 29 67 75 01 95 68 81 02 c0 05 0c 09 01 a1 01 85 02 05 0c 15 00 26 ff 0f 19 00 2a ff 0f 75 10 95 06 81 00 c0 05 84 09 05 a1 01 05 85 85 05 09 44 15 00 25 01 35 00 45 01 75 08 95 01 81 02 09 65 15 00 25 64 35 00 45 64 75 08 95 01 81 02 c0 05 84 09 05 a1 01 05 85 85 06 09 44 15 00 25 01 35 00 45 01 75 08 95 01 81 02 09 65 15 00 25 64 35 00 45 64 75 08 95 01 81 02 c0
> N: ZMK Project Dactyl 5x6
> P: usb-0000:2d:00.3-4.2/input2
> I: 3 1d50 615e
> D: 0
> E: 0.000000 3 05 00 56
> E: 0.000977 3 05 00 56
> E: 1.490974 3 06 00 52
> E: 1.491958 3 06 00 52
> E: 6.492979 3 06 00 53
> E: 6.493962 3 06 00 53
>
> The recording shows both batteries reporting with different charge levels
> (Report ID 05: 86%, Report ID 06: 82%-83%), demonstrating the multi-battery
> functionality. This can be used to verify UPower compatibility.
Same, please keep that testing note in future versions.
>
> ## Future Work: Userspace Integration
>
> As suggested by Bastien, semantic battery differentiation (e.g., "left
> earbud" vs "right earbud") requires userspace coordination, as HID
> reports typically lack role metadata.
The sad part is that HID can report a lot of things, but manufacturers
don't make a full use of it.
For example, in the split keyboard case, you could make use of the
Handedness NAry of the generic device Controls page (0x06) which can
help describing Left Hand (0x32) and Right Hand (0x33).
But the problem is more likely that no known manufacturer will follow
your lead on this, and the NAry code you put here will only be used by
your firmware.
For the bluetooth earbuds, the Logical collection Channel Left (0x161,
page Consumer Page 0x0c) and Channel Right (0x162, same page) could be
used.
>
> This will require:
> 1. systemd/hwdb entries for device-specific battery role mappings
> 2. UPower updates to enumerate and group multi-battery devices
> 3. Desktop environment changes to display batteries with meaningful labels
>
> This kernel infrastructure is a prerequisite for that userspace work.
>
> ## Request for Comments
>
> Is list-based storage appropriate or would another structure work better?
List based storage should be fine. We don't receive battery updates that
often, so if that is simpler for you, that's all right.
> Should we support usage-based identification in addition to report ID for
> devices using the same report ID?
If the question is:
- should we enforce each battery to have a different name, the answer is
yes
- should we take the report ID as the battery identifier in the name:
why not?
- should we make use of other collections to split up batteries: maybe.
If you have a device which reports multiple batteries on the same
report ID, then yes, we will need a way to split them.
But again, we are only theorically speaking here, so it's hard to
answer. I really wish we had an actual consumer device we could use as a
reference, not a custom firmware we can tune in any way we want.
> Is sequential naming (battery-N) sufficient
> or should batteries have semantic role identifiers like "main", "stylus", "dock"?
Semantic would be ideal, but at least having a unique identifier is a
requirement. That's more a question for the upower guys.
>
> To HID maintainers (Jiri Kosina, Benjamin Tissoires): Does this belong in
> hid-input.c or should it be separate?
hid-input.c seems fine to me. There are technically not a lot of
differences from the existing AFAICT, it's just that we iterate over a
list.
> Any concerns about the backwards
> compatibility approach? Meaning, should I have removed the whole
> dev->bat legacy mapping and use the new struct?
I'm not sure I follow that backwards compatibility. AFAIU that API is
kernel internal, so it should be easier to drop it entirely instead of
duplicating the data for no benefit.
>
> To power supply maintainers (Sebastian Reichel): Any issues with multiple
> power_supply devices from a single HID device?
Not a power supply maintainer, but unless upower builds a graph of the
devices it sees, I don't see why this would be a problem.
>
> 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
What the point of those?
>
> 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 (3):
> HID: input: Introduce struct hid_battery
> HID: input: Refactor battery code to use struct hid_battery
> HID: input: Add support for multiple batteries per device
>
> Changes in v2:
> - Split the monolithic v1 patch into three logical patches for easier review:
> 1. Introduce struct hid_battery (pure structure addition)
> 2. Refactor existing code to use the new structure (internal changes)
> 3. Add multi-battery support (new functionality)
> - Added detailed testing section with hardware specifics
> - Added hid-recorder output (dactyl-hid-recording.txt) demonstrating two-battery
> HID descriptor for UPower validation
> - Added "Future Work: Userspace Integration" section addressing Bastien's feedback
> about semantic battery differentiation
> - Added hardware examples with product links to commit messages (per Bastien's
> suggestion)
> - No functional changes from v1, only improved patch organization and documentation
>
> drivers/hid/hid-core.c | 4 +
> drivers/hid/hid-input.c | 196 +++++++++++++++++++++++++++-------------
> include/linux/hid.h | 42 ++++++++-
> 3 files changed, 179 insertions(+), 63 deletions(-)
>
> --
> 2.51.1
>
And now after all of this bikeshedding, I can move on to the review of
the series.
Cheers,
Benjamin
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [RFC PATCH v2 0/3] HID: Add support for multiple batteries per device
2025-11-13 10:44 ` [RFC PATCH v2 0/3] HID: " Benjamin Tissoires
@ 2025-11-16 11:38 ` Lucas Zampieri
0 siblings, 0 replies; 13+ messages in thread
From: Lucas Zampieri @ 2025-11-16 11:38 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: linux-input, linux-kernel, Jiri Kosina, Sebastian Reichel,
Bastien Nocera, linux-pm
Hey Benjamin
On Thu, Nov 13, 2025 at 10:44 AM Benjamin Tissoires <bentiss@kernel.org> wrote:
>
> Hi Lucas,
>
> On Nov 13 2025, Lucas Zampieri wrote:
> > This RFC introduces support for multiple batteries per HID device, addressing
> > a long-standing architectural limitation in the HID battery reporting subsystem.
>
> (TBH, that cover letter screams AI :-P)
(partly yes, but also the cover letter kept growing and growing with
more things to point out/data to make this request seem sane)
>
> Stating a "long-standing architectural limitation" is a bit rough. The
> need wasn't there or nobody took the time and effort to plug this hole
> :)
>
> General comment on this cover letter actually: it is way too detailed,
> and try to iron out every corner case the AI bot thought would be fought
> against.
> Which leads to people actually nitpicking on the details below:
> for example, I immediately thought the Wacom bit down below was wrong.
>
> But really, all you need is multiple battery support on devices.
> And this is fine. A couple of example would be good enough, and we can
> move on.
>
Ack, will make it simpler and remove those many examples.
> >
> > ## 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
>
> Technically, most Wacom styluses are battery-less for their pencils. The
> battery issue that was related below is for a AES device, which is an
> integrated wacom tablet in a laptop, so the only battery we have is on
> the stylus itself. Arguably we can have several styluses, but only one
> can be used at the same time, so reusing the same power supply device is
> fine.
>
> (see, I told you, that's the "someone is wrong on the internet effect")
>
oh okay, thats the only type of driver that I found did a similar
thing to what I'm proposing, but yeah I missed the part we most
styluses where battery-less.
> > 3. Wireless earbuds with per-earbud batteries plus charging case
> > 4. Multi-device receivers (Logitech Unifying) - requires proprietary HID++
> > protocol parsing
>
> That's an entire different reason why there is HID++, and the fact that
> Solaar handles batteries itself is lack of support in the kernel for
> some devices. (and because sometimes it's much simpler to update a user
> space client than a kernel driver)
>
I know I was picking an specific HID++ feature, over the many others
that it provides just to make a point.
> >
> > 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.
>
> I don't think Linux ever forced any manufacturer to use proprietary
> protocols or vendor-specific software. For Logitech (or any gaming mouse
> FWIW), those protocols were in place well before Linux ever had support
> for them.
>
Yes, what I mean, it that there's no path to make the multi-battery
reporting native to Linux(Other than ofk you can report only the
lowest of the average of all batteries, but not individual batteries),
so I feel manufactures have no other option other than write their own
perfiferal-sw (eg, steelseries sonar, Logitech GHUB, etc.)(and sure,
many of those dont even work on linux, but then we are left
reverse-engineering protocols in Solaar, HeadsetControl and others).
> >
> > ## 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.
>
> This is completely BS and AI generated. There is no such cycle.
>
> But again, the need is there, so we need to push this series forward.
> The packaging of the cover letter is wrong. We are not salesmen, there
> is no need to pitch this like that.
>
Well, BS maybe, but completely human fabricated :p
And sure, I have no ground in reality for this supposition other than
inferring from the peripherals that I've seen. But my questions here
mostly is, why would manufactures spend time and money creating and
maintaining their custom protocols and accompanying desktop-software
if there was a native way? But I guess you got to the same conclusion
as I did, so then, I'll remove my suppositions, and only keep the real
world examples.
> >
> > ## Proposed Solution
> >
> > This series introduces struct hid_battery to encapsulate individual battery
> > state, refactors the code to use this structure internally, and adds support
> > for multiple batteries tracked in a list within struct hid_device. Batteries
> > are identified by report ID. The implementation maintains full backwards
> > compatibility with existing single-battery code.
>
> That paragraph is good. Keep it please :)
>
> >
> > ## Testing
> >
> > Tested with split keyboard hardware (Dactyl 5x6) using custom ZMK firmware
> > that implements per-side HID battery reporting. Each battery (left and right
> > keyboard halves) reports independently through the power supply interface with
> > distinct report IDs (0x05 and 0x06).
> >
> > Test firmware available on my personal fork at:
> > https://github.com/zampierilucas/zmk/tree/feat/individual-hid-battery-reporting
> > If this series gets merged, these changes will be proposed to upstream ZMK.
> >
> > HID descriptor and recording captured with hid-recorder:
> >
> > D: 0
> > R: 162 05 01 09 06 a1 01 85 01 05 07 19 e0 29 e7 15 00 25 01 75 01 95 08 81 02 05 07 75 08 95 01 81 03 05 07 15 00 25 01 19 00 29 67 75 01 95 68 81 02 c0 05 0c 09 01 a1 01 85 02 05 0c 15 00 26 ff 0f 19 00 2a ff 0f 75 10 95 06 81 00 c0 05 84 09 05 a1 01 05 85 85 05 09 44 15 00 25 01 35 00 45 01 75 08 95 01 81 02 09 65 15 00 25 64 35 00 45 64 75 08 95 01 81 02 c0 05 84 09 05 a1 01 05 85 85 06 09 44 15 00 25 01 35 00 45 01 75 08 95 01 81 02 09 65 15 00 25 64 35 00 45 64 75 08 95 01 81 02 c0
> > N: ZMK Project Dactyl 5x6
> > P: usb-0000:2d:00.3-4.2/input2
> > I: 3 1d50 615e
> > D: 0
> > E: 0.000000 3 05 00 56
> > E: 0.000977 3 05 00 56
> > E: 1.490974 3 06 00 52
> > E: 1.491958 3 06 00 52
> > E: 6.492979 3 06 00 53
> > E: 6.493962 3 06 00 53
> >
> > The recording shows both batteries reporting with different charge levels
> > (Report ID 05: 86%, Report ID 06: 82%-83%), demonstrating the multi-battery
> > functionality. This can be used to verify UPower compatibility.
>
> Same, please keep that testing note in future versions.
>
> >
> > ## Future Work: Userspace Integration
> >
> > As suggested by Bastien, semantic battery differentiation (e.g., "left
> > earbud" vs "right earbud") requires userspace coordination, as HID
> > reports typically lack role metadata.
>
> The sad part is that HID can report a lot of things, but manufacturers
> don't make a full use of it.
>
> For example, in the split keyboard case, you could make use of the
> Handedness NAry of the generic device Controls page (0x06) which can
> help describing Left Hand (0x32) and Right Hand (0x33).
>
> But the problem is more likely that no known manufacturer will follow
> your lead on this, and the NAry code you put here will only be used by
> your firmware.
>
> For the bluetooth earbuds, the Logical collection Channel Left (0x161,
> page Consumer Page 0x0c) and Channel Right (0x162, same page) could be
> used.
>
> >
> > This will require:
> > 1. systemd/hwdb entries for device-specific battery role mappings
> > 2. UPower updates to enumerate and group multi-battery devices
> > 3. Desktop environment changes to display batteries with meaningful labels
> >
> > This kernel infrastructure is a prerequisite for that userspace work.
> >
> > ## Request for Comments
> >
> > Is list-based storage appropriate or would another structure work better?
>
> List based storage should be fine. We don't receive battery updates that
> often, so if that is simpler for you, that's all right.
>
> > Should we support usage-based identification in addition to report ID for
> > devices using the same report ID?
>
> If the question is:
> - should we enforce each battery to have a different name, the answer is
> yes
> - should we take the report ID as the battery identifier in the name:
> why not?
> - should we make use of other collections to split up batteries: maybe.
> If you have a device which reports multiple batteries on the same
> report ID, then yes, we will need a way to split them.
>
> But again, we are only theorically speaking here, so it's hard to
> answer. I really wish we had an actual consumer device we could use as a
> reference, not a custom firmware we can tune in any way we want.
>
yeah, but I guess we are also showing the way of how it could be
implemented in a future device, so might as well stress the
theoretical aspect of it, so we can provide a well rounded api. For
now I'll work on a v3 with report ID, but I'm open to changing things
around if we come up with something better.
> > Is sequential naming (battery-N) sufficient
> > or should batteries have semantic role identifiers like "main", "stylus", "dock"?
>
> Semantic would be ideal, but at least having a unique identifier is a
> requirement. That's more a question for the upower guys.
>
> >
> > To HID maintainers (Jiri Kosina, Benjamin Tissoires): Does this belong in
> > hid-input.c or should it be separate?
>
> hid-input.c seems fine to me. There are technically not a lot of
> differences from the existing AFAICT, it's just that we iterate over a
> list.
>
> > Any concerns about the backwards
> > compatibility approach? Meaning, should I have removed the whole
> > dev->bat legacy mapping and use the new struct?
>
> I'm not sure I follow that backwards compatibility. AFAIU that API is
> kernel internal, so it should be easier to drop it entirely instead of
> duplicating the data for no benefit.
>
Got it, I was mostly worried that it was expected somewhere else, but
given your feedback, I'll make sure to have a proper new api without
so many edge case handling for a single device.
> >
> > To power supply maintainers (Sebastian Reichel): Any issues with multiple
> > power_supply devices from a single HID device?
>
> Not a power supply maintainer, but unless upower builds a graph of the
> devices it sees, I don't see why this would be a problem.
>
> >
> > 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
>
> What the point of those?
Mostly to say, "Hey I know it has been done there!", but I'll get rid
of those in v3 as its just noise.
>
> >
> > 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 (3):
> > HID: input: Introduce struct hid_battery
> > HID: input: Refactor battery code to use struct hid_battery
> > HID: input: Add support for multiple batteries per device
> >
> > Changes in v2:
> > - Split the monolithic v1 patch into three logical patches for easier review:
> > 1. Introduce struct hid_battery (pure structure addition)
> > 2. Refactor existing code to use the new structure (internal changes)
> > 3. Add multi-battery support (new functionality)
> > - Added detailed testing section with hardware specifics
> > - Added hid-recorder output (dactyl-hid-recording.txt) demonstrating two-battery
> > HID descriptor for UPower validation
> > - Added "Future Work: Userspace Integration" section addressing Bastien's feedback
> > about semantic battery differentiation
> > - Added hardware examples with product links to commit messages (per Bastien's
> > suggestion)
> > - No functional changes from v1, only improved patch organization and documentation
> >
> > drivers/hid/hid-core.c | 4 +
> > drivers/hid/hid-input.c | 196 +++++++++++++++++++++++++++-------------
> > include/linux/hid.h | 42 ++++++++-
> > 3 files changed, 179 insertions(+), 63 deletions(-)
> >
> > --
> > 2.51.1
> >
>
> And now after all of this bikeshedding, I can move on to the review of
> the series.
>
> Cheers,
> Benjamin
>
Thank you for the bikeshedding, and sorry for such a long cover
letter, I just wanted to justify every aspect of what/why/how was
needed, and ended up with this monstruosity :D I'll clean this up for
v3 and make sure only what is relevant needed for this to the throught
is there.
Best,
Lucas Zampieri
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v2 0/3] HID: Add support for multiple batteries per device
2025-11-13 0:15 [RFC PATCH v2 0/3] HID: Add support for multiple batteries per device Lucas Zampieri
` (3 preceding siblings ...)
2025-11-13 10:44 ` [RFC PATCH v2 0/3] HID: " Benjamin Tissoires
@ 2025-11-13 11:16 ` Sebastian Reichel
4 siblings, 0 replies; 13+ messages in thread
From: Sebastian Reichel @ 2025-11-13 11:16 UTC (permalink / raw)
To: Lucas Zampieri
Cc: linux-input, linux-kernel, Jiri Kosina, Benjamin Tissoires,
Bastien Nocera, linux-pm
[-- Attachment #1: Type: text/plain, Size: 978 bytes --]
Hi,
On Thu, Nov 13, 2025 at 12:15:02AM +0000, Lucas Zampieri wrote:
> ## 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"?
If unique semantic identifiers are available they are obviously a better
choice.
> 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?
I don't see any issues with that.
Greetings,
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread