On Tue, 27 Jun 2023 11:30:37 -0700, Badal Nilawar wrote: > Hi Badal, > This series adds the hwmon support on xe driver for DGFX Needs some discussion but I have a general comment on this series first. The implementation here follow what was done for i915. But how "hwmon attributes are defined" I think we should look at how this was done in other drm drivers, namely amdgpu and radeon. Look here (search for "hwmon_attributes"): drivers/gpu/drm/amd/pm/amdgpu_pm.c, and drivers/gpu/drm/radeon/radeon_pm.c Here the hwmon attribute definition is very similar to how general sysfs attributes are defined (they will just appear in hwmon directories) and does not carry baggage of the hwmon infrastructure (what i915 has). So my preference is to shift to this amd/radeon way for xe. There is also a separate discussion on whether to use hwmon sysfs for other custom attributes, as has been done in these other drm drivers, and using this light-weight method should help if we went this route too. Thanks. -- Ashutosh
