2024-01-26 11:55:42

by Naresh Solanki

[permalink] [raw]
Subject: [PATCH] dt-bindings: iio: afe: voltage-divider: Add io-channel-cells

Add #io-channel-cells expected by driver. i.e., below is the message
seen in kernel log:
OF: /iio-hwmon: could not get #io-channel-cells for /voltage_divider1

TEST=Run below command & make sure there is no error:
make DT_CHECKER_FLAGS=-m dt_binding_check -j1

Signed-off-by: Naresh Solanki <[email protected]>
---
Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml | 3 +++
1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml b/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
index dddf97b50549..b4b5489ad98e 100644
--- a/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
+++ b/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
@@ -39,6 +39,9 @@ properties:
description: |
Channel node of a voltage io-channel.

+ '#io-channel-cells':
+ const: 1
+
output-ohms:
description:
Resistance Rout over which the output voltage is measured. See full-ohms.

base-commit: ecb1b8288dc7ccbdcb3b9df005fa1c0e0c0388a7
--
2.42.0



2024-01-26 16:17:05

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: iio: afe: voltage-divider: Add io-channel-cells

Hey,

On Fri, Jan 26, 2024 at 05:25:08PM +0530, Naresh Solanki wrote:
> Add #io-channel-cells expected by driver. i.e., below is the message
> seen in kernel log:
> OF: /iio-hwmon: could not get #io-channel-cells for /voltage_divider1
>

> TEST=Run below command & make sure there is no error:
> make DT_CHECKER_FLAGS=-m dt_binding_check -j1

This shouldn't be in the commit message.

>
> Signed-off-by: Naresh Solanki <[email protected]>
> ---
> Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml b/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
> index dddf97b50549..b4b5489ad98e 100644
> --- a/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
> +++ b/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
> @@ -39,6 +39,9 @@ properties:
> description: |
> Channel node of a voltage io-channel.
>
> + '#io-channel-cells':
> + const: 1

The example in this binding looks like the voltage-divider is intended
to be an "IIO consumer" but "#io-channels-cells" is an "IIO provider"
property.

Are you sure this is correct?

> +
> output-ohms:
> description:
> Resistance Rout over which the output voltage is measured. See full-ohms.
>
> base-commit: ecb1b8288dc7ccbdcb3b9df005fa1c0e0c0388a7
> --
> 2.42.0
>


Attachments:
(No filename) (1.44 kB)
signature.asc (235.00 B)
Download all attachments

2024-01-26 16:28:00

by Naresh Solanki

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: iio: afe: voltage-divider: Add io-channel-cells

Hi,


On Fri, 26 Jan 2024 at 21:47, Conor Dooley <[email protected]> wrote:
>
> Hey,
>
> On Fri, Jan 26, 2024 at 05:25:08PM +0530, Naresh Solanki wrote:
> > Add #io-channel-cells expected by driver. i.e., below is the message
> > seen in kernel log:
> > OF: /iio-hwmon: could not get #io-channel-cells for /voltage_divider1
> >
>
> > TEST=Run below command & make sure there is no error:
> > make DT_CHECKER_FLAGS=-m dt_binding_check -j1
>
> This shouldn't be in the commit message.
ok Will remove.
>
> >
> > Signed-off-by: Naresh Solanki <[email protected]>
> > ---
> > Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml b/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
> > index dddf97b50549..b4b5489ad98e 100644
> > --- a/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
> > +++ b/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
> > @@ -39,6 +39,9 @@ properties:
> > description: |
> > Channel node of a voltage io-channel.
> >
> > + '#io-channel-cells':
> > + const: 1
>
> The example in this binding looks like the voltage-divider is intended
> to be an "IIO consumer" but "#io-channels-cells" is an "IIO provider"
> property.
>
> Are you sure this is correct?
I'm not aware that #io-channels-cells is only for IIO provider.
But I do get some kernel message as mention in commit messages
if this is specified in DT.

Regards,
Naresh

>
> > +
> > output-ohms:
> > description:
> > Resistance Rout over which the output voltage is measured. See full-ohms.
> >
> > base-commit: ecb1b8288dc7ccbdcb3b9df005fa1c0e0c0388a7
> > --
> > 2.42.0
> >

2024-01-26 16:57:12

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: iio: afe: voltage-divider: Add io-channel-cells

On Fri, Jan 26, 2024 at 09:55:20PM +0530, Naresh Solanki wrote:
> On Fri, 26 Jan 2024 at 21:47, Conor Dooley <[email protected]> wrote:
> > On Fri, Jan 26, 2024 at 05:25:08PM +0530, Naresh Solanki wrote:
> > > Signed-off-by: Naresh Solanki <[email protected]>
> > > ---
> > > Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml b/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
> > > index dddf97b50549..b4b5489ad98e 100644
> > > --- a/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
> > > +++ b/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
> > > @@ -39,6 +39,9 @@ properties:
> > > description: |
> > > Channel node of a voltage io-channel.
> > >
> > > + '#io-channel-cells':
> > > + const: 1
> >
> > The example in this binding looks like the voltage-divider is intended
> > to be an "IIO consumer" but "#io-channels-cells" is an "IIO provider"
> > property.
> >
> > Are you sure this is correct?
> I'm not aware that #io-channels-cells is only for IIO provider.

#foo-cells properties are always for resource providers

> But I do get some kernel message as mention in commit messages
> if this is specified in DT.

Can you please share the DT in question? Or at least, the section that
describes the IIO provider and consumer?

It should look like the example:

spi {
#address-cells = <1>;
#size-cells = <0>;
adc@0 {
compatible = "maxim,max1027";
reg = <0>;
#io-channel-cells = <1>;
interrupt-parent = <&gpio5>;
interrupts = <15 IRQ_TYPE_EDGE_RISING>;
spi-max-frequency = <1000000>;
};
};

sysv {
compatible = "voltage-divider";
io-channels = <&maxadc 1>;

/* Scale the system voltage by 22/222 to fit the ADC range. */
output-ohms = <22>;
full-ohms = <222>; /* 200 + 22 */
};

Thanks,
Conor.


Attachments:
(No filename) (2.09 kB)
signature.asc (235.00 B)
Download all attachments

2024-01-26 17:42:08

by Naresh Solanki

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: iio: afe: voltage-divider: Add io-channel-cells

Hi Conor,


On Fri, 26 Jan 2024 at 22:22, Conor Dooley <[email protected]> wrote:
>
> On Fri, Jan 26, 2024 at 09:55:20PM +0530, Naresh Solanki wrote:
> > On Fri, 26 Jan 2024 at 21:47, Conor Dooley <[email protected]> wrote:
> > > On Fri, Jan 26, 2024 at 05:25:08PM +0530, Naresh Solanki wrote:
> > > > Signed-off-by: Naresh Solanki <[email protected]>
> > > > ---
> > > > Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml b/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
> > > > index dddf97b50549..b4b5489ad98e 100644
> > > > --- a/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
> > > > +++ b/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
> > > > @@ -39,6 +39,9 @@ properties:
> > > > description: |
> > > > Channel node of a voltage io-channel.
> > > >
> > > > + '#io-channel-cells':
> > > > + const: 1
> > >
> > > The example in this binding looks like the voltage-divider is intended
> > > to be an "IIO consumer" but "#io-channels-cells" is an "IIO provider"
> > > property.
> > >
> > > Are you sure this is correct?
> > I'm not aware that #io-channels-cells is only for IIO provider.
>
> #foo-cells properties are always for resource providers
>
> > But I do get some kernel message as mention in commit messages
> > if this is specified in DT.
>
> Can you please share the DT in question? Or at least, the section that
> describes the IIO provider and consumer?
Below is link to complete DT:
https://github.com/torvalds/linux/commit/522bf7f2d6b085f69d4538535bfc1eb965632f54
>
> It should look like the example:
I reference the below example previously but didn't help.
If io-channel-cell isn't provided then there is print in kernel dmesg as:
OF: /iio-hwmon: could not get #io-channel-cells for /voltage_divider1

Thanks,
Naresh
>
> spi {
> #address-cells = <1>;
> #size-cells = <0>;
> adc@0 {
> compatible = "maxim,max1027";
> reg = <0>;
> #io-channel-cells = <1>;
> interrupt-parent = <&gpio5>;
> interrupts = <15 IRQ_TYPE_EDGE_RISING>;
> spi-max-frequency = <1000000>;
> };
> };
>
> sysv {
> compatible = "voltage-divider";
> io-channels = <&maxadc 1>;
>
> /* Scale the system voltage by 22/222 to fit the ADC range. */
> output-ohms = <22>;
> full-ohms = <222>; /* 200 + 22 */
> };
>
> Thanks,
> Conor.

2024-01-26 22:15:02

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: iio: afe: voltage-divider: Add io-channel-cells

On Fri, Jan 26, 2024 at 11:10:36PM +0530, Naresh Solanki wrote:
> Hi Conor,
>
>
> On Fri, 26 Jan 2024 at 22:22, Conor Dooley <[email protected]> wrote:
> >
> > On Fri, Jan 26, 2024 at 09:55:20PM +0530, Naresh Solanki wrote:
> > > On Fri, 26 Jan 2024 at 21:47, Conor Dooley <[email protected]> wrote:
> > > > On Fri, Jan 26, 2024 at 05:25:08PM +0530, Naresh Solanki wrote:
> > > > > Signed-off-by: Naresh Solanki <[email protected]>
> > > > > ---
> > > > > Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml | 3 +++
> > > > > 1 file changed, 3 insertions(+)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml b/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
> > > > > index dddf97b50549..b4b5489ad98e 100644
> > > > > --- a/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
> > > > > +++ b/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
> > > > > @@ -39,6 +39,9 @@ properties:
> > > > > description: |
> > > > > Channel node of a voltage io-channel.
> > > > >
> > > > > + '#io-channel-cells':
> > > > > + const: 1
> > > >
> > > > The example in this binding looks like the voltage-divider is intended
> > > > to be an "IIO consumer" but "#io-channels-cells" is an "IIO provider"
> > > > property.
> > > >
> > > > Are you sure this is correct?
> > > I'm not aware that #io-channels-cells is only for IIO provider.
> >
> > #foo-cells properties are always for resource providers
> >
> > > But I do get some kernel message as mention in commit messages
> > > if this is specified in DT.
> >
> > Can you please share the DT in question? Or at least, the section that
> > describes the IIO provider and consumer?
> Below is link to complete DT:
> https://github.com/torvalds/linux/commit/522bf7f2d6b085f69d4538535bfc1eb965632f54

If you're gonna link something that is in a vendor tree, you should link
the actual vendor tree and not something that "does not belong to any
branch on this repository, and may belong to a fork outside of the
repository"!

I did look at what you have there and I think your dts is wrong.

The iio-hwmon binding says:
| description: >
| Bindings for hardware monitoring devices connected to ADC controllers
| supporting the Industrial I/O bindings.
|
| io-channels:
| minItems: 1
| maxItems: 51 # Should be enough
| description: >
| List of phandles to ADC channels to read the monitoring values

And then you have:
| iio-hwmon {
| compatible = "iio-hwmon";
| // Voltage sensors top to down
| io-channels = <&p12v_vd 0>, <&p5v_aux_vd 0>, <&p5v_bmc_aux_vd 0>, <&p3v3_aux_vd 0>,
| <&p3v3_bmc_aux_vd 0>, <&p1v8_bmc_aux_vd 0>, <&adc1 4>, <&adc0 2>, <&adc1 0>,
| <&p2V5_aux_vd 0>, <&p3v3_rtc_vd 0>;
| };
|
| p12v_vd: voltage_divider1 {
| compatible = "voltage-divider";
| io-channels = <&adc1 3>;
| #io-channel-cells = <1>;
|
| /* Scale the system voltage by 1127/127 to fit the ADC range.
| * Use small nominator to prevent integer overflow.
| */
| output-ohms = <15>;
| full-ohms = <133>;
| };

A voltage divider is _not_ an ADC channel, so I don't know why you are
treating it as one in the iio-hwmon entry. Can you explain this please?

Thanks,
Conor.


Attachments:
(No filename) (3.26 kB)
signature.asc (235.00 B)
Download all attachments

2024-01-26 22:18:08

by Peter Rosin

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: iio: afe: voltage-divider: Add io-channel-cells

Hi!

2024-01-26 at 17:16, Conor Dooley wrote:
> Hey,
>
> On Fri, Jan 26, 2024 at 05:25:08PM +0530, Naresh Solanki wrote:
>> Add #io-channel-cells expected by driver. i.e., below is the message
>> seen in kernel log:
>> OF: /iio-hwmon: could not get #io-channel-cells for /voltage_divider1
>>
>
>> TEST=Run below command & make sure there is no error:
>> make DT_CHECKER_FLAGS=-m dt_binding_check -j1
>
> This shouldn't be in the commit message.
>
>>
>> Signed-off-by: Naresh Solanki <[email protected]>
>> ---
>> Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml b/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
>> index dddf97b50549..b4b5489ad98e 100644
>> --- a/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
>> +++ b/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
>> @@ -39,6 +39,9 @@ properties:
>> description: |
>> Channel node of a voltage io-channel.
>>
>> + '#io-channel-cells':
>> + const: 1
>
> The example in this binding looks like the voltage-divider is intended
> to be an "IIO consumer" but "#io-channels-cells" is an "IIO provider"
> property.
>
> Are you sure this is correct?

A voltage-divider is always an iio consumer. And like all iio things,
you may access its output from user space (typically via libiio). At
the same time a voltage-divider is optionally an iio provider for other
in-kernel thingies, in which case you need to specify
#io-channel-cells.

BTW, this is the case for for all bindings handled by the iio-rescale
driver.

Cheers,
Peter

2024-01-27 09:41:15

by Peter Rosin

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: iio: afe: voltage-divider: Add io-channel-cells



2024-01-26 at 23:14, Conor Dooley wrote:
> On Fri, Jan 26, 2024 at 11:10:36PM +0530, Naresh Solanki wrote:

> I did look at what you have there and I think your dts is wrong.
>
> The iio-hwmon binding says:
> | description: >
> | Bindings for hardware monitoring devices connected to ADC controllers
> | supporting the Industrial I/O bindings.
> |
> | io-channels:
> | minItems: 1
> | maxItems: 51 # Should be enough
> | description: >
> | List of phandles to ADC channels to read the monitoring values
>
> And then you have:
> | iio-hwmon {
> | compatible = "iio-hwmon";
> | // Voltage sensors top to down
> | io-channels = <&p12v_vd 0>, <&p5v_aux_vd 0>, <&p5v_bmc_aux_vd 0>, <&p3v3_aux_vd 0>,
> | <&p3v3_bmc_aux_vd 0>, <&p1v8_bmc_aux_vd 0>, <&adc1 4>, <&adc0 2>, <&adc1 0>,
> | <&p2V5_aux_vd 0>, <&p3v3_rtc_vd 0>;
> | };
> |
> | p12v_vd: voltage_divider1 {
> | compatible = "voltage-divider";
> | io-channels = <&adc1 3>;
> | #io-channel-cells = <1>;
> |
> | /* Scale the system voltage by 1127/127 to fit the ADC range.
> | * Use small nominator to prevent integer overflow.
> | */
> | output-ohms = <15>;
> | full-ohms = <133>;
> | };
>
> A voltage divider is _not_ an ADC channel, so I don't know why you are
> treating it as one in the iio-hwmon entry. Can you explain this please?

This is the exact intent of the voltage divider (and the other bindings
handled by the iio-rescaler). The raw ADC reports the voltage at its input,
which is fine, but if there is an analog frontend in front of the ADC
such as a voltage divider the voltage at the ADC is not the interesting
property. You are likely to want the "real" voltage before the voltage
divider to better understand the value.

In this case it's much more interesting to see values such as 12.050V
which is presumably close to the nominal voltage (12V? guessing from
the node name) rather than some unscaled raw ADC voltage (in this
example it would be ~1.359V, which tells you rather little w/o rescaling
it first).

It's all in the description of the binding...

Cheers,
Peter

2024-01-27 11:03:51

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: iio: afe: voltage-divider: Add io-channel-cells

On Sat, Jan 27, 2024 at 10:40:31AM +0100, Peter Rosin wrote:
>
>
> 2024-01-26 at 23:14, Conor Dooley wrote:
> > On Fri, Jan 26, 2024 at 11:10:36PM +0530, Naresh Solanki wrote:
>
> > I did look at what you have there and I think your dts is wrong.
> >
> > The iio-hwmon binding says:
> > | description: >
> > | Bindings for hardware monitoring devices connected to ADC controllers
> > | supporting the Industrial I/O bindings.
> > |
> > | io-channels:
> > | minItems: 1
> > | maxItems: 51 # Should be enough
> > | description: >
> > | List of phandles to ADC channels to read the monitoring values
> >
> > And then you have:
> > | iio-hwmon {
> > | compatible = "iio-hwmon";
> > | // Voltage sensors top to down
> > | io-channels = <&p12v_vd 0>, <&p5v_aux_vd 0>, <&p5v_bmc_aux_vd 0>, <&p3v3_aux_vd 0>,
> > | <&p3v3_bmc_aux_vd 0>, <&p1v8_bmc_aux_vd 0>, <&adc1 4>, <&adc0 2>, <&adc1 0>,
> > | <&p2V5_aux_vd 0>, <&p3v3_rtc_vd 0>;
> > | };
> > |
> > | p12v_vd: voltage_divider1 {
> > | compatible = "voltage-divider";
> > | io-channels = <&adc1 3>;
> > | #io-channel-cells = <1>;
> > |
> > | /* Scale the system voltage by 1127/127 to fit the ADC range.
> > | * Use small nominator to prevent integer overflow.
> > | */
> > | output-ohms = <15>;
> > | full-ohms = <133>;
> > | };
> >
> > A voltage divider is _not_ an ADC channel, so I don't know why you are
> > treating it as one in the iio-hwmon entry. Can you explain this please?
>
> This is the exact intent of the voltage divider (and the other bindings
> handled by the iio-rescaler). The raw ADC reports the voltage at its input,
> which is fine, but if there is an analog frontend in front of the ADC
> such as a voltage divider the voltage at the ADC is not the interesting
> property. You are likely to want the "real" voltage before the voltage
> divider to better understand the value.
>
> In this case it's much more interesting to see values such as 12.050V
> which is presumably close to the nominal voltage (12V? guessing from
> the node name) rather than some unscaled raw ADC voltage (in this
> example it would be ~1.359V, which tells you rather little w/o rescaling
> it first).

Thanks for explaining it. Naresh, can you respin please with an
explanation of why the property belongs in the binding please?

> It's all in the description of the binding...

Obviously it was not sufficiently clear, it's not as if I didn't look at
it...

Cheers,
Conor.


Attachments:
(No filename) (2.48 kB)
signature.asc (235.00 B)
Download all attachments

2024-01-27 14:49:44

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: iio: afe: voltage-divider: Add io-channel-cells

On Sat, 27 Jan 2024 11:03:14 +0000
Conor Dooley <[email protected]> wrote:

> On Sat, Jan 27, 2024 at 10:40:31AM +0100, Peter Rosin wrote:
> >
> >
> > 2024-01-26 at 23:14, Conor Dooley wrote:
> > > On Fri, Jan 26, 2024 at 11:10:36PM +0530, Naresh Solanki wrote:
> >
> > > I did look at what you have there and I think your dts is wrong.
> > >
> > > The iio-hwmon binding says:
> > > | description: >
> > > | Bindings for hardware monitoring devices connected to ADC controllers
> > > | supporting the Industrial I/O bindings.
> > > |
> > > | io-channels:
> > > | minItems: 1
> > > | maxItems: 51 # Should be enough
> > > | description: >
> > > | List of phandles to ADC channels to read the monitoring values
> > >
> > > And then you have:
> > > | iio-hwmon {
> > > | compatible = "iio-hwmon";
> > > | // Voltage sensors top to down
> > > | io-channels = <&p12v_vd 0>, <&p5v_aux_vd 0>, <&p5v_bmc_aux_vd 0>, <&p3v3_aux_vd 0>,
> > > | <&p3v3_bmc_aux_vd 0>, <&p1v8_bmc_aux_vd 0>, <&adc1 4>, <&adc0 2>, <&adc1 0>,
> > > | <&p2V5_aux_vd 0>, <&p3v3_rtc_vd 0>;
> > > | };
> > > |
> > > | p12v_vd: voltage_divider1 {
> > > | compatible = "voltage-divider";
> > > | io-channels = <&adc1 3>;
> > > | #io-channel-cells = <1>;
> > > |
> > > | /* Scale the system voltage by 1127/127 to fit the ADC range.
> > > | * Use small nominator to prevent integer overflow.
> > > | */
> > > | output-ohms = <15>;
> > > | full-ohms = <133>;
> > > | };
> > >
> > > A voltage divider is _not_ an ADC channel, so I don't know why you are
> > > treating it as one in the iio-hwmon entry. Can you explain this please?
> >
> > This is the exact intent of the voltage divider (and the other bindings
> > handled by the iio-rescaler). The raw ADC reports the voltage at its input,
> > which is fine, but if there is an analog frontend in front of the ADC
> > such as a voltage divider the voltage at the ADC is not the interesting
> > property. You are likely to want the "real" voltage before the voltage
> > divider to better understand the value.
> >
> > In this case it's much more interesting to see values such as 12.050V
> > which is presumably close to the nominal voltage (12V? guessing from
> > the node name) rather than some unscaled raw ADC voltage (in this
> > example it would be ~1.359V, which tells you rather little w/o rescaling
> > it first).
>
> Thanks for explaining it. Naresh, can you respin please with an
> explanation of why the property belongs in the binding please?

Agreed - the explanation of 'why' is needed.
As discussed, in this case the end consumer is iio-hwmon
(reflecting that the thing we are ultimately 'measuring' as a voltage
of a wire that needs monitoring for hwmon usecases) and
that is measured via a voltage divider (hence that's providing
the measurement service) which in turn needs to consume the reading
from and ADC and hence the middle device is here acting as a
provider to iio-hwmon and a consumer of the ADC channel.

p.s. No one really likes the iio-hwmon binding because it is reflecting
a usecase rather than hardware, but meh, it's been around a long time
and we never figured out a better option.

Things are much cleaner for circuits like the voltage divider.

>
> > It's all in the description of the binding...
>
> Obviously it was not sufficiently clear, it's not as if I didn't look at
> it...

Given this device fits in both categories, perhaps a tiny bit of
additional documentation would help?

'#io-channels-cells':
description:
In addition to consuming the measurement services of an ADC,
the voltage divider can act as an provider of measurement
services to other devices.
const: 1

>
> Cheers,
> Conor.


2024-01-27 16:48:27

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: iio: afe: voltage-divider: Add io-channel-cells

On Sat, Jan 27, 2024 at 02:49:20PM +0000, Jonathan Cameron wrote:

> > > It's all in the description of the binding...
> >
> > Obviously it was not sufficiently clear, it's not as if I didn't look at
> > it...
>
> Given this device fits in both categories, perhaps a tiny bit of
> additional documentation would help?

That would be nice.

> '#io-channels-cells':
> description:
> In addition to consuming the measurement services of an ADC,
> the voltage divider can act as an provider of measurement
> services to other devices.
> const: 1

But I am not sure that that covers things. I think an example, like
Peter gave, would be good?


Attachments:
(No filename) (691.00 B)
signature.asc (235.00 B)
Download all attachments

2024-01-27 16:56:33

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: iio: afe: voltage-divider: Add io-channel-cells

On Sat, 27 Jan 2024 16:48:04 +0000
Conor Dooley <[email protected]> wrote:

> On Sat, Jan 27, 2024 at 02:49:20PM +0000, Jonathan Cameron wrote:
>
> > > > It's all in the description of the binding...
> > >
> > > Obviously it was not sufficiently clear, it's not as if I didn't look at
> > > it...
> >
> > Given this device fits in both categories, perhaps a tiny bit of
> > additional documentation would help?
>
> That would be nice.
>
> > '#io-channels-cells':
> > description:
> > In addition to consuming the measurement services of an ADC,
> > the voltage divider can act as an provider of measurement
> > services to other devices.
> > const: 1
>
> But I am not sure that that covers things. I think an example, like
> Peter gave, would be good?

Ok. An example is fine.