Re: [PATCH v2 5/6] misc: amd-sbi: Add SBTSI ioctl register transfer interface

[Guenter Roeck]

On 5/15/26 06:45, Akshay Gupta wrote:
> From: Prathima <Prathima.Lk@xxxxxxx>
>
> Implement IOCTL interface for SB-TSI driver to enable userspace access
> to TSI register read/write operations through the AMD Advanced Platform
> Management Link (APML) protocol.
> Add an ioctl command (SBTSI_IOCTL_REG_XFER_CMD) that accepts a register
> address, data byte, and direction flag. Serialize access with a mutex
> shared between the hwmon and ioctl paths to prevent concurrent bus
> transactions from corrupting register state.
>
> Reviewed-by: Akshay Gupta <Akshay.Gupta@xxxxxxx>
> Signed-off-by: Prathima <Prathima.Lk@xxxxxxx>
> ---
> Changes since v1:
> - Use of devm_mutex_init in place of mutex_init
> - Use of guard_mutex in place of mutex_lock()/mutex_unlock()
> - Use of devm_add_action_or_reset() for clean removal
>   
>   drivers/hwmon/sbtsi_temp.c      |  6 +++
>   drivers/misc/amd-sbi/tsi-core.c | 84 ++++++++++++++++++++++++++++++++-
>   drivers/misc/amd-sbi/tsi-core.h | 15 ++++++
>   drivers/misc/amd-sbi/tsi.c      | 20 ++++++--
>   include/linux/misc/tsi.h        |  8 ++++
>   include/uapi/misc/amd-apml.h    | 23 +++++++++
>   6 files changed, 151 insertions(+), 5 deletions(-)
>   create mode 100644 drivers/misc/amd-sbi/tsi-core.h
>
> diff --git a/drivers/hwmon/sbtsi_temp.c b/drivers/hwmon/sbtsi_temp.c
> index d7ae986d824c..00e982f4c716 100644
> --- a/drivers/hwmon/sbtsi_temp.c
> +++ b/drivers/hwmon/sbtsi_temp.c
> @@ -64,12 +64,15 @@ static inline void sbtsi_mc_to_reg(s32 temp, u8 *integer, u8 *decimal)
>   /*
>    * Read integer and decimal parts of an SB-TSI temperature register pair
>    * The read order is determined by the ReadOrder bit to ensure atomic latching.
> + * The mutex protects against concurrent access to the shared I2C/I3C bus by
> + * the hwmon sysfs and a userspace ioctl
>    */
>   static int sbtsi_temp_read(struct sbtsi_data *data, u8 reg1, u8 reg2,
>   			   u8 *val1, u8 *val2)
>   {
>   	int ret;
>   
> +	guard(mutex)(&data->lock);

I would suggest to hide this behind access functions such as sbtsi_lock(),
sbtsi_unlock(), and the matching guard functions. That can be done in a
separate patch; it should not be necessary to include hwmon in the patch
introducing the ioctl.

Thanks,
Guenter