Re: [PATCH] hwmon: (asus-ec-sensors) deduce sensor signess from its type

[Guenter Roeck]

On 2/10/22 16:36, Eugene Shalygin wrote:
> Reading DSDT code for ASUS X470-based boards (the ones served by the
> asus_wmi_Sensors driver), where ASUS put hardware monitoring functions
> into the WMI code, reveals that fan and current sensors data is
> unsigned. For the current sensor that was cofirmed by a user who showed
> high enough current value for overflow.
>
> Thus let's assume that the signess of the sensors is determined by its

signedness

> type and that only temperature ones provide signed numbers.
>
> Signed-off-by: Eugene Shalygin <eugene.shalygin@xxxxxxxxx>
> ---
>   drivers/hwmon/asus-ec-sensors.c | 38 +++++++++++++++++++++++++--------
>   1 file changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/hwmon/asus-ec-sensors.c b/drivers/hwmon/asus-ec-sensors.c
> index bfac08a5dc57..a1b13fe149ac 100644
> --- a/drivers/hwmon/asus-ec-sensors.c
> +++ b/drivers/hwmon/asus-ec-sensors.c
> @@ -266,6 +266,13 @@ static u8 register_index(u16 reg)
>   	return reg & 0x00ff;
>   }
>   
> +static bool is_sensor_data_signed(const struct ec_sensor_info *si)
> +{
> +	// guessed from WMI functions in DSDT code for boards
> +	// of the X470 generation

/*
 * Please lets stick with standard multi-line comments
 */

> +	return si->type == hwmon_temp;
> +}
> +
>   static const struct ec_sensor_info *
>   get_sensor_info(const struct ec_sensors_data *state, int index)
>   {
> @@ -420,15 +427,28 @@ static int asus_ec_block_read(const struct device *dev,
>   
>   static inline s32 get_sensor_value(const struct ec_sensor_info *si, u8 *data)
>   {
> -	switch (si->addr.components.size) {
> -	case 1:
> -		return (s8)*data;
> -	case 2:
> -		return (s16)get_unaligned_be16(data);
> -	case 4:
> -		return (s32)get_unaligned_be32(data);
> -	default:
> -		return 0;
> +	if (is_sensor_data_signed(si)) {
> +		switch (si->addr.components.size) {
> +		case 1:
> +			return (s8)*data;
> +		case 2:
> +			return (s16)get_unaligned_be16(data);
> +		case 4:
> +			return (s32)get_unaligned_be32(data);
> +		default:
> +			return 0;
> +		}
> +	} else {
> +		switch (si->addr.components.size) {
> +		case 1:
> +			return *data;
> +		case 2:
> +			return get_unaligned_be16(data);
> +		case 4:
> +			return get_unaligned_be32(data);
> +		default:
> +			return 0;
> +		}
>   	}
>   }
>