[PATCH v2 0/4] HID: avoid setting up battery timer when not needed in hid-apple and magicmouse
Both hid-apple and hid-magicmouse require set up a battery timer for certain devices in order to fetch battery status. However, the timer is being set unconditionally for all devices. This patch series introduces checks to ensure that the battery timer is only set up for devices that actually require it.
v2: - Address the cases of out_err and err_stop_hw left in v1 - Create a function to check if the device is a USB Magic Mouse 2 or Magic Trackpad 2 to reduce code duplication. - Add 2 new patches that convert the battery timeout to use secs_to_jiffies() instead of msecs_to_jiffies().
Aditya Garg (4): HID: apple: avoid setting up battery timer for devices without battery HID: magicmouse: avoid setting up battery timer when not needed HID: apple: use secs_to_jiffies() for battery timeout HID: magicmouse: use secs_to_jiffies() for battery timeout
drivers/hid/hid-apple.c | 21 +++++++----- drivers/hid/hid-magicmouse.c | 66 ++++++++++++++++++++++-------------- 2 files changed, 54 insertions(+), 33 deletions(-)
Range-diff against v1: 1: 05b6ac964 ! 1: 41c49e1d6 HID: apple: avoid setting up battery timer for devices without battery @@ drivers/hid/hid-apple.c: static int apple_probe(struct hid_device *hdev,
if (quirks & APPLE_BACKLIGHT_CTL) apple_backlight_init(hdev); +@@ drivers/hid/hid-apple.c: static int apple_probe(struct hid_device *hdev, + return 0; + + out_err: +- timer_delete_sync(&asc->battery_timer); ++ if (quirks & APPLE_RDESC_BATTERY) ++ timer_delete_sync(&asc->battery_timer); ++ + hid_hw_stop(hdev); + return ret; + } @@ drivers/hid/hid-apple.c: static void apple_remove(struct hid_device *hdev) { struct apple_sc *asc = hid_get_drvdata(hdev); 2: 25b52facf ! 2: a1617042f HID: magicmouse: avoid setting up battery timer when not needed @@ Commit message Signed-off-by: Aditya Garg gargaditya08@live.com
## drivers/hid/hid-magicmouse.c ## +@@ drivers/hid/hid-magicmouse.c: static void magicmouse_enable_mt_work(struct work_struct *work) + hid_err(msc->hdev, "unable to request touch data (%d)\n", ret); + } + ++static bool is_usb_magicmouse2(__u32 vendor, __u32 product) ++{ ++ if (vendor != USB_VENDOR_ID_APPLE) ++ return false; ++ return product == USB_DEVICE_ID_APPLE_MAGICMOUSE2 || ++ product == USB_DEVICE_ID_APPLE_MAGICMOUSE2_USBC; ++} ++ ++static bool is_usb_magictrackpad2(__u32 vendor, __u32 product) ++{ ++ if (vendor != USB_VENDOR_ID_APPLE) ++ return false; ++ return product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 || ++ product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC; ++} ++ + static int magicmouse_fetch_battery(struct hid_device *hdev) + { + #ifdef CONFIG_HID_BATTERY_STRENGTH + struct hid_report_enum *report_enum; + struct hid_report *report; + +- if (!hdev->battery || hdev->vendor != USB_VENDOR_ID_APPLE || +- (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)) ++ if (!hdev->battery || ++ (!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]; @@ drivers/hid/hid-magicmouse.c: static int magicmouse_probe(struct hid_device *hdev, return ret; } @@ drivers/hid/hid-magicmouse.c: static int magicmouse_probe(struct hid_device *hde - ((id->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 || - id->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC) && - hdev->type != HID_TYPE_USBMOUSE))) -- return 0; -+ if (id->vendor == USB_VENDOR_ID_APPLE) { -+ bool is_mouse2 = (id->product == USB_DEVICE_ID_APPLE_MAGICMOUSE2 || -+ id->product == USB_DEVICE_ID_APPLE_MAGICMOUSE2_USBC); -+ bool is_trackpad2 = (id->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 || -+ id->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC); -+ -+ if (is_mouse2 || is_trackpad2) { -+ timer_setup(&msc->battery_timer, magicmouse_battery_timer_tick, 0); -+ mod_timer(&msc->battery_timer, -+ jiffies + msecs_to_jiffies(USB_BATTERY_TIMEOUT_MS)); -+ magicmouse_fetch_battery(hdev); -+ } -+ -+ if (is_mouse2 || (is_trackpad2 && hdev->type != HID_TYPE_USBMOUSE)) -+ return 0; ++ 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 + msecs_to_jiffies(USB_BATTERY_TIMEOUT_MS)); ++ magicmouse_fetch_battery(hdev); + } ++ ++ if (is_usb_magicmouse2(id->vendor, id->product) || ++ (is_usb_magictrackpad2(id->vendor, id->product) && ++ hdev->type != HID_TYPE_USBMOUSE)) + return 0;
if (!msc->input) { - hid_err(hdev, "magicmouse input not registered\n"); +@@ drivers/hid/hid-magicmouse.c: static int magicmouse_probe(struct hid_device *hdev, + + return 0; + err_stop_hw: +- timer_delete_sync(&msc->battery_timer); ++ if (is_usb_magicmouse2(id->vendor, id->product) || ++ is_usb_magictrackpad2(id->vendor, id->product)) ++ timer_delete_sync(&msc->battery_timer); ++ + hid_hw_stop(hdev); + return ret; + } @@ drivers/hid/hid-magicmouse.c: static void magicmouse_remove(struct hid_device *hdev)
if (msc) { cancel_delayed_work_sync(&msc->work); - timer_delete_sync(&msc->battery_timer); -+ if (hdev->vendor == USB_VENDOR_ID_APPLE && -+ (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)) -+ ++ if (is_usb_magicmouse2(hdev->vendor, hdev->product) || ++ is_usb_magictrackpad2(hdev->vendor, hdev->product)) + timer_delete_sync(&msc->battery_timer); }
hid_hw_stop(hdev); +@@ drivers/hid/hid-magicmouse.c: static const __u8 *magicmouse_report_fixup(struct hid_device *hdev, __u8 *rdesc, + * 0x05, 0x01, // Usage Page (Generic Desktop) 0 + * 0x09, 0x02, // Usage (Mouse) 2 + */ +- if (hdev->vendor == USB_VENDOR_ID_APPLE && +- (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) && ++ if ((is_usb_magicmouse2(hdev->vendor, hdev->product) || ++ is_usb_magictrackpad2(hdev->vendor, hdev->product)) && + *rsize == 83 && rdesc[46] == 0x84 && rdesc[58] == 0x85) { + hid_info(hdev, + "fixing up magicmouse battery report descriptor\n"); -: --------- > 3: 3ebe25998 HID: apple: use secs_to_jiffies() for battery timeout -: --------- > 4: 1d3400714 HID: magicmouse: use secs_to_jiffies() for battery timeout
Currently, the battery timer is set up for all devices using hid-magicmouse, irrespective of whether they actually need it or not.
The current implementation requires the battery timer for Magic Mouse 2 and Magic Trackpad 2 when connected via USB only. Add checks to ensure that the battery timer is only set up when they are connected via USB.
Fixes: 0b91b4e4dae6 ("HID: magicmouse: Report battery level over USB") Cc: stable@vger.kernel.org Signed-off-by: Aditya Garg gargaditya08@live.com --- drivers/hid/hid-magicmouse.c | 62 +++++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 23 deletions(-)
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c index d4d91e49b..4ca0cbac9 100644 --- a/drivers/hid/hid-magicmouse.c +++ b/drivers/hid/hid-magicmouse.c @@ -791,17 +791,31 @@ static void magicmouse_enable_mt_work(struct work_struct *work) hid_err(msc->hdev, "unable to request touch data (%d)\n", ret); }
+static bool is_usb_magicmouse2(__u32 vendor, __u32 product) +{ + if (vendor != USB_VENDOR_ID_APPLE) + return false; + return product == USB_DEVICE_ID_APPLE_MAGICMOUSE2 || + product == USB_DEVICE_ID_APPLE_MAGICMOUSE2_USBC; +} + +static bool is_usb_magictrackpad2(__u32 vendor, __u32 product) +{ + if (vendor != USB_VENDOR_ID_APPLE) + return false; + return product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 || + product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC; +} + static int magicmouse_fetch_battery(struct hid_device *hdev) { #ifdef CONFIG_HID_BATTERY_STRENGTH struct hid_report_enum *report_enum; struct hid_report *report;
- if (!hdev->battery || hdev->vendor != USB_VENDOR_ID_APPLE || - (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)) + if (!hdev->battery || + (!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]; @@ -863,17 +877,17 @@ static int magicmouse_probe(struct hid_device *hdev, return ret; }
- timer_setup(&msc->battery_timer, magicmouse_battery_timer_tick, 0); - mod_timer(&msc->battery_timer, - jiffies + msecs_to_jiffies(USB_BATTERY_TIMEOUT_MS)); - magicmouse_fetch_battery(hdev); - - if (id->vendor == USB_VENDOR_ID_APPLE && - (id->product == USB_DEVICE_ID_APPLE_MAGICMOUSE2 || - id->product == USB_DEVICE_ID_APPLE_MAGICMOUSE2_USBC || - ((id->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 || - id->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC) && - hdev->type != HID_TYPE_USBMOUSE))) + 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 + msecs_to_jiffies(USB_BATTERY_TIMEOUT_MS)); + magicmouse_fetch_battery(hdev); + } + + if (is_usb_magicmouse2(id->vendor, id->product) || + (is_usb_magictrackpad2(id->vendor, id->product) && + hdev->type != HID_TYPE_USBMOUSE)) return 0;
if (!msc->input) { @@ -936,7 +950,10 @@ static int magicmouse_probe(struct hid_device *hdev,
return 0; err_stop_hw: - timer_delete_sync(&msc->battery_timer); + if (is_usb_magicmouse2(id->vendor, id->product) || + is_usb_magictrackpad2(id->vendor, id->product)) + timer_delete_sync(&msc->battery_timer); + hid_hw_stop(hdev); return ret; } @@ -947,7 +964,9 @@ static void magicmouse_remove(struct hid_device *hdev)
if (msc) { cancel_delayed_work_sync(&msc->work); - timer_delete_sync(&msc->battery_timer); + if (is_usb_magicmouse2(hdev->vendor, hdev->product) || + is_usb_magictrackpad2(hdev->vendor, hdev->product)) + timer_delete_sync(&msc->battery_timer); }
hid_hw_stop(hdev); @@ -964,11 +983,8 @@ static const __u8 *magicmouse_report_fixup(struct hid_device *hdev, __u8 *rdesc, * 0x05, 0x01, // Usage Page (Generic Desktop) 0 * 0x09, 0x02, // Usage (Mouse) 2 */ - if (hdev->vendor == USB_VENDOR_ID_APPLE && - (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) && + if ((is_usb_magicmouse2(hdev->vendor, hdev->product) || + is_usb_magictrackpad2(hdev->vendor, hdev->product)) && *rsize == 83 && rdesc[46] == 0x84 && rdesc[58] == 0x85) { hid_info(hdev, "fixing up magicmouse battery report descriptor\n");
Currently, the battery timer is set up for all devices using hid-apple, irrespective of whether they actually have a battery or not.
APPLE_RDESC_BATTERY is a quirk that indicates the device has a battery and needs the battery timer. This patch checks for this quirk before setting up the timer, ensuring that only devices with a battery will have the timer set up.
Fixes: 6e143293e17a ("HID: apple: Report Magic Keyboard battery over USB") Cc: stable@vger.kernel.org Signed-off-by: Aditya Garg gargaditya08@live.com --- drivers/hid/hid-apple.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c index b8b99eb01..c8f0e2446 100644 --- a/drivers/hid/hid-apple.c +++ b/drivers/hid/hid-apple.c @@ -959,10 +959,12 @@ static int apple_probe(struct hid_device *hdev, return ret; }
- timer_setup(&asc->battery_timer, apple_battery_timer_tick, 0); - mod_timer(&asc->battery_timer, - jiffies + msecs_to_jiffies(APPLE_BATTERY_TIMEOUT_MS)); - apple_fetch_battery(hdev); + if (quirks & APPLE_RDESC_BATTERY) { + timer_setup(&asc->battery_timer, apple_battery_timer_tick, 0); + mod_timer(&asc->battery_timer, + jiffies + msecs_to_jiffies(APPLE_BATTERY_TIMEOUT_MS)); + apple_fetch_battery(hdev); + }
if (quirks & APPLE_BACKLIGHT_CTL) apple_backlight_init(hdev); @@ -976,7 +978,9 @@ static int apple_probe(struct hid_device *hdev, return 0;
out_err: - timer_delete_sync(&asc->battery_timer); + if (quirks & APPLE_RDESC_BATTERY) + timer_delete_sync(&asc->battery_timer); + hid_hw_stop(hdev); return ret; } @@ -985,7 +989,8 @@ static void apple_remove(struct hid_device *hdev) { struct apple_sc *asc = hid_get_drvdata(hdev);
- timer_delete_sync(&asc->battery_timer); + if (asc->quirks & APPLE_RDESC_BATTERY) + timer_delete_sync(&asc->battery_timer);
hid_hw_stop(hdev); }
The kernel now has a secs_to_jiffies() function which expands to a simpler code than msecs_to_jiffies(). Use the same for battery timeout which was using 60000 milliseconds (60 seconds).
Signed-off-by: Aditya Garg gargaditya08@live.com --- drivers/hid/hid-magicmouse.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c index 4ca0cbac9..e933bebf6 100644 --- a/drivers/hid/hid-magicmouse.c +++ b/drivers/hid/hid-magicmouse.c @@ -60,7 +60,7 @@ 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 -#define USB_BATTERY_TIMEOUT_MS 60000 +#define USB_BATTERY_TIMEOUT_SEC 60
/* These definitions are not precise, but they're close enough. (Bits * 0x03 seem to indicate the aspect ratio of the touch, bits 0x70 seem @@ -841,7 +841,7 @@ static void magicmouse_battery_timer_tick(struct timer_list *t)
if (magicmouse_fetch_battery(hdev) == 0) { mod_timer(&msc->battery_timer, - jiffies + msecs_to_jiffies(USB_BATTERY_TIMEOUT_MS)); + jiffies + secs_to_jiffies(USB_BATTERY_TIMEOUT_SEC)); } }
@@ -881,7 +881,7 @@ static int magicmouse_probe(struct hid_device *hdev, is_usb_magictrackpad2(id->vendor, id->product)) { timer_setup(&msc->battery_timer, magicmouse_battery_timer_tick, 0); mod_timer(&msc->battery_timer, - jiffies + msecs_to_jiffies(USB_BATTERY_TIMEOUT_MS)); + jiffies + secs_to_jiffies(USB_BATTERY_TIMEOUT_SEC)); magicmouse_fetch_battery(hdev); }
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#opti...
Rule: add the tag "Cc: stable@vger.kernel.org" in the sign-off area to have the patch automatically included in the stable tree. Subject: [PATCH v2 4/4] HID: magicmouse: use secs_to_jiffies() for battery timeout Link: https://lore.kernel.org/stable/20250630123649.80395-5-gargaditya08%40live.co...
On 30-06-2025 06:09 pm, kernel test robot wrote:
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
3/4 and 4/4 of that patch series are not meant for stable. Only 1/4 and 2/4 are.
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#opti...
Rule: add the tag "Cc: stable@vger.kernel.org" in the sign-off area to have the patch automatically included in the stable tree. Subject: [PATCH v2 4/4] HID: magicmouse: use secs_to_jiffies() for battery timeout Link: https://lore.kernel.org/stable/20250630123649.80395-5-gargaditya08%40live.co...
The kernel now has a secs_to_jiffies() function which expands to a simpler code than msecs_to_jiffies(). Use the same for battery timeout which was using 60000 milliseconds (60 seconds).
Signed-off-by: Aditya Garg gargaditya08@live.com --- drivers/hid/hid-apple.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c index c8f0e2446..8ee99d603 100644 --- a/drivers/hid/hid-apple.c +++ b/drivers/hid/hid-apple.c @@ -48,7 +48,7 @@ #define APPLE_FLAG_TB_FKEY BIT(1)
#define HID_COUNTRY_INTERNATIONAL_ISO 13 -#define APPLE_BATTERY_TIMEOUT_MS 60000 +#define APPLE_BATTERY_TIMEOUT_SEC 60
#define HID_USAGE_MAGIC_BL 0xff00000f #define APPLE_MAGIC_REPORT_ID_POWER 3 @@ -645,7 +645,7 @@ static void apple_battery_timer_tick(struct timer_list *t)
if (apple_fetch_battery(hdev) == 0) { mod_timer(&asc->battery_timer, - jiffies + msecs_to_jiffies(APPLE_BATTERY_TIMEOUT_MS)); + jiffies + secs_to_jiffies(APPLE_BATTERY_TIMEOUT_SEC)); } }
@@ -962,7 +962,7 @@ 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 + msecs_to_jiffies(APPLE_BATTERY_TIMEOUT_MS)); + jiffies + secs_to_jiffies(APPLE_BATTERY_TIMEOUT_SEC)); apple_fetch_battery(hdev); }
linux-stable-mirror@lists.linaro.org
-
Aditya Garg -
kernel test robot