2022-05-26 08:39:22

by Dmitry Rokosov

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] dt-bindings: iio: accel: add dt-binding schema for msa311 accel driver

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?

--
Thank you,
Dmitry


2022-05-28 20:02:26

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] dt-bindings: iio: accel: add dt-binding schema for msa311 accel driver

On Wed, 25 May 2022 18:32:45 +0000
Dmitry Rokosov <[email protected]> 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?
>


2022-06-03 17:47:41

by Dmitry Rokosov

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] dt-bindings: iio: accel: add dt-binding schema for msa311 accel driver

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 <[email protected]> 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!

--
Thank you,
Dmitry