linux-kernel - Re: [PATCH v2] hwmon: (dell-smm) Use one DMI match for all XPS models

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] [day] [month] [year] [list]

Message-ID: <c830927d-3365-4b58-9bea-6c99ca2d9edb@roeck-us.net>
Date:   Tue, 7 Apr 2020 19:48:50 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Pali Rohár <pali@...nel.org>,
        Thomas Hebb <tommyhebb@...il.com>
Cc:     linux-kernel@...r.kernel.org, Jean Delvare <jdelvare@...e.com>,
        linux-hwmon@...r.kernel.org
Subject: Re: [PATCH v2] hwmon: (dell-smm) Use one DMI match for all XPS models

On 4/7/20 3:22 AM, Pali Rohár wrote:
> Hi!
> 
> On Saturday 04 April 2020 16:49:00 Thomas Hebb wrote:
>> Currently, each new XPS has to be added manually for module autoloading
>> to work. Since fan multiplier autodetection should work fine on all XPS
>> models, just match them all with one block like is done for Precision
>> and Studio.
> 
> It makes sense. We already load driver for all Inspirion, Latitude,
> Precision, Vostro and Studio models so I do not see reason why not to
> load it also for all XPS models. I doubt that Dell uses one base
> firmware for all mentioned models and second one specially for XPS.
> 
>> The only match we replace that doesn't already use autodetection is
>> "XPS13" which, according to Google, only matches the XPS 13 9333. (All
>> other XPS 13 models have "XPS" as its own word, surrounded by spaces.)
>> According to the thread at [1], autodetection works for the XPS 13 9333,
>> meaning this shouldn't regress it. I do not own one to confirm with,
>> though.
>>
>> Tested on an XPS 13 9350 and confirmed the module now autoloads and
>> reports reasonable-looking data. I am using BIOS 1.12.2 and do not see
>> any freezes when querying fan speed.
>>
>> [1] https://lore.kernel.org/patchwork/patch/525367/
> 
> I guess that these two tests are enough based on the fact that lot of
> XPS models are already whitelisted.
> 
> Guenter, it is fine for you now? Or is something else needed?
> 

I still have my reservations, but ...

>> Signed-off-by: Thomas Hebb <tommyhebb@...il.com>
> 
> Acked-by: Pali Rohár <pali@...nel.org>
> 
I'll apply it to linux-next with your approval. After all, the entire driver
is a mess to start with. We'll see if it blows up in our face.

Guenter

>> ---
>>
>> Changes in v2:
>> - Remove another now-redundant XPS entry that I'd missed.
>>
>>  drivers/hwmon/dell-smm-hwmon.c | 26 ++------------------------
>>  1 file changed, 2 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
>> index d4c83009d625..ca30bf903ec7 100644
>> --- a/drivers/hwmon/dell-smm-hwmon.c
>> +++ b/drivers/hwmon/dell-smm-hwmon.c
>> @@ -1072,13 +1072,6 @@ static const struct dmi_system_id i8k_dmi_table[] __initconst = {
>>  			DMI_MATCH(DMI_PRODUCT_NAME, "Vostro"),
>>  		},
>>  	},
>> -	{
>> -		.ident = "Dell XPS421",
>> -		.matches = {
>> -			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>> -			DMI_MATCH(DMI_PRODUCT_NAME, "XPS L421X"),
>> -		},
>> -	},
>>  	{
>>  		.ident = "Dell Studio",
>>  		.matches = {
>> @@ -1087,14 +1080,6 @@ static const struct dmi_system_id i8k_dmi_table[] __initconst = {
>>  		},
>>  		.driver_data = (void *)&i8k_config_data[DELL_STUDIO],
>>  	},
>> -	{
>> -		.ident = "Dell XPS 13",
>> -		.matches = {
>> -			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>> -			DMI_MATCH(DMI_PRODUCT_NAME, "XPS13"),
>> -		},
>> -		.driver_data = (void *)&i8k_config_data[DELL_XPS],
>> -	},
>>  	{
>>  		.ident = "Dell XPS M140",
>>  		.matches = {
>> @@ -1104,17 +1089,10 @@ static const struct dmi_system_id i8k_dmi_table[] __initconst = {
>>  		.driver_data = (void *)&i8k_config_data[DELL_XPS],
>>  	},
>>  	{
>> -		.ident = "Dell XPS 15 9560",
>> -		.matches = {
>> -			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>> -			DMI_MATCH(DMI_PRODUCT_NAME, "XPS 15 9560"),
>> -		},
>> -	},
>> -	{
>> -		.ident = "Dell XPS 15 9570",
>> +		.ident = "Dell XPS",
>>  		.matches = {
>>  			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>> -			DMI_MATCH(DMI_PRODUCT_NAME, "XPS 15 9570"),
>> +			DMI_MATCH(DMI_PRODUCT_NAME, "XPS"),
>>  		},
>>  	},
>>  	{ }
>> -- 
>> 2.25.2
>>