On 22/11/2024 22:15, Marcelo Schmitt wrote: > On 11/22, Krzysztof Kozlowski wrote: >> On 22/11/2024 16:33, Marcelo Schmitt wrote: >>>> >>>>> + - items: >>>>> + - enum: >>>>> + - adi,ad7942 >>>>> + - const: adi,ad7946 >>>>> + >>>>> + - const: adi,ad7983 >>>>> + - items: >>>>> + - enum: >>>>> + - adi,ad7980 >>>>> + - adi,ad7988-5 >>>>> + - adi,ad7686 >>>>> + - adi,ad7685 >>>> >>>> Keep alphabetical order. >>> >>> Do the fallbacks declared here have any impact on the match try order or on how >>> the compatible list should be ordered? >> >> I don't understand, we do not talk about fallbacks. I also do not >> understand at all how this relates to my comment. > > I was wondering if the arrangement in which compatible strings appear in dt doc > could be used to suggest the sequence to add them to the compatible property of a > device node in a dts. Apparently, the arrangement of compatible strings in dt doc > has nothing to do with how they can appear in a dts file. Will sort them in > alphabetical order. We talk here about enum. Enum enumerates, so obviously they cannot appear one after another. > >> >>> The only significant difference between each group of devices is the sample rate. >>> A faster device can read at slower sample rates so if somebody knows to have >>> a 16-bit pseudo-differential PulSAR but doesn't know about the exact model they >>> could have a compatible like >>> compatible = "adi,ad7980", "adi,ad7988-5", "adi,ad7686", "adi,ad7685", >>> "adi,ad7988-1", "adi,ad7983"; >> >> Can't you autodetect this? > > There is no way of detecting the maximum sample rate other than the compatible > string or, maybe, running a data capture. Devices do not have version/revision/model register? > >> >>> >>> to try from fastest to slowest device. >>> The dt doc would indicate that order in the fallback list? >>> - items: >>> - enum: >>> - adi,ad7980 # Fastest 16-bit pseudo-differential ADC >>> - adi,ad7988-5 # 2nd fastest 16-bit pseudo-differential ADC >>> - adi,ad7686 # 3rd fastest 16-bit pseudo-differential ADC >>> - adi,ad7685 # 4th fastest 16-bit pseudo-differential ADC >>> - adi,ad7988-1 # 5th fastest 16-bit pseudo-differential ADC >>> - const: adi,ad7983 # Slowest 16-bit pseudo-differential ADC >> > [...] >>> >>> writing-bindings.rst says "DO use fallback compatibles when devices are the same >>> as or a subset of prior implementations." >>> But, how can we use fallbacks properly? >> >> How DT spec and tutorials like elinux ask... What is exactly the problem >> or question? > > Never mind. Do the bellow follow the preferred syntax? > > - items: > - enum: > - adi,ad7980 > - adi,ad7685 > - adi,ad7686 > - adi,ad7988-1 > - adi,ad7988-5 > - const: adi,ad7983 > > - items: > - enum: > - adi,ad7688 > - adi,ad7693 > - const: adi,ad7687 > > - items: > - enum: > - adi,ad7982 > - adi,ad7984 > - adi,ad7690 > - const: adi,ad7691 > > - enum: > - adi,ad7942 > - adi,ad7946 > - adi,ad7984 Yes > >> >>> From Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml I'm >> >> How LVDS bridge is related to this one here? > > Aside from having compatible fallbacks, not related. > >> >>> inferring only one fallback should be provided per group of devices. >>> >>>> >>>>> + - adi,ad7988-1 >>>>> + - const: adi,ad7983 >>>>> + >>>>> + - const: adi,ad7688 >>>>> + - items: >>>>> + - enum: >>>>> + - adi,ad7693 >>>>> + - adi,ad7687 >>>>> + - const: adi,ad7688 >>>>> + >>>>> + - const: adi,ad7984 >>>>> + - items: >>>>> + - enum: >>>>> + - adi,ad7982 >>>>> + - adi,ad7690 >>>>> + - adi,ad7691 >>>>> + - const: adi,ad7984 >>>>> + >>>>> reg: >>>>> maxItems: 1 >>>>> >>>>> @@ -133,6 +178,32 @@ required: >>>>> - ref-supply >>>>> >>>>> allOf: >>>>> + # Single-channel PulSAR devices have SDI either tied to VIO, GND, or host CS. >>>>> + - if: >>>>> + properties: >>>>> + compatible: >>>>> + contains: >>>>> + enum: >>>>> + - adi,ad7685 >>>> >>>> Why do you need this? It's fallback is already here. >>> >>> So dtbs_check can provide an error message if for example compatible = "adi,ad7687"; >>> and adi,sdi-pin = "sdi"; >> >> >> I mean this compatible, not if clause. > > dtbs_check don't show an error message if the allOf list only has the fallback > compatible for adi,ad7685 and a device node has both > compatible = "adi,ad7685" and adi,sdi-pin = "sdi". It must and your compatibles should not affect it. I don't know which code you are testing, but I even tested the correct approach and it correctly shows error. > > The new set of devices that will be supported by this binding don't have a > configuration register like the previous ones did. Because the PulSAR devices > don't have a config reg, they don't support all features of AD4000-like devices > and thus fewer IIO ABI interfaces are provided to user space. Though, AD4000 > devices also can be wired in a way that no reg access is possible, in which > case they provide the same IIO interfaces that PulSAR devices do. The difference > is on what is connected to the peripheral SDI pin. When AD4000 SDI is connected > to SPI controller MOSI line, more interfaces are provided because the config > reg can be accessed to set additional features. But that is not an option for > PulSAR devices. Even if controller MOSI is connected to a PulSAR device, we > cannot provide the additional interfaces because every attempt to use them would > fail (the device has no register to configure). No datasheets mentions > connecting a PulSAR device SDI pin to a SPI MOSI line. All datasheets show > PulSAR SDI pin connected either to VIO (high), GND (low), or controller CS. > > IMHO, it would be nice to have dtbs_check warn about invalid SDI pin > configuration otherwise it may only be noticed on driver probe. > Anyway, I'm also fine keeping only the fallback compatibles in the allOf list > if that makes dt maintainers happy. Only fallbacks go there, Best regards, Krzysztof
