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

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

[Naresh Solanki]

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 <naresh.solanki@9elements.com>
---
 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

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1475 bytes --]

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 <naresh.solanki@9elements.com>
> ---
>  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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Naresh Solanki]

Hi,


On Fri, 26 Jan 2024 at 21:47, Conor Dooley <conor@kernel.org> 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 <naresh.solanki@9elements.com>
> > ---
> >  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
> >

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2078 bytes --]

On Fri, Jan 26, 2024 at 09:55:20PM +0530, Naresh Solanki wrote:
> On Fri, 26 Jan 2024 at 21:47, Conor Dooley <conor@kernel.org> wrote:
> > On Fri, Jan 26, 2024 at 05:25:08PM +0530, Naresh Solanki wrote:
> > > Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com>
> > > ---
> > >  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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Naresh Solanki]

Hi Conor,


On Fri, 26 Jan 2024 at 22:22, Conor Dooley <conor@kernel.org> wrote:
>
> On Fri, Jan 26, 2024 at 09:55:20PM +0530, Naresh Solanki wrote:
> > On Fri, 26 Jan 2024 at 21:47, Conor Dooley <conor@kernel.org> wrote:
> > > On Fri, Jan 26, 2024 at 05:25:08PM +0530, Naresh Solanki wrote:
> > > > Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com>
> > > > ---
> > > >  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.

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3334 bytes --]

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 <conor@kernel.org> wrote:
> >
> > On Fri, Jan 26, 2024 at 09:55:20PM +0530, Naresh Solanki wrote:
> > > On Fri, 26 Jan 2024 at 21:47, Conor Dooley <conor@kernel.org> wrote:
> > > > On Fri, Jan 26, 2024 at 05:25:08PM +0530, Naresh Solanki wrote:
> > > > > Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com>
> > > > > ---
> > > > >  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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Peter Rosin]

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 <naresh.solanki@9elements.com>
>> ---
>>  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

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

[Peter Rosin]

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

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2535 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Jonathan Cameron]

On Sat, 27 Jan 2024 11:03:14 +0000
Conor Dooley <conor@kernel.org> 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.

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 691 bytes --]

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?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Jonathan Cameron]

On Sat, 27 Jan 2024 16:48:04 +0000
Conor Dooley <conor@kernel.org> 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.