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

[Alec Hall <signshop.alec@gmail.com>]

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.