linux-kernel - Re: [PATCH 1/2] x86/Kconfig: move and modify CONFIG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b0642e67-7752-8d27-9e67-7e9a493adb08@o2.pl>
Date:   Mon, 7 Feb 2022 20:31:07 +0100
From:   Mateusz Jończyk <mat.jonczyk@...pl>
To:     Hans de Goede <hdegoede@...hat.com>, linux-kernel@...r.kernel.org,
        x86@...nel.org, linux-hwmon@...r.kernel.org
Cc:     Pali Rohár <pali@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Jean Delvare <jdelvare@...e.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Mark Gross <markgross@...nel.org>
Subject: Re: [PATCH 1/2] x86/Kconfig: move and modify CONFIG_I8K

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@...hat.com>
>
> for the series.

Thanks,

Mateusz

> Regards,
>
> Hans
>
>