[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <c0084820-6e4b-8a76-6220-5262774a8f69@roeck-us.net>
Date: Tue, 26 Oct 2021 12:29:20 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Armin Wolf <W_Armin@....de>, pali@...nel.org
Cc: jdelvare@...e.com, linux-hwmon@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [RFC] hwmon: (dell-smm) Allow for specifying fan control commands
as module params
On 10/26/21 10:18 AM, Armin Wolf wrote:
> Right now, the only way to test if setting manual/auto fan control works
> is to edit and recompile the module, which might not be possible for
> everyone. Allow for specifying the necessary SMM commands when loading
> the module, but taint the kernel if so since those commands might
> cause strange side effects. Also update kernel-parameters.txt
> accordingly.
>
> Tested on a Dell Inspiron 3505.
>
> Signed-off-by: Armin Wolf <W_Armin@....de>
As far as I can see there are three logical changes in this patch:
- move parameters from i8k to dell_smm_hwmon
- add documentation for undocumented parameters
- add new parameters
"One patch per logical change", please, as documented.
Thanks,
Guenter
> ---
> .../admin-guide/kernel-parameters.txt | 45 ++++++++++++++-----
> Documentation/hwmon/dell-smm-hwmon.rst | 27 ++++++++---
> drivers/hwmon/dell-smm-hwmon.c | 22 +++++++--
> 3 files changed, 73 insertions(+), 21 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 91ba391f9b32..7025f5bf625f 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -940,6 +940,40 @@
> dump out devices still on the deferred probe list after
> retrying.
>
> + dell_smm_hwmon.ignore_dmi=
> + [HW] Continue probing hardware even if DMI data
> + indicates that the driver is running on unsupported
> + hardware.
> +
> + dell_smm_hwmon.force=
> + [HW] Activate driver even if SMM BIOS signature does
> + not match list of supported models or certain
> + features are disabled.
> +
> + dell_smm_hwmon.power_status=
> + [HW] Report power status in /proc/i8k
> + (disabled by default).
> +
> + dell_smm_hwmon.restricted=
> + [HW] Allow controlling fans only if SYS_ADMIN
> + capability is set.
> +
> + dell_smm_hwmon.fan_mult=
> + [HW] Factor to multiply fan speed with.
> +
> + dell_smm_hwmon.fan_max=
> + [HW] Maximum configurable fan speed.
> +
> + dell_smm_hwmon.manual_fan=
> + [HW] SMM command to disable BIOS fan control, must be
> + set together with dell_smm_hwmon.auto_fan to work.
> + This can be dangerous, yet let us know if a certain
> + command combination worked.
> +
> + dell_smm_hwmon.auto_fan=
> + [HW] SMM command to enable BIOS fan control, see
> + dell_smm_hwmon.manual_fan for further explaination.
> +
> dfltcc= [HW,S390]
> Format: { on | off | def_only | inf_only | always }
> on: s390 zlib hardware support for compression on
> @@ -1693,17 +1727,6 @@
>
> i810= [HW,DRM]
>
> - i8k.ignore_dmi [HW] Continue probing hardware even if DMI data
> - indicates that the driver is running on unsupported
> - hardware.
> - i8k.force [HW] Activate i8k driver even if SMM BIOS signature
> - does not match list of supported models.
> - i8k.power_status
> - [HW] Report power status in /proc/i8k
> - (disabled by default)
> - i8k.restricted [HW] Allow controlling fans only if SYS_ADMIN
> - capability is set.
> -
> i915.invert_brightness=
> [DRM] Invert the sense of the variable that is used to
> set the brightness of the panel backlight. Normally a
> diff --git a/Documentation/hwmon/dell-smm-hwmon.rst b/Documentation/hwmon/dell-smm-hwmon.rst
> index beec88491171..df12dce7b02e 100644
> --- a/Documentation/hwmon/dell-smm-hwmon.rst
> +++ b/Documentation/hwmon/dell-smm-hwmon.rst
> @@ -67,13 +67,13 @@ 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
> 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``
> -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
> +Check the list ``i8k_fan_control_data`` in file
> +``drivers/hwmon/dell-smm-hwmon.c`` inside the kernel tree:
> +as a first attempt you can load the module with ``manual_fan`` and
> +``auto_fan`` set to an already-known code pair. If you see that
> +``pwm1_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
> :ref:`Documentation/process/submitting-patches.rst <submittingpatches>`
> for information on submitting patches.
>
> @@ -120,6 +120,19 @@ Module parameters
> Maximum configurable fan speed. (default:
> autodetect)
>
> +* manual_fan:uint
> + SMM code to disable BIOS fan control, must
> + be set together with auto_fan to work.
> + Please note that this parameter is unsafe and
> + will taint your kernel when set, use only for
> + testing codes on your hardware!
> + (default: retrieved from whitelist)
> +
> +* auto_fan:uint
> + SMM command to enable BIOS fan control, see
> + manual_fan for further explaination.
> + (default: retrieved from whitelist)
> +
> Legacy ``/proc`` interface
> --------------------------
>
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index eaace478f508..4257aa297d7a 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -111,6 +111,14 @@ static uint fan_max;
> module_param(fan_max, uint, 0);
> MODULE_PARM_DESC(fan_max, "Maximum configurable fan speed (default: autodetect)");
>
> +static uint manual_fan;
> +module_param_unsafe(manual_fan, uint, 0);
> +MODULE_PARM_DESC(manual_fan, "SMM command to disable BIOS fan control (default: disabled)");
> +
> +static uint auto_fan;
> +module_param_unsafe(auto_fan, uint, 0);
> +MODULE_PARM_DESC(auto_fan, "SMM command to enable BIOS fan control (default: disabled)");
> +
> struct smm_regs {
> unsigned int eax;
> unsigned int ebx __packed;
> @@ -700,7 +708,7 @@ static umode_t dell_smm_is_visible(const void *drvdata, enum hwmon_sensor_types
>
> break;
> case hwmon_pwm_enable:
> - if (data->auto_fan)
> + if (data->auto_fan && data->manual_fan)
> /*
> * There is no command for retrieve the current status
> * from BIOS, and userspace/firmware itself can change
> @@ -1305,12 +1313,20 @@ static int __init dell_smm_probe(struct platform_device *pdev)
> data->i8k_fan_max = fan_max ? : I8K_FAN_HIGH; /* Must not be 0 */
> data->i8k_pwm_mult = DIV_ROUND_UP(255, data->i8k_fan_max);
>
> + /* Values specified in module parameters override values from dmi */
> fan_control = dmi_first_match(i8k_whitelist_fan_control);
> if (fan_control && fan_control->driver_data) {
> const struct i8k_fan_control_data *control = fan_control->driver_data;
>
> - data->manual_fan = control->manual_fan;
> - data->auto_fan = control->auto_fan;
> + if (!manual_fan)
> + manual_fan = control->manual_fan;
> +
> + if (!auto_fan)
> + auto_fan = control->auto_fan;
> + }
> + if (manual_fan && auto_fan) {
> + data->manual_fan = manual_fan;
> + data->auto_fan = auto_fan;
> dev_info(&pdev->dev, "enabling support for setting automatic/manual fan control\n");
> }
>
> --
> 2.30.2
>
Powered by blists - more mailing lists