On 9/2/24 1:38 PM, Alexandru Ardelean wrote: > On Mon, Sep 2, 2024 at 2:55 PM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote: >> >> On Mon, Sep 02, 2024 at 01:36:30PM +0300, Alexandru Ardelean wrote: >>> reg: >>> @@ -114,6 +118,25 @@ properties: >>> assumed that the pins are hardwired to VDD. >>> type: boolean >>> >>> +patternProperties: >>> + "^channel@([0-7])$": >>> + type: object >>> + $ref: adc.yaml >>> + unevaluatedProperties: false >>> + >>> + properties: >>> + reg: >>> + description: The channel number. >>> + minimum: 0 >>> + maximum: 7 >>> + >>> + diff-channels: true >> >> Shouldn't this be specific? > > Umm. > Specific how? > Like if:then check for certain compatible strings? > diff-channels is not a boolean property, it is an array of two integers that specify the positive and negative pins. So this should have e.g. minimum: and maximum: constraints for each item in the array. Even if we only use this like a boolean flag in the driver, we need to make the bindings use the already established definition of diff-channels from adc.yaml. It would look like this in the .dts: diff-channels = <1 1>; The datasheet numbers the pins 1 to 8 instead of 0 to 7, so it would make sense to have the pin number be reg + 1 (or redefine reg to be minimum: 1, maximum: 8). We also recently introduced a single-channel property that can be used when the pin number of of the input doesn't match the reg number. >> >>> + >>> + bipolar: true >>> + >>> + required: >>> + - reg >>> + >>> required: >>> - compatible >>> - reg >>> @@ -202,4 +225,44 @@ examples: >>> standby-gpios = <&gpio 24 GPIO_ACTIVE_LOW>; >>> }; >>> }; >>> + - | >>> + #include <dt-bindings/gpio/gpio.h> >>> + #include <dt-bindings/interrupt-controller/irq.h> >>> + spi { >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + adc@0 { >>> + compatible = "adi,ad7606c-18"; >>> + reg = <0>; >>> + spi-max-frequency = <1000000>; >>> + spi-cpol; >>> + spi-cpha; >>> + >>> + avcc-supply = <&adc_vref>; >>> + vdrive-supply = <&vdd_supply>; >>> + >>> + interrupts = <25 IRQ_TYPE_EDGE_FALLING>; >>> + interrupt-parent = <&gpio>; >>> + >>> + adi,conversion-start-gpios = <&gpio 17 GPIO_ACTIVE_HIGH>; >>> + >>> + adi,conversion-start-gpios = <&gpio 17 GPIO_ACTIVE_HIGH>; >>> + reset-gpios = <&gpio 27 GPIO_ACTIVE_HIGH>; >>> + adi,first-data-gpios = <&gpio 22 GPIO_ACTIVE_HIGH>; >>> + standby-gpios = <&gpio 24 GPIO_ACTIVE_LOW>; >>> + >>> + adi,sw-mode; >>> + >>> + channel@1 { >>> + reg = <1>; >>> + diff-channel; >> >> Where is this property defined (which schema)? >> >> Did you test it? > > Tested on my board. > But forgot to update the DT schema docs. > Though, if you're referring to testing it somehow via some make > command, I'm a little behind on how all this works now. > I'll go re-check the "make dtbs_check" and similar commands. > > Maybe I sound a bit old (now), but when I last saw these DT bindings > going from txt-to-yaml, they seemed relatively simple. > Now, they're almost like their own programming language. > I'll search for some quick setup guides for these; any pointers are welcome :) I can help you with this.