| Message ID | 20240612-iio-adc-ad4695-v1-1-6a4ed251fc86@baylibre.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: adc: ad4695: new driver for AD4695 and similar ADCs | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 12/06/2024 21:20, David Lechner wrote: > Add device tree bindings for AD4695 and similar ADCs. > > Signed-off-by: David Lechner <dlechner@baylibre.com> > --- > .../devicetree/bindings/iio/adc/adi,ad4695.yaml | 297 +++++++++++++++++++++ > MAINTAINERS | 9 + > 2 files changed, 306 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml > new file mode 100644 > index 000000000000..8ff5bbbbef9f > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml > @@ -0,0 +1,297 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/adc/adi,ad4695.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices Easy Drive Multiplexed SAR Analog to Digital Converters > + > +maintainers: > + - Michael Hennerich <Michael.Hennerich@analog.com> > + - Nuno Sá <nuno.sa@analog.com> > + > +description: | > + A family of similar multi-channel analog to digital converters with SPI bus. > + > + * https://www.analog.com/en/products/ad4695.html > + * https://www.analog.com/en/products/ad4696.html > + * https://www.analog.com/en/products/ad4697.html > + * https://www.analog.com/en/products/ad4698.html > + > +$ref: /schemas/spi/spi-peripheral-props.yaml# > + > +properties: > + compatible: > + oneOf: > + - enum: > + - adi,ad4695 > + - adi,ad4697 > + # same chips in WLCSP package with more pins > + - items: > + - const: adi,ad4695-wlcsp Usually we do not add compatibles for such differences. Programming model is the same. Same for all other wlcsp. Unless something differs? > + - const: adi,ad4695 > + - items: > + - const: adi,ad4697-wlcsp > + - const: adi,ad4697 > + # same chips with higher max sample rate > + - items: > + - const: adi,ad4696 Anyway, keep all fallbacked variants in one entry, so enum with const fallback. > + - const: adi,ad4695 > + - items: > + - const: adi,ad4698 > + - const: adi,ad4697 > + # same chips with higher max sample rate in WLCSP package > + - items: > + - const: adi,ad4696-wlcsp > + - const: adi,ad4696 That's wrong. ad4696 is compatible with 4695 as stated before. It is not compatible with ad4695-wlcsp. > + - const: adi,ad4695-wlcsp > + - const: adi,ad4695 > + - items: > + - const: adi,ad4698-wlcsp > + - const: adi,ad4698 > + - const: adi,ad4697-wlcsp > + - const: adi,ad4697 > + > + reg: > + maxItems: 1 > + > + spi-max-frequency: > + maximum: 80000000 > + > + spi-cpol: true > + spi-cpha: true > + > + spi-rx-bus-width: > + minimum: 1 > + maximum: 4 > + > + avdd-supply: > + description: A 2.7 V to 5.5 V supply that powers the analog circuitry. > + > + ldo-in-supply: > + description: A 2.4 V to 5.5 V supply connected to the internal LDO input. > + > + vdd-supply: > + description: A 1.8V supply that powers the core circuitry. > + > + vio-supply: > + description: A 1.2V to 1.8V supply for the digital inputs and outputs. > + > + ref-supply: > + description: A 2.4 V to 5.1 V supply for the external reference voltage. > + > + refin-supply: > + description: A 2.4 V to 5.1 V supply for the internal reference buffer input. > + > + com-supply: > + description: Common voltage supply for pseudo-differential analog inputs. > + > + adi,no-ref-current-limit: > + $ref: /schemas/types.yaml#/definitions/flag > + description: > + When this flag is present, the REF Overvoltage Reduced Current protection > + is disabled. > + > + adi,no-ref-high-z: > + $ref: /schemas/types.yaml#/definitions/flag > + description: > + Enable this flag if the ref-supply requires Reference Input High-Z Mode > + to be disabled for proper operation. > + > + cnv-gpios: > + description: The Convert Input (CNV). If omitted, CNV is tied to SPI CS. > + maxItems: 1 > + > + reset-gpios: > + description: The Reset Input (RESET). Should be configured GPIO_ACTIVE_LOW. > + maxItems: 1 > + > + interrupts: > + minItems: 1 > + items: > + - description: > + Signal coming from the BSY_ALT_GP0 or GP3 pin that indicates a busy > + condition. > + - description: > + Signal coming from the BSY_ALT_GP0 or GP2 pin that indicates an alert > + condition. > + > + interrupt-names: > + minItems: 1 > + items: > + - const: busy > + - const: alert > + > + gpio-controller: true > + > + "#gpio-cells": > + const: 2 > + description: | > + The first cell is the GPn number: 0 to 3. > + The second cell takes standard GPIO flags. > + > + "#address-cells": > + const: 1 Blank line > + "#size-cells": > + const: 0 > + Best regards, Krzysztof
On 6/13/24 2:11 AM, Krzysztof Kozlowski wrote: > On 12/06/2024 21:20, David Lechner wrote: >> Add device tree bindings for AD4695 and similar ADCs. >> >> Signed-off-by: David Lechner <dlechner@baylibre.com> >> --- >> .../devicetree/bindings/iio/adc/adi,ad4695.yaml | 297 +++++++++++++++++++++ >> MAINTAINERS | 9 + >> 2 files changed, 306 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml >> new file mode 100644 >> index 000000000000..8ff5bbbbef9f >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml >> @@ -0,0 +1,297 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/iio/adc/adi,ad4695.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Analog Devices Easy Drive Multiplexed SAR Analog to Digital Converters >> + >> +maintainers: >> + - Michael Hennerich <Michael.Hennerich@analog.com> >> + - Nuno Sá <nuno.sa@analog.com> >> + >> +description: | >> + A family of similar multi-channel analog to digital converters with SPI bus. >> + >> + * https://www.analog.com/en/products/ad4695.html >> + * https://www.analog.com/en/products/ad4696.html >> + * https://www.analog.com/en/products/ad4697.html >> + * https://www.analog.com/en/products/ad4698.html >> + >> +$ref: /schemas/spi/spi-peripheral-props.yaml# >> + >> +properties: >> + compatible: >> + oneOf: >> + - enum: >> + - adi,ad4695 >> + - adi,ad4697 >> + # same chips in WLCSP package with more pins >> + - items: >> + - const: adi,ad4695-wlcsp > > Usually we do not add compatibles for such differences. Programming > model is the same. Same for all other wlcsp. Unless something differs? OK, I will drop all of the -wlcsp stuff. It made the .dts validation catch more stuff, but wasn't being used in the driver because there really isn't a difference other than some of the pins not being there. > >> + - const: adi,ad4695 >> + - items: >> + - const: adi,ad4697-wlcsp >> + - const: adi,ad4697 >> + # same chips with higher max sample rate I suppose one could make the argument that the programming model is the same on these too, but the maximum sampling frequency does seem like an important bit of information so that you don't try to set the conversion trigger rate too high. >> + - items: >> + - const: adi,ad4696 > > Anyway, keep all fallbacked variants in one entry, so enum with const > fallback. > >> + - const: adi,ad4695 >> + - items: >> + - const: adi,ad4698 >> + - const: adi,ad4697 >> + # same chips with higher max sample rate in WLCSP package >> + - items: >> + - const: adi,ad4696-wlcsp >> + - const: adi,ad4696 > > That's wrong. ad4696 is compatible with 4695 as stated before. It is not > compatible with ad4695-wlcsp. > >> + - const: adi,ad4695-wlcsp >> + - const: adi,ad4695 >> + - items: >> + - const: adi,ad4698-wlcsp >> + - const: adi,ad4698 >> + - const: adi,ad4697-wlcsp >> + - const: adi,ad4697 >> +
On 13/06/2024 15:57, David Lechner wrote: > >> >>> + - const: adi,ad4695 >>> + - items: >>> + - const: adi,ad4697-wlcsp >>> + - const: adi,ad4697 >>> + # same chips with higher max sample rate > > I suppose one could make the argument that the programming model is > the same on these too, but the maximum sampling frequency does seem > like an important bit of information so that you don't try to set > the conversion trigger rate too high. > which property is that? I don't see differences in the driver, so I don't get how these wlcsp compatibles allow you to control value of conversion trigger. Best regards, Krzysztof
On 6/13/24 9:18 AM, Krzysztof Kozlowski wrote: > On 13/06/2024 15:57, David Lechner wrote: >> >>> >>>> + - const: adi,ad4695 >>>> + - items: >>>> + - const: adi,ad4697-wlcsp >>>> + - const: adi,ad4697 >>>> + # same chips with higher max sample rate >> >> I suppose one could make the argument that the programming model is >> the same on these too, but the maximum sampling frequency does seem >> like an important bit of information so that you don't try to set >> the conversion trigger rate too high. >> > > which property is that? I don't see differences in the driver, so I > don't get how these wlcsp compatibles allow you to control value of > conversion trigger. This comment is unrelated to the package type (WLCSP or LFCSP). What I mean is that e.g. AD4695 and AD4696 are virtually identical other than the maximum allowable sample rate (500 kSPS or 1 MSPS). So my thinking was that it would make sense to have: compatible = "ad4695"; for the lower sample rate chip and compatible = "ad4696", "ad4695"; for the higher sample rate chip since ad4696 can do everything that ad4695 does plus a bit more. We haven't implemented buffered reads in the driver yet, so there isn't anything currently to be seen there. But when we do, we probably want to limit the allowable value for the sampling_frequency attribute based on which version of the chip is present. (I would like to get Jonathan's opinion of if this is something we actually need to do or not.)
On Thu, 2024-06-13 at 09:39 -0500, David Lechner wrote: > On 6/13/24 9:18 AM, Krzysztof Kozlowski wrote: > > On 13/06/2024 15:57, David Lechner wrote: > > > > > > > > > > > > + - const: adi,ad4695 > > > > > + - items: > > > > > + - const: adi,ad4697-wlcsp > > > > > + - const: adi,ad4697 > > > > > + # same chips with higher max sample rate > > > > > > I suppose one could make the argument that the programming model is > > > the same on these too, but the maximum sampling frequency does seem > > > like an important bit of information so that you don't try to set > > > the conversion trigger rate too high. > > > > > > > which property is that? I don't see differences in the driver, so I > > don't get how these wlcsp compatibles allow you to control value of > > conversion trigger. > > This comment is unrelated to the package type (WLCSP or LFCSP). > > What I mean is that e.g. AD4695 and AD4696 are virtually identical > other than the maximum allowable sample rate (500 kSPS or 1 MSPS). > > So my thinking was that it would make sense to have: > > compatible = "ad4695"; > > for the lower sample rate chip and > > compatible = "ad4696", "ad4695"; > > for the higher sample rate chip since ad4696 can do everything > that ad4695 does plus a bit more. > IMO, that would make sense yes. If the higher sample rate chip fallsback, it will still work but not at full speed. The other way around is the one that we can't allow naturally. But possibly dumb question now... since both devices will be supported at the same time, do we actually care about having the fallback compatible? My understanding of the fallback story is that we may load a DTS in an older kernel where chip A is supported but chip B is not and it is ok for chip B to fallback to chip A. Since these devices will be supported at the same time, do we need to care? Unless out of tree stuff enters the equation? Or is there another usecase that I'm not aware about (or maybe it just makes sense to document properly...)? - Nuno Sá >
On Thu, Jun 13, 2024 at 05:11:48PM +0200, Nuno Sá wrote: > On Thu, 2024-06-13 at 09:39 -0500, David Lechner wrote: > > On 6/13/24 9:18 AM, Krzysztof Kozlowski wrote: > > > On 13/06/2024 15:57, David Lechner wrote: > > > > > > > > > > > > > > > + - const: adi,ad4695 > > > > > > + - items: > > > > > > + - const: adi,ad4697-wlcsp > > > > > > + - const: adi,ad4697 > > > > > > + # same chips with higher max sample rate > > > > > > > > I suppose one could make the argument that the programming model is > > > > the same on these too, but the maximum sampling frequency does seem > > > > like an important bit of information so that you don't try to set > > > > the conversion trigger rate too high. > > > > > > > > > > which property is that? I don't see differences in the driver, so I > > > don't get how these wlcsp compatibles allow you to control value of > > > conversion trigger. > > > > This comment is unrelated to the package type (WLCSP or LFCSP). > > > > What I mean is that e.g. AD4695 and AD4696 are virtually identical > > other than the maximum allowable sample rate (500 kSPS or 1 MSPS). > > > > So my thinking was that it would make sense to have: > > > > compatible = "ad4695"; > > > > for the lower sample rate chip and > > > > compatible = "ad4696", "ad4695"; > > > > for the higher sample rate chip since ad4696 can do everything > > that ad4695 does plus a bit more. > > > > IMO, that would make sense yes. If the higher sample rate chip fallsback, it will > still work but not at full speed. The other way around is the one that we can't allow > naturally. > > But possibly dumb question now... since both devices will be supported at the same > time, do we actually care about having the fallback compatible? My understanding of > the fallback story is that we may load a DTS in an older kernel where chip A is > supported but chip B is not and it is ok for chip B to fallback to chip A. Since > these devices will be supported at the same time, do we need to care? Unless out of > tree stuff enters the equation? Yeah, it doesn't really matter much in that case. > Or is there another usecase that I'm not aware about (or maybe it just makes sense to > document properly...)? Somewhat I guess. Perhaps if there's a 3rd chip with higher rate, then it will be more obvious what to do and we don't have to have this discussion again for it. :) Rob
On 6/13/24 2:43 PM, Rob Herring wrote: > On Thu, Jun 13, 2024 at 05:11:48PM +0200, Nuno Sá wrote: >> On Thu, 2024-06-13 at 09:39 -0500, David Lechner wrote: >>> On 6/13/24 9:18 AM, Krzysztof Kozlowski wrote: >>>> On 13/06/2024 15:57, David Lechner wrote: >>>>> >>>>>> >>>>>>> + - const: adi,ad4695 >>>>>>> + - items: >>>>>>> + - const: adi,ad4697-wlcsp >>>>>>> + - const: adi,ad4697 >>>>>>> + # same chips with higher max sample rate >>>>> >>>>> I suppose one could make the argument that the programming model is >>>>> the same on these too, but the maximum sampling frequency does seem >>>>> like an important bit of information so that you don't try to set >>>>> the conversion trigger rate too high. >>>>> >>>> >>>> which property is that? I don't see differences in the driver, so I >>>> don't get how these wlcsp compatibles allow you to control value of >>>> conversion trigger. >>> >>> This comment is unrelated to the package type (WLCSP or LFCSP). >>> >>> What I mean is that e.g. AD4695 and AD4696 are virtually identical >>> other than the maximum allowable sample rate (500 kSPS or 1 MSPS). >>> >>> So my thinking was that it would make sense to have: >>> >>> compatible = "ad4695"; >>> >>> for the lower sample rate chip and >>> >>> compatible = "ad4696", "ad4695"; >>> >>> for the higher sample rate chip since ad4696 can do everything >>> that ad4695 does plus a bit more. >>> >> >> IMO, that would make sense yes. If the higher sample rate chip fallsback, it will >> still work but not at full speed. The other way around is the one that we can't allow >> naturally. >> >> But possibly dumb question now... since both devices will be supported at the same >> time, do we actually care about having the fallback compatible? My understanding of >> the fallback story is that we may load a DTS in an older kernel where chip A is >> supported but chip B is not and it is ok for chip B to fallback to chip A. Since >> these devices will be supported at the same time, do we need to care? Unless out of >> tree stuff enters the equation? > > Yeah, it doesn't really matter much in that case. > >> Or is there another usecase that I'm not aware about (or maybe it just makes sense to >> document properly...)? > > Somewhat I guess. Perhaps if there's a 3rd chip with higher rate, then > it will be more obvious what to do and we don't have to have this > discussion again for it. :) > > Rob It sounds like maybe the best thing to do here then is just keep it simple? Like this: compatible: enum: - adi,ad4695 - adi,ad4696 - adi,ad4697 - adi,ad4698
On Thu, 13 Jun 2024 15:09:40 -0500 David Lechner <dlechner@baylibre.com> wrote: > On 6/13/24 2:43 PM, Rob Herring wrote: > > On Thu, Jun 13, 2024 at 05:11:48PM +0200, Nuno Sá wrote: > >> On Thu, 2024-06-13 at 09:39 -0500, David Lechner wrote: > >>> On 6/13/24 9:18 AM, Krzysztof Kozlowski wrote: > >>>> On 13/06/2024 15:57, David Lechner wrote: > >>>>> > >>>>>> > >>>>>>> + - const: adi,ad4695 > >>>>>>> + - items: > >>>>>>> + - const: adi,ad4697-wlcsp > >>>>>>> + - const: adi,ad4697 > >>>>>>> + # same chips with higher max sample rate > >>>>> > >>>>> I suppose one could make the argument that the programming model is > >>>>> the same on these too, but the maximum sampling frequency does seem > >>>>> like an important bit of information so that you don't try to set > >>>>> the conversion trigger rate too high. > >>>>> > >>>> > >>>> which property is that? I don't see differences in the driver, so I > >>>> don't get how these wlcsp compatibles allow you to control value of > >>>> conversion trigger. > >>> > >>> This comment is unrelated to the package type (WLCSP or LFCSP). > >>> > >>> What I mean is that e.g. AD4695 and AD4696 are virtually identical > >>> other than the maximum allowable sample rate (500 kSPS or 1 MSPS). > >>> > >>> So my thinking was that it would make sense to have: > >>> > >>> compatible = "ad4695"; > >>> > >>> for the lower sample rate chip and > >>> > >>> compatible = "ad4696", "ad4695"; > >>> > >>> for the higher sample rate chip since ad4696 can do everything > >>> that ad4695 does plus a bit more. > >>> > >> > >> IMO, that would make sense yes. If the higher sample rate chip fallsback, it will > >> still work but not at full speed. The other way around is the one that we can't allow > >> naturally. > >> > >> But possibly dumb question now... since both devices will be supported at the same > >> time, do we actually care about having the fallback compatible? My understanding of > >> the fallback story is that we may load a DTS in an older kernel where chip A is > >> supported but chip B is not and it is ok for chip B to fallback to chip A. Since > >> these devices will be supported at the same time, do we need to care? Unless out of > >> tree stuff enters the equation? > > > > Yeah, it doesn't really matter much in that case. > > > >> Or is there another usecase that I'm not aware about (or maybe it just makes sense to > >> document properly...)? > > > > Somewhat I guess. Perhaps if there's a 3rd chip with higher rate, then > > it will be more obvious what to do and we don't have to have this > > discussion again for it. :) > > > > Rob > > It sounds like maybe the best thing to do here then is just keep it simple? > > Like this: > > compatible: > enum: > - adi,ad4695 > - adi,ad4696 > - adi,ad4697 > - adi,ad4698 > Quick comments late in discussion. Simply compatible list is fine as they are all coming together (though that may in theory not be true on some other OS!) I'm also fine with the fallback as you discussed so less capable chip is the one we fall back to. Definitely need to differentiate chips with different possible ranges of a parameter as that goes straight through to userspace as sampling_frequency_available and userspace is going to get confused if it gets told it can do things that silently don't work. This sort of thing is why I always want to see chip IDs listed even if we think they'll just work with another one. a) Documents hardware right and people expect to see the ID of the chip on the BoM b) Lets us deal with cases where we have missed subtle differences in capabilities because the driver didn't support them yet. So in conclusion either option you proposed is fine (as above, or fallback to the part we think is a subset of functionality.) Jonathan
On Wed, 12 Jun 2024 14:20:40 -0500 David Lechner <dlechner@baylibre.com> wrote: > Add device tree bindings for AD4695 and similar ADCs. > > Signed-off-by: David Lechner <dlechner@baylibre.com> > --- > .../devicetree/bindings/iio/adc/adi,ad4695.yaml | 297 +++++++++++++++++++++ > MAINTAINERS | 9 + > 2 files changed, 306 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml > new file mode 100644 > index 000000000000..8ff5bbbbef9f > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml > @@ -0,0 +1,297 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/adc/adi,ad4695.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices Easy Drive Multiplexed SAR Analog to Digital Converters > + > +maintainers: > + - Michael Hennerich <Michael.Hennerich@analog.com> > + - Nuno Sá <nuno.sa@analog.com> > + > +description: | > + A family of similar multi-channel analog to digital converters with SPI bus. > + > + * https://www.analog.com/en/products/ad4695.html > + * https://www.analog.com/en/products/ad4696.html > + * https://www.analog.com/en/products/ad4697.html > + * https://www.analog.com/en/products/ad4698.html > + > +$ref: /schemas/spi/spi-peripheral-props.yaml# > + > +properties: > + compatible: > + oneOf: > + - enum: > + - adi,ad4695 > + - adi,ad4697 > + # same chips in WLCSP package with more pins > + - items: > + - const: adi,ad4695-wlcsp > + - const: adi,ad4695 > + - items: > + - const: adi,ad4697-wlcsp > + - const: adi,ad4697 > + # same chips with higher max sample rate > + - items: > + - const: adi,ad4696 > + - const: adi,ad4695 > + - items: > + - const: adi,ad4698 > + - const: adi,ad4697 > + # same chips with higher max sample rate in WLCSP package > + - items: > + - const: adi,ad4696-wlcsp > + - const: adi,ad4696 > + - const: adi,ad4695-wlcsp > + - const: adi,ad4695 > + - items: > + - const: adi,ad4698-wlcsp > + - const: adi,ad4698 > + - const: adi,ad4697-wlcsp > + - const: adi,ad4697 > + > + reg: > + maxItems: 1 > + > + spi-max-frequency: > + maximum: 80000000 > + > + spi-cpol: true > + spi-cpha: true > + > + spi-rx-bus-width: > + minimum: 1 > + maximum: 4 > + > + avdd-supply: > + description: A 2.7 V to 5.5 V supply that powers the analog circuitry. I'm a cynic. Do we care about the supported voltages in this binding doc? Feels just somewhere we might make a mistake. > + > + ldo-in-supply: > + description: A 2.4 V to 5.5 V supply connected to the internal LDO input. > + > + vdd-supply: > + description: A 1.8V supply that powers the core circuitry. > + > + vio-supply: > + description: A 1.2V to 1.8V supply for the digital inputs and outputs. > + > + ref-supply: > + description: A 2.4 V to 5.1 V supply for the external reference voltage. > + > + refin-supply: > + description: A 2.4 V to 5.1 V supply for the internal reference buffer input. > + > + com-supply: > + description: Common voltage supply for pseudo-differential analog inputs. These last few have more info in them so definitely good to have the descriptions > + > + adi,no-ref-current-limit: > + $ref: /schemas/types.yaml#/definitions/flag > + description: > + When this flag is present, the REF Overvoltage Reduced Current protection > + is disabled. > + > + adi,no-ref-high-z: > + $ref: /schemas/types.yaml#/definitions/flag > + description: > + Enable this flag if the ref-supply requires Reference Input High-Z Mode > + to be disabled for proper operation. > + > + cnv-gpios: > + description: The Convert Input (CNV). If omitted, CNV is tied to SPI CS. > + maxItems: 1 > + > + reset-gpios: > + description: The Reset Input (RESET). Should be configured GPIO_ACTIVE_LOW. > + maxItems: 1 > + > + interrupts: > + minItems: 1 > + items: > + - description: > + Signal coming from the BSY_ALT_GP0 or GP3 pin that indicates a busy > + condition. > + - description: > + Signal coming from the BSY_ALT_GP0 or GP2 pin that indicates an alert > + condition. > + > + interrupt-names: > + minItems: 1 > + items: > + - const: busy > + - const: alert > + > + gpio-controller: true > + > + "#gpio-cells": > + const: 2 > + description: | > + The first cell is the GPn number: 0 to 3. > + The second cell takes standard GPIO flags. > + > + "#address-cells": > + const: 1 > + "#size-cells": > + const: 0 > + > +patternProperties: > + "^channel@[0-9a-f]$": > + type: object > + $ref: adc.yaml > + unevaluatedProperties: false > + description: > + Describes each individual channel. In addition the properties defined > + below, bipolar from adc.yaml is also supported. > + > + properties: > + reg: > + maximum: 15 > + description: Input pin number (INx). I'd drop this description as the pin pairing makes it messy. If you switch to diff-channels etc, just leave it as a index value not connected to the pin numbers. > + > + adi,pin-pairing: > + description: | > + The input pin pairing for the negative input. This can be: > + - REFGND, normally 0V (single-ended) > + - COM, normally V_REF/2, see com-supply (pseudo-differential) > + - For even numbered pins, the next odd numbered pin (differential) > + $ref: /schemas/types.yaml#/definitions/string > + enum: [refgnd, com, next] Next is full on differential, just provide both channels via diff-channels. You can constrain the particular combinations in the binding. Refcnd is normal single ended. Probably want to use the new single-channel property for that as we are mixing differential and single ended channels so reg is pretty much just an index. Hmm. For comm we haven't had done a recent binding for a chip with the option of pseudo differential that is per channel, they've been whole device only. That feels like it will be common enough we need to support it cleanly with a 'general' scheme. Problem is I know someone will have a chip with 2 vincom pins and selecting between them, so we can't just have pseudo-differential as a boolean and adc.yaml There are horrible solutions like a magic channel number that changes the meaning of diff-channels but that's ugly. Maybe pseudo-differential for now and we have to later we add pseudo-differential-comm = <0> etc? > + default: refgnd > + > + adi,no-high-z: > + $ref: /schemas/types.yaml#/definitions/flag > + description: | Do we need the | given not really formatted? > + Enable this flag if the input pin requires the Analog Input High-Z > + Mode to be disabled for proper operation. > + > + required: > + - reg > + > + allOf: > + # only even number pins can be paired with the next pin > + - if: > + properties: > + reg: > + not: > + multipleOf: 2 > + then: > + properties: > + adi,pin-pairing: > + enum: [refgnd, com] > + # bipolar mode is not supported with REFGND pairing > + - if: > + not: > + required: > + - adi,pin-pairing > + then: > + properties: > + bipolar: false > + else: > + if: > + properties: > + adi,pin-pairing: > + const: refgnd > + then: > + properties: > + bipolar: false > + > +required: > + - compatible > + - reg > + - avdd-supply > + - vio-supply > + > +allOf: > + - oneOf: > + - required: > + - ref-supply > + - required: > + - refin-supply > + > + - oneOf: > + - required: > + - ldo-in-supply > + - required: > + - vdd-supply > + > + # LFSCP package has fewer pins, so a few things are not valid in that case > + - if: > + properties: > + compatible: > + not: > + contains: > + pattern: -wlcsp$ > + then: > + properties: > + refin-supply: false > + spi-rx-bus-width: > + maximum: 2 > + > + # the internal reference buffer always requires high-z mode > + - if: > + required: > + - refin-supply > + then: > + properties: > + adi,no-ref-high-z: false > + > + # limit channels for 8-channel chips > + - if: > + properties: > + compatible: > + contains: > + const: adi,ad4697 > + then: > + patternProperties: > + "^channel@[0-7]$": > + properties: > + reg: > + maximum: 7 > + "^channel@[8-9a-f]$": false > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + > + spi { > + #address-cells = <1>; > + #size-cells = <0>; > + > + adc@0 { > + compatible = "adi,ad4695"; > + reg = <0>; > + spi-cpol; > + spi-cpha; > + spi-max-frequency = <80000000>; > + avdd-supply = <&supply_2_5V>; > + vdd-supply = <&supply_1_8V>; > + vio-supply = <&supply_1_2V>; > + ref-supply = <&supply_5V>; > + reset-gpios = <&gpio 1 GPIO_ACTIVE_LOW>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + /* Differential channel between IN0 and IN1. */ > + channel@0 { > + reg = <0>; > + adi,pin-pairing = "next"; > + bipolar; > + }; > + > + /* Single-ended channel between IN2 and REFGND. */ > + channel@2 { > + reg = <2>; > + }; > + > + /* Pseudo-differential channel between IN3 and COM. */ > + channel@f { > + reg = <3>; > + adi,pin-pairing = "com"; > + bipolar; > + }; > + }; > + };
On 6/15/24 7:41 AM, Jonathan Cameron wrote: > On Wed, 12 Jun 2024 14:20:40 -0500 > David Lechner <dlechner@baylibre.com> wrote: > >> Add device tree bindings for AD4695 and similar ADCs. >> ... >> + >> + avdd-supply: >> + description: A 2.7 V to 5.5 V supply that powers the analog circuitry. > > I'm a cynic. Do we care about the supported voltages in this binding doc? > Feels just somewhere we might make a mistake. Not especially useful, I suppose. I'll clean these up a bit. > >> + >> + ldo-in-supply: >> + description: A 2.4 V to 5.5 V supply connected to the internal LDO input. >> + >> + vdd-supply: >> + description: A 1.8V supply that powers the core circuitry. >> + >> + vio-supply: >> + description: A 1.2V to 1.8V supply for the digital inputs and outputs. >> + >> + ref-supply: >> + description: A 2.4 V to 5.1 V supply for the external reference voltage. >> + >> + refin-supply: >> + description: A 2.4 V to 5.1 V supply for the internal reference buffer input. >> + >> + com-supply: >> + description: Common voltage supply for pseudo-differential analog inputs. > > These last few have more info in them so definitely good to have the descriptions > ... >> + >> +patternProperties: >> + "^channel@[0-9a-f]$": >> + type: object >> + $ref: adc.yaml >> + unevaluatedProperties: false >> + description: >> + Describes each individual channel. In addition the properties defined >> + below, bipolar from adc.yaml is also supported. >> + >> + properties: >> + reg: >> + maximum: 15 >> + description: Input pin number (INx). > > I'd drop this description as the pin pairing makes it messy. > If you switch to diff-channels etc, just leave it as a index value not > connected to the pin numbers. > >> + >> + adi,pin-pairing: >> + description: | >> + The input pin pairing for the negative input. This can be: >> + - REFGND, normally 0V (single-ended) >> + - COM, normally V_REF/2, see com-supply (pseudo-differential) >> + - For even numbered pins, the next odd numbered pin (differential) >> + $ref: /schemas/types.yaml#/definitions/string >> + enum: [refgnd, com, next] > > Next is full on differential, just provide both channels via > diff-channels. You can constrain the particular combinations in the binding. > > Refcnd is normal single ended. Probably want to use the new single-channel > property for that as we are mixing differential and single ended channels > so reg is pretty much just an index. > > Hmm. For comm we haven't had done a recent binding for a chip with the option > of pseudo differential that is per channel, they've been whole device only. > That feels like it will be common enough we need to support it cleanly > with a 'general' scheme. I think we have. :-) https://lore.kernel.org/linux-iio/adc6cba9-2e79-475f-9c24-039fe9d3345d@baylibre.com/T/#mcbc1ce3a2541db502bf7870b7ea8574626a46312 > > Problem is I know someone will have a chip with 2 vincom pins and selecting > between them, so we can't just have pseudo-differential as a boolean and adc.yaml > > There are horrible solutions like a magic channel number that changes the > meaning of diff-channels but that's ugly. > Maybe pseudo-differential for now and we have to later we add > pseudo-differential-comm = <0> etc? > I was trying to keep things simple with 1 property instead of 3, but we can drop adi,pin-pairing and use the standard diff-channels, single-channel and common-mode-channel properties. > >> + default: refgnd >> + >> + adi,no-high-z: >> + $ref: /schemas/types.yaml#/definitions/flag >> + description: | > > Do we need the | given not really formatted? No. Was probably copy/paste. > >> + Enable this flag if the input pin requires the Analog Input High-Z >> + Mode to be disabled for proper operation. >> + ... >> +examples: >> + - | >> + #include <dt-bindings/gpio/gpio.h> >> + >> + spi { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + adc@0 { >> + compatible = "adi,ad4695"; >> + reg = <0>; >> + spi-cpol; >> + spi-cpha; >> + spi-max-frequency = <80000000>; >> + avdd-supply = <&supply_2_5V>; >> + vdd-supply = <&supply_1_8V>; >> + vio-supply = <&supply_1_2V>; >> + ref-supply = <&supply_5V>; >> + reset-gpios = <&gpio 1 GPIO_ACTIVE_LOW>; >> + >> + #address-cells = <1>; >> + #size-cells = <0>; >> + Using the standard adc.yaml properties, these would now be: >> + /* Differential channel between IN0 and IN1. */ >> + channel@0 { >> + reg = <0>; diff-channels = <0>, <1>; >> + bipolar; >> + }; >> + >> + /* Single-ended channel between IN2 and REFGND. */ >> + channel@2 { >> + reg = <2>; single-channel = <2>; common-mode-channel = <0>; >> + }; >> + >> + /* Pseudo-differential channel between IN3 and COM. */ >> + channel@f { >> + reg = <3>; single-channel = <3>; common-mode-channel = <1>; >> + bipolar; >> + }; >> + }; >> + }; > And I will add a header file with macros for the common mode channel values.
> >> + > >> + adi,pin-pairing: > >> + description: | > >> + The input pin pairing for the negative input. This can be: > >> + - REFGND, normally 0V (single-ended) > >> + - COM, normally V_REF/2, see com-supply (pseudo-differential) > >> + - For even numbered pins, the next odd numbered pin (differential) > >> + $ref: /schemas/types.yaml#/definitions/string > >> + enum: [refgnd, com, next] > > > > Next is full on differential, just provide both channels via > > diff-channels. You can constrain the particular combinations in the binding. > > > > Refcnd is normal single ended. Probably want to use the new single-channel > > property for that as we are mixing differential and single ended channels > > so reg is pretty much just an index. > > > > Hmm. For comm we haven't had done a recent binding for a chip with the option > > of pseudo differential that is per channel, they've been whole device only. > > That feels like it will be common enough we need to support it cleanly > > with a 'general' scheme. > > I think we have. :-) > > https://lore.kernel.org/linux-iio/adc6cba9-2e79-475f-9c24-039fe9d3345d@baylibre.com/T/#mcbc1ce3a2541db502bf7870b7ea8574626a46312 > My goldfish like memory strikes again. Had completely forgotten that :) > > > > Problem is I know someone will have a chip with 2 vincom pins and selecting > > between them, so we can't just have pseudo-differential as a boolean and adc.yaml > > > > There are horrible solutions like a magic channel number that changes the > > meaning of diff-channels but that's ugly. > > Maybe pseudo-differential for now and we have to later we add > > pseudo-differential-comm = <0> etc? > > > > I was trying to keep things simple with 1 property instead of 3, but we > can drop adi,pin-pairing and use the standard diff-channels, single-channel > and common-mode-channel properties. Sounds good. > > >> +examples: > >> + - | > >> + #include <dt-bindings/gpio/gpio.h> > >> + > >> + spi { > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> + > >> + adc@0 { > >> + compatible = "adi,ad4695"; > >> + reg = <0>; > >> + spi-cpol; > >> + spi-cpha; > >> + spi-max-frequency = <80000000>; > >> + avdd-supply = <&supply_2_5V>; > >> + vdd-supply = <&supply_1_8V>; > >> + vio-supply = <&supply_1_2V>; > >> + ref-supply = <&supply_5V>; > >> + reset-gpios = <&gpio 1 GPIO_ACTIVE_LOW>; > >> + > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> + > > Using the standard adc.yaml properties, these would now be: > > >> + /* Differential channel between IN0 and IN1. */ > >> + channel@0 { > >> + reg = <0>; > > diff-channels = <0>, <1>; > > >> + bipolar; > >> + }; > >> + > >> + /* Single-ended channel between IN2 and REFGND. */ > >> + channel@2 { > >> + reg = <2>; > > single-channel = <2>; > common-mode-channel = <0>; I wonder if we count ground ad default and so don't necessarily specify this? I don't really mind either way though. Maybe just default to 0 works. > > >> + }; > >> + > >> + /* Pseudo-differential channel between IN3 and COM. */ > >> + channel@f { > >> + reg = <3>; > > single-channel = <3>; > common-mode-channel = <1>; > > >> + bipolar; > >> + }; > >> + }; > >> + }; > > > > And I will add a header file with macros for the common mode > channel values. Great Jonathan >
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml new file mode 100644 index 000000000000..8ff5bbbbef9f --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml @@ -0,0 +1,297 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/adi,ad4695.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices Easy Drive Multiplexed SAR Analog to Digital Converters + +maintainers: + - Michael Hennerich <Michael.Hennerich@analog.com> + - Nuno Sá <nuno.sa@analog.com> + +description: | + A family of similar multi-channel analog to digital converters with SPI bus. + + * https://www.analog.com/en/products/ad4695.html + * https://www.analog.com/en/products/ad4696.html + * https://www.analog.com/en/products/ad4697.html + * https://www.analog.com/en/products/ad4698.html + +$ref: /schemas/spi/spi-peripheral-props.yaml# + +properties: + compatible: + oneOf: + - enum: + - adi,ad4695 + - adi,ad4697 + # same chips in WLCSP package with more pins + - items: + - const: adi,ad4695-wlcsp + - const: adi,ad4695 + - items: + - const: adi,ad4697-wlcsp + - const: adi,ad4697 + # same chips with higher max sample rate + - items: + - const: adi,ad4696 + - const: adi,ad4695 + - items: + - const: adi,ad4698 + - const: adi,ad4697 + # same chips with higher max sample rate in WLCSP package + - items: + - const: adi,ad4696-wlcsp + - const: adi,ad4696 + - const: adi,ad4695-wlcsp + - const: adi,ad4695 + - items: + - const: adi,ad4698-wlcsp + - const: adi,ad4698 + - const: adi,ad4697-wlcsp + - const: adi,ad4697 + + reg: + maxItems: 1 + + spi-max-frequency: + maximum: 80000000 + + spi-cpol: true + spi-cpha: true + + spi-rx-bus-width: + minimum: 1 + maximum: 4 + + avdd-supply: + description: A 2.7 V to 5.5 V supply that powers the analog circuitry. + + ldo-in-supply: + description: A 2.4 V to 5.5 V supply connected to the internal LDO input. + + vdd-supply: + description: A 1.8V supply that powers the core circuitry. + + vio-supply: + description: A 1.2V to 1.8V supply for the digital inputs and outputs. + + ref-supply: + description: A 2.4 V to 5.1 V supply for the external reference voltage. + + refin-supply: + description: A 2.4 V to 5.1 V supply for the internal reference buffer input. + + com-supply: + description: Common voltage supply for pseudo-differential analog inputs. + + adi,no-ref-current-limit: + $ref: /schemas/types.yaml#/definitions/flag + description: + When this flag is present, the REF Overvoltage Reduced Current protection + is disabled. + + adi,no-ref-high-z: + $ref: /schemas/types.yaml#/definitions/flag + description: + Enable this flag if the ref-supply requires Reference Input High-Z Mode + to be disabled for proper operation. + + cnv-gpios: + description: The Convert Input (CNV). If omitted, CNV is tied to SPI CS. + maxItems: 1 + + reset-gpios: + description: The Reset Input (RESET). Should be configured GPIO_ACTIVE_LOW. + maxItems: 1 + + interrupts: + minItems: 1 + items: + - description: + Signal coming from the BSY_ALT_GP0 or GP3 pin that indicates a busy + condition. + - description: + Signal coming from the BSY_ALT_GP0 or GP2 pin that indicates an alert + condition. + + interrupt-names: + minItems: 1 + items: + - const: busy + - const: alert + + gpio-controller: true + + "#gpio-cells": + const: 2 + description: | + The first cell is the GPn number: 0 to 3. + The second cell takes standard GPIO flags. + + "#address-cells": + const: 1 + "#size-cells": + const: 0 + +patternProperties: + "^channel@[0-9a-f]$": + type: object + $ref: adc.yaml + unevaluatedProperties: false + description: + Describes each individual channel. In addition the properties defined + below, bipolar from adc.yaml is also supported. + + properties: + reg: + maximum: 15 + description: Input pin number (INx). + + adi,pin-pairing: + description: | + The input pin pairing for the negative input. This can be: + - REFGND, normally 0V (single-ended) + - COM, normally V_REF/2, see com-supply (pseudo-differential) + - For even numbered pins, the next odd numbered pin (differential) + $ref: /schemas/types.yaml#/definitions/string + enum: [refgnd, com, next] + default: refgnd + + adi,no-high-z: + $ref: /schemas/types.yaml#/definitions/flag + description: | + Enable this flag if the input pin requires the Analog Input High-Z + Mode to be disabled for proper operation. + + required: + - reg + + allOf: + # only even number pins can be paired with the next pin + - if: + properties: + reg: + not: + multipleOf: 2 + then: + properties: + adi,pin-pairing: + enum: [refgnd, com] + # bipolar mode is not supported with REFGND pairing + - if: + not: + required: + - adi,pin-pairing + then: + properties: + bipolar: false + else: + if: + properties: + adi,pin-pairing: + const: refgnd + then: + properties: + bipolar: false + +required: + - compatible + - reg + - avdd-supply + - vio-supply + +allOf: + - oneOf: + - required: + - ref-supply + - required: + - refin-supply + + - oneOf: + - required: + - ldo-in-supply + - required: + - vdd-supply + + # LFSCP package has fewer pins, so a few things are not valid in that case + - if: + properties: + compatible: + not: + contains: + pattern: -wlcsp$ + then: + properties: + refin-supply: false + spi-rx-bus-width: + maximum: 2 + + # the internal reference buffer always requires high-z mode + - if: + required: + - refin-supply + then: + properties: + adi,no-ref-high-z: false + + # limit channels for 8-channel chips + - if: + properties: + compatible: + contains: + const: adi,ad4697 + then: + patternProperties: + "^channel@[0-7]$": + properties: + reg: + maximum: 7 + "^channel@[8-9a-f]$": false + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + spi { + #address-cells = <1>; + #size-cells = <0>; + + adc@0 { + compatible = "adi,ad4695"; + reg = <0>; + spi-cpol; + spi-cpha; + spi-max-frequency = <80000000>; + avdd-supply = <&supply_2_5V>; + vdd-supply = <&supply_1_8V>; + vio-supply = <&supply_1_2V>; + ref-supply = <&supply_5V>; + reset-gpios = <&gpio 1 GPIO_ACTIVE_LOW>; + + #address-cells = <1>; + #size-cells = <0>; + + /* Differential channel between IN0 and IN1. */ + channel@0 { + reg = <0>; + adi,pin-pairing = "next"; + bipolar; + }; + + /* Single-ended channel between IN2 and REFGND. */ + channel@2 { + reg = <2>; + }; + + /* Pseudo-differential channel between IN3 and COM. */ + channel@f { + reg = <3>; + adi,pin-pairing = "com"; + bipolar; + }; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index c870bc6b8d63..8d15e4089d7c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1209,6 +1209,15 @@ F: Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130 F: Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml F: drivers/iio/adc/ad4130.c +ANALOG DEVICES INC AD4695 DRIVER +M: Michael Hennerich <michael.hennerich@analog.com> +M: Nuno Sá <nuno.sa@analog.com> +R: David Lechner <dlechner@baylibre.com> +L: linux-iio@vger.kernel.org +S: Supported +W: https://ez.analog.com/linux-software-drivers +F: Documentation/devicetree/bindings/iio/adc/adi,ad4695.yaml + ANALOG DEVICES INC AD7091R DRIVER M: Marcelo Schmitt <marcelo.schmitt@analog.com> L: linux-iio@vger.kernel.org
Add device tree bindings for AD4695 and similar ADCs. Signed-off-by: David Lechner <dlechner@baylibre.com> --- .../devicetree/bindings/iio/adc/adi,ad4695.yaml | 297 +++++++++++++++++++++ MAINTAINERS | 9 + 2 files changed, 306 insertions(+)