Message ID | 20220525181532.6805-4-ddrokosov@sberdevices.ru |
---|---|
State | Superseded |
Headers | show |
Series | iio: accel: add MSA311 accelerometer driver | expand |
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? >
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 --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
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