Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <b0642e67-7752-8d27-9e67-7e9a493adb08@o2.pl>
Date:   Mon, 7 Feb 2022 20:31:07 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.1
Subject: Re: [PATCH 1/2] x86/Kconfig: move and modify CONFIG_I8K
Content-Language: en-GB
To:     Hans de Goede <hdegoede@redhat.com>, linux-kernel@vger.kernel.org,
        x86@kernel.org, linux-hwmon@vger.kernel.org
Cc:     =?UTF-8?Q?Pali_Roh=c3=a1r?= <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>
References: <20220207182940.242838-1-mat.jonczyk@o2.pl>
 <43bfd6d9-58a3-6f9c-0848-4ef147552afb@redhat.com>
From:   =?UTF-8?Q?Mateusz_Jo=c5=84czyk?= <mat.jonczyk@o2.pl>
In-Reply-To: <43bfd6d9-58a3-6f9c-0848-4ef147552afb@redhat.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

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
>
>