* [PATCH v2 0/2] hwmon: (pmbus/max31785) refactor inter-access delay
@ 2026-03-09 0:45 Sanman Pradhan
2026-03-09 0:45 ` [PATCH v2 1/2] hwmon: (pmbus) export pmbus_wait and pmbus_update_ts Sanman Pradhan
2026-03-09 0:45 ` [PATCH v2 2/2] hwmon: (pmbus/max31785) use access_delay for PMBus-mediated accesses Sanman Pradhan
0 siblings, 2 replies; 5+ messages in thread
From: Sanman Pradhan @ 2026-03-09 0:45 UTC (permalink / raw)
To: Sanman Pradhan, Guenter Roeck; +Cc: Sanman Pradhan, linux-hwmon, linux-kernel
From: Sanman Pradhan <psanman@juniper.net>
Thanks for applying Patch 1 from the previous series.
This v2 series addresses the feedback regarding the raw i2c_transfer()
error path in the MAX31785 driver. Instead of relying on local
usleep_range() calls, this series exports the PMBus core timing helpers
so the driver can utilize the core's delay management.
Changes in v2:
- Dropped the i2c_smbus_write_byte_data() wrapper cleanup (applied upstream).
- Patch 1 (New): Export pmbus_wait() and pmbus_update_ts() from the PMBus
core, and move PMBUS_OP_* macros to drivers/hwmon/pmbus/pmbus.h.
- Patch 2: Replaced the usleep_range() sandwich in max31785_read_long_data()
with the newly exported core timing functions. Placed pmbus_update_ts()
before the error check to ensure the core accurately tracks the bus
transaction even on failure.
Sanman Pradhan (2):
hwmon: (pmbus/core) export pmbus_wait and pmbus_update_ts
hwmon: (pmbus/max31785) use access_delay for PMBus-mediated accesses
drivers/hwmon/pmbus/max31785.c | 184 ++++++++++---------------------
drivers/hwmon/pmbus/pmbus.h | 9 ++
drivers/hwmon/pmbus/pmbus_core.c | 13 +--
3 files changed, 70 insertions(+), 136 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] hwmon: (pmbus) export pmbus_wait and pmbus_update_ts
2026-03-09 0:45 [PATCH v2 0/2] hwmon: (pmbus/max31785) refactor inter-access delay Sanman Pradhan
@ 2026-03-09 0:45 ` Sanman Pradhan
2026-03-09 0:45 ` [PATCH v2 2/2] hwmon: (pmbus/max31785) use access_delay for PMBus-mediated accesses Sanman Pradhan
1 sibling, 0 replies; 5+ messages in thread
From: Sanman Pradhan @ 2026-03-09 0:45 UTC (permalink / raw)
To: Sanman Pradhan, Guenter Roeck; +Cc: Sanman Pradhan, linux-hwmon, linux-kernel
From: Sanman Pradhan <psanman@juniper.net>
Some PMBus devices require strict inter-transaction delays to avoid
NACKs or communication faults. The PMBus core manages this automatically
for standard PMBus accesses via pmbus_wait() and pmbus_update_ts().
However, when a device driver performs raw I2C/SMBus transfers (e.g.,
for long reads or custom commands) that bypass the PMBus core, the core's
timing state machine is unaware of the transaction. This can cause the
next core-mediated PMBus access to violate the device's required delay.
Export pmbus_wait() and pmbus_update_ts() to the PMBUS namespace so
device-specific drivers can explicitly synchronize their raw transfers
with the core's delay management.
Additionally, move the PMBUS_OP_WRITE and PMBUS_OP_PAGE_CHANGE bitmasks
into the drivers/hwmon/pmbus/pmbus.h header so callers can accurately
report the nature of their raw transactions.
Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
drivers/hwmon/pmbus/pmbus.h | 9 +++++++++
drivers/hwmon/pmbus/pmbus_core.c | 13 ++++---------
2 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index d2e9bfb5320f..afd58128b37e 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -424,6 +424,13 @@ enum vrm_version { vr11 = 0, vr12, vr13, imvp9, amd625mv };
#define PMBUS_REV_12 0x22 /* PMBus revision 1.2 */
#define PMBUS_REV_13 0x33 /* PMBus revision 1.3 */
+/*
+ * The type of operation used for picking the delay between
+ * successive pmbus operations.
+ */
+#define PMBUS_OP_WRITE BIT(0)
+#define PMBUS_OP_PAGE_CHANGE BIT(1)
+
struct pmbus_driver_info {
int pages; /* Total number of pages */
u8 phases[PMBUS_PAGES]; /* Number of phases per page */
@@ -555,6 +562,8 @@ int pmbus_update_byte_data(struct i2c_client *client, int page, u8 reg,
void pmbus_clear_faults(struct i2c_client *client);
bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg);
bool pmbus_check_word_register(struct i2c_client *client, int page, int reg);
+void pmbus_wait(struct i2c_client *client);
+void pmbus_update_ts(struct i2c_client *client, int op);
int pmbus_do_probe(struct i2c_client *client, struct pmbus_driver_info *info);
const struct pmbus_driver_info *pmbus_get_driver_info(struct i2c_client
*client);
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index be6d05def115..4864a30988bf 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -32,13 +32,6 @@
#define PMBUS_ATTR_ALLOC_SIZE 32
#define PMBUS_NAME_SIZE 24
-/*
- * The type of operation used for picking the delay between
- * successive pmbus operations.
- */
-#define PMBUS_OP_WRITE BIT(0)
-#define PMBUS_OP_PAGE_CHANGE BIT(1)
-
static int wp = -1;
module_param(wp, int, 0444);
@@ -173,7 +166,7 @@ void pmbus_set_update(struct i2c_client *client, u8 reg, bool update)
EXPORT_SYMBOL_NS_GPL(pmbus_set_update, "PMBUS");
/* Some chips need a delay between accesses. */
-static void pmbus_wait(struct i2c_client *client)
+void pmbus_wait(struct i2c_client *client)
{
struct pmbus_data *data = i2c_get_clientdata(client);
s64 delay = ktime_us_delta(data->next_access_backoff, ktime_get());
@@ -181,9 +174,10 @@ static void pmbus_wait(struct i2c_client *client)
if (delay > 0)
fsleep(delay);
}
+EXPORT_SYMBOL_NS_GPL(pmbus_wait, "PMBUS");
/* Sets the last operation timestamp for pmbus_wait */
-static void pmbus_update_ts(struct i2c_client *client, int op)
+void pmbus_update_ts(struct i2c_client *client, int op)
{
struct pmbus_data *data = i2c_get_clientdata(client);
const struct pmbus_driver_info *info = data->info;
@@ -197,6 +191,7 @@ static void pmbus_update_ts(struct i2c_client *client, int op)
if (delay > 0)
data->next_access_backoff = ktime_add_us(ktime_get(), delay);
}
+EXPORT_SYMBOL_NS_GPL(pmbus_update_ts, "PMBUS");
int pmbus_set_page(struct i2c_client *client, int page, int phase)
{
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] hwmon: (pmbus/max31785) use access_delay for PMBus-mediated accesses
2026-03-09 0:45 [PATCH v2 0/2] hwmon: (pmbus/max31785) refactor inter-access delay Sanman Pradhan
2026-03-09 0:45 ` [PATCH v2 1/2] hwmon: (pmbus) export pmbus_wait and pmbus_update_ts Sanman Pradhan
@ 2026-03-09 0:45 ` Sanman Pradhan
2026-03-17 18:11 ` Sanman Pradhan
2026-03-17 18:53 ` Guenter Roeck
1 sibling, 2 replies; 5+ messages in thread
From: Sanman Pradhan @ 2026-03-09 0:45 UTC (permalink / raw)
To: Sanman Pradhan, Guenter Roeck; +Cc: Sanman Pradhan, linux-hwmon, linux-kernel
From: Sanman Pradhan <psanman@juniper.net>
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.
Simplify the driver by using 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(). For the raw i2c_transfer() long-read path, which
bypasses PMBus core timing, leverage the newly exported pmbus_wait() and
pmbus_update_ts() core functions. This replaces hardcoded usleep_range()
calls and ensures the PMBus core tracks the raw transaction. Placing
pmbus_update_ts() before the error check fixes a bug where a failed
i2c_transfer() would skip the delay and impact subsequent PMBus commands.
Additionally, 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, we can safely
drop the custom max31785_update_fan() wrapper in favor of the core
pmbus_update_fan() helper.
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@juniper.net>
---
drivers/hwmon/pmbus/max31785.c | 187 +++++++++++----------------------
1 file changed, 59 insertions(+), 128 deletions(-)
diff --git a/drivers/hwmon/pmbus/max31785.c b/drivers/hwmon/pmbus/max31785.c
index 50073fe0c5e8..098f43486c45 100644
--- a/drivers/hwmon/pmbus/max31785.c
+++ b/drivers/hwmon/pmbus/max31785.c
@@ -31,8 +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 +38,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 +51,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,7 +128,20 @@ static int max31785_read_long_data(struct i2c_client *client, int page,
if (rc < 0)
return rc;
+ /* Ensure the raw transfer is properly spaced from the
+ * preceding PMBus transaction.
+ */
+ pmbus_wait(client);
+
rc = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
+
+ /*
+ * Update PMBus core timing state for the raw transfer, even on error.
+ * Pass 0 as the operation mask since this is a raw read, intentionally
+ * neither PMBUS_OP_WRITE nor PMBUS_OP_PAGE_CHANGE.
+ */
+ pmbus_update_ts(client, 0);
+
if (rc < 0)
return rc;
@@ -203,19 +166,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 +195,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 +223,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 +254,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 +283,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 +321,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 +369,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 +402,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 +411,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 +440,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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] hwmon: (pmbus/max31785) use access_delay for PMBus-mediated accesses
2026-03-09 0:45 ` [PATCH v2 2/2] hwmon: (pmbus/max31785) use access_delay for PMBus-mediated accesses Sanman Pradhan
@ 2026-03-17 18:11 ` Sanman Pradhan
2026-03-17 18:53 ` Guenter Roeck
1 sibling, 0 replies; 5+ messages in thread
From: Sanman Pradhan @ 2026-03-17 18:11 UTC (permalink / raw)
To: Sanman Pradhan, Guenter Roeck; +Cc: Sanman Pradhan, linux-hwmon, linux-kernel
Hi Guenter,
Just a friendly ping on this v2 series when you have a moment.
Let me know if you need any further changes
Thank you.
Regards,
Sanman Pradhan
On Sun, 8 Mar 2026 at 17:46, Sanman Pradhan <sanman.p211993@gmail.com> wrote:
>
> From: Sanman Pradhan <psanman@juniper.net>
>
> 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.
>
> Simplify the driver by using 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(). For the raw i2c_transfer() long-read path, which
> bypasses PMBus core timing, leverage the newly exported pmbus_wait() and
> pmbus_update_ts() core functions. This replaces hardcoded usleep_range()
> calls and ensures the PMBus core tracks the raw transaction. Placing
> pmbus_update_ts() before the error check fixes a bug where a failed
> i2c_transfer() would skip the delay and impact subsequent PMBus commands.
>
> Additionally, 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, we can safely
> drop the custom max31785_update_fan() wrapper in favor of the core
> pmbus_update_fan() helper.
>
> 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@juniper.net>
> ---
> drivers/hwmon/pmbus/max31785.c | 187 +++++++++++----------------------
> 1 file changed, 59 insertions(+), 128 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/max31785.c b/drivers/hwmon/pmbus/max31785.c
> index 50073fe0c5e8..098f43486c45 100644
> --- a/drivers/hwmon/pmbus/max31785.c
> +++ b/drivers/hwmon/pmbus/max31785.c
> @@ -31,8 +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 +38,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 +51,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,7 +128,20 @@ static int max31785_read_long_data(struct i2c_client *client, int page,
> if (rc < 0)
> return rc;
>
> + /* Ensure the raw transfer is properly spaced from the
> + * preceding PMBus transaction.
> + */
> + pmbus_wait(client);
> +
> rc = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
> +
> + /*
> + * Update PMBus core timing state for the raw transfer, even on error.
> + * Pass 0 as the operation mask since this is a raw read, intentionally
> + * neither PMBUS_OP_WRITE nor PMBUS_OP_PAGE_CHANGE.
> + */
> + pmbus_update_ts(client, 0);
> +
> if (rc < 0)
> return rc;
>
> @@ -203,19 +166,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 +195,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 +223,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 +254,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 +283,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 +321,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 +369,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 +402,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 +411,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 +440,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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] hwmon: (pmbus/max31785) use access_delay for PMBus-mediated accesses
2026-03-09 0:45 ` [PATCH v2 2/2] hwmon: (pmbus/max31785) use access_delay for PMBus-mediated accesses Sanman Pradhan
2026-03-17 18:11 ` Sanman Pradhan
@ 2026-03-17 18:53 ` Guenter Roeck
1 sibling, 0 replies; 5+ messages in thread
From: Guenter Roeck @ 2026-03-17 18:53 UTC (permalink / raw)
To: Sanman Pradhan; +Cc: Sanman Pradhan, linux-hwmon, linux-kernel
On 3/8/26 17:45, Sanman Pradhan wrote:
> From: Sanman Pradhan <psanman@juniper.net>
>
> 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.
>
> Simplify the driver by using 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(). For the raw i2c_transfer() long-read path, which
> bypasses PMBus core timing, leverage the newly exported pmbus_wait() and
> pmbus_update_ts() core functions. This replaces hardcoded usleep_range()
> calls and ensures the PMBus core tracks the raw transaction. Placing
> pmbus_update_ts() before the error check fixes a bug where a failed
> i2c_transfer() would skip the delay and impact subsequent PMBus commands.
>
> Additionally, 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, we can safely
> drop the custom max31785_update_fan() wrapper in favor of the core
> pmbus_update_fan() helper.
>
> 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@juniper.net>
AI review feedback (from https://sashiko.dev/#/patchset/20260309004508.78609-1-sanman.p211993%40gmail.com)
below.
Guenter
> ---
> drivers/hwmon/pmbus/max31785.c | 187 +++++++++++----------------------
> 1 file changed, 59 insertions(+), 128 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/max31785.c b/drivers/hwmon/pmbus/max31785.c
> index 50073fe0c5e8..098f43486c45 100644
> --- a/drivers/hwmon/pmbus/max31785.c
> +++ b/drivers/hwmon/pmbus/max31785.c
> @@ -31,8 +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 +38,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 +51,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,7 +128,20 @@ static int max31785_read_long_data(struct i2c_client *client, int page,
> if (rc < 0)
> return rc;
>
> + /* Ensure the raw transfer is properly spaced from the
> + * preceding PMBus transaction.
> + */
> + pmbus_wait(client);
> +
> rc = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
> +
> + /*
> + * Update PMBus core timing state for the raw transfer, even on error.
> + * Pass 0 as the operation mask since this is a raw read, intentionally
> + * neither PMBUS_OP_WRITE nor PMBUS_OP_PAGE_CHANGE.
> + */
> + pmbus_update_ts(client, 0);
> +
> if (rc < 0)
> return rc;
>
> @@ -203,19 +166,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 +195,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 +223,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 +254,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 +283,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 +321,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 +369,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 +402,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 +411,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 +440,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;
> }
Is there a missing inter-access delay before the first PMBus core transaction?
During probe, the driver performs raw I2C accesses and enforces the delay
using max31785_wait(data). The driver then calls pmbus_do_probe(), which
initializes the PMBus core delay tracking variable.
When the PMBus core executes its very first command, its internal delay
logic calculates a negative delay against the newly initialized variable and
executes the transaction without delay.
Because the PMBus core is unaware of the driver local timestamp, could
this violate the 250us delay requirement from the last pre-probe driver
access and cause the controller to NACK the first command?
Should there be an explicit max31785_wait(data) right before calling
pmbus_do_probe() to prevent this?
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-03-17 18:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-09 0:45 [PATCH v2 0/2] hwmon: (pmbus/max31785) refactor inter-access delay Sanman Pradhan
2026-03-09 0:45 ` [PATCH v2 1/2] hwmon: (pmbus) export pmbus_wait and pmbus_update_ts Sanman Pradhan
2026-03-09 0:45 ` [PATCH v2 2/2] hwmon: (pmbus/max31785) use access_delay for PMBus-mediated accesses Sanman Pradhan
2026-03-17 18:11 ` Sanman Pradhan
2026-03-17 18:53 ` Guenter Roeck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®