This series fixes some missing clean-up function calls in the error handling of
the probe.
Patch 1 and 2 fix some similar issues introduced in 2 different commits (hence 2
patches)
Patch 3 is an enhancement that creates a common function for cleaning up
thunderstrike instances.
Changes:
v1->v2:
- Add the LED_RETAIN_AT_SHUTDOWN flag to prevent
led_classdev_unregister from trying to set the LED to off before a
successful call to hid_hw_start.
- Rename err_haptics label to err_ts_create to make the label name more
accurate.
- Re-order operations in thunderstrike_destroy to be in LIFO order with
regards to the operations in thunderstrike_create.
v2->v3:
- Refactor thunderstrike_destroy to take a thunderstrike instance
pointer as a parameter and prevent a variable from being unused
in shield_probe.
Link: https://lore.kernel.org/linux-input/[email protected]/
Link: https://lore.kernel.org/linux-input/[email protected]/
Notes from Rahul:
- Thank you so much Christophe for these patches.
- Sent v2 without accounting for the fact that thunderstrike_destroy in v1
makes the thunderstrike instance in shield_probe unused. Tested v3 with W=1.
Christophe JAILLET (3):
HID: nvidia-shield: Fix a missing led_classdev_unregister() in the
probe error handling path
HID: nvidia-shield: Fix some missing function calls() in the probe
error handling path
HID: nvidia-shield: Introduce thunderstrike_destroy()
drivers/hid/hid-nvidia-shield.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
--
2.40.1
From: Christophe JAILLET <[email protected]>
In order to simplify some error handling paths and avoid code duplication,
introduce thunderstrike_destroy() which undoes thunderstrike_create().
Signed-off-by: Christophe JAILLET <[email protected]>
Reviewed-by: Rahul Rameshbabu <[email protected]>
---
Notes:
Changes:
v1->v2:
- Re-order operations in thunderstrike_destroy to be in LIFO order with
regards to the operations in thunderstrike_create.
v2->v3:
- Refactor thunderstrike_destroy to take a thunderstrike instance
pointer as a parameter and prevent a variable from being unused
in shield_probe.
drivers/hid/hid-nvidia-shield.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/hid/hid-nvidia-shield.c b/drivers/hid/hid-nvidia-shield.c
index a566f9cdc97d..bc47e19ef117 100644
--- a/drivers/hid/hid-nvidia-shield.c
+++ b/drivers/hid/hid-nvidia-shield.c
@@ -915,6 +915,15 @@ static struct shield_device *thunderstrike_create(struct hid_device *hdev)
return ERR_PTR(ret);
}
+static void thunderstrike_destroy(struct thunderstrike *ts)
+{
+ led_classdev_unregister(&ts->led_dev);
+ power_supply_unregister(ts->base.battery_dev.psy);
+ if (ts->haptics_dev)
+ input_unregister_device(ts->haptics_dev);
+ ida_free(&thunderstrike_ida, ts->id);
+}
+
static int android_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field,
struct hid_usage *usage, unsigned long **bit,
@@ -1074,11 +1083,7 @@ static int shield_probe(struct hid_device *hdev, const struct hid_device_id *id)
err_stop:
hid_hw_stop(hdev);
err_ts_create:
- power_supply_unregister(ts->base.battery_dev.psy);
- if (ts->haptics_dev)
- input_unregister_device(ts->haptics_dev);
- led_classdev_unregister(&ts->led_dev);
- ida_free(&thunderstrike_ida, ts->id);
+ thunderstrike_destroy(ts);
return ret;
}
@@ -1090,11 +1095,7 @@ static void shield_remove(struct hid_device *hdev)
ts = container_of(dev, struct thunderstrike, base);
hid_hw_close(hdev);
- power_supply_unregister(dev->battery_dev.psy);
- if (ts->haptics_dev)
- input_unregister_device(ts->haptics_dev);
- led_classdev_unregister(&ts->led_dev);
- ida_free(&thunderstrike_ida, ts->id);
+ thunderstrike_destroy(ts);
del_timer_sync(&ts->psy_stats_timer);
cancel_work_sync(&ts->hostcmd_req_work);
hid_hw_stop(hdev);
--
2.40.1
From: Christophe JAILLET <[email protected]>
The commit in Fixes updated the error handling path of
thunderstrike_create() and the remove function but not the error handling
path of shield_probe(), should an error occur after a successful
thunderstrike_create() call.
Add the missing call. Make sure it is safe to call in the probe error
handling path by preventing the led_classdev from attempting to set the LED
brightness to the off state on unregister.
Fixes: f88af60e74a5 ("HID: nvidia-shield: Support LED functionality for Thunderstrike")
Signed-off-by: Christophe JAILLET <[email protected]>
Reviewed-by: Rahul Rameshbabu <[email protected]>
---
Notes:
Changes:
v1->v2:
- Add the LED_RETAIN_AT_SHUTDOWN flag to prevent
led_classdev_unregister from trying to set the LED to off before a
successful call to hid_hw_start.
v2->v3:
- No changes.
drivers/hid/hid-nvidia-shield.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-nvidia-shield.c b/drivers/hid/hid-nvidia-shield.c
index 43784bb57d3f..c144641452d3 100644
--- a/drivers/hid/hid-nvidia-shield.c
+++ b/drivers/hid/hid-nvidia-shield.c
@@ -801,7 +801,7 @@ static inline int thunderstrike_led_create(struct thunderstrike *ts)
led->name = devm_kasprintf(&ts->base.hdev->dev, GFP_KERNEL,
"thunderstrike%d:blue:led", ts->id);
led->max_brightness = 1;
- led->flags = LED_CORE_SUSPENDRESUME;
+ led->flags = LED_CORE_SUSPENDRESUME | LED_RETAIN_AT_SHUTDOWN;
led->brightness_get = &thunderstrike_led_get_brightness;
led->brightness_set = &thunderstrike_led_set_brightness;
@@ -1076,6 +1076,7 @@ static int shield_probe(struct hid_device *hdev, const struct hid_device_id *id)
err_haptics:
if (ts->haptics_dev)
input_unregister_device(ts->haptics_dev);
+ led_classdev_unregister(&ts->led_dev);
return ret;
}
--
2.40.1
On Mon, 18 Sep 2023, Rahul Rameshbabu wrote:
> This series fixes some missing clean-up function calls in the error handling of
> the probe.
>
> Patch 1 and 2 fix some similar issues introduced in 2 different commits (hence 2
> patches)
>
> Patch 3 is an enhancement that creates a common function for cleaning up
> thunderstrike instances.
>
> Changes:
>
> v1->v2:
> - Add the LED_RETAIN_AT_SHUTDOWN flag to prevent
> led_classdev_unregister from trying to set the LED to off before a
> successful call to hid_hw_start.
> - Rename err_haptics label to err_ts_create to make the label name more
> accurate.
> - Re-order operations in thunderstrike_destroy to be in LIFO order with
> regards to the operations in thunderstrike_create.
> v2->v3:
> - Refactor thunderstrike_destroy to take a thunderstrike instance
> pointer as a parameter and prevent a variable from being unused
> in shield_probe.
>
> Link: https://lore.kernel.org/linux-input/[email protected]/
> Link: https://lore.kernel.org/linux-input/[email protected]/
>
> Notes from Rahul:
> - Thank you so much Christophe for these patches.
> - Sent v2 without accounting for the fact that thunderstrike_destroy in v1
> makes the thunderstrike instance in shield_probe unused. Tested v3 with W=1.
>
> Christophe JAILLET (3):
> HID: nvidia-shield: Fix a missing led_classdev_unregister() in the
> probe error handling path
> HID: nvidia-shield: Fix some missing function calls() in the probe
> error handling path
> HID: nvidia-shield: Introduce thunderstrike_destroy()
I have applied 1/3 and 2/3 to for-6.6/upstream-fixes and 3/3 to
for-6.7/nvidia-shield.
Thanks,
--
Jiri Kosina
SUSE Labs