Re: [PATCH v13 03/16] drm/xe/hwmon: Fix thermal sensor data handling

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Wed, Oct 14, 2026 at 07:12:07PM +0530, Poosa, Karthik wrote:
> 
> On 14-10-2026 17:46, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [Low] hwmon: Subsystem API Violation - Driver uses private custom mutex instead of hwmon_lock()
> > --
> > 
> > commit 5b607ffd1e7d970e08aa24f25b6f029c27e76b6d
> > Author: Karthik Poosa <karthik.poosa@intel.com>
> > 
> > drm/xe/hwmon: Fix thermal sensor data handling
> > 
> > This commit reads required thermal groups in get_mc_temp() and stores them
> > in a dword-aligned buffer, avoiding unaligned pcode accesses. It also stores
> > the pcode thermal data and thermal limit words as little-endian.
> > 
> > > diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> > > index 382bf9625a8bd..1b4efbf4ac999 100644
> > > --- a/drivers/gpu/drm/xe/xe_hwmon.c
> > > +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> > [ ... ]
> > > @@ -840,16 +845,20 @@ static int mbx_temp_decode(u8 value)
> > >   static int get_mc_temp(struct xe_hwmon *hwmon, long *val)
> > >   {
> > >   	struct xe_tile *root_tile = xe_device_get_root_tile(hwmon->xe);
> > > -	u32 *dword = (u32 *)hwmon->temp.value;
> > >   	s32 average = 0;
> > >   	int ret, i;
> > > -	for (i = 0; i < DIV_ROUND_UP(TEMP_LIMIT_MAX, sizeof(u32)); i++) {
> > > +	guard(mutex)(&hwmon->hwmon_lock);
> > [Severity: Low]
> > Does this violate the hardware monitoring subsystem locking requirements?
> > 
> > The hwmon subsystem documentation states that drivers implementing locking
> > required for interrupt handling or for attributes registered by any other
> > means must use the hwmon_lock() and hwmon_unlock() APIs.
> > 
> > Can we use the hwmon subsystem's locking API here instead of the
> > driver-specific hwmon_lock mutex?
> 
> The mutex here protects driver-internal concurrent accesses and does not
> provide synchronization required by the hwmon core callbacks.
> 
> Therefore, I don't believe this falls under the case requiring hwmon_lock()
> / hwmon_unlock().
> 
> @rodrigo, can you share your comment on this.

that's right. we need to protect our data, nor take care of any synchronization...

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> > > +
> > > +	for (i = 0; i < DIV_ROUND_UP(hwmon->temp.count, sizeof(u32)); i++) {