* [PATCH 1/5] hwmon: (pmbus) Mark lowest/average/highest/rated attributes as read-only
2026-03-25 18:16 [PATCH 0/5] hwmon: (pmbus) PMBus fixes and improvements Guenter Roeck
@ 2026-03-25 18:16 ` Guenter Roeck
2026-03-25 18:16 ` [PATCH 2/5] hwmon: (pmbus) Introduce the concept of "write-only" attributes Guenter Roeck
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2026-03-25 18:16 UTC (permalink / raw)
To: Hardware Monitoring; +Cc: Sanman Pradhan, Guenter Roeck
Writing those attributes is not supported, so mark them as read-only.
Prior to this change, attempts to write into these attributes returned
an error.
Mark boolean fields in struct pmbus_limit_attr and in struct
pmbus_sensor_attr as bit fields to reduce configuration data size.
The data is scanned only while probing, so performance is not a concern.
Fixes: 6f183d33a02e6 ("hwmon: (pmbus) Add support for peak attributes")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/hwmon/pmbus/pmbus_core.c | 48 ++++++++++++++++++++++++++++----
1 file changed, 42 insertions(+), 6 deletions(-)
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index be6d05def115..ecd1dddcbe0f 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -1495,8 +1495,9 @@ static int pmbus_add_label(struct pmbus_data *data,
struct pmbus_limit_attr {
u16 reg; /* Limit register */
u16 sbit; /* Alarm attribute status bit */
- bool update; /* True if register needs updates */
- bool low; /* True if low limit; for limits with compare functions only */
+ bool readonly:1; /* True if the attribute is read-only */
+ bool update:1; /* True if register needs updates */
+ bool low:1; /* True if low limit; for limits with compare functions only */
const char *attr; /* Attribute name */
const char *alarm; /* Alarm attribute name */
};
@@ -1511,9 +1512,9 @@ struct pmbus_sensor_attr {
u8 nlimit; /* # of limit registers */
enum pmbus_sensor_classes class;/* sensor class */
const char *label; /* sensor label */
- bool paged; /* true if paged sensor */
- bool update; /* true if update needed */
- bool compare; /* true if compare function needed */
+ bool paged:1; /* true if paged sensor */
+ bool update:1; /* true if update needed */
+ bool compare:1; /* true if compare function needed */
u32 func; /* sensor mask */
u32 sfunc; /* sensor status mask */
int sreg; /* status register */
@@ -1544,7 +1545,7 @@ static int pmbus_add_limit_attrs(struct i2c_client *client,
curr = pmbus_add_sensor(data, name, l->attr, index,
page, 0xff, l->reg, attr->class,
attr->update || l->update,
- false, true);
+ l->readonly, true);
if (!curr)
return -ENOMEM;
if (l->sbit && (info->func[page] & attr->sfunc)) {
@@ -1707,23 +1708,28 @@ static const struct pmbus_limit_attr vin_limit_attrs[] = {
}, {
.reg = PMBUS_VIRT_READ_VIN_AVG,
.update = true,
+ .readonly = true,
.attr = "average",
}, {
.reg = PMBUS_VIRT_READ_VIN_MIN,
.update = true,
+ .readonly = true,
.attr = "lowest",
}, {
.reg = PMBUS_VIRT_READ_VIN_MAX,
.update = true,
+ .readonly = true,
.attr = "highest",
}, {
.reg = PMBUS_VIRT_RESET_VIN_HISTORY,
.attr = "reset_history",
}, {
.reg = PMBUS_MFR_VIN_MIN,
+ .readonly = true,
.attr = "rated_min",
}, {
.reg = PMBUS_MFR_VIN_MAX,
+ .readonly = true,
.attr = "rated_max",
},
};
@@ -1776,23 +1782,28 @@ static const struct pmbus_limit_attr vout_limit_attrs[] = {
}, {
.reg = PMBUS_VIRT_READ_VOUT_AVG,
.update = true,
+ .readonly = true,
.attr = "average",
}, {
.reg = PMBUS_VIRT_READ_VOUT_MIN,
.update = true,
+ .readonly = true,
.attr = "lowest",
}, {
.reg = PMBUS_VIRT_READ_VOUT_MAX,
.update = true,
+ .readonly = true,
.attr = "highest",
}, {
.reg = PMBUS_VIRT_RESET_VOUT_HISTORY,
.attr = "reset_history",
}, {
.reg = PMBUS_MFR_VOUT_MIN,
+ .readonly = true,
.attr = "rated_min",
}, {
.reg = PMBUS_MFR_VOUT_MAX,
+ .readonly = true,
.attr = "rated_max",
},
};
@@ -1852,20 +1863,24 @@ static const struct pmbus_limit_attr iin_limit_attrs[] = {
}, {
.reg = PMBUS_VIRT_READ_IIN_AVG,
.update = true,
+ .readonly = true,
.attr = "average",
}, {
.reg = PMBUS_VIRT_READ_IIN_MIN,
.update = true,
+ .readonly = true,
.attr = "lowest",
}, {
.reg = PMBUS_VIRT_READ_IIN_MAX,
.update = true,
+ .readonly = true,
.attr = "highest",
}, {
.reg = PMBUS_VIRT_RESET_IIN_HISTORY,
.attr = "reset_history",
}, {
.reg = PMBUS_MFR_IIN_MAX,
+ .readonly = true,
.attr = "rated_max",
},
};
@@ -1889,20 +1904,24 @@ static const struct pmbus_limit_attr iout_limit_attrs[] = {
}, {
.reg = PMBUS_VIRT_READ_IOUT_AVG,
.update = true,
+ .readonly = true,
.attr = "average",
}, {
.reg = PMBUS_VIRT_READ_IOUT_MIN,
.update = true,
+ .readonly = true,
.attr = "lowest",
}, {
.reg = PMBUS_VIRT_READ_IOUT_MAX,
.update = true,
+ .readonly = true,
.attr = "highest",
}, {
.reg = PMBUS_VIRT_RESET_IOUT_HISTORY,
.attr = "reset_history",
}, {
.reg = PMBUS_MFR_IOUT_MAX,
+ .readonly = true,
.attr = "rated_max",
},
};
@@ -1943,20 +1962,24 @@ static const struct pmbus_limit_attr pin_limit_attrs[] = {
}, {
.reg = PMBUS_VIRT_READ_PIN_AVG,
.update = true,
+ .readonly = true,
.attr = "average",
}, {
.reg = PMBUS_VIRT_READ_PIN_MIN,
.update = true,
+ .readonly = true,
.attr = "input_lowest",
}, {
.reg = PMBUS_VIRT_READ_PIN_MAX,
.update = true,
+ .readonly = true,
.attr = "input_highest",
}, {
.reg = PMBUS_VIRT_RESET_PIN_HISTORY,
.attr = "reset_history",
}, {
.reg = PMBUS_MFR_PIN_MAX,
+ .readonly = true,
.attr = "rated_max",
},
};
@@ -1980,20 +2003,24 @@ static const struct pmbus_limit_attr pout_limit_attrs[] = {
}, {
.reg = PMBUS_VIRT_READ_POUT_AVG,
.update = true,
+ .readonly = true,
.attr = "average",
}, {
.reg = PMBUS_VIRT_READ_POUT_MIN,
.update = true,
+ .readonly = true,
.attr = "input_lowest",
}, {
.reg = PMBUS_VIRT_READ_POUT_MAX,
.update = true,
+ .readonly = true,
.attr = "input_highest",
}, {
.reg = PMBUS_VIRT_RESET_POUT_HISTORY,
.attr = "reset_history",
}, {
.reg = PMBUS_MFR_POUT_MAX,
+ .readonly = true,
.attr = "rated_max",
},
};
@@ -2049,18 +2076,22 @@ static const struct pmbus_limit_attr temp_limit_attrs[] = {
.sbit = PB_TEMP_OT_FAULT,
}, {
.reg = PMBUS_VIRT_READ_TEMP_MIN,
+ .readonly = true,
.attr = "lowest",
}, {
.reg = PMBUS_VIRT_READ_TEMP_AVG,
+ .readonly = true,
.attr = "average",
}, {
.reg = PMBUS_VIRT_READ_TEMP_MAX,
+ .readonly = true,
.attr = "highest",
}, {
.reg = PMBUS_VIRT_RESET_TEMP_HISTORY,
.attr = "reset_history",
}, {
.reg = PMBUS_MFR_MAX_TEMP_1,
+ .readonly = true,
.attr = "rated_max",
},
};
@@ -2090,18 +2121,22 @@ static const struct pmbus_limit_attr temp_limit_attrs2[] = {
.sbit = PB_TEMP_OT_FAULT,
}, {
.reg = PMBUS_VIRT_READ_TEMP2_MIN,
+ .readonly = true,
.attr = "lowest",
}, {
.reg = PMBUS_VIRT_READ_TEMP2_AVG,
+ .readonly = true,
.attr = "average",
}, {
.reg = PMBUS_VIRT_READ_TEMP2_MAX,
+ .readonly = true,
.attr = "highest",
}, {
.reg = PMBUS_VIRT_RESET_TEMP2_HISTORY,
.attr = "reset_history",
}, {
.reg = PMBUS_MFR_MAX_TEMP_2,
+ .readonly = true,
.attr = "rated_max",
},
};
@@ -2131,6 +2166,7 @@ static const struct pmbus_limit_attr temp_limit_attrs3[] = {
.sbit = PB_TEMP_OT_FAULT,
}, {
.reg = PMBUS_MFR_MAX_TEMP_3,
+ .readonly = true,
.attr = "rated_max",
},
};
--
2.45.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/5] hwmon: (pmbus) Introduce the concept of "write-only" attributes
2026-03-25 18:16 [PATCH 0/5] hwmon: (pmbus) PMBus fixes and improvements Guenter Roeck
2026-03-25 18:16 ` [PATCH 1/5] hwmon: (pmbus) Mark lowest/average/highest/rated attributes as read-only Guenter Roeck
@ 2026-03-25 18:16 ` Guenter Roeck
2026-03-25 18:16 ` [PATCH 3/5] hwmon: (pmbus/core) Protect regulator operations with mutex Guenter Roeck
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2026-03-25 18:16 UTC (permalink / raw)
To: Hardware Monitoring; +Cc: Sanman Pradhan, Guenter Roeck
Attributes intended to clear sensor history are intended to be writeable
only. Reading those attributes today results in reporting more or less
random values. To avoid ABI surprises, have those attributes explicitly
return 0 when reading.
Fixes: 787c095edaa9d ("hwmon: (pmbus/core) Add support for rated attributes")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/hwmon/pmbus/pmbus_core.c | 32 ++++++++++++++++++++++++--------
1 file changed, 24 insertions(+), 8 deletions(-)
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index ecd1dddcbe0f..cbc36f0ba4bf 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -1209,6 +1209,12 @@ static ssize_t pmbus_show_boolean(struct device *dev,
return sysfs_emit(buf, "%d\n", val);
}
+static ssize_t pmbus_show_zero(struct device *dev,
+ struct device_attribute *devattr, char *buf)
+{
+ return sysfs_emit(buf, "0\n");
+}
+
static ssize_t pmbus_show_sensor(struct device *dev,
struct device_attribute *devattr, char *buf)
{
@@ -1407,7 +1413,7 @@ static struct pmbus_sensor *pmbus_add_sensor(struct pmbus_data *data,
int reg,
enum pmbus_sensor_classes class,
bool update, bool readonly,
- bool convert)
+ bool writeonly, bool convert)
{
struct pmbus_sensor *sensor;
struct device_attribute *a;
@@ -1436,7 +1442,8 @@ static struct pmbus_sensor *pmbus_add_sensor(struct pmbus_data *data,
sensor->data = -ENODATA;
pmbus_dev_attr_init(a, sensor->name,
readonly ? 0444 : 0644,
- pmbus_show_sensor, pmbus_set_sensor);
+ writeonly ? pmbus_show_zero : pmbus_show_sensor,
+ pmbus_set_sensor);
if (pmbus_add_attribute(data, &a->attr))
return NULL;
@@ -1496,6 +1503,7 @@ struct pmbus_limit_attr {
u16 reg; /* Limit register */
u16 sbit; /* Alarm attribute status bit */
bool readonly:1; /* True if the attribute is read-only */
+ bool writeonly:1; /* True if the attribute is write-only */
bool update:1; /* True if register needs updates */
bool low:1; /* True if low limit; for limits with compare functions only */
const char *attr; /* Attribute name */
@@ -1545,7 +1553,7 @@ static int pmbus_add_limit_attrs(struct i2c_client *client,
curr = pmbus_add_sensor(data, name, l->attr, index,
page, 0xff, l->reg, attr->class,
attr->update || l->update,
- l->readonly, true);
+ l->readonly, l->writeonly, true);
if (!curr)
return -ENOMEM;
if (l->sbit && (info->func[page] & attr->sfunc)) {
@@ -1585,7 +1593,7 @@ static int pmbus_add_sensor_attrs_one(struct i2c_client *client,
return ret;
}
base = pmbus_add_sensor(data, name, "input", index, page, phase,
- attr->reg, attr->class, true, true, true);
+ attr->reg, attr->class, true, true, false, true);
if (!base)
return -ENOMEM;
/* No limit and alarm attributes for phase specific sensors */
@@ -1722,6 +1730,7 @@ static const struct pmbus_limit_attr vin_limit_attrs[] = {
.attr = "highest",
}, {
.reg = PMBUS_VIRT_RESET_VIN_HISTORY,
+ .writeonly = true,
.attr = "reset_history",
}, {
.reg = PMBUS_MFR_VIN_MIN,
@@ -1796,6 +1805,7 @@ static const struct pmbus_limit_attr vout_limit_attrs[] = {
.attr = "highest",
}, {
.reg = PMBUS_VIRT_RESET_VOUT_HISTORY,
+ .writeonly = true,
.attr = "reset_history",
}, {
.reg = PMBUS_MFR_VOUT_MIN,
@@ -1877,6 +1887,7 @@ static const struct pmbus_limit_attr iin_limit_attrs[] = {
.attr = "highest",
}, {
.reg = PMBUS_VIRT_RESET_IIN_HISTORY,
+ .writeonly = true,
.attr = "reset_history",
}, {
.reg = PMBUS_MFR_IIN_MAX,
@@ -1918,6 +1929,7 @@ static const struct pmbus_limit_attr iout_limit_attrs[] = {
.attr = "highest",
}, {
.reg = PMBUS_VIRT_RESET_IOUT_HISTORY,
+ .writeonly = true,
.attr = "reset_history",
}, {
.reg = PMBUS_MFR_IOUT_MAX,
@@ -1976,6 +1988,7 @@ static const struct pmbus_limit_attr pin_limit_attrs[] = {
.attr = "input_highest",
}, {
.reg = PMBUS_VIRT_RESET_PIN_HISTORY,
+ .writeonly = true,
.attr = "reset_history",
}, {
.reg = PMBUS_MFR_PIN_MAX,
@@ -2017,6 +2030,7 @@ static const struct pmbus_limit_attr pout_limit_attrs[] = {
.attr = "input_highest",
}, {
.reg = PMBUS_VIRT_RESET_POUT_HISTORY,
+ .writeonly = true,
.attr = "reset_history",
}, {
.reg = PMBUS_MFR_POUT_MAX,
@@ -2088,6 +2102,7 @@ static const struct pmbus_limit_attr temp_limit_attrs[] = {
.attr = "highest",
}, {
.reg = PMBUS_VIRT_RESET_TEMP_HISTORY,
+ .writeonly = true,
.attr = "reset_history",
}, {
.reg = PMBUS_MFR_MAX_TEMP_1,
@@ -2133,6 +2148,7 @@ static const struct pmbus_limit_attr temp_limit_attrs2[] = {
.attr = "highest",
}, {
.reg = PMBUS_VIRT_RESET_TEMP2_HISTORY,
+ .writeonly = true,
.attr = "reset_history",
}, {
.reg = PMBUS_MFR_MAX_TEMP_2,
@@ -2250,7 +2266,7 @@ static int pmbus_add_fan_ctrl(struct i2c_client *client,
sensor = pmbus_add_sensor(data, "fan", "target", index, page,
0xff, PMBUS_VIRT_FAN_TARGET_1 + id, PSC_FAN,
- false, false, true);
+ false, false, false, true);
if (!sensor)
return -ENOMEM;
@@ -2261,14 +2277,14 @@ static int pmbus_add_fan_ctrl(struct i2c_client *client,
sensor = pmbus_add_sensor(data, "pwm", NULL, index, page,
0xff, PMBUS_VIRT_PWM_1 + id, PSC_PWM,
- false, false, true);
+ false, false, false, true);
if (!sensor)
return -ENOMEM;
sensor = pmbus_add_sensor(data, "pwm", "enable", index, page,
0xff, PMBUS_VIRT_PWM_ENABLE_1 + id, PSC_PWM,
- true, false, false);
+ true, false, false, false);
if (!sensor)
return -ENOMEM;
@@ -2310,7 +2326,7 @@ static int pmbus_add_fan_attributes(struct i2c_client *client,
if (pmbus_add_sensor(data, "fan", "input", index,
page, 0xff, pmbus_fan_registers[f],
- PSC_FAN, true, true, true) == NULL)
+ PSC_FAN, true, true, false, true) == NULL)
return -ENOMEM;
/* Fan control */
--
2.45.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/5] hwmon: (pmbus/core) Protect regulator operations with mutex
2026-03-25 18:16 [PATCH 0/5] hwmon: (pmbus) PMBus fixes and improvements Guenter Roeck
2026-03-25 18:16 ` [PATCH 1/5] hwmon: (pmbus) Mark lowest/average/highest/rated attributes as read-only Guenter Roeck
2026-03-25 18:16 ` [PATCH 2/5] hwmon: (pmbus) Introduce the concept of "write-only" attributes Guenter Roeck
@ 2026-03-25 18:16 ` Guenter Roeck
2026-03-25 19:05 ` Pradhan, Sanman
2026-03-25 18:16 ` [PATCH 4/5] hwmon: (pmbus) Add support for guarded PMBus lock Guenter Roeck
` (2 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2026-03-25 18:16 UTC (permalink / raw)
To: Hardware Monitoring; +Cc: Sanman Pradhan, Guenter Roeck
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")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/hwmon/pmbus/pmbus_core.c | 114 ++++++++++++++++++++++++-------
1 file changed, 89 insertions(+), 25 deletions(-)
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index cbc36f0ba4bf..572be3ebc03d 100644
--- 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)
--
2.45.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 3/5] hwmon: (pmbus/core) Protect regulator operations with mutex
2026-03-25 18:16 ` [PATCH 3/5] hwmon: (pmbus/core) Protect regulator operations with mutex Guenter Roeck
@ 2026-03-25 19:05 ` Pradhan, Sanman
2026-03-25 20:02 ` Guenter Roeck
0 siblings, 1 reply; 10+ messages in thread
From: Pradhan, Sanman @ 2026-03-25 19:05 UTC (permalink / raw)
To: linux@roeck-us.net
Cc: linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org,
Sanman Pradhan
From: Sanman Pradhan <psanman@juniper.net>
Reviewed the series.
Very minor hardening thought: the notifier worker is
initialized after the devm_regulator_register() loop.
The current probe ordering makes that fine, so I don't
think this is a bug, but would it be cleaner to initialize
the notifier state a bit earlier just to keep that
setup self-contained?
Don't see any blocking issues in the rest of the series.
Thank you.
Regards,
Sanman Pradhan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/5] hwmon: (pmbus/core) Protect regulator operations with mutex
2026-03-25 19:05 ` Pradhan, Sanman
@ 2026-03-25 20:02 ` Guenter Roeck
0 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2026-03-25 20:02 UTC (permalink / raw)
To: Pradhan, Sanman
Cc: linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org,
Sanman Pradhan
Hi,
On 3/25/26 12:05, Pradhan, Sanman wrote:
> From: Sanman Pradhan <psanman@juniper.net>
>
> Reviewed the series.
>
> Very minor hardening thought: the notifier worker is
> initialized after the devm_regulator_register() loop.
>
> The current probe ordering makes that fine, so I don't
> think this is a bug, but would it be cleaner to initialize
> the notifier state a bit earlier just to keep that
> setup self-contained?
>
The interrupt handler is installed after registering the regulator,
so I don't think that is a problem since the regulator notifier
is called from the interrupt handler.
> Don't see any blocking issues in the rest of the series.
>
Thanks a lot for the review and feedback. Surprisingly, Sashiko did not
find anything either:
https://sashiko.dev/#/patchset/20260325181631.17259-1-linux%40roeck-us.net
If you don't mind, please send me a formal Reviewed-by: tag for the series
or for individual patches.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/5] hwmon: (pmbus) Add support for guarded PMBus lock
2026-03-25 18:16 [PATCH 0/5] hwmon: (pmbus) PMBus fixes and improvements Guenter Roeck
` (2 preceding siblings ...)
2026-03-25 18:16 ` [PATCH 3/5] hwmon: (pmbus/core) Protect regulator operations with mutex Guenter Roeck
@ 2026-03-25 18:16 ` Guenter Roeck
2026-03-25 18:16 ` [PATCH 5/5] hwmon: (pmbus_core) Use guard() for mutex protection Guenter Roeck
2026-03-25 20:25 ` [PATCH 0/5] hwmon: (pmbus) PMBus fixes and improvements Pradhan, Sanman
5 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2026-03-25 18:16 UTC (permalink / raw)
To: Hardware Monitoring; +Cc: Sanman Pradhan, Guenter Roeck
Add support for guard(pmbus_lock)() and scoped_guard(pmbus_lock)()
to be able to simplify the PMBus code.
Also introduce pmbus_lock() as pre-requisite for supporting
guard().
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/hwmon/pmbus/pmbus.h | 5 +++++
drivers/hwmon/pmbus/pmbus_core.c | 8 ++++++++
2 files changed, 13 insertions(+)
diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index d2e9bfb5320f..e499cdae9442 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -10,6 +10,7 @@
#define PMBUS_H
#include <linux/bitops.h>
+#include <linux/cleanup.h>
#include <linux/regulator/driver.h>
/*
@@ -563,7 +564,11 @@ int pmbus_get_fan_rate_device(struct i2c_client *client, int page, int id,
int pmbus_get_fan_rate_cached(struct i2c_client *client, int page, int id,
enum pmbus_fan_mode mode);
int pmbus_lock_interruptible(struct i2c_client *client);
+void pmbus_lock(struct i2c_client *client);
void pmbus_unlock(struct i2c_client *client);
+
+DEFINE_GUARD(pmbus_lock, struct i2c_client *, pmbus_lock(_T), pmbus_unlock(_T))
+
int pmbus_update_fan(struct i2c_client *client, int page, int id,
u8 config, u8 mask, u16 command);
struct dentry *pmbus_get_debugfs_dir(struct i2c_client *client);
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 572be3ebc03d..7150f12d2630 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -3871,6 +3871,14 @@ struct dentry *pmbus_get_debugfs_dir(struct i2c_client *client)
}
EXPORT_SYMBOL_NS_GPL(pmbus_get_debugfs_dir, "PMBUS");
+void pmbus_lock(struct i2c_client *client)
+{
+ struct pmbus_data *data = i2c_get_clientdata(client);
+
+ mutex_lock(&data->update_lock);
+}
+EXPORT_SYMBOL_NS_GPL(pmbus_lock, "PMBUS");
+
int pmbus_lock_interruptible(struct i2c_client *client)
{
struct pmbus_data *data = i2c_get_clientdata(client);
--
2.45.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 5/5] hwmon: (pmbus_core) Use guard() for mutex protection
2026-03-25 18:16 [PATCH 0/5] hwmon: (pmbus) PMBus fixes and improvements Guenter Roeck
` (3 preceding siblings ...)
2026-03-25 18:16 ` [PATCH 4/5] hwmon: (pmbus) Add support for guarded PMBus lock Guenter Roeck
@ 2026-03-25 18:16 ` Guenter Roeck
2026-03-25 20:25 ` [PATCH 0/5] hwmon: (pmbus) PMBus fixes and improvements Pradhan, Sanman
5 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2026-03-25 18:16 UTC (permalink / raw)
To: Hardware Monitoring; +Cc: Sanman Pradhan, Guenter Roeck
Simplify the code by using guard() and scoped_guard() instead of
mutex_lock()/mutex_unlock() sequences.
This patch changes semantics for debugfs accesses. Previously, those
used mutex_lock_interruptible() and not mutex_lock(). This change is
intentional and should have little if any impact since locks should not
be held for a significant amount of time and debugfs accesses are less
critical than sysfs accesses (which never used interruptable locks).
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/hwmon/pmbus/pmbus_core.c | 279 ++++++++++++-------------------
1 file changed, 108 insertions(+), 171 deletions(-)
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 7150f12d2630..1f5bde83d55b 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -1156,12 +1156,11 @@ static int pmbus_get_boolean(struct i2c_client *client, struct pmbus_boolean *b,
int ret, status;
u16 regval;
- mutex_lock(&data->update_lock);
+ guard(pmbus_lock)(client);
+
status = pmbus_get_status(client, page, reg);
- if (status < 0) {
- ret = status;
- goto unlock;
- }
+ if (status < 0)
+ return status;
if (s1)
pmbus_update_sensor_data(client, s1);
@@ -1173,7 +1172,7 @@ static int pmbus_get_boolean(struct i2c_client *client, struct pmbus_boolean *b,
if (data->revision >= PMBUS_REV_12) {
ret = _pmbus_write_byte_data(client, page, reg, regval);
if (ret)
- goto unlock;
+ return ret;
} else {
pmbus_clear_fault_page(client, page);
}
@@ -1181,14 +1180,10 @@ static int pmbus_get_boolean(struct i2c_client *client, struct pmbus_boolean *b,
if (s1 && s2) {
s64 v1, v2;
- if (s1->data < 0) {
- ret = s1->data;
- goto unlock;
- }
- if (s2->data < 0) {
- ret = s2->data;
- goto unlock;
- }
+ if (s1->data < 0)
+ return s1->data;
+ if (s2->data < 0)
+ return s2->data;
v1 = pmbus_reg2data(data, s1);
v2 = pmbus_reg2data(data, s2);
@@ -1196,8 +1191,6 @@ static int pmbus_get_boolean(struct i2c_client *client, struct pmbus_boolean *b,
} else {
ret = !!regval;
}
-unlock:
- mutex_unlock(&data->update_lock);
return ret;
}
@@ -1227,16 +1220,16 @@ static ssize_t pmbus_show_sensor(struct device *dev,
struct i2c_client *client = to_i2c_client(dev->parent);
struct pmbus_sensor *sensor = to_pmbus_sensor(devattr);
struct pmbus_data *data = i2c_get_clientdata(client);
- ssize_t ret;
+ s64 val;
- mutex_lock(&data->update_lock);
- pmbus_update_sensor_data(client, sensor);
- if (sensor->data < 0)
- ret = sensor->data;
- else
- ret = sysfs_emit(buf, "%lld\n", pmbus_reg2data(data, sensor));
- mutex_unlock(&data->update_lock);
- return ret;
+ scoped_guard(pmbus_lock, client) {
+ pmbus_update_sensor_data(client, sensor);
+ if (sensor->data < 0)
+ return sensor->data;
+ val = pmbus_reg2data(data, sensor);
+ }
+
+ return sysfs_emit(buf, "%lld\n", val);
}
static ssize_t pmbus_set_sensor(struct device *dev,
@@ -1246,7 +1239,6 @@ static ssize_t pmbus_set_sensor(struct device *dev,
struct i2c_client *client = to_i2c_client(dev->parent);
struct pmbus_data *data = i2c_get_clientdata(client);
struct pmbus_sensor *sensor = to_pmbus_sensor(devattr);
- ssize_t rv = count;
s64 val;
int ret;
u16 regval;
@@ -1254,15 +1246,15 @@ static ssize_t pmbus_set_sensor(struct device *dev,
if (kstrtos64(buf, 10, &val) < 0)
return -EINVAL;
- mutex_lock(&data->update_lock);
+ guard(pmbus_lock)(client);
+
regval = pmbus_data2reg(data, sensor, val);
ret = _pmbus_write_word_data(client, sensor->page, sensor->reg, regval);
if (ret < 0)
- rv = ret;
- else
- sensor->data = -ENODATA;
- mutex_unlock(&data->update_lock);
- return rv;
+ return ret;
+
+ sensor->data = -ENODATA;
+ return count;
}
static ssize_t pmbus_show_label(struct device *dev,
@@ -1364,7 +1356,7 @@ static int pmbus_thermal_get_temp(struct thermal_zone_device *tz, int *temp)
struct pmbus_data *pmbus_data = tdata->pmbus_data;
struct i2c_client *client = to_i2c_client(pmbus_data->dev);
struct device *dev = pmbus_data->hwmon_dev;
- int ret = 0;
+ int _temp;
if (!dev) {
/* May not even get to hwmon yet */
@@ -1372,15 +1364,15 @@ static int pmbus_thermal_get_temp(struct thermal_zone_device *tz, int *temp)
return 0;
}
- mutex_lock(&pmbus_data->update_lock);
- pmbus_update_sensor_data(client, sensor);
- if (sensor->data < 0)
- ret = sensor->data;
- else
- *temp = (int)pmbus_reg2data(pmbus_data, sensor);
- mutex_unlock(&pmbus_data->update_lock);
+ scoped_guard(pmbus_lock, client) {
+ pmbus_update_sensor_data(client, sensor);
+ if (sensor->data < 0)
+ return sensor->data;
+ _temp = (int)pmbus_reg2data(pmbus_data, sensor);
+ }
- return ret;
+ *temp = _temp;
+ return 0;
}
static const struct thermal_zone_device_ops pmbus_thermal_ops = {
@@ -2412,13 +2404,12 @@ static ssize_t pmbus_show_samples(struct device *dev,
int val;
struct i2c_client *client = to_i2c_client(dev->parent);
struct pmbus_samples_reg *reg = to_samples_reg(devattr);
- struct pmbus_data *data = i2c_get_clientdata(client);
- mutex_lock(&data->update_lock);
- val = _pmbus_read_word_data(client, reg->page, 0xff, reg->attr->reg);
- mutex_unlock(&data->update_lock);
- if (val < 0)
- return val;
+ scoped_guard(pmbus_lock, client) {
+ val = _pmbus_read_word_data(client, reg->page, 0xff, reg->attr->reg);
+ if (val < 0)
+ return val;
+ }
return sysfs_emit(buf, "%d\n", val);
}
@@ -2431,14 +2422,13 @@ static ssize_t pmbus_set_samples(struct device *dev,
long val;
struct i2c_client *client = to_i2c_client(dev->parent);
struct pmbus_samples_reg *reg = to_samples_reg(devattr);
- struct pmbus_data *data = i2c_get_clientdata(client);
if (kstrtol(buf, 0, &val) < 0)
return -EINVAL;
- mutex_lock(&data->update_lock);
+ guard(pmbus_lock)(client);
+
ret = _pmbus_write_word_data(client, reg->page, reg->attr->reg, val);
- mutex_unlock(&data->update_lock);
return ret ? : count;
}
@@ -2950,14 +2940,9 @@ static int _pmbus_is_enabled(struct i2c_client *client, u8 page)
static int __maybe_unused pmbus_is_enabled(struct i2c_client *client, u8 page)
{
- struct pmbus_data *data = i2c_get_clientdata(client);
- int ret;
+ guard(pmbus_lock)(client);
- mutex_lock(&data->update_lock);
- ret = _pmbus_is_enabled(client, page);
- mutex_unlock(&data->update_lock);
-
- return ret;
+ return _pmbus_is_enabled(client, page);
}
#define to_dev_attr(_dev_attr) \
@@ -2987,14 +2972,13 @@ static void pmbus_notify(struct pmbus_data *data, int page, int reg, int flags)
}
}
-static int _pmbus_get_flags(struct pmbus_data *data, u8 page, unsigned int *flags,
+static int _pmbus_get_flags(struct i2c_client *client, u8 page, unsigned int *flags,
unsigned int *event, bool notify)
{
+ struct pmbus_data *data = i2c_get_clientdata(client);
int i, status;
const struct pmbus_status_category *cat;
const struct pmbus_status_assoc *bit;
- struct device *dev = data->dev;
- struct i2c_client *client = to_i2c_client(dev);
int func = data->info->func[page];
*flags = 0;
@@ -3070,16 +3054,12 @@ static int _pmbus_get_flags(struct pmbus_data *data, u8 page, unsigned int *flag
return 0;
}
-static int __maybe_unused pmbus_get_flags(struct pmbus_data *data, u8 page, unsigned int *flags,
+static int __maybe_unused pmbus_get_flags(struct i2c_client *client, u8 page, unsigned int *flags,
unsigned int *event, bool notify)
{
- int ret;
+ guard(pmbus_lock)(client);
- mutex_lock(&data->update_lock);
- ret = _pmbus_get_flags(data, page, flags, event, notify);
- mutex_unlock(&data->update_lock);
-
- return ret;
+ return _pmbus_get_flags(client, page, flags, event, notify);
}
#if IS_ENABLED(CONFIG_REGULATOR)
@@ -3095,17 +3075,13 @@ static int _pmbus_regulator_on_off(struct regulator_dev *rdev, bool enable)
{
struct device *dev = rdev_get_dev(rdev);
struct i2c_client *client = to_i2c_client(dev->parent);
- struct pmbus_data *data = i2c_get_clientdata(client);
u8 page = rdev_get_id(rdev);
- int ret;
- mutex_lock(&data->update_lock);
- ret = pmbus_update_byte_data(client, page, PMBUS_OPERATION,
- PB_OPERATION_CONTROL_ON,
- enable ? PB_OPERATION_CONTROL_ON : 0);
- mutex_unlock(&data->update_lock);
+ guard(pmbus_lock)(client);
- return ret;
+ return pmbus_update_byte_data(client, page, PMBUS_OPERATION,
+ PB_OPERATION_CONTROL_ON,
+ enable ? PB_OPERATION_CONTROL_ON : 0);
}
static int pmbus_regulator_enable(struct regulator_dev *rdev)
@@ -3122,54 +3098,41 @@ static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned
{
struct device *dev = rdev_get_dev(rdev);
struct i2c_client *client = to_i2c_client(dev->parent);
- struct pmbus_data *data = i2c_get_clientdata(client);
int event;
- return pmbus_get_flags(data, rdev_get_id(rdev), flags, &event, false);
+ return pmbus_get_flags(client, rdev_get_id(rdev), flags, &event, false);
}
static int pmbus_regulator_get_status(struct regulator_dev *rdev)
{
struct device *dev = rdev_get_dev(rdev);
struct i2c_client *client = to_i2c_client(dev->parent);
- struct pmbus_data *data = i2c_get_clientdata(client);
u8 page = rdev_get_id(rdev);
int status, ret;
int event;
- mutex_lock(&data->update_lock);
- status = pmbus_get_status(client, page, PMBUS_STATUS_WORD);
- if (status < 0) {
- ret = status;
- goto unlock;
- }
+ guard(pmbus_lock)(client);
- if (status & PB_STATUS_OFF) {
- ret = REGULATOR_STATUS_OFF;
- goto unlock;
- }
+ status = pmbus_get_status(client, page, PMBUS_STATUS_WORD);
+ if (status < 0)
+ return status;
+
+ if (status & PB_STATUS_OFF)
+ return REGULATOR_STATUS_OFF;
/* If regulator is ON & reports power good then return ON */
- if (!(status & PB_STATUS_POWER_GOOD_N)) {
- ret = REGULATOR_STATUS_ON;
- goto unlock;
- }
+ if (!(status & PB_STATUS_POWER_GOOD_N))
+ return REGULATOR_STATUS_ON;
- ret = _pmbus_get_flags(data, rdev_get_id(rdev), &status, &event, false);
+ ret = _pmbus_get_flags(client, rdev_get_id(rdev), &status, &event, false);
if (ret)
- goto unlock;
+ return ret;
if (status & (REGULATOR_ERROR_UNDER_VOLTAGE | REGULATOR_ERROR_OVER_CURRENT |
- REGULATOR_ERROR_REGULATION_OUT | REGULATOR_ERROR_FAIL | REGULATOR_ERROR_OVER_TEMP)) {
- ret = REGULATOR_STATUS_ERROR;
- goto unlock;
- }
+ REGULATOR_ERROR_REGULATION_OUT | REGULATOR_ERROR_FAIL | REGULATOR_ERROR_OVER_TEMP))
+ return REGULATOR_STATUS_ERROR;
- ret = REGULATOR_STATUS_UNDEFINED;
-
-unlock:
- mutex_unlock(&data->update_lock);
- return ret;
+ return REGULATOR_STATUS_UNDEFINED;
}
static int pmbus_regulator_get_low_margin(struct i2c_client *client, int page)
@@ -3234,19 +3197,16 @@ static int pmbus_regulator_get_voltage(struct regulator_dev *rdev)
.class = PSC_VOLTAGE_OUT,
.convert = true,
};
- int ret;
+ int voltage;
- mutex_lock(&data->update_lock);
- s.data = _pmbus_read_word_data(client, s.page, 0xff, PMBUS_READ_VOUT);
- if (s.data < 0) {
- ret = s.data;
- goto unlock;
+ scoped_guard(pmbus_lock, client) {
+ s.data = _pmbus_read_word_data(client, s.page, 0xff, PMBUS_READ_VOUT);
+ if (s.data < 0)
+ return s.data;
+ voltage = (int)pmbus_reg2data(data, &s);
}
- ret = (int)pmbus_reg2data(data, &s) * 1000; /* unit is uV */
-unlock:
- mutex_unlock(&data->update_lock);
- return ret;
+ return voltage * 1000; /* unit is uV */
}
static int pmbus_regulator_set_voltage(struct regulator_dev *rdev, int min_uv,
@@ -3263,22 +3223,18 @@ 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);
+ guard(pmbus_lock)(client);
+
low = pmbus_regulator_get_low_margin(client, s.page);
- if (low < 0) {
- ret = low;
- goto unlock;
- }
+ if (low < 0)
+ return low;
high = pmbus_regulator_get_high_margin(client, s.page);
- if (high < 0) {
- ret = high;
- goto unlock;
- }
+ if (high < 0)
+ return high;
/* Make sure we are within margins */
if (low > val)
@@ -3288,10 +3244,7 @@ static int pmbus_regulator_set_voltage(struct regulator_dev *rdev, int min_uv,
val = pmbus_data2reg(data, &s, val);
- ret = _pmbus_write_word_data(client, s.page, PMBUS_VOUT_COMMAND, (u16)val);
-unlock:
- mutex_unlock(&data->update_lock);
- return ret;
+ return _pmbus_write_word_data(client, s.page, PMBUS_VOUT_COMMAND, (u16)val);
}
static int pmbus_regulator_list_voltage(struct regulator_dev *rdev,
@@ -3301,7 +3254,6 @@ 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;
@@ -3314,29 +3266,20 @@ 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);
+ guard(pmbus_lock)(client);
low = pmbus_regulator_get_low_margin(client, rdev_get_id(rdev));
- if (low < 0) {
- ret = low;
- goto unlock;
- }
+ if (low < 0)
+ return low;
high = pmbus_regulator_get_high_margin(client, rdev_get_id(rdev));
- if (high < 0) {
- ret = high;
- goto unlock;
- }
+ if (high < 0)
+ return high;
- if (val >= low && val <= high) {
- ret = val * 1000; /* unit is uV */
- goto unlock;
- }
+ if (val >= low && val <= high)
+ return val * 1000; /* unit is uV */
- ret = 0;
-unlock:
- mutex_unlock(&data->update_lock);
- return ret;
+ return 0;
}
const struct regulator_ops pmbus_regulator_ops = {
@@ -3472,16 +3415,16 @@ static irqreturn_t pmbus_fault_handler(int irq, void *pdata)
struct i2c_client *client = to_i2c_client(data->dev);
int i, status, event;
- mutex_lock(&data->update_lock);
+ guard(pmbus_lock)(client);
+
for (i = 0; i < data->info->pages; i++) {
- _pmbus_get_flags(data, i, &status, &event, true);
+ _pmbus_get_flags(client, i, &status, &event, true);
if (event)
pmbus_regulator_notify(data, i, event);
}
pmbus_clear_faults(client);
- mutex_unlock(&data->update_lock);
return IRQ_HANDLED;
}
@@ -3537,15 +3480,13 @@ static struct dentry *pmbus_debugfs_dir; /* pmbus debugfs directory */
static int pmbus_debugfs_get(void *data, u64 *val)
{
- int rc;
struct pmbus_debugfs_entry *entry = data;
- struct pmbus_data *pdata = i2c_get_clientdata(entry->client);
+ struct i2c_client *client = entry->client;
+ int rc;
- rc = mutex_lock_interruptible(&pdata->update_lock);
- if (rc)
- return rc;
- rc = _pmbus_read_byte_data(entry->client, entry->page, entry->reg);
- mutex_unlock(&pdata->update_lock);
+ guard(pmbus_lock)(client);
+
+ rc = _pmbus_read_byte_data(client, entry->page, entry->reg);
if (rc < 0)
return rc;
@@ -3558,15 +3499,14 @@ DEFINE_DEBUGFS_ATTRIBUTE(pmbus_debugfs_ops, pmbus_debugfs_get, NULL,
static int pmbus_debugfs_get_status(void *data, u64 *val)
{
- int rc;
struct pmbus_debugfs_entry *entry = data;
- struct pmbus_data *pdata = i2c_get_clientdata(entry->client);
+ struct i2c_client *client = entry->client;
+ struct pmbus_data *pdata = i2c_get_clientdata(client);
+ int rc;
- rc = mutex_lock_interruptible(&pdata->update_lock);
- if (rc)
- return rc;
- rc = pdata->read_status(entry->client, entry->page);
- mutex_unlock(&pdata->update_lock);
+ guard(pmbus_lock)(client);
+
+ rc = pdata->read_status(client, entry->page);
if (rc < 0)
return rc;
@@ -3582,17 +3522,14 @@ static ssize_t pmbus_debugfs_block_read(struct file *file, char __user *buf,
{
int rc;
struct pmbus_debugfs_entry *entry = file->private_data;
- struct pmbus_data *pdata = i2c_get_clientdata(entry->client);
+ struct i2c_client *client = entry->client;
char data[I2C_SMBUS_BLOCK_MAX + 2] = { 0 };
- rc = mutex_lock_interruptible(&pdata->update_lock);
- if (rc)
- return rc;
- rc = pmbus_read_block_data(entry->client, entry->page, entry->reg,
- data);
- mutex_unlock(&pdata->update_lock);
- if (rc < 0)
- return rc;
+ scoped_guard(pmbus_lock, client) {
+ rc = pmbus_read_block_data(client, entry->page, entry->reg, data);
+ if (rc < 0)
+ return rc;
+ }
/* Add newline at the end of a read data */
data[rc] = '\n';
--
2.45.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 0/5] hwmon: (pmbus) PMBus fixes and improvements
2026-03-25 18:16 [PATCH 0/5] hwmon: (pmbus) PMBus fixes and improvements Guenter Roeck
` (4 preceding siblings ...)
2026-03-25 18:16 ` [PATCH 5/5] hwmon: (pmbus_core) Use guard() for mutex protection Guenter Roeck
@ 2026-03-25 20:25 ` Pradhan, Sanman
2026-03-25 20:31 ` Guenter Roeck
5 siblings, 1 reply; 10+ messages in thread
From: Pradhan, Sanman @ 2026-03-25 20:25 UTC (permalink / raw)
To: linux@roeck-us.net
Cc: linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org,
Sanman Pradhan
From: Sanman Pradhan <psanman@juniper.net>
Thanks for the clarification on patch 3/5.
Reviewed the series and happy to add my tag.
Reviewed-by: Sanman Pradhan <psanman@juniper.net>
^ permalink raw reply [flat|nested] 10+ messages in thread