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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7c850374-8caa-430d-b114-1aaa944bbc89@roeck-us.net>
Date: Fri, 7 Mar 2025 15:59:37 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Armin Wolf <W_Armin@....de>, Kurt Borja <kuurtb@...il.com>,
 Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: Hans de Goede <hdegoede@...hat.com>, platform-driver-x86@...r.kernel.org,
 Dell.Client.Kernel@...l.com, linux-kernel@...r.kernel.org,
 Jean Delvare <jdelvare@...e.com>, linux-hwmon@...r.kernel.org
Subject: Re: [PATCH v3 07/10] platform/x86: alienware-wmi-wmax: Add HWMON
 support

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