On Mon, May 06, 2019 at 02:29:54PM +0000, Ardelean, Alexandru wrote: > On Mon, 2019-05-06 at 09:17 -0500, Rob Herring wrote: > > [External] > > > > > > On Mon, May 6, 2019 at 6:46 AM Alexandru Ardelean > > <alexandru.ardelean@xxxxxxxxxx> wrote: > > > > > > This patch adds a YAML binding for the Analog Devices ADXL345 I2C/SPI > > > accelerometer. > > > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx> > > > --- > > > > > > And now for the RFC part. > > > Normally, I would dig into source-code to try to figure this out, but > > > at > > > this point-in-time, I am low on time/energy to do this. > > > And maybe this helps trigger a discussion about this. > > > > > > Apologies if this has been coverted on the devicetree mailing list, but > > > at least we'd get some coverage on the IIO list (with this). > > > > > > The ADXL345 device (as others) supports both I2C & SPI interfaces. > > > > > > Question1: do we write 2 YAML files, or 1 ? I was looking at Zephyr > > > (for > > > some ideas/reference) but it seems to me that the YAML DT binding > > > format is > > > different than this one ? They write 2 files for ADXL372 (1 for SPI, 1 > > > for > > > I2C). > > > > > > Question1-a: one thing is that SPI requires some props to be `required` > > > that would not be required for the I2C binding. This could be solved by > > > doing 2 files, but if doing 1 YAML file, is there a way to do > > > conditional > > > `required` ? i.e. property is required if `SPI` ? not sure how to check > > > for > > > SPI vs I2C, it would be interesting (at some point) to somehow enforce > > > SPI/I2C bindings correctness. > > > > The challenge here is there's not really any way for the schema to > > know which bus it is. The only ways to know this are knowing all > > possible spi or i2c controller compatibles or using the parent node > > name (which hasn't been strictly enforced). In order to get this > > information available to the schema, we'd need to add the information > > to the node. We do this with '$nodename'. We could add a '$bus' > > property for example. The tools would have to understand different > > buses and things like I2C muxes complicate doing that. > > > > Once you have something like $bus available, you could either have 2 > > files with a custom 'select' that checks compatible and $bus or we > > could have 1 file using if/then/else keywords. However, we don't yet > > support if/then/else json-schema that was added in draft7, but that's > > being worked on by Maxime Ripard. > > > > However, for this case, I'd just not worry about the issue. Really, > > spi-cpha and spi-cpol should not be required. If only 1 mode is > > supported, the driver can know that. IOW, it is implied by the > > compatible strings. > > > > > > > Question2: `make dt_binding_check` seems to generate only the first > > > example. Is this known behavior, or do I need to take something else > > > into > > > consideration ? > > > > That's correct. I haven't figured out how to do a variable number of > > examples in kbuild. > > Then, would it be fine to have multiple examples, and wait for this to pop- > in the YAML dt stuff at a later point in time ? > Or, just 1 example ? I've now fixed this by extracting each example into a sub-node in the generated dts file, so multiple examples are fine now. The only restriction is labels can't be repeated. Rob