Re: [PATCH v2] hwmon: (scpi-hwmon) shows the negative temperature properly

[Guenter Roeck]

On Thu, Jun 03, 2021 at 04:38:45PM +0800, Riwen Lu wrote:
> The scpi hwmon shows the sub-zero temperature in an unsigned integer,
> which would confuse the users when the machine works in low temperature
> environment. This shows the sub-zero temperature in an signed value and
> users can get it properly from sensors.
> 
> Signed-off-by: Riwen Lu <luriwen@xxxxxxxxxx>
> Tested-by: Xin Chen <chenxin@xxxxxxxxxx>

Please ask Xin Chen to send a separate Tested-by: tag.

> 
> ---
> Changes since v1:
> - Add judgment for sensor->info.class. If it is TEMPERATURE situation,
>   return the sensor value as a signed value, otherwise return it as a
>   unsigned value.
> ---
>  drivers/hwmon/scpi-hwmon.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c
> index 25aac40f2764..45a7e319143d 100644
> --- a/drivers/hwmon/scpi-hwmon.c
> +++ b/drivers/hwmon/scpi-hwmon.c
> @@ -99,7 +99,10 @@ scpi_show_sensor(struct device *dev, struct device_attribute *attr, char *buf)
>  
>  	scpi_scale_reading(&value, sensor);
>  
> -	return sprintf(buf, "%llu\n", value);
> +	if (sensor->info.class == TEMPERATURE)
> +		return sprintf(buf, "%lld\n", value);

'value' is u64, so it needs a typecast to s64.

Also, please add a comment ahead of the if statement explaining that temperature
sensor values are treated as signed values based on observation even though that
is not explicitly specified, and because an unsigned u64 temperature does not
really make practical sense.

> +	else

else after return is not needed

Thanks,
Guenter

> +		return sprintf(buf, "%llu\n", value);
>  }
>  
>  static ssize_t
> -- 
> 2.25.1
> 
> 
> No virus found
> 		Checked by Hillstone Network AntiVirus