Re: [PATCH v9 10/14] drm/xe/hwmon: Add platform-aware VRAM thermal channel support

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

On 09-10-2026 17:48, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] hwmon: DRM driver implements hwmon directly instead of as auxiliary device
> --
>
> commit 55ff84e7912b9e195885552d6a750a68cc2c19d9
> Author: Karthik Poosa <karthik.poosa@intel.com>
>
> drm/xe/hwmon: Add platform-aware VRAM thermal channel support
>
> On CRI, this derives the number of available VRAM temperature channels from
> the CRI_MSU_VRAM_ENABLE register. It increases the maximum supported VRAM
> channel count to 80 and uses the derived count to control VRAM sensor
> enumeration, channel access, and VRAM label allocation.
>
>> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
>> index bafa175f84578..9d83d9d2286ed 100644
>> --- a/drivers/gpu/drm/xe/xe_hwmon.c
>> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> [ ... ]
>> @@ -1683,7 +1809,9 @@ int xe_hwmon_register(struct xe_device *xe)
>>   	hwmon->xe = xe;
>>   	xe->hwmon = hwmon;
>>   
>> -	xe_hwmon_get_preregistration_info(hwmon);
>> +	ret = xe_hwmon_get_preregistration_info(hwmon);
>> +	if (ret)
>> +		goto err_null_hwmon;
>>   
>>   	drm_dbg(&xe->drm, "Register xe hwmon interface\n");
>>   
>> @@ -1693,10 +1821,14 @@ int xe_hwmon_register(struct xe_device *xe)
>>   								hwmon_groups);
> [Severity: Low]
> The hwmon subsystem guidelines state that if a chip's main functionality
> is not hardware monitoring (such as a DRM controller), its hardware
> monitoring functionality should be implemented as an auxiliary device
> driver residing in drivers/hwmon/.
>
> Does registering the hardware monitoring device directly from within the
> drivers/gpu/drm/ subsystem bypass these structural layering rules?
The hwmon documentation expresses a preference for separating monitoring 
functionality from devices whose primary purpose is not hardware 
monitoring. However, direct hwmon registration from DRM drivers is 
already common practice in the kernel (e.g. amdgpu, nouveau, xe), so the 
proposed approach would be consistent with existing GPU driver 
implementations."

>
>>   	if (IS_ERR(hwmon->hwmon_dev)) {
>>   		drm_err(&xe->drm, "Failed to register xe hwmon (%pe)\n", hwmon->hwmon_dev);
>> -		xe->hwmon = NULL;
>> -		return PTR_ERR(hwmon->hwmon_dev);
>> +		ret = PTR_ERR(hwmon->hwmon_dev);
>> +		goto err_null_hwmon;
>>   	}
>>   
>>   	return 0;
>> +
>> +err_null_hwmon:
>> +	xe->hwmon = NULL;
>> +	return ret;
>>   }
>>   MODULE_IMPORT_NS("INTEL_PMT_TELEMETRY");