| Message ID | 20221224041855.83981-2-W_Armin@gmx.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | hwmon: (ftsteutates) Various fixes | expand |
On Sat, Dec 24, 2022 at 05:18:53AM +0100, Armin Wolf wrote: > A user complained that the ftsteutates driver was displaying > bogus values since its introduction. This happens because the > sensor measurements need to be scaled in order to produce > meaningful results: > - the fan speed needs to be multiplied by 60 since its in RPS > - the temperature is in degrees celsius and needs an offset of 64 > - the voltage is in 1/256 of 3.3V > > The offical datasheet says the voltage needs to be divided by 256, > but this is likely an off-by-one-error, since even the BIOS > devides by 255 (otherwise 3.3V could not be measured). > > The voltage channels additionally need a board-specific multiplier, > however this can be done by the driver since its board-specific. > > The reason the missing scaling of measurements is the way Fujitsu > used this driver when it was still out-of-tree. Back then, all > scaling was done in userspace by libsensors, even the generic one. > > Tested on a Fujitsu DS3401-B1. > > Fixes: 08426eda58e0 ("hwmon: Add driver for FTS BMC chip "Teutates"") > Signed-off-by: Armin Wolf <W_Armin@gmx.de> Applied. Thanks, Guenter > --- > Documentation/hwmon/ftsteutates.rst | 4 ++++ > drivers/hwmon/ftsteutates.c | 19 +++++++++++++------ > 2 files changed, 17 insertions(+), 6 deletions(-) > > -- > 2.30.2 > > diff --git a/Documentation/hwmon/ftsteutates.rst b/Documentation/hwmon/ftsteutates.rst > index 58a2483d8d0d..198fa8e2819d 100644 > --- a/Documentation/hwmon/ftsteutates.rst > +++ b/Documentation/hwmon/ftsteutates.rst > @@ -22,6 +22,10 @@ enhancements. It can monitor up to 4 voltages, 16 temperatures and > 8 fans. It also contains an integrated watchdog which is currently > implemented in this driver. > > +The 4 voltages require a board-specific multiplier, since the BMC can > +only measure voltages up to 3.3V and thus relies on voltage dividers. > +Consult your motherboard manual for details. > + > To clear a temperature or fan alarm, execute the following command with the > correct path to the alarm file:: > > diff --git a/drivers/hwmon/ftsteutates.c b/drivers/hwmon/ftsteutates.c > index f5b8e724a8ca..ffa0bb364877 100644 > --- a/drivers/hwmon/ftsteutates.c > +++ b/drivers/hwmon/ftsteutates.c > @@ -12,6 +12,7 @@ > #include <linux/i2c.h> > #include <linux/init.h> > #include <linux/jiffies.h> > +#include <linux/math.h> > #include <linux/module.h> > #include <linux/mutex.h> > #include <linux/slab.h> > @@ -347,13 +348,15 @@ static ssize_t in_value_show(struct device *dev, > { > struct fts_data *data = dev_get_drvdata(dev); > int index = to_sensor_dev_attr(devattr)->index; > - int err; > + int value, err; > > err = fts_update_device(data); > if (err < 0) > return err; > > - return sprintf(buf, "%u\n", data->volt[index]); > + value = DIV_ROUND_CLOSEST(data->volt[index] * 3300, 255); > + > + return sprintf(buf, "%d\n", value); > } > > static ssize_t temp_value_show(struct device *dev, > @@ -361,13 +364,15 @@ static ssize_t temp_value_show(struct device *dev, > { > struct fts_data *data = dev_get_drvdata(dev); > int index = to_sensor_dev_attr(devattr)->index; > - int err; > + int value, err; > > err = fts_update_device(data); > if (err < 0) > return err; > > - return sprintf(buf, "%u\n", data->temp_input[index]); > + value = (data->temp_input[index] - 64) * 1000; > + > + return sprintf(buf, "%d\n", value); > } > > static ssize_t temp_fault_show(struct device *dev, > @@ -436,13 +441,15 @@ static ssize_t fan_value_show(struct device *dev, > { > struct fts_data *data = dev_get_drvdata(dev); > int index = to_sensor_dev_attr(devattr)->index; > - int err; > + int value, err; > > err = fts_update_device(data); > if (err < 0) > return err; > > - return sprintf(buf, "%u\n", data->fan_input[index]); > + value = data->fan_input[index] * 60; > + > + return sprintf(buf, "%d\n", value); > } > > static ssize_t fan_source_show(struct device *dev,
diff --git a/Documentation/hwmon/ftsteutates.rst b/Documentation/hwmon/ftsteutates.rst index 58a2483d8d0d..198fa8e2819d 100644 --- a/Documentation/hwmon/ftsteutates.rst +++ b/Documentation/hwmon/ftsteutates.rst @@ -22,6 +22,10 @@ enhancements. It can monitor up to 4 voltages, 16 temperatures and 8 fans. It also contains an integrated watchdog which is currently implemented in this driver. +The 4 voltages require a board-specific multiplier, since the BMC can +only measure voltages up to 3.3V and thus relies on voltage dividers. +Consult your motherboard manual for details. + To clear a temperature or fan alarm, execute the following command with the correct path to the alarm file:: diff --git a/drivers/hwmon/ftsteutates.c b/drivers/hwmon/ftsteutates.c index f5b8e724a8ca..ffa0bb364877 100644 --- a/drivers/hwmon/ftsteutates.c +++ b/drivers/hwmon/ftsteutates.c @@ -12,6 +12,7 @@ #include <linux/i2c.h> #include <linux/init.h> #include <linux/jiffies.h> +#include <linux/math.h> #include <linux/module.h> #include <linux/mutex.h> #include <linux/slab.h> @@ -347,13 +348,15 @@ static ssize_t in_value_show(struct device *dev, { struct fts_data *data = dev_get_drvdata(dev); int index = to_sensor_dev_attr(devattr)->index; - int err; + int value, err; err = fts_update_device(data); if (err < 0) return err; - return sprintf(buf, "%u\n", data->volt[index]); + value = DIV_ROUND_CLOSEST(data->volt[index] * 3300, 255); + + return sprintf(buf, "%d\n", value); } static ssize_t temp_value_show(struct device *dev, @@ -361,13 +364,15 @@ static ssize_t temp_value_show(struct device *dev, { struct fts_data *data = dev_get_drvdata(dev); int index = to_sensor_dev_attr(devattr)->index; - int err; + int value, err; err = fts_update_device(data); if (err < 0) return err; - return sprintf(buf, "%u\n", data->temp_input[index]); + value = (data->temp_input[index] - 64) * 1000; + + return sprintf(buf, "%d\n", value); } static ssize_t temp_fault_show(struct device *dev, @@ -436,13 +441,15 @@ static ssize_t fan_value_show(struct device *dev, { struct fts_data *data = dev_get_drvdata(dev); int index = to_sensor_dev_attr(devattr)->index; - int err; + int value, err; err = fts_update_device(data); if (err < 0) return err; - return sprintf(buf, "%u\n", data->fan_input[index]); + value = data->fan_input[index] * 60; + + return sprintf(buf, "%d\n", value); } static ssize_t fan_source_show(struct device *dev,
A user complained that the ftsteutates driver was displaying bogus values since its introduction. This happens because the sensor measurements need to be scaled in order to produce meaningful results: - the fan speed needs to be multiplied by 60 since its in RPS - the temperature is in degrees celsius and needs an offset of 64 - the voltage is in 1/256 of 3.3V The offical datasheet says the voltage needs to be divided by 256, but this is likely an off-by-one-error, since even the BIOS devides by 255 (otherwise 3.3V could not be measured). The voltage channels additionally need a board-specific multiplier, however this can be done by the driver since its board-specific. The reason the missing scaling of measurements is the way Fujitsu used this driver when it was still out-of-tree. Back then, all scaling was done in userspace by libsensors, even the generic one. Tested on a Fujitsu DS3401-B1. Fixes: 08426eda58e0 ("hwmon: Add driver for FTS BMC chip "Teutates"") Signed-off-by: Armin Wolf <W_Armin@gmx.de> --- Documentation/hwmon/ftsteutates.rst | 4 ++++ drivers/hwmon/ftsteutates.c | 19 +++++++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-) -- 2.30.2