[PATCH v2 1/1] dt-bindings: iio: imx8qxp-adc: add missing vref-supply

[PATCH v2 1/1] dt-bindings: iio: imx8qxp-adc: add missing vref-supply

[Alexander Stein]

Although this property is used right now for IIO_CHAN_INFO_SCALE,
this ADC has two internal reference voltages, which the driver currently
doesn't make use of.

Fixes: db73419d8c06 ("dt-bindings: iio: adc: Add binding documentation for NXP IMX8QXP ADC")
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Thanks for your inputs. This improved descritpion should make it clear
that this property is only about the external reference voltage, not
about the optional, internal voltages.

Changes in v2:
* Improved commit message subject as suggested
* Add hint about feature flag regarding multiple, internal, reference
  voltages

 .../devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml       | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml b/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
index 63369ba388e4..878e67054d7c 100644
--- a/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
@@ -39,6 +39,12 @@ properties:
   power-domains:
     maxItems: 1
 
+  vref-supply:
+    description: |
+      External ADC reference voltage supply on VREFH pad. If VERID[MVI] is
+      set, there are additional, internal reference voltages selectable.
+      VREFH1 is always from VREFH pad.
+
   "#io-channel-cells":
     const: 1
 
@@ -72,6 +78,7 @@ examples:
             assigned-clocks = <&clk IMX_SC_R_ADC_0>;
             assigned-clock-rates = <24000000>;
             power-domains = <&pd IMX_SC_R_ADC_0>;
+            vref-supply = <&reg_1v8>;
             #io-channel-cells = <1>;
         };
     };
-- 
2.34.1

Re: [PATCH v2 1/1] dt-bindings: iio: imx8qxp-adc: add missing vref-supply

[Krzysztof Kozlowski]

On 24/04/2023 11:23, Alexander Stein wrote:
> Although this property is used right now for IIO_CHAN_INFO_SCALE,
> this ADC has two internal reference voltages, which the driver currently
> doesn't make use of.
> 
> Fixes: db73419d8c06 ("dt-bindings: iio: adc: Add binding documentation for NXP IMX8QXP ADC")
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

RE: [PATCH v2 1/1] dt-bindings: iio: imx8qxp-adc: add missing vref-supply

[Bough Chen]

> -----Original Message-----
> From: Alexander Stein <alexander.stein@ew.tq-group.com>
> Sent: 2023年4月24日 17:23
> To: Cai Huoqing <cai.huoqing@linux.dev>; Bough Chen <haibo.chen@nxp.com>;
> Jonathan Cameron <jic23@kernel.org>; Lars-Peter Clausen <lars@metafoo.de>;
> Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski
> <krzysztof.kozlowski+dt@linaro.org>; Shawn Guo <shawnguo@kernel.org>;
> Sascha Hauer <s.hauer@pengutronix.de>; Fabio Estevam
> <festevam@gmail.com>
> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>; Pengutronix Kernel
> Team <kernel@pengutronix.de>; dl-linux-imx <linux-imx@nxp.com>;
> linux-iio@vger.kernel.org; devicetree@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; Marco Felsch <m.felsch@pengutronix.de>
> Subject: [PATCH v2 1/1] dt-bindings: iio: imx8qxp-adc: add missing vref-supply
> 
> Although this property is used right now for IIO_CHAN_INFO_SCALE, this ADC
> has two internal reference voltages, which the driver currently doesn't make use
> of.
> 
> Fixes: db73419d8c06 ("dt-bindings: iio: adc: Add binding documentation for NXP
> IMX8QXP ADC")
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>

Thanks! 
Reviewed-by: Haibo Chen <haibo.chen@nxp.com>

> ---
> Thanks for your inputs. This improved descritpion should make it clear that this
> property is only about the external reference voltage, not about the optional,
> internal voltages.
> 
> Changes in v2:
> * Improved commit message subject as suggested
> * Add hint about feature flag regarding multiple, internal, reference
>   voltages
> 
>  .../devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml       | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
> b/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
> index 63369ba388e4..878e67054d7c 100644
> --- a/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
> @@ -39,6 +39,12 @@ properties:
>    power-domains:
>      maxItems: 1
> 
> +  vref-supply:
> +    description: |
> +      External ADC reference voltage supply on VREFH pad. If VERID[MVI] is
> +      set, there are additional, internal reference voltages selectable.
> +      VREFH1 is always from VREFH pad.
> +
>    "#io-channel-cells":
>      const: 1
> 
> @@ -72,6 +78,7 @@ examples:
>              assigned-clocks = <&clk IMX_SC_R_ADC_0>;
>              assigned-clock-rates = <24000000>;
>              power-domains = <&pd IMX_SC_R_ADC_0>;
> +            vref-supply = <&reg_1v8>;
>              #io-channel-cells = <1>;
>          };
>      };
> --
> 2.34.1

Re: [PATCH v2 1/1] dt-bindings: iio: imx8qxp-adc: add missing vref-supply

[Jonathan Cameron]

On Mon, 24 Apr 2023 11:23:12 +0200
Alexander Stein <alexander.stein@ew.tq-group.com> wrote:

> Although this property is used right now for IIO_CHAN_INFO_SCALE,
> this ADC has two internal reference voltages, which the driver currently
> doesn't make use of.
> 
> Fixes: db73419d8c06 ("dt-bindings: iio: adc: Add binding documentation for NXP IMX8QXP ADC")
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>

Interesting that we are talking her only about vrefh, what about vrefp?
I guess the assumption is that will be wired to 0V?

The first reference I found didn't seem to imply that was necessarily the
case. https://www.mouser.com/pdfDocs/IMX8QXPAEC.pdf


Jonathan

> ---
> Thanks for your inputs. This improved descritpion should make it clear
> that this property is only about the external reference voltage, not
> about the optional, internal voltages.
> 
> Changes in v2:
> * Improved commit message subject as suggested
> * Add hint about feature flag regarding multiple, internal, reference
>   voltages
> 
>  .../devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml       | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml b/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
> index 63369ba388e4..878e67054d7c 100644
> --- a/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
> @@ -39,6 +39,12 @@ properties:
>    power-domains:
>      maxItems: 1
>  
> +  vref-supply:
> +    description: |
> +      External ADC reference voltage supply on VREFH pad. If VERID[MVI] is
> +      set, there are additional, internal reference voltages selectable.
> +      VREFH1 is always from VREFH pad.
> +
>    "#io-channel-cells":
>      const: 1
>  
> @@ -72,6 +78,7 @@ examples:
>              assigned-clocks = <&clk IMX_SC_R_ADC_0>;
>              assigned-clock-rates = <24000000>;
>              power-domains = <&pd IMX_SC_R_ADC_0>;
> +            vref-supply = <&reg_1v8>;
>              #io-channel-cells = <1>;
>          };
>      };

Re: [PATCH v2 1/1] dt-bindings: iio: imx8qxp-adc: add missing vref-supply

[Alexander Stein]

Am Sonntag, 30. April 2023, 19:15:58 CEST schrieb Jonathan Cameron:
> On Mon, 24 Apr 2023 11:23:12 +0200
> 
> Alexander Stein <alexander.stein@ew.tq-group.com> wrote:
> > Although this property is used right now for IIO_CHAN_INFO_SCALE,
> > this ADC has two internal reference voltages, which the driver currently
> > doesn't make use of.
> > 
> > Fixes: db73419d8c06 ("dt-bindings: iio: adc: Add binding documentation for
> > NXP IMX8QXP ADC") Signed-off-by: Alexander Stein
> > <alexander.stein@ew.tq-group.com>
> 
> Interesting that we are talking her only about vrefh, what about vrefp?
> I guess the assumption is that will be wired to 0V?

What's vrefp? The only other signal related to ADC is vrefl.

> The first reference I found didn't seem to imply that was necessarily the
> case. https://www.mouser.com/pdfDocs/IMX8QXPAEC.pdf

Assuming you are talking about vrefh and vrefl, how is this a problem? Even if 
vrefl is not 0V, the voltage difference is what the ADC uses as reference 
voltage, so this should be supplied in the DT then.

Best regards
Alexander

> Jonathan
> 
> > ---
> > Thanks for your inputs. This improved descritpion should make it clear
> > that this property is only about the external reference voltage, not
> > about the optional, internal voltages.
> > 
> > Changes in v2:
> > * Improved commit message subject as suggested
> > * Add hint about feature flag regarding multiple, internal, reference
> > 
> >   voltages
> >  
> >  .../devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml       | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
> > b/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml index
> > 63369ba388e4..878e67054d7c 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
> > +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
> > 
> > @@ -39,6 +39,12 @@ properties:
> >    power-domains:
> >      maxItems: 1
> > 
> > +  vref-supply:
> > +    description: |
> > +      External ADC reference voltage supply on VREFH pad. If VERID[MVI]
> > is
> > +      set, there are additional, internal reference voltages selectable.
> > +      VREFH1 is always from VREFH pad.
> > +
> > 
> >    "#io-channel-cells":
> >      const: 1
> > 
> > @@ -72,6 +78,7 @@ examples:
> >              assigned-clocks = <&clk IMX_SC_R_ADC_0>;
> >              assigned-clock-rates = <24000000>;
> >              power-domains = <&pd IMX_SC_R_ADC_0>;
> > 
> > +            vref-supply = <&reg_1v8>;
> > 
> >              #io-channel-cells = <1>;
> >          
> >          };
> >      
> >      };


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH v2 1/1] dt-bindings: iio: imx8qxp-adc: add missing vref-supply

[Jonathan Cameron]

On Tue, 02 May 2023 08:34:12 +0200
Alexander Stein <alexander.stein@ew.tq-group.com> wrote:

> Am Sonntag, 30. April 2023, 19:15:58 CEST schrieb Jonathan Cameron:
> > On Mon, 24 Apr 2023 11:23:12 +0200
> > 
> > Alexander Stein <alexander.stein@ew.tq-group.com> wrote:  
> > > Although this property is used right now for IIO_CHAN_INFO_SCALE,
> > > this ADC has two internal reference voltages, which the driver currently
> > > doesn't make use of.
> > > 
> > > Fixes: db73419d8c06 ("dt-bindings: iio: adc: Add binding documentation for
> > > NXP IMX8QXP ADC") Signed-off-by: Alexander Stein
> > > <alexander.stein@ew.tq-group.com>  
> > 
> > Interesting that we are talking her only about vrefh, what about vrefp?
> > I guess the assumption is that will be wired to 0V?  
> 
> What's vrefp? The only other signal related to ADC is vrefl.

oops. Indeed vrefl.

> 
> > The first reference I found didn't seem to imply that was necessarily the
> > case. https://www.mouser.com/pdfDocs/IMX8QXPAEC.pdf  
> 
> Assuming you are talking about vrefh and vrefl, how is this a problem? Even if 
> vrefl is not 0V, the voltage difference is what the ADC uses as reference 
> voltage, so this should be supplied in the DT then.

Those two voltages can be supplied by entirely unrelated regulators (well
probably high precision references, but represented as regulators).  Those
may well need turning on.

I don't think there is any way to define that in DT without the consumer (here
the ADC) taking two different supplies.

Maybe that's a usecase no one cares about though and they will either supply
this with fixed regulators, or with a single precision reference supplying
both voltages.

Also, assuming these are used in a moderately normal reference way, if
vrefl is not 0V the driver should be supplying offset information to
consumers of this channel (userspace etc) to account for that.

meh. I don't care that much if this isn't a case we actually need to care about.
I just found it interesting the datasheet didn't talk about these as either
a differential pair (so balanced) or a reference 0V and vref.

I'll apply this as it stands.

Applied to the fixes-togreg branch of iio.git

Thanks,

Jonathan


> 
> Best regards
> Alexander
> 
> > Jonathan
> >   
> > > ---
> > > Thanks for your inputs. This improved descritpion should make it clear
> > > that this property is only about the external reference voltage, not
> > > about the optional, internal voltages.
> > > 
> > > Changes in v2:
> > > * Improved commit message subject as suggested
> > > * Add hint about feature flag regarding multiple, internal, reference
> > > 
> > >   voltages
> > >  
> > >  .../devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml       | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git
> > > a/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
> > > b/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml index
> > > 63369ba388e4..878e67054d7c 100644
> > > --- a/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
> > > +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
> > > 
> > > @@ -39,6 +39,12 @@ properties:
> > >    power-domains:
> > >      maxItems: 1
> > > 
> > > +  vref-supply:
> > > +    description: |
> > > +      External ADC reference voltage supply on VREFH pad. If VERID[MVI]
> > > is
> > > +      set, there are additional, internal reference voltages selectable.
> > > +      VREFH1 is always from VREFH pad.
> > > +
> > > 
> > >    "#io-channel-cells":
> > >      const: 1
> > > 
> > > @@ -72,6 +78,7 @@ examples:
> > >              assigned-clocks = <&clk IMX_SC_R_ADC_0>;
> > >              assigned-clock-rates = <24000000>;
> > >              power-domains = <&pd IMX_SC_R_ADC_0>;
> > > 
> > > +            vref-supply = <&reg_1v8>;
> > > 
> > >              #io-channel-cells = <1>;
> > >          
> > >          };
> > >      
> > >      };  
> 
>