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

[sashiko-bot@kernel.org]

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?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014115806.831459-1-karthik.poosa@intel.com?part=3