From: Dmitry Torokhov Date: Fri, 17 Oct 2008 15:51:12 +0000 (+0200) Subject: hwmon: (ams) Fix locking issues X-Git-Tag: v2.6.28-rc1~259^2~32 X-Git-Url: http://pilppa.com/gitweb/?a=commitdiff_plain;h=ee4cd32ee8a68e22081f698602e4315fb4272853;p=linux-2.6-omap-h63xx.git hwmon: (ams) Fix locking issues Use a separate mutex to serialize input device creation/removal, otheriwse we deadlock if we try to remove input device while it is being polled. Also do not take ams_info.lock when it is not needed. Signed-off-by: Dmitry Torokhov Signed-off-by: Johannes Berg Signed-off-by: Jean Delvare --- diff --git a/drivers/hwmon/ams/ams-core.c b/drivers/hwmon/ams/ams-core.c index fbefa82a015..9b4a0e7a564 100644 --- a/drivers/hwmon/ams/ams-core.c +++ b/drivers/hwmon/ams/ams-core.c @@ -223,34 +223,28 @@ int __init ams_init(void) void ams_exit(void) { - mutex_lock(&ams_info.lock); - - if (ams_info.has_device) { - /* Remove input device */ - ams_input_exit(); + /* Remove input device */ + ams_input_exit(); - /* Shut down implementation */ - ams_info.exit(); - - /* Flush interrupt worker - * - * We do this after ams_info.exit(), because an interrupt might - * have arrived before disabling them. - */ - flush_scheduled_work(); + /* Remove attributes */ + device_remove_file(&ams_info.of_dev->dev, &dev_attr_current); - /* Remove attributes */ - device_remove_file(&ams_info.of_dev->dev, &dev_attr_current); + /* Shut down implementation */ + ams_info.exit(); - /* Remove device */ - of_device_unregister(ams_info.of_dev); + /* Flush interrupt worker + * + * We do this after ams_info.exit(), because an interrupt might + * have arrived before disabling them. + */ + flush_scheduled_work(); - /* Remove handler */ - pmf_unregister_irq_client(&ams_shock_client); - pmf_unregister_irq_client(&ams_freefall_client); - } + /* Remove device */ + of_device_unregister(ams_info.of_dev); - mutex_unlock(&ams_info.lock); + /* Remove handler */ + pmf_unregister_irq_client(&ams_shock_client); + pmf_unregister_irq_client(&ams_freefall_client); } MODULE_AUTHOR("Stelian Pop, Michael Hanselmann"); diff --git a/drivers/hwmon/ams/ams-i2c.c b/drivers/hwmon/ams/ams-i2c.c index da26de01068..2cbf8a6506c 100644 --- a/drivers/hwmon/ams/ams-i2c.c +++ b/drivers/hwmon/ams/ams-i2c.c @@ -261,8 +261,6 @@ int __init ams_i2c_init(struct device_node *np) { int result; - mutex_lock(&ams_info.lock); - /* Set implementation stuff */ ams_info.of_node = np; ams_info.exit = ams_i2c_exit; @@ -273,7 +271,5 @@ int __init ams_i2c_init(struct device_node *np) result = i2c_add_driver(&ams_i2c_driver); - mutex_unlock(&ams_info.lock); - return result; } diff --git a/drivers/hwmon/ams/ams-input.c b/drivers/hwmon/ams/ams-input.c index 48dbf7d6a66..8a712392cd3 100644 --- a/drivers/hwmon/ams/ams-input.c +++ b/drivers/hwmon/ams/ams-input.c @@ -27,6 +27,8 @@ static unsigned int invert; module_param(invert, bool, S_IWUSR | S_IRUGO); MODULE_PARM_DESC(invert, "Invert input data on X and Y axis"); +static DEFINE_MUTEX(ams_input_mutex); + static void ams_idev_poll(struct input_polled_dev *dev) { struct input_dev *idev = dev->input; @@ -50,13 +52,11 @@ static void ams_idev_poll(struct input_polled_dev *dev) } /* Call with ams_info.lock held! */ -static void ams_input_enable(void) +static int ams_input_enable(void) { struct input_dev *input; s8 x, y, z; - - if (ams_info.idev) - return; + int error; ams_sensors(&x, &y, &z); ams_info.xcalib = x; @@ -65,7 +65,7 @@ static void ams_input_enable(void) ams_info.idev = input_allocate_polled_device(); if (!ams_info.idev) - return; + return -ENOMEM; ams_info.idev->poll = ams_idev_poll; ams_info.idev->poll_interval = 25; @@ -84,14 +84,18 @@ static void ams_input_enable(void) set_bit(EV_KEY, input->evbit); set_bit(BTN_TOUCH, input->keybit); - if (input_register_polled_device(ams_info.idev)) { + error = input_register_polled_device(ams_info.idev); + if (error) { input_free_polled_device(ams_info.idev); ams_info.idev = NULL; - return; + return error; } + + joystick = 1; + + return 0; } -/* Call with ams_info.lock held! */ static void ams_input_disable(void) { if (ams_info.idev) { @@ -99,6 +103,8 @@ static void ams_input_disable(void) input_free_polled_device(ams_info.idev); ams_info.idev = NULL; } + + joystick = 0; } static ssize_t ams_input_show_joystick(struct device *dev, @@ -110,39 +116,42 @@ static ssize_t ams_input_show_joystick(struct device *dev, static ssize_t ams_input_store_joystick(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { - if (sscanf(buf, "%d\n", &joystick) != 1) + unsigned long enable; + int error = 0; + + if (strict_strtoul(buf, 0, &enable) || enable > 1) return -EINVAL; - mutex_lock(&ams_info.lock); + mutex_lock(&ams_input_mutex); - if (joystick) - ams_input_enable(); - else - ams_input_disable(); + if (enable != joystick) { + if (enable) + error = ams_input_enable(); + else + ams_input_disable(); + } - mutex_unlock(&ams_info.lock); + mutex_unlock(&ams_input_mutex); - return count; + return error ? error : count; } static DEVICE_ATTR(joystick, S_IRUGO | S_IWUSR, ams_input_show_joystick, ams_input_store_joystick); -/* Call with ams_info.lock held! */ int ams_input_init(void) { - int result; - - result = device_create_file(&ams_info.of_dev->dev, &dev_attr_joystick); - - if (!result && joystick) + if (joystick) ams_input_enable(); - return result; + + return device_create_file(&ams_info.of_dev->dev, &dev_attr_joystick); } -/* Call with ams_info.lock held! */ void ams_input_exit(void) { - ams_input_disable(); device_remove_file(&ams_info.of_dev->dev, &dev_attr_joystick); + + mutex_lock(&ams_input_mutex); + ams_input_disable(); + mutex_unlock(&ams_input_mutex); } diff --git a/drivers/hwmon/ams/ams-pmu.c b/drivers/hwmon/ams/ams-pmu.c index 9463e9768f6..fb18b3d3162 100644 --- a/drivers/hwmon/ams/ams-pmu.c +++ b/drivers/hwmon/ams/ams-pmu.c @@ -149,8 +149,6 @@ int __init ams_pmu_init(struct device_node *np) const u32 *prop; int result; - mutex_lock(&ams_info.lock); - /* Set implementation stuff */ ams_info.of_node = np; ams_info.exit = ams_pmu_exit; @@ -161,10 +159,9 @@ int __init ams_pmu_init(struct device_node *np) /* Get PMU command, should be 0x4e, but we can never know */ prop = of_get_property(ams_info.of_node, "reg", NULL); - if (!prop) { - result = -ENODEV; - goto exit; - } + if (!prop) + return -ENODEV; + ams_pmu_cmd = ((*prop) >> 8) & 0xff; /* Disable interrupts */ @@ -175,7 +172,7 @@ int __init ams_pmu_init(struct device_node *np) result = ams_sensor_attach(); if (result < 0) - goto exit; + return result; /* Set default values */ ams_pmu_set_register(AMS_FF_LOW_LIMIT, 0x15); @@ -198,10 +195,5 @@ int __init ams_pmu_init(struct device_node *np) printk(KERN_INFO "ams: Found PMU based motion sensor\n"); - result = 0; - -exit: - mutex_unlock(&ams_info.lock); - - return result; + return 0; }