| Message ID | 20241007-ad7380-fix-supplies-v1-2-badcf813c9b9@baylibre.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: adc: ad7380: fix several supplies issues | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 1 warnings, 30 lines checked |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Mon, Oct 07, 2024 at 05:45:45PM +0200, Julien Stephan wrote: > ad7380-4 is the only device from ad738x family that doesn't have an > internal reference. Moreover its external reference is called REFIN in > the datasheet while all other use REFIO as an optional external > reference. If refio-supply is omitted the internal reference is > used. > > Fix the binding by adding refin-supply and makes it required for > ad7380-4 only. Maybe let's just use refio as refin? Reference-IO fits here well. Otherwise you have two supplies for the same. Best regards, Krzysztof
On Tue, 8 Oct 2024 09:52:50 +0200 Krzysztof Kozlowski <krzk@kernel.org> wrote: > On Mon, Oct 07, 2024 at 05:45:45PM +0200, Julien Stephan wrote: > > ad7380-4 is the only device from ad738x family that doesn't have an > > internal reference. Moreover its external reference is called REFIN in > > the datasheet while all other use REFIO as an optional external > > reference. If refio-supply is omitted the internal reference is > > used. > > > > Fix the binding by adding refin-supply and makes it required for > > ad7380-4 only. > > Maybe let's just use refio as refin? Reference-IO fits here well. > Otherwise you have two supplies for the same. Whilst it is ugly, the effort this series is going to in order to paper over a naming mismatch makes me agree with Krzysztof. I think adding a comment to the dt-binding would be sensible though as people might fall into this hole. Other than the missing ret =, rest of series looks fine to me Jonathan > > Best regards, > Krzysztof >
Le jeu. 10 oct. 2024 à 20:22, Jonathan Cameron <jic23@kernel.org> a écrit : > > On Tue, 8 Oct 2024 09:52:50 +0200 > Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > On Mon, Oct 07, 2024 at 05:45:45PM +0200, Julien Stephan wrote: > > > ad7380-4 is the only device from ad738x family that doesn't have an > > > internal reference. Moreover its external reference is called REFIN in > > > the datasheet while all other use REFIO as an optional external > > > reference. If refio-supply is omitted the internal reference is > > > used. > > > > > > Fix the binding by adding refin-supply and makes it required for > > > ad7380-4 only. > > > > Maybe let's just use refio as refin? Reference-IO fits here well. > > Otherwise you have two supplies for the same. > Whilst it is ugly, the effort this series is going to in order > to paper over a naming mismatch makes me agree with Krzysztof. > > I think adding a comment to the dt-binding would be sensible > though as people might fall into this hole. > Hi Jonathan and Krzysztof, I am currently adding support for another chip to this family (ADAQ4380-4) and it also uses REFIN.. but in another way ad7380-4 does.. So: - ad7380-4 does not have any internal reference and use a mandatory refin supply as external reference - adaq4380-4 does not have external reference but uses a 3V internal reference derived from a 5V mandatory refin supply - all others (AFAIK) use an optional refio external reference. If omitted, use an internal 2.5V reference. I am not sure using a single refio-supply for all will make things clearer.. What do you think? Should I also send the adaq series now to bring more context? (I wanted feedback on this series first). Cheers Julien > Other than the missing ret =, rest of series looks fine to me > > Jonathan > > > > > Best regards, > > Krzysztof > > >
On Mon, 14 Oct 2024 11:00:39 +0200 Julien Stephan <jstephan@baylibre.com> wrote: > Le jeu. 10 oct. 2024 à 20:22, Jonathan Cameron <jic23@kernel.org> a écrit : > > > > On Tue, 8 Oct 2024 09:52:50 +0200 > > Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > > On Mon, Oct 07, 2024 at 05:45:45PM +0200, Julien Stephan wrote: > > > > ad7380-4 is the only device from ad738x family that doesn't have an > > > > internal reference. Moreover its external reference is called REFIN in > > > > the datasheet while all other use REFIO as an optional external > > > > reference. If refio-supply is omitted the internal reference is > > > > used. > > > > > > > > Fix the binding by adding refin-supply and makes it required for > > > > ad7380-4 only. > > > > > > Maybe let's just use refio as refin? Reference-IO fits here well. > > > Otherwise you have two supplies for the same. > > Whilst it is ugly, the effort this series is going to in order > > to paper over a naming mismatch makes me agree with Krzysztof. > > > > I think adding a comment to the dt-binding would be sensible > > though as people might fall into this hole. > > > > Hi Jonathan and Krzysztof, > > I am currently adding support for another chip to this family > (ADAQ4380-4) and it also uses REFIN.. but in another way ad7380-4 > does.. > So: > - ad7380-4 does not have any internal reference and use a mandatory > refin supply as external reference > - adaq4380-4 does not have external reference but uses a 3V internal > reference derived from a 5V mandatory refin supply > - all others (AFAIK) use an optional refio external reference. If > omitted, use an internal 2.5V reference. > > I am not sure using a single refio-supply for all will make things > clearer.. What do you think? Should I also send the adaq series now to > bring more context? (I wanted feedback on this series first). > Sounds like that context would be useful if you have it more or less ready to send anyway. I don't have particularly strong views on this either way. If we 'fix' the case you have here, old bindings should continue to work for the part you are moving over (though no need to have them in the dt-bindings file). Jonathan > Cheers > Julien > > > Other than the missing ret =, rest of series looks fine to me > > > > Jonathan > > > > > > > > Best regards, > > > Krzysztof > > > > >
Le lun. 14 oct. 2024 à 20:37, Jonathan Cameron <jic23@kernel.org> a écrit : > > On Mon, 14 Oct 2024 11:00:39 +0200 > Julien Stephan <jstephan@baylibre.com> wrote: > > > Le jeu. 10 oct. 2024 à 20:22, Jonathan Cameron <jic23@kernel.org> a écrit : > > > > > > On Tue, 8 Oct 2024 09:52:50 +0200 > > > Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > > > > On Mon, Oct 07, 2024 at 05:45:45PM +0200, Julien Stephan wrote: > > > > > ad7380-4 is the only device from ad738x family that doesn't have an > > > > > internal reference. Moreover its external reference is called REFIN in > > > > > the datasheet while all other use REFIO as an optional external > > > > > reference. If refio-supply is omitted the internal reference is > > > > > used. > > > > > > > > > > Fix the binding by adding refin-supply and makes it required for > > > > > ad7380-4 only. > > > > > > > > Maybe let's just use refio as refin? Reference-IO fits here well. > > > > Otherwise you have two supplies for the same. > > > Whilst it is ugly, the effort this series is going to in order > > > to paper over a naming mismatch makes me agree with Krzysztof. > > > > > > I think adding a comment to the dt-binding would be sensible > > > though as people might fall into this hole. > > > > > > > Hi Jonathan and Krzysztof, > > > > I am currently adding support for another chip to this family > > (ADAQ4380-4) and it also uses REFIN.. but in another way ad7380-4 > > does.. > > So: > > - ad7380-4 does not have any internal reference and use a mandatory > > refin supply as external reference > > - adaq4380-4 does not have external reference but uses a 3V internal > > reference derived from a 5V mandatory refin supply > > - all others (AFAIK) use an optional refio external reference. If > > omitted, use an internal 2.5V reference. > > > > I am not sure using a single refio-supply for all will make things > > clearer.. What do you think? Should I also send the adaq series now to > > bring more context? (I wanted feedback on this series first). > > > > Sounds like that context would be useful if you have it more or less > ready to send anyway. I don't have particularly strong views on this > either way. If we 'fix' the case you have here, old bindings should > continue to work for the part you are moving over (though no need > to have them in the dt-bindings file). > Hi Jonathan, Just sent the new series with an RFC tag. Cheers Julien > Jonathan > > > Cheers > > Julien > > > > > Other than the missing ret =, rest of series looks fine to me > > > > > > Jonathan > > > > > > > > > > > Best regards, > > > > Krzysztof > > > > > > > >
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml index 72c51b3de97b..74d82721637c 100644 --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml @@ -58,6 +58,7 @@ properties: vcc-supply: true vlogic-supply: true refio-supply: true + refin-supply: true aina-supply: description: @@ -127,6 +128,23 @@ allOf: ainc-supply: false aind-supply: false + # ad7380-4 uses refin-supply as external reference. + # All other chips from ad738x family use refio as optional external reference. + # When refio-supply is omitted, internal reference is used. + - if: + properties: + compatible: + enum: + - adi,ad7380-4 + then: + properties: + refio-supply: false + required: + - refin-supply + else: + properties: + refin-supply: false + examples: - | #include <dt-bindings/interrupt-controller/irq.h>
ad7380-4 is the only device from ad738x family that doesn't have an internal reference. Moreover its external reference is called REFIN in the datasheet while all other use REFIO as an optional external reference. If refio-supply is omitted the internal reference is used. Fix the binding by adding refin-supply and makes it required for ad7380-4 only. Fixes: 1a291cc8ee17 ("dt-bindings: iio: adc: ad7380: add support for ad738x-4 4 channels variants") Signed-off-by: Julien Stephan <jstephan@baylibre.com> --- .../devicetree/bindings/iio/adc/adi,ad7380.yaml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)