linux-kernel - Re: [PATCH v1 1/2] dt-bindings: iio: accel: add ADXL367

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20211206143934.000017b8@Huawei.com>
Date:   Mon, 6 Dec 2021 14:39:47 +0000
From:   Jonathan Cameron <Jonathan.Cameron@...wei.com>
To:     Cosmin Tanislav <demonsingur@...il.com>
CC:     <cosmin.tanislav@...log.com>, Lars-Peter Clausen <lars@...afoo.de>,
        Michael Hennerich <Michael.Hennerich@...log.com>,
        Rob Herring <robh+dt@...nel.org>, <linux-iio@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 1/2] dt-bindings: iio: accel: add ADXL367

On Mon,  6 Dec 2021 12:54:02 +0200
Cosmin Tanislav <demonsingur@...il.com> wrote:

> The ADXL367 is an ultralow power, 3-axis MEMS accelerometer.
> 
> The ADXL367 does not alias input signals to achieve ultralow power
> consumption, it samples the full bandwidth of the sensor at all
> data rates. Measurement ranges of +-2g, +-4g, and +-8g are available,
> with a resolution of 0.25mg/LSB on the +-2 g range.
> 
> In addition to its ultralow power consumption, the ADXL367
> has many features to enable true system level power reduction.
> It includes a deep multimode output FIFO, a built-in micropower
> temperature sensor, and an internal ADC for synchronous conversion
> of an additional analog input.
> 
> Signed-off-by: Cosmin Tanislav <cosmin.tanislav@...log.com>

Hi Cosmin,

Given how often we get patches later to add regulators for devices like
these I'd like them supported from the start.

I'm guessing it needs power, but how many supplies? I'm not sure as doesn't
seem to be a public datasheet yet.

> ---
>  .../bindings/iio/accel/adi,adxl367.yaml       | 79 +++++++++++++++++++
>  1 file changed, 79 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/accel/adi,adxl367.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adxl367.yaml b/Docu`mentation/devicetree/bindings/iio/accel/adi,adxl367.yaml
> new file mode 100644
> index 000000000000..1bf9e1602480
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/accel/adi,adxl367.yaml
> @@ -0,0 +1,79 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/accel/adi,adxl367.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices ADXL367 3-Axis Digital Accelerometer
> +
> +maintainers:
> +  - Cosmin Tanislav <cosmin.tanislav@...log.com>
> +
> +description: |
> +  The ADXL367 is an ultralow power, 3-axis MEMS accelerometer.
> +
> +  The ADXL367 does not alias input signals by to achieve ultralow power
> +  consumption, it samples the full bandwidth of the sensor at all
> +  data rates. Measurement ranges of +-2g, +-4g, and +-8g are available,
> +  with a resolution of 0.25mg/LSB on the +-2 g range.
> +
> +  In addition to its ultralow power consumption, the ADXL367
> +  has many features to enable true system level power reduction.
> +  It includes a deep multimode output FIFO, a built-in micropower
> +  temperature sensor, and an internal ADC for synchronous conversion
> +  of an additional analog input.
> +    https://www.analog.com/en/products/adxl367.html

"We can't find that page". I guess this driver is running slightly ahead
of the datasheet being made public.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,adxl367
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1

spi-max-frequency: true

to fix the issue Rob's bot reported.

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      adxl367@53 {
> +        compatible = "adi,adxl367";
> +        reg = <0x53>;
> +        interrupt-parent = <&gpio>;
> +        interrupts = <25 IRQ_TYPE_EDGE_RISING>;
> +      };
> +    };
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    spi {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      cs-gpios = <&gpio 17 GPIO_ACTIVE_LOW>;
> +      status = "okay";

We don't normally list status in example bindings.
Also, the cs-gpio is part of the spi master binding
so no need to have it here as we are showing how the
actual device binding works.

That should let you drop the gpio.h header.


> +
> +      adxl367@0 {
> +        compatible = "adi,adxl367";
> +        reg = <0>;
> +        spi-max-frequency = <1000000>;
> +        interrupt-parent = <&gpio>;
> +        interrupts = <25 IRQ_TYPE_EDGE_RISING>;
> +      };
> +    };