* [PATCH 0/2] HID: avoid setting up battery timer when not needed in hid-apple and magicmouse @ 2025-06-25 14:16 Aditya Garg 2025-06-25 14:16 ` [PATCH 1/2] HID: apple: avoid setting up battery timer for devices without battery Aditya Garg 2025-06-25 14:16 ` [PATCH 2/2] HID: magicmouse: avoid setting up battery timer when not needed Aditya Garg 0 siblings, 2 replies; 6+ messages in thread From: Aditya Garg @ 2025-06-25 14:16 UTC (permalink / raw) To: Jiri Kosina, Benjamin Tissoires Cc: linux-input, linux-kernel, José Expósito 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. Aditya Garg (2): HID: apple: avoid setting up battery timer for devices without battery HID: magicmouse: avoid setting up battery timer when not needed drivers/hid/hid-apple.c | 13 ++++++++----- drivers/hid/hid-magicmouse.c | 36 +++++++++++++++++++++++------------- 2 files changed, 31 insertions(+), 18 deletions(-) -- 2.43.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] HID: apple: avoid setting up battery timer for devices without battery 2025-06-25 14:16 [PATCH 0/2] HID: avoid setting up battery timer when not needed in hid-apple and magicmouse Aditya Garg @ 2025-06-25 14:16 ` Aditya Garg 2025-06-30 9:33 ` José Expósito 2025-06-25 14:16 ` [PATCH 2/2] HID: magicmouse: avoid setting up battery timer when not needed Aditya Garg 1 sibling, 1 reply; 6+ messages in thread From: Aditya Garg @ 2025-06-25 14:16 UTC (permalink / raw) To: Jiri Kosina, Benjamin Tissoires Cc: linux-input, linux-kernel, José Expósito 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 | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c index b8b99eb01..b9f45c089 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); @@ -985,7 +987,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); } -- 2.43.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] HID: apple: avoid setting up battery timer for devices without battery 2025-06-25 14:16 ` [PATCH 1/2] HID: apple: avoid setting up battery timer for devices without battery Aditya Garg @ 2025-06-30 9:33 ` José Expósito 2025-06-30 10:10 ` Aditya Garg 0 siblings, 1 reply; 6+ messages in thread From: José Expósito @ 2025-06-30 9:33 UTC (permalink / raw) To: Aditya Garg; +Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel On Wed, Jun 25, 2025 at 07:46:03PM +0530, Aditya Garg wrote: > 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 | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c > index b8b99eb01..b9f45c089 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); > + } > The same here, the `out_err:` error case uses the timer and it can be uninitialized. Jose > if (quirks & APPLE_BACKLIGHT_CTL) > apple_backlight_init(hdev); > @@ -985,7 +987,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); > } > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] HID: apple: avoid setting up battery timer for devices without battery 2025-06-30 9:33 ` José Expósito @ 2025-06-30 10:10 ` Aditya Garg 0 siblings, 0 replies; 6+ messages in thread From: Aditya Garg @ 2025-06-30 10:10 UTC (permalink / raw) To: José Expósito Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel On 30-06-2025 03:03 pm, José Expósito wrote: > On Wed, Jun 25, 2025 at 07:46:03PM +0530, Aditya Garg wrote: >> 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 | 13 ++++++++----- >> 1 file changed, 8 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c >> index b8b99eb01..b9f45c089 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); >> + } >> > > The same here, the `out_err:` error case uses the timer and it can > be uninitialized. Ah yes, good catch > > Jose > >> if (quirks & APPLE_BACKLIGHT_CTL) >> apple_backlight_init(hdev); >> @@ -985,7 +987,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); >> } >> -- >> 2.43.0 >> ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] HID: magicmouse: avoid setting up battery timer when not needed 2025-06-25 14:16 [PATCH 0/2] HID: avoid setting up battery timer when not needed in hid-apple and magicmouse Aditya Garg 2025-06-25 14:16 ` [PATCH 1/2] HID: apple: avoid setting up battery timer for devices without battery Aditya Garg @ 2025-06-25 14:16 ` Aditya Garg 2025-06-30 9:32 ` José Expósito 1 sibling, 1 reply; 6+ messages in thread From: Aditya Garg @ 2025-06-25 14:16 UTC (permalink / raw) To: Jiri Kosina, Benjamin Tissoires Cc: linux-input, linux-kernel, José Expósito 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 | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c index f49405d38..3e531905b 100644 --- a/drivers/hid/hid-magicmouse.c +++ b/drivers/hid/hid-magicmouse.c @@ -863,18 +863,22 @@ 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))) - 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 (!msc->input) { hid_err(hdev, "magicmouse input not registered\n"); @@ -947,7 +951,13 @@ 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)) + + timer_delete_sync(&msc->battery_timer); } hid_hw_stop(hdev); -- 2.43.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] HID: magicmouse: avoid setting up battery timer when not needed 2025-06-25 14:16 ` [PATCH 2/2] HID: magicmouse: avoid setting up battery timer when not needed Aditya Garg @ 2025-06-30 9:32 ` José Expósito 0 siblings, 0 replies; 6+ messages in thread From: José Expósito @ 2025-06-30 9:32 UTC (permalink / raw) To: Aditya Garg; +Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel Hi Aditya, On Wed, Jun 25, 2025 at 07:46:04PM +0530, Aditya Garg wrote: > 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 | 36 +++++++++++++++++++++++------------- > 1 file changed, 23 insertions(+), 13 deletions(-) > > diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c > index f49405d38..3e531905b 100644 > --- a/drivers/hid/hid-magicmouse.c > +++ b/drivers/hid/hid-magicmouse.c > @@ -863,18 +863,22 @@ 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))) > - 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; > + } Instead of duplicating these conditions here and in magicmouse_remove(), you could move them into a helper function. Also, watch out the `err_stop_hw:` error case, the timer could be used there uninitialized. Jose > if (!msc->input) { > hid_err(hdev, "magicmouse input not registered\n"); > @@ -947,7 +951,13 @@ 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)) > + > + timer_delete_sync(&msc->battery_timer); > } > > hid_hw_stop(hdev); > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-06-30 10:10 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-06-25 14:16 [PATCH 0/2] HID: avoid setting up battery timer when not needed in hid-apple and magicmouse Aditya Garg 2025-06-25 14:16 ` [PATCH 1/2] HID: apple: avoid setting up battery timer for devices without battery Aditya Garg 2025-06-30 9:33 ` José Expósito 2025-06-30 10:10 ` Aditya Garg 2025-06-25 14:16 ` [PATCH 2/2] HID: magicmouse: avoid setting up battery timer when not needed Aditya Garg 2025-06-30 9:32 ` José Expósito
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox