lkml.org 
[lkml]   [2022]   [Apr]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/3] hwmon: (dell-smm) Avoid unnecessary SMM calls during init
On Tue, Apr 26, 2022 at 01:20:48PM -0700, Guenter Roeck wrote:
> On Tue, Apr 26, 2022 at 10:03:18PM +0200, Armin Wolf wrote:
> > 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 <W_Armin@gmx.de>
> > ---
> > 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(&regs) ? : (regs.eax & 0xffff) * data->i8k_fan_mult;
> > + return i8k_smm(&regs) ? : (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)
>
> Might as well drop the if statement here.
>
Never mind, I see that is removed later anyway.

Guenter

> > data->i8k_fan_mult = fan_mult;
> > - }
> >
> > ret = dell_smm_init_hwmon(&pdev->dev);
> > if (ret)
> > --
> > 2.30.2
> >

\
 
 \ /
  Last update: 2022-04-26 22:35    [W:0.073 / U:0.268 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site