diff mbox series

[v2,3/3] dt-bindings: iio: accel: add dt-binding schema for msa311 accel driver

Message ID 20220525181532.6805-4-ddrokosov@sberdevices.ru (mailing list archive)
State Changes Requested
Headers show
Series iio: accel: add MSA311 accelerometer driver | expand

Commit Message

Dmitry Rokosov May 25, 2022, 6:15 p.m. UTC 
Introduce devicetree binding json-schema for MSA311 tri-axial,
low-g accelerometer driver.

Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
---
 .../bindings/iio/accel/memsensing,msa311.yaml | 62 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 63 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/accel/memsensing,msa311.yaml

Comments

Dmitry Rokosov May 25, 2022, 6:32 p.m. UTC | #1 
Hello Jonathan and Rob,

I didn't change two places which you mentioned in the v1 review, please
find my comments below.

On Wed, May 25, 2022 at 06:15:33PM +0000, Dmitry Rokosov wrote:

> +  interrupt-names:
> +    const: irq
I stay interrupt-names node here, because otherwise dt_binding_check
command shows such a warning:

====
  CHECK   Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dt.yaml
Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dt.yaml: accelerometer@62: 'interrupt-names' does not match any of the regexes: 'pinctrl-[0-9]+'
====

I can't delete this node from the example as well, because it's required for
msa311 dts i2c irq declaration.

Please help me to resolve this problem properly if possible. If we can
ignore such warning I'll delete interrupt-names in the next patchset's
version.

> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
Properties #address-cells and #size-cells are still located in the
schema example, because otherwise dt_binding_check raises the below 
warnings if we delete these properties:

=====
  DTC     Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dt.yaml
Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dts:27.17-30: Warning (reg_format): /example-0/i2c/accelerometer@62:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dt.yaml: Warning (pci_device_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dt.yaml: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dt.yaml: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dts:24.13-32.11: Warning (i2c_bus_bridge): /example-0/i2c: incorrect #address-cells for I2C bus
Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dts:24.13-32.11: Warning (i2c_bus_bridge): /example-0/i2c: incorrect #size-cells for I2C bus
Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dt.yaml: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dt.yaml: Warning (i2c_bus_reg): Failed prerequisite 'i2c_bus_bridge'
Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dt.yaml: Warning (spi_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dts:25.30-31.15: Warning (avoid_default_addr_size): /example-0/i2c/accelerometer@62: Relying on default #address-cells value
Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dts:25.30-31.15: Warning (avoid_default_addr_size): /example-0/i2c/accelerometer@62: Relying on default #size-cells value
Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dt.yaml: Warning (unique_unit_address_if_enabled): Failed prerequisite 'avoid_default_addr_size'
=====

What is best way to resolve such issues without providing #address-cells
and #size-cells values?
Jonathan Cameron May 28, 2022, 5:43 p.m. UTC | #2 
On Wed, 25 May 2022 18:32:45 +0000
Dmitry Rokosov <DDRokosov@sberdevices.ru> wrote:

> Hello Jonathan and Rob,
> 
> I didn't change two places which you mentioned in the v1 review, please
> find my comments below.
> 
> On Wed, May 25, 2022 at 06:15:33PM +0000, Dmitry Rokosov wrote:
> 
> > +  interrupt-names:
> > +    const: irq  
> I stay interrupt-names node here, because otherwise dt_binding_check
> command shows such a warning:
> 
> ====
>   CHECK   Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dt.yaml
> Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dt.yaml: accelerometer@62: 'interrupt-names' does not match any of the regexes: 'pinctrl-[0-9]+'
> ====
> 
> I can't delete this node from the example as well, because it's required for
> msa311 dts i2c irq declaration.

Sorry, you've lost me - what breaks if you drop it from the example?
I'd expect to see no interrupt-names being documented or in the example.

> 
> Please help me to resolve this problem properly if possible. If we can
> ignore such warning I'll delete interrupt-names in the next patchset's
> version.

We can't ignore the warning, so this comes down to what am I missing with
the need for it in the example...

> 
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +  
> Properties #address-cells and #size-cells are still located in the
> schema example, because otherwise dt_binding_check raises the below 
> warnings if we delete these properties:

They should be there for the i2c node, (as they are required for an i2c bus master
node) but you had them documented as being in the msa311 node.  If it's
not in the
accelerometer@62 {

}

section of the example documetnation doesn't belong on this file (it will be
elsewhere). 

The request is to drop the documentation of them (as we are documenting
the msa311 part of the binding only).  They should indeed still be there
in the example.

Jonathan



> 
> =====
>   DTC     Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dt.yaml
> Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dts:27.17-30: Warning (reg_format): /example-0/i2c/accelerometer@62:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
> Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dt.yaml: Warning (pci_device_reg): Failed prerequisite 'reg_format'
> Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dt.yaml: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
> Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dt.yaml: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
> Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dts:24.13-32.11: Warning (i2c_bus_bridge): /example-0/i2c: incorrect #address-cells for I2C bus
> Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dts:24.13-32.11: Warning (i2c_bus_bridge): /example-0/i2c: incorrect #size-cells for I2C bus
> Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dt.yaml: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
> Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dt.yaml: Warning (i2c_bus_reg): Failed prerequisite 'i2c_bus_bridge'
> Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dt.yaml: Warning (spi_bus_reg): Failed prerequisite 'reg_format'
> Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dts:25.30-31.15: Warning (avoid_default_addr_size): /example-0/i2c/accelerometer@62: Relying on default #address-cells value
> Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dts:25.30-31.15: Warning (avoid_default_addr_size): /example-0/i2c/accelerometer@62: Relying on default #size-cells value
> Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dt.yaml: Warning (unique_unit_address_if_enabled): Failed prerequisite 'avoid_default_addr_size'
> =====
> 
> What is best way to resolve such issues without providing #address-cells
> and #size-cells values?
>
Dmitry Rokosov June 2, 2022, 5:39 p.m. UTC | #3 
Jonathan,

On Sat, May 28, 2022 at 06:43:37PM +0100, Jonathan Cameron wrote:
> On Wed, 25 May 2022 18:32:45 +0000
> Dmitry Rokosov <DDRokosov@sberdevices.ru> wrote:
> 
> > > +  interrupt-names:
> > > +    const: irq  
> > I stay interrupt-names node here, because otherwise dt_binding_check
> > command shows such a warning:
> > 
> > ====
> >   CHECK   Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dt.yaml
> > Documentation/devicetree/bindings/iio/accel/memsensing,msa311.example.dt.yaml: accelerometer@62: 'interrupt-names' does not match any of the regexes: 'pinctrl-[0-9]+'
> > ====
> > 
> > I can't delete this node from the example as well, because it's required for
> > msa311 dts i2c irq declaration.
> 
> Sorry, you've lost me - what breaks if you drop it from the example?
> I'd expect to see no interrupt-names being documented or in the example.
> 
> > 
> > Please help me to resolve this problem properly if possible. If we can
> > ignore such warning I'll delete interrupt-names in the next patchset's
> > version.
> 
> We can't ignore the warning, so this comes down to what am I missing with
> the need for it in the example...
> 

You are totally right. I thought during i2c device probe we should
provide interrupt-names dts property because i2c irq parsing requires
it, but I was wrong. i2c_device_probe() function tries to parse irq
value using interrupt-names property and fallbacks to simple
of_irq_get() if interrupt-names property is missing. In other words,
interrupt-names property is not required for device node declaration, so
it can be removed from documentation. Thank you for pointing this out.

> > 
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/interrupt-controller/irq.h>
> > > +    i2c {
> > > +        #address-cells = <1>;
> > > +        #size-cells = <0>;
> > > +  
> > Properties #address-cells and #size-cells are still located in the
> > schema example, because otherwise dt_binding_check raises the below 
> > warnings if we delete these properties:
> 
> They should be there for the i2c node, (as they are required for an i2c bus master
> node) but you had them documented as being in the msa311 node.  If it's
> not in the
> accelerometer@62 {
> 
> }
> 
> section of the example documetnation doesn't belong on this file (it will be
> elsewhere). 
> 
> The request is to drop the documentation of them (as we are documenting
> the msa311 part of the binding only).  They should indeed still be there
> in the example.
> 
> Jonathan
> 

I've removed #address-cells and #size-cells properties from
doc section as well as interrupt-names. All dtbs checkings have passed
successfully. Thank you!
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/iio/accel/memsensing,msa311.yaml b/Documentation/devicetree/bindings/iio/accel/memsensing,msa311.yaml
new file mode 100644
index 000000000000..8d9cd56288cb
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/accel/memsensing,msa311.yaml
@@ -0,0 +1,62 @@ 
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/iio/accel/memsensing,msa311.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: MEMSensing digital 3-Axis accelerometer
+
+maintainers:
+  - Dmitry Rokosov <ddrokosov@sberdevices.ru>
+
+description: |
+  MSA311 is a tri-axial, low-g accelerometer with I2C digital output for
+  sensitivity consumer applications. It has dynamical user selectable full
+  scales range of +-2g/+-4g/+-8g/+-16g and allows acceleration measurements
+  with output data rates from 1Hz to 1000Hz.
+  Datasheet can be found at following URL
+  https://cdn-shop.adafruit.com/product-files/5309/MSA311-V1.1-ENG.pdf
+
+properties:
+  compatible:
+    const: memsensing,msa311
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  reg:
+    maxItems: 1
+    description: I2C registers address
+
+  interrupts:
+    maxItems: 1
+    description: optional I2C int pin can be freely mapped to specific func
+
+  interrupt-names:
+    const: irq
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        accelerometer@62 {
+            compatible = "memsensing,msa311";
+            reg = <0x62>;
+            interrupt-parent = <&gpio_intc>;
+            interrupts = <29 IRQ_TYPE_EDGE_RISING>;
+            interrupt-names = "irq";
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 55aeb25c004c..be39e5c214fe 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12482,6 +12482,7 @@  MEMSENSING MICROSYSTEMS MSA311 ACCELEROMETER DRIVER
 M:	Dmitry Rokosov <ddrokosov@sberdevices.ru>
 L:	linux-iio@vger.kernel.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/iio/accel/memsensing,msa311.yaml
 F:	drivers/iio/accel/msa311.c
 
 MEN A21 WATCHDOG DRIVER