[PATCH 0/3] HID: Apple Magic Keyboard/Trackpad battery over Bluetooth

[PATCH 0/3] HID: Apple Magic Keyboard/Trackpad battery over Bluetooth

[Alec Hall]

Apple's Magic Keyboard and Magic Trackpad 2 / Magic Mouse 2 report their
battery to the host, but the kernel only surfaces it over USB; over
Bluetooth the power_supply is stuck at 0%, even though these devices are
used over Bluetooth nearly all the time.

Both drivers already fetch the battery (a GET_REPORT for input report
0x90) but only over USB. Enabling it over Bluetooth naively hard-locks the
machine: the fetch runs from a timer callback (atomic context) and the
uhid transport sleeps inside hid_hw_request(). Each driver's patch
therefore moves the fetch to a workqueue before enabling it over
Bluetooth. The last patch teaches hid-magicmouse to report charge status
from the device's status byte.

Patch 3 interprets a vendor status byte whose bit meanings were determined
empirically; I can rework it as a report-descriptor fixup exposing
HID_BAT_CHARGING instead if reviewers prefer.

Tested on a Magic Keyboard 2021 and a Magic Trackpad 2 over BlueZ (uhid).

Alec Hall (3):
  HID: apple: report Magic Keyboard battery over Bluetooth
  HID: magicmouse: report battery over Bluetooth
  HID: magicmouse: report charge status over Bluetooth

 drivers/hid/hid-apple.c      | 36 ++++++-------
 drivers/hid/hid-magicmouse.c | 99 ++++++++++++++++++++++++++----------
 2 files changed, 92 insertions(+), 43 deletions(-)


base-commit: b7556c8e713c88596046a906c7c4385218d44736
-- 
2.55.0

[PATCH 1/3] HID: apple: report Magic Keyboard battery over Bluetooth

[Alec Hall]

The Magic Keyboard answers a GET_REPORT for battery input report 0x90
over Bluetooth exactly as it does over USB, but battery reporting was
only ever enabled for USB: the Bluetooth device-table entry lacks the
APPLE_RDESC_BATTERY quirk, so apple_fetch_battery() returns early and
the kernel's power_supply is stuck at 0%.

The USB-only restriction is not cosmetic. apple_fetch_battery() issues
its GET_REPORT through hid_hw_request(), and over Bluetooth that goes
through the uhid transport, whose raw_request sleeps waiting on user
space. The fetch was driven from a timer callback
(apple_battery_timer_tick), which runs in atomic softirq context where
sleeping is forbidden, so requesting the battery there deadlocks the
machine:

    run_timer_softirq
     __run_timer_base
      apple_battery_timer_tick
       __hid_request
        uhid_hid_raw_request   /* sleeps in atomic context */

Move the periodic battery fetch to a delayed work item, which runs in
process context and may sleep, and set APPLE_RDESC_BATTERY on the
Bluetooth Magic Keyboard 2021 entry. The descriptor fixup guarded by
the quirk only fires for the 83-byte USB report descriptor, so enabling
the quirk over Bluetooth does not disturb the (different) Bluetooth
descriptor. Also stop returning early once capacity reaches max: over
Bluetooth the device sends no unsolicited updates, so the driver must
keep polling to notice the level fall after a full charge.

Only the plain Magic Keyboard 2021 is enabled and tested here; the
fingerprint, numpad and 2024 Bluetooth variants likely need the same
change but were not available to test.

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Alec Hall <signshop.alec@gmail.com>
---
 drivers/hid/hid-apple.c | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index bf7dd0fbf249..e46a59844a30 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -23,6 +23,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/timer.h>
+#include <linux/workqueue.h>
 #include <linux/string.h>
 #include <linux/leds.h>
 #include <dt-bindings/leds/common.h>
@@ -116,7 +117,7 @@ struct apple_sc {
 	unsigned int fn_on;
 	unsigned int fn_found;
 	DECLARE_BITMAP(pressed_numlock, KEY_CNT);
-	struct timer_list battery_timer;
+	struct delayed_work battery_work;
 	struct apple_sc_backlight *backlight;
 };
 
@@ -635,9 +636,6 @@ static int apple_fetch_battery(struct hid_device *hdev)
 	if (!report || report->maxfield < 1)
 		return -1;
 
-	if (bat->capacity == bat->max)
-		return -1;
-
 	hid_hw_request(hdev, report, HID_REQ_GET_REPORT);
 	return 0;
 #else
@@ -645,15 +643,20 @@ static int apple_fetch_battery(struct hid_device *hdev)
 #endif
 }
 
-static void apple_battery_timer_tick(struct timer_list *t)
+static void apple_battery_work(struct work_struct *work)
 {
-	struct apple_sc *asc = timer_container_of(asc, t, battery_timer);
+	struct apple_sc *asc = container_of(work, struct apple_sc, battery_work.work);
 	struct hid_device *hdev = asc->hdev;
 
-	if (apple_fetch_battery(hdev) == 0) {
-		mod_timer(&asc->battery_timer,
-			  jiffies + secs_to_jiffies(APPLE_BATTERY_TIMEOUT_SEC));
-	}
+	/*
+	 * Runs in process context (workqueue), so the battery GET_REPORT is
+	 * allowed to sleep. This is required for the uhid/Bluetooth transport,
+	 * whose raw_request blocks waiting on userspace -- unlike a timer_list
+	 * callback, which runs in atomic softirq context and would deadlock.
+	 */
+	if (apple_fetch_battery(hdev) == 0)
+		schedule_delayed_work(&asc->battery_work,
+				      secs_to_jiffies(APPLE_BATTERY_TIMEOUT_SEC));
 }
 
 /*
@@ -968,10 +971,9 @@ static int apple_probe(struct hid_device *hdev,
 	}
 
 	if (quirks & APPLE_RDESC_BATTERY) {
-		timer_setup(&asc->battery_timer, apple_battery_timer_tick, 0);
-		mod_timer(&asc->battery_timer,
-			  jiffies + secs_to_jiffies(APPLE_BATTERY_TIMEOUT_SEC));
-		apple_fetch_battery(hdev);
+		INIT_DELAYED_WORK(&asc->battery_work, apple_battery_work);
+		/* Kick an initial fetch; the work re-arms itself every timeout. */
+		schedule_delayed_work(&asc->battery_work, 0);
 	}
 
 	if (quirks & APPLE_BACKLIGHT_CTL)
@@ -987,7 +989,7 @@ static int apple_probe(struct hid_device *hdev,
 
 out_err:
 	if (quirks & APPLE_RDESC_BATTERY)
-		timer_delete_sync(&asc->battery_timer);
+		cancel_delayed_work_sync(&asc->battery_work);
 
 	hid_hw_stop(hdev);
 	return ret;
@@ -998,7 +1000,7 @@ static void apple_remove(struct hid_device *hdev)
 	struct apple_sc *asc = hid_get_drvdata(hdev);
 
 	if (asc->quirks & APPLE_RDESC_BATTERY)
-		timer_delete_sync(&asc->battery_timer);
+		cancel_delayed_work_sync(&asc->battery_work);
 
 	hid_hw_stop(hdev);
 }
@@ -1201,7 +1203,7 @@ static const struct hid_device_id apple_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_2021),
 		.driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK | APPLE_RDESC_BATTERY },
 	{ HID_BLUETOOTH_DEVICE(BT_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_2021),
-		.driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK },
+		.driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK | APPLE_RDESC_BATTERY },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_FINGERPRINT_2021),
 		.driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK | APPLE_RDESC_BATTERY },
 	{ HID_BLUETOOTH_DEVICE(BT_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_FINGERPRINT_2021),
-- 
2.55.0

[PATCH 2/3] HID: magicmouse: report battery over Bluetooth

[Alec Hall]

magicmouse_fetch_battery() only runs for USB Magic Mouse 2 / Magic
Trackpad 2. Over Bluetooth the kernel never queries the battery and the
power_supply reports 0%, even though the device answers a GET_REPORT for
battery input report 0x90. (A Magic Trackpad 2 appears to work while
charging only because it then pushes battery reports unsolicited; off
the charger the reading goes stale.)

As with hid-apple, the fetch cannot run from the battery timer over
Bluetooth: hid_hw_request() sleeps on the uhid transport, but
magicmouse_battery_timer_tick() runs in atomic softirq context. Move
the periodic fetch to a delayed work item, add magicmouse_has_battery()
and use it to arm the fetch for Magic Mouse 2 / Magic Trackpad 2
regardless of transport, and keep polling once full so the reading
stays current over Bluetooth.

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Alec Hall <signshop.alec@gmail.com>
---
 drivers/hid/hid-magicmouse.c | 62 +++++++++++++++++++++---------------
 1 file changed, 36 insertions(+), 26 deletions(-)

diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 802a3479e24b..6d0e76314b10 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -123,7 +123,7 @@ MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state fie
  * @tracking_ids: Mapping of current touch input data to @touches.
  * @hdev: Pointer to the underlying HID device.
  * @work: Workqueue to handle initialization retry for quirky devices.
- * @battery_timer: Timer for obtaining battery level information.
+ * @battery_work: Delayed work for obtaining battery level information.
  */
 struct magicmouse_sc {
 	struct input_dev *input;
@@ -148,7 +148,7 @@ struct magicmouse_sc {
 
 	struct hid_device *hdev;
 	struct delayed_work work;
-	struct timer_list battery_timer;
+	struct delayed_work battery_work;
 };
 
 static int magicmouse_firm_touch(struct magicmouse_sc *msc)
@@ -828,6 +828,19 @@ static bool is_usb_magictrackpad2(__u32 vendor, __u32 product)
 	       product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC;
 }
 
+/*
+ * Magic Mouse 2 and Magic Trackpad 2 expose a battery over both USB and
+ * Bluetooth (the pre-2 Magic Mouse/Trackpad do not). Transport-agnostic:
+ * over Bluetooth the vendor is BT_VENDOR_ID_APPLE but the product id matches.
+ */
+static bool magicmouse_has_battery(struct hid_device *hdev)
+{
+	return hdev->product == USB_DEVICE_ID_APPLE_MAGICMOUSE2 ||
+	       hdev->product == USB_DEVICE_ID_APPLE_MAGICMOUSE2_USBC ||
+	       hdev->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 ||
+	       hdev->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC;
+}
+
 static int magicmouse_fetch_battery(struct hid_device *hdev)
 {
 #ifdef CONFIG_HID_BATTERY_STRENGTH
@@ -836,9 +849,7 @@ static int magicmouse_fetch_battery(struct hid_device *hdev)
 	struct hid_battery *bat;
 
 	bat = hid_get_battery(hdev);
-	if (!bat ||
-	    (!is_usb_magicmouse2(hdev->vendor, hdev->product) &&
-	     !is_usb_magictrackpad2(hdev->vendor, hdev->product)))
+	if (!bat || !magicmouse_has_battery(hdev))
 		return -1;
 
 	report_enum = &hdev->report_enum[bat->report_type];
@@ -847,9 +858,6 @@ static int magicmouse_fetch_battery(struct hid_device *hdev)
 	if (!report || report->maxfield < 1)
 		return -1;
 
-	if (bat->capacity == bat->max)
-		return -1;
-
 	hid_hw_request(hdev, report, HID_REQ_GET_REPORT);
 	return 0;
 #else
@@ -857,15 +865,21 @@ static int magicmouse_fetch_battery(struct hid_device *hdev)
 #endif
 }
 
-static void magicmouse_battery_timer_tick(struct timer_list *t)
+static void magicmouse_battery_work(struct work_struct *work)
 {
-	struct magicmouse_sc *msc = timer_container_of(msc, t, battery_timer);
+	struct magicmouse_sc *msc = container_of(work, struct magicmouse_sc,
+						 battery_work.work);
 	struct hid_device *hdev = msc->hdev;
 
-	if (magicmouse_fetch_battery(hdev) == 0) {
-		mod_timer(&msc->battery_timer,
-			  jiffies + secs_to_jiffies(USB_BATTERY_TIMEOUT_SEC));
-	}
+	/*
+	 * Runs in process context (workqueue), so the battery GET_REPORT may
+	 * sleep. This is required for the uhid/Bluetooth transport, whose
+	 * raw_request blocks on userspace -- unlike a timer_list callback,
+	 * which runs in atomic softirq context and would deadlock.
+	 */
+	if (magicmouse_fetch_battery(hdev) == 0)
+		schedule_delayed_work(&msc->battery_work,
+				      secs_to_jiffies(USB_BATTERY_TIMEOUT_SEC));
 }
 
 static int magicmouse_probe(struct hid_device *hdev,
@@ -900,12 +914,10 @@ static int magicmouse_probe(struct hid_device *hdev,
 		return ret;
 	}
 
-	if (is_usb_magicmouse2(id->vendor, id->product) ||
-	    is_usb_magictrackpad2(id->vendor, id->product)) {
-		timer_setup(&msc->battery_timer, magicmouse_battery_timer_tick, 0);
-		mod_timer(&msc->battery_timer,
-			  jiffies + secs_to_jiffies(USB_BATTERY_TIMEOUT_SEC));
-		magicmouse_fetch_battery(hdev);
+	if (magicmouse_has_battery(hdev)) {
+		INIT_DELAYED_WORK(&msc->battery_work, magicmouse_battery_work);
+		/* Kick an initial fetch; the work re-arms itself each timeout. */
+		schedule_delayed_work(&msc->battery_work, 0);
 	}
 
 	if (is_usb_magicmouse2(id->vendor, id->product) ||
@@ -973,9 +985,8 @@ static int magicmouse_probe(struct hid_device *hdev,
 
 	return 0;
 err_stop_hw:
-	if (is_usb_magicmouse2(id->vendor, id->product) ||
-	    is_usb_magictrackpad2(id->vendor, id->product))
-		timer_delete_sync(&msc->battery_timer);
+	if (magicmouse_has_battery(hdev))
+		cancel_delayed_work_sync(&msc->battery_work);
 
 	hid_hw_stop(hdev);
 	return ret;
@@ -987,9 +998,8 @@ static void magicmouse_remove(struct hid_device *hdev)
 
 	if (msc) {
 		cancel_delayed_work_sync(&msc->work);
-		if (is_usb_magicmouse2(hdev->vendor, hdev->product) ||
-		    is_usb_magictrackpad2(hdev->vendor, hdev->product))
-			timer_delete_sync(&msc->battery_timer);
+		if (magicmouse_has_battery(hdev))
+			cancel_delayed_work_sync(&msc->battery_work);
 	}
 
 	hid_hw_stop(hdev);
-- 
2.55.0

[PATCH 3/3] HID: magicmouse: report charge status over Bluetooth

[Alec Hall]

Battery input report 0x90 carries a status byte that the generic HID
battery code does not map to HID_BAT_CHARGING, so charge_status keeps
its POWER_SUPPLY_STATUS_DISCHARGING default and user space never learns
whether the device is on external power (e.g. a Magic Trackpad 2 shows
"discharging" by default even while charging, and cannot report
"discharging" the moment the cable is pulled at 100%).

Parse the status byte in raw_event and set the battery charge status
accordingly (bit 1 = external power, bit 0 = charge complete), notifying
user space with power_supply_changed(). The bit meanings were determined
by observation on a Magic Trackpad 2 rather than from documentation.

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Alec Hall <signshop.alec@gmail.com>
---
 drivers/hid/hid-magicmouse.c | 37 ++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 6d0e76314b10..8bb791efef3a 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/workqueue.h>
+#include <linux/power_supply.h>
 
 #include "hid-ids.h"
 
@@ -60,6 +61,10 @@ MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state fie
 #define MOUSE_REPORT_ID    0x29
 #define MOUSE2_REPORT_ID   0x12
 #define DOUBLE_REPORT_ID   0xf7
+/* Battery Input Report 0x90 = [report_id, status, capacity]. */
+#define MAGICMOUSE_BATTERY_REPORT_ID	0x90
+#define MAGICMOUSE_BATTERY_POWERED	0x02	/* external power connected */
+#define MAGICMOUSE_BATTERY_CHARGED	0x01	/* charge complete (on charger) */
 #define USB_BATTERY_TIMEOUT_SEC 60
 
 /* These definitions are not precise, but they're close enough.  (Bits
@@ -383,6 +388,35 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda
 	}
 }
 
+/*
+ * The battery report carries a charge-status byte the generic HID battery code
+ * doesn't map, so translate the vendor status bits into a power_supply status.
+ * This makes charging/discharging honest -- e.g. "discharging" as soon as the
+ * cable is pulled, even at 100%.
+ */
+static void magicmouse_report_charge_status(struct hid_device *hdev, u8 status)
+{
+#ifdef CONFIG_HID_BATTERY_STRENGTH
+	struct hid_battery *bat = hid_get_battery(hdev);
+	int cs;
+
+	if (!bat || !bat->ps)
+		return;
+
+	if (!(status & MAGICMOUSE_BATTERY_POWERED))
+		cs = POWER_SUPPLY_STATUS_DISCHARGING;
+	else if (status & MAGICMOUSE_BATTERY_CHARGED)
+		cs = POWER_SUPPLY_STATUS_FULL;
+	else
+		cs = POWER_SUPPLY_STATUS_CHARGING;
+
+	if (bat->charge_status != cs) {
+		bat->charge_status = cs;
+		power_supply_changed(bat->ps);
+	}
+#endif
+}
+
 static int magicmouse_raw_event(struct hid_device *hdev,
 		struct hid_report *report, u8 *data, int size)
 {
@@ -394,6 +428,9 @@ static int magicmouse_raw_event(struct hid_device *hdev,
 	if (size < 1)
 		return 0;
 
+	if (data[0] == MAGICMOUSE_BATTERY_REPORT_ID && size >= 3)
+		magicmouse_report_charge_status(hdev, data[1]);
+
 	switch (data[0]) {
 	case TRACKPAD_REPORT_ID:
 	case TRACKPAD2_BT_REPORT_ID:
-- 
2.55.0

Re: [PATCH 3/3] HID: magicmouse: report charge status over Bluetooth

[Jose Villaseñor Montfort]

Hi Alec,

Heads-up on a textual overlap: I posted an independent fix earlier today
that also touches magicmouse_raw_event(), so it will conflict with this
patch:

  HID: magicmouse: prevent unbounded recursion in magicmouse_raw_event()
  https://lore.kernel.org/linux-input/20260715053526.574725-1-pepemontfort@gmail.com/

It bounds the DOUBLE_REPORT_ID recursion -- a malicious device can chain
0xf7 packets and drive magicmouse_raw_event() deep enough to overflow the
kernel stack. Mechanically it renames the body to
__magicmouse_raw_event(..., bool nested) and adds a small wrapper, which
lands in the same region where your 3/3 inserts
magicmouse_report_charge_status() and its call site. We share the same
base-commit, so whichever goes in first the other needs a trivial rebase.

No action needed on your side; I just wanted it on the radar so it is not
a surprise at apply time. I'm happy to rebase mine on top of your series,
or the maintainers can take them in either order -- the two changes are
independent and straightforward to combine.

Thanks,
Jose

Re: [PATCH 3/3] HID: magicmouse: report charge status over Bluetooth

[Alec Hall]

Hi Jose,

Thanks for the heads-up, and for fixing this.

> whichever goes in first the other needs a trivial rebase [...] I'm happy to
> rebase mine on top of your series, or the maintainers can take them in
> either order

Agreed, it's trivial either way. My 3/3 only adds a battery-report check and
its call near the top of magicmouse_raw_event(), so rebasing it onto your
__magicmouse_raw_event()/wrapper split is just moving that hunk. I'm fine with
whatever order is least work for the maintainers -- if yours lands first I'll
respin 3/3 on top.

For what it's worth, your recursion bound is one of two issues an automated
review (Sashiko) raised on my series; the other was a potential NULL
msc->input dereference in the same function (a device that takes the early
return in magicmouse_probe() leaves msc->input NULL, then a raw report
dereferences it). I'm happy to send a small guard for that as a follow-up so
between us both are covered -- and I'll coordinate the context so we don't
collide again.

Thanks,
Alec

Re: [PATCH 3/3] HID: magicmouse: report charge status over Bluetooth

[Jose Villaseñor Montfort]

Hi Alec,

Thanks, that all sounds good -- if mine lands first, great; the wrapper
split is exactly as you describe, so rebasing 3/3 is just relocating your
report-check hunk.

On the NULL msc->input deref: good timing -- I actually already have a fix
for that one ready, from digging into the same Sashiko review on your
series. Rather than a guard in the callbacks, I went at the root cause in
magicmouse_probe(): the USB Magic Mouse 2 / Trackpad 2 path returns 0
before the existing "input not registered" check, so I moved that check
ahead of the early return. That rejects a device that bound without an
input and covers both ->raw_event and ->event in one go.

I just sent it as a separate patch so we don't duplicate -- happy for you
to review, and shout if you'd rather take it instead.

Thanks,
Jose

Re: [PATCH 3/3] HID: magicmouse: report charge status over Bluetooth

[Alec Hall]

Hi Jose,

Great -- doing it in magicmouse_probe() is the better fix. Moving the
"input not registered" check ahead of the early return rejects the bad
device up front and covers both ->raw_event and ->event, which is cleaner
than a guard in the hot path. No need for me to take it; please keep it.

I'll review both patches and give them a run on my hardware (Magic Keyboard
2021 and Magic Trackpad 2 over Bluetooth) to confirm the normal
enumeration and battery path is unaffected, and follow up with
Reviewed-by/Tested-by on the respective threads.

Agreed on the rebase too -- if yours land first I'll just relocate the 3/3
report-check hunk into the new wrapper.

Thanks for taking both of these,
Alec