Commit 0c28e4d1 authored by Stuart Hayhurst's avatar Stuart Hayhurst Committed by Jiri Kosina
Browse files

HID: corsair-void: Update power supply values with a unified work handler 

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: 6ea2a6fd ("HID: corsair-void: Add Corsair Void headset family driver")
Cc: stable@vger.kernel.org
Signed-off-by: default avatarStuart Hayhurst <stuart.a.hayhurst@gmail.com>
Reviewed-by: default avatarJiri Slaby <jirislaby@kernel.org>
Signed-off-by: default avatarJiri Kosina <jkosina@suse.com>
parent b051ffa2

drivers/hid/hid-corsair-void.c

+43 −40
Original line number Diff line number Diff line
@@ -71,11 +71,9 @@ 


#include <linux/bitfield.h>

#include <linux/bitops.h>

#include <linux/cleanup.h>

#include <linux/device.h>

#include <linux/hid.h>

#include <linux/module.h>

#include <linux/mutex.h>

#include <linux/power_supply.h>

#include <linux/usb.h>

#include <linux/workqueue.h>
@@ -120,6 +118,12 @@ enum { 
	CORSAIR_VOID_BATTERY_CHARGING	= 5,

};



enum {

	CORSAIR_VOID_ADD_BATTERY	= 0,

	CORSAIR_VOID_REMOVE_BATTERY	= 1,

	CORSAIR_VOID_UPDATE_BATTERY	= 2,

};



static enum power_supply_property corsair_void_battery_props[] = {

	POWER_SUPPLY_PROP_STATUS,

	POWER_SUPPLY_PROP_PRESENT,
@@ -155,12 +159,12 @@ struct corsair_void_drvdata { 


	struct power_supply *battery;

	struct power_supply_desc battery_desc;

	struct mutex battery_mutex;



	struct delayed_work delayed_status_work;

	struct delayed_work delayed_firmware_work;

	struct work_struct battery_remove_work;

	struct work_struct battery_add_work;



	unsigned long battery_work_flags;

	struct work_struct battery_work;

};



/*
@@ -260,11 +264,9 @@ static void corsair_void_process_receiver(struct corsair_void_drvdata *drvdata, 


	/* Inform power supply if battery values changed */

	if (memcmp(&orig_battery_data, battery_data, sizeof(*battery_data))) {

		scoped_guard(mutex, &drvdata->battery_mutex) {

			if (drvdata->battery) {

				power_supply_changed(drvdata->battery);

			}

		}

		set_bit(CORSAIR_VOID_UPDATE_BATTERY,

			&drvdata->battery_work_flags);

		schedule_work(&drvdata->battery_work);

	}

}
@@ -536,29 +538,11 @@ static void corsair_void_firmware_work_handler(struct work_struct *work) 


}



static void corsair_void_battery_remove_work_handler(struct work_struct *work)

{

	struct corsair_void_drvdata *drvdata;



	drvdata = container_of(work, struct corsair_void_drvdata,

			       battery_remove_work);

	scoped_guard(mutex, &drvdata->battery_mutex) {

		if (drvdata->battery) {

			power_supply_unregister(drvdata->battery);

			drvdata->battery = NULL;

		}

	}

}



static void corsair_void_battery_add_work_handler(struct work_struct *work)

static void corsair_void_add_battery(struct corsair_void_drvdata *drvdata)

{

	struct corsair_void_drvdata *drvdata;

	struct power_supply_config psy_cfg = {};

	struct power_supply *new_supply;



	drvdata = container_of(work, struct corsair_void_drvdata,

			       battery_add_work);

	guard(mutex)(&drvdata->battery_mutex);

	if (drvdata->battery)

		return;
@@ -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;

	}



	if (update_battery && drvdata->battery)

		power_supply_changed(drvdata->battery);



}



static void corsair_void_headset_connected(struct corsair_void_drvdata *drvdata)

{

	schedule_work(&drvdata->battery_add_work);

	set_bit(CORSAIR_VOID_ADD_BATTERY, &drvdata->battery_work_flags);

	schedule_work(&drvdata->battery_work);

	schedule_delayed_work(&drvdata->delayed_firmware_work,

			      msecs_to_jiffies(100));

}



static void corsair_void_headset_disconnected(struct corsair_void_drvdata *drvdata)

{

	schedule_work(&drvdata->battery_remove_work);

	set_bit(CORSAIR_VOID_REMOVE_BATTERY, &drvdata->battery_work_flags);

	schedule_work(&drvdata->battery_work);



	corsair_void_set_unknown_wireless_data(drvdata);

	corsair_void_set_unknown_batt(drvdata);
@@ -678,13 +688,7 @@ static int corsair_void_probe(struct hid_device *hid_dev, 
	drvdata->battery_desc.get_property = corsair_void_battery_get_property;



	drvdata->battery = NULL;

	INIT_WORK(&drvdata->battery_remove_work,

		  corsair_void_battery_remove_work_handler);

	INIT_WORK(&drvdata->battery_add_work,

		  corsair_void_battery_add_work_handler);

	ret = devm_mutex_init(drvdata->dev, &drvdata->battery_mutex);

	if (ret)

		return ret;

	INIT_WORK(&drvdata->battery_work, corsair_void_battery_work_handler);



	ret = sysfs_create_group(&hid_dev->dev.kobj, &corsair_void_attr_group);

	if (ret)
@@ -721,8 +725,7 @@ static void corsair_void_remove(struct hid_device *hid_dev) 
	struct corsair_void_drvdata *drvdata = hid_get_drvdata(hid_dev);



	hid_hw_stop(hid_dev);

	cancel_work_sync(&drvdata->battery_remove_work);

	cancel_work_sync(&drvdata->battery_add_work);

	cancel_work_sync(&drvdata->battery_work);

	if (drvdata->battery)

		power_supply_unregister(drvdata->battery);