Re: [PATCH 3/4] hwmon: (dell-smm) Enable automatic fan speed control for all channels

["Pali Rohár" <pali@kernel.org>]

On Saturday 14 August 2021 16:36:36 W_Armin@gmx.de wrote:
> From: Armin Wolf <W_Armin@gmx.de>
> 
> Add automatic fan speed control for the remaining two pwm channels
> since the pwmX_enable setting affects all pwm channels.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>

This behavior was already rejected by Guenter:
https://lore.kernel.org/linux-hwmon/3a10f96a-06e1-39f4-74a6-908d25b1f496@roeck-us.net/

"Having three attributes do all the same is not very valuable.
I would suggest to stick with pwm1_enable and document that it applies
to all pwm channels."

> ---
>  Documentation/hwmon/dell-smm-hwmon.rst | 14 +++++++-------
>  drivers/hwmon/dell-smm-hwmon.c         |  4 ++--
>  2 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/hwmon/dell-smm-hwmon.rst b/Documentation/hwmon/dell-smm-hwmon.rst
> index 3bf77a5df995..57b30fc9d03a 100644
> --- a/Documentation/hwmon/dell-smm-hwmon.rst
> +++ b/Documentation/hwmon/dell-smm-hwmon.rst
> @@ -35,7 +35,7 @@ Name				Perm	Description
>  fan[1-3]_input                  RO      Fan speed in RPM.
>  fan[1-3]_label                  RO      Fan label.
>  pwm[1-3]                        RW      Control the fan PWM duty-cycle.
> -pwm1_enable                     WO      Enable or disable automatic BIOS fan
> +pwm[1-3]_enable                 WO      Enable or disable automatic BIOS fan
>                                          control (not supported on all laptops,
>                                          see below for details).
>  temp[1-10]_input                RO      Temperature reading in milli-degrees
> @@ -52,13 +52,13 @@ overwritten.
> 
>  There is experimental support for disabling automatic BIOS fan
>  control, at least on laptops where the corresponding SMM command is
> -known, by writing the value ``1`` in the attribute ``pwm1_enable``
> -(writing ``2`` enables automatic BIOS control again). Even if you have
> +known, by writing the value ``1`` in the attribute ``pwm[1-3]_enable``
> +(writing ``2`` enables automatic BIOS control again). If you have
>  more than one fan, all of them are set to either enabled or disabled
> -automatic fan control at the same time and, notwithstanding the name,
> -``pwm1_enable`` sets automatic control for all fans.
> +automatic fan control at the same time so ``pwm[1-3]_enable``
> +sets automatic fan control for **all** fans.
> 
> -If ``pwm1_enable`` is not available, then it means that SMM codes for
> +If ``pwm[1-3]_enable`` is not available, then it means that SMM codes for
>  enabling and disabling automatic BIOS fan control are not whitelisted
>  for your hardware. It is possible that codes that work for other
>  laptops actually work for yours as well, or that you have to discover
> @@ -67,7 +67,7 @@ new codes.
>  Check the list ``i8k_whitelist_fan_control`` in file
>  ``drivers/hwmon/dell-smm-hwmon.c`` in the kernel tree: as a first
>  attempt you can try to add your machine and use an already-known code
> -pair. If, after recompiling the kernel, you see that ``pwm1_enable``
> +pair. If, after recompiling the kernel, you see that ``pwm[1-3]_enable``
>  is present and works (i.e., you can manually control the fan speed),
>  then please submit your finding as a kernel patch, so that other users
>  can benefit from it. Please see
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index 3aa09c1e4b1d..0e229e3dae33 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -898,8 +898,8 @@ static const struct hwmon_channel_info *dell_smm_info[] = {
>  			   ),
>  	HWMON_CHANNEL_INFO(pwm,
>  			   HWMON_PWM_INPUT | HWMON_PWM_ENABLE,
> -			   HWMON_PWM_INPUT,
> -			   HWMON_PWM_INPUT
> +			   HWMON_PWM_INPUT | HWMON_PWM_ENABLE,
> +			   HWMON_PWM_INPUT | HWMON_PWM_ENABLE
>  			   ),
>  	NULL
>  };
> --
> 2.20.1
>