On 11. 02. 25, 23:46, 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
No \n ^^ here.
Fixes: 6ea2a6fd3872 ("HID: corsair-void: Add Corsair Void headset family driver") Cc: stable@vger.kernel.org Signed-off-by: Stuart Hayhurst stuart.a.hayhurst@gmail.com
--- a/drivers/hid/hid-corsair-void.c +++ b/drivers/hid/hid-corsair-void.c
...
@@ -107,6 +105,10 @@ #define CORSAIR_VOID_SIDETONE_MAX_WIRELESS 55 #define CORSAIR_VOID_SIDETONE_MAX_WIRED 4096 +#define CORSAIR_VOID_ADD_BATTERY BIT(0) +#define CORSAIR_VOID_REMOVE_BATTERY BIT(1) +#define CORSAIR_VOID_UPDATE_BATTERY BIT(2)
I would do an enum, but it's a matter of taste/preference.
@@ -583,16 +565,48 @@ 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);- /* Add, remove or skip battery */
What is to skip a battery? Anyway, the comments here seem to be superfluous as the code is obvious™.
- if (add_battery && !remove_battery) {
corsair_void_add_battery(drvdata);- } else if (remove_battery && !add_battery) {
if (drvdata->battery) {
Perhaps '&& drvdata->battery' instead of the nested 'if'?
power_supply_unregister(drvdata->battery);drvdata->battery = NULL;}- }
- /* Communicate that battery values changed */
- if (update_battery) {
if (drvdata->battery)
Ditto.
power_supply_changed(drvdata->battery);- }
+}
Overall, LGTM.
thanks,