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

[Guenter Roeck]

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@xxxxxxxxxxx>

I get:

WARNING: From:/Signed-off-by: email address mismatch: 'From: Sanman Pradhan <sanman.p211993@xxxxxxxxx>' != 'Signed-off-by: Sanman Pradhan <psanman@xxxxxxxxxxx>'

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)
>  {