diff mbox series

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

Message ID 20210820191714.69898-13-liambeguin@gmail.com (mailing list archive)
State Changes Requested
Headers show
Series iio: afe: add temperature rescaling support | expand

Commit Message

Liam Beguin Aug. 20, 2021, 7:17 p.m. UTC 
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(+)

Comments

Peter Rosin Aug. 26, 2021, 8:56 a.m. UTC | #1 
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);
>
Liam Beguin Aug. 29, 2021, 2:33 a.m. UTC | #2 
On Thu, Aug 26, 2021 at 10:56:11AM +0200, Peter Rosin wrote:
> 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 :-)

Oof! You're right, that doesn't look too good...

I guess I haven't looked at this part of the series for a few
revisions, my bad.

> 
> 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.
> 

Yeah, I agree! The way I had it spelled out wasn't much more readable.
I'll use your suggestion instead.

Thanks,
Liam

> 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);
> >
diff mbox series

Patch

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);
+
+	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);