[PATCH 2/2] hwmon: (pmbus/max31785) use access_delay for PMBus-mediated accesses
From: Sanman Pradhan
Date: Sat Mar 07 2026 - 17:46:28 EST
From: Sanman Pradhan <psanman@xxxxxxxxxxx>
The MAX31785 fan controller occasionally NACKs master transactions if
accesses are too tightly spaced. To avoid this, the driver currently
enforces a 250us inter-access delay with a private timestamp and wrapper
functions around both raw SMBus accesses and PMBus helper paths.
Use pmbus_driver_info.access_delay for normal PMBus-mediated accesses
instead, and remove the driver-local PMBus read/write wrappers.
Keep local delay handling for raw SMBus accesses used before
pmbus_do_probe(). Also add explicit delays around the raw i2c_transfer()
long-read path, since it bypasses PMBus core timing and still needs to
be spaced from surrounding transactions.
Update max31785_read_byte_data() so PMBUS_FAN_CONFIG_12 accesses are
only remapped for virtual pages, allowing physical-page accesses to fall
back to the PMBus core. With that change, use pmbus_update_fan() for fan
configuration updates.
Also use the delayed raw read helper for MFR_REVISION during probe, drop
the unused to_max31785_data() macro, and rename the local variable
"virtual" to "vpage".
Signed-off-by: Sanman Pradhan <psanman@xxxxxxxxxxx>
---
drivers/hwmon/pmbus/max31785.c | 186 +++++++++++----------------------
1 file changed, 59 insertions(+), 127 deletions(-)
diff --git a/drivers/hwmon/pmbus/max31785.c b/drivers/hwmon/pmbus/max31785.c
index 50073fe0c5e8..e9c3c36c8663 100644
--- a/drivers/hwmon/pmbus/max31785.c
+++ b/drivers/hwmon/pmbus/max31785.c
@@ -31,7 +31,6 @@ struct max31785_data {
struct pmbus_driver_info info;
};
-#define to_max31785_data(x) container_of(x, struct max31785_data, info)
/*
* MAX31785 Driver Workaround
@@ -40,9 +39,8 @@ struct max31785_data {
* These issues are not indicated by the device itself, except for occasional
* NACK responses during master transactions. No error bits are set in STATUS_BYTE.
*
- * To address this, we introduce a delay of 250us between consecutive accesses
- * to the fan controller. This delay helps mitigate communication problems by
- * allowing sufficient time between accesses.
+ * Keep minimal local delay handling for raw pre-probe SMBus accesses.
+ * Normal PMBus-mediated accesses use pmbus_driver_info.access_delay instead.
*/
static inline void max31785_wait(const struct max31785_data *data)
{
@@ -54,89 +52,42 @@ static inline void max31785_wait(const struct max31785_data *data)
}
static int max31785_i2c_write_byte_data(struct i2c_client *client,
- struct max31785_data *driver_data,
- int command, u8 data)
+ struct max31785_data *data,
+ int command, u8 value)
{
int rc;
- max31785_wait(driver_data);
- rc = i2c_smbus_write_byte_data(client, command, data);
- driver_data->access = ktime_get();
+ max31785_wait(data);
+ rc = i2c_smbus_write_byte_data(client, command, value);
+ data->access = ktime_get();
return rc;
}
static int max31785_i2c_read_word_data(struct i2c_client *client,
- struct max31785_data *driver_data,
+ struct max31785_data *data,
int command)
{
int rc;
- max31785_wait(driver_data);
+ max31785_wait(data);
rc = i2c_smbus_read_word_data(client, command);
- driver_data->access = ktime_get();
- return rc;
-}
-
-static int _max31785_read_byte_data(struct i2c_client *client,
- struct max31785_data *driver_data,
- int page, int command)
-{
- int rc;
-
- max31785_wait(driver_data);
- rc = pmbus_read_byte_data(client, page, command);
- driver_data->access = ktime_get();
- return rc;
-}
-
-static int _max31785_write_byte_data(struct i2c_client *client,
- struct max31785_data *driver_data,
- int page, int command, u16 data)
-{
- int rc;
-
- max31785_wait(driver_data);
- rc = pmbus_write_byte_data(client, page, command, data);
- driver_data->access = ktime_get();
- return rc;
-}
-
-static int _max31785_read_word_data(struct i2c_client *client,
- struct max31785_data *driver_data,
- int page, int phase, int command)
-{
- int rc;
-
- max31785_wait(driver_data);
- rc = pmbus_read_word_data(client, page, phase, command);
- driver_data->access = ktime_get();
- return rc;
-}
-
-static int _max31785_write_word_data(struct i2c_client *client,
- struct max31785_data *driver_data,
- int page, int command, u16 data)
-{
- int rc;
-
- max31785_wait(driver_data);
- rc = pmbus_write_word_data(client, page, command, data);
- driver_data->access = ktime_get();
+ data->access = ktime_get();
return rc;
}
static int max31785_read_byte_data(struct i2c_client *client, int page, int reg)
{
- const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
- struct max31785_data *driver_data = to_max31785_data(info);
switch (reg) {
case PMBUS_VOUT_MODE:
return -ENOTSUPP;
case PMBUS_FAN_CONFIG_12:
- return _max31785_read_byte_data(client, driver_data,
- page - MAX31785_NR_PAGES,
- reg);
+ if (page < MAX31785_NR_PAGES)
+ return -ENODATA;
+
+ return pmbus_read_byte_data(client,
+ page - MAX31785_NR_PAGES,
+ reg);
}
return -ENODATA;
@@ -178,9 +129,22 @@ static int max31785_read_long_data(struct i2c_client *client, int page,
if (rc < 0)
return rc;
+ /*
+ * The following raw i2c_transfer() bypasses PMBus core access timing.
+ * Add an explicit delay before the transfer so it is properly spaced
+ * from the preceding PMBus transaction.
+ */
+ usleep_range(MAX31785_WAIT_DELAY_US, MAX31785_WAIT_DELAY_US + 50);
+
rc = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
if (rc < 0)
return rc;
+ /*
+ * The preceding raw i2c_transfer() bypasses PMBus core access timing.
+ * Add an explicit delay after the transfer so the next PMBus access
+ * preserves the required inter-transaction spacing.
+ */
+ usleep_range(MAX31785_WAIT_DELAY_US, MAX31785_WAIT_DELAY_US + 50);
*data = (rspbuf[0] << (0 * 8)) | (rspbuf[1] << (1 * 8)) |
(rspbuf[2] << (2 * 8)) | (rspbuf[3] << (3 * 8));
@@ -203,19 +167,18 @@ static int max31785_get_pwm(struct i2c_client *client, int page)
return rv;
}
-static int max31785_get_pwm_mode(struct i2c_client *client,
- struct max31785_data *driver_data, int page)
+static int max31785_get_pwm_mode(struct i2c_client *client, int page)
{
int config;
int command;
- config = _max31785_read_byte_data(client, driver_data, page,
- PMBUS_FAN_CONFIG_12);
+ config = pmbus_read_byte_data(client, page,
+ PMBUS_FAN_CONFIG_12);
if (config < 0)
return config;
- command = _max31785_read_word_data(client, driver_data, page, 0xff,
- PMBUS_FAN_COMMAND_1);
+ command = pmbus_read_word_data(client, page, 0xff,
+ PMBUS_FAN_COMMAND_1);
if (command < 0)
return command;
@@ -233,8 +196,6 @@ static int max31785_get_pwm_mode(struct i2c_client *client,
static int max31785_read_word_data(struct i2c_client *client, int page,
int phase, int reg)
{
- const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
- struct max31785_data *driver_data = to_max31785_data(info);
u32 val;
int rv;
@@ -263,7 +224,7 @@ static int max31785_read_word_data(struct i2c_client *client, int page,
rv = max31785_get_pwm(client, page);
break;
case PMBUS_VIRT_PWM_ENABLE_1:
- rv = max31785_get_pwm_mode(client, driver_data, page);
+ rv = max31785_get_pwm_mode(client, page);
break;
default:
rv = -ENODATA;
@@ -294,35 +255,7 @@ static inline u32 max31785_scale_pwm(u32 sensor_val)
return (sensor_val * 100) / 255;
}
-static int max31785_update_fan(struct i2c_client *client,
- struct max31785_data *driver_data, int page,
- u8 config, u8 mask, u16 command)
-{
- int from, rv;
- u8 to;
-
- from = _max31785_read_byte_data(client, driver_data, page,
- PMBUS_FAN_CONFIG_12);
- if (from < 0)
- return from;
-
- to = (from & ~mask) | (config & mask);
-
- if (to != from) {
- rv = _max31785_write_byte_data(client, driver_data, page,
- PMBUS_FAN_CONFIG_12, to);
- if (rv < 0)
- return rv;
- }
-
- rv = _max31785_write_word_data(client, driver_data, page,
- PMBUS_FAN_COMMAND_1, command);
-
- return rv;
-}
-
-static int max31785_pwm_enable(struct i2c_client *client,
- struct max31785_data *driver_data, int page,
+static int max31785_pwm_enable(struct i2c_client *client, int page,
u16 word)
{
int config = 0;
@@ -351,23 +284,21 @@ static int max31785_pwm_enable(struct i2c_client *client,
return -EINVAL;
}
- return max31785_update_fan(client, driver_data, page, config,
+ return pmbus_update_fan(client, page, 0, config,
PB_FAN_1_RPM, rate);
}
static int max31785_write_word_data(struct i2c_client *client, int page,
int reg, u16 word)
{
- const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
- struct max31785_data *driver_data = to_max31785_data(info);
switch (reg) {
case PMBUS_VIRT_PWM_1:
- return max31785_update_fan(client, driver_data, page, 0,
- PB_FAN_1_RPM,
- max31785_scale_pwm(word));
+ return pmbus_update_fan(client, page, 0, 0,
+ PB_FAN_1_RPM,
+ max31785_scale_pwm(word));
case PMBUS_VIRT_PWM_ENABLE_1:
- return max31785_pwm_enable(client, driver_data, page, word);
+ return max31785_pwm_enable(client, page, word);
default:
break;
}
@@ -391,6 +322,7 @@ static const struct pmbus_driver_info max31785_info = {
.read_byte_data = max31785_read_byte_data,
.read_word_data = max31785_read_word_data,
.write_byte = max31785_write_byte,
+ .access_delay = MAX31785_WAIT_DELAY_US,
/* RPM */
.format[PSC_FAN] = direct,
@@ -438,29 +370,29 @@ static const struct pmbus_driver_info max31785_info = {
};
static int max31785_configure_dual_tach(struct i2c_client *client,
- struct pmbus_driver_info *info)
+ struct max31785_data *data)
{
+ struct pmbus_driver_info *info = &data->info;
int ret;
int i;
- struct max31785_data *driver_data = to_max31785_data(info);
for (i = 0; i < MAX31785_NR_FAN_PAGES; i++) {
- ret = max31785_i2c_write_byte_data(client, driver_data,
+ ret = max31785_i2c_write_byte_data(client, data,
PMBUS_PAGE, i);
if (ret < 0)
return ret;
- ret = max31785_i2c_read_word_data(client, driver_data,
+ ret = max31785_i2c_read_word_data(client, data,
MFR_FAN_CONFIG);
if (ret < 0)
return ret;
if (ret & MFR_FAN_CONFIG_DUAL_TACH) {
- int virtual = MAX31785_NR_PAGES + i;
+ int vpage = MAX31785_NR_PAGES + i;
- info->pages = virtual + 1;
- info->func[virtual] |= PMBUS_HAVE_FAN12;
- info->func[virtual] |= PMBUS_PAGE_VIRTUAL;
+ info->pages = vpage + 1;
+ info->func[vpage] |= PMBUS_HAVE_FAN12;
+ info->func[vpage] |= PMBUS_PAGE_VIRTUAL;
}
}
@@ -471,7 +403,7 @@ static int max31785_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
struct pmbus_driver_info *info;
- struct max31785_data *driver_data;
+ struct max31785_data *data;
bool dual_tach = false;
int ret;
@@ -480,20 +412,20 @@ static int max31785_probe(struct i2c_client *client)
I2C_FUNC_SMBUS_WORD_DATA))
return -ENODEV;
- driver_data = devm_kzalloc(dev, sizeof(struct max31785_data), GFP_KERNEL);
- if (!driver_data)
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
return -ENOMEM;
- info = &driver_data->info;
- driver_data->access = ktime_get();
+ data->access = ktime_get();
+ info = &data->info;
*info = max31785_info;
- ret = max31785_i2c_write_byte_data(client, driver_data,
- PMBUS_PAGE, 255);
+ ret = max31785_i2c_write_byte_data(client, data,
+ PMBUS_PAGE, 0xff);
if (ret < 0)
return ret;
- ret = i2c_smbus_read_word_data(client, MFR_REVISION);
+ ret = max31785_i2c_read_word_data(client, data, MFR_REVISION);
if (ret < 0)
return ret;
@@ -509,7 +441,7 @@ static int max31785_probe(struct i2c_client *client)
}
if (dual_tach) {
- ret = max31785_configure_dual_tach(client, info);
+ ret = max31785_configure_dual_tach(client, data);
if (ret < 0)
return ret;
}
--
2.34.1