On Monday 27 September 2021 00:10:43 W_Armin@xxxxxx wrote: > From: Armin Wolf <W_Armin@xxxxxx> > > The nominal speed of each fan can be obtained with > i8k_get_fan_nominal_speed(), however the result is not available > from userspace. > Change that by adding fanX_min, fanX_max and fanX_target attributes. > All are RO since fan control happens over pwm. > > Tested on a Dell Inspiron 3505 and a Dell Latitude C600. > > Signed-off-by: Armin Wolf <W_Armin@xxxxxx> > --- > Documentation/hwmon/dell-smm-hwmon.rst | 3 ++ > drivers/hwmon/dell-smm-hwmon.c | 61 +++++++++++++++++++++++--- > 2 files changed, 58 insertions(+), 6 deletions(-) > > diff --git a/Documentation/hwmon/dell-smm-hwmon.rst b/Documentation/hwmon/dell-smm-hwmon.rst > index 3bf77a5df995..beec88491171 100644 > --- a/Documentation/hwmon/dell-smm-hwmon.rst > +++ b/Documentation/hwmon/dell-smm-hwmon.rst > @@ -34,6 +34,9 @@ Name Perm Description > =============================== ======= ======================================= > fan[1-3]_input RO Fan speed in RPM. > fan[1-3]_label RO Fan label. > +fan[1-3]_min RO Minimal Fan speed in RPM > +fan[1-3]_max RO Maximal Fan speed in RPM > +fan[1-3]_target RO Expected Fan speed in RPM Hello! I do not know API / meaning of these 3 properties, so I looked into hwmon documentation at: https://www.kernel.org/doc/html/latest/hwmon/sysfs-interface.html#fans And in documentation is written that both 3 properties (min, max and target) are RW. I'm somehow lost as I have not fully understood what is the original meaning of these 3 properties. Guenter could you help? After reading I understood it as that these properties are for HW which supports controlling directly fan speed via RPM (and not PWM). And so user can set lower and upper limit of fan speed (via _min and _max) or can set exact RPM value (via _target). > pwm[1-3] RW Control the fan PWM duty-cycle. > pwm1_enable WO Enable or disable automatic BIOS fan > control (not supported on all laptops, > diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c > index 774c1b0715d9..476f2a74bd8c 100644 > --- a/drivers/hwmon/dell-smm-hwmon.c > +++ b/drivers/hwmon/dell-smm-hwmon.c > @@ -76,6 +76,7 @@ struct dell_smm_data { > int temp_type[DELL_SMM_NO_TEMP]; > bool fan[DELL_SMM_NO_FANS]; > int fan_type[DELL_SMM_NO_FANS]; > + int *fan_nominal_speed[DELL_SMM_NO_FANS]; > }; > > MODULE_AUTHOR("Massimo Dal Zotto (dz@xxxxxxxxxx)"); > @@ -673,6 +674,13 @@ static umode_t dell_smm_is_visible(const void *drvdata, enum hwmon_sensor_types > if (data->fan[channel] && !data->disallow_fan_type_call) > return 0444; > > + break; > + case hwmon_fan_min: > + case hwmon_fan_max: > + case hwmon_fan_target: > + if (data->fan_nominal_speed[channel]) > + return 0444; > + > break; > default: > break; > @@ -740,6 +748,25 @@ static int dell_smm_read(struct device *dev, enum hwmon_sensor_types type, u32 a > > *val = ret; > > + return 0; > + case hwmon_fan_min: > + *val = data->fan_nominal_speed[channel][0]; When fan is turned off then it has speed 0 RPM. So should not be minimal fan speed always hardcoded to zero? > + > + return 0; > + case hwmon_fan_max: > + *val = data->fan_nominal_speed[channel][data->i8k_fan_max]; > + > + return 0; > + case hwmon_fan_target: > + ret = i8k_get_fan_status(data, channel); > + if (ret < 0) > + return ret; > + > + if (ret > data->i8k_fan_max) > + ret = data->i8k_fan_max; > + > + *val = data->fan_nominal_speed[channel][ret]; > + > return 0; > default: > break; > @@ -889,9 +916,12 @@ static const struct hwmon_channel_info *dell_smm_info[] = { > HWMON_T_INPUT | HWMON_T_LABEL > ), > HWMON_CHANNEL_INFO(fan, > - HWMON_F_INPUT | HWMON_F_LABEL, > - HWMON_F_INPUT | HWMON_F_LABEL, > - HWMON_F_INPUT | HWMON_F_LABEL > + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MIN | HWMON_F_MAX | > + HWMON_F_TARGET, > + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MIN | HWMON_F_MAX | > + HWMON_F_TARGET, > + HWMON_F_INPUT | HWMON_F_LABEL | HWMON_F_MIN | HWMON_F_MAX | > + HWMON_F_TARGET > ), > HWMON_CHANNEL_INFO(pwm, > HWMON_PWM_INPUT | HWMON_PWM_ENABLE, > @@ -910,7 +940,7 @@ static int __init dell_smm_init_hwmon(struct device *dev) > { > struct dell_smm_data *data = dev_get_drvdata(dev); > struct device *dell_smm_hwmon_dev; > - int i, err; > + int i, state, err; > > for (i = 0; i < DELL_SMM_NO_TEMP; i++) { > data->temp_type[i] = i8k_get_temp_type(i); > @@ -926,8 +956,27 @@ static int __init dell_smm_init_hwmon(struct device *dev) > err = i8k_get_fan_status(data, i); > if (err < 0) > err = i8k_get_fan_type(data, i); > - if (err >= 0) > - data->fan[i] = true; > + > + if (err < 0) > + continue; > + > + data->fan[i] = true; > + data->fan_nominal_speed[i] = devm_kmalloc_array(dev, data->i8k_fan_max + 1, > + sizeof(*data->fan_nominal_speed[i]), > + GFP_KERNEL); > + if (!data->fan_nominal_speed[i]) > + continue; > + > + for (state = 0; state <= data->i8k_fan_max; state++) { > + err = i8k_get_fan_nominal_speed(data, i, state); > + if (err < 0) { > + /* Mark nominal speed table as invalid in case of error */ > + devm_kfree(dev, data->fan_nominal_speed[i]); > + data->fan_nominal_speed[i] = NULL; > + break; > + } > + data->fan_nominal_speed[i][state] = err; > + } > } > > dell_smm_hwmon_dev = devm_hwmon_device_register_with_info(dev, "dell_smm", data, > -- > 2.20.1 >
