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

From: Mateusz Jończyk
Date: Mon Feb 07 2022 - 14:33:46 EST


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

Thanks,

Mateusz

> Regards,
>
> Hans
>
>