Re: [PATCH v3 07/10] platform/x86: alienware-wmi-wmax: Add HWMON support

[Guenter Roeck]

On 3/7/25 13:09, Armin Wolf wrote:
> Am 07.03.25 um 01:35 schrieb Kurt Borja:
...
> > > > +static const struct hwmon_ops awcc_hwmon_ops = {
> > > > +    .is_visible = awcc_hwmon_is_visible,
> > > > +    .read = awcc_hwmon_read,
> > > > +    .read_string = awcc_hwmon_read_string,
> > > > +};
> > > > +
> > > > +static const struct hwmon_channel_info * const awcc_hwmon_info[] = {
> > > > +    HWMON_CHANNEL_INFO(temp,
> > > > +               HWMON_T_LABEL | HWMON_T_INPUT,
> > > > +               HWMON_T_LABEL | HWMON_T_INPUT,
> > > > +               HWMON_T_LABEL | HWMON_T_INPUT,
> > > > +               HWMON_T_LABEL | HWMON_T_INPUT,
> > > > +               HWMON_T_LABEL | HWMON_T_INPUT,
> > > > +               HWMON_T_LABEL | HWMON_T_INPUT
> > > > +               ),
> > > > +    HWMON_CHANNEL_INFO(fan,
> > > > +               HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
> > > > +               HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
> > > > +               HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
> > > > +               HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
> > > > +               HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
> > > > +               HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX
> > > > +               ),
> > > > +    HWMON_CHANNEL_INFO(pwm,
> > > > +               HWMON_PWM_AUTO_CHANNELS_TEMP,
> > > > +               HWMON_PWM_AUTO_CHANNELS_TEMP,
> > > > +               HWMON_PWM_AUTO_CHANNELS_TEMP,
> > > > +               HWMON_PWM_AUTO_CHANNELS_TEMP,
> > > > +               HWMON_PWM_AUTO_CHANNELS_TEMP,
> > > > +               HWMON_PWM_AUTO_CHANNELS_TEMP
> > > > +               ),
> > > Since the number of fans and temperature sensors is only known at runtime creating awcc_hwmon_info
> > > would make sense.
> > IIRC Guenter asked another dev to add more CHANNEL_INFO entries instead
> > of doing that? I might be wrong tho.
> >
> > I'm fine either way.
> If Guenter is fine with your current approach then you can keep it.

In drivers/hwmon, I prefer static descriptions such as the above and using
the is_visible() function to determine if sensor attributes should actually
be created. However, as I have mentioned several times, I do not comment on style
questions like this (or, for that matter, non-standard sysfs attributes) outside
drivers/hwmon, so you can do or request whatever you like.

Guenter