Commit 754bd2b4 authored by Guenter Roeck's avatar Guenter Roeck
Browse files

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: ddbb4db4 ("hwmon: (pmbus) Add regulator support")
Reviewed-by: default avatarSanman Pradhan <psanman@juniper.net>
Signed-off-by: default avatarGuenter Roeck <linux@roeck-us.net>
parent cd658475

drivers/hwmon/pmbus/pmbus_core.c

+89 −25
Original line number Diff line number Diff line
@@ -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)