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 <dtor@mail.ru>
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Jean Delvare <khali@linux-fr.org>

diff --git a/drivers/hwmon/ams/ams-core.c b/drivers/hwmon/ams/ams-core.c
index fbefa82a015cc6175f40106abc0d7017bee1ecea..9b4a0e7a564fb030d0d3945626bab2b7320d63bc 100644 (file)
--- 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 da26de01068e6b84ff016dea30e7c50280808672..2cbf8a6506c7e8fad72736773efa238367bf03f5 100644 (file)
--- 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 48dbf7d6a66d9e6a14577667868901087abaff07..8a712392cd3829a32dd471f50a557aaa8e774cc3 100644 (file)
--- 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 9463e9768f6f55408ce35215a79274346e41a77d..fb18b3d3162bcebfe908fa5c678ccf6893fdb25d 100644 (file)
--- 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;
 }