[PATCH 0/2] HID: avoid setting up battery timer when not needed in hid-apple and magicmouse

[PATCH 0/2] HID: avoid setting up battery timer when not needed in hid-apple and magicmouse

[Aditya Garg]

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

[PATCH 1/2] HID: apple: avoid setting up battery timer for devices without battery

[Aditya Garg]

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

[PATCH 2/2] HID: magicmouse: avoid setting up battery timer when not needed

[Aditya Garg]

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

Re: [PATCH 2/2] HID: magicmouse: avoid setting up battery timer when not needed

[José Expósito]

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
> 

Re: [PATCH 1/2] HID: apple: avoid setting up battery timer for devices without battery

[José Expósito]

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
> 

Re: [PATCH 1/2] HID: apple: avoid setting up battery timer for devices without battery

[Aditya Garg]

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
>>