On 6/15/24 7:41 AM, Jonathan Cameron wrote: > On Wed, 12 Jun 2024 14:20:40 -0500 > David Lechner <dlechner@xxxxxxxxxxxx> 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@xxxxxxxxxxxx/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.
