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 for Android: free password hash cracker in your pocket

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

Message-Id: <D8AM7U5UNBX5.XYJUL18HC2M5@gmail.com>
Date: Fri, 07 Mar 2025 23:54:55 -0500
From: "Kurt Borja" <kuurtb@...il.com>
To: "Guenter Roeck" <linux@...ck-us.net>, "Armin Wolf" <W_Armin@....de>,
 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 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