Re: [PATCH v2] drm/xe/hwmon: expose fan speed

[Raag Jadav <raag.jadav@intel.com>]

On Mon, Mar 10, 2025 at 01:01:11PM -0400, Rodrigo Vivi wrote:
> On Sat, Mar 08, 2025 at 06:12:16PM +0200, Raag Jadav wrote:
> > On Fri, Mar 07, 2025 at 09:33:05AM -0500, Rodrigo Vivi wrote:
> > > On Fri, Mar 07, 2025 at 06:21:12PM +0530, Raag Jadav wrote:
> > 
> > ...
> > 
> > > > @@ -454,6 +485,7 @@ static const struct hwmon_channel_info * const hwmon_info[] = {
> > > >  	HWMON_CHANNEL_INFO(curr, HWMON_C_LABEL, HWMON_C_CRIT | HWMON_C_LABEL),
> > > >  	HWMON_CHANNEL_INFO(in, HWMON_I_INPUT | HWMON_I_LABEL, HWMON_I_INPUT | HWMON_I_LABEL),
> > > >  	HWMON_CHANNEL_INFO(energy, HWMON_E_INPUT | HWMON_E_LABEL, HWMON_E_INPUT | HWMON_E_LABEL),
> > > > +	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT, HWMON_F_INPUT, HWMON_F_INPUT),
> > > 
> > > here as well... I believe we should not expose the fan files if !has_fan_control
> > 
> > I'm not sure if I'm following here. We'll still to define channel attributes
> > to be able to let hwmon expose them at runtime based on has_fan_control flag.
> > 
> > One way I could think of is to do a manual sysfs_create_file(), but at that
> > point it defeats the purpose of having hwmon.
> 
> Well, there might be a way. I don't believe we are the only hwmon users with this
> case.
> 
> Ashutosh, Karthik, thoughts on this?
> 
> Have you guys seen around hwmon a way to avoid creating an entry based on the
> platform? Like, in this case, avoid fan interfaces to appear at all on a platform
> that doesn't support fan control?

Isn't that the whole point of ->is_visible(), and isn't that what we're already
doing here?

Raag