| Message ID | 20220204163045.576903-1-eugene.shalygin@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | hwmon: (asus-ec-sensors) read sensors as signed ints | expand |
On Fri, Feb 04, 2022 at 05:30:45PM +0100, Eugene Shalygin wrote: > Temperature sensor readings are signed, which is hinted by their blank > value (oxd8, 216 as unsigned and -40 as signed). T_Sensor, Crosshair > VIII Hero, and a freezer were used to confirm that. > > Here we read fan sensors as signed too, because with their typical > values and 2-byte width, I can't tell a difference between signed and > unsigned, as I don't have a high speed chipset fan. > Hmm, I hope we won't get a situation where an unsigned is reported and needed, but I guess we can deal with that if/when it happens. > Signed-off-by: Eugene Shalygin <eugene.shalygin@gmail.com> Applied. Thanks, Guenter > --- > drivers/hwmon/asus-ec-sensors.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/hwmon/asus-ec-sensors.c b/drivers/hwmon/asus-ec-sensors.c > index 05244209c0c6..8a04c76527a4 100644 > --- a/drivers/hwmon/asus-ec-sensors.c > +++ b/drivers/hwmon/asus-ec-sensors.c > @@ -221,7 +221,7 @@ static const struct dmi_system_id asus_ec_dmi_table[] __initconst = { > > struct ec_sensor { > unsigned int info_index; > - u32 cached_value; > + s32 cached_value; > }; > > struct ec_sensors_data { > @@ -408,15 +408,15 @@ static int asus_ec_block_read(const struct device *dev, > return status; > } > > -static inline u32 get_sensor_value(const struct ec_sensor_info *si, u8 *data) > +static inline s32 get_sensor_value(const struct ec_sensor_info *si, u8 *data) > { > switch (si->addr.components.size) { > case 1: > - return *data; > + return (s8)*data; > case 2: > - return get_unaligned_be16(data); > + return (s16)get_unaligned_be16(data); > case 4: > - return get_unaligned_be32(data); > + return (s32)get_unaligned_be32(data); > default: > return 0; > } > @@ -462,7 +462,7 @@ static int update_ec_sensors(const struct device *dev, > return status; > } > > -static int scale_sensor_value(u32 value, int data_type) > +static long scale_sensor_value(s32 value, int data_type) > { > switch (data_type) { > case hwmon_curr: > @@ -476,7 +476,7 @@ static int scale_sensor_value(u32 value, int data_type) > > static int get_cached_value_or_update(const struct device *dev, > int sensor_index, > - struct ec_sensors_data *state, u32 *value) > + struct ec_sensors_data *state, s32 *value) > { > if (time_after(jiffies, state->last_updated + HZ)) { > if (update_ec_sensors(dev, state)) { > @@ -499,7 +499,7 @@ static int asus_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type, > u32 attr, int channel, long *val) > { > int ret; > - u32 value = 0; > + s32 value = 0; > > struct ec_sensors_data *state = dev_get_drvdata(dev); > int sidx = find_ec_sensor_index(state, type, channel);
> Hmm, I hope we won't get a situation where an unsigned is reported > and needed, but I guess we can deal with that if/when it happens. There is still one byte left in the sensor address structure, which I planned to put data format flags into, and the signees can be one of those. I know that interpreting the reading for the VRM temperature misses something, because while the reported value matches the one reported by the ASUS software when the system thermal state is close to a thermal equilibrium, right after booting its value (at least with my board) is lower than the ambient temperature. Eugene
diff --git a/drivers/hwmon/asus-ec-sensors.c b/drivers/hwmon/asus-ec-sensors.c index 05244209c0c6..8a04c76527a4 100644 --- a/drivers/hwmon/asus-ec-sensors.c +++ b/drivers/hwmon/asus-ec-sensors.c @@ -221,7 +221,7 @@ static const struct dmi_system_id asus_ec_dmi_table[] __initconst = { struct ec_sensor { unsigned int info_index; - u32 cached_value; + s32 cached_value; }; struct ec_sensors_data { @@ -408,15 +408,15 @@ static int asus_ec_block_read(const struct device *dev, return status; } -static inline u32 get_sensor_value(const struct ec_sensor_info *si, u8 *data) +static inline s32 get_sensor_value(const struct ec_sensor_info *si, u8 *data) { switch (si->addr.components.size) { case 1: - return *data; + return (s8)*data; case 2: - return get_unaligned_be16(data); + return (s16)get_unaligned_be16(data); case 4: - return get_unaligned_be32(data); + return (s32)get_unaligned_be32(data); default: return 0; } @@ -462,7 +462,7 @@ static int update_ec_sensors(const struct device *dev, return status; } -static int scale_sensor_value(u32 value, int data_type) +static long scale_sensor_value(s32 value, int data_type) { switch (data_type) { case hwmon_curr: @@ -476,7 +476,7 @@ static int scale_sensor_value(u32 value, int data_type) static int get_cached_value_or_update(const struct device *dev, int sensor_index, - struct ec_sensors_data *state, u32 *value) + struct ec_sensors_data *state, s32 *value) { if (time_after(jiffies, state->last_updated + HZ)) { if (update_ec_sensors(dev, state)) { @@ -499,7 +499,7 @@ static int asus_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type, u32 attr, int channel, long *val) { int ret; - u32 value = 0; + s32 value = 0; struct ec_sensors_data *state = dev_get_drvdata(dev); int sidx = find_ec_sensor_index(state, type, channel);
Temperature sensor readings are signed, which is hinted by their blank value (oxd8, 216 as unsigned and -40 as signed). T_Sensor, Crosshair VIII Hero, and a freezer were used to confirm that. Here we read fan sensors as signed too, because with their typical values and 2-byte width, I can't tell a difference between signed and unsigned, as I don't have a high speed chipset fan. Signed-off-by: Eugene Shalygin <eugene.shalygin@gmail.com> --- drivers/hwmon/asus-ec-sensors.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)