Re: [PATCH v8 12/14] iio: afe: rescale: add temperature transducers

[Peter Rosin <peda@axentia.se>]

On 2021-08-20 21:17, Liam Beguin wrote:
> From: Liam Beguin <lvb@xiphos.com>
> 
> A temperature transducer is a device that converts a thermal quantity
> into any other physical quantity. This patch add support for temperature
> to voltage (like the LTC2997) and temperature to current (like the
> AD590) linear transducers.
> In both cases these are assumed to be connected to a voltage ADC.
> 
> Signed-off-by: Liam Beguin <lvb@xiphos.com>
> ---
>  drivers/iio/afe/iio-rescale.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c
> index 8cdcb6ffb563..12de44058bea 100644
> --- a/drivers/iio/afe/iio-rescale.c
> +++ b/drivers/iio/afe/iio-rescale.c
> @@ -427,11 +427,38 @@ static int rescale_temp_sense_rtd_props(struct device *dev,
>  	return 0;
>  }
>  
> +static int rescale_temp_transducer_props(struct device *dev,
> +					 struct rescale *rescale)
> +{
> +	s32 offset = 0;
> +	s32 sense = 1;
> +	s32 alpha;
> +	s64 tmp;
> +	int ret;
> +
> +	device_property_read_u32(dev, "sense-offset-millicelsius", &offset);
> +	device_property_read_u32(dev, "sense-resistor-ohms", &sense);
> +	ret = device_property_read_u32(dev, "alpha-ppm-per-celsius", &alpha);
> +	if (ret) {
> +		dev_err(dev, "failed to read alpha-ppm-per-celsius: %d\n", ret);
> +		return ret;
> +	}
> +
> +	rescale->numerator = 1000000;
> +	rescale->denominator = alpha * sense;
> +
> +	tmp = (s64)offset * (s64)alpha * (s64)sense;
> +	rescale->offset = div_s64(tmp, (s32)1000000);

Error: Too many casts :-)

I think it would make sense to lose tmp, and just spell it out in one
statement?

	rescale->offset = div_s64((s64)offset * rescale->denominator,
				  rescale->numerator);

Because you are prepping the offset so that it will survive a later rescaler
multiplication, and all the "random" multiplications and divisions don't
make that very clear.

Cheers,
Peter

> +
> +	return 0;
> +}
> +
>  enum rescale_variant {
>  	CURRENT_SENSE_AMPLIFIER,
>  	CURRENT_SENSE_SHUNT,
>  	VOLTAGE_DIVIDER,
>  	TEMP_SENSE_RTD,
> +	TEMP_TRANSDUCER,
>  };
>  
>  static const struct rescale_cfg rescale_cfg[] = {
> @@ -451,6 +478,10 @@ static const struct rescale_cfg rescale_cfg[] = {
>  		.type = IIO_TEMP,
>  		.props = rescale_temp_sense_rtd_props,
>  	},
> +	[TEMP_TRANSDUCER] = {
> +		.type = IIO_TEMP,
> +		.props = rescale_temp_transducer_props,
> +	},
>  };
>  
>  static const struct of_device_id rescale_match[] = {
> @@ -462,6 +493,8 @@ static const struct of_device_id rescale_match[] = {
>  	  .data = &rescale_cfg[VOLTAGE_DIVIDER], },
>  	{ .compatible = "temperature-sense-rtd",
>  	  .data = &rescale_cfg[TEMP_SENSE_RTD], },
> +	{ .compatible = "temperature-transducer",
> +	  .data = &rescale_cfg[TEMP_TRANSDUCER], },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, rescale_match);
>