[RFC PATCH v1 0/2] hwmon: (iio_hwmon) optionally force iio channel type

[RFC PATCH v1 0/2] hwmon: (iio_hwmon) optionally force iio channel type

[Liam Beguin]

Add a devicetree binding to optionally force a different IIO channel
type.

This is useful in cases where ADC channels are connected to a circuit
that represent another unit such as a temperature or a current.

`channel-types` was chosen instead of `io-channel-types` as this is not
part of the iio consumer bindings.

In the current form, this patch does what it's intended to do:
change the unit displayed by `sensors`, but feels like the wrong way to
address the problem.

Would it be possible to force the type of different IIO channels for
this kind of use case with a devicetree binding from the IIO subsystem?

It would be convenient to do it within the IIO subsystem to have the
right unit there too.

Thanks for your time,
Liam

Liam Beguin (2):
  hwmon: (iio_hwmon) optionally force iio channel type
  dt-bindings: hwmon: add iio-hwmon bindings

 .../devicetree/bindings/hwmon/iio-hwmon.yaml  | 41 +++++++++++++++++++
 drivers/hwmon/iio_hwmon.c                     |  2 +
 2 files changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/iio-hwmon.yaml


base-commit: 9f4ad9e425a1d3b6a34617b8ea226d56a119a717
-- 
2.30.1.489.g328c10930387

[RFC PATCH v1 1/2] hwmon: (iio_hwmon) optionally force iio channel type

[Liam Beguin]

Add a devicetree binding to optionally force a different IIO channel
type.

This is useful in cases where ADC channels are connected to a circuit
that represent another unit such as a temperature or a current.

`channel-types` was chosen instead of `io-channel-types` as this is not
part of the iio consumer bindings.

Signed-off-by: Liam Beguin <liambeguin@gmail.com>
---
 drivers/hwmon/iio_hwmon.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hwmon/iio_hwmon.c b/drivers/hwmon/iio_hwmon.c
index 580a7d125b88..365ea2359b22 100644
--- a/drivers/hwmon/iio_hwmon.c
+++ b/drivers/hwmon/iio_hwmon.c
@@ -109,6 +109,8 @@ static int iio_hwmon_probe(struct platform_device *pdev)
 		if (ret < 0)
 			return ret;
 
+		of_property_read_u32_index(dev->of_node, "channel-types",
+					   i, &type);
 		switch (type) {
 		case IIO_VOLTAGE:
 			n = in_i++;
-- 
2.30.1.489.g328c10930387

[RFC PATCH v1 2/2] dt-bindings: hwmon: add iio-hwmon bindings

[Liam Beguin]

Document devicetree bindings for the iio-hwmon driver.
Also add documentation for the channel-types option.

Signed-off-by: Liam Beguin <liambeguin@gmail.com>
---
 .../devicetree/bindings/hwmon/iio-hwmon.yaml  | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/iio-hwmon.yaml

diff --git a/Documentation/devicetree/bindings/hwmon/iio-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/iio-hwmon.yaml
new file mode 100644
index 000000000000..fb8b437112a5
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/iio-hwmon.yaml
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/hwmon/iio-hwmon.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: bindings for the IIO hwmon bridge
+
+maintainers:
+  - Jean Delvare <jdelvare@suse.com>
+  - Guenter Roeck <linux@roeck-us.net>
+
+properties:
+  compatible:
+    enum:
+      - iio-hwmon
+
+  io-channels:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+
+  io-channel-names:
+    $ref: /schemas/types.yaml#/definitions/string-array
+
+  channel-types:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description: |
+      an array of channel types used to override the IIO type of each channel.
+
+additionalProperties: false
+
+examples:
+  - |
+    #define IIO_VOLTAGE 0
+    #define IIO_CURRENT 1
+    iio_hwmon_adc0 {
+        compatible = "iio-hwmon";
+        io-channels = <&adc 0>, <&adc 1>;
+        io-channel-names = "input_current", "input_voltage" ;
+        channel-types = <IIO_CURRENT>, <IIO_VOLTAGE>;
+    };
-- 
2.30.1.489.g328c10930387

Re: [RFC PATCH v1 0/2] hwmon: (iio_hwmon) optionally force iio channel type

[Guenter Roeck]

On 5/15/21 9:43 PM, Liam Beguin wrote:
> Add a devicetree binding to optionally force a different IIO channel
> type.
> 
> This is useful in cases where ADC channels are connected to a circuit
> that represent another unit such as a temperature or a current.
> 
> `channel-types` was chosen instead of `io-channel-types` as this is not
> part of the iio consumer bindings.
> 
> In the current form, this patch does what it's intended to do:
> change the unit displayed by `sensors`, but feels like the wrong way to
> address the problem.
> 
> Would it be possible to force the type of different IIO channels for
> this kind of use case with a devicetree binding from the IIO subsystem?
> 

That doesn't make sense to me. If an ADC is used to report temperatures,
it would be a thermistor, and the ntc_thermistor driver should be used.
Not sure what to do with currents, but overriding "voltage" with "current"
seems wrong.

Guenter

> It would be convenient to do it within the IIO subsystem to have the
> right unit there too.
> 
> Thanks for your time,
> Liam
> 
> Liam Beguin (2):
>    hwmon: (iio_hwmon) optionally force iio channel type
>    dt-bindings: hwmon: add iio-hwmon bindings
> 
>   .../devicetree/bindings/hwmon/iio-hwmon.yaml  | 41 +++++++++++++++++++
>   drivers/hwmon/iio_hwmon.c                     |  2 +
>   2 files changed, 43 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/hwmon/iio-hwmon.yaml
> 
> 
> base-commit: 9f4ad9e425a1d3b6a34617b8ea226d56a119a717
> 

Re: [RFC PATCH v1 0/2] hwmon: (iio_hwmon) optionally force iio channel type

[Jonathan Cameron]

On Sun, 16 May 2021 00:43:13 -0400
Liam Beguin <liambeguin@gmail.com> wrote:

> Add a devicetree binding to optionally force a different IIO channel
> type.
> 
> This is useful in cases where ADC channels are connected to a circuit
> that represent another unit such as a temperature or a current.
> 
> `channel-types` was chosen instead of `io-channel-types` as this is not
> part of the iio consumer bindings.
> 
> In the current form, this patch does what it's intended to do:
> change the unit displayed by `sensors`, but feels like the wrong way to
> address the problem.
> 
> Would it be possible to force the type of different IIO channels for
> this kind of use case with a devicetree binding from the IIO subsystem?
> 
> It would be convenient to do it within the IIO subsystem to have the
> right unit there too.
> 
> Thanks for your time,
> Liam

Hi Liam,

+CC Peter for AFE part.

It's an interesting approach, but I would suggest we think about this
a different way.

Whenever a channel is being used to measure something 'different' from
what it actually measures (e.g. a voltage ADC measuring a current) that
reflects their being some analog component involved.
If you look at drivers/iio/afe/iio-rescale.c you can see the approach
we currently use to handle this.

Effectively what you add to devicetree is a consumer of the ADC channel
which in turn provides services to other devices. For this current case
it would be either a current-sense-amplifier or a current-sense-shunt 
depending on what the analog front end looks like.  We have to describe
the characteristics of that front end which isn't something that can
be done via a simple channel type.

That afe consumer device can then provide services to another consumer
(e.g. iio-hwmon) which work for your usecase.

The main limitation of this approach currently is you end up with
one device per channel.  That could be improved upon if you have a usecase
where it matters.

I don't think we currently have an equivalent for temperature sensing
but it would be easy enough to do something similar.

Jonathan


> 
> Liam Beguin (2):
>   hwmon: (iio_hwmon) optionally force iio channel type
>   dt-bindings: hwmon: add iio-hwmon bindings
> 
>  .../devicetree/bindings/hwmon/iio-hwmon.yaml  | 41 +++++++++++++++++++
>  drivers/hwmon/iio_hwmon.c                     |  2 +
>  2 files changed, 43 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/iio-hwmon.yaml
> 
> 
> base-commit: 9f4ad9e425a1d3b6a34617b8ea226d56a119a717

Re: [RFC PATCH v1 0/2] hwmon: (iio_hwmon) optionally force iio channel type

[Liam Beguin]

Hi Guenter,

On Sun May 16, 2021 at 4:56 AM EDT, Guenter Roeck wrote:
> On 5/15/21 9:43 PM, Liam Beguin wrote:
> > Add a devicetree binding to optionally force a different IIO channel
> > type.
> > 
> > This is useful in cases where ADC channels are connected to a circuit
> > that represent another unit such as a temperature or a current.
> > 
> > `channel-types` was chosen instead of `io-channel-types` as this is not
> > part of the iio consumer bindings.
> > 
> > In the current form, this patch does what it's intended to do:
> > change the unit displayed by `sensors`, but feels like the wrong way to
> > address the problem.
> > 
> > Would it be possible to force the type of different IIO channels for
> > this kind of use case with a devicetree binding from the IIO subsystem?
> > 
>
> That doesn't make sense to me. If an ADC is used to report temperatures,
> it would be a thermistor, and the ntc_thermistor driver should be used.
> Not sure what to do with currents, but overriding "voltage" with
> "current"
> seems wrong.

Thanks for pointing out the ntc_thermistor.
It makes sense that the ADC channel would become a thermistor.
I'll have a look and see if it fits my use case.

Liam

>
> Guenter
>
> > It would be convenient to do it within the IIO subsystem to have the
> > right unit there too.
> > 
> > Thanks for your time,
> > Liam
> > 
> > Liam Beguin (2):
> >    hwmon: (iio_hwmon) optionally force iio channel type
> >    dt-bindings: hwmon: add iio-hwmon bindings
> > 
> >   .../devicetree/bindings/hwmon/iio-hwmon.yaml  | 41 +++++++++++++++++++
> >   drivers/hwmon/iio_hwmon.c                     |  2 +
> >   2 files changed, 43 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/hwmon/iio-hwmon.yaml
> > 
> > 
> > base-commit: 9f4ad9e425a1d3b6a34617b8ea226d56a119a717
> > 

Re: [RFC PATCH v1 0/2] hwmon: (iio_hwmon) optionally force iio channel type

[Liam Beguin]

Hi Jonathan,

On Sun May 16, 2021 at 5:06 AM EDT, Jonathan Cameron wrote:
> On Sun, 16 May 2021 00:43:13 -0400
> Liam Beguin <liambeguin@gmail.com> wrote:
>
> > Add a devicetree binding to optionally force a different IIO channel
> > type.
> > 
> > This is useful in cases where ADC channels are connected to a circuit
> > that represent another unit such as a temperature or a current.
> > 
> > `channel-types` was chosen instead of `io-channel-types` as this is not
> > part of the iio consumer bindings.
> > 
> > In the current form, this patch does what it's intended to do:
> > change the unit displayed by `sensors`, but feels like the wrong way to
> > address the problem.
> > 
> > Would it be possible to force the type of different IIO channels for
> > this kind of use case with a devicetree binding from the IIO subsystem?
> > 
> > It would be convenient to do it within the IIO subsystem to have the
> > right unit there too.
> > 
> > Thanks for your time,
> > Liam
>
> Hi Liam,
>
> +CC Peter for AFE part.
>
> It's an interesting approach, but I would suggest we think about this
> a different way.
>
> Whenever a channel is being used to measure something 'different' from
> what it actually measures (e.g. a voltage ADC measuring a current) that
> reflects their being some analog component involved.
> If you look at drivers/iio/afe/iio-rescale.c you can see the approach
> we currently use to handle this.

Many thanks for pointing out the AFE code. That look like what I was
hoping to accomplish, but in a much better way.

>
> Effectively what you add to devicetree is a consumer of the ADC channel
> which in turn provides services to other devices. For this current case
> it would be either a current-sense-amplifier or a current-sense-shunt
> depending on what the analog front end looks like. We have to describe
> the characteristics of that front end which isn't something that can
> be done via a simple channel type.
>

Understood. My original intention was to use sensors.conf to do the
conversions and take into accounts those parameters.

> That afe consumer device can then provide services to another consumer
> (e.g. iio-hwmon) which work for your usecase.
>
> The main limitation of this approach currently is you end up with
> one device per channel. That could be improved upon if you have a
> usecase
> where it matters.
>
> I don't think we currently have an equivalent for temperature sensing
> but it would be easy enough to do something similar.

Wonderful, thanks again for pointing out the AFE!

Liam

>
> Jonathan
>
>
> > 
> > Liam Beguin (2):
> >   hwmon: (iio_hwmon) optionally force iio channel type
> >   dt-bindings: hwmon: add iio-hwmon bindings
> > 
> >  .../devicetree/bindings/hwmon/iio-hwmon.yaml  | 41 +++++++++++++++++++
> >  drivers/hwmon/iio_hwmon.c                     |  2 +
> >  2 files changed, 43 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/hwmon/iio-hwmon.yaml
> > 
> > 
> > base-commit: 9f4ad9e425a1d3b6a34617b8ea226d56a119a717

Re: [RFC PATCH v1 0/2] hwmon: (iio_hwmon) optionally force iio channel type

[Guenter Roeck]

On 5/16/21 8:02 AM, Liam Beguin wrote:
> Hi Jonathan,
> 
> On Sun May 16, 2021 at 5:06 AM EDT, Jonathan Cameron wrote:
>> On Sun, 16 May 2021 00:43:13 -0400
>> Liam Beguin <liambeguin@gmail.com> wrote:
>>
>>> Add a devicetree binding to optionally force a different IIO channel
>>> type.
>>>
>>> This is useful in cases where ADC channels are connected to a circuit
>>> that represent another unit such as a temperature or a current.
>>>
>>> `channel-types` was chosen instead of `io-channel-types` as this is not
>>> part of the iio consumer bindings.
>>>
>>> In the current form, this patch does what it's intended to do:
>>> change the unit displayed by `sensors`, but feels like the wrong way to
>>> address the problem.
>>>
>>> Would it be possible to force the type of different IIO channels for
>>> this kind of use case with a devicetree binding from the IIO subsystem?
>>>
>>> It would be convenient to do it within the IIO subsystem to have the
>>> right unit there too.
>>>
>>> Thanks for your time,
>>> Liam
>>
>> Hi Liam,
>>
>> +CC Peter for AFE part.
>>
>> It's an interesting approach, but I would suggest we think about this
>> a different way.
>>
>> Whenever a channel is being used to measure something 'different' from
>> what it actually measures (e.g. a voltage ADC measuring a current) that
>> reflects their being some analog component involved.
>> If you look at drivers/iio/afe/iio-rescale.c you can see the approach
>> we currently use to handle this.
> 
> Many thanks for pointing out the AFE code. That look like what I was
> hoping to accomplish, but in a much better way.
> 
>>
>> Effectively what you add to devicetree is a consumer of the ADC channel
>> which in turn provides services to other devices. For this current case
>> it would be either a current-sense-amplifier or a current-sense-shunt
>> depending on what the analog front end looks like. We have to describe
>> the characteristics of that front end which isn't something that can
>> be done via a simple channel type.
>>
> 
> Understood. My original intention was to use sensors.conf to do the
> conversions and take into accounts those parameters.
> 
>> That afe consumer device can then provide services to another consumer
>> (e.g. iio-hwmon) which work for your usecase.
>>
>> The main limitation of this approach currently is you end up with
>> one device per channel. That could be improved upon if you have a
>> usecase
>> where it matters.
>>
>> I don't think we currently have an equivalent for temperature sensing
>> but it would be easy enough to do something similar.
> 
> Wonderful, thanks again for pointing out the AFE!
> 

Please don't reinvent the ntc_thermistor driver.

Thanks,
Guenter

> Liam
> 
>>
>> Jonathan
>>
>>
>>>
>>> Liam Beguin (2):
>>>    hwmon: (iio_hwmon) optionally force iio channel type
>>>    dt-bindings: hwmon: add iio-hwmon bindings
>>>
>>>   .../devicetree/bindings/hwmon/iio-hwmon.yaml  | 41 +++++++++++++++++++
>>>   drivers/hwmon/iio_hwmon.c                     |  2 +
>>>   2 files changed, 43 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/hwmon/iio-hwmon.yaml
>>>
>>>
>>> base-commit: 9f4ad9e425a1d3b6a34617b8ea226d56a119a717
> 

Re: [RFC PATCH v1 0/2] hwmon: (iio_hwmon) optionally force iio channel type

[Jonathan Cameron]

On Sun, 16 May 2021 08:54:06 -0700
Guenter Roeck <linux@roeck-us.net> wrote:

> On 5/16/21 8:02 AM, Liam Beguin wrote:
> > Hi Jonathan,
> > 
> > On Sun May 16, 2021 at 5:06 AM EDT, Jonathan Cameron wrote:  
> >> On Sun, 16 May 2021 00:43:13 -0400
> >> Liam Beguin <liambeguin@gmail.com> wrote:
> >>  
> >>> Add a devicetree binding to optionally force a different IIO channel
> >>> type.
> >>>
> >>> This is useful in cases where ADC channels are connected to a circuit
> >>> that represent another unit such as a temperature or a current.
> >>>
> >>> `channel-types` was chosen instead of `io-channel-types` as this is not
> >>> part of the iio consumer bindings.
> >>>
> >>> In the current form, this patch does what it's intended to do:
> >>> change the unit displayed by `sensors`, but feels like the wrong way to
> >>> address the problem.
> >>>
> >>> Would it be possible to force the type of different IIO channels for
> >>> this kind of use case with a devicetree binding from the IIO subsystem?
> >>>
> >>> It would be convenient to do it within the IIO subsystem to have the
> >>> right unit there too.
> >>>
> >>> Thanks for your time,
> >>> Liam  
> >>
> >> Hi Liam,
> >>
> >> +CC Peter for AFE part.
> >>
> >> It's an interesting approach, but I would suggest we think about this
> >> a different way.
> >>
> >> Whenever a channel is being used to measure something 'different' from
> >> what it actually measures (e.g. a voltage ADC measuring a current) that
> >> reflects their being some analog component involved.
> >> If you look at drivers/iio/afe/iio-rescale.c you can see the approach
> >> we currently use to handle this.  
> > 
> > Many thanks for pointing out the AFE code. That look like what I was
> > hoping to accomplish, but in a much better way.
> >   
> >>
> >> Effectively what you add to devicetree is a consumer of the ADC channel
> >> which in turn provides services to other devices. For this current case
> >> it would be either a current-sense-amplifier or a current-sense-shunt
> >> depending on what the analog front end looks like. We have to describe
> >> the characteristics of that front end which isn't something that can
> >> be done via a simple channel type.
> >>  
> > 
> > Understood. My original intention was to use sensors.conf to do the
> > conversions and take into accounts those parameters.
> >   
> >> That afe consumer device can then provide services to another consumer
> >> (e.g. iio-hwmon) which work for your usecase.
> >>
> >> The main limitation of this approach currently is you end up with
> >> one device per channel. That could be improved upon if you have a
> >> usecase
> >> where it matters.
> >>
> >> I don't think we currently have an equivalent for temperature sensing
> >> but it would be easy enough to do something similar.  
> > 
> > Wonderful, thanks again for pointing out the AFE!
> >   
> 
> Please don't reinvent the ntc_thermistor driver.
Agreed, I'd forgotten it existed :(  Had a feeling we'd solved that problem before
but couldn't remember the name of the driver.

The afe driver already deals with current / voltage scaling and conversion
for common analog circuits. Potential dividers, current shunts etc, but they
are all the linear cases IIRC.

ntc_thermistor deals with the much more complex job of dealing with a thermistor.

Thanks,

Jonathan

> 
> Thanks,
> Guenter
> 
> > Liam
> >   
> >>
> >> Jonathan
> >>
> >>  
> >>>
> >>> Liam Beguin (2):
> >>>    hwmon: (iio_hwmon) optionally force iio channel type
> >>>    dt-bindings: hwmon: add iio-hwmon bindings
> >>>
> >>>   .../devicetree/bindings/hwmon/iio-hwmon.yaml  | 41 +++++++++++++++++++
> >>>   drivers/hwmon/iio_hwmon.c                     |  2 +
> >>>   2 files changed, 43 insertions(+)
> >>>   create mode 100644 Documentation/devicetree/bindings/hwmon/iio-hwmon.yaml
> >>>
> >>>
> >>> base-commit: 9f4ad9e425a1d3b6a34617b8ea226d56a119a717  
> >   
> 

Re: [RFC PATCH v1 0/2] hwmon: (iio_hwmon) optionally force iio channel type

[Liam Beguin]

On Sun May 16, 2021 at 12:26 PM EDT, Jonathan Cameron wrote:
> On Sun, 16 May 2021 08:54:06 -0700
> Guenter Roeck <linux@roeck-us.net> wrote:
>
> > On 5/16/21 8:02 AM, Liam Beguin wrote:
> > > Hi Jonathan,
> > > 
> > > On Sun May 16, 2021 at 5:06 AM EDT, Jonathan Cameron wrote:  
> > >> On Sun, 16 May 2021 00:43:13 -0400
> > >> Liam Beguin <liambeguin@gmail.com> wrote:
> > >>  
> > >>> Add a devicetree binding to optionally force a different IIO channel
> > >>> type.
> > >>>
> > >>> This is useful in cases where ADC channels are connected to a circuit
> > >>> that represent another unit such as a temperature or a current.
> > >>>
> > >>> `channel-types` was chosen instead of `io-channel-types` as this is not
> > >>> part of the iio consumer bindings.
> > >>>
> > >>> In the current form, this patch does what it's intended to do:
> > >>> change the unit displayed by `sensors`, but feels like the wrong way to
> > >>> address the problem.
> > >>>
> > >>> Would it be possible to force the type of different IIO channels for
> > >>> this kind of use case with a devicetree binding from the IIO subsystem?
> > >>>
> > >>> It would be convenient to do it within the IIO subsystem to have the
> > >>> right unit there too.
> > >>>
> > >>> Thanks for your time,
> > >>> Liam  
> > >>
> > >> Hi Liam,
> > >>
> > >> +CC Peter for AFE part.
> > >>
> > >> It's an interesting approach, but I would suggest we think about this
> > >> a different way.
> > >>
> > >> Whenever a channel is being used to measure something 'different' from
> > >> what it actually measures (e.g. a voltage ADC measuring a current) that
> > >> reflects their being some analog component involved.
> > >> If you look at drivers/iio/afe/iio-rescale.c you can see the approach
> > >> we currently use to handle this.  
> > > 
> > > Many thanks for pointing out the AFE code. That look like what I was
> > > hoping to accomplish, but in a much better way.
> > >   
> > >>
> > >> Effectively what you add to devicetree is a consumer of the ADC channel
> > >> which in turn provides services to other devices. For this current case
> > >> it would be either a current-sense-amplifier or a current-sense-shunt
> > >> depending on what the analog front end looks like. We have to describe
> > >> the characteristics of that front end which isn't something that can
> > >> be done via a simple channel type.
> > >>  
> > > 
> > > Understood. My original intention was to use sensors.conf to do the
> > > conversions and take into accounts those parameters.
> > >   
> > >> That afe consumer device can then provide services to another consumer
> > >> (e.g. iio-hwmon) which work for your usecase.
> > >>
> > >> The main limitation of this approach currently is you end up with
> > >> one device per channel. That could be improved upon if you have a
> > >> usecase
> > >> where it matters.
> > >>
> > >> I don't think we currently have an equivalent for temperature sensing
> > >> but it would be easy enough to do something similar.  
> > > 
> > > Wonderful, thanks again for pointing out the AFE!
> > >   
> > 
> > Please don't reinvent the ntc_thermistor driver.

> Agreed, I'd forgotten it existed :( Had a feeling we'd solved that
> problem before
> but couldn't remember the name of the driver.
>
> The afe driver already deals with current / voltage scaling and
> conversion
> for common analog circuits. Potential dividers, current shunts etc, but
> they
> are all the linear cases IIRC.
>
> ntc_thermistor deals with the much more complex job of dealing with a
> thermistor.

I agree, no need to reinvent this.

Like Jonathan said, the ntc_thermistor driver seems to handle much more
complex cases. Where would be the best place to add support for PT100
and PT1000? iio-rescale?

Thanks,
Liam

>
> Thanks,
>
> Jonathan
>
> > 
> > Thanks,
> > Guenter
> > 
> > > Liam
> > >   
> > >>
> > >> Jonathan
> > >>
> > >>  
> > >>>
> > >>> Liam Beguin (2):
> > >>>    hwmon: (iio_hwmon) optionally force iio channel type
> > >>>    dt-bindings: hwmon: add iio-hwmon bindings
> > >>>
> > >>>   .../devicetree/bindings/hwmon/iio-hwmon.yaml  | 41 +++++++++++++++++++
> > >>>   drivers/hwmon/iio_hwmon.c                     |  2 +
> > >>>   2 files changed, 43 insertions(+)
> > >>>   create mode 100644 Documentation/devicetree/bindings/hwmon/iio-hwmon.yaml
> > >>>
> > >>>
> > >>> base-commit: 9f4ad9e425a1d3b6a34617b8ea226d56a119a717  
> > >   
> > 

Re: [RFC PATCH v1 0/2] hwmon: (iio_hwmon) optionally force iio channel type

[Guenter Roeck]

On 5/16/21 11:14 AM, Liam Beguin wrote:
> On Sun May 16, 2021 at 12:26 PM EDT, Jonathan Cameron wrote:
>> On Sun, 16 May 2021 08:54:06 -0700
>> Guenter Roeck <linux@roeck-us.net> wrote:
>>
>>> On 5/16/21 8:02 AM, Liam Beguin wrote:
>>>> Hi Jonathan,
>>>>
>>>> On Sun May 16, 2021 at 5:06 AM EDT, Jonathan Cameron wrote:
>>>>> On Sun, 16 May 2021 00:43:13 -0400
>>>>> Liam Beguin <liambeguin@gmail.com> wrote:
>>>>>   
>>>>>> Add a devicetree binding to optionally force a different IIO channel
>>>>>> type.
>>>>>>
>>>>>> This is useful in cases where ADC channels are connected to a circuit
>>>>>> that represent another unit such as a temperature or a current.
>>>>>>
>>>>>> `channel-types` was chosen instead of `io-channel-types` as this is not
>>>>>> part of the iio consumer bindings.
>>>>>>
>>>>>> In the current form, this patch does what it's intended to do:
>>>>>> change the unit displayed by `sensors`, but feels like the wrong way to
>>>>>> address the problem.
>>>>>>
>>>>>> Would it be possible to force the type of different IIO channels for
>>>>>> this kind of use case with a devicetree binding from the IIO subsystem?
>>>>>>
>>>>>> It would be convenient to do it within the IIO subsystem to have the
>>>>>> right unit there too.
>>>>>>
>>>>>> Thanks for your time,
>>>>>> Liam
>>>>>
>>>>> Hi Liam,
>>>>>
>>>>> +CC Peter for AFE part.
>>>>>
>>>>> It's an interesting approach, but I would suggest we think about this
>>>>> a different way.
>>>>>
>>>>> Whenever a channel is being used to measure something 'different' from
>>>>> what it actually measures (e.g. a voltage ADC measuring a current) that
>>>>> reflects their being some analog component involved.
>>>>> If you look at drivers/iio/afe/iio-rescale.c you can see the approach
>>>>> we currently use to handle this.
>>>>
>>>> Many thanks for pointing out the AFE code. That look like what I was
>>>> hoping to accomplish, but in a much better way.
>>>>    
>>>>>
>>>>> Effectively what you add to devicetree is a consumer of the ADC channel
>>>>> which in turn provides services to other devices. For this current case
>>>>> it would be either a current-sense-amplifier or a current-sense-shunt
>>>>> depending on what the analog front end looks like. We have to describe
>>>>> the characteristics of that front end which isn't something that can
>>>>> be done via a simple channel type.
>>>>>   
>>>>
>>>> Understood. My original intention was to use sensors.conf to do the
>>>> conversions and take into accounts those parameters.
>>>>    
>>>>> That afe consumer device can then provide services to another consumer
>>>>> (e.g. iio-hwmon) which work for your usecase.
>>>>>
>>>>> The main limitation of this approach currently is you end up with
>>>>> one device per channel. That could be improved upon if you have a
>>>>> usecase
>>>>> where it matters.
>>>>>
>>>>> I don't think we currently have an equivalent for temperature sensing
>>>>> but it would be easy enough to do something similar.
>>>>
>>>> Wonderful, thanks again for pointing out the AFE!
>>>>    
>>>
>>> Please don't reinvent the ntc_thermistor driver.
> 
>> Agreed, I'd forgotten it existed :( Had a feeling we'd solved that
>> problem before
>> but couldn't remember the name of the driver.
>>
>> The afe driver already deals with current / voltage scaling and
>> conversion
>> for common analog circuits. Potential dividers, current shunts etc, but
>> they
>> are all the linear cases IIRC.
>>
>> ntc_thermistor deals with the much more complex job of dealing with a
>> thermistor.
> 
> I agree, no need to reinvent this.
> 
> Like Jonathan said, the ntc_thermistor driver seems to handle much more
> complex cases. Where would be the best place to add support for PT100
> and PT1000? iio-rescale?
> 

Those sensors don't seem to be even useful for hardware monitoring, so
if they are linear (and it looks like that that the case) iio would be
a better place.

Guenter