diff mbox series

[3/3] hwmon: (dell-smm) Improve warning messages

Message ID 20220821151713.11198-4-W_Armin@gmx.de (mailing list archive)
State Superseded
Headers show
Series hwmon: (dell-smm) Minor fixes | expand

Commit Message

Armin Wolf Aug. 21, 2022, 3:17 p.m. UTC 
When dell-smm-hwmon is loaded on a machine with a buggy BIOS
with the option "force" being enabled, it wrongly prints
what the buggy features where disabled. This may cause
users to wrongly assume that the driver still protects them
from these BIOS bugs even with "force" being enabled.
Change the warning message to avoid such a misunderstanding.

Tested on a Dell Inspiron 3505.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/hwmon/dell-smm-hwmon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--
2.30.2

Comments

Pali Rohár Aug. 21, 2022, 3:46 p.m. UTC | #1 
On Sunday 21 August 2022 17:17:13 Armin Wolf wrote:
> When dell-smm-hwmon is loaded on a machine with a buggy BIOS
> with the option "force" being enabled, it wrongly prints
> what the buggy features where disabled. This may cause
> users to wrongly assume that the driver still protects them
> from these BIOS bugs even with "force" being enabled.
> Change the warning message to avoid such a misunderstanding.

Should not there be also FW_BUG too?

I'm thinking more about message, would not it be better to print also
information if fan support and fan type call is allowed or disallowed
(based on force argument) when broken BIOS is detected?

> Tested on a Dell Inspiron 3505.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/hwmon/dell-smm-hwmon.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index f7bab1a91b93..bf13852afe48 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -1354,13 +1354,13 @@ static int __init dell_smm_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, data);
> 
>  	if (dmi_check_system(i8k_blacklist_fan_support_dmi_table)) {
> -		dev_warn(&pdev->dev, "broken Dell BIOS detected, disallow fan support\n");
> +		dev_warn(&pdev->dev, "BIOS has broken fan support\n");
>  		if (!force)
>  			data->disallow_fan_support = true;
>  	}
> 
>  	if (dmi_check_system(i8k_blacklist_fan_type_dmi_table)) {
> -		dev_warn(&pdev->dev, "broken Dell BIOS detected, disallow fan type call\n");
> +		dev_warn(&pdev->dev, "BIOS has broken fan type call\n");
>  		if (!force)
>  			data->disallow_fan_type_call = true;
>  	}
> --
> 2.30.2
>
Armin Wolf Aug. 21, 2022, 7:17 p.m. UTC | #2 
Am 21.08.22 um 17:46 schrieb Pali Rohár:

> On Sunday 21 August 2022 17:17:13 Armin Wolf wrote:
>> When dell-smm-hwmon is loaded on a machine with a buggy BIOS
>> with the option "force" being enabled, it wrongly prints
>> what the buggy features where disabled. This may cause
>> users to wrongly assume that the driver still protects them
>> from these BIOS bugs even with "force" being enabled.
>> Change the warning message to avoid such a misunderstanding.
> Should not there be also FW_BUG too?

Since the driver itself is unable to detect if a buggy BIOS was fixed,
i would not use FW_BUG here since it may cause confusion if such a message
was printed on an already fixed BIOS.

> I'm thinking more about message, would not it be better to print also
> information if fan support and fan type call is allowed or disallowed
> (based on force argument) when broken BIOS is detected?

Something like "Disabling fan support due to BIOS bugs" and "Enabling fan support despite BIOS bugs"?
The first message could only be a notice message, while the second message could be a warning message.

Armin Wolf

>> Tested on a Dell Inspiron 3505.
>>
>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>> ---
>>   drivers/hwmon/dell-smm-hwmon.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
>> index f7bab1a91b93..bf13852afe48 100644
>> --- a/drivers/hwmon/dell-smm-hwmon.c
>> +++ b/drivers/hwmon/dell-smm-hwmon.c
>> @@ -1354,13 +1354,13 @@ static int __init dell_smm_probe(struct platform_device *pdev)
>>   	platform_set_drvdata(pdev, data);
>>
>>   	if (dmi_check_system(i8k_blacklist_fan_support_dmi_table)) {
>> -		dev_warn(&pdev->dev, "broken Dell BIOS detected, disallow fan support\n");
>> +		dev_warn(&pdev->dev, "BIOS has broken fan support\n");
>>   		if (!force)
>>   			data->disallow_fan_support = true;
>>   	}
>>
>>   	if (dmi_check_system(i8k_blacklist_fan_type_dmi_table)) {
>> -		dev_warn(&pdev->dev, "broken Dell BIOS detected, disallow fan type call\n");
>> +		dev_warn(&pdev->dev, "BIOS has broken fan type call\n");
>>   		if (!force)
>>   			data->disallow_fan_type_call = true;
>>   	}
>> --
>> 2.30.2
>>
Guenter Roeck Aug. 22, 2022, 4:21 p.m. UTC | #3 
On Sun, Aug 21, 2022 at 05:17:13PM +0200, Armin Wolf wrote:
> When dell-smm-hwmon is loaded on a machine with a buggy BIOS
> with the option "force" being enabled, it wrongly prints
> what the buggy features where disabled. This may cause
> users to wrongly assume that the driver still protects them
> from these BIOS bugs even with "force" being enabled.
> Change the warning message to avoid such a misunderstanding.
> 
> Tested on a Dell Inspiron 3505.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/hwmon/dell-smm-hwmon.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index f7bab1a91b93..bf13852afe48 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -1354,13 +1354,13 @@ static int __init dell_smm_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, data);
> 
>  	if (dmi_check_system(i8k_blacklist_fan_support_dmi_table)) {
> -		dev_warn(&pdev->dev, "broken Dell BIOS detected, disallow fan support\n");
> +		dev_warn(&pdev->dev, "BIOS has broken fan support\n");

I don't see that as improvement. It no longer mentions that fan support
is disabled if it is disabled.

>  		if (!force)
>  			data->disallow_fan_support = true;
>  	}
> 
>  	if (dmi_check_system(i8k_blacklist_fan_type_dmi_table)) {
> -		dev_warn(&pdev->dev, "broken Dell BIOS detected, disallow fan type call\n");
> +		dev_warn(&pdev->dev, "BIOS has broken fan type call\n");

Same as above.

>  		if (!force)
>  			data->disallow_fan_type_call = true;
>  	}
> --
> 2.30.2
>
diff mbox series

Patch

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index f7bab1a91b93..bf13852afe48 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -1354,13 +1354,13 @@  static int __init dell_smm_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, data);

 	if (dmi_check_system(i8k_blacklist_fan_support_dmi_table)) {
-		dev_warn(&pdev->dev, "broken Dell BIOS detected, disallow fan support\n");
+		dev_warn(&pdev->dev, "BIOS has broken fan support\n");
 		if (!force)
 			data->disallow_fan_support = true;
 	}

 	if (dmi_check_system(i8k_blacklist_fan_type_dmi_table)) {
-		dev_warn(&pdev->dev, "broken Dell BIOS detected, disallow fan type call\n");
+		dev_warn(&pdev->dev, "BIOS has broken fan type call\n");
 		if (!force)
 			data->disallow_fan_type_call = true;
 	}