| Message ID | 20210814143637.11922-4-W_Armin@gmx.de (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | hwmon: (dell-smm) Misc cleanups | expand |
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 >
On 8/14/21 8:13 AM, Pali Rohár wrote: > 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." > Yes. In situations like this I normally I suggest to have one read-write attribute and have the others as read-only. However, that doesn't work here because the attribute is write-only. Having more than one really doesn't add value. Guenter >> --- >> 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 >>
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 };