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

["Kurt Borja" <kuurtb@gmail.com>]

On Fri Mar 7, 2025 at 6:59 PM -05, Guenter Roeck wrote:
> 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

Thank you for clarifying!

If there are no objections, then I prefer to keep it this way. With the
few extra HWMON_CHANNEL_INFO entries.

-- 
 ~ Kurt