diff mbox series

[RFC] hwmon: (dell-smm) Allow for specifying fan control commands as module params

Message ID 20211026171821.25944-1-W_Armin@gmx.de (mailing list archive)
State Changes Requested
Headers show
Series [RFC] hwmon: (dell-smm) Allow for specifying fan control commands as module params | expand

Commit Message

Armin Wolf Oct. 26, 2021, 5:18 p.m. UTC 
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@gmx.de>
---
 .../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(-)

--
2.30.2

Comments

Guenter Roeck Oct. 26, 2021, 7:29 p.m. UTC | #1 
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@gmx.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
>
diff mbox series

Patch

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");
 	}