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

From: Hans de Goede <hdegoede@redhat.com>
To: "Mateusz Jończyk" <mat.jonczyk@o2.pl>,
	linux-kernel@vger.kernel.org, x86@kernel.org,
	linux-hwmon@vger.kernel.org
Cc: "Pali Rohár" <pali@kernel.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Ingo Molnar" <mingo@redhat.com>,
	"Borislav Petkov" <bp@alien8.de>,
	"Dave Hansen" <dave.hansen@linux.intel.com>,
	"Jean Delvare" <jdelvare@suse.com>,
	"Guenter Roeck" <linux@roeck-us.net>,
	"Mark Gross" <markgross@kernel.org>
Subject: Re: [PATCH 1/2] x86/Kconfig: move and modify CONFIG_I8K
Date: Mon, 7 Feb 2022 22:18:21 +0100	[thread overview]
Message-ID: <9d2f23e3-712e-5df2-28a5-2779a107759c@redhat.com> (raw)
In-Reply-To: <b0642e67-7752-8d27-9e67-7e9a493adb08@o2.pl>

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