Re: [PATCH v3 3/3] dt-bindings: iio: accel: add dt-binding schema for msa311 accel driver

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 16 Jun 2022 10:42:17 +0000
Dmitry Rokosov <DDRokosov@sberdevices.ru> wrote:

> Introduce devicetree binding json-schema for MSA311 tri-axial,
> low-g accelerometer driver.
> 
> Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
Hi Dmitry,

A few trivial suggestions to drop description entries that don't
useful information.

One thing we often end up adding very soon after new bindings are
introduced is power supplies.  If sensible to do so, it's better
to introduce them at the start and save on the noise.
Looks like this one just needs a property entry of

  vdd-supply: true

Obviously you then need to get it in the driver and turn it on
(+ off via a devm_add_action_or_reset() call) as minimal support.

Thanks,

Jonathan

> ---
>  .../bindings/iio/accel/memsensing,msa311.yaml | 52 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 53 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/accel/memsensing,msa311.yaml
> 
> 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..072632708d42
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/accel/memsensing,msa311.yaml
> @@ -0,0 +1,52 @@
> +# 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
> +
> +  reg:
> +    maxItems: 1
> +    description: I2C registers address

No need for description. It always means that for i2c devices.

> +
> +  interrupts:
> +    maxItems: 1
> +    description: optional I2C int pin can be freely mapped to specific func

Why I2C int?  Is there anything associating it with i2c specifically?

I'm not sure the description adds anything useful for this so I'd
drop it.

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