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

[Gupta, Akshay]

On 5/15/2026 7:41 PM, Guenter Roeck wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> 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

Thank you for the feedback, Guenter.

In the next version, we will address this by implementing the matching 
guard function in the ioctl patch and creating a new patch for hwmon.

>