[PATCH v2] HID: input: read battery capacity from its actual report offset

[PATCH v2] HID: input: read battery capacity from its actual report offset

[Jose Villaseñor Montfort]

hidinput_query_battery_capacity() assumes the state-of-charge value is
the first byte following the report ID (buf[1]) and ignores where the
battery field actually sits within the report.

An Apple Magic Trackpad 2 precedes the AbsoluteStateOfCharge byte with a
byte of status flags in its battery reports, so this query returns the
flags byte instead of the charge level.

The device happens to make that easy to observe, because it exposes the
same cell twice: its report descriptor declares AbsoluteStateOfCharge in
two reports (0x90 and 0x9b), so hidinput_setup_battery() registers two
power supplies. Only the first one is refreshed by hid-magicmouse -- it
uses hid_get_battery(), which returns the first battery of the list --
and that refresh goes through the report event path, which parses the
field correctly. Nothing ever reports the second one, so every read of
its capacity takes the query path above. On a USB-C Magic Trackpad over
USB, on an unpatched 7.1.5:

  hid-<serial>-battery-144 = 100%  (Charging)      <- report event path
  hid-<serial>-battery-155 =   3%  (Discharging)   <- query path

Both are the same physical battery. A raw HIDIOCGINPUT of the two
reports at that same moment:

  report 0x90 -> [90 03 64]
  report 0x9b -> [9b 03 64 64 00 00 10 00 00 00 00 00 00 00]
                     ^flags ^SoC = 0x64 = 100%

The device answers correctly in both cases; only the offset the kernel
reads the capacity from is wrong. 0x03 is the flags byte (present,
charging), reported as "3%".

Bluetooth takes the same query path for its capacity, where the trackpad
reported a bogus near-constant ~4% -- 0b100, the FullyCharged flag --
regardless of the real charge.

Store the battery field's offset within the report at setup time and use
it when querying, so the capacity is read from its real position. The
report event path already parses the field correctly through the HID
core; only the explicit GET_REPORT query was wrong.

Devices whose capacity field is the first field in the report have a
report_offset of 0 and are unaffected (buf[1 + 0] == buf[1]).

Fixes: 581c4484769e ("HID: input: map digitizer battery usage")
Cc: stable@vger.kernel.org
Signed-off-by: Jose Villaseñor Montfort <pepemontfort@gmail.com>
---
No code changes since v1, only the commit message and tags.

Changes in v2:
- Added Fixes: 581c4484769e, which introduced
  hidinput_query_battery_capacity() with the hardcoded buf[1], and a
  stable tag.
- Dropped the claim in v1 that USB is unaffected. It is not: the
  trackpad's descriptor declares AbsoluteStateOfCharge in two reports,
  hidinput_setup_battery() registers a power supply for each, and
  hid-magicmouse only ever refreshes the first one (hid_get_battery()
  returns the head of the list). The second power supply therefore
  serves every read from the broken query path, and shows 3% -- the
  flags byte -- permanently, on USB, next to the first one showing the
  correct 100%. That replaces the v1 example, since it puts the working
  and the broken path on the same cell at the same instant.
- v1: https://lore.kernel.org/linux-input/20260702192139.114809-1-pepemontfort@gmail.com/

This is complementary to the driver-side battery work in flight for the
same hardware [1][2] rather than a competitor to it: those enable or
adjust *who* fetches the battery, this fixes *where* the generic query
reads the value from. Worth noting for that discussion that
HID_BATTERY_QUIRK_AVOID_QUERY would not help the case above -- with the
query skipped, the second power supply falls back to bat->capacity,
which nothing ever writes, so it would report 0% instead of 3%.

The duplicate power supply itself looks like a separate issue and I am
not addressing it here.

[1] https://lore.kernel.org/linux-input/20260706175507.47288-1-andfed.net@gmail.com/
[2] https://lore.kernel.org/linux-input/20260714101235.99447-1-signshop.alec@gmail.com/

 drivers/hid/hid-input.c | 17 +++++++++++++----
 include/linux/hid.h     |  2 ++
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 3487600ca..b55cbe7f6 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -432,17 +432,25 @@ static int hidinput_scale_battery_capacity(struct hid_battery *bat,
 static int hidinput_query_battery_capacity(struct hid_battery *bat)
 {
 	int ret;
+	/*
+	 * The capacity field may not be the first field in the report: some
+	 * devices (e.g. the Apple Magic Trackpad 2 over Bluetooth) precede it
+	 * with status flags. Read it from its actual byte offset in the report
+	 * (report_offset is in bits; the leading byte is the report id).
+	 */
+	int offset = 1 + bat->report_offset / 8;
+	int len = offset + 1;
 
-	u8 *buf __free(kfree) = kmalloc(4, GFP_KERNEL);
+	u8 *buf __free(kfree) = kmalloc(max(len, 4), GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
 
-	ret = hid_hw_raw_request(bat->dev, bat->report_id, buf, 4,
+	ret = hid_hw_raw_request(bat->dev, bat->report_id, buf, max(len, 4),
 				 bat->report_type, HID_REQ_GET_REPORT);
-	if (ret < 2)
+	if (ret < len)
 		return -ENODATA;
 
-	return hidinput_scale_battery_capacity(bat, buf[1]);
+	return hidinput_scale_battery_capacity(bat, buf[offset]);
 }
 
 static int hidinput_get_battery_property(struct power_supply *psy,
@@ -593,6 +601,7 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
 	bat->max = max;
 	bat->report_type = report_type;
 	bat->report_id = field->report->id;
+	bat->report_offset = field->report_offset;
 	bat->charge_status = POWER_SUPPLY_STATUS_DISCHARGING;
 	bat->status = HID_BATTERY_UNKNOWN;
 
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 47dc0bc89..51b21f980 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -642,6 +642,7 @@ enum hid_battery_status {
  * @max: maximum battery value from HID descriptor
  * @report_type: HID report type (input/feature)
  * @report_id: HID report ID for this battery
+ * @report_offset: bit offset of the capacity field within its report
  * @charge_status: current charging status
  * @status: battery reporting status
  * @capacity: current battery capacity (0-100)
@@ -657,6 +658,7 @@ struct hid_battery {
 	__s32 max;
 	__s32 report_type;
 	__s32 report_id;
+	__s32 report_offset;
 	__s32 charge_status;
 	enum hid_battery_status status;
 	__s32 capacity;

base-commit: b7556c8e713c88596046a906c7c4385218d44736
-- 
2.55.0

Re: [PATCH v2] HID: input: read battery capacity from its actual report offset

[Alec Hall]

On Tue, Jul 28, 2026, Jose Villaseñor Montfort wrote:
> Store the battery field's offset within the report at setup time and use
> it when querying, so the capacity is read from its real position.

I can reproduce this on a second model and a second transport, and the
report descriptor explains it exactly. Magic Trackpad 2 [Lightning]
(05ac:0265) over Bluetooth, 7.1.5, battery report 0x90:

  Field(0)  Usage(3): Power.Good, BatterySystem.Charging,
                      BatterySystem.FullyCharged
            Report Size(1)  Report Count(3)  Report Offset(0)
  Field(1)  Usage(1): BatterySystem.AbsoluteStateOfCharge
            Report Size(8)  Report Count(1)  Report Offset(8)

So the capacity lives at report_offset 8, i.e. buf[2], and buf[1] is the
flags byte -- the same layout you found on the 0324.

Sampling sysfs capacity and a raw HIDIOCGINPUT of report 0x90 side by
side, from the moment the trackpad reconnects over Bluetooth:

  00:50:36  sysfs=0    raw bytes: 90 00 4f
  00:50:42  sysfs=0    raw bytes: 90 00 4f
  [...]
  00:51:35  sysfs=0    raw bytes: 90 00 4f
  00:51:40  sysfs=79   raw bytes: 90 00 4f
  00:51:44  sysfs=79   raw bytes: 90 00 4f

The device says 0x4f (79%) in byte 2 throughout and never changes. For
64 seconds the kernel reports 0%, which is buf[1] with the trackpad
discharging (flags 0x00); your 3% is the same byte with the thing plugged
in (flags 0x03). At 00:51:40 the first battery report is parsed, status
becomes HID_BATTERY_REPORTED, and reads switch to the correctly parsed
value. Only the path changed, not the device.

I checked the offset semantics the patch relies on and they hold:

- struct hid_field documents report_offset as "bit offset in the report",
  and hid_report_raw_event() strips the report ID before parsing
  (cdata++/csize-- when the enum is numbered), so the offset is relative
  to the data after the ID. 1 + report_offset/8 is the right expression.

- The +1 also survives unnumbered reports, which was my first worry.
  usbhid_get_raw_report() states "Byte 0 is the report number. Report
  data starts at byte 1" and offsets the buffer when the number is 0;
  hidp_get_raw_report() likewise puts the report number in data[0]. Byte
  0 always holds the ID. Might be worth a line in the commit message,
  since it is the obvious objection.

- Growing the request from a fixed 4 to max(len, 4) is safe on Bluetooth:
  hidp_get_raw_report() copies min(skb->len, count) rather than failing
  on a size mismatch.

Two things I would raise, neither a blocker:

1. hidinput_setup_battery() only receives the field, so bat->report_offset
   is the offset of usage 0. hid_input_field() extracts usage n at
   report_offset + n * report_size, so a device that declares the state of
   charge as a non-first usage of a multi-usage field would still read the
   wrong byte. Not the case on either of our trackpads -- above, the
   capacity is the only usage in its field -- but the flags field right
   next to it is Size(1) x Count(3), so multi-usage fields are entirely
   normal in this same descriptor. hidinput_configure_usage() already has
   usage_index, and the feature-report loop has its index too, so storing
   field->report_offset + usage_index * field->report_size would close
   that without much churn. That said, I would not want to load scope onto
   a fix that is already correct for every device we know of and is headed
   for stable -- if you would rather keep this one minimal, I am happy to
   send that refinement as a follow-up on top of yours instead.

2. offset/8 plus a single-byte read assumes report_size == 8 and a
   byte-aligned offset. True here and true of the old hardcoded buf[1],
   so this is not a regression, but a device with a 16-bit or bit-packed
   capacity would read garbage silently. A guard that falls back rather
   than mis-reading might be worth it.

One observation that supports your "permanently 3%" case: the STATUS
branch of hidinput_get_battery_property() does not just query, it also
writes the queried value into bat->capacity and sets HID_BATTERY_QUERIED.
So on a power supply that nothing ever reports, the flags byte is cached
and stays, rather than being re-read as a transient. That is consistent
with the second supply on your 0324 sitting at 3% indefinitely.

For what it is worth on the duplicate-power-supply side: my 0265 over
Bluetooth declares AbsoluteStateOfCharge only in report 0x90 and gets a
single power supply, so the duplicate pair looks specific to the 0324
descriptor rather than common to Magic Trackpad 2.

Reviewed-by: Alec Hall <signshop.alec@gmail.com>

I have not built a patched hid.ko, so no Tested-by from me -- the above is
the unpatched behaviour plus a code read. Happy to test a v3 here over
both USB and Bluetooth if that is useful.

Re: [PATCH v2] HID: input: read battery capacity from its actual report offset

[Jiri Kosina]

On Tue, 28 Jul 2026, Jose Villaseñor Montfort wrote:

> hidinput_query_battery_capacity() assumes the state-of-charge value is
> the first byte following the report ID (buf[1]) and ignores where the
> battery field actually sits within the report.
> 
> An Apple Magic Trackpad 2 precedes the AbsoluteStateOfCharge byte with a
> byte of status flags in its battery reports, so this query returns the
> flags byte instead of the charge level.
> 
> The device happens to make that easy to observe, because it exposes the
> same cell twice: its report descriptor declares AbsoluteStateOfCharge in
> two reports (0x90 and 0x9b), so hidinput_setup_battery() registers two
> power supplies. Only the first one is refreshed by hid-magicmouse -- it
> uses hid_get_battery(), which returns the first battery of the list --
> and that refresh goes through the report event path, which parses the
> field correctly. Nothing ever reports the second one, so every read of
> its capacity takes the query path above. On a USB-C Magic Trackpad over
> USB, on an unpatched 7.1.5:
> 
>   hid-<serial>-battery-144 = 100%  (Charging)      <- report event path
>   hid-<serial>-battery-155 =   3%  (Discharging)   <- query path
> 
> Both are the same physical battery. A raw HIDIOCGINPUT of the two
> reports at that same moment:
> 
>   report 0x90 -> [90 03 64]
>   report 0x9b -> [9b 03 64 64 00 00 10 00 00 00 00 00 00 00]
>                      ^flags ^SoC = 0x64 = 100%
> 
> The device answers correctly in both cases; only the offset the kernel
> reads the capacity from is wrong. 0x03 is the flags byte (present,
> charging), reported as "3%".
> 
> Bluetooth takes the same query path for its capacity, where the trackpad
> reported a bogus near-constant ~4% -- 0b100, the FullyCharged flag --
> regardless of the real charge.
> 
> Store the battery field's offset within the report at setup time and use
> it when querying, so the capacity is read from its real position. The
> report event path already parses the field correctly through the HID
> core; only the explicit GET_REPORT query was wrong.
> 
> Devices whose capacity field is the first field in the report have a
> report_offset of 0 and are unaffected (buf[1 + 0] == buf[1]).
> 
> Fixes: 581c4484769e ("HID: input: map digitizer battery usage")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jose Villaseñor Montfort <pepemontfort@gmail.com>
> ---
> No code changes since v1, only the commit message and tags.
> 
> Changes in v2:
> - Added Fixes: 581c4484769e, which introduced
>   hidinput_query_battery_capacity() with the hardcoded buf[1], and a
>   stable tag.
> - Dropped the claim in v1 that USB is unaffected. It is not: the
>   trackpad's descriptor declares AbsoluteStateOfCharge in two reports,
>   hidinput_setup_battery() registers a power supply for each, and
>   hid-magicmouse only ever refreshes the first one (hid_get_battery()
>   returns the head of the list). The second power supply therefore
>   serves every read from the broken query path, and shows 3% -- the
>   flags byte -- permanently, on USB, next to the first one showing the
>   correct 100%. That replaces the v1 example, since it puts the working
>   and the broken path on the same cell at the same instant.
> - v1: https://lore.kernel.org/linux-input/20260702192139.114809-1-pepemontfort@gmail.com/

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs