:p
atchew
Login
This RFC introduces support for multiple batteries per HID device, addressing a long-standing architectural limitation in the HID battery reporting subsystem. ## Background The current HID implementation explicitly prevents multiple batteries per device through an early return in hidinput_setup_battery() that enforces a single-battery assumption. Linux treats peripheral batteries (scope=Device) differently from system batteries, with desktop environments often displaying them separately or ignoring them entirely. However, this design doesn't account for modern multi-battery hardware patterns. ## Problem Statement Multiple battery scenarios that cannot be properly reported today: 1. Gaming headsets with charging docks (e.g., SteelSeries Arctis Nova Pro Wireless) - headset battery reported, dock battery invisible 2. Graphics tablets with stylus batteries (Wacom) - requires driver-specific workarounds 3. Wireless earbuds with per-earbud batteries plus charging case 4. Multi-device receivers (Logitech Unifying) - requires proprietary HID++ protocol parsing This forces manufacturers to use proprietary protocols and vendor-specific software. Community projects parse USB packets directly because standard HID battery reporting cannot handle multi-battery scenarios. ## Why This Matters The current limitation creates a cycle: OS lacks support, so manufacturers implement proprietary protocols, which makes vendor software necessary, which reduces pressure to fix the OS limitation. Improving HID core support for multiple batteries would enable standardized reporting, reduce the need for vendor software, improve OS integration, reduce driver duplication, and provide a foundation for future multi-battery devices. ## Proposed Solution This 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. ## 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. ## 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. 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? Should we support usage-based identification in addition to report ID for devices using the same report ID? Is sequential naming (battery-N) sufficient or should batteries have semantic role identifiers like "main", "stylus", "dock"? To HID maintainers (Jiri Kosina, Benjamin Tissoires): Does this belong in hid-input.c or should it be separate? Any concerns about the backwards compatibility approach? Meaning, should I have removed the whole dev->bat legacy mapping and use the new struct? To power supply maintainers (Sebastian Reichel): Any issues with multiple power_supply devices from a single HID device? Related commits: - c6838eeef2fb: HID: hid-input: occasionally report stylus battery - a608dc1c0639: HID: input: map battery system charging - fd2a9b29dc9c: HID: wacom: Remove AES power_supply after inactivity Community projects demonstrating the need: - HeadsetControl: https://github.com/Sapd/HeadsetControl - Solaar: https://github.com/pwr-Solaar/Solaar - OpenRazer: https://github.com/openrazer/openrazer Lucas Zampieri (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
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 XXXXXXX..XXXXXXX 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -XXX,XX +XXX,XX @@ 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
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 XXXXXXX..XXXXXXX 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -XXX,XX +XXX,XX @@ 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; @@ -XXX,XX +XXX,XX @@ 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; @@ -XXX,XX +XXX,XX @@ 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: @@ -XXX,XX +XXX,XX @@ 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; @@ -XXX,XX +XXX,XX @@ 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; @@ -XXX,XX +XXX,XX @@ 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; } @@ -XXX,XX +XXX,XX @@ 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
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 XXXXXXX..XXXXXXX 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -XXX,XX +XXX,XX @@ 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; @@ -XXX,XX +XXX,XX @@ 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; @@ -XXX,XX +XXX,XX @@ 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; @@ -XXX,XX +XXX,XX @@ 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) { @@ -XXX,XX +XXX,XX @@ 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; @@ -XXX,XX +XXX,XX @@ 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 */ @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -XXX,XX +XXX,XX @@ 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
This series adds support for HID devices with multiple batteries. Currently, the HID battery reporting subsystem only supports one battery per device. There are several devices with multiple batteries that would benefit from this support: - Gaming headsets with batteries in both the headset and charging dock - Wireless earbuds with per-earbud batteries plus charging case - Split keyboards with per-side batteries ## Proposed Solution This series introduces struct hid_battery to encapsulate individual battery state, replaces the old battery fields with a list-based approach, and adds support for multiple batteries tracked within struct hid_device. Batteries are identified by report ID and named as hid-{uniq}-battery-{id}. The implementation is fully backwards compatible with single-battery devices through a helper function. The series first converts the battery code to devm_* as preparatory cleanup, which simplifies the subsequent refactoring and reduces risk of memory management bugs. ## 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. ## 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. 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. Lucas Zampieri (2): HID: input: Convert battery code to devm_* HID: Refactor battery code to use struct hid_battery and add multi-battery support Signed-off-by: Lucas Zampieri <lzampier@redhat.com> Changes in v5: - Split the monolithic v4 patch into two logical patches as suggested by Benjamin, devm_* conversion, then struct refactor and multi-battery support combined Changes in v4: - Added missing hidinput_update_battery() stub in #else block for CONFIG_HID_BATTERY_STRENGTH=n builds - Reported-by: kernel test robot <lkp@intel.com> - Closes: https://lore.kernel.org/oe-kbuild-all/202511201624.yUv4VtBv-lkp@intel.com/ Changes in v3: - Squashed the three v2 patches into a single patch as suggested by Benjamin - Removed all legacy dev->battery_* fields, using list-based storage only - Changed battery naming to include report ID: hid-{uniq}-battery-{report_id} - Converted battery memory management to devm_* for automatic cleanup - Updated hidinput_update_battery() to take struct hid_battery directly - Added hid_get_first_battery() helper for external driver compatibility - Updated hid-apple.c and hid-magicmouse.c to use new battery API - Simplified cover letter based on feedback 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-apple.c | 10 +- drivers/hid/hid-core.c | 4 + drivers/hid/hid-input-test.c | 39 ++++--- drivers/hid/hid-input.c | 196 ++++++++++++++++++----------------- drivers/hid/hid-magicmouse.c | 10 +- include/linux/hid.h | 54 +++++++--- 6 files changed, 180 insertions(+), 133 deletions(-) base-commit: 8b690556d8fe0ee15151cc37ec49c5bbfe41d5b1 -- 2.51.1
Convert the HID battery code to use devm_* managed resource APIs. This changes the following allocations: - kzalloc() -> devm_kzalloc() for power_supply_desc - kasprintf() -> devm_kasprintf() for battery name - power_supply_register() -> devm_power_supply_register() No functional behavior changes. Signed-off-by: Lucas Zampieri <lzampier@redhat.com> --- drivers/hid/hid-input.c | 49 +++++++++-------------------------------- 1 file changed, 10 insertions(+), 39 deletions(-) diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -XXX,XX +XXX,XX @@ 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); + psy_desc = devm_kzalloc(&dev->dev, sizeof(*psy_desc), GFP_KERNEL); if (!psy_desc) return -ENOMEM; - 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; - } + psy_desc->name = devm_kasprintf(&dev->dev, GFP_KERNEL, "hid-%s-battery", + strlen(dev->uniq) ? + dev->uniq : dev_name(&dev->dev)); + if (!psy_desc->name) + return -ENOMEM; psy_desc->type = POWER_SUPPLY_TYPE_BATTERY; psy_desc->properties = hidinput_battery_props; @@ -XXX,XX +XXX,XX @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type, if (quirks & HID_BATTERY_QUIRK_AVOID_QUERY) dev->battery_avoid_query = true; - dev->battery = power_supply_register(&dev->dev, psy_desc, &psy_cfg); + dev->battery = devm_power_supply_register(&dev->dev, psy_desc, &psy_cfg); if (IS_ERR(dev->battery)) { - error = PTR_ERR(dev->battery); - hid_warn(dev, "can't register power supply: %d\n", error); - goto err_free_name; + hid_warn(dev, "can't register power supply: %ld\n", + PTR_ERR(dev->battery)); + return PTR_ERR(dev->battery); } power_supply_powers(dev->battery, &dev->dev); return 0; - -err_free_name: - kfree(psy_desc->name); -err_free_mem: - kfree(psy_desc); - dev->battery = NULL; - return error; -} - -static void hidinput_cleanup_battery(struct hid_device *dev) -{ - const struct power_supply_desc *psy_desc; - - if (!dev->battery) - return; - - psy_desc = dev->battery->desc; - power_supply_unregister(dev->battery); - kfree(psy_desc->name); - kfree(psy_desc); - dev->battery = NULL; } static bool hidinput_update_battery_charge_status(struct hid_device *dev, @@ -XXX,XX +XXX,XX @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type, return 0; } -static void hidinput_cleanup_battery(struct hid_device *dev) -{ -} - static void hidinput_update_battery(struct hid_device *dev, unsigned int usage, int value) { @@ -XXX,XX +XXX,XX @@ void hidinput_disconnect(struct hid_device *hid) { struct hid_input *hidinput, *next; - hidinput_cleanup_battery(hid); - list_for_each_entry_safe(hidinput, next, &hid->inputs, list) { list_del(&hidinput->list); if (hidinput->registered) -- 2.51.1
Introduce struct hid_battery to encapsulate individual battery state and enable HID devices to register multiple batteries, each identified by its report ID. This struct hid_battery replaces the legacy dev->battery_* fields with a batteries list. Batteries are named using their report ID with the pattern hid-{uniq}-battery-{report_id}. External drivers hid-apple and hid-magicmouse are updated to use the new battery API via the hid_get_first_battery() helper, and hid-input-test is updated for the new battery structure. This enables proper battery reporting for devices with multiple batteries such as split keyboards, gaming headsets with charging docks, and wireless earbuds with per-earbud batteries. Suggested-by: Benjamin Tissoires <bentiss@kernel.org> Signed-off-by: Lucas Zampieri <lzampier@redhat.com> --- drivers/hid/hid-apple.c | 10 ++- drivers/hid/hid-core.c | 4 + drivers/hid/hid-input-test.c | 39 +++++---- drivers/hid/hid-input.c | 163 +++++++++++++++++++++-------------- drivers/hid/hid-magicmouse.c | 10 ++- include/linux/hid.h | 54 +++++++++--- 6 files changed, 178 insertions(+), 102 deletions(-) diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/hid/hid-apple.c +++ b/drivers/hid/hid-apple.c @@ -XXX,XX +XXX,XX @@ static int apple_fetch_battery(struct hid_device *hdev) struct apple_sc *asc = hid_get_drvdata(hdev); struct hid_report_enum *report_enum; struct hid_report *report; + struct hid_battery *bat; - if (!(asc->quirks & APPLE_RDESC_BATTERY) || !hdev->battery) + bat = hid_get_first_battery(hdev); + if (!(asc->quirks & APPLE_RDESC_BATTERY) || !bat) return -1; - report_enum = &hdev->report_enum[hdev->battery_report_type]; - report = report_enum->report_id_hash[hdev->battery_report_id]; + report_enum = &hdev->report_enum[bat->report_type]; + report = report_enum->report_id_hash[bat->report_id]; if (!report || report->maxfield < 1) return -1; - if (hdev->battery_capacity == hdev->battery_max) + if (bat->capacity == bat->max) return -1; hid_hw_request(hdev, report, HID_REQ_GET_REPORT); diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -XXX,XX +XXX,XX @@ 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-test.c b/drivers/hid/hid-input-test.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/hid/hid-input-test.c +++ b/drivers/hid/hid-input-test.c @@ -XXX,XX +XXX,XX @@ static void hid_test_input_update_battery_charge_status(struct kunit *test) { - struct hid_device *dev; + struct hid_battery *bat; bool handled; - dev = kunit_kzalloc(test, sizeof(*dev), GFP_KERNEL); - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev); + bat = kunit_kzalloc(test, sizeof(*bat), GFP_KERNEL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, bat); - handled = hidinput_update_battery_charge_status(dev, HID_DG_HEIGHT, 0); + handled = hidinput_update_battery_charge_status(bat, HID_DG_HEIGHT, 0); KUNIT_EXPECT_FALSE(test, handled); - KUNIT_EXPECT_EQ(test, dev->battery_charge_status, POWER_SUPPLY_STATUS_UNKNOWN); + KUNIT_EXPECT_EQ(test, bat->charge_status, POWER_SUPPLY_STATUS_UNKNOWN); - handled = hidinput_update_battery_charge_status(dev, HID_BAT_CHARGING, 0); + handled = hidinput_update_battery_charge_status(bat, HID_BAT_CHARGING, 0); KUNIT_EXPECT_TRUE(test, handled); - KUNIT_EXPECT_EQ(test, dev->battery_charge_status, POWER_SUPPLY_STATUS_DISCHARGING); + KUNIT_EXPECT_EQ(test, bat->charge_status, POWER_SUPPLY_STATUS_DISCHARGING); - handled = hidinput_update_battery_charge_status(dev, HID_BAT_CHARGING, 1); + handled = hidinput_update_battery_charge_status(bat, HID_BAT_CHARGING, 1); KUNIT_EXPECT_TRUE(test, handled); - KUNIT_EXPECT_EQ(test, dev->battery_charge_status, POWER_SUPPLY_STATUS_CHARGING); + KUNIT_EXPECT_EQ(test, bat->charge_status, POWER_SUPPLY_STATUS_CHARGING); } static void hid_test_input_get_battery_property(struct kunit *test) { struct power_supply *psy; + struct hid_battery *bat; struct hid_device *dev; union power_supply_propval val; int ret; dev = kunit_kzalloc(test, sizeof(*dev), GFP_KERNEL); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev); - dev->battery_avoid_query = true; + + bat = kunit_kzalloc(test, sizeof(*bat), GFP_KERNEL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, bat); + bat->dev = dev; + bat->avoid_query = true; psy = kunit_kzalloc(test, sizeof(*psy), GFP_KERNEL); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, psy); - psy->drv_data = dev; + psy->drv_data = bat; - dev->battery_status = HID_BATTERY_UNKNOWN; - dev->battery_charge_status = POWER_SUPPLY_STATUS_CHARGING; + bat->status = HID_BATTERY_UNKNOWN; + bat->charge_status = POWER_SUPPLY_STATUS_CHARGING; ret = hidinput_get_battery_property(psy, POWER_SUPPLY_PROP_STATUS, &val); KUNIT_EXPECT_EQ(test, ret, 0); KUNIT_EXPECT_EQ(test, val.intval, POWER_SUPPLY_STATUS_UNKNOWN); - dev->battery_status = HID_BATTERY_REPORTED; - dev->battery_charge_status = POWER_SUPPLY_STATUS_CHARGING; + bat->status = HID_BATTERY_REPORTED; + bat->charge_status = POWER_SUPPLY_STATUS_CHARGING; ret = hidinput_get_battery_property(psy, POWER_SUPPLY_PROP_STATUS, &val); KUNIT_EXPECT_EQ(test, ret, 0); KUNIT_EXPECT_EQ(test, val.intval, POWER_SUPPLY_STATUS_CHARGING); - dev->battery_status = HID_BATTERY_REPORTED; - dev->battery_charge_status = POWER_SUPPLY_STATUS_DISCHARGING; + bat->status = HID_BATTERY_REPORTED; + bat->charge_status = POWER_SUPPLY_STATUS_DISCHARGING; ret = hidinput_get_battery_property(psy, POWER_SUPPLY_PROP_STATUS, &val); KUNIT_EXPECT_EQ(test, ret, 0); KUNIT_EXPECT_EQ(test, val.intval, POWER_SUPPLY_STATUS_DISCHARGING); diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -XXX,XX +XXX,XX @@ static unsigned find_battery_quirk(struct hid_device *hdev) return quirks; } -static int hidinput_scale_battery_capacity(struct hid_device *dev, +static int hidinput_scale_battery_capacity(struct hid_battery *bat, int value) { - if (dev->battery_min < dev->battery_max && - value >= dev->battery_min && value <= dev->battery_max) - value = ((value - dev->battery_min) * 100) / - (dev->battery_max - dev->battery_min); + if (bat->min < bat->max && + value >= bat->min && value <= bat->max) + value = ((value - bat->min) * 100) / + (bat->max - bat->min); return value; } -static int hidinput_query_battery_capacity(struct hid_device *dev) +static int hidinput_query_battery_capacity(struct hid_battery *bat) { u8 *buf; int ret; @@ -XXX,XX +XXX,XX @@ static int hidinput_query_battery_capacity(struct hid_device *dev) if (!buf) return -ENOMEM; - ret = hid_hw_raw_request(dev, dev->battery_report_id, buf, 4, - dev->battery_report_type, HID_REQ_GET_REPORT); + ret = hid_hw_raw_request(bat->dev, bat->report_id, buf, 4, + bat->report_type, HID_REQ_GET_REPORT); if (ret < 2) { kfree(buf); return -ENODATA; } - ret = hidinput_scale_battery_capacity(dev, buf[1]); + ret = hidinput_scale_battery_capacity(bat, buf[1]); kfree(buf); return ret; } @@ -XXX,XX +XXX,XX @@ 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; @@ -XXX,XX +XXX,XX @@ 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) { - value = hidinput_query_battery_capacity(dev); + if (bat->status != HID_BATTERY_REPORTED && + !bat->avoid_query) { + value = hidinput_query_battery_capacity(bat); if (value < 0) return value; } else { - value = dev->battery_capacity; + value = bat->capacity; } val->intval = value; @@ -XXX,XX +XXX,XX @@ 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) { - value = hidinput_query_battery_capacity(dev); + if (bat->status != HID_BATTERY_REPORTED && + !bat->avoid_query) { + value = hidinput_query_battery_capacity(bat); 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: @@ -XXX,XX +XXX,XX @@ static int hidinput_get_battery_property(struct power_supply *psy, return ret; } +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 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 = { 0 }; unsigned quirks; s32 min, max; - int error; - if (dev->battery) - return 0; /* already initialized? */ + if (hidinput_find_battery(dev, field->report->id)) + return 0; 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; + bat = devm_kzalloc(&dev->dev, sizeof(*bat), GFP_KERNEL); + if (!bat) + return -ENOMEM; + psy_desc = devm_kzalloc(&dev->dev, sizeof(*psy_desc), GFP_KERNEL); if (!psy_desc) return -ENOMEM; - psy_desc->name = devm_kasprintf(&dev->dev, GFP_KERNEL, "hid-%s-battery", + psy_desc->name = devm_kasprintf(&dev->dev, GFP_KERNEL, + "hid-%s-battery-%d", strlen(dev->uniq) ? - dev->uniq : dev_name(&dev->dev)); + dev->uniq : dev_name(&dev->dev), + field->report->id); if (!psy_desc->name) return -ENOMEM; @@ -XXX,XX +XXX,XX @@ 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; + 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 = devm_power_supply_register(&dev->dev, psy_desc, &psy_cfg); - if (IS_ERR(dev->battery)) { + psy_cfg.drv_data = bat; + bat->ps = devm_power_supply_register(&dev->dev, psy_desc, &psy_cfg); + if (IS_ERR(bat->ps)) { hid_warn(dev, "can't register power supply: %ld\n", - PTR_ERR(dev->battery)); - return PTR_ERR(dev->battery); + PTR_ERR(bat->ps)); + return PTR_ERR(bat->ps); } - power_supply_powers(dev->battery, &dev->dev); + power_supply_powers(bat->ps, &dev->dev); + + list_add_tail(&bat->list, &dev->batteries); + return 0; } -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; return true; } return false; } -static void hidinput_update_battery(struct hid_device *dev, unsigned int usage, - int value) +static void hidinput_update_battery(struct hid_battery *bat, + unsigned int usage, int value) { int capacity; - if (!dev->battery) - return; - - if (hidinput_update_battery_charge_status(dev, usage, value)) { - power_supply_changed(dev->battery); + if (hidinput_update_battery_charge_status(bat, usage, value)) { + power_supply_changed(bat->ps); return; } if ((usage & HID_USAGE_PAGE) == HID_UP_DIGITIZER && value == 0) return; - if (value < dev->battery_min || value > dev->battery_max) + if (value < bat->min || value > bat->max) return; - capacity = hidinput_scale_battery_capacity(dev, value); + capacity = hidinput_scale_battery_capacity(bat, 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); + power_supply_changed(bat->ps); } } #else /* !CONFIG_HID_BATTERY_STRENGTH */ @@ -XXX,XX +XXX,XX @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type, return 0; } -static void hidinput_update_battery(struct hid_device *dev, unsigned int usage, - int value) +static struct hid_battery *hidinput_find_battery(struct hid_device *dev, + int report_id) +{ + return NULL; +} + +static void hidinput_update_battery(struct hid_battery *bat, + unsigned int usage, int value) { } #endif /* CONFIG_HID_BATTERY_STRENGTH */ @@ -XXX,XX +XXX,XX @@ 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); + struct hid_battery *bat = hidinput_find_battery(hid, + report->id); + + if (bat) + hidinput_update_battery(bat, usage->hid, value); return; } diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/hid/hid-magicmouse.c +++ b/drivers/hid/hid-magicmouse.c @@ -XXX,XX +XXX,XX @@ static int magicmouse_fetch_battery(struct hid_device *hdev) #ifdef CONFIG_HID_BATTERY_STRENGTH struct hid_report_enum *report_enum; struct hid_report *report; + struct hid_battery *bat; - if (!hdev->battery || + bat = hid_get_first_battery(hdev); + if (!bat || (!is_usb_magicmouse2(hdev->vendor, hdev->product) && !is_usb_magictrackpad2(hdev->vendor, hdev->product))) return -1; - report_enum = &hdev->report_enum[hdev->battery_report_type]; - report = report_enum->report_id_hash[hdev->battery_report_id]; + report_enum = &hdev->report_enum[bat->report_type]; + report = report_enum->report_id_hash[bat->report_id]; if (!report || report->maxfield < 1) return -1; - if (hdev->battery_capacity == hdev->battery_max) + if (bat->capacity == bat->max) return -1; hid_hw_request(hdev, report, HID_REQ_GET_REPORT); diff --git a/include/linux/hid.h b/include/linux/hid.h index XXXXXXX..XXXXXXX 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -XXX,XX +XXX,XX @@ enum hid_battery_status { HID_BATTERY_REPORTED, /* Device sent unsolicited battery strength report */ }; +/** + * struct hid_battery - represents a single battery power supply + * @dev: pointer to the parent hid_device + * @ps: the power supply instance + * @min: minimum battery value from HID descriptor + * @max: maximum battery value from HID descriptor + * @report_type: HID report type (input/feature) + * @report_id: HID report ID for this battery + * @charge_status: current charging status + * @status: battery reporting status + * @capacity: current battery capacity (0-100) + * @avoid_query: if true, avoid querying battery (e.g., for stylus) + * @ratelimit_time: rate limiting for battery reports + * @list: list node for linking into hid_device's battery list + */ +struct hid_battery { + struct hid_device *dev; + struct power_supply *ps; + __s32 min; + __s32 max; + __s32 report_type; + __s32 report_id; + __s32 charge_status; + enum hid_battery_status status; + __s32 capacity; + bool avoid_query; + ktime_t ratelimit_time; + struct list_head list; +}; + struct hid_driver; struct hid_ll_driver; @@ -XXX,XX +XXX,XX @@ 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 power_supply *battery; - __s32 battery_capacity; - __s32 battery_min; - __s32 battery_max; - __s32 battery_report_type; - __s32 battery_report_id; - __s32 battery_charge_status; - enum hid_battery_status battery_status; - bool battery_avoid_query; - ktime_t battery_ratelimit_time; + struct list_head batteries; #endif unsigned long status; /* see STAT flags above */ @@ -XXX,XX +XXX,XX @@ static inline void hid_set_drvdata(struct hid_device *hdev, void *data) dev_set_drvdata(&hdev->dev, data); } +#ifdef CONFIG_HID_BATTERY_STRENGTH +static inline struct hid_battery *hid_get_first_battery(struct hid_device *hdev) +{ + if (list_empty(&hdev->batteries)) + return NULL; + return list_first_entry(&hdev->batteries, struct hid_battery, list); +} +#endif + #define HID_GLOBAL_STACK_SIZE 4 #define HID_COLLECTION_STACK_SIZE 4 -- 2.51.1