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

["Poosa, Karthik" <karthik.poosa@intel.com>]

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.

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