Hi, On Sat, Jul 01, 2023 at 08:02:51PM -0700, Guenter Roeck wrote: > On 7/1/23 18:31, Dixit, Ashutosh wrote: > > 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. > > > > You mean your preference is to use a deprecated hardware monitoring > registration function and to explicitly violate the following statement > from Documentation/hwmon/hwmon-kernel-api.rst ? > > All other hardware monitoring device registration functions are deprecated > and must not be used in new drivers. > > That is quite interesting. Please elaborate and explain your rationale. how about using iio instead of hwmon? Andi
