[PATCH v1 0/9] iio: afe: add temperature rescaling support

[PATCH v1 0/9] iio: afe: add temperature rescaling support

[Liam Beguin]

From: Liam Beguin <lvb@xiphos.com>

Add temperature rescaling support to the IIO Analog Front End driver.

This series adds support for three kinds of temperature front end
circuits:
- temperature-sense-rtd for which the resistance is proportional to the
  temperature
- temperature-sense-current for which the current is proportional to the
  temperature
- temperature-sense-amplifier for which the voltage is proportional to
  the temperature

At first I tried to use iio_convert_raw_to_processed() to get more
precision out of processed values but ran into issues when one of my
ADCs didn't provide a scale. I tried to address this in first patch.

When adding offset support to iio-rescale, I also noticed that
iio_read_channel_processed() assumes that the offset is always an
integer which I tried to address in the second patch.
If it makes sense to add support for fractional offsets, I could give it
a try in another series.

Related to: https://patchwork.kernel.org/project/linux-iio/list/?series=483087

Thanks for your time

Liam Beguin (9):
  iio: inkern: always apply scale requested by consumer
  iio: inkern: error out on unsupported offset type
  iio: afe: rescale: use core to get processed value
  iio: afe: rescale: add offset support
  iio: afe: rescale: add support for temperature sensors
  dt-bindings: iio: afe: update MAINTAINERS file
  dt-bindings: iio: afe: add binding for temperature-sense-rtd
  dt-bindings: iio: afe: add binding for temperature-sense-current
  dt-bindings: iio: afe: add binding for temperature-sense-amplifier

 .../iio/afe/temperature-sense-amplifier.yaml  |  55 +++++
 .../iio/afe/temperature-sense-current.yaml    |  61 ++++++
 .../iio/afe/temperature-sense-rtd.yaml        |  65 ++++++
 MAINTAINERS                                   |   9 +-
 drivers/iio/afe/iio-rescale.c                 | 190 +++++++++++++++---
 drivers/iio/inkern.c                          |  12 +-
 6 files changed, 354 insertions(+), 38 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/afe/temperature-sense-amplifier.yaml
 create mode 100644 Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml
 create mode 100644 Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml


base-commit: 9f4ad9e425a1d3b6a34617b8ea226d56a119a717
-- 
2.30.1.489.g328c10930387

[PATCH v1 1/9] iio: inkern: always apply scale requested by consumer

[Liam Beguin]

From: Liam Beguin <lvb@xiphos.com>

When a consumer calls iio_read_channel_processed() and no scaling is
available on the channel, it's assumed that the scale is one and the raw
value is returned as expected.

On the other hand, if the consumer calls iio_convert_raw_to_processed()
the scaling factor requested by the consumer is not applied.
This for example causes the consumer to process mV when expecting uV.

Make sure to always apply the scaling factor requested by the consumer.

Signed-off-by: Liam Beguin <lvb@xiphos.com>
---
 drivers/iio/inkern.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index db77a2d4a56b..4b6a8e11116a 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -601,11 +601,7 @@ static int iio_convert_raw_to_processed_unlocked(struct iio_channel *chan,
 	scale_type = iio_channel_read(chan, &scale_val, &scale_val2,
 					IIO_CHAN_INFO_SCALE);
 	if (scale_type < 0) {
-		/*
-		 * Just pass raw values as processed if no scaling is
-		 * available.
-		 */
-		*processed = raw;
+		*processed = raw * scale;
 		return 0;
 	}
 
-- 
2.30.1.489.g328c10930387

Re: [PATCH v1 1/9] iio: inkern: always apply scale requested by consumer

[Peter Rosin]

Hi!

On 2021-05-30 02:59, Liam Beguin wrote:
> From: Liam Beguin <lvb@xiphos.com>
> 
> When a consumer calls iio_read_channel_processed() and no scaling is
> available on the channel, it's assumed that the scale is one and the raw
> value is returned as expected.
> 
> On the other hand, if the consumer calls iio_convert_raw_to_processed()
> the scaling factor requested by the consumer is not applied.
> This for example causes the consumer to process mV when expecting uV.
> 
> Make sure to always apply the scaling factor requested by the consumer.
> 
> Signed-off-by: Liam Beguin <lvb@xiphos.com>
> ---
>  drivers/iio/inkern.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index db77a2d4a56b..4b6a8e11116a 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -601,11 +601,7 @@ static int iio_convert_raw_to_processed_unlocked(struct iio_channel *chan,
>  	scale_type = iio_channel_read(chan, &scale_val, &scale_val2,
>  					IIO_CHAN_INFO_SCALE);
>  	if (scale_type < 0) {
> -		/*
> -		 * Just pass raw values as processed if no scaling is
> -		 * available.
> -		 */
> -		*processed = raw;
> +		*processed = raw * scale;

I would keep the comment. Sure, it's now completely confusing since it's from
before the function had a scale parameter. Perhaps reword it to talk about
"no channel scaling" instead of plain old "no scaling"?

Also, this looks like a bugfix, no more, no less, and should perhaps have a
fixes tag?

Cheers,
Peter

>  		return 0;
>  	}
>  
> 

[PATCH v1 2/9] iio: inkern: error out on unsupported offset type

[Liam Beguin]

From: Liam Beguin <lvb@xiphos.com>

iio_convert_raw_to_processed_unlocked() assumes the offset is an
integer. Make that clear to the consumer by returning an error when an
unsupported offset type is detected.

Signed-off-by: Liam Beguin <lvb@xiphos.com>
---
 drivers/iio/inkern.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index 4b6a8e11116a..dede4536d499 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -595,8 +595,12 @@ static int iio_convert_raw_to_processed_unlocked(struct iio_channel *chan,
 	int ret;
 
 	ret = iio_channel_read(chan, &offset, NULL, IIO_CHAN_INFO_OFFSET);
-	if (ret >= 0)
+	if (ret == IIO_VAL_INT) {
 		raw64 += offset;
+	} else if (ret >= 0) {
+		dev_err(&chan->indio_dev->dev, "unsupported offset type");
+		return -EINVAL;
+	}
 
 	scale_type = iio_channel_read(chan, &scale_val, &scale_val2,
 					IIO_CHAN_INFO_SCALE);
-- 
2.30.1.489.g328c10930387

Re: [PATCH v1 2/9] iio: inkern: error out on unsupported offset type

[Peter Rosin]

Hi!

On 2021-05-30 02:59, Liam Beguin wrote:
> From: Liam Beguin <lvb@xiphos.com>
> 
> iio_convert_raw_to_processed_unlocked() assumes the offset is an
> integer. Make that clear to the consumer by returning an error when an
> unsupported offset type is detected.
> 
> Signed-off-by: Liam Beguin <lvb@xiphos.com>
> ---
>  drivers/iio/inkern.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 4b6a8e11116a..dede4536d499 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -595,8 +595,12 @@ static int iio_convert_raw_to_processed_unlocked(struct iio_channel *chan,
>  	int ret;
>  
>  	ret = iio_channel_read(chan, &offset, NULL, IIO_CHAN_INFO_OFFSET);
> -	if (ret >= 0)
> +	if (ret == IIO_VAL_INT) {
>  		raw64 += offset;
> +	} else if (ret >= 0) {
> +		dev_err(&chan->indio_dev->dev, "unsupported offset type");
> +		return -EINVAL;
> +	}
>  
>  	scale_type = iio_channel_read(chan, &scale_val, &scale_val2,
>  					IIO_CHAN_INFO_SCALE);
> 

This breaks the implicit truncation that happens for drivers that have
offsets of type IIO_VAL_INT_PLUS_{MICRO_DB,MICRO,NANO}

Implicit truncation might be more appropriate than an error?

However, to error out on fractional offsets etc still seem appropriate, but
there are corner cases where the existing code did the right thing. E.g.
a denominator of one or a fractional-log2 of zero, but a big denominator and
a smaller numerator would also just result in a relatively harmless truncation.

I don't know if it's really right to just break that?

Cheers,
Peter

Re: [PATCH v1 2/9] iio: inkern: error out on unsupported offset type

[Liam Beguin]

Hi Peter,

On Mon May 31, 2021 at 5:45 AM EDT, Peter Rosin wrote:
> Hi!
>
> On 2021-05-30 02:59, Liam Beguin wrote:
> > From: Liam Beguin <lvb@xiphos.com>
> > 
> > iio_convert_raw_to_processed_unlocked() assumes the offset is an
> > integer. Make that clear to the consumer by returning an error when an
> > unsupported offset type is detected.
> > 
> > Signed-off-by: Liam Beguin <lvb@xiphos.com>
> > ---
> >  drivers/iio/inkern.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> > index 4b6a8e11116a..dede4536d499 100644
> > --- a/drivers/iio/inkern.c
> > +++ b/drivers/iio/inkern.c
> > @@ -595,8 +595,12 @@ static int iio_convert_raw_to_processed_unlocked(struct iio_channel *chan,
> >  	int ret;
> >  
> >  	ret = iio_channel_read(chan, &offset, NULL, IIO_CHAN_INFO_OFFSET);
> > -	if (ret >= 0)
> > +	if (ret == IIO_VAL_INT) {
> >  		raw64 += offset;
> > +	} else if (ret >= 0) {
> > +		dev_err(&chan->indio_dev->dev, "unsupported offset type");
> > +		return -EINVAL;
> > +	}
> >  
> >  	scale_type = iio_channel_read(chan, &scale_val, &scale_val2,
> >  					IIO_CHAN_INFO_SCALE);
> > 
>
> This breaks the implicit truncation that happens for drivers that have
> offsets of type IIO_VAL_INT_PLUS_{MICRO_DB,MICRO,NANO}
>
> Implicit truncation might be more appropriate than an error?
>
> However, to error out on fractional offsets etc still seem appropriate,
> but
> there are corner cases where the existing code did the right thing. E.g.
> a denominator of one or a fractional-log2 of zero, but a big denominator
> and
> a smaller numerator would also just result in a relatively harmless
> truncation.
>
> I don't know if it's really right to just break that?

Apologies for missing these. You're right that this change shouldn't
break what used to work implicitly. I'll rework this.

Liam

>
> Cheers,
> Peter

[PATCH v1 3/9] iio: afe: rescale: use core to get processed value

[Liam Beguin]

From: Liam Beguin <lvb@xiphos.com>

Make use of the IIO core to compute the source channel's processed
value. This reduces duplication and will facilitate the addition of
offsets in the iio-rescale driver.

Signed-off-by: Liam Beguin <lvb@xiphos.com>
---
 drivers/iio/afe/iio-rescale.c | 37 ++++++++++-------------------------
 1 file changed, 10 insertions(+), 27 deletions(-)

diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c
index e42ea2b1707d..4d0813b274d1 100644
--- a/drivers/iio/afe/iio-rescale.c
+++ b/drivers/iio/afe/iio-rescale.c
@@ -38,36 +38,20 @@ static int rescale_read_raw(struct iio_dev *indio_dev,
 			    int *val, int *val2, long mask)
 {
 	struct rescale *rescale = iio_priv(indio_dev);
-	unsigned long long tmp;
 	int ret;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		return iio_read_channel_raw(rescale->source, val);
+		ret = iio_read_channel_processed(rescale->source, val);
 
+		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
-		ret = iio_read_channel_scale(rescale->source, val, val2);
-		switch (ret) {
-		case IIO_VAL_FRACTIONAL:
-			*val *= rescale->numerator;
-			*val2 *= rescale->denominator;
-			return ret;
-		case IIO_VAL_INT:
-			*val *= rescale->numerator;
-			if (rescale->denominator == 1)
-				return ret;
-			*val2 = rescale->denominator;
-			return IIO_VAL_FRACTIONAL;
-		case IIO_VAL_FRACTIONAL_LOG2:
-			tmp = *val * 1000000000LL;
-			do_div(tmp, rescale->denominator);
-			tmp *= rescale->numerator;
-			do_div(tmp, 1000000000LL);
-			*val = tmp;
-			return ret;
-		default:
-			return -EOPNOTSUPP;
-		}
+		*val = rescale->numerator;
+		if (rescale->denominator == 1)
+			return IIO_VAL_INT;
+		*val2 = rescale->denominator;
+
+		return IIO_VAL_FRACTIONAL;
 	default:
 		return -EINVAL;
 	}
@@ -130,9 +114,8 @@ static int rescale_configure_channel(struct device *dev,
 	chan->ext_info = rescale->ext_info;
 	chan->type = rescale->cfg->type;
 
-	if (!iio_channel_has_info(schan, IIO_CHAN_INFO_RAW) ||
-	    !iio_channel_has_info(schan, IIO_CHAN_INFO_SCALE)) {
-		dev_err(dev, "source channel does not support raw/scale\n");
+	if (!iio_channel_has_info(schan, IIO_CHAN_INFO_RAW)) {
+		dev_err(dev, "source channel does not support raw\n");
 		return -EINVAL;
 	}
 
-- 
2.30.1.489.g328c10930387

Re: [PATCH v1 3/9] iio: afe: rescale: use core to get processed value

[Peter Rosin]

Hi!

Thanks for the patches. However, things have recently changed under your feet.
Can you please adjust to

https://patchwork.kernel.org/project/linux-iio/list/?series=484153
https://lore.kernel.org/linux-iio/20210518190201.26657c49@jic23-huawei/T/#m0de421cc9f6bc10bfa2622d65be750aaced3810c

and resend?

On 2021-05-30 02:59, Liam Beguin wrote:
> From: Liam Beguin <lvb@xiphos.com>
> 
> Make use of the IIO core to compute the source channel's processed
> value. This reduces duplication and will facilitate the addition of
> offsets in the iio-rescale driver.

Cheers,
Peter

Re: [PATCH v1 3/9] iio: afe: rescale: use core to get processed value

[Liam Beguin]

Hi Peter,

On Mon May 31, 2021 at 3:09 AM EDT, Peter Rosin wrote:
> Hi!
>
> Thanks for the patches. However, things have recently changed under your
> feet.
> Can you please adjust to
>
> https://patchwork.kernel.org/project/linux-iio/list/?series=484153
> https://lore.kernel.org/linux-iio/20210518190201.26657c49@jic23-huawei/T/#m0de421cc9f6bc10bfa2622d65be750aaced3810c
>
> and resend?

Thanks for pointing those out. I'll rebase on the latest -rc and resend.

Liam

>
> On 2021-05-30 02:59, Liam Beguin wrote:
> > From: Liam Beguin <lvb@xiphos.com>
> > 
> > Make use of the IIO core to compute the source channel's processed
> > value. This reduces duplication and will facilitate the addition of
> > offsets in the iio-rescale driver.
>
> Cheers,
> Peter

Re: [PATCH v1 3/9] iio: afe: rescale: use core to get processed value

[Jonathan Cameron]

On Sat, 29 May 2021 20:59:11 -0400
Liam Beguin <liambeguin@gmail.com> wrote:

> From: Liam Beguin <lvb@xiphos.com>
> 
> Make use of the IIO core to compute the source channel's processed
> value. This reduces duplication and will facilitate the addition of
> offsets in the iio-rescale driver.

Fairly sure you just dropped a lot or precision if the devices do provide
a scale.
> 
> Signed-off-by: Liam Beguin <lvb@xiphos.com>
> ---
>  drivers/iio/afe/iio-rescale.c | 37 ++++++++++-------------------------
>  1 file changed, 10 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c
> index e42ea2b1707d..4d0813b274d1 100644
> --- a/drivers/iio/afe/iio-rescale.c
> +++ b/drivers/iio/afe/iio-rescale.c
> @@ -38,36 +38,20 @@ static int rescale_read_raw(struct iio_dev *indio_dev,
>  			    int *val, int *val2, long mask)
>  {
>  	struct rescale *rescale = iio_priv(indio_dev);
> -	unsigned long long tmp;
>  	int ret;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
> -		return iio_read_channel_raw(rescale->source, val);
> +		ret = iio_read_channel_processed(rescale->source, val);

iio_read_channel_processed() provides a IIO_VAL_INT as you can see.
Now that can be heavily truncated.  In some cases it is always 0
(e.g. device reading very small currents only).

To maintain that scaling we deliberately combine it with the
scaling from the afe, hopefully maintaining that precision because
the scale value has much higher degree of precision.

We could probably do this better than currently with a more complex
conversion function.

It might seem like a good idea to fix up iio_read_channel_processed
to return IIO_VAL_INT_PLUS_MICRO or similar, but potentially that
would still throw away precision, for example if the scale is
expressed as IIO_VAL_FRACTIONAL to a high degree of precision.

Jonathan

>  
> +		return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_SCALE:
> -		ret = iio_read_channel_scale(rescale->source, val, val2);
> -		switch (ret) {
> -		case IIO_VAL_FRACTIONAL:
> -			*val *= rescale->numerator;
> -			*val2 *= rescale->denominator;
> -			return ret;
> -		case IIO_VAL_INT:
> -			*val *= rescale->numerator;
> -			if (rescale->denominator == 1)
> -				return ret;
> -			*val2 = rescale->denominator;
> -			return IIO_VAL_FRACTIONAL;
> -		case IIO_VAL_FRACTIONAL_LOG2:
> -			tmp = *val * 1000000000LL;
> -			do_div(tmp, rescale->denominator);
> -			tmp *= rescale->numerator;
> -			do_div(tmp, 1000000000LL);
> -			*val = tmp;
> -			return ret;
> -		default:
> -			return -EOPNOTSUPP;
> -		}
> +		*val = rescale->numerator;
> +		if (rescale->denominator == 1)
> +			return IIO_VAL_INT;
> +		*val2 = rescale->denominator;
> +
> +		return IIO_VAL_FRACTIONAL;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -130,9 +114,8 @@ static int rescale_configure_channel(struct device *dev,
>  	chan->ext_info = rescale->ext_info;
>  	chan->type = rescale->cfg->type;
>  
> -	if (!iio_channel_has_info(schan, IIO_CHAN_INFO_RAW) ||
> -	    !iio_channel_has_info(schan, IIO_CHAN_INFO_SCALE)) {
> -		dev_err(dev, "source channel does not support raw/scale\n");
> +	if (!iio_channel_has_info(schan, IIO_CHAN_INFO_RAW)) {
> +		dev_err(dev, "source channel does not support raw\n");
>  		return -EINVAL;
>  	}
>  

[PATCH v1 4/9] iio: afe: rescale: add offset support

[Liam Beguin]

From: Liam Beguin <lvb@xiphos.com>

This is a preparatory change required for the addition of temperature
sensing front ends.

Signed-off-by: Liam Beguin <lvb@xiphos.com>
---
 drivers/iio/afe/iio-rescale.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c
index 4d0813b274d1..3bd1f11f21db 100644
--- a/drivers/iio/afe/iio-rescale.c
+++ b/drivers/iio/afe/iio-rescale.c
@@ -31,6 +31,7 @@ struct rescale {
 	struct iio_chan_spec_ext_info *ext_info;
 	s32 numerator;
 	s32 denominator;
+	s32 offset;
 };
 
 static int rescale_read_raw(struct iio_dev *indio_dev,
@@ -52,6 +53,10 @@ static int rescale_read_raw(struct iio_dev *indio_dev,
 		*val2 = rescale->denominator;
 
 		return IIO_VAL_FRACTIONAL;
+	case IIO_CHAN_INFO_OFFSET:
+		*val = rescale->offset;
+
+		return IIO_VAL_INT;
 	default:
 		return -EINVAL;
 	}
@@ -119,8 +124,10 @@ static int rescale_configure_channel(struct device *dev,
 		return -EINVAL;
 	}
 
-	chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
-		BIT(IIO_CHAN_INFO_SCALE);
+	chan->info_mask_separate =
+		BIT(IIO_CHAN_INFO_RAW) |
+		BIT(IIO_CHAN_INFO_SCALE) |
+		BIT(IIO_CHAN_INFO_OFFSET);
 
 	if (iio_channel_has_available(schan, IIO_CHAN_INFO_RAW))
 		chan->info_mask_separate_available |= BIT(IIO_CHAN_INFO_RAW);
@@ -280,6 +287,7 @@ static int rescale_probe(struct platform_device *pdev)
 	rescale->cfg = of_device_get_match_data(dev);
 	rescale->numerator = 1;
 	rescale->denominator = 1;
+	rescale->offset = 0;
 
 	ret = rescale->cfg->props(dev, rescale);
 	if (ret)
-- 
2.30.1.489.g328c10930387

Re: [PATCH v1 4/9] iio: afe: rescale: add offset support

[Peter Rosin]

Hi!

Thanks for the patch!

On 2021-05-30 02:59, Liam Beguin wrote:
> From: Liam Beguin <lvb@xiphos.com>
> 
> This is a preparatory change required for the addition of temperature
> sensing front ends.

I think this is too simplistic. I think that if the upstream iio-dev has
an offset, it should be dealt with (i.e. be rescaled). The rescale driver
cannot ignore such an upstream offset and then throw in some other
unrelated offset of its own. That would be thoroughly confusing.

Also, I see no reason of expose an offset channel if there is no offset.

Cheers,
Peter

> Signed-off-by: Liam Beguin <lvb@xiphos.com>
> ---
>  drivers/iio/afe/iio-rescale.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c
> index 4d0813b274d1..3bd1f11f21db 100644
> --- a/drivers/iio/afe/iio-rescale.c
> +++ b/drivers/iio/afe/iio-rescale.c
> @@ -31,6 +31,7 @@ struct rescale {
>  	struct iio_chan_spec_ext_info *ext_info;
>  	s32 numerator;
>  	s32 denominator;
> +	s32 offset;
>  };
>  
>  static int rescale_read_raw(struct iio_dev *indio_dev,
> @@ -52,6 +53,10 @@ static int rescale_read_raw(struct iio_dev *indio_dev,
>  		*val2 = rescale->denominator;
>  
>  		return IIO_VAL_FRACTIONAL;
> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = rescale->offset;
> +
> +		return IIO_VAL_INT;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -119,8 +124,10 @@ static int rescale_configure_channel(struct device *dev,
>  		return -EINVAL;
>  	}
>  
> -	chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> -		BIT(IIO_CHAN_INFO_SCALE);
> +	chan->info_mask_separate =
> +		BIT(IIO_CHAN_INFO_RAW) |
> +		BIT(IIO_CHAN_INFO_SCALE) |
> +		BIT(IIO_CHAN_INFO_OFFSET);
>  
>  	if (iio_channel_has_available(schan, IIO_CHAN_INFO_RAW))
>  		chan->info_mask_separate_available |= BIT(IIO_CHAN_INFO_RAW);
> @@ -280,6 +287,7 @@ static int rescale_probe(struct platform_device *pdev)
>  	rescale->cfg = of_device_get_match_data(dev);
>  	rescale->numerator = 1;
>  	rescale->denominator = 1;
> +	rescale->offset = 0;
>  
>  	ret = rescale->cfg->props(dev, rescale);
>  	if (ret)
> 

Re: [PATCH v1 4/9] iio: afe: rescale: add offset support

[Liam Beguin]

Hi Peter,

On Mon May 31, 2021 at 4:52 AM EDT, Peter Rosin wrote:
> Hi!
>
> Thanks for the patch!
>
> On 2021-05-30 02:59, Liam Beguin wrote:
> > From: Liam Beguin <lvb@xiphos.com>
> > 
> > This is a preparatory change required for the addition of temperature
> > sensing front ends.
>
> I think this is too simplistic. I think that if the upstream iio-dev has
> an offset, it should be dealt with (i.e. be rescaled). The rescale
> driver
> cannot ignore such an upstream offset and then throw in some other
> unrelated offset of its own. That would be thoroughly confusing.

I'm not sure I fully understand. The upstream offset should be dealt
with when calling iio_read_channel_processed().  That was my main
motivation behind using the IIO core to get a processed value.

>
> Also, I see no reason of expose an offset channel if there is no offset.

You're right, I'll add conditions to only expose an offset when
required.

Liam

>
> Cheers,
> Peter
>
> > Signed-off-by: Liam Beguin <lvb@xiphos.com>
> > ---
> >  drivers/iio/afe/iio-rescale.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c
> > index 4d0813b274d1..3bd1f11f21db 100644
> > --- a/drivers/iio/afe/iio-rescale.c
> > +++ b/drivers/iio/afe/iio-rescale.c
> > @@ -31,6 +31,7 @@ struct rescale {
> >  	struct iio_chan_spec_ext_info *ext_info;
> >  	s32 numerator;
> >  	s32 denominator;
> > +	s32 offset;
> >  };
> >  
> >  static int rescale_read_raw(struct iio_dev *indio_dev,
> > @@ -52,6 +53,10 @@ static int rescale_read_raw(struct iio_dev *indio_dev,
> >  		*val2 = rescale->denominator;
> >  
> >  		return IIO_VAL_FRACTIONAL;
> > +	case IIO_CHAN_INFO_OFFSET:
> > +		*val = rescale->offset;
> > +
> > +		return IIO_VAL_INT;
> >  	default:
> >  		return -EINVAL;
> >  	}
> > @@ -119,8 +124,10 @@ static int rescale_configure_channel(struct device *dev,
> >  		return -EINVAL;
> >  	}
> >  
> > -	chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > -		BIT(IIO_CHAN_INFO_SCALE);
> > +	chan->info_mask_separate =
> > +		BIT(IIO_CHAN_INFO_RAW) |
> > +		BIT(IIO_CHAN_INFO_SCALE) |
> > +		BIT(IIO_CHAN_INFO_OFFSET);
> >  
> >  	if (iio_channel_has_available(schan, IIO_CHAN_INFO_RAW))
> >  		chan->info_mask_separate_available |= BIT(IIO_CHAN_INFO_RAW);
> > @@ -280,6 +287,7 @@ static int rescale_probe(struct platform_device *pdev)
> >  	rescale->cfg = of_device_get_match_data(dev);
> >  	rescale->numerator = 1;
> >  	rescale->denominator = 1;
> > +	rescale->offset = 0;
> >  
> >  	ret = rescale->cfg->props(dev, rescale);
> >  	if (ret)
> > 

Re: [PATCH v1 4/9] iio: afe: rescale: add offset support

[Peter Rosin]

On 2021-05-31 15:36, Liam Beguin wrote:
> Hi Peter,
> 
> On Mon May 31, 2021 at 4:52 AM EDT, Peter Rosin wrote:
>> Hi!
>>
>> Thanks for the patch!
>>
>> On 2021-05-30 02:59, Liam Beguin wrote:
>>> From: Liam Beguin <lvb@xiphos.com>
>>>
>>> This is a preparatory change required for the addition of temperature
>>> sensing front ends.
>>
>> I think this is too simplistic. I think that if the upstream iio-dev has
>> an offset, it should be dealt with (i.e. be rescaled). The rescale
>> driver
>> cannot ignore such an upstream offset and then throw in some other
>> unrelated offset of its own. That would be thoroughly confusing.
> 
> I'm not sure I fully understand. The upstream offset should be dealt
> with when calling iio_read_channel_processed().  That was my main
> motivation behind using the IIO core to get a processed value.

You can rescale a channel with an offset, but without using processed
values. I.e. the upstream channel provides raw values, a scale and an
offset. The current rescale code ignores the upstream offset. I did not
need that when I created the driver, and at a glace it felt "difficult".
So I punted.

But if the rescaler is going to start to handle offsets of any kind, it
will get very confusing if the upstream offset is ignored. The proper
way to do that is not something I have thought deeply about, and I
don't know what the proper behavior is. For a processed channel, the
offset is baked into the value that is scaled. Maybe the sane thing
is to do that for a non-processed channel as well? But that gets a bit
ugly, as it is counter to the simplicity, beauty and efficiency of the
rescaler driver. In the non-processed case the driver is just adjusting
the scale value. But since we are talking about proportional
relationships, it should be possible to rescale a non-processed
channel with an offset by just adjusting the offset in some way related
to the rescale factor. Doing it with integer math is the "difficult"
part...

Cheers,
Peter

Re: [PATCH v1 4/9] iio: afe: rescale: add offset support

[Liam Beguin]

On Mon May 31, 2021 at 10:08 AM EDT, Peter Rosin wrote:
> On 2021-05-31 15:36, Liam Beguin wrote:
> > Hi Peter,
> > 
> > On Mon May 31, 2021 at 4:52 AM EDT, Peter Rosin wrote:
> >> Hi!
> >>
> >> Thanks for the patch!
> >>
> >> On 2021-05-30 02:59, Liam Beguin wrote:
> >>> From: Liam Beguin <lvb@xiphos.com>
> >>>
> >>> This is a preparatory change required for the addition of temperature
> >>> sensing front ends.
> >>
> >> I think this is too simplistic. I think that if the upstream iio-dev has
> >> an offset, it should be dealt with (i.e. be rescaled). The rescale
> >> driver
> >> cannot ignore such an upstream offset and then throw in some other
> >> unrelated offset of its own. That would be thoroughly confusing.
> > 
> > I'm not sure I fully understand. The upstream offset should be dealt
> > with when calling iio_read_channel_processed().  That was my main
> > motivation behind using the IIO core to get a processed value.
>
> You can rescale a channel with an offset, but without using processed
> values. I.e. the upstream channel provides raw values, a scale and an
> offset. The current rescale code ignores the upstream offset. I did not
> need that when I created the driver, and at a glace it felt "difficult".
> So I punted.

I understand what you meant now.

At first, I tried to apply the upstream offset from inside the rescaler.
As you said it felt difficult and it felt like this must've been
implemented somewhere else before.

After looking around, I noticed that the code to do that was already
part of inkern.c and exposed through iio_read_channel_processed().
If the upstream channel doesn't provide a processed value, the upstream
offset and scale are automatically applied.

So with the changes in [3/9] the rescaler's raw value becomes the
upstream channel's processed value.

This seems like an easier and probably cleaner way of adding offset
support in the rescaler.

Does that make sense?

Cheers,
Liam

>
> But if the rescaler is going to start to handle offsets of any kind, it
> will get very confusing if the upstream offset is ignored. The proper
> way to do that is not something I have thought deeply about, and I
> don't know what the proper behavior is. For a processed channel, the
> offset is baked into the value that is scaled. Maybe the sane thing
> is to do that for a non-processed channel as well? But that gets a bit
> ugly, as it is counter to the simplicity, beauty and efficiency of the
> rescaler driver. In the non-processed case the driver is just adjusting
> the scale value. But since we are talking about proportional
> relationships, it should be possible to rescale a non-processed
> channel with an offset by just adjusting the offset in some way related
> to the rescale factor. Doing it with integer math is the "difficult"
> part...
>
> Cheers,
> Peter

Re: [PATCH v1 4/9] iio: afe: rescale: add offset support

[Peter Rosin]

On 2021-05-31 16:51, Liam Beguin wrote:
> On Mon May 31, 2021 at 10:08 AM EDT, Peter Rosin wrote:
>> On 2021-05-31 15:36, Liam Beguin wrote:
>>> Hi Peter,
>>>
>>> On Mon May 31, 2021 at 4:52 AM EDT, Peter Rosin wrote:
>>>> Hi!
>>>>
>>>> Thanks for the patch!
>>>>
>>>> On 2021-05-30 02:59, Liam Beguin wrote:
>>>>> From: Liam Beguin <lvb@xiphos.com>
>>>>>
>>>>> This is a preparatory change required for the addition of temperature
>>>>> sensing front ends.
>>>>
>>>> I think this is too simplistic. I think that if the upstream iio-dev has
>>>> an offset, it should be dealt with (i.e. be rescaled). The rescale
>>>> driver
>>>> cannot ignore such an upstream offset and then throw in some other
>>>> unrelated offset of its own. That would be thoroughly confusing.
>>>
>>> I'm not sure I fully understand. The upstream offset should be dealt
>>> with when calling iio_read_channel_processed().  That was my main
>>> motivation behind using the IIO core to get a processed value.
>>
>> You can rescale a channel with an offset, but without using processed
>> values. I.e. the upstream channel provides raw values, a scale and an
>> offset. The current rescale code ignores the upstream offset. I did not
>> need that when I created the driver, and at a glace it felt "difficult".
>> So I punted.
> 
> I understand what you meant now.
> 
> At first, I tried to apply the upstream offset from inside the rescaler.
> As you said it felt difficult and it felt like this must've been
> implemented somewhere else before.
> 
> After looking around, I noticed that the code to do that was already
> part of inkern.c and exposed through iio_read_channel_processed().
> If the upstream channel doesn't provide a processed value, the upstream
> offset and scale are automatically applied.
> 
> So with the changes in [3/9] the rescaler's raw value becomes the
> upstream channel's processed value.
> 
> This seems like an easier and probably cleaner way of adding offset
> support in the rescaler.
> 
> Does that make sense?

Yes, it does. Doing generic calculations like this efficiently with
integer math without losing precision is ... difficult.

I think that perhaps IF the upstream channel has an offset, the
rescaler could revert to always use the upstream processed channel in
preference of the raw channel. That would fix the missing support for
upstream offset and still not penalize the sweet case of no upstream
offset. Because the processed channel costs processing for each and
every sample and I think it should be avoided as much as possible.

Does that make sense?

Or are a bunch of drivers adding an explicit zero offset "just because"?
That would be a nuisance.

Cheers,
Peter

Re: [PATCH v1 4/9] iio: afe: rescale: add offset support

[Liam Beguin]

On Mon May 31, 2021 at 12:25 PM EDT, Peter Rosin wrote:
> On 2021-05-31 16:51, Liam Beguin wrote:
> > On Mon May 31, 2021 at 10:08 AM EDT, Peter Rosin wrote:
> >> On 2021-05-31 15:36, Liam Beguin wrote:
> >>> Hi Peter,
> >>>
> >>> On Mon May 31, 2021 at 4:52 AM EDT, Peter Rosin wrote:
> >>>> Hi!
> >>>>
> >>>> Thanks for the patch!
> >>>>
> >>>> On 2021-05-30 02:59, Liam Beguin wrote:
> >>>>> From: Liam Beguin <lvb@xiphos.com>
> >>>>>
> >>>>> This is a preparatory change required for the addition of temperature
> >>>>> sensing front ends.
> >>>>
> >>>> I think this is too simplistic. I think that if the upstream iio-dev has
> >>>> an offset, it should be dealt with (i.e. be rescaled). The rescale
> >>>> driver
> >>>> cannot ignore such an upstream offset and then throw in some other
> >>>> unrelated offset of its own. That would be thoroughly confusing.
> >>>
> >>> I'm not sure I fully understand. The upstream offset should be dealt
> >>> with when calling iio_read_channel_processed().  That was my main
> >>> motivation behind using the IIO core to get a processed value.
> >>
> >> You can rescale a channel with an offset, but without using processed
> >> values. I.e. the upstream channel provides raw values, a scale and an
> >> offset. The current rescale code ignores the upstream offset. I did not
> >> need that when I created the driver, and at a glace it felt "difficult".
> >> So I punted.
> > 
> > I understand what you meant now.
> > 
> > At first, I tried to apply the upstream offset from inside the rescaler.
> > As you said it felt difficult and it felt like this must've been
> > implemented somewhere else before.
> > 
> > After looking around, I noticed that the code to do that was already
> > part of inkern.c and exposed through iio_read_channel_processed().
> > If the upstream channel doesn't provide a processed value, the upstream
> > offset and scale are automatically applied.
> > 
> > So with the changes in [3/9] the rescaler's raw value becomes the
> > upstream channel's processed value.
> > 
> > This seems like an easier and probably cleaner way of adding offset
> > support in the rescaler.
> > 
> > Does that make sense?
>
> Yes, it does. Doing generic calculations like this efficiently with
> integer math without losing precision is ... difficult.

You're right, I realized it's more complicated that it seems working on
this.

>
> I think that perhaps IF the upstream channel has an offset, the
> rescaler could revert to always use the upstream processed channel in
> preference of the raw channel. That would fix the missing support for
> upstream offset and still not penalize the sweet case of no upstream
> offset. Because the processed channel costs processing for each and
> every sample and I think it should be avoided as much as possible.
>
> Does that make sense?

Totally, I see what you're saying and will give it a try.

I still believe it would make sense to get the upstream scaling factor
the same way, to avoid duplicating that code.

Also it might be confusing to have the raw value be the upstream raw
value in some cases and the upstream processed value in others.

>
> Or are a bunch of drivers adding an explicit zero offset "just because"?
> That would be a nuisance.

A quick search seems to indicate that this isn't the case.

Thanks for your time,
Liam

>
> Cheers,
> Peter

Re: [PATCH v1 4/9] iio: afe: rescale: add offset support

[Jonathan Cameron]

On Mon, 31 May 2021 13:42:09 -0400
"Liam Beguin" <liambeguin@gmail.com> wrote:

> On Mon May 31, 2021 at 12:25 PM EDT, Peter Rosin wrote:
> > On 2021-05-31 16:51, Liam Beguin wrote:  
> > > On Mon May 31, 2021 at 10:08 AM EDT, Peter Rosin wrote:  
> > >> On 2021-05-31 15:36, Liam Beguin wrote:  
> > >>> Hi Peter,
> > >>>
> > >>> On Mon May 31, 2021 at 4:52 AM EDT, Peter Rosin wrote:  
> > >>>> Hi!
> > >>>>
> > >>>> Thanks for the patch!
> > >>>>
> > >>>> On 2021-05-30 02:59, Liam Beguin wrote:  
> > >>>>> From: Liam Beguin <lvb@xiphos.com>
> > >>>>>
> > >>>>> This is a preparatory change required for the addition of temperature
> > >>>>> sensing front ends.  
> > >>>>
> > >>>> I think this is too simplistic. I think that if the upstream iio-dev has
> > >>>> an offset, it should be dealt with (i.e. be rescaled). The rescale
> > >>>> driver
> > >>>> cannot ignore such an upstream offset and then throw in some other
> > >>>> unrelated offset of its own. That would be thoroughly confusing.  
> > >>>
> > >>> I'm not sure I fully understand. The upstream offset should be dealt
> > >>> with when calling iio_read_channel_processed().  That was my main
> > >>> motivation behind using the IIO core to get a processed value.  
> > >>
> > >> You can rescale a channel with an offset, but without using processed
> > >> values. I.e. the upstream channel provides raw values, a scale and an
> > >> offset. The current rescale code ignores the upstream offset. I did not
> > >> need that when I created the driver, and at a glace it felt "difficult".
> > >> So I punted.  
> > > 
> > > I understand what you meant now.
> > > 
> > > At first, I tried to apply the upstream offset from inside the rescaler.
> > > As you said it felt difficult and it felt like this must've been
> > > implemented somewhere else before.
> > > 
> > > After looking around, I noticed that the code to do that was already
> > > part of inkern.c and exposed through iio_read_channel_processed().
> > > If the upstream channel doesn't provide a processed value, the upstream
> > > offset and scale are automatically applied.
> > > 
> > > So with the changes in [3/9] the rescaler's raw value becomes the
> > > upstream channel's processed value.
> > > 
> > > This seems like an easier and probably cleaner way of adding offset
> > > support in the rescaler.
> > > 
> > > Does that make sense?  
> >
> > Yes, it does. Doing generic calculations like this efficiently with
> > integer math without losing precision is ... difficult.  
> 
> You're right, I realized it's more complicated that it seems working on
> this.

Yup, particularly given the processed version doesn't work because of
scale precision loss.  To avoid that mess you would have to do the
maths to rescale the offset.

If we assume offsets are integer (not always true, but often true for
ADCs) then it wouldn't be too bad, but you will need to handle all the
different ways scale can be specified (or support a subset perhaps).


> 
> >
> > I think that perhaps IF the upstream channel has an offset, the
> > rescaler could revert to always use the upstream processed channel in
> > preference of the raw channel. That would fix the missing support for
> > upstream offset and still not penalize the sweet case of no upstream
> > offset. Because the processed channel costs processing for each and
> > every sample and I think it should be avoided as much as possible.
> >
> > Does that make sense?  
> 
> Totally, I see what you're saying and will give it a try.
> 
> I still believe it would make sense to get the upstream scaling factor
> the same way, to avoid duplicating that code.
> 
> Also it might be confusing to have the raw value be the upstream raw
> value in some cases and the upstream processed value in others.
> 
> >
> > Or are a bunch of drivers adding an explicit zero offset "just because"?
> > That would be a nuisance.  
> 
> A quick search seems to indicate that this isn't the case.

We were pretty rigorous about this, though there are drivers that have
variable offsets where it will 'sometimes' be 0. 

It's an interesting corner, that we've been avoiding, but probably
not too bad to do at least the common combinations.

The fun will come when you are trying to sensible combine a scaled
offset and your new offset and need to do integer maths for.

A/(2**B) + C.D

There will probably be cases where we just take a decent stab at it
and assume precision might not be great.

Jonathan

> 
> Thanks for your time,
> Liam
> 
> >
> > Cheers,
> > Peter  
> 

[PATCH v1 5/9] iio: afe: rescale: add support for temperature sensors

[Liam Beguin]

From: Liam Beguin <lvb@xiphos.com>

Add support for various linear temperature sensors.

temperature-sense-rtd is used when the measured temperature is a
function of the sensor's resistance (like RTD sensors).

temperature-sense-current is used when the measured temperature is a
function of the sensor's output current (like the AD590)

temperature-sense-amplifier is used when the measured temperature is a
function of the sensor's voltage (like the LTC2997)

Signed-off-by: Liam Beguin <lvb@xiphos.com>
---
 drivers/iio/afe/iio-rescale.c | 141 ++++++++++++++++++++++++++++++++++
 1 file changed, 141 insertions(+)

diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c
index 3bd1f11f21db..eb53d833bf7c 100644
--- a/drivers/iio/afe/iio-rescale.c
+++ b/drivers/iio/afe/iio-rescale.c
@@ -222,10 +222,133 @@ static int rescale_voltage_divider_props(struct device *dev,
 	return 0;
 }
 
+static int rescale_temp_sense_rtd_props(struct device *dev,
+					struct rescale *rescale)
+{
+	u32 factor;
+	u32 alpha;
+	u32 iexc;
+	u32 tmp;
+	int ret;
+	u32 r0;
+
+	ret = device_property_read_u32(dev, "excitation-current-microamp",
+				       &iexc);
+	if (ret) {
+		dev_err(dev, "failed to read excitation-current-microamp: %d\n",
+			ret);
+		return ret;
+	}
+
+	ret = device_property_read_u32(dev, "alpha-micro-ohms-per-ohm-celsius",
+				       &alpha);
+	if (ret) {
+		dev_err(dev, "failed to read alpha-micro-ohms-per-celsius: %d\n",
+			ret);
+		return ret;
+	}
+
+	ret = device_property_read_u32(dev, "r-naught-ohms", &r0);
+	if (ret) {
+		dev_err(dev, "failed to read r-naught-ohms: %d\n", ret);
+		return ret;
+	}
+
+	/*
+	 * The transfer function:
+	 *
+	 *	- V(T) = R(T) * iexc
+	 *	- R(T) = r0 * (1 + alpha * T)
+	 *
+	 *	T = 1 / (alpha * r0 * iexc) * (V - r0 * iexc)
+	 */
+	tmp = r0 * iexc * alpha / 1000000;
+	factor = gcd(tmp, 1000000);
+	rescale->numerator = 1000000 / factor;
+	rescale->denominator = tmp / factor;
+
+	rescale->offset = -1 * ((r0 * iexc) / 1000);
+
+	return 0;
+}
+
+static int rescale_temp_sense_current_props(struct device *dev,
+					    struct rescale *rescale)
+{
+	u32 alpha;
+	u32 sense;
+	int ret;
+
+	ret = device_property_read_u32(dev, "sense-resistor-ohms", &sense);
+	if (ret) {
+		dev_err(dev, "failed to read the sense resistance: %d\n", ret);
+		return ret;
+	}
+
+	ret = device_property_read_u32(dev, "alpha-micro-amps-per-degree",
+				       &alpha);
+	if (ret) {
+		dev_err(dev, "failed to read alpha-micro-amps-per-degree: %d\n",
+			ret);
+		return ret;
+	}
+
+	/*
+	 * The transfer function:
+	 *
+	 *	- V(K) = Rsense * Isense(K)
+	 *	- K = Isense(K) / alpha
+	 *	- C = K - 273.15
+	 *
+	 *	C = 1 / (Rsense * alpha) * (V - 273.15 * Rsense * alpha)
+	 */
+	rescale->numerator = 1000000;
+	rescale->denominator = alpha * sense;
+
+	if (device_property_read_bool(dev, "use-kelvin-scale"))
+		rescale->offset = -1 * ((27315 * alpha * sense) / 100000);
+
+	return 0;
+}
+
+static int rescale_temp_sense_amplifier_props(struct device *dev,
+					      struct rescale *rescale)
+{
+	u32 alpha;
+	int ret;
+
+	ret = device_property_read_u32(dev, "alpha-micro-volts-per-degree",
+				       &alpha);
+	if (ret) {
+		dev_err(dev, "failed to read alpha-micro-volts-per-degree: %d\n",
+			ret);
+		return ret;
+	}
+
+	/*
+	 * The transfer function:
+	 *
+	 *	- K = V(K) / alpha
+	 *	- C = K - 273.15
+	 *
+	 *	C = 1 / (alpha) * (V - 273.15 * alpha)
+	 */
+	rescale->numerator = 1000000;
+	rescale->denominator = alpha;
+
+	if (device_property_read_bool(dev, "use-kelvin-scale"))
+		rescale->offset = -1 * ((27315 * alpha) / 100000);
+
+	return 0;
+}
+
 enum rescale_variant {
 	CURRENT_SENSE_AMPLIFIER,
 	CURRENT_SENSE_SHUNT,
 	VOLTAGE_DIVIDER,
+	TEMP_SENSE_RTD,
+	TEMP_SENSE_CURRENT,
+	TEMP_SENSE_AMPLIFIER,
 };
 
 static const struct rescale_cfg rescale_cfg[] = {
@@ -241,6 +364,18 @@ static const struct rescale_cfg rescale_cfg[] = {
 		.type = IIO_VOLTAGE,
 		.props = rescale_voltage_divider_props,
 	},
+	[TEMP_SENSE_RTD] = {
+		.type = IIO_TEMP,
+		.props = rescale_temp_sense_rtd_props,
+	},
+	[TEMP_SENSE_CURRENT] = {
+		.type = IIO_TEMP,
+		.props = rescale_temp_sense_current_props,
+	},
+	[TEMP_SENSE_AMPLIFIER] = {
+		.type = IIO_TEMP,
+		.props = rescale_temp_sense_amplifier_props,
+	},
 };
 
 static const struct of_device_id rescale_match[] = {
@@ -250,6 +385,12 @@ static const struct of_device_id rescale_match[] = {
 	  .data = &rescale_cfg[CURRENT_SENSE_SHUNT], },
 	{ .compatible = "voltage-divider",
 	  .data = &rescale_cfg[VOLTAGE_DIVIDER], },
+	{ .compatible = "temperature-sense-rtd",
+	  .data = &rescale_cfg[TEMP_SENSE_RTD], },
+	{ .compatible = "temperature-sense-current",
+	  .data = &rescale_cfg[TEMP_SENSE_CURRENT], },
+	{ .compatible = "temperature-sense-amplifier",
+	  .data = &rescale_cfg[TEMP_SENSE_AMPLIFIER], },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, rescale_match);
-- 
2.30.1.489.g328c10930387

Re: [PATCH v1 5/9] iio: afe: rescale: add support for temperature sensors

[Jonathan Cameron]

On Sat, 29 May 2021 20:59:13 -0400
Liam Beguin <liambeguin@gmail.com> wrote:

> From: Liam Beguin <lvb@xiphos.com>
> 
> Add support for various linear temperature sensors.
> 
> temperature-sense-rtd is used when the measured temperature is a
> function of the sensor's resistance (like RTD sensors).
> 
> temperature-sense-current is used when the measured temperature is a
> function of the sensor's output current (like the AD590)
> 
> temperature-sense-amplifier is used when the measured temperature is a
> function of the sensor's voltage (like the LTC2997)
> 
> Signed-off-by: Liam Beguin <lvb@xiphos.com>
Hi Liam,

Comments in here follow through from the bindings.

Jonathan

> ---
>  drivers/iio/afe/iio-rescale.c | 141 ++++++++++++++++++++++++++++++++++
>  1 file changed, 141 insertions(+)
> 
> diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c
> index 3bd1f11f21db..eb53d833bf7c 100644
> --- a/drivers/iio/afe/iio-rescale.c
> +++ b/drivers/iio/afe/iio-rescale.c
> @@ -222,10 +222,133 @@ static int rescale_voltage_divider_props(struct device *dev,
>  	return 0;
>  }
>  
> +static int rescale_temp_sense_rtd_props(struct device *dev,
> +					struct rescale *rescale)
> +{
> +	u32 factor;
> +	u32 alpha;
> +	u32 iexc;
> +	u32 tmp;
> +	int ret;
> +	u32 r0;
> +
> +	ret = device_property_read_u32(dev, "excitation-current-microamp",
> +				       &iexc);
> +	if (ret) {
> +		dev_err(dev, "failed to read excitation-current-microamp: %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +	ret = device_property_read_u32(dev, "alpha-micro-ohms-per-ohm-celsius",
> +				       &alpha);
> +	if (ret) {
> +		dev_err(dev, "failed to read alpha-micro-ohms-per-celsius: %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +	ret = device_property_read_u32(dev, "r-naught-ohms", &r0);
> +	if (ret) {
> +		dev_err(dev, "failed to read r-naught-ohms: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/*
> +	 * The transfer function:
> +	 *
> +	 *	- V(T) = R(T) * iexc
> +	 *	- R(T) = r0 * (1 + alpha * T)
> +	 *
> +	 *	T = 1 / (alpha * r0 * iexc) * (V - r0 * iexc)
> +	 */
> +	tmp = r0 * iexc * alpha / 1000000;
> +	factor = gcd(tmp, 1000000);
> +	rescale->numerator = 1000000 / factor;
> +	rescale->denominator = tmp / factor;
> +
> +	rescale->offset = -1 * ((r0 * iexc) / 1000);
> +
> +	return 0;
> +}
> +
> +static int rescale_temp_sense_current_props(struct device *dev,
> +					    struct rescale *rescale)
> +{
> +	u32 alpha;
> +	u32 sense;
> +	int ret;
> +
> +	ret = device_property_read_u32(dev, "sense-resistor-ohms", &sense);
> +	if (ret) {
> +		dev_err(dev, "failed to read the sense resistance: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = device_property_read_u32(dev, "alpha-micro-amps-per-degree",
> +				       &alpha);
> +	if (ret) {
> +		dev_err(dev, "failed to read alpha-micro-amps-per-degree: %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +	/*
> +	 * The transfer function:
> +	 *
> +	 *	- V(K) = Rsense * Isense(K)
> +	 *	- K = Isense(K) / alpha
> +	 *	- C = K - 273.15
> +	 *
> +	 *	C = 1 / (Rsense * alpha) * (V - 273.15 * Rsense * alpha)
> +	 */
> +	rescale->numerator = 1000000;
> +	rescale->denominator = alpha * sense;
> +
> +	if (device_property_read_bool(dev, "use-kelvin-scale"))
> +		rescale->offset = -1 * ((27315 * alpha * sense) / 100000);

As below. Generic offset, not this specific one please ;)

> +
> +	return 0;
> +}
> +
> +static int rescale_temp_sense_amplifier_props(struct device *dev,
> +					      struct rescale *rescale)
> +{
> +	u32 alpha;
> +	int ret;
> +
> +	ret = device_property_read_u32(dev, "alpha-micro-volts-per-degree",
> +				       &alpha);
> +	if (ret) {
> +		dev_err(dev, "failed to read alpha-micro-volts-per-degree: %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +	/*
> +	 * The transfer function:
> +	 *
> +	 *	- K = V(K) / alpha
> +	 *	- C = K - 273.15
> +	 *
> +	 *	C = 1 / (alpha) * (V - 273.15 * alpha)
> +	 */
> +	rescale->numerator = 1000000;
> +	rescale->denominator = alpha;
> +
> +	if (device_property_read_bool(dev, "use-kelvin-scale"))

As mentioned later, stick to celcius + an explicit offset.

There will be devices that have their own offset which doesn't happen to
be -273.15

> +		rescale->offset = -1 * ((27315 * alpha) / 100000);
> +
> +	return 0;
> +}
> +
>  enum rescale_variant {
>  	CURRENT_SENSE_AMPLIFIER,
>  	CURRENT_SENSE_SHUNT,
>  	VOLTAGE_DIVIDER,
> +	TEMP_SENSE_RTD,
> +	TEMP_SENSE_CURRENT,
> +	TEMP_SENSE_AMPLIFIER,
>  };
>  
>  static const struct rescale_cfg rescale_cfg[] = {
> @@ -241,6 +364,18 @@ static const struct rescale_cfg rescale_cfg[] = {
>  		.type = IIO_VOLTAGE,
>  		.props = rescale_voltage_divider_props,
>  	},
> +	[TEMP_SENSE_RTD] = {
> +		.type = IIO_TEMP,
> +		.props = rescale_temp_sense_rtd_props,
> +	},
> +	[TEMP_SENSE_CURRENT] = {
> +		.type = IIO_TEMP,
> +		.props = rescale_temp_sense_current_props,
> +	},
> +	[TEMP_SENSE_AMPLIFIER] = {
> +		.type = IIO_TEMP,
> +		.props = rescale_temp_sense_amplifier_props,
> +	},
>  };
>  
>  static const struct of_device_id rescale_match[] = {
> @@ -250,6 +385,12 @@ static const struct of_device_id rescale_match[] = {
>  	  .data = &rescale_cfg[CURRENT_SENSE_SHUNT], },
>  	{ .compatible = "voltage-divider",
>  	  .data = &rescale_cfg[VOLTAGE_DIVIDER], },
> +	{ .compatible = "temperature-sense-rtd",
> +	  .data = &rescale_cfg[TEMP_SENSE_RTD], },
> +	{ .compatible = "temperature-sense-current",
> +	  .data = &rescale_cfg[TEMP_SENSE_CURRENT], },
> +	{ .compatible = "temperature-sense-amplifier",
> +	  .data = &rescale_cfg[TEMP_SENSE_AMPLIFIER], },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, rescale_match);

[PATCH v1 6/9] dt-bindings: iio: afe: update MAINTAINERS file

[Liam Beguin]

From: Liam Beguin <lvb@xiphos.com>

The IIO Analog Front End devicetree bindings files have been update to
yaml. Make sure the MAINTAINERS file matches the tree.

Signed-off-by: Liam Beguin <lvb@xiphos.com>
---
 MAINTAINERS | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9450e052f1b1..d3ab0ccc34ab 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8719,9 +8719,9 @@ IIO UNIT CONVERTER
 M:	Peter Rosin <peda@axentia.se>
 L:	linux-iio@vger.kernel.org
 S:	Maintained
-F:	Documentation/devicetree/bindings/iio/afe/current-sense-amplifier.txt
-F:	Documentation/devicetree/bindings/iio/afe/current-sense-shunt.txt
-F:	Documentation/devicetree/bindings/iio/afe/voltage-divider.txt
+F:	Documentation/devicetree/bindings/iio/afe/current-sense-amplifier.yaml
+F:	Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
+F:	Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
 F:	drivers/iio/afe/iio-rescale.c
 
 IKANOS/ADI EAGLE ADSL USB DRIVER
-- 
2.30.1.489.g328c10930387

Re: [PATCH v1 6/9] dt-bindings: iio: afe: update MAINTAINERS file

[Peter Rosin]

Hi!

On 2021-05-30 02:59, Liam Beguin wrote:
> From: Liam Beguin <lvb@xiphos.com>
> 
> The IIO Analog Front End devicetree bindings files have been update to
> yaml. Make sure the MAINTAINERS file matches the tree.
> 
> Signed-off-by: Liam Beguin <lvb@xiphos.com>

This patch can be dropped. It was fixed in 5.13-rc1.

Cheers,
Peter

[PATCH v1 7/9] dt-bindings: iio: afe: add binding for temperature-sense-rtd

[Liam Beguin]

From: Liam Beguin <lvb@xiphos.com>

An ADC is often used to measure other quantities indirectly. This
binding describe one cases, the measurement of a temperature through
the voltage across an RTD resistor such as a PT1000.

Signed-off-by: Liam Beguin <lvb@xiphos.com>
---
 .../iio/afe/temperature-sense-rtd.yaml        | 65 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 66 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml

diff --git a/Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml b/Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
new file mode 100644
index 000000000000..4798eda6e533
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/afe/temperature-sense-rtd.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Temperature Sense RTD
+
+maintainers:
+  - Liam Beguin <lvb@xiphos.com>
+
+description: |
+  When an io-channel measures the output voltage across an RTD such as a
+  PT1000, the interesting measurement is almost always the corresponding
+  temperature, not the voltage output. This binding describes such a circuit.
+
+properties:
+  compatible:
+    const: temperature-sense-rtd
+
+  io-channels:
+    maxItems: 1
+    description: |
+      Channel node of a voltage io-channel.
+
+  '#io-channel-cells':
+    const: 1
+
+  excitation-current-microamp:
+    description: The current fed through the RTD sensor.
+
+  alpha-micro-ohms-per-ohm-celsius:
+    description: |
+      Linear approximation of the resistance versus temperature relationship
+      between 0 and 100 degrees Celsius.
+
+      Pure platinum has an alpha of 3925. Industry standards such as IEC60751
+      and ASTM E-1137 specify an alpha of 3850.
+
+  r-naught-ohms:
+    description: |
+      Resistance of the sensor at 0 degrees Celsius.
+      Common values are 100 for PT100 and 1000 for PT1000.
+
+additionalProperties: false
+required:
+  - compatible
+  - io-channels
+  - excitation-current-microamp
+  - alpha-micro-ohms-per-ohm-celsius
+  - r-naught-ohms
+
+examples:
+  - |
+    pt1000_1: iio-rescale0 {
+        compatible = "temperature-sense-rtd";
+        #io-channel-cells = <1>;
+        io-channels = <&temp_adc1 0>;
+
+        excitation-current-microamp = <1000>;
+        alpha-micro-ohms-per-ohm-celsius = <3908>;
+        r-naught-ohms = <1000>;
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index d3ab0ccc34ab..a7279af85adb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8721,6 +8721,7 @@ L:	linux-iio@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/iio/afe/current-sense-amplifier.yaml
 F:	Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
+F:	Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
 F:	Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
 F:	drivers/iio/afe/iio-rescale.c
 
-- 
2.30.1.489.g328c10930387

Re: [PATCH v1 7/9] dt-bindings: iio: afe: add binding for temperature-sense-rtd

[Jonathan Cameron]

On Sat, 29 May 2021 20:59:15 -0400
Liam Beguin <liambeguin@gmail.com> wrote:

> From: Liam Beguin <lvb@xiphos.com>
> 
> An ADC is often used to measure other quantities indirectly. This
> binding describe one cases, the measurement of a temperature through
> the voltage across an RTD resistor such as a PT1000.
> 
> Signed-off-by: Liam Beguin <lvb@xiphos.com>

Just one nit pick inline.

Hmm. these devices use some 'special' units for their coefficients.
Ah well, guess we copy the industry standard.

> ---
>  .../iio/afe/temperature-sense-rtd.yaml        | 65 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 66 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml b/Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
> new file mode 100644
> index 000000000000..4798eda6e533
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/afe/temperature-sense-rtd.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Temperature Sense RTD
> +
> +maintainers:
> +  - Liam Beguin <lvb@xiphos.com>
> +
> +description: |
> +  When an io-channel measures the output voltage across an RTD such as a
> +  PT1000, the interesting measurement is almost always the corresponding
> +  temperature, not the voltage output. This binding describes such a circuit.
> +
> +properties:
> +  compatible:
> +    const: temperature-sense-rtd
> +
> +  io-channels:
> +    maxItems: 1
> +    description: |
> +      Channel node of a voltage io-channel.
> +
> +  '#io-channel-cells':
> +    const: 1
> +
> +  excitation-current-microamp:
> +    description: The current fed through the RTD sensor.
> +
> +  alpha-micro-ohms-per-ohm-celsius:
> +    description: |
> +      Linear approximation of the resistance versus temperature relationship
> +      between 0 and 100 degrees Celsius.
> +
> +      Pure platinum has an alpha of 3925. Industry standards such as IEC60751
> +      and ASTM E-1137 specify an alpha of 3850.
> +
> +  r-naught-ohms:
> +    description: |
> +      Resistance of the sensor at 0 degrees Celsius.
> +      Common values are 100 for PT100 and 1000 for PT1000.
> +
> +additionalProperties: false
> +required:
> +  - compatible
> +  - io-channels
> +  - excitation-current-microamp
> +  - alpha-micro-ohms-per-ohm-celsius
> +  - r-naught-ohms
> +
> +examples:
> +  - |
> +    pt1000_1: iio-rescale0 {
> +        compatible = "temperature-sense-rtd";
> +        #io-channel-cells = <1>;
> +        io-channels = <&temp_adc1 0>;
> +
> +        excitation-current-microamp = <1000>;
> +        alpha-micro-ohms-per-ohm-celsius = <3908>;
> +        r-naught-ohms = <1000>;
> +    };
> +

Drop this blank line.  Doesn't add anything ;)

> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d3ab0ccc34ab..a7279af85adb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8721,6 +8721,7 @@ L:	linux-iio@vger.kernel.org
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/iio/afe/current-sense-amplifier.yaml
>  F:	Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
> +F:	Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
>  F:	Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
>  F:	drivers/iio/afe/iio-rescale.c
>  

Re: [PATCH v1 7/9] dt-bindings: iio: afe: add binding for temperature-sense-rtd

[Rob Herring]

On Sat, May 29, 2021 at 08:59:15PM -0400, Liam Beguin wrote:
> From: Liam Beguin <lvb@xiphos.com>
> 
> An ADC is often used to measure other quantities indirectly. This
> binding describe one cases, the measurement of a temperature through
> the voltage across an RTD resistor such as a PT1000.
> 
> Signed-off-by: Liam Beguin <lvb@xiphos.com>
> ---
>  .../iio/afe/temperature-sense-rtd.yaml        | 65 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 66 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml b/Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
> new file mode 100644
> index 000000000000..4798eda6e533
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/afe/temperature-sense-rtd.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Temperature Sense RTD
> +
> +maintainers:
> +  - Liam Beguin <lvb@xiphos.com>
> +
> +description: |
> +  When an io-channel measures the output voltage across an RTD such as a

What's an RTD? Not defined anywhere here.

> +  PT1000, the interesting measurement is almost always the corresponding
> +  temperature, not the voltage output. This binding describes such a circuit.
> +
> +properties:
> +  compatible:
> +    const: temperature-sense-rtd
> +
> +  io-channels:
> +    maxItems: 1
> +    description: |
> +      Channel node of a voltage io-channel.
> +
> +  '#io-channel-cells':
> +    const: 1

Doesn't this belong in the provider?

> +
> +  excitation-current-microamp:
> +    description: The current fed through the RTD sensor.
> +
> +  alpha-micro-ohms-per-ohm-celsius:
> +    description: |
> +      Linear approximation of the resistance versus temperature relationship
> +      between 0 and 100 degrees Celsius.
> +
> +      Pure platinum has an alpha of 3925. Industry standards such as IEC60751
> +      and ASTM E-1137 specify an alpha of 3850.

Is there a max and min value?

> +
> +  r-naught-ohms:
> +    description: |
> +      Resistance of the sensor at 0 degrees Celsius.
> +      Common values are 100 for PT100 and 1000 for PT1000.

max or min?

> +
> +additionalProperties: false

blank line here.

> +required:
> +  - compatible
> +  - io-channels
> +  - excitation-current-microamp
> +  - alpha-micro-ohms-per-ohm-celsius
> +  - r-naught-ohms
> +
> +examples:
> +  - |
> +    pt1000_1: iio-rescale0 {
> +        compatible = "temperature-sense-rtd";
> +        #io-channel-cells = <1>;
> +        io-channels = <&temp_adc1 0>;
> +
> +        excitation-current-microamp = <1000>;
> +        alpha-micro-ohms-per-ohm-celsius = <3908>;
> +        r-naught-ohms = <1000>;
> +    };
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d3ab0ccc34ab..a7279af85adb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8721,6 +8721,7 @@ L:	linux-iio@vger.kernel.org
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/iio/afe/current-sense-amplifier.yaml
>  F:	Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
> +F:	Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
>  F:	Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
>  F:	drivers/iio/afe/iio-rescale.c
>  
> -- 
> 2.30.1.489.g328c10930387

Re: [PATCH v1 7/9] dt-bindings: iio: afe: add binding for temperature-sense-rtd

[Jonathan Cameron]

On Fri, 4 Jun 2021 16:17:02 -0500
Rob Herring <robh@kernel.org> wrote:

> On Sat, May 29, 2021 at 08:59:15PM -0400, Liam Beguin wrote:
> > From: Liam Beguin <lvb@xiphos.com>
> > 
> > An ADC is often used to measure other quantities indirectly. This
> > binding describe one cases, the measurement of a temperature through
> > the voltage across an RTD resistor such as a PT1000.
> > 
> > Signed-off-by: Liam Beguin <lvb@xiphos.com>
> > ---
> >  .../iio/afe/temperature-sense-rtd.yaml        | 65 +++++++++++++++++++
> >  MAINTAINERS                                   |  1 +
> >  2 files changed, 66 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml b/Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
> > new file mode 100644
> > index 000000000000..4798eda6e533
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
> > @@ -0,0 +1,65 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/afe/temperature-sense-rtd.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Temperature Sense RTD
> > +
> > +maintainers:
> > +  - Liam Beguin <lvb@xiphos.com>
> > +
> > +description: |
> > +  When an io-channel measures the output voltage across an RTD such as a  
> 
> What's an RTD? Not defined anywhere here.
> 
> > +  PT1000, the interesting measurement is almost always the corresponding
> > +  temperature, not the voltage output. This binding describes such a circuit.
> > +
> > +properties:
> > +  compatible:
> > +    const: temperature-sense-rtd
> > +
> > +  io-channels:
> > +    maxItems: 1
> > +    description: |
> > +      Channel node of a voltage io-channel.
> > +
> > +  '#io-channel-cells':
> > +    const: 1  
> 
> Doesn't this belong in the provider?

Potentially this could in turn also be a provider if wired up to iio-hwmon or
similar.  Certainly an option for a temperature sensor.

> 
> > +
> > +  excitation-current-microamp:
> > +    description: The current fed through the RTD sensor.
> > +
> > +  alpha-micro-ohms-per-ohm-celsius:
> > +    description: |
> > +      Linear approximation of the resistance versus temperature relationship
> > +      between 0 and 100 degrees Celsius.
> > +
> > +      Pure platinum has an alpha of 3925. Industry standards such as IEC60751
> > +      and ASTM E-1137 specify an alpha of 3850.  
> 
> Is there a max and min value?
> 
> > +
> > +  r-naught-ohms:
> > +    description: |
> > +      Resistance of the sensor at 0 degrees Celsius.
> > +      Common values are 100 for PT100 and 1000 for PT1000.  
> 
> max or min?
> 
> > +
> > +additionalProperties: false  
> 
> blank line here.
> 
> > +required:
> > +  - compatible
> > +  - io-channels
> > +  - excitation-current-microamp
> > +  - alpha-micro-ohms-per-ohm-celsius
> > +  - r-naught-ohms
> > +
> > +examples:
> > +  - |
> > +    pt1000_1: iio-rescale0 {
> > +        compatible = "temperature-sense-rtd";
> > +        #io-channel-cells = <1>;
> > +        io-channels = <&temp_adc1 0>;
> > +
> > +        excitation-current-microamp = <1000>;
> > +        alpha-micro-ohms-per-ohm-celsius = <3908>;
> > +        r-naught-ohms = <1000>;
> > +    };
> > +
> > +...
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index d3ab0ccc34ab..a7279af85adb 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -8721,6 +8721,7 @@ L:	linux-iio@vger.kernel.org
> >  S:	Maintained
> >  F:	Documentation/devicetree/bindings/iio/afe/current-sense-amplifier.yaml
> >  F:	Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
> > +F:	Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
> >  F:	Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
> >  F:	drivers/iio/afe/iio-rescale.c
> >  
> > -- 
> > 2.30.1.489.g328c10930387  

[PATCH v1 8/9] dt-bindings: iio: afe: add binding for temperature-sense-current

[Liam Beguin]

From: Liam Beguin <lvb@xiphos.com>

An ADC is often used to measure other quantities indirectly. This
binding describe one cases, the measurement of a temperature through
a current sense amplifier (such as an AD590) and a sense resistor.

Signed-off-by: Liam Beguin <lvb@xiphos.com>
---
 .../iio/afe/temperature-sense-current.yaml    | 61 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 62 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml

diff --git a/Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml b/Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml
new file mode 100644
index 000000000000..1bac74486102
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/afe/temperature-sense-current.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Temperature Sense Current
+
+maintainers:
+  - Liam Beguin <lvb@xiphos.com>
+
+description: |
+  When an io-channel measures the output voltage for a temperature current
+  sense amplifier such as the AD950, the interesting measurement is almost
+  always the corresponding temperature, not the voltage output.
+  This binding describes such a circuit.
+
+properties:
+  compatible:
+    const: temperature-sense-current
+
+  io-channels:
+    maxItems: 1
+    description: |
+      Channel node of a voltage io-channel.
+
+  '#io-channel-cells':
+    const: 1
+
+  sense-resistor-ohms:
+    description: The sense resistance.
+
+  alpha-micro-amps-per-degree:
+    description: |
+      Linear output current gain of the temperature IC.
+
+  use-kelvin-scale:
+    type: boolean
+    description: |
+      Boolean indicating if alpha uses Kelvin degrees instead of Celsius.
+
+additionalProperties: false
+required:
+  - compatible
+  - io-channels
+  - sense-resistor-ohms
+  - alpha-micro-amps-per-degree
+
+examples:
+  - |
+    ad590: iio-rescale0 {
+        compatible = "temperature-sense-current";
+        #io-channel-cells = <1>;
+        io-channels = <&temp_adc 2>;
+
+        sense-resistor-ohms = <8060>;
+        use-kelvin-scale;
+        alpha-micro-amps-per-degree = <1>;
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index a7279af85adb..0eb7fcd94b66 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8721,6 +8721,7 @@ L:	linux-iio@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/iio/afe/current-sense-amplifier.yaml
 F:	Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
+F:	Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml
 F:	Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
 F:	Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
 F:	drivers/iio/afe/iio-rescale.c
-- 
2.30.1.489.g328c10930387

Re: [PATCH v1 8/9] dt-bindings: iio: afe: add binding for temperature-sense-current

[Peter Rosin]

Hi!

On 2021-05-30 02:59, Liam Beguin wrote:
> From: Liam Beguin <lvb@xiphos.com>
> 
> An ADC is often used to measure other quantities indirectly. This
> binding describe one cases, the measurement of a temperature through
> a current sense amplifier (such as an AD590) and a sense resistor.
> 
> Signed-off-by: Liam Beguin <lvb@xiphos.com>
> ---
>  .../iio/afe/temperature-sense-current.yaml    | 61 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml b/Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml
> new file mode 100644
> index 000000000000..1bac74486102
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/afe/temperature-sense-current.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Temperature Sense Current
> +
> +maintainers:
> +  - Liam Beguin <lvb@xiphos.com>
> +
> +description: |
> +  When an io-channel measures the output voltage for a temperature current
> +  sense amplifier such as the AD950, the interesting measurement is almost
> +  always the corresponding temperature, not the voltage output.
> +  This binding describes such a circuit.
> +
> +properties:
> +  compatible:
> +    const: temperature-sense-current
> +
> +  io-channels:
> +    maxItems: 1
> +    description: |
> +      Channel node of a voltage io-channel.
> +
> +  '#io-channel-cells':
> +    const: 1
> +
> +  sense-resistor-ohms:
> +    description: The sense resistance.
> +
> +  alpha-micro-amps-per-degree:
> +    description: |
> +      Linear output current gain of the temperature IC.
> +
> +  use-kelvin-scale:
> +    type: boolean
> +    description: |
> +      Boolean indicating if alpha uses Kelvin degrees instead of Celsius.

It's "kelvin", not "Kelvin degrees", and it's "degrees Celsius".

But what exactly is this property for? We always want degrees Celsius, don't we,
and any offset can be handled...with an offset. No?

Cheers,
Peter

> +
> +additionalProperties: false
> +required:
> +  - compatible
> +  - io-channels
> +  - sense-resistor-ohms
> +  - alpha-micro-amps-per-degree
> +
> +examples:
> +  - |
> +    ad590: iio-rescale0 {
> +        compatible = "temperature-sense-current";
> +        #io-channel-cells = <1>;
> +        io-channels = <&temp_adc 2>;
> +
> +        sense-resistor-ohms = <8060>;
> +        use-kelvin-scale;
> +        alpha-micro-amps-per-degree = <1>;
> +    };
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a7279af85adb..0eb7fcd94b66 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8721,6 +8721,7 @@ L:	linux-iio@vger.kernel.org
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/iio/afe/current-sense-amplifier.yaml
>  F:	Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
> +F:	Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml
>  F:	Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
>  F:	Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
>  F:	drivers/iio/afe/iio-rescale.c
> 

-- 
Peter Rosin
+46 730 746 224
Axentia Technologies AB

Re: [PATCH v1 8/9] dt-bindings: iio: afe: add binding for temperature-sense-current

[Peter Rosin]

On 2021-05-31 09:28, Peter Rosin wrote:
>> +  use-kelvin-scale:
>> +    type: boolean
>> +    description: |
>> +      Boolean indicating if alpha uses Kelvin degrees instead of Celsius.
> 
> It's "kelvin", not "Kelvin degrees", and it's "degrees Celsius".
> 
> But what exactly is this property for? We always want degrees Celsius, don't we,
> and any offset can be handled...with an offset. No?

Ahh, I hit send too soon. I now see that you calculate the offset in the driver
instead of requiring the devicetree author to do it "by hand".

Cheers,
Peter

Re: [PATCH v1 8/9] dt-bindings: iio: afe: add binding for temperature-sense-current

[Liam Beguin]

Hi Peter,

On Mon May 31, 2021 at 4:58 AM EDT, Peter Rosin wrote:
>
>
> On 2021-05-31 09:28, Peter Rosin wrote:
> >> +  use-kelvin-scale:
> >> +    type: boolean
> >> +    description: |
> >> +      Boolean indicating if alpha uses Kelvin degrees instead of Celsius.
> > 
> > It's "kelvin", not "Kelvin degrees", and it's "degrees Celsius".

I'll rephrase the description base on your comment.

I also thought of using alpha-micro-amps-per-degree-celsius and
alpha-micro-amps-kelvin instead. I don't know if that would be better.

Thanks,
Liam

> > 
> > But what exactly is this property for? We always want degrees Celsius, don't we,
> > and any offset can be handled...with an offset. No?
>
> Ahh, I hit send too soon. I now see that you calculate the offset in the
> driver
> instead of requiring the devicetree author to do it "by hand".
>
> Cheers,
> Peter

Re: [PATCH v1 8/9] dt-bindings: iio: afe: add binding for temperature-sense-current

[Jonathan Cameron]

On Sat, 29 May 2021 20:59:16 -0400
Liam Beguin <liambeguin@gmail.com> wrote:

> From: Liam Beguin <lvb@xiphos.com>
> 
> An ADC is often used to measure other quantities indirectly. This
> binding describe one cases, the measurement of a temperature through
> a current sense amplifier (such as an AD590) and a sense resistor.
> 
> Signed-off-by: Liam Beguin <lvb@xiphos.com>
> ---
>  .../iio/afe/temperature-sense-current.yaml    | 61 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml b/Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml
> new file mode 100644
> index 000000000000..1bac74486102
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/afe/temperature-sense-current.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Temperature Sense Current
> +
> +maintainers:
> +  - Liam Beguin <lvb@xiphos.com>
> +
> +description: |
> +  When an io-channel measures the output voltage for a temperature current
> +  sense amplifier such as the AD950, the interesting measurement is almost
> +  always the corresponding temperature, not the voltage output.
> +  This binding describes such a circuit.
> +
> +properties:
> +  compatible:
> +    const: temperature-sense-current

What about such a sensor connected to a current ADC? That was what I was
immediately expecting when I saw the naming.

You could daisy chain this with the current-sense-resistor AFE though
that does seem overly messy.

Anyhow this should be called something that reflects the presence of
that sense resitor.

> +
> +  io-channels:
> +    maxItems: 1
> +    description: |
> +      Channel node of a voltage io-channel.
> +
> +  '#io-channel-cells':
> +    const: 1
> +
> +  sense-resistor-ohms:
> +    description: The sense resistance.
> +
> +  alpha-micro-amps-per-degree:
> +    description: |
> +      Linear output current gain of the temperature IC.
> +
> +  use-kelvin-scale:
> +    type: boolean
> +    description: |
> +      Boolean indicating if alpha uses Kelvin degrees instead of Celsius.
> +
> +additionalProperties: false
> +required:
> +  - compatible
> +  - io-channels
> +  - sense-resistor-ohms
> +  - alpha-micro-amps-per-degree
> +
> +examples:
> +  - |
> +    ad590: iio-rescale0 {
> +        compatible = "temperature-sense-current";
> +        #io-channel-cells = <1>;
> +        io-channels = <&temp_adc 2>;
> +
> +        sense-resistor-ohms = <8060>;
> +        use-kelvin-scale;
> +        alpha-micro-amps-per-degree = <1>;
> +    };
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a7279af85adb..0eb7fcd94b66 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8721,6 +8721,7 @@ L:	linux-iio@vger.kernel.org
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/iio/afe/current-sense-amplifier.yaml
>  F:	Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
> +F:	Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml
>  F:	Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
>  F:	Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
>  F:	drivers/iio/afe/iio-rescale.c

Re: [PATCH v1 8/9] dt-bindings: iio: afe: add binding for temperature-sense-current

[Rob Herring]

On Sat, May 29, 2021 at 08:59:16PM -0400, Liam Beguin wrote:
> From: Liam Beguin <lvb@xiphos.com>
> 
> An ADC is often used to measure other quantities indirectly. This
> binding describe one cases, the measurement of a temperature through
> a current sense amplifier (such as an AD590) and a sense resistor.
> 
> Signed-off-by: Liam Beguin <lvb@xiphos.com>
> ---
>  .../iio/afe/temperature-sense-current.yaml    | 61 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml b/Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml
> new file mode 100644
> index 000000000000..1bac74486102
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/afe/temperature-sense-current.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Temperature Sense Current
> +
> +maintainers:
> +  - Liam Beguin <lvb@xiphos.com>
> +
> +description: |
> +  When an io-channel measures the output voltage for a temperature current
> +  sense amplifier such as the AD950, the interesting measurement is almost
> +  always the corresponding temperature, not the voltage output.
> +  This binding describes such a circuit.
> +
> +properties:
> +  compatible:
> +    const: temperature-sense-current
> +
> +  io-channels:
> +    maxItems: 1
> +    description: |
> +      Channel node of a voltage io-channel.
> +
> +  '#io-channel-cells':
> +    const: 1

Belongs in provider unless this is a consumer and provider.

> +
> +  sense-resistor-ohms:
> +    description: The sense resistance.
> +
> +  alpha-micro-amps-per-degree:
> +    description: |
> +      Linear output current gain of the temperature IC.
> +
> +  use-kelvin-scale:
> +    type: boolean
> +    description: |
> +      Boolean indicating if alpha uses Kelvin degrees instead of Celsius.
> +
> +additionalProperties: false

blank line

> +required:
> +  - compatible
> +  - io-channels
> +  - sense-resistor-ohms
> +  - alpha-micro-amps-per-degree
> +
> +examples:
> +  - |
> +    ad590: iio-rescale0 {
> +        compatible = "temperature-sense-current";
> +        #io-channel-cells = <1>;
> +        io-channels = <&temp_adc 2>;
> +
> +        sense-resistor-ohms = <8060>;
> +        use-kelvin-scale;
> +        alpha-micro-amps-per-degree = <1>;
> +    };
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a7279af85adb..0eb7fcd94b66 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8721,6 +8721,7 @@ L:	linux-iio@vger.kernel.org
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/iio/afe/current-sense-amplifier.yaml
>  F:	Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
> +F:	Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml
>  F:	Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
>  F:	Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
>  F:	drivers/iio/afe/iio-rescale.c
> -- 
> 2.30.1.489.g328c10930387

[PATCH v1 9/9] dt-bindings: iio: afe: add binding for temperature-sense-amplifier

[Liam Beguin]

From: Liam Beguin <lvb@xiphos.com>

An ADC is often used to measure other quantities indirectly. This
binding describe one cases, the measurement of a temperature through a
voltage sense amplifier such as the LTC2997.

Signed-off-by: Liam Beguin <lvb@xiphos.com>
---
 .../iio/afe/temperature-sense-amplifier.yaml  | 55 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 56 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/afe/temperature-sense-amplifier.yaml

diff --git a/Documentation/devicetree/bindings/iio/afe/temperature-sense-amplifier.yaml b/Documentation/devicetree/bindings/iio/afe/temperature-sense-amplifier.yaml
new file mode 100644
index 000000000000..015413cbffbc
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/afe/temperature-sense-amplifier.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/afe/temperature-sense-amplifier.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Temperature Sense Amplifier
+
+maintainers:
+  - Liam Beguin <lvb@xiphos.com>
+
+description: |
+  When an io-channel measures the output voltage of a temperature IC such as
+  the LTC2997, the interesting measurement is almost always the corresponding
+  temperature, not the voltage output. This binding describes such a circuit.
+
+properties:
+  compatible:
+    const: temperature-sense-amplifier
+
+  io-channels:
+    maxItems: 1
+    description: |
+      Channel node of a voltage io-channel.
+
+  '#io-channel-cells':
+    const: 1
+
+  alpha-micro-volts-per-degree:
+    description: |
+      Output voltage gain of the temperature IC.
+
+  use-kelvin-scale:
+    type: boolean
+    description: |
+      Boolean indicating if alpha uses Kelvin degrees instead of Celsius.
+
+additionalProperties: false
+required:
+  - compatible
+  - io-channels
+  - alpha-micro-volts-per-degree
+
+examples:
+  - |
+    znq_temp: iio-rescale0 {
+        compatible = "temperature-sense-amplifier";
+        #io-channel-cells = <1>;
+        io-channels = <&temp_adc 3>;
+
+        use-kelvin-scale;
+        alpha-micro-volts-per-degree = <4000>;
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 0eb7fcd94b66..f224bd8e6125 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8721,6 +8721,7 @@ L:	linux-iio@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/iio/afe/current-sense-amplifier.yaml
 F:	Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
+F:	Documentation/devicetree/bindings/iio/afe/temperature-sense-amplifier.yaml
 F:	Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml
 F:	Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
 F:	Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
-- 
2.30.1.489.g328c10930387

Re: [PATCH v1 9/9] dt-bindings: iio: afe: add binding for temperature-sense-amplifier

[Peter Rosin]

Hi!

On 2021-05-30 02:59, Liam Beguin wrote:
> From: Liam Beguin <lvb@xiphos.com>
> 
> An ADC is often used to measure other quantities indirectly. This
> binding describe one cases, the measurement of a temperature through a
> voltage sense amplifier such as the LTC2997.
> 
> Signed-off-by: Liam Beguin <lvb@xiphos.com>

What's the significant difference between this and the RTD binding? Does
not both simply scale/offset a voltage to a temperature?

Cheers,
Peter

Re: [PATCH v1 9/9] dt-bindings: iio: afe: add binding for temperature-sense-amplifier

[Liam Beguin]

Hi Peter,

On Mon May 31, 2021 at 3:32 AM EDT, Peter Rosin wrote:
> Hi!
>
> On 2021-05-30 02:59, Liam Beguin wrote:
> > From: Liam Beguin <lvb@xiphos.com>
> > 
> > An ADC is often used to measure other quantities indirectly. This
> > binding describe one cases, the measurement of a temperature through a
> > voltage sense amplifier such as the LTC2997.
> > 
> > Signed-off-by: Liam Beguin <lvb@xiphos.com>
>
> What's the significant difference between this and the RTD binding? Does
> not both simply scale/offset a voltage to a temperature?
>

The way I looked at it was one binding per sensor type (resistance
driven, current driven, and voltage driven).

Thinking about it more, these three bindings could be factorized into
one if the user is required to enter parameters "by hand".

These could become something like:
- sense-gain-mult
- sense-gain-div
- sense-offset

I like the idea of having the "datasheet parameters" in the devicetree,
but this would be a lot more versatile.

What do you think?

Cheers,
Liam

> Cheers,
> Peter

Re: [PATCH v1 9/9] dt-bindings: iio: afe: add binding for temperature-sense-amplifier

[Jonathan Cameron]

On Mon, 31 May 2021 10:03:23 -0400
"Liam Beguin" <liambeguin@gmail.com> wrote:

> Hi Peter,
> 
> On Mon May 31, 2021 at 3:32 AM EDT, Peter Rosin wrote:
> > Hi!
> >
> > On 2021-05-30 02:59, Liam Beguin wrote:  
> > > From: Liam Beguin <lvb@xiphos.com>
> > > 
> > > An ADC is often used to measure other quantities indirectly. This
> > > binding describe one cases, the measurement of a temperature through a
> > > voltage sense amplifier such as the LTC2997.
> > > 
> > > Signed-off-by: Liam Beguin <lvb@xiphos.com>  
> >
> > What's the significant difference between this and the RTD binding? Does
> > not both simply scale/offset a voltage to a temperature?

I'm lost: what RTD binding?

> >  
> 
> The way I looked at it was one binding per sensor type (resistance
> driven, current driven, and voltage driven).
> 
> Thinking about it more, these three bindings could be factorized into
> one if the user is required to enter parameters "by hand".

Don't. They are effectively different types of devices and we just end
up with a more complex binding if we try to cover them all.

There is an argument to go the other way and actually have bindings for
individual temperature sensors like the LTC2997.  Then the parameters
become a driver problem rather than one for the binding.

Jonathan


> 
> These could become something like:
> - sense-gain-mult
> - sense-gain-div
> - sense-offset
> 
> I like the idea of having the "datasheet parameters" in the devicetree,
> but this would be a lot more versatile.
> 
> What do you think?
> 
> Cheers,
> Liam
> 
> > Cheers,
> > Peter  
> 

Re: [PATCH v1 9/9] dt-bindings: iio: afe: add binding for temperature-sense-amplifier

[Jonathan Cameron]

On Tue, 1 Jun 2021 17:02:51 +0100
Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:

> On Mon, 31 May 2021 10:03:23 -0400
> "Liam Beguin" <liambeguin@gmail.com> wrote:
> 
> > Hi Peter,
> > 
> > On Mon May 31, 2021 at 3:32 AM EDT, Peter Rosin wrote:  
> > > Hi!
> > >
> > > On 2021-05-30 02:59, Liam Beguin wrote:    
> > > > From: Liam Beguin <lvb@xiphos.com>
> > > > 
> > > > An ADC is often used to measure other quantities indirectly. This
> > > > binding describe one cases, the measurement of a temperature through a
> > > > voltage sense amplifier such as the LTC2997.
> > > > 
> > > > Signed-off-by: Liam Beguin <lvb@xiphos.com>    
> > >
> > > What's the significant difference between this and the RTD binding? Does
> > > not both simply scale/offset a voltage to a temperature?  
> 
> I'm lost: what RTD binding?
Ignore this email - I was reading the series backwards and thought we were
talking about a preexisting binding.

> 
> > >    
> > 
> > The way I looked at it was one binding per sensor type (resistance
> > driven, current driven, and voltage driven).
> > 
> > Thinking about it more, these three bindings could be factorized into
> > one if the user is required to enter parameters "by hand".  
> 
> Don't. They are effectively different types of devices and we just end
> up with a more complex binding if we try to cover them all.
Ignore that as well. If the bindings combine fairly easily it is nice
to do so, but be careful not to throw too many things in together and
make it very hard to write the binding. However, I'm not keen on entirely
generic bindings and would like the channel type at least to come from
the compatible.

> 
> There is an argument to go the other way and actually have bindings for
> individual temperature sensors like the LTC2997.  Then the parameters
> become a driver problem rather than one for the binding.
> 
> Jonathan
> 
> 
> > 
> > These could become something like:
> > - sense-gain-mult
> > - sense-gain-div
> > - sense-offset
> > 
> > I like the idea of having the "datasheet parameters" in the devicetree,
> > but this would be a lot more versatile.
> > 
> > What do you think?
> > 
> > Cheers,
> > Liam
> >   
> > > Cheers,
> > > Peter    
> >   
> 

Re: [PATCH v1 9/9] dt-bindings: iio: afe: add binding for temperature-sense-amplifier

[Jonathan Cameron]

On Sat, 29 May 2021 20:59:17 -0400
Liam Beguin <liambeguin@gmail.com> wrote:

> From: Liam Beguin <lvb@xiphos.com>
> 
> An ADC is often used to measure other quantities indirectly. This
> binding describe one cases, the measurement of a temperature through a
> voltage sense amplifier such as the LTC2997.
> 
> Signed-off-by: Liam Beguin <lvb@xiphos.com>
> ---
>  .../iio/afe/temperature-sense-amplifier.yaml  | 55 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 56 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/afe/temperature-sense-amplifier.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/afe/temperature-sense-amplifier.yaml b/Documentation/devicetree/bindings/iio/afe/temperature-sense-amplifier.yaml
> new file mode 100644
> index 000000000000..015413cbffbc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/afe/temperature-sense-amplifier.yaml
> @@ -0,0 +1,55 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/afe/temperature-sense-amplifier.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Temperature Sense Amplifier
> +
> +maintainers:
> +  - Liam Beguin <lvb@xiphos.com>
> +
> +description: |
> +  When an io-channel measures the output voltage of a temperature IC such as
> +  the LTC2997, the interesting measurement is almost always the corresponding
> +  temperature, not the voltage output. This binding describes such a circuit.
> +
> +properties:
> +  compatible:
> +    const: temperature-sense-amplifier
> +
> +  io-channels:
> +    maxItems: 1
> +    description: |
> +      Channel node of a voltage io-channel.
> +
> +  '#io-channel-cells':
> +    const: 1
> +
> +  alpha-micro-volts-per-degree:

Include units in the naming.

micro-volts-per-degree-celsius: perhaps?
That will then get the type from dt-schema/schema/property-units.
Though amusing it will identify it based on celsius, when the units are arguably
volts.


> +    description: |
> +      Output voltage gain of the temperature IC.
> +
> +  use-kelvin-scale:
> +    type: boolean
> +    description: |
> +      Boolean indicating if alpha uses Kelvin degrees instead of Celsius.

I'm not clear why that change would make any difference to alpha?  It would make
a difference to an offset though (and you should allow for one of those if
you want this to be generic).

Pick one and stick to it for all cases.  It might make the dts author do
some simple maths but that is preferable to having this flexibility
when we don't need it.

> +
> +additionalProperties: false
> +required:
> +  - compatible
> +  - io-channels
> +  - alpha-micro-volts-per-degree
> +
> +examples:
> +  - |
> +    znq_temp: iio-rescale0 {

The end result is a temperature sensor, so this should
have a name reflecting that.  Here that would be
temperature-sensor as per the dt schema specification:
https://www.devicetree.org/specifications/

> +        compatible = "temperature-sense-amplifier";
> +        #io-channel-cells = <1>;
> +        io-channels = <&temp_adc 3>;
> +
> +        use-kelvin-scale;
> +        alpha-micro-volts-per-degree = <4000>;
> +    };
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0eb7fcd94b66..f224bd8e6125 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8721,6 +8721,7 @@ L:	linux-iio@vger.kernel.org
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/iio/afe/current-sense-amplifier.yaml
>  F:	Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml
> +F:	Documentation/devicetree/bindings/iio/afe/temperature-sense-amplifier.yaml
>  F:	Documentation/devicetree/bindings/iio/afe/temperature-sense-current.yaml
>  F:	Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
>  F:	Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml