On 13. 02. 25, 14:38, Stuart Hayhurst wrote:
corsair_void_process_receiver can be called from an interrupt context,
locking battery_mutex in it was causing a kernel panic.
Fix it by moving the critical section into its own work, sharing this
work with battery_add_work and battery_remove_work to remove the need
for any locking
Closes: https://bugzilla.suse.com/show_bug.cgi?id=1236843
Fixes: 6ea2a6fd3872 ("HID: corsair-void: Add Corsair Void headset family driver")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Stuart Hayhurst <stuart.a.hayhurst@xxxxxxxxx>
Reviewed-by: Jiri Slaby <jirislaby@xxxxxxxxxx>
---
v2 -> v3:
- Use an enum instead of a define for battery flag values
- Use an integer instead of BIT() for the bit index
Good catch :).
- Drop unhelpful comments
- Simplify corsair_void_battery_work_handler logic
- Remove extra newline in commit message
v1 -> v2:
- Actually remove the mutex
---
drivers/hid/hid-corsair-void.c | 83 ++++++++++++++++++----------------
1 file changed, 43 insertions(+), 40 deletions(-)
diff --git a/drivers/hid/hid-corsair-void.c b/drivers/hid/hid-corsair-void.c
index 56e858066c3c..afbd67aa9719 100644
--- a/drivers/hid/hid-corsair-void.c
+++ b/drivers/hid/hid-corsair-void.c
...
@@ -583,16 +567,42 @@ static void corsair_void_battery_add_work_handler(struct work_struct *work)
drvdata->battery = new_supply;
}
+static void corsair_void_battery_work_handler(struct work_struct *work)
+{
+ struct corsair_void_drvdata *drvdata = container_of(work,
+ struct corsair_void_drvdata, battery_work);
+
+ bool add_battery = test_and_clear_bit(CORSAIR_VOID_ADD_BATTERY,
+ &drvdata->battery_work_flags);
+ bool remove_battery = test_and_clear_bit(CORSAIR_VOID_REMOVE_BATTERY,
+ &drvdata->battery_work_flags);
+ bool update_battery = test_and_clear_bit(CORSAIR_VOID_UPDATE_BATTERY,
+ &drvdata->battery_work_flags);
+
+ if (add_battery && !remove_battery) {
+ corsair_void_add_battery(drvdata);
+ } else if (remove_battery && !add_battery && drvdata->battery) {
+ power_supply_unregister(drvdata->battery);
+ drvdata->battery = NULL;
+ }
Now I think, what is actually expected to happen if both add_battery and
remove_battery is set? Do nothing as the code does?
+ if (update_battery && drvdata->battery)
+ power_supply_changed(drvdata->battery);
+
+}
thanks,
--
js
suse labs
