hwmon: (pmbus/core) Protect regulator operations with mutex

The regulator operations pmbus_regulator_get_voltage(),
pmbus_regulator_set_voltage(), and pmbus_regulator_list_voltage()
access PMBus registers and shared data but were not protected by
the update_lock mutex. This could lead to race conditions.

However, adding mutex protection directly to these functions causes
a deadlock because pmbus_regulator_notify() (which calls
regulator_notifier_call_chain()) is often called with the mutex
already held (e.g., from pmbus_fault_handler()). If a regulator
callback then calls one of the now-protected voltage functions,
it will attempt to acquire the same mutex.

Rework pmbus_regulator_notify() to utilize a worker function to
send notifications outside of the mutex protection. Events are
stored as atomics in a per-page bitmask and processed by the worker.

Initialize the worker and its associated data during regulator
registration, and ensure it is cancelled on device removal using
devm_add_action_or_reset().

While at it, remove the unnecessary include of linux/of.h.

Cc: Sanman Pradhan <psanman@juniper.net>
Fixes: ddbb4db4ced1b ("hwmon: (pmbus) Add regulator support")
Reviewed-by: Sanman Pradhan <psanman@juniper.net>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>

diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index cbc36f0ba4bf9004492fa5e40dc53172e23e8a9e..572be3ebc03df1bc555737eac8cf91f814f9ba53 100644 (file)
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -6,6 +6,7 @@
  * Copyright (c) 2012 Guenter Roeck
  */
 
+#include <linux/atomic.h>
 #include <linux/debugfs.h>
 #include <linux/delay.h>
 #include <linux/dcache.h>
@@ -21,8 +22,8 @@
 #include <linux/pmbus.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
-#include <linux/of.h>
 #include <linux/thermal.h>
+#include <linux/workqueue.h>
 #include "pmbus.h"
 
 /*
@@ -112,6 +113,11 @@ struct pmbus_data {
 
        struct mutex update_lock;
 
+#if IS_ENABLED(CONFIG_REGULATOR)
+       atomic_t regulator_events[PMBUS_PAGES];
+       struct work_struct regulator_notify_work;
+#endif
+
        bool has_status_word;           /* device uses STATUS_WORD register */
        int (*read_status)(struct i2c_client *client, int page);
 
@@ -3228,12 +3234,19 @@ static int pmbus_regulator_get_voltage(struct regulator_dev *rdev)
                .class = PSC_VOLTAGE_OUT,
                .convert = true,
        };
+       int ret;
 
+       mutex_lock(&data->update_lock);
        s.data = _pmbus_read_word_data(client, s.page, 0xff, PMBUS_READ_VOUT);
-       if (s.data < 0)
-               return s.data;
+       if (s.data < 0) {
+               ret = s.data;
+               goto unlock;
+       }
 
-       return (int)pmbus_reg2data(data, &s) * 1000; /* unit is uV */
+       ret = (int)pmbus_reg2data(data, &s) * 1000; /* unit is uV */
+unlock:
+       mutex_unlock(&data->update_lock);
+       return ret;
 }
 
 static int pmbus_regulator_set_voltage(struct regulator_dev *rdev, int min_uv,
@@ -3250,16 +3263,22 @@ static int pmbus_regulator_set_voltage(struct regulator_dev *rdev, int min_uv,
        };
        int val = DIV_ROUND_CLOSEST(min_uv, 1000); /* convert to mV */
        int low, high;
+       int ret;
 
        *selector = 0;
 
+       mutex_lock(&data->update_lock);
        low = pmbus_regulator_get_low_margin(client, s.page);
-       if (low < 0)
-               return low;
+       if (low < 0) {
+               ret = low;
+               goto unlock;
+       }
 
        high = pmbus_regulator_get_high_margin(client, s.page);
-       if (high < 0)
-               return high;
+       if (high < 0) {
+               ret = high;
+               goto unlock;
+       }
 
        /* Make sure we are within margins */
        if (low > val)
@@ -3269,7 +3288,10 @@ static int pmbus_regulator_set_voltage(struct regulator_dev *rdev, int min_uv,
 
        val = pmbus_data2reg(data, &s, val);
 
-       return _pmbus_write_word_data(client, s.page, PMBUS_VOUT_COMMAND, (u16)val);
+       ret = _pmbus_write_word_data(client, s.page, PMBUS_VOUT_COMMAND, (u16)val);
+unlock:
+       mutex_unlock(&data->update_lock);
+       return ret;
 }
 
 static int pmbus_regulator_list_voltage(struct regulator_dev *rdev,
@@ -3279,6 +3301,7 @@ static int pmbus_regulator_list_voltage(struct regulator_dev *rdev,
        struct i2c_client *client = to_i2c_client(dev->parent);
        struct pmbus_data *data = i2c_get_clientdata(client);
        int val, low, high;
+       int ret;
 
        if (data->flags & PMBUS_VOUT_PROTECTED)
                return 0;
@@ -3291,18 +3314,29 @@ static int pmbus_regulator_list_voltage(struct regulator_dev *rdev,
        val = DIV_ROUND_CLOSEST(rdev->desc->min_uV +
                                (rdev->desc->uV_step * selector), 1000); /* convert to mV */
 
+       mutex_lock(&data->update_lock);
+
        low = pmbus_regulator_get_low_margin(client, rdev_get_id(rdev));
-       if (low < 0)
-               return low;
+       if (low < 0) {
+               ret = low;
+               goto unlock;
+       }
 
        high = pmbus_regulator_get_high_margin(client, rdev_get_id(rdev));
-       if (high < 0)
-               return high;
+       if (high < 0) {
+               ret = high;
+               goto unlock;
+       }
 
-       if (val >= low && val <= high)
-               return val * 1000; /* unit is uV */
+       if (val >= low && val <= high) {
+               ret = val * 1000; /* unit is uV */
+               goto unlock;
+       }
 
-       return 0;
+       ret = 0;
+unlock:
+       mutex_unlock(&data->update_lock);
+       return ret;
 }
 
 const struct regulator_ops pmbus_regulator_ops = {
@@ -3333,12 +3367,42 @@ int pmbus_regulator_init_cb(struct regulator_dev *rdev,
 }
 EXPORT_SYMBOL_NS_GPL(pmbus_regulator_init_cb, "PMBUS");
 
+static void pmbus_regulator_notify_work_cancel(void *data)
+{
+       struct pmbus_data *pdata = data;
+
+       cancel_work_sync(&pdata->regulator_notify_work);
+}
+
+static void pmbus_regulator_notify_worker(struct work_struct *work)
+{
+       struct pmbus_data *data =
+               container_of(work, struct pmbus_data, regulator_notify_work);
+       int i, j;
+
+       for (i = 0; i < data->info->pages; i++) {
+               int event;
+
+               event = atomic_xchg(&data->regulator_events[i], 0);
+               if (!event)
+                       continue;
+
+               for (j = 0; j < data->info->num_regulators; j++) {
+                       if (i == rdev_get_id(data->rdevs[j])) {
+                               regulator_notifier_call_chain(data->rdevs[j],
+                                                             event, NULL);
+                               break;
+                       }
+               }
+       }
+}
+
 static int pmbus_regulator_register(struct pmbus_data *data)
 {
        struct device *dev = data->dev;
        const struct pmbus_driver_info *info = data->info;
        const struct pmbus_platform_data *pdata = dev_get_platdata(dev);
-       int i;
+       int i, ret;
 
        data->rdevs = devm_kzalloc(dev, sizeof(struct regulator_dev *) * info->num_regulators,
                                   GFP_KERNEL);
@@ -3362,19 +3426,19 @@ static int pmbus_regulator_register(struct pmbus_data *data)
                                             info->reg_desc[i].name);
        }
 
+       INIT_WORK(&data->regulator_notify_work, pmbus_regulator_notify_worker);
+
+       ret = devm_add_action_or_reset(dev, pmbus_regulator_notify_work_cancel, data);
+       if (ret)
+               return ret;
+
        return 0;
 }
 
 static void pmbus_regulator_notify(struct pmbus_data *data, int page, int event)
 {
-       int j;
-
-       for (j = 0; j < data->info->num_regulators; j++) {
-               if (page == rdev_get_id(data->rdevs[j])) {
-                       regulator_notifier_call_chain(data->rdevs[j], event, NULL);
-                       break;
-               }
-       }
+       atomic_or(event, &data->regulator_events[page]);
+       schedule_work(&data->regulator_notify_work);
 }
 #else
 static int pmbus_regulator_register(struct pmbus_data *data)