[PATCH v2 0/2] hwmon: (dell-smm) Minor fixes

[PATCH v2 0/2] hwmon: (dell-smm) Minor fixes

[Armin Wolf]

This patch series fixes some minor issues inside dell-smm-hwmon.
The two patches change some warning messages to be more
informative/unambiguous.

All changes where tested on a Dell Inspiron 3505.
---
Changes in v2:
- drop patch which caused probe to fail when cooling device
registration failed.
- Change last patch so users are able to determine if any
features where disabled due to BIOS bugs.

Armin Wolf (2):
  hwmon: (dell-smm) Add FW_BUG to SMM warning message
  hwmon: (dell-smm) Improve warning messages

 drivers/hwmon/dell-smm-hwmon.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

--
2.30.2

[PATCH v2 1/2] hwmon: (dell-smm) Add FW_BUG to SMM warning message

[Armin Wolf]

When a SMM call takes very long to execute, then it definitely
is a firmware bug which should be marked with FW_BUG.
Also add the number of the buggy SMM call to the warning message
so BIOS developers, etc immediately know which part of the SMM
interface is buggy.

Tested on a Dell Inspiron 3505.

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

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 7f8d95dd2717..9cac80358072 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -26,6 +26,7 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/platform_device.h>
+#include <linux/printk.h>
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
@@ -198,7 +199,7 @@ static int i8k_smm_func(void *par)
 		 eax, ebx, regs->eax & 0xffff, carry, duration);

 	if (duration > DELL_SMM_MAX_DURATION)
-		pr_warn_once("SMM call took %lld usecs!\n", duration);
+		pr_warn_once(FW_BUG "SMM call 0x%.4x took %lld usecs!\n", eax, duration);

 	if (carry || (regs->eax & 0xffff) == 0xffff || regs->eax == eax)
 		return -EINVAL;
--
2.30.2

Re: [PATCH v2 1/2] hwmon: (dell-smm) Add FW_BUG to SMM warning message

[Guenter Roeck]

On Mon, Aug 22, 2022 at 07:40:52PM +0200, Armin Wolf wrote:
> When a SMM call takes very long to execute, then it definitely
> is a firmware bug which should be marked with FW_BUG.
> Also add the number of the buggy SMM call to the warning message
> so BIOS developers, etc immediately know which part of the SMM
> interface is buggy.
> 

I am not going to apply this patch.

It is well known that some of those calls take forever to execute,
and the current limit is just an assumption. This may be a firmware
bug, or it works as expected. We simply don't know, and we can not
declare it to be a firmware bug unless we have confirmation from Dell.

Guenter

> Tested on a Dell Inspiron 3505.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/hwmon/dell-smm-hwmon.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> --
> 2.30.2
> 
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index 7f8d95dd2717..9cac80358072 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -26,6 +26,7 @@
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/platform_device.h>
> +#include <linux/printk.h>
>  #include <linux/proc_fs.h>
>  #include <linux/seq_file.h>
>  #include <linux/slab.h>
> @@ -198,7 +199,7 @@ static int i8k_smm_func(void *par)
>  		 eax, ebx, regs->eax & 0xffff, carry, duration);
> 
>  	if (duration > DELL_SMM_MAX_DURATION)
> -		pr_warn_once("SMM call took %lld usecs!\n", duration);
> +		pr_warn_once(FW_BUG "SMM call 0x%.4x took %lld usecs!\n", eax, duration);
> 
>  	if (carry || (regs->eax & 0xffff) == 0xffff || regs->eax == eax)
>  		return -EINVAL;

[PATCH v2 2/2] hwmon: (dell-smm) Improve warning messages

[Armin Wolf]

When dell-smm-hwmon is loaded on a machine with a buggy BIOS
with the option "force" being enabled, it wrongly prints
that 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.

Replace the messages with two messages each which are depending
on the value of the "force" parameter. The messages which are
being printed when "force" is not set use dev_notice() instead
of dev_warn() since they only serve as a notice.

Tested on a Dell Inspiron 3505.

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

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 9cac80358072..01a94b62c2ab 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -1356,15 +1356,21 @@ 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");
-		if (!force)
+		if (!force) {
+			dev_notice(&pdev->dev, "Disabling fan support due to BIOS bugs\n");
 			data->disallow_fan_support = true;
+		} else {
+			dev_warn(&pdev->dev, "Enabling fan support despite BIOS bugs\n");
+		}
 	}

 	if (dmi_check_system(i8k_blacklist_fan_type_dmi_table)) {
-		dev_warn(&pdev->dev, "broken Dell BIOS detected, disallow fan type call\n");
-		if (!force)
+		if (!force) {
+			dev_notice(&pdev->dev, "Disabling fan type call due to BIOS bugs\n");
 			data->disallow_fan_type_call = true;
+		} else {
+			dev_warn(&pdev->dev, "Enabling fan type call despite BIOS bugs\n");
+		}
 	}

 	strscpy(data->bios_version, i8k_get_dmi_data(DMI_BIOS_VERSION),
--
2.30.2

Re: [PATCH v2 2/2] hwmon: (dell-smm) Improve warning messages

[Guenter Roeck]

On Mon, Aug 22, 2022 at 07:40:53PM +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
> that 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.
> 
> Replace the messages with two messages each which are depending
> on the value of the "force" parameter. The messages which are
> being printed when "force" is not set use dev_notice() instead
> of dev_warn() since they only serve as a notice.
> 
> Tested on a Dell Inspiron 3505.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>

Applied to hwmon-next.

Thanks,
Guenter

> ---
>  drivers/hwmon/dell-smm-hwmon.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> --
> 2.30.2
> 
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index 9cac80358072..01a94b62c2ab 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -1356,15 +1356,21 @@ 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");
> -		if (!force)
> +		if (!force) {
> +			dev_notice(&pdev->dev, "Disabling fan support due to BIOS bugs\n");
>  			data->disallow_fan_support = true;
> +		} else {
> +			dev_warn(&pdev->dev, "Enabling fan support despite BIOS bugs\n");
> +		}
>  	}
> 
>  	if (dmi_check_system(i8k_blacklist_fan_type_dmi_table)) {
> -		dev_warn(&pdev->dev, "broken Dell BIOS detected, disallow fan type call\n");
> -		if (!force)
> +		if (!force) {
> +			dev_notice(&pdev->dev, "Disabling fan type call due to BIOS bugs\n");
>  			data->disallow_fan_type_call = true;
> +		} else {
> +			dev_warn(&pdev->dev, "Enabling fan type call despite BIOS bugs\n");
> +		}
>  	}
> 
>  	strscpy(data->bios_version, i8k_get_dmi_data(DMI_BIOS_VERSION),