[PATCH v3 1/2] hwmon: (pmbus) export pmbus_wait and pmbus_update_ts

Sanman Pradhan posted 2 patches 2 weeks, 4 days ago
There is a newer version of this series
[PATCH v3 1/2] hwmon: (pmbus) export pmbus_wait and pmbus_update_ts
Posted by Sanman Pradhan 2 weeks, 4 days ago
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>
---
v3:
- No changes to this patch in this version.

v2:
- New patch in the series.
- Export pmbus_wait() and pmbus_update_ts() to the PMBUS namespace.
- Relocate PMBUS_OP_* bitmasks to the subsystem header.
---
 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 3ddcb742d289..56620ed4ac9c 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, nvidia195mv };
 #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 4d7634ee6148..b150c2ee670a 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
Re: [PATCH v3 1/2] hwmon: (pmbus) export pmbus_wait and pmbus_update_ts
Posted by Guenter Roeck 2 weeks, 4 days ago
On Wed, Mar 18, 2026 at 12:06:42PM -0700, Sanman Pradhan wrote:
> 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>

I get:

WARNING: From:/Signed-off-by: email address mismatch: 'From: Sanman Pradhan <sanman.p211993@gmail.com>' != 'Signed-off-by: Sanman Pradhan <psanman@juniper.net>'

when trying to apply this patch. Please resend both patches
with matching addresses.

Please have a look at the AI review results at
https://sashiko.dev/#/patchset/20260318190643.54372-1-psanman%40juniper.net

This is unrelated to this patch, but we'll have to find a solution
for the torn reads on 32-bit platforms (I never thought about that
possibility).

Thanks,
Guenter

> ---
> v3:
> - No changes to this patch in this version.
> 
> v2:
> - New patch in the series.
> - Export pmbus_wait() and pmbus_update_ts() to the PMBUS namespace.
> - Relocate PMBUS_OP_* bitmasks to the subsystem header.
> ---
>  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 3ddcb742d289..56620ed4ac9c 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, nvidia195mv };
>  #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 4d7634ee6148..b150c2ee670a 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)
>  {