| Message ID | 20230424092312.61746-1-alexander.stein@ew.tq-group.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/1] dt-bindings: iio: imx8qxp-adc: add missing vref-supply | expand |
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
> -----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 = <®_1v8>; > #io-channel-cells = <1>; > }; > }; > -- > 2.34.1
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 = <®_1v8>; > #io-channel-cells = <1>; > }; > };
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 = <®_1v8>; > > > > #io-channel-cells = <1>; > > > > }; > > > > };
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 = <®_1v8>; > > > > > > #io-channel-cells = <1>; > > > > > > }; > > > > > > }; > >
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 = <®_1v8>; #io-channel-cells = <1>; }; };
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(+)