[RFC][PATCH] dt-bindings: iio: accel: add adi,adxl345.yaml binding

[RFC][PATCH] dt-bindings: iio: accel: add adi,adxl345.yaml binding

[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

Re: [RFC][PATCH] dt-bindings: iio: accel: add adi,adxl345.yaml binding

[Ardelean, Alexandru]

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;
> +    };

Re: [RFC][PATCH] dt-bindings: iio: accel: add adi,adxl345.yaml binding

[Rob Herring]

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
>

Re: [RFC][PATCH] dt-bindings: iio: accel: add adi,adxl345.yaml binding

[Ardelean, Alexandru]

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
> > 

Re: [RFC][PATCH] dt-bindings: iio: accel: add adi,adxl345.yaml binding

[Rob Herring]

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

Re: [RFC][PATCH] dt-bindings: iio: accel: add adi,adxl345.yaml binding

[Ardelean, Alexandru]

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