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

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

[Naresh Solanki]

voltage-divider is always an iio consumer at the same time it is
optionally an iio provider.
Hence add #io-channel-cells
Also update example.

Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com>
---
 .../bindings/iio/afe/voltage-divider.yaml          | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml b/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
index dddf97b50549..98fec8548cc3 100644
--- a/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
+++ b/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
@@ -39,6 +39,13 @@ properties:
     description: |
       Channel node of a voltage io-channel.
 
+  '#io-channel-cells':
+    description:
+      In addition to consuming the measurement services of a voltage output
+      channel the voltage divider can act as a provider of measurement
+      services to other devices.
+    const: 1
+
   output-ohms:
     description:
       Resistance Rout over which the output voltage is measured. See full-ohms.
@@ -75,12 +82,17 @@ examples:
             spi-max-frequency = <1000000>;
         };
     };
-    sysv {
+    p12v_vd: sysv {
         compatible = "voltage-divider";
         io-channels = <&maxadc 1>;
+        #io-channel-cells = <1>;
 
         /* Scale the system voltage by 22/222 to fit the ADC range. */
         output-ohms = <22>;
         full-ohms = <222>; /* 200 + 22 */
     };
+    iio-hwmon {
+        compatible = "iio-hwmon";
+        io-channels = <&p12v_vd 0>;
+    };
 ...

base-commit: 861c0981648f5b64c86fd028ee622096eb7af05a
-- 
2.42.0

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

[Conor Dooley]

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

Hey,

On Tue, Jan 30, 2024 at 05:26:50PM +0530, Naresh Solanki wrote:
> voltage-divider is always an iio consumer at the same time it is
> optionally an iio provider.
> Hence add #io-channel-cells
> Also update example.
> 
> Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com>
> ---
>  .../bindings/iio/afe/voltage-divider.yaml          | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml b/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
> index dddf97b50549..98fec8548cc3 100644
> --- a/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
> +++ b/Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml
> @@ -39,6 +39,13 @@ properties:
>      description: |
>        Channel node of a voltage io-channel.
>  
> +  '#io-channel-cells':
> +    description:
> +      In addition to consuming the measurement services of a voltage output
> +      channel the voltage divider can act as a provider of measurement
> +      services to other devices.

Did you miss:
https://lore.kernel.org/all/20240127165542.6eeb23e9@jic23-huawei/
Where I said that I would like to have an example of where this would be
used in the description? Peter gave a good example that you can draw on.

> +    const: 1
> +
>    output-ohms:
>      description:
>        Resistance Rout over which the output voltage is measured. See full-ohms.
> @@ -75,12 +82,17 @@ examples:
>              spi-max-frequency = <1000000>;
>          };
>      };
> -    sysv {
> +    p12v_vd: sysv {
>          compatible = "voltage-divider";
>          io-channels = <&maxadc 1>;
> +        #io-channel-cells = <1>;
>  
>          /* Scale the system voltage by 22/222 to fit the ADC range. */
>          output-ohms = <22>;
>          full-ohms = <222>; /* 200 + 22 */
>      };

Blank line here please.

Thanks,
Conor.

> +    iio-hwmon {
> +        compatible = "iio-hwmon";
> +        io-channels = <&p12v_vd 0>;
> +    };
>  ...
> 
> base-commit: 861c0981648f5b64c86fd028ee622096eb7af05a
> -- 
> 2.42.0
> 

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

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

[Krzysztof Kozlowski]

On 30/01/2024 12:56, Naresh Solanki wrote:
> voltage-divider is always an iio consumer at the same time it is
> optionally an iio provider.
> Hence add #io-channel-cells
> Also update example.
> 

Fix
wrapping
and
proper
sentences. Each sentence finishes with full stop.

...
>    output-ohms:
>      description:
>        Resistance Rout over which the output voltage is measured. See full-ohms.
> @@ -75,12 +82,17 @@ examples:
>              spi-max-frequency = <1000000>;
>          };
>      };
> -    sysv {
> +    p12v_vd: sysv {

No, drop label.

>          compatible = "voltage-divider";
>          io-channels = <&maxadc 1>;
> +        #io-channel-cells = <1>;
>  
>          /* Scale the system voltage by 22/222 to fit the ADC range. */
>          output-ohms = <22>;
>          full-ohms = <222>; /* 200 + 22 */
>      };
> +    iio-hwmon {
> +        compatible = "iio-hwmon";
> +        io-channels = <&p12v_vd 0>;

The same question as for v2. Drop unrelated example.

Also, remember about rule of posting only one version per day, so people
actually have some chance to look at your patch.

Best regards,
Krzysztof

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

[Jonathan Cameron]

On Wed, 31 Jan 2024 09:29:59 +0100
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 30/01/2024 12:56, Naresh Solanki wrote:
> > voltage-divider is always an iio consumer at the same time it is
> > optionally an iio provider.
> > Hence add #io-channel-cells
> > Also update example.
> >   
> 
> Fix
> wrapping
> and
> proper
> sentences. Each sentence finishes with full stop.
> 
> ...
> >    output-ohms:
> >      description:
> >        Resistance Rout over which the output voltage is measured. See full-ohms.
> > @@ -75,12 +82,17 @@ examples:
> >              spi-max-frequency = <1000000>;
> >          };
> >      };
> > -    sysv {
> > +    p12v_vd: sysv {  
> 
> No, drop label.
> 
> >          compatible = "voltage-divider";
> >          io-channels = <&maxadc 1>;
> > +        #io-channel-cells = <1>;
> >  
> >          /* Scale the system voltage by 22/222 to fit the ADC range. */
> >          output-ohms = <22>;
> >          full-ohms = <222>; /* 200 + 22 */
> >      };
> > +    iio-hwmon {
> > +        compatible = "iio-hwmon";
> > +        io-channels = <&p12v_vd 0>;  
> 
> The same question as for v2. Drop unrelated example.

Conor requested an example of the device acting as a consumer and a provider.
Might have meant in the patch description?

Conor?
> 
> Also, remember about rule of posting only one version per day, so people
> actually have some chance to look at your patch.
> 
> Best regards,
> Krzysztof
> 
> 

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

[Conor Dooley]

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

On Wed, Jan 31, 2024 at 04:35:16PM +0000, Jonathan Cameron wrote:
> On Wed, 31 Jan 2024 09:29:59 +0100
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> 
> > On 30/01/2024 12:56, Naresh Solanki wrote:
> > > voltage-divider is always an iio consumer at the same time it is
> > > optionally an iio provider.
> > > Hence add #io-channel-cells
> > > Also update example.
> > >   
> > 
> > Fix
> > wrapping
> > and
> > proper
> > sentences. Each sentence finishes with full stop.
> > 
> > ...
> > >    output-ohms:
> > >      description:
> > >        Resistance Rout over which the output voltage is measured. See full-ohms.
> > > @@ -75,12 +82,17 @@ examples:
> > >              spi-max-frequency = <1000000>;
> > >          };
> > >      };
> > > -    sysv {
> > > +    p12v_vd: sysv {  
> > 
> > No, drop label.
> > 
> > >          compatible = "voltage-divider";
> > >          io-channels = <&maxadc 1>;
> > > +        #io-channel-cells = <1>;
> > >  
> > >          /* Scale the system voltage by 22/222 to fit the ADC range. */
> > >          output-ohms = <22>;
> > >          full-ohms = <222>; /* 200 + 22 */
> > >      };
> > > +    iio-hwmon {
> > > +        compatible = "iio-hwmon";
> > > +        io-channels = <&p12v_vd 0>;  
> > 
> > The same question as for v2. Drop unrelated example.
> 
> Conor requested an example of the device acting as a consumer and a provider.
> Might have meant in the patch description?
> 
> Conor?

I wanted it in the property description to help with understanding when
to use it. I don't think the extra example nodes actually help you
understand what it is doing, only how to write one yourself once you
know you need it.

Thanks,
Conor.

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

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

[Naresh Solanki]

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

Hi,


On Wed, 31 Jan 2024 at 22:24, Conor Dooley <conor@kernel.org> wrote:
>
> On Wed, Jan 31, 2024 at 04:35:16PM +0000, Jonathan Cameron wrote:
> > On Wed, 31 Jan 2024 09:29:59 +0100
> > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> >
> > > On 30/01/2024 12:56, Naresh Solanki wrote:
> > > > voltage-divider is always an iio consumer at the same time it is
> > > > optionally an iio provider.
> > > > Hence add #io-channel-cells
> > > > Also update example.
> > > >
> > >
> > > Fix
> > > wrapping
> > > and
> > > proper
> > > sentences. Each sentence finishes with full stop.
Sure
> > >
> > > ...
> > > >    output-ohms:
> > > >      description:
> > > >        Resistance Rout over which the output voltage is measured. See full-ohms.
> > > > @@ -75,12 +82,17 @@ examples:
> > > >              spi-max-frequency = <1000000>;
> > > >          };
> > > >      };
> > > > -    sysv {
> > > > +    p12v_vd: sysv {
> > >
> > > No, drop label.
Sure
> > >
> > > >          compatible = "voltage-divider";
> > > >          io-channels = <&maxadc 1>;
> > > > +        #io-channel-cells = <1>;
> > > >
> > > >          /* Scale the system voltage by 22/222 to fit the ADC range. */
> > > >          output-ohms = <22>;
> > > >          full-ohms = <222>; /* 200 + 22 */
> > > >      };
> > > > +    iio-hwmon {
> > > > +        compatible = "iio-hwmon";
> > > > +        io-channels = <&p12v_vd 0>;
> > >
> > > The same question as for v2. Drop unrelated example.
Sure
> >
> > Conor requested an example of the device acting as a consumer and a provider.
> > Might have meant in the patch description?
> >
> > Conor?
>
> I wanted it in the property description to help with understanding when
> to use it. I don't think the extra example nodes actually help you
> understand what it is doing, only how to write one yourself once you
> know you need it.
I'm not sure if I get it right but what I understood is that a
voltage-divider can
also be a provider to other devices & hence the property.
Also do you want me to put a complete example of it in description ?

Regards,
Naresh
>
> Thanks,
> Conor.

[-- Attachment #2: signature.asc --]
[-- Type: text/plain, Size: 235 bytes --]

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZbp7UQAKCRB4tDGHoIJi
0qUKAP0Ww48y8Vpl0AK0ojq0GTFz+pnr+vWFetK6V8fbSyQ0YwEAg+sb0e5EXttx
Yc5wqU6OG+ali+z2CNC/Op4kdAyPMAE=
=i3bX
-----END PGP SIGNATURE-----

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

[Peter Rosin]

Hi!

2024-02-02 at 11:43, Naresh Solanki wrote:
> Hi,
> 
> 
> On Wed, 31 Jan 2024 at 22:24, Conor Dooley <conor@kernel.org> wrote:
>>
>> On Wed, Jan 31, 2024 at 04:35:16PM +0000, Jonathan Cameron wrote:
>>> On Wed, 31 Jan 2024 09:29:59 +0100
>>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>>>
>>>> On 30/01/2024 12:56, Naresh Solanki wrote:
>>> Conor requested an example of the device acting as a consumer and a provider.
>>> Might have meant in the patch description?
>>>
>>> Conor?
>>
>> I wanted it in the property description to help with understanding when
>> to use it. I don't think the extra example nodes actually help you
>> understand what it is doing, only how to write one yourself once you
>> know you need it.
> I'm not sure if I get it right but what I understood is that a
> voltage-divider can
> also be a provider to other devices & hence the property.
> Also do you want me to put a complete example of it in description ?

My understanding is the requested example in the description should not
be exactly /how/ to hook up the voltage-divider as a provider, but
instead have some words about why it is interesting to do so at all. And
those words would also make it clear that is even possible. The latter
is something which, to be honest, is perhaps not all that obvious. It
has always been totally obvious to me of course, sorry for not being
clearer when I wrote the binding...

Cheers,
Peter

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

[Conor Dooley]

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

On Fri, Feb 02, 2024 at 12:49:26PM +0100, Peter Rosin wrote:
> 2024-02-02 at 11:43, Naresh Solanki wrote:
> > On Wed, 31 Jan 2024 at 22:24, Conor Dooley <conor@kernel.org> wrote:
> >> On Wed, Jan 31, 2024 at 04:35:16PM +0000, Jonathan Cameron wrote:
> >>> On Wed, 31 Jan 2024 09:29:59 +0100
> >>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> >>>> On 30/01/2024 12:56, Naresh Solanki wrote:
> >>> Conor requested an example of the device acting as a consumer and a provider.
> >>> Might have meant in the patch description?
> >>>
> >>> Conor?
> >>
> >> I wanted it in the property description to help with understanding when
> >> to use it. I don't think the extra example nodes actually help you
> >> understand what it is doing, only how to write one yourself once you
> >> know you need it.
> >
> > I'm not sure if I get it right but what I understood is that a
> > voltage-divider can
> > also be a provider to other devices & hence the property.
> > Also do you want me to put a complete example of it in description ?
> 
> My understanding is the requested example in the description should not
> be exactly /how/ to hook up the voltage-divider as a provider, but
> instead have some words about why it is interesting to do so at all. And
> those words would also make it clear that is even possible. The latter
> is something which, to be honest, is perhaps not all that obvious. It
> has always been totally obvious to me of course, sorry for not being
> clearer when I wrote the binding...

Yeah, you're right about what I was looking for Peter.

In my original request, which I think I already linked to in this
thread, I said that I would like an example like the one that Peter had
used to explain to me the scenario in which someone would want to use
this feature:
https://lore.kernel.org/all/536971eb-51f0-40e5-d025-7c4c1d683d49@axentia.se/

Cheers,
Conor.

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

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

[Naresh Solanki]

Hi Conor, Peter,

On Fri, 2 Feb 2024 at 18:39, Conor Dooley <conor.dooley@microchip.com> wrote:
>
> On Fri, Feb 02, 2024 at 12:49:26PM +0100, Peter Rosin wrote:
> > 2024-02-02 at 11:43, Naresh Solanki wrote:
> > > On Wed, 31 Jan 2024 at 22:24, Conor Dooley <conor@kernel.org> wrote:
> > >> On Wed, Jan 31, 2024 at 04:35:16PM +0000, Jonathan Cameron wrote:
> > >>> On Wed, 31 Jan 2024 09:29:59 +0100
> > >>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> > >>>> On 30/01/2024 12:56, Naresh Solanki wrote:
> > >>> Conor requested an example of the device acting as a consumer and a provider.
> > >>> Might have meant in the patch description?
> > >>>
> > >>> Conor?
> > >>
> > >> I wanted it in the property description to help with understanding when
> > >> to use it. I don't think the extra example nodes actually help you
> > >> understand what it is doing, only how to write one yourself once you
> > >> know you need it.
> > >
> > > I'm not sure if I get it right but what I understood is that a
> > > voltage-divider can
> > > also be a provider to other devices & hence the property.
> > > Also do you want me to put a complete example of it in description ?
> >
> > My understanding is the requested example in the description should not
> > be exactly /how/ to hook up the voltage-divider as a provider, but
> > instead have some words about why it is interesting to do so at all. And
> > those words would also make it clear that is even possible. The latter
> > is something which, to be honest, is perhaps not all that obvious. It
> > has always been totally obvious to me of course, sorry for not being
> > clearer when I wrote the binding...
>
> Yeah, you're right about what I was looking for Peter.
>
> In my original request, which I think I already linked to in this
> thread, I said that I would like an example like the one that Peter had
> used to explain to me the scenario in which someone would want to use
> this feature:
> https://lore.kernel.org/all/536971eb-51f0-40e5-d025-7c4c1d683d49@axentia.se/
ok. Based on my understanding, I'll update the property description
with an example.

description:
In addition to consuming the measurement services of a voltage output
channel the voltage divider can act as a provider of measurement
services to other devices. This is particularly useful in scenarios wherein,
ADC has analog frontend such as voltage divider then consuming its raw
value isn't interesting. It is desired to get real voltage before
voltage divider.

Regards,
Naresh

>
> Cheers,
> Conor.

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

[Conor Dooley]

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

On Mon, Feb 05, 2024 at 07:24:07PM +0530, Naresh Solanki wrote:
> Hi Conor, Peter,
> 
> On Fri, 2 Feb 2024 at 18:39, Conor Dooley <conor.dooley@microchip.com> wrote:
> >
> > On Fri, Feb 02, 2024 at 12:49:26PM +0100, Peter Rosin wrote:
> > > 2024-02-02 at 11:43, Naresh Solanki wrote:
> > > > On Wed, 31 Jan 2024 at 22:24, Conor Dooley <conor@kernel.org> wrote:
> > > >> On Wed, Jan 31, 2024 at 04:35:16PM +0000, Jonathan Cameron wrote:
> > > >>> On Wed, 31 Jan 2024 09:29:59 +0100
> > > >>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> > > >>>> On 30/01/2024 12:56, Naresh Solanki wrote:
> > > >>> Conor requested an example of the device acting as a consumer and a provider.
> > > >>> Might have meant in the patch description?
> > > >>>
> > > >>> Conor?
> > > >>
> > > >> I wanted it in the property description to help with understanding when
> > > >> to use it. I don't think the extra example nodes actually help you
> > > >> understand what it is doing, only how to write one yourself once you
> > > >> know you need it.
> > > >
> > > > I'm not sure if I get it right but what I understood is that a
> > > > voltage-divider can
> > > > also be a provider to other devices & hence the property.
> > > > Also do you want me to put a complete example of it in description ?
> > >
> > > My understanding is the requested example in the description should not
> > > be exactly /how/ to hook up the voltage-divider as a provider, but
> > > instead have some words about why it is interesting to do so at all. And
> > > those words would also make it clear that is even possible. The latter
> > > is something which, to be honest, is perhaps not all that obvious. It
> > > has always been totally obvious to me of course, sorry for not being
> > > clearer when I wrote the binding...
> >
> > Yeah, you're right about what I was looking for Peter.
> >
> > In my original request, which I think I already linked to in this
> > thread, I said that I would like an example like the one that Peter had
> > used to explain to me the scenario in which someone would want to use
> > this feature:
> > https://lore.kernel.org/all/536971eb-51f0-40e5-d025-7c4c1d683d49@axentia.se/
> ok. Based on my understanding, I'll update the property description
> with an example.
> 
> description:
> In addition to consuming the measurement services of a voltage output
> channel the voltage divider can act as a provider of measurement
> services to other devices.

> This is particularly useful in scenarios wherein,
> ADC has analog frontend such as voltage divider then consuming its raw
> value isn't interesting.

This sentence is structured pretty weirdly, it's missing articles and
prepositions, but you have the right idea here, thanks.


> It is desired to get real voltage before
> voltage divider.



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