This patch series improves the init code of the dell_smm_hwmon
driver. The first patch speeds up device initialisation by avoiding
unnecessary SMM calls during init, which might be slow on some
machines. The second patch is a small cleanup patch, while the
third patch allows for easier diagnosis of audio problems caused
by really slow SMM calls.
Tested on a Dell Inspiron 3505.
Changes in v2:
- replace pr_warn() with pr_warn_once()
Armin Wolf (3):
hwmon: (dell-smm) Avoid unnecessary SMM calls during init
hwmon: (dell-smm) Cleanup init code
hwmon: (dell-smm) Warn if SMM call took a very long time to execute
drivers/hwmon/dell-smm-hwmon.c | 50 +++++++++++++++-------------------
1 file changed, 22 insertions(+), 28 deletions(-)
--
2.30.2
The default values for i8k_fan_mult and i8k_fan_max
should be assigend only if the values specified as
module params or in DMI are invalid/missing.
Signed-off-by: Armin Wolf <[email protected]>
---
drivers/hwmon/dell-smm-hwmon.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 202ee884de9e..f13902414615 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -1373,8 +1373,6 @@ static int __init dell_smm_probe(struct platform_device *pdev)
return -ENOMEM;
mutex_init(&data->i8k_mutex);
- data->i8k_fan_mult = I8K_FAN_MULT;
- data->i8k_fan_max = I8K_FAN_HIGH;
platform_set_drvdata(pdev, data);
if (dmi_check_system(i8k_blacklist_fan_support_dmi_table)) {
@@ -1409,7 +1407,9 @@ static int __init dell_smm_probe(struct platform_device *pdev)
fan_max = conf->fan_max;
}
- data->i8k_fan_max = fan_max ? : I8K_FAN_HIGH; /* Must not be 0 */
+ /* All options must not be 0 */
+ data->i8k_fan_mult = fan_mult ? : I8K_FAN_MULT;
+ data->i8k_fan_max = fan_max ? : I8K_FAN_HIGH;
data->i8k_pwm_mult = DIV_ROUND_UP(255, data->i8k_fan_max);
fan_control = dmi_first_match(i8k_whitelist_fan_control);
@@ -1421,9 +1421,6 @@ static int __init dell_smm_probe(struct platform_device *pdev)
dev_info(&pdev->dev, "enabling support for setting automatic/manual fan control\n");
}
- if (fan_mult)
- data->i8k_fan_mult = fan_mult;
-
ret = dell_smm_init_hwmon(&pdev->dev);
if (ret)
return ret;
--
2.30.2
On 4/26/22 14:31, Armin Wolf wrote:
> This patch series improves the init code of the dell_smm_hwmon
> driver. The first patch speeds up device initialisation by avoiding
> unnecessary SMM calls during init, which might be slow on some
> machines. The second patch is a small cleanup patch, while the
> third patch allows for easier diagnosis of audio problems caused
> by really slow SMM calls.
>
> Tested on a Dell Inspiron 3505.
Series applied to hwmon-next.
Thanks,
Guenter
>
> Changes in v2:
> - replace pr_warn() with pr_warn_once()
>
> Armin Wolf (3):
> hwmon: (dell-smm) Avoid unnecessary SMM calls during init
> hwmon: (dell-smm) Cleanup init code
> hwmon: (dell-smm) Warn if SMM call took a very long time to execute
>
> drivers/hwmon/dell-smm-hwmon.c | 50 +++++++++++++++-------------------
> 1 file changed, 22 insertions(+), 28 deletions(-)
>
> --
> 2.30.2
>
If a particular SMM call takes a very long time to execute,
the user might experience audio problems. Print a warning
if a particular SMM call took over 0.250 seconds to execute,
so the user can check whether or not possible audio problems
are caused by this driver.
Signed-off-by: Armin Wolf <[email protected]>
---
drivers/hwmon/dell-smm-hwmon.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index f13902414615..071aa6f4e109 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -49,6 +49,9 @@
#define I8K_SMM_GET_DELL_SIG1 0xfea3
#define I8K_SMM_GET_DELL_SIG2 0xffa3
+/* in usecs */
+#define DELL_SMM_MAX_DURATION 250000
+
#define I8K_FAN_MULT 30
#define I8K_FAN_RPM_THRESHOLD 1000
#define I8K_MAX_TEMP 127
@@ -239,6 +242,9 @@ static int i8k_smm_func(void *par)
pr_debug("smm(0x%.4x 0x%.4x) = 0x%.4x (took %7lld usecs)\n", eax, ebx,
(rc ? 0xffff : regs->eax & 0xffff), duration);
+ if (duration > DELL_SMM_MAX_DURATION)
+ pr_warn_once("SMM call took %lld usecs!\n", duration);
+
return rc;
}
--
2.30.2
When the driver tries to detect the fan multiplier during
module initialisation, it issues one SMM call for each fan.
Those SMM calls are however redundant and also try to query
fans which may not be present.
Fix that by detecting the fan multiplier during hwmon
initialisation when no extra SMM calls are needed.
Also dont assume the last nominal speed entry to be the
biggest and instead check all entries.
Signed-off-by: Armin Wolf <[email protected]>
---
drivers/hwmon/dell-smm-hwmon.c | 37 +++++++++++++---------------------
1 file changed, 14 insertions(+), 23 deletions(-)
diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 30b6f0c28093..202ee884de9e 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -50,7 +50,7 @@
#define I8K_SMM_GET_DELL_SIG2 0xffa3
#define I8K_FAN_MULT 30
-#define I8K_FAN_MAX_RPM 30000
+#define I8K_FAN_RPM_THRESHOLD 1000
#define I8K_MAX_TEMP 127
#define I8K_FN_NONE 0x00
@@ -326,7 +326,7 @@ static int __init i8k_get_fan_nominal_speed(const struct dell_smm_data *data, u8
if (data->disallow_fan_support)
return -EINVAL;
- return i8k_smm(®s) ? : (regs.eax & 0xffff) * data->i8k_fan_mult;
+ return i8k_smm(®s) ? : (regs.eax & 0xffff);
}
/*
@@ -776,6 +776,7 @@ static int dell_smm_read(struct device *dev, enum hwmon_sensor_types type, u32 a
long *val)
{
struct dell_smm_data *data = dev_get_drvdata(dev);
+ int mult = data->i8k_fan_mult;
int ret;
switch (type) {
@@ -804,11 +805,11 @@ static int dell_smm_read(struct device *dev, enum hwmon_sensor_types type, u32 a
return 0;
case hwmon_fan_min:
- *val = data->fan_nominal_speed[channel][0];
+ *val = data->fan_nominal_speed[channel][0] * mult;
return 0;
case hwmon_fan_max:
- *val = data->fan_nominal_speed[channel][data->i8k_fan_max];
+ *val = data->fan_nominal_speed[channel][data->i8k_fan_max] * mult;
return 0;
case hwmon_fan_target:
@@ -819,7 +820,7 @@ static int dell_smm_read(struct device *dev, enum hwmon_sensor_types type, u32 a
if (ret > data->i8k_fan_max)
ret = data->i8k_fan_max;
- *val = data->fan_nominal_speed[channel][ret];
+ *val = data->fan_nominal_speed[channel][ret] * mult;
return 0;
default:
@@ -1071,6 +1072,13 @@ static int __init dell_smm_init_hwmon(struct device *dev)
break;
}
data->fan_nominal_speed[i][state] = err;
+ /*
+ * Autodetect fan multiplier based on nominal rpm if multiplier
+ * was not specified as module param or in DMI. If fan reports
+ * rpm value too high then set multiplier to 1.
+ */
+ if (!fan_mult && err > I8K_FAN_RPM_THRESHOLD)
+ data->i8k_fan_mult = 1;
}
}
@@ -1359,7 +1367,6 @@ static int __init dell_smm_probe(struct platform_device *pdev)
struct dell_smm_data *data;
const struct dmi_system_id *id, *fan_control;
int ret;
- u8 fan;
data = devm_kzalloc(&pdev->dev, sizeof(struct dell_smm_data), GFP_KERNEL);
if (!data)
@@ -1414,24 +1421,8 @@ static int __init dell_smm_probe(struct platform_device *pdev)
dev_info(&pdev->dev, "enabling support for setting automatic/manual fan control\n");
}
- if (!fan_mult) {
- /*
- * Autodetect fan multiplier based on nominal rpm
- * If fan reports rpm value too high then set multiplier to 1
- */
- for (fan = 0; fan < DELL_SMM_NO_FANS; ++fan) {
- ret = i8k_get_fan_nominal_speed(data, fan, data->i8k_fan_max);
- if (ret < 0)
- continue;
-
- if (ret > I8K_FAN_MAX_RPM)
- data->i8k_fan_mult = 1;
- break;
- }
- } else {
- /* Fan multiplier was specified in module param or in dmi */
+ if (fan_mult)
data->i8k_fan_mult = fan_mult;
- }
ret = dell_smm_init_hwmon(&pdev->dev);
if (ret)
--
2.30.2