* [RFC][PATCH] dt-bindings: iio: accel: add adi,adxl345.yaml binding @ 2019-05-06 11:46 Alexandru Ardelean 2019-05-06 12:39 ` Ardelean, Alexandru 2019-05-06 14:17 ` Rob Herring 0 siblings, 2 replies; 6+ messages in thread From: Alexandru Ardelean @ 2019-05-06 11:46 UTC (permalink / raw) To: linux-iio, devicetree; +Cc: robh+dt, Alexandru Ardelean This patch adds a YAML binding for the Analog Devices ADXL345 I2C/SPI accelerometer. Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> --- 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. 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 ? Question3: one idea that was neat in Zephyr, is that there is a 'inherits' + `!include` mechanism for including common SPI & I2C device stuff. It would be neat to have this. Is there a way to do this now, or maybe this would come later ? Maybe, just having a way to include a YAML file into another would be interesting. .../bindings/iio/accel/adi,adxl345.yaml | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml b/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml new file mode 100644 index 000000000000..246b90c07aaa --- /dev/null +++ b/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml @@ -0,0 +1,63 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/accelerometers/adi,adxl345.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices ADXL345/ADXL375 3-Axis Digital Accelerometers + +maintainers: + - Michael Hennerich <michael.hennerich@analog.com> + +description: | + Driver for Analog Devices ADXL345/ADXL375 3-Axis Digital Accelerometers + http://www.analog.com/en/products/mems/accelerometers/adxl345.html + http://www.analog.com/en/products/sensors-mems/accelerometers/adxl375.html + +properties: + compatible: + oneOf: + - items: + - enum: + - adi,adxl345 + - adi,adxl375 + + reg: + description: + The I2C address or SPI chip select number of the sensor + maxItems: 1 + + spi-cpha: + description: + SPI clock phase must be set, to select SPI mode 3 + + spi-cpol: + description: + SPI clock polarity must be set, to select SPI mode 3 + + interrupts: + description: + A variable number of interrupts warrants a description of what conditions + affect the number of interrupts. Otherwise, descriptions on standard + properties are not necessary. + +required: + - compatible + - reg + +examples: + - | + /* Example for a I2C device node */ + accelerometer@2a { + compatible = "adi,adxl345"; + reg = <0x53>; + }; + - | + /* Example for a SPI device node */ + accelerometer@0 { + compatible = "adi,adxl345"; + reg = <0>; + spi-max-frequency = <5000000>; + spi-cpol; + spi-cpha; + }; -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC][PATCH] dt-bindings: iio: accel: add adi,adxl345.yaml binding 2019-05-06 11:46 [RFC][PATCH] dt-bindings: iio: accel: add adi,adxl345.yaml binding Alexandru Ardelean @ 2019-05-06 12:39 ` Ardelean, Alexandru 2019-05-06 14:17 ` Rob Herring 1 sibling, 0 replies; 6+ messages in thread From: Ardelean, Alexandru @ 2019-05-06 12:39 UTC (permalink / raw) To: linux-iio, devicetree; +Cc: robh+dt On Mon, 2019-05-06 at 14:46 +0300, Alexandru Ardelean wrote: > This patch adds a YAML binding for the Analog Devices ADXL345 I2C/SPI > accelerometer. > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > --- > > 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. > > 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 ? > > Question3: one idea that was neat in Zephyr, is that there is a > 'inherits' > + `!include` mechanism for including common SPI & I2C device stuff. It > would be neat to have this. Is there a way to do this now, or maybe this > would come later ? Maybe, just having a way to include a YAML file into > another would be interesting. > > .../bindings/iio/accel/adi,adxl345.yaml | 63 +++++++++++++++++++ > 1 file changed, 63 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml > > diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml > b/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml > new file mode 100644 > index 000000000000..246b90c07aaa > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml > @@ -0,0 +1,63 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/accelerometers/adi,adxl345.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices ADXL345/ADXL375 3-Axis Digital Accelerometers > + > +maintainers: > + - Michael Hennerich <michael.hennerich@analog.com> > + > +description: | > + Driver for Analog Devices ADXL345/ADXL375 3-Axis Digital > Accelerometers > + http://www.analog.com/en/products/mems/accelerometers/adxl345.html > + > http://www.analog.com/en/products/sensors-mems/accelerometers/adxl375.html > + > +properties: > + compatible: > + oneOf: > + - items: > + - enum: > + - adi,adxl345 > + - adi,adxl375 > + > + reg: > + description: > + The I2C address or SPI chip select number of the sensor > + maxItems: 1 > + > + spi-cpha: > + description: > + SPI clock phase must be set, to select SPI mode 3 > + > + spi-cpol: > + description: > + SPI clock polarity must be set, to select SPI mode 3 > + > + interrupts: > + description: > + A variable number of interrupts warrants a description of what > conditions > + affect the number of interrupts. Otherwise, descriptions on > standard > + properties are not necessary. > + oops, since i left this un-changed oh, well, it's an RFC anyway :) it would have received a V2 anyway > +required: > + - compatible > + - reg > + > +examples: > + - | > + /* Example for a I2C device node */ > + accelerometer@2a { > + compatible = "adi,adxl345"; > + reg = <0x53>; > + }; > + - | > + /* Example for a SPI device node */ > + accelerometer@0 { > + compatible = "adi,adxl345"; > + reg = <0>; > + spi-max-frequency = <5000000>; > + spi-cpol; > + spi-cpha; > + }; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC][PATCH] dt-bindings: iio: accel: add adi,adxl345.yaml binding 2019-05-06 11:46 [RFC][PATCH] dt-bindings: iio: accel: add adi,adxl345.yaml binding Alexandru Ardelean 2019-05-06 12:39 ` Ardelean, Alexandru @ 2019-05-06 14:17 ` Rob Herring 2019-05-06 14:29 ` Ardelean, Alexandru 1 sibling, 1 reply; 6+ messages in thread From: Rob Herring @ 2019-05-06 14:17 UTC (permalink / raw) To: Alexandru Ardelean; +Cc: open list:IIO SUBSYSTEM AND DRIVERS, devicetree On Mon, May 6, 2019 at 6:46 AM Alexandru Ardelean <alexandru.ardelean@analog.com> wrote: > > This patch adds a YAML binding for the Analog Devices ADXL345 I2C/SPI > accelerometer. > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > --- > > 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. > Question3: one idea that was neat in Zephyr, is that there is a 'inherits' > + `!include` mechanism for including common SPI & I2C device stuff. It > would be neat to have this. Is there a way to do this now, or maybe this > would come later ? Maybe, just having a way to include a YAML file into > another would be interesting. This is done with the 'allOf' keyword at the top-level. See i2c-gpio.yaml for an example. I don't think that helps you here. If you had a long list of custom properties, then it might be useful. But that's orthogonal to your issue of needing conditional logic. > > .../bindings/iio/accel/adi,adxl345.yaml | 63 +++++++++++++++++++ > 1 file changed, 63 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml > > diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml b/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml > new file mode 100644 > index 000000000000..246b90c07aaa > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml > @@ -0,0 +1,63 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/accelerometers/adi,adxl345.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices ADXL345/ADXL375 3-Axis Digital Accelerometers > + > +maintainers: > + - Michael Hennerich <michael.hennerich@analog.com> > + > +description: | > + Driver for Analog Devices ADXL345/ADXL375 3-Axis Digital Accelerometers This is not a driver. > + http://www.analog.com/en/products/mems/accelerometers/adxl345.html > + http://www.analog.com/en/products/sensors-mems/accelerometers/adxl375.html > + > +properties: > + compatible: > + oneOf: > + - items: > + - enum: > + - adi,adxl345 > + - adi,adxl375 This can be simplified to just: compatible: enum: - adi,adxl345 - adi,adxl375 'oneOf' is only needed if you have different combinations of compatibles (a variable number or multiple fallbacks). 'items' is only needed if compatible is more than 1 string. > + > + reg: > + description: > + The I2C address or SPI chip select number of the sensor The top-level description should probably say the device supports I2C and SPI. You don't need a description here for a common property unless you have something device specific to add. > + maxItems: 1 > + > + spi-cpha: > + description: > + SPI clock phase must be set, to select SPI mode 3 Again, standard property, so just 'spi-cpha: true' is sufficient. > + > + spi-cpol: > + description: > + SPI clock polarity must be set, to select SPI mode 3 > + > + interrupts: > + description: > + A variable number of interrupts warrants a description of what conditions > + affect the number of interrupts. Otherwise, descriptions on standard > + properties are not necessary. This description from the example is explaining when you need a description. For a single interrupt, just need 'maxItems: 1'. > + > +required: > + - compatible > + - reg > + > +examples: > + - | > + /* Example for a I2C device node */ > + accelerometer@2a { This should give you an error because the examples default to assuming a simple-bus. So you need an 'i2c' node around this. > + compatible = "adi,adxl345"; > + reg = <0x53>; > + }; > + - | > + /* Example for a SPI device node */ > + accelerometer@0 { > + compatible = "adi,adxl345"; > + reg = <0>; > + spi-max-frequency = <5000000>; > + spi-cpol; > + spi-cpha; > + }; > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC][PATCH] dt-bindings: iio: accel: add adi,adxl345.yaml binding 2019-05-06 14:17 ` Rob Herring @ 2019-05-06 14:29 ` Ardelean, Alexandru 2019-05-13 17:25 ` Rob Herring 0 siblings, 1 reply; 6+ messages in thread From: Ardelean, Alexandru @ 2019-05-06 14:29 UTC (permalink / raw) To: robh+dt; +Cc: linux-iio, devicetree 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@analog.com> wrote: > > > > This patch adds a YAML binding for the Analog Devices ADXL345 I2C/SPI > > accelerometer. > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > --- > > > > 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 ? > > > Question3: one idea that was neat in Zephyr, is that there is a > > 'inherits' > > + `!include` mechanism for including common SPI & I2C device stuff. It > > would be neat to have this. Is there a way to do this now, or maybe > > this > > would come later ? Maybe, just having a way to include a YAML file into > > another would be interesting. > > This is done with the 'allOf' keyword at the top-level. See > i2c-gpio.yaml for an example. > > I don't think that helps you here. If you had a long list of custom > properties, then it might be useful. But that's orthogonal to your > issue of needing conditional logic. > > > > > .../bindings/iio/accel/adi,adxl345.yaml | 63 +++++++++++++++++++ > > 1 file changed, 63 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml > > b/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml > > new file mode 100644 > > index 000000000000..246b90c07aaa > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml > > @@ -0,0 +1,63 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > +%YAML 1.2 > > +--- > > +$id: > > http://devicetree.org/schemas/iio/accelerometers/adi,adxl345.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Analog Devices ADXL345/ADXL375 3-Axis Digital Accelerometers > > + > > +maintainers: > > + - Michael Hennerich <michael.hennerich@analog.com> > > + > > +description: | > > + Driver for Analog Devices ADXL345/ADXL375 3-Axis Digital > > Accelerometers > > This is not a driver. > > > + http://www.analog.com/en/products/mems/accelerometers/adxl345.html > > + > > http://www.analog.com/en/products/sensors-mems/accelerometers/adxl375.html > > + > > +properties: > > + compatible: > > + oneOf: > > + - items: > > + - enum: > > + - adi,adxl345 > > + - adi,adxl375 > > This can be simplified to just: > > compatible: > enum: > - adi,adxl345 > - adi,adxl375 > > 'oneOf' is only needed if you have different combinations of > compatibles (a variable number or multiple fallbacks). 'items' is only > needed if compatible is more than 1 string. > > > + > > + reg: > > + description: > > + The I2C address or SPI chip select number of the sensor > > The top-level description should probably say the device supports I2C > and SPI. You don't need a description here for a common property > unless you have something device specific to add. > > > + maxItems: 1 > > + > > + spi-cpha: > > + description: > > + SPI clock phase must be set, to select SPI mode 3 > > Again, standard property, so just 'spi-cpha: true' is sufficient. > > > + > > + spi-cpol: > > + description: > > + SPI clock polarity must be set, to select SPI mode 3 > > + > > + interrupts: > > + description: > > + A variable number of interrupts warrants a description of what > > conditions > > + affect the number of interrupts. Otherwise, descriptions on > > standard > > + properties are not necessary. > > This description from the example is explaining when you need a > description. For a single interrupt, just need 'maxItems: 1'. Yep, I noticed right after sending. Sorry about that. I think I started this file a few weeks ago, and came back from vacation a bit of an airhead. > > > + > > +required: > > + - compatible > > + - reg > > + > > +examples: > > + - | > > + /* Example for a I2C device node */ > > + accelerometer@2a { > > This should give you an error because the examples default to assuming > a simple-bus. So you need an 'i2c' node around this. > > > + compatible = "adi,adxl345"; > > + reg = <0x53>; > > + }; > > + - | > > + /* Example for a SPI device node */ > > + accelerometer@0 { > > + compatible = "adi,adxl345"; > > + reg = <0>; > > + spi-max-frequency = <5000000>; > > + spi-cpol; > > + spi-cpha; > > + }; > > -- > > 2.17.1 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC][PATCH] dt-bindings: iio: accel: add adi,adxl345.yaml binding 2019-05-06 14:29 ` Ardelean, Alexandru @ 2019-05-13 17:25 ` Rob Herring 2019-05-14 6:28 ` Ardelean, Alexandru 0 siblings, 1 reply; 6+ messages in thread From: Rob Herring @ 2019-05-13 17:25 UTC (permalink / raw) To: Ardelean, Alexandru; +Cc: linux-iio, devicetree 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@analog.com> wrote: > > > > > > This patch adds a YAML binding for the Analog Devices ADXL345 I2C/SPI > > > accelerometer. > > > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > > --- > > > > > > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC][PATCH] dt-bindings: iio: accel: add adi,adxl345.yaml binding 2019-05-13 17:25 ` Rob Herring @ 2019-05-14 6:28 ` Ardelean, Alexandru 0 siblings, 0 replies; 6+ messages in thread From: Ardelean, Alexandru @ 2019-05-14 6:28 UTC (permalink / raw) To: robh; +Cc: linux-iio, devicetree On Mon, 2019-05-13 at 12:25 -0500, Rob Herring wrote: > [External] > > > 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@analog.com> wrote: > > > > > > > > This patch adds a YAML binding for the Analog Devices ADXL345 > > > > I2C/SPI > > > > accelerometer. > > > > > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > > > --- > > > > > > > > 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. Hey, Many thanks :) I'll take a look the next couple of days. Alex > > Rob ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-05-14 6:28 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-06 11:46 [RFC][PATCH] dt-bindings: iio: accel: add adi,adxl345.yaml binding Alexandru Ardelean 2019-05-06 12:39 ` Ardelean, Alexandru 2019-05-06 14:17 ` Rob Herring 2019-05-06 14:29 ` Ardelean, Alexandru 2019-05-13 17:25 ` Rob Herring 2019-05-14 6:28 ` Ardelean, Alexandru
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).