[PATCH 1/2] x86/Kconfig: move and modify CONFIG_I8K

[PATCH 1/2] x86/Kconfig: move and modify CONFIG_I8K

[Mateusz Jończyk]

In Kconfig, inside the "Processor type and features" menu, there is
the CONFIG_I8K option: "Dell i8k legacy laptop support". This is
very confusing - enabling CONFIG_I8K is not required for the kernel to
support old Dell laptops. This option is specific to the dell-smm-hwmon
driver, which mostly exports some hardware monitoring information and
allows the user to change fan speed.

This option is misplaced, so move CONFIG_I8K to drivers/hwmon/Kconfig,
where it belongs.

Also, modify the dependency order - change
        select SENSORS_DELL_SMM
to
        depends on SENSORS_DELL_SMM
as it is just a configuration option of dell-smm-hwmon. This includes
changing the option type from tristate to bool. It was tristate because
it could select CONFIG_SENSORS_DELL_SMM=m .

When running "make oldconfig" on configurations with
CONFIG_SENSORS_DELL_SMM enabled , this change will result in an
additional question (which could be printed several times during
bisecting). I think that tidying up the configuration is worth it,
though.

Next patch tweaks the description of CONFIG_I8K.

Signed-off-by: Mateusz Jończyk <mat.jonczyk@o2.pl>
Cc: Pali Rohár <pali@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Jean Delvare <jdelvare@suse.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Mark Gross <markgross@kernel.org>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
Acked-by: Borislav Petkov <bp@suse.de>

---

v2:
  - do not drop "depends on PROC_FS" from CONFIG_I8K

Thanks to all for reviewing.
Mr Randy Dunlap: The first patch just moves CONFIG_I8K, without modifying
the help text.

 arch/x86/Kconfig      | 17 -----------------
 drivers/hwmon/Kconfig | 16 ++++++++++++++++
 2 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 9f5bd41bf660..71d4ddd48c02 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1275,23 +1275,6 @@ config TOSHIBA
 	  Say Y if you intend to run this kernel on a Toshiba portable.
 	  Say N otherwise.
 
-config I8K
-	tristate "Dell i8k legacy laptop support"
-	depends on HWMON
-	depends on PROC_FS
-	select SENSORS_DELL_SMM
-	help
-	  This option enables legacy /proc/i8k userspace interface in hwmon
-	  dell-smm-hwmon driver. Character file /proc/i8k reports bios version,
-	  temperature and allows controlling fan speeds of Dell laptops via
-	  System Management Mode. For old Dell laptops (like Dell Inspiron 8000)
-	  it reports also power and hotkey status. For fan speed control is
-	  needed userspace package i8kutils.
-
-	  Say Y if you intend to run this kernel on old Dell laptops or want to
-	  use userspace package i8kutils.
-	  Say N otherwise.
-
 config X86_REBOOTFIXUPS
 	bool "Enable X86 board specific fixups for reboot"
 	depends on X86_32
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 8df25f1079ba..1ee4e5eff567 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -505,6 +505,22 @@ config SENSORS_DELL_SMM
 	  When option I8K is also enabled this driver provides legacy /proc/i8k
 	  userspace interface for i8kutils package.
 
+config I8K
+	bool "Dell i8k legacy laptop support"
+	depends on SENSORS_DELL_SMM
+	depends on PROC_FS
+	help
+	  This option enables legacy /proc/i8k userspace interface in hwmon
+	  dell-smm-hwmon driver. Character file /proc/i8k reports bios version,
+	  temperature and allows controlling fan speeds of Dell laptops via
+	  System Management Mode. For old Dell laptops (like Dell Inspiron 8000)
+	  it reports also power and hotkey status. For fan speed control is
+	  needed userspace package i8kutils.
+
+	  Say Y if you intend to run this kernel on old Dell laptops or want to
+	  use userspace package i8kutils.
+	  Say N otherwise.
+
 config SENSORS_DA9052_ADC
 	tristate "Dialog DA9052/DA9053 ADC"
 	depends on PMIC_DA9052

base-commit: dfd42facf1e4ada021b939b4e19c935dcdd55566
-- 
2.25.1

[PATCH 2/2] dell-smm-hwmon: rewrite CONFIG_I8K description

[Mateusz Jończyk]

It is not the laptops, but the /proc/i8k interface that is legacy (or so
I think was the intention of the help text author). The old description
was confusing, fix this.

The phrase "Say Y if you intend to run this kernel on old Dell laptops
or want to use userspace package i8kutils." was introduced in 2015, in
commit 039ae58503f3 ("hwmon: Allow to compile dell-smm-hwmon driver without /proc/i8k")

I think that "old laptops" was about hotkey and Fn key support - this
driver in the 2.4 kernels' era apparently had these capabilities
(see: https://github.com/vitorafsr/i8kutils , description of
"repeat_rate" kernel module parameter).

Signed-off-by: Mateusz Jończyk <mat.jonczyk@o2.pl>
Cc: Pali Rohár <pali@kernel.org>
Cc: Jean Delvare <jdelvare@suse.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Mark Gross <markgross@kernel.org>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Randy Dunlap <rdunlap@infradead.org>

---
v2:
  - help text language fixes (thanks: Randy Dunlap),
  - modify commit description.
---
 drivers/hwmon/Kconfig | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 1ee4e5eff567..39aeecc72800 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -506,19 +506,18 @@ config SENSORS_DELL_SMM
 	  userspace interface for i8kutils package.
 
 config I8K
-	bool "Dell i8k legacy laptop support"
+	bool "Legacy /proc/i8k interface of Dell laptop SMM BIOS hwmon driver"
 	depends on SENSORS_DELL_SMM
 	depends on PROC_FS
 	help
-	  This option enables legacy /proc/i8k userspace interface in hwmon
-	  dell-smm-hwmon driver. Character file /proc/i8k reports bios version,
-	  temperature and allows controlling fan speeds of Dell laptops via
-	  System Management Mode. For old Dell laptops (like Dell Inspiron 8000)
-	  it reports also power and hotkey status. For fan speed control is
-	  needed userspace package i8kutils.
+	  This option enables the legacy /proc/i8k userspace interface of the
+	  dell-smm-hwmon driver. The character file /proc/i8k exposes the BIOS
+	  version, temperatures and allows control of fan speeds of some Dell
+	  laptops. Sometimes it also reports power and hotkey status.
 
-	  Say Y if you intend to run this kernel on old Dell laptops or want to
-	  use userspace package i8kutils.
+	  This interface is required to run programs from the i8kutils package.
+
+	  Say Y if you intend to run userspace programs that use this interface.
 	  Say N otherwise.
 
 config SENSORS_DA9052_ADC
-- 
2.25.1

Re: [PATCH 2/2] dell-smm-hwmon: rewrite CONFIG_I8K description

[Guenter Roeck]

On Sat, Feb 12, 2022 at 01:56:54PM +0100, Mateusz Jończyk wrote:
> It is not the laptops, but the /proc/i8k interface that is legacy (or so
> I think was the intention of the help text author). The old description
> was confusing, fix this.
> 
> The phrase "Say Y if you intend to run this kernel on old Dell laptops
> or want to use userspace package i8kutils." was introduced in 2015, in
> commit 039ae58503f3 ("hwmon: Allow to compile dell-smm-hwmon driver without /proc/i8k")
> 
> I think that "old laptops" was about hotkey and Fn key support - this
> driver in the 2.4 kernels' era apparently had these capabilities
> (see: https://github.com/vitorafsr/i8kutils , description of
> "repeat_rate" kernel module parameter).
> 
> Signed-off-by: Mateusz Jończyk <mat.jonczyk@o2.pl>
> Cc: Pali Rohár <pali@kernel.org>
> Cc: Jean Delvare <jdelvare@suse.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Mark Gross <markgross@kernel.org>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Randy Dunlap <rdunlap@infradead.org>

Applied to hwmon-next.

Thanks,
Guenter

> ---
> v2:
>   - help text language fixes (thanks: Randy Dunlap),
>   - modify commit description.
> ---
>  drivers/hwmon/Kconfig | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 1ee4e5eff567..39aeecc72800 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -506,19 +506,18 @@ config SENSORS_DELL_SMM
>  	  userspace interface for i8kutils package.
>  
>  config I8K
> -	bool "Dell i8k legacy laptop support"
> +	bool "Legacy /proc/i8k interface of Dell laptop SMM BIOS hwmon driver"
>  	depends on SENSORS_DELL_SMM
>  	depends on PROC_FS
>  	help
> -	  This option enables legacy /proc/i8k userspace interface in hwmon
> -	  dell-smm-hwmon driver. Character file /proc/i8k reports bios version,
> -	  temperature and allows controlling fan speeds of Dell laptops via
> -	  System Management Mode. For old Dell laptops (like Dell Inspiron 8000)
> -	  it reports also power and hotkey status. For fan speed control is
> -	  needed userspace package i8kutils.
> +	  This option enables the legacy /proc/i8k userspace interface of the
> +	  dell-smm-hwmon driver. The character file /proc/i8k exposes the BIOS
> +	  version, temperatures and allows control of fan speeds of some Dell
> +	  laptops. Sometimes it also reports power and hotkey status.
>  
> -	  Say Y if you intend to run this kernel on old Dell laptops or want to
> -	  use userspace package i8kutils.
> +	  This interface is required to run programs from the i8kutils package.
> +
> +	  Say Y if you intend to run userspace programs that use this interface.
>  	  Say N otherwise.
>  
>  config SENSORS_DA9052_ADC

Re: [PATCH 1/2] x86/Kconfig: move and modify CONFIG_I8K

[Guenter Roeck]

On Sat, Feb 12, 2022 at 01:56:53PM +0100, Mateusz Jończyk wrote:
> In Kconfig, inside the "Processor type and features" menu, there is
> the CONFIG_I8K option: "Dell i8k legacy laptop support". This is
> very confusing - enabling CONFIG_I8K is not required for the kernel to
> support old Dell laptops. This option is specific to the dell-smm-hwmon
> driver, which mostly exports some hardware monitoring information and
> allows the user to change fan speed.
> 
> This option is misplaced, so move CONFIG_I8K to drivers/hwmon/Kconfig,
> where it belongs.
> 
> Also, modify the dependency order - change
>         select SENSORS_DELL_SMM
> to
>         depends on SENSORS_DELL_SMM
> as it is just a configuration option of dell-smm-hwmon. This includes
> changing the option type from tristate to bool. It was tristate because
> it could select CONFIG_SENSORS_DELL_SMM=m .
> 
> When running "make oldconfig" on configurations with
> CONFIG_SENSORS_DELL_SMM enabled , this change will result in an
> additional question (which could be printed several times during
> bisecting). I think that tidying up the configuration is worth it,
> though.
> 
> Next patch tweaks the description of CONFIG_I8K.
> 
> Signed-off-by: Mateusz Jończyk <mat.jonczyk@o2.pl>
> Cc: Pali Rohár <pali@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Jean Delvare <jdelvare@suse.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Mark Gross <markgross@kernel.org>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
> Acked-by: Borislav Petkov <bp@suse.de>

Applied  to hwmon-next.

Thanks,
Guenter

> ---
> 
> v2:
>   - do not drop "depends on PROC_FS" from CONFIG_I8K
> 
> Thanks to all for reviewing.
> Mr Randy Dunlap: The first patch just moves CONFIG_I8K, without modifying
> the help text.
> 
>  arch/x86/Kconfig      | 17 -----------------
>  drivers/hwmon/Kconfig | 16 ++++++++++++++++
>  2 files changed, 16 insertions(+), 17 deletions(-)
> 
> 
> base-commit: dfd42facf1e4ada021b939b4e19c935dcdd55566
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 9f5bd41bf660..71d4ddd48c02 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1275,23 +1275,6 @@ config TOSHIBA
>  	  Say Y if you intend to run this kernel on a Toshiba portable.
>  	  Say N otherwise.
>  
> -config I8K
> -	tristate "Dell i8k legacy laptop support"
> -	depends on HWMON
> -	depends on PROC_FS
> -	select SENSORS_DELL_SMM
> -	help
> -	  This option enables legacy /proc/i8k userspace interface in hwmon
> -	  dell-smm-hwmon driver. Character file /proc/i8k reports bios version,
> -	  temperature and allows controlling fan speeds of Dell laptops via
> -	  System Management Mode. For old Dell laptops (like Dell Inspiron 8000)
> -	  it reports also power and hotkey status. For fan speed control is
> -	  needed userspace package i8kutils.
> -
> -	  Say Y if you intend to run this kernel on old Dell laptops or want to
> -	  use userspace package i8kutils.
> -	  Say N otherwise.
> -
>  config X86_REBOOTFIXUPS
>  	bool "Enable X86 board specific fixups for reboot"
>  	depends on X86_32
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 8df25f1079ba..1ee4e5eff567 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -505,6 +505,22 @@ config SENSORS_DELL_SMM
>  	  When option I8K is also enabled this driver provides legacy /proc/i8k
>  	  userspace interface for i8kutils package.
>  
> +config I8K
> +	bool "Dell i8k legacy laptop support"
> +	depends on SENSORS_DELL_SMM
> +	depends on PROC_FS
> +	help
> +	  This option enables legacy /proc/i8k userspace interface in hwmon
> +	  dell-smm-hwmon driver. Character file /proc/i8k reports bios version,
> +	  temperature and allows controlling fan speeds of Dell laptops via
> +	  System Management Mode. For old Dell laptops (like Dell Inspiron 8000)
> +	  it reports also power and hotkey status. For fan speed control is
> +	  needed userspace package i8kutils.
> +
> +	  Say Y if you intend to run this kernel on old Dell laptops or want to
> +	  use userspace package i8kutils.
> +	  Say N otherwise.
> +
>  config SENSORS_DA9052_ADC
>  	tristate "Dialog DA9052/DA9053 ADC"
>  	depends on PMIC_DA9052

[PATCH 1/2] x86/Kconfig: move and modify CONFIG_I8K

[Mateusz Jończyk]

In Kconfig, inside the "Processor type and features" menu, there is
the CONFIG_I8K option: "Dell i8k legacy laptop support". This is
very confusing - enabling CONFIG_I8K is not required for the kernel to
support old Dell laptops. This option is specific to the dell-smm-hwmon
driver, which mostly exports some hardware monitoring information and
allows the user to change fan speed.

This option is misplaced, so move CONFIG_I8K to drivers/hwmon/Kconfig,
where it belongs.

Also, modify the dependency order - change
        select SENSORS_DELL_SMM
to
        depends on SENSORS_DELL_SMM
as it is just a configuration option of dell-smm-hwmon. This includes
changing the option type from tristate to bool. It was tristate because
it could select CONFIG_SENSORS_DELL_SMM=m .

When running "make oldconfig" on configurations with
CONFIG_SENSORS_DELL_SMM enabled , this change will result in an
additional question (which could be printed several times during
bisecting). I think that tidying up the configuration is worth it,
though.

Next patch tweaks the description of CONFIG_I8K.

Signed-off-by: Mateusz Jończyk <mat.jonczyk@o2.pl>
Cc: Pali Rohár <pali@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Jean Delvare <jdelvare@suse.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Mark Gross <markgross@kernel.org>
---
 arch/x86/Kconfig      | 17 -----------------
 drivers/hwmon/Kconfig | 15 +++++++++++++++
 2 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 9f5bd41bf660..71d4ddd48c02 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1275,23 +1275,6 @@ config TOSHIBA
 	  Say Y if you intend to run this kernel on a Toshiba portable.
 	  Say N otherwise.
 
-config I8K
-	tristate "Dell i8k legacy laptop support"
-	depends on HWMON
-	depends on PROC_FS
-	select SENSORS_DELL_SMM
-	help
-	  This option enables legacy /proc/i8k userspace interface in hwmon
-	  dell-smm-hwmon driver. Character file /proc/i8k reports bios version,
-	  temperature and allows controlling fan speeds of Dell laptops via
-	  System Management Mode. For old Dell laptops (like Dell Inspiron 8000)
-	  it reports also power and hotkey status. For fan speed control is
-	  needed userspace package i8kutils.
-
-	  Say Y if you intend to run this kernel on old Dell laptops or want to
-	  use userspace package i8kutils.
-	  Say N otherwise.
-
 config X86_REBOOTFIXUPS
 	bool "Enable X86 board specific fixups for reboot"
 	depends on X86_32
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 8df25f1079ba..dd244aa747ad 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -505,6 +505,21 @@ config SENSORS_DELL_SMM
 	  When option I8K is also enabled this driver provides legacy /proc/i8k
 	  userspace interface for i8kutils package.
 
+config I8K
+	bool "Dell i8k legacy laptop support"
+	depends on SENSORS_DELL_SMM
+	help
+	  This option enables legacy /proc/i8k userspace interface in hwmon
+	  dell-smm-hwmon driver. Character file /proc/i8k reports bios version,
+	  temperature and allows controlling fan speeds of Dell laptops via
+	  System Management Mode. For old Dell laptops (like Dell Inspiron 8000)
+	  it reports also power and hotkey status. For fan speed control is
+	  needed userspace package i8kutils.
+
+	  Say Y if you intend to run this kernel on old Dell laptops or want to
+	  use userspace package i8kutils.
+	  Say N otherwise.
+
 config SENSORS_DA9052_ADC
 	tristate "Dialog DA9052/DA9053 ADC"
 	depends on PMIC_DA9052
-- 
2.25.1

Re: [PATCH 1/2] x86/Kconfig: move and modify CONFIG_I8K

[Hans de Goede]

Hi,

On 2/7/22 19:29, Mateusz Jończyk wrote:
> In Kconfig, inside the "Processor type and features" menu, there is
> the CONFIG_I8K option: "Dell i8k legacy laptop support". This is
> very confusing - enabling CONFIG_I8K is not required for the kernel to
> support old Dell laptops. This option is specific to the dell-smm-hwmon
> driver, which mostly exports some hardware monitoring information and
> allows the user to change fan speed.
> 
> This option is misplaced, so move CONFIG_I8K to drivers/hwmon/Kconfig,
> where it belongs.
> 
> Also, modify the dependency order - change
>         select SENSORS_DELL_SMM
> to
>         depends on SENSORS_DELL_SMM
> as it is just a configuration option of dell-smm-hwmon. This includes
> changing the option type from tristate to bool. It was tristate because
> it could select CONFIG_SENSORS_DELL_SMM=m .
> 
> When running "make oldconfig" on configurations with
> CONFIG_SENSORS_DELL_SMM enabled , this change will result in an
> additional question (which could be printed several times during
> bisecting). I think that tidying up the configuration is worth it,
> though.
> 
> Next patch tweaks the description of CONFIG_I8K.
> 
> Signed-off-by: Mateusz Jończyk <mat.jonczyk@o2.pl>
> Cc: Pali Rohár <pali@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Jean Delvare <jdelvare@suse.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Mark Gross <markgross@kernel.org>

For other reviewers, the only consumer of the CONFIG_I8K
option is drivers/hwmon/dell-smm-hwmon.c
which has a couple of:
"#if IS_ENABLED(CONFIG_I8K)" checks to enable its old
legacy /proc/i8k interface.

So this move definitely makes sense.

I wonder if it would not be better to just completely drop
the old  /proc/i8k interface though ?

With that said, this series looks good to me, so:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

for the series.

Regards,

Hans




> ---
>  arch/x86/Kconfig      | 17 -----------------
>  drivers/hwmon/Kconfig | 15 +++++++++++++++
>  2 files changed, 15 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 9f5bd41bf660..71d4ddd48c02 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1275,23 +1275,6 @@ config TOSHIBA
>  	  Say Y if you intend to run this kernel on a Toshiba portable.
>  	  Say N otherwise.
>  
> -config I8K
> -	tristate "Dell i8k legacy laptop support"
> -	depends on HWMON
> -	depends on PROC_FS
> -	select SENSORS_DELL_SMM
> -	help
> -	  This option enables legacy /proc/i8k userspace interface in hwmon
> -	  dell-smm-hwmon driver. Character file /proc/i8k reports bios version,
> -	  temperature and allows controlling fan speeds of Dell laptops via
> -	  System Management Mode. For old Dell laptops (like Dell Inspiron 8000)
> -	  it reports also power and hotkey status. For fan speed control is
> -	  needed userspace package i8kutils.
> -
> -	  Say Y if you intend to run this kernel on old Dell laptops or want to
> -	  use userspace package i8kutils.
> -	  Say N otherwise.
> -
>  config X86_REBOOTFIXUPS
>  	bool "Enable X86 board specific fixups for reboot"
>  	depends on X86_32
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 8df25f1079ba..dd244aa747ad 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -505,6 +505,21 @@ config SENSORS_DELL_SMM
>  	  When option I8K is also enabled this driver provides legacy /proc/i8k
>  	  userspace interface for i8kutils package.
>  
> +config I8K
> +	bool "Dell i8k legacy laptop support"
> +	depends on SENSORS_DELL_SMM
> +	help
> +	  This option enables legacy /proc/i8k userspace interface in hwmon
> +	  dell-smm-hwmon driver. Character file /proc/i8k reports bios version,
> +	  temperature and allows controlling fan speeds of Dell laptops via
> +	  System Management Mode. For old Dell laptops (like Dell Inspiron 8000)
> +	  it reports also power and hotkey status. For fan speed control is
> +	  needed userspace package i8kutils.
> +
> +	  Say Y if you intend to run this kernel on old Dell laptops or want to
> +	  use userspace package i8kutils.
> +	  Say N otherwise.
> +
>  config SENSORS_DA9052_ADC
>  	tristate "Dialog DA9052/DA9053 ADC"
>  	depends on PMIC_DA9052
> 

Re: [PATCH 1/2] x86/Kconfig: move and modify CONFIG_I8K

[Borislav Petkov]

On Mon, Feb 07, 2022 at 07:51:10PM +0100, Hans de Goede wrote:
> For other reviewers, the only consumer of the CONFIG_I8K
> option is drivers/hwmon/dell-smm-hwmon.c
> which has a couple of:
> "#if IS_ENABLED(CONFIG_I8K)" checks to enable its old
> legacy /proc/i8k interface.
> 
> So this move definitely makes sense.

I love patches removing code from arch/x86/ so for the move:

Acked-by: Borislav Petkov <bp@suse.de>

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH 1/2] x86/Kconfig: move and modify CONFIG_I8K

[Mateusz Jończyk]

W dniu 07.02.2022 o 19:51, Hans de Goede pisze:
> Hi,
>
> On 2/7/22 19:29, Mateusz Jończyk wrote:
>> In Kconfig, inside the "Processor type and features" menu, there is
>> the CONFIG_I8K option: "Dell i8k legacy laptop support". This is
>> very confusing - enabling CONFIG_I8K is not required for the kernel to
>> support old Dell laptops. This option is specific to the dell-smm-hwmon
>> driver, which mostly exports some hardware monitoring information and
>> allows the user to change fan speed.
>>
>> This option is misplaced, so move CONFIG_I8K to drivers/hwmon/Kconfig,
>> where it belongs.
>>
>> Also, modify the dependency order - change
>>         select SENSORS_DELL_SMM
>> to
>>         depends on SENSORS_DELL_SMM
>> as it is just a configuration option of dell-smm-hwmon. This includes
>> changing the option type from tristate to bool. It was tristate because
>> it could select CONFIG_SENSORS_DELL_SMM=m .
>>
>> When running "make oldconfig" on configurations with
>> CONFIG_SENSORS_DELL_SMM enabled , this change will result in an
>> additional question (which could be printed several times during
>> bisecting). I think that tidying up the configuration is worth it,
>> though.
>>
>> Next patch tweaks the description of CONFIG_I8K.
>>
>> [snip]
> For other reviewers, the only consumer of the CONFIG_I8K
> option is drivers/hwmon/dell-smm-hwmon.c
> which has a couple of:
> "#if IS_ENABLED(CONFIG_I8K)" checks to enable its old
> legacy /proc/i8k interface.
>
> So this move definitely makes sense.
>
> I wonder if it would not be better to just completely drop
> the old  /proc/i8k interface though ?

No!!! I use it. The problem is that the laptop (2010-ish Dell Latitude E6500)
has only three fan power levels: off, mild and full. So I think it is
not well-suited to traditional fancontrol. On the other hand,
i8kmon (slightly modified), which is designed for a small number of fan power levels,
works well.

> With that said, this series looks good to me, so:
>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>
> for the series.

Thanks,

Mateusz

> Regards,
>
> Hans
>
>

Re: [PATCH 1/2] x86/Kconfig: move and modify CONFIG_I8K

[Hans de Goede]

Hi,

On 2/7/22 20:31, Mateusz Jończyk wrote:
> W dniu 07.02.2022 o 19:51, Hans de Goede pisze:
>> Hi,
>>
>> On 2/7/22 19:29, Mateusz Jończyk wrote:
>>> In Kconfig, inside the "Processor type and features" menu, there is
>>> the CONFIG_I8K option: "Dell i8k legacy laptop support". This is
>>> very confusing - enabling CONFIG_I8K is not required for the kernel to
>>> support old Dell laptops. This option is specific to the dell-smm-hwmon
>>> driver, which mostly exports some hardware monitoring information and
>>> allows the user to change fan speed.
>>>
>>> This option is misplaced, so move CONFIG_I8K to drivers/hwmon/Kconfig,
>>> where it belongs.
>>>
>>> Also, modify the dependency order - change
>>>         select SENSORS_DELL_SMM
>>> to
>>>         depends on SENSORS_DELL_SMM
>>> as it is just a configuration option of dell-smm-hwmon. This includes
>>> changing the option type from tristate to bool. It was tristate because
>>> it could select CONFIG_SENSORS_DELL_SMM=m .
>>>
>>> When running "make oldconfig" on configurations with
>>> CONFIG_SENSORS_DELL_SMM enabled , this change will result in an
>>> additional question (which could be printed several times during
>>> bisecting). I think that tidying up the configuration is worth it,
>>> though.
>>>
>>> Next patch tweaks the description of CONFIG_I8K.
>>>
>>> [snip]
>> For other reviewers, the only consumer of the CONFIG_I8K
>> option is drivers/hwmon/dell-smm-hwmon.c
>> which has a couple of:
>> "#if IS_ENABLED(CONFIG_I8K)" checks to enable its old
>> legacy /proc/i8k interface.
>>
>> So this move definitely makes sense.
>>
>> I wonder if it would not be better to just completely drop
>> the old  /proc/i8k interface though ?
> 
> No!!! I use it. The problem is that the laptop (2010-ish Dell Latitude E6500)
> has only three fan power levels: off, mild and full. So I think it is
> not well-suited to traditional fancontrol. On the other hand,
> i8kmon (slightly modified), which is designed for a small number of fan power levels,
> works well.

Ok, I was wondering if there would still be any users and
usually the best way to find out is just remove something
and revert the removal if it turns out there are still users...

But since you still use it the question if there are still
users is answered now :)

Regards,

Hans

Re: [PATCH 1/2] x86/Kconfig: move and modify CONFIG_I8K

[Randy Dunlap]

Hi--

I like it. (I had a note to myself to do this also.)


On 2/7/22 10:29, Mateusz Jończyk wrote:
> In Kconfig, inside the "Processor type and features" menu, there is
> the CONFIG_I8K option: "Dell i8k legacy laptop support". This is
> very confusing - enabling CONFIG_I8K is not required for the kernel to
> support old Dell laptops. This option is specific to the dell-smm-hwmon
> driver, which mostly exports some hardware monitoring information and
> allows the user to change fan speed.
> 
> This option is misplaced, so move CONFIG_I8K to drivers/hwmon/Kconfig,
> where it belongs.
> 
> Also, modify the dependency order - change
>         select SENSORS_DELL_SMM
> to
>         depends on SENSORS_DELL_SMM
> as it is just a configuration option of dell-smm-hwmon. This includes
> changing the option type from tristate to bool. It was tristate because
> it could select CONFIG_SENSORS_DELL_SMM=m .
> 
> When running "make oldconfig" on configurations with
> CONFIG_SENSORS_DELL_SMM enabled , this change will result in an
> additional question (which could be printed several times during
> bisecting). I think that tidying up the configuration is worth it,
> though.
> 
> Next patch tweaks the description of CONFIG_I8K.
> 
> Signed-off-by: Mateusz Jończyk <mat.jonczyk@o2.pl>
> Cc: Pali Rohár <pali@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Jean Delvare <jdelvare@suse.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Mark Gross <markgross@kernel.org>
> ---
>  arch/x86/Kconfig      | 17 -----------------
>  drivers/hwmon/Kconfig | 15 +++++++++++++++
>  2 files changed, 15 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 9f5bd41bf660..71d4ddd48c02 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1275,23 +1275,6 @@ config TOSHIBA
>  	  Say Y if you intend to run this kernel on a Toshiba portable.
>  	  Say N otherwise.
>  
> -config I8K
> -	tristate "Dell i8k legacy laptop support"
> -	depends on HWMON
> -	depends on PROC_FS
> -	select SENSORS_DELL_SMM
> -	help
> -	  This option enables legacy /proc/i8k userspace interface in hwmon
> -	  dell-smm-hwmon driver. Character file /proc/i8k reports bios version,
> -	  temperature and allows controlling fan speeds of Dell laptops via
> -	  System Management Mode. For old Dell laptops (like Dell Inspiron 8000)
> -	  it reports also power and hotkey status. For fan speed control is
> -	  needed userspace package i8kutils.
> -
> -	  Say Y if you intend to run this kernel on old Dell laptops or want to
> -	  use userspace package i8kutils.
> -	  Say N otherwise.
> -
>  config X86_REBOOTFIXUPS
>  	bool "Enable X86 board specific fixups for reboot"
>  	depends on X86_32
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 8df25f1079ba..dd244aa747ad 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -505,6 +505,21 @@ config SENSORS_DELL_SMM
>  	  When option I8K is also enabled this driver provides legacy /proc/i8k
>  	  userspace interface for i8kutils package.
>  
> +config I8K
> +	bool "Dell i8k legacy laptop support"
> +	depends on SENSORS_DELL_SMM
> +	help
> +	  This option enables legacy /proc/i8k userspace interface in hwmon
> +	  dell-smm-hwmon driver. Character file /proc/i8k reports bios version,

	                                                          BIOS

> +	  temperature and allows controlling fan speeds of Dell laptops via
> +	  System Management Mode. For old Dell laptops (like Dell Inspiron 8000)
> +	  it reports also power and hotkey status. For fan speed control is
> +	  needed userspace package i8kutils.

Last sentence above is awkward. How about:

	  it also reports power and hotkey status. For fan speed control, the
	  i8kutils userspace package is needed.

> +
> +	  Say Y if you intend to run this kernel on old Dell laptops or want to
> +	  use userspace package i8kutils.
> +	  Say N otherwise.
> +
>  config SENSORS_DA9052_ADC
>  	tristate "Dialog DA9052/DA9053 ADC"
>  	depends on PMIC_DA9052


Reviewed-by: Randy Dunlap <rdunlap@infradead.org>

thanks.
-- 
~Randy

Re: [PATCH 1/2] x86/Kconfig: move and modify CONFIG_I8K

[Mateusz Jończyk]

W dniu 07.02.2022 o 19:29, Mateusz Jończyk pisze:
> In Kconfig, inside the "Processor type and features" menu, there is
> the CONFIG_I8K option: "Dell i8k legacy laptop support". This is
> very confusing - enabling CONFIG_I8K is not required for the kernel to
> support old Dell laptops. This option is specific to the dell-smm-hwmon
> driver, which mostly exports some hardware monitoring information and
> allows the user to change fan speed.
>
> This option is misplaced, so move CONFIG_I8K to drivers/hwmon/Kconfig,
> where it belongs.
>
> Also, modify the dependency order - change
>         select SENSORS_DELL_SMM
> to
>         depends on SENSORS_DELL_SMM
> as it is just a configuration option of dell-smm-hwmon. This includes
> changing the option type from tristate to bool. It was tristate because
> it could select CONFIG_SENSORS_DELL_SMM=m .
>
> When running "make oldconfig" on configurations with
> CONFIG_SENSORS_DELL_SMM enabled , this change will result in an
> additional question (which could be printed several times during
> bisecting). I think that tidying up the configuration is worth it,
> though.
>
> Next patch tweaks the description of CONFIG_I8K.
>
> Signed-off-by: Mateusz Jończyk <mat.jonczyk@o2.pl>
> Cc: Pali Rohár <pali@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Jean Delvare <jdelvare@suse.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Mark Gross <markgross@kernel.org>
> ---
>  arch/x86/Kconfig      | 17 -----------------
>  drivers/hwmon/Kconfig | 15 +++++++++++++++
>  2 files changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 9f5bd41bf660..71d4ddd48c02 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1275,23 +1275,6 @@ config TOSHIBA
>  	  Say Y if you intend to run this kernel on a Toshiba portable.
>  	  Say N otherwise.
>  
> -config I8K
> -	tristate "Dell i8k legacy laptop support"
> -	depends on HWMON
> -	depends on PROC_FS
> -	select SENSORS_DELL_SMM
> -	help
> -	  This option enables legacy /proc/i8k userspace interface in hwmon
> -	  dell-smm-hwmon driver. Character file /proc/i8k reports bios version,
> -	  temperature and allows controlling fan speeds of Dell laptops via
> -	  System Management Mode. For old Dell laptops (like Dell Inspiron 8000)
> -	  it reports also power and hotkey status. For fan speed control is
> -	  needed userspace package i8kutils.
> -
> -	  Say Y if you intend to run this kernel on old Dell laptops or want to
> -	  use userspace package i8kutils.
> -	  Say N otherwise.
> -
>  config X86_REBOOTFIXUPS
>  	bool "Enable X86 board specific fixups for reboot"
>  	depends on X86_32
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 8df25f1079ba..dd244aa747ad 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -505,6 +505,21 @@ config SENSORS_DELL_SMM
>  	  When option I8K is also enabled this driver provides legacy /proc/i8k
>  	  userspace interface for i8kutils package.
>  
> +config I8K
> +	bool "Dell i8k legacy laptop support"
> +	depends on SENSORS_DELL_SMM

Oops, I dropped "depends on PROC_FS". Will fix this in next revision.

> +	help
> +	  This option enables legacy /proc/i8k userspace interface in hwmon
> +	  dell-smm-hwmon driver. Character file /proc/i8k reports bios version,
> +	  temperature and allows controlling fan speeds of Dell laptops via
> +	  System Management Mode. For old Dell laptops (like Dell Inspiron 8000)
> +	  it reports also power and hotkey status. For fan speed control is
> +	  needed userspace package i8kutils.
> +
> +	  Say Y if you intend to run this kernel on old Dell laptops or want to
> +	  use userspace package i8kutils.
> +	  Say N otherwise.
> +
>  config SENSORS_DA9052_ADC
>  	tristate "Dialog DA9052/DA9053 ADC"
>  	depends on PMIC_DA9052

Greetings,

Mateusz