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 <[email protected]>
Cc: Pali Rohár <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: Jean Delvare <[email protected]>
Cc: Guenter Roeck <[email protected]>
Cc: Hans de Goede <[email protected]>
Cc: Mark Gross <[email protected]>
---
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
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 <[email protected]>
> Cc: Pali Rohár <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Dave Hansen <[email protected]>
> Cc: Jean Delvare <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Cc: Hans de Goede <[email protected]>
> Cc: Mark Gross <[email protected]>
> ---
> 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 <[email protected]>
thanks.
--
~Randy
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 <[email protected]>
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
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 <[email protected]>
> Cc: Pali Rohár <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Dave Hansen <[email protected]>
> Cc: Jean Delvare <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Cc: Hans de Goede <[email protected]>
> Cc: Mark Gross <[email protected]>
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 <[email protected]>
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
>
It is not the laptops, but the /proc/i8k interface that is legacy. The
old description was confusing, fix this.
I'm not a native English speaker, so I'd like that someone proofread
this description.
Signed-off-by: Mateusz Jończyk <[email protected]>
Cc: Pali Rohár <[email protected]>
Cc: Jean Delvare <[email protected]>
Cc: Guenter Roeck <[email protected]>
Cc: Hans de Goede <[email protected]>
Cc: Mark Gross <[email protected]>
---
drivers/hwmon/Kconfig | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index dd244aa747ad..8f9f41a9ef70 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -506,18 +506,17 @@ 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
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 reports also 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
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
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 <[email protected]>
> Cc: Pali Rohár <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Dave Hansen <[email protected]>
> Cc: Jean Delvare <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Cc: Hans de Goede <[email protected]>
> Cc: Mark Gross <[email protected]>
> ---
> 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
Hi--
On 2/7/22 10:29, Mateusz Jończyk wrote:
> It is not the laptops, but the /proc/i8k interface that is legacy. The
> old description was confusing, fix this.
>
> I'm not a native English speaker, so I'd like that someone proofread
> this description.
>
> Signed-off-by: Mateusz Jończyk <[email protected]>
> Cc: Pali Rohár <[email protected]>
> Cc: Jean Delvare <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Cc: Hans de Goede <[email protected]>
> Cc: Mark Gross <[email protected]>
> ---
> drivers/hwmon/Kconfig | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index dd244aa747ad..8f9f41a9ef70 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -506,18 +506,17 @@ 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
> 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 reports also power and hotkey status.
Comma not needed ^^^; "it also reports ..." would be more common.
>
> - 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
thanks.
--
~Randy
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 <[email protected]>
>
> for the series.
Thanks,
Mateusz
> Regards,
>
> Hans
>
>