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

Message ID 20240130115651.457800-1-naresh.solanki@9elements.com
State New
Headers
Series [v3] dt-bindings: iio: afe: voltage-divider: Add io-channel-cells |

Commit Message

Naresh Solanki Jan. 30, 2024, 11:56 a.m. UTC
  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(-)


base-commit: 861c0981648f5b64c86fd028ee622096eb7af05a
  

Comments

Conor Dooley Jan. 30, 2024, 5:23 p.m. UTC | #1
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
>
  
Conor Dooley Jan. 31, 2024, 4:54 p.m. UTC | #2
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.
  
Peter Rosin Feb. 2, 2024, 11:49 a.m. UTC | #3
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
  
Conor Dooley Feb. 2, 2024, 1:07 p.m. UTC | #4
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.
  
Naresh Solanki Feb. 5, 2024, 1:54 p.m. UTC | #5
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.
  
Conor Dooley Feb. 5, 2024, 5:11 p.m. UTC | #6
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.
  

Patch

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>;
+    };
 ...