[PATCH v8 0/3] iio: accel: add support for ADXL355

[PATCH v8 0/3] iio: accel: add support for ADXL355

[Puranjay Mohan]

Add the dt-bindings and the driver for ADXL355 3-axis MEMS Accelerometer.

Changes since v7:
1. Update MAINTAINERS to show all driver files.
2. Set CONFIGS for buffered support in Kconfig.

Changes since v6:
1. Use interrupt-names property in device tree document.
2. Add triggered buffer support.
3. Use a static table for offset and data registers.
4. Fix coding style issues.
5. move defines from header to c file.

Changes since v5:
1. Used get_unaligned_be24() and  get_unaligned_be16() to parse
acceleration and temperature data. This solves sparse errors and also
make the code more understandable.

Changes since v4:
1. Fix errors reported by sparse.

Changes since v3:
1. Fix errors in yaml DT doc.
2. Change SPDX-License-Identifier to GPL-2.0-only OR BSD-2-Clause

Changes since v2:
1. Add separate DT binding doc in yaml.
2. Use ____cacheline_aligned buffer for regmap_bulk_read/write calls.
3. Make code consistent by using same style in switch case.
4. Use FIELD_PREP in place of custom macros.
5. Make Kconfig description more informative.

Changes since v1:
1. Remove the declarations for static regmap structures from adxl355.h.
This was missed in the v1 and caused errors.
2. Make switch case statements consistent by directly returning from
each case rather than saving the return in a variable.
3. Some coding style changes.

Changes since v0:
1. Move adxl355_hpf_3db_table to adxl355_data structure. This is done to make
sure that each device gets its own table.
2. Make local regmap definitions private to adxl355_core.c.
3. Other minor coding style changes.

Puranjay Mohan (3):
  dt-bindings: iio: accel: Add DT binding doc for ADXL355
  iio: accel: Add driver support for ADXL355
  iio: accel: adxl355: Add triggered buffer support

 .../bindings/iio/accel/adi,adxl355.yaml       |  88 +++
 MAINTAINERS                                   |  10 +
 drivers/iio/accel/Kconfig                     |  33 +
 drivers/iio/accel/Makefile                    |   3 +
 drivers/iio/accel/adxl355.h                   |  19 +
 drivers/iio/accel/adxl355_core.c              | 682 ++++++++++++++++++
 drivers/iio/accel/adxl355_i2c.c               |  65 ++
 drivers/iio/accel/adxl355_spi.c               |  67 ++
 8 files changed, 967 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/accel/adi,adxl355.yaml
 create mode 100644 drivers/iio/accel/adxl355.h
 create mode 100644 drivers/iio/accel/adxl355_core.c
 create mode 100644 drivers/iio/accel/adxl355_i2c.c
 create mode 100644 drivers/iio/accel/adxl355_spi.c

-- 
2.30.1

[PATCH v8 1/3] dt-bindings: iio: accel: Add DT binding doc for ADXL355

[Puranjay Mohan]

Add devicetree binding document for ADXL355, a 3-Axis MEMS Accelerometer.

Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
---
 .../bindings/iio/accel/adi,adxl355.yaml       | 88 +++++++++++++++++++
 1 file changed, 88 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/accel/adi,adxl355.yaml

diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adxl355.yaml b/Documentation/devicetree/bindings/iio/accel/adi,adxl355.yaml
new file mode 100644
index 000000000..5da3fd5ad
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/accel/adi,adxl355.yaml
@@ -0,0 +1,88 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/accel/adi,adxl355.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices ADXL355 3-Axis, Low noise MEMS Accelerometer
+
+maintainers:
+  - Puranjay Mohan <puranjay12@gmail.com>
+
+description: |
+  Analog Devices ADXL355 3-Axis, Low noise MEMS Accelerometer that supports
+  both I2C & SPI interfaces
+    https://www.analog.com/en/products/adxl355.html
+
+properties:
+  compatible:
+    enum:
+      - adi,adxl355
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    minItems: 1
+    maxItems: 3
+    description: |
+      Type should be IRQ_TYPE_LEVEL_HIGH.
+      Three configurable interrupt lines exist.
+
+  interrupt-names:
+    description: Specify which interrupt line is in use.
+    items:
+      enum:
+        - INT1
+        - INT2
+        - DRDY
+    minItems: 1
+    maxItems: 3
+
+  vdd-supply:
+    description: Regulator that provides power to the sensor
+
+  vddio-supply:
+    description: Regulator that provides power to the bus
+
+  spi-max-frequency: true
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+        #include <dt-bindings/gpio/gpio.h>
+        #include <dt-bindings/interrupt-controller/irq.h>
+        i2c {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                /* Example for a I2C device node */
+                accelerometer@1d {
+                        compatible = "adi,adxl355";
+                        reg = <0x1d>;
+                        interrupt-parent = <&gpio>;
+                        interrupts = <25 IRQ_TYPE_EDGE_RISING>;
+                        interrupt-names = "DRDY";
+                };
+        };
+  - |
+        #include <dt-bindings/gpio/gpio.h>
+        #include <dt-bindings/interrupt-controller/irq.h>
+        spi {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                accelerometer@0 {
+                        compatible = "adi,adxl355";
+                        reg = <0>;
+                        spi-max-frequency = <1000000>;
+                        interrupt-parent = <&gpio>;
+                        interrupts = <25 IRQ_TYPE_EDGE_RISING>;
+                        interrupt-names = "DRDY";
+                };
+        };
-- 
2.30.1

Re: [PATCH v8 1/3] dt-bindings: iio: accel: Add DT binding doc for ADXL355

[Jonathan Cameron]

On Wed,  4 Aug 2021 19:33:07 +0530
Puranjay Mohan <puranjay12@gmail.com> wrote:

> Add devicetree binding document for ADXL355, a 3-Axis MEMS Accelerometer.
> 
> Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>

Looks good to me. I'll pick this up once Rob and anyone else interested
has had time to take a look assuming they don't ask for any changes.

Thanks,

Jonathan


> ---
>  .../bindings/iio/accel/adi,adxl355.yaml       | 88 +++++++++++++++++++
>  1 file changed, 88 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/accel/adi,adxl355.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adxl355.yaml b/Documentation/devicetree/bindings/iio/accel/adi,adxl355.yaml
> new file mode 100644
> index 000000000..5da3fd5ad
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/accel/adi,adxl355.yaml
> @@ -0,0 +1,88 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/accel/adi,adxl355.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices ADXL355 3-Axis, Low noise MEMS Accelerometer
> +
> +maintainers:
> +  - Puranjay Mohan <puranjay12@gmail.com>
> +
> +description: |
> +  Analog Devices ADXL355 3-Axis, Low noise MEMS Accelerometer that supports
> +  both I2C & SPI interfaces
> +    https://www.analog.com/en/products/adxl355.html
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,adxl355
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    minItems: 1
> +    maxItems: 3
> +    description: |
> +      Type should be IRQ_TYPE_LEVEL_HIGH.
> +      Three configurable interrupt lines exist.
> +
> +  interrupt-names:
> +    description: Specify which interrupt line is in use.
> +    items:
> +      enum:
> +        - INT1
> +        - INT2
> +        - DRDY
> +    minItems: 1
> +    maxItems: 3
> +
> +  vdd-supply:
> +    description: Regulator that provides power to the sensor
> +
> +  vddio-supply:
> +    description: Regulator that provides power to the bus
> +
> +  spi-max-frequency: true
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +        #include <dt-bindings/gpio/gpio.h>
> +        #include <dt-bindings/interrupt-controller/irq.h>
> +        i2c {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                /* Example for a I2C device node */
> +                accelerometer@1d {
> +                        compatible = "adi,adxl355";
> +                        reg = <0x1d>;
> +                        interrupt-parent = <&gpio>;
> +                        interrupts = <25 IRQ_TYPE_EDGE_RISING>;
> +                        interrupt-names = "DRDY";
> +                };
> +        };
> +  - |
> +        #include <dt-bindings/gpio/gpio.h>
> +        #include <dt-bindings/interrupt-controller/irq.h>
> +        spi {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                accelerometer@0 {
> +                        compatible = "adi,adxl355";
> +                        reg = <0>;
> +                        spi-max-frequency = <1000000>;
> +                        interrupt-parent = <&gpio>;
> +                        interrupts = <25 IRQ_TYPE_EDGE_RISING>;
> +                        interrupt-names = "DRDY";
> +                };
> +        };

Re: [PATCH v8 1/3] dt-bindings: iio: accel: Add DT binding doc for ADXL355

[Jonathan Cameron]

On Sun, 8 Aug 2021 16:06:28 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Wed,  4 Aug 2021 19:33:07 +0530
> Puranjay Mohan <puranjay12@gmail.com> wrote:
> 
> > Add devicetree binding document for ADXL355, a 3-Axis MEMS Accelerometer.
> > 
> > Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>  
> 
> Looks good to me. I'll pick this up once Rob and anyone else interested
> has had time to take a look assuming they don't ask for any changes.
> 
> Thanks,
> 
> Jonathan
> 
> 
> > ---
> >  .../bindings/iio/accel/adi,adxl355.yaml       | 88 +++++++++++++++++++
> >  1 file changed, 88 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/iio/accel/adi,adxl355.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adxl355.yaml b/Documentation/devicetree/bindings/iio/accel/adi,adxl355.yaml
> > new file mode 100644
> > index 000000000..5da3fd5ad
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/accel/adi,adxl355.yaml
> > @@ -0,0 +1,88 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/accel/adi,adxl355.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices ADXL355 3-Axis, Low noise MEMS Accelerometer
> > +
> > +maintainers:
> > +  - Puranjay Mohan <puranjay12@gmail.com>
> > +
> > +description: |
> > +  Analog Devices ADXL355 3-Axis, Low noise MEMS Accelerometer that supports
> > +  both I2C & SPI interfaces
> > +    https://www.analog.com/en/products/adxl355.html
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - adi,adxl355
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    minItems: 1
> > +    maxItems: 3
> > +    description: |
> > +      Type should be IRQ_TYPE_LEVEL_HIGH.
> > +      Three configurable interrupt lines exist.

I just noticed from the datasheet, that INT1 and INT2 have controllable
polarity on the device.  We should look to support that rather than
stating all 3 interrupts are LEVEL_HIGH.  For now, I'd just not
state the type here, or make that statement only for the dataready pin.

Once you enable these interrupts, you will want to handle the polarity setting
in a similar fashion to the bmi160.

It is a bit messy to combine the interrupt polarity at the interrupt controller
with that at the device, but that is how we've handled these in the past.

Jonathan

> > +
> > +  interrupt-names:
> > +    description: Specify which interrupt line is in use.
> > +    items:
> > +      enum:
> > +        - INT1
> > +        - INT2
> > +        - DRDY
> > +    minItems: 1
> > +    maxItems: 3
> > +
> > +  vdd-supply:
> > +    description: Regulator that provides power to the sensor
> > +
> > +  vddio-supply:
> > +    description: Regulator that provides power to the bus
> > +
> > +  spi-max-frequency: true
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +        #include <dt-bindings/gpio/gpio.h>
> > +        #include <dt-bindings/interrupt-controller/irq.h>
> > +        i2c {
> > +                #address-cells = <1>;
> > +                #size-cells = <0>;
> > +
> > +                /* Example for a I2C device node */
> > +                accelerometer@1d {
> > +                        compatible = "adi,adxl355";
> > +                        reg = <0x1d>;
> > +                        interrupt-parent = <&gpio>;
> > +                        interrupts = <25 IRQ_TYPE_EDGE_RISING>;
> > +                        interrupt-names = "DRDY";
> > +                };
> > +        };
> > +  - |
> > +        #include <dt-bindings/gpio/gpio.h>
> > +        #include <dt-bindings/interrupt-controller/irq.h>
> > +        spi {
> > +                #address-cells = <1>;
> > +                #size-cells = <0>;
> > +
> > +                accelerometer@0 {
> > +                        compatible = "adi,adxl355";
> > +                        reg = <0>;
> > +                        spi-max-frequency = <1000000>;
> > +                        interrupt-parent = <&gpio>;
> > +                        interrupts = <25 IRQ_TYPE_EDGE_RISING>;
> > +                        interrupt-names = "DRDY";
> > +                };
> > +        };  
> 

Re: [PATCH v8 1/3] dt-bindings: iio: accel: Add DT binding doc for ADXL355

[Puranjay Mohan]

On Sun, Aug 8, 2021 at 9:10 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Sun, 8 Aug 2021 16:06:28 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
>
> > On Wed,  4 Aug 2021 19:33:07 +0530
> > Puranjay Mohan <puranjay12@gmail.com> wrote:
> >
> > > Add devicetree binding document for ADXL355, a 3-Axis MEMS Accelerometer.
> > >
> > > Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
> >
> > Looks good to me. I'll pick this up once Rob and anyone else interested
> > has had time to take a look assuming they don't ask for any changes.
> >
> > Thanks,
> >
> > Jonathan
> >
> >
> > > ---
> > >  .../bindings/iio/accel/adi,adxl355.yaml       | 88 +++++++++++++++++++
> > >  1 file changed, 88 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/iio/accel/adi,adxl355.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adxl355.yaml b/Documentation/devicetree/bindings/iio/accel/adi,adxl355.yaml
> > > new file mode 100644
> > > index 000000000..5da3fd5ad
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/accel/adi,adxl355.yaml
> > > @@ -0,0 +1,88 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/iio/accel/adi,adxl355.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Analog Devices ADXL355 3-Axis, Low noise MEMS Accelerometer
> > > +
> > > +maintainers:
> > > +  - Puranjay Mohan <puranjay12@gmail.com>
> > > +
> > > +description: |
> > > +  Analog Devices ADXL355 3-Axis, Low noise MEMS Accelerometer that supports
> > > +  both I2C & SPI interfaces
> > > +    https://www.analog.com/en/products/adxl355.html
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - adi,adxl355
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  interrupts:
> > > +    minItems: 1
> > > +    maxItems: 3
> > > +    description: |
> > > +      Type should be IRQ_TYPE_LEVEL_HIGH.
> > > +      Three configurable interrupt lines exist.
>
> I just noticed from the datasheet, that INT1 and INT2 have controllable
> polarity on the device.  We should look to support that rather than
> stating all 3 interrupts are LEVEL_HIGH.  For now, I'd just not
> state the type here, or make that statement only for the dataready pin.
>

Hi Jonathan,
I will be sending v9 of this series where I will state this only for
the DRDY pin. I will also make the scale defines inline for the other
patch.
Also, I will not send the third patch: trigger support in v9 because
it needs a lot of work.
It would be great if the base driver gets accepted and I will keep
working on the trigger support separately and not as a part of this
patch series.
I am also interested in looking into fwnode_get_irq_byname() for the
trigger patch.

> Once you enable these interrupts, you will want to handle the polarity setting
> in a similar fashion to the bmi160.
>
> It is a bit messy to combine the interrupt polarity at the interrupt controller
> with that at the device, but that is how we've handled these in the past.
>
> Jonathan
>
> > > +
> > > +  interrupt-names:
> > > +    description: Specify which interrupt line is in use.
> > > +    items:
> > > +      enum:
> > > +        - INT1
> > > +        - INT2
> > > +        - DRDY
> > > +    minItems: 1
> > > +    maxItems: 3
> > > +
> > > +  vdd-supply:
> > > +    description: Regulator that provides power to the sensor
> > > +
> > > +  vddio-supply:
> > > +    description: Regulator that provides power to the bus
> > > +
> > > +  spi-max-frequency: true
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +        #include <dt-bindings/gpio/gpio.h>
> > > +        #include <dt-bindings/interrupt-controller/irq.h>
> > > +        i2c {
> > > +                #address-cells = <1>;
> > > +                #size-cells = <0>;
> > > +
> > > +                /* Example for a I2C device node */
> > > +                accelerometer@1d {
> > > +                        compatible = "adi,adxl355";
> > > +                        reg = <0x1d>;
> > > +                        interrupt-parent = <&gpio>;
> > > +                        interrupts = <25 IRQ_TYPE_EDGE_RISING>;
> > > +                        interrupt-names = "DRDY";
> > > +                };
> > > +        };
> > > +  - |
> > > +        #include <dt-bindings/gpio/gpio.h>
> > > +        #include <dt-bindings/interrupt-controller/irq.h>
> > > +        spi {
> > > +                #address-cells = <1>;
> > > +                #size-cells = <0>;
> > > +
> > > +                accelerometer@0 {
> > > +                        compatible = "adi,adxl355";
> > > +                        reg = <0>;
> > > +                        spi-max-frequency = <1000000>;
> > > +                        interrupt-parent = <&gpio>;
> > > +                        interrupts = <25 IRQ_TYPE_EDGE_RISING>;
> > > +                        interrupt-names = "DRDY";
> > > +                };
> > > +        };
> >
>


-- 
Thanks and Regards

Yours Truly,

Puranjay Mohan

[PATCH v8 2/3] iio: accel: Add driver support for ADXL355

[Puranjay Mohan]

ADXL355 is 3-axis MEMS Accelerometer. It offers low noise density,
low 0g offset drift, low power with selectable measurement ranges.
It also features programmable high-pass and low-pass filters.

Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adxl354_adxl355.pdf
Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
---
 MAINTAINERS                      |  10 +
 drivers/iio/accel/Kconfig        |  29 ++
 drivers/iio/accel/Makefile       |   3 +
 drivers/iio/accel/adxl355.h      |  19 +
 drivers/iio/accel/adxl355_core.c | 584 +++++++++++++++++++++++++++++++
 drivers/iio/accel/adxl355_i2c.c  |  64 ++++
 drivers/iio/accel/adxl355_spi.c  |  67 ++++
 7 files changed, 776 insertions(+)
 create mode 100644 drivers/iio/accel/adxl355.h
 create mode 100644 drivers/iio/accel/adxl355_core.c
 create mode 100644 drivers/iio/accel/adxl355_i2c.c
 create mode 100644 drivers/iio/accel/adxl355_spi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index bd7aff0c1..704e70b86 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -586,6 +586,16 @@ W:	http://ez.analog.com/community/linux-device-drivers
 F:	Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml
 F:	drivers/input/misc/adxl34x.c
 
+ADXL355 THREE-AXIS DIGITAL ACCELEROMETER DRIVER
+M:	Puranjay Mohan <puranjay12@gmail.com>
+L:	linux-iio@vger.kernel.org
+S:	Supported
+F:	drivers/iio/accel/adxl355.h
+F:	drivers/iio/accel/adxl355_core.c
+F:	drivers/iio/accel/adxl355_i2c.c
+F:	drivers/iio/accel/adxl355_spi.c
+F:	Documentation/devicetree/bindings/iio/accel/adi,adxl355.yaml
+
 ADXL372 THREE-AXIS DIGITAL ACCELEROMETER DRIVER
 M:	Michael Hennerich <michael.hennerich@analog.com>
 S:	Supported
diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index cceda3cec..d0c45c809 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -61,6 +61,35 @@ config ADXL345_SPI
 	  will be called adxl345_spi and you will also get adxl345_core
 	  for the core module.
 
+config ADXL355
+	tristate
+
+config ADXL355_I2C
+	tristate "Analog Devices ADXL355 3-Axis Digital Accelerometer I2C Driver"
+	depends on I2C
+	select ADXL355
+	select REGMAP_I2C
+	help
+	  Say Y here if you want to build i2c support for the Analog Devices
+	  ADXL355 3-axis digital accelerometer.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called adxl355_i2c and you will also get adxl355_core
+	  for the core module.
+
+config ADXL355_SPI
+	tristate "Analog Devices ADXL355 3-Axis Digital Accelerometer SPI Driver"
+	depends on SPI
+	select ADXL355
+	select REGMAP_SPI
+	help
+	  Say Y here if you want to build spi support for the Analog Devices
+	  ADXL355 3-axis digital accelerometer.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called adxl355_spi and you will also get adxl355_core
+	  for the core module.
+
 config ADXL372
 	tristate
 	select IIO_BUFFER
diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
index 32cd1342a..0e4721d2d 100644
--- a/drivers/iio/accel/Makefile
+++ b/drivers/iio/accel/Makefile
@@ -9,6 +9,9 @@ obj-$(CONFIG_ADIS16209) += adis16209.o
 obj-$(CONFIG_ADXL345) += adxl345_core.o
 obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o
 obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o
+obj-$(CONFIG_ADXL355) += adxl355_core.o
+obj-$(CONFIG_ADXL355_I2C) += adxl355_i2c.o
+obj-$(CONFIG_ADXL355_SPI) += adxl355_spi.o
 obj-$(CONFIG_ADXL372) += adxl372.o
 obj-$(CONFIG_ADXL372_I2C) += adxl372_i2c.o
 obj-$(CONFIG_ADXL372_SPI) += adxl372_spi.o
diff --git a/drivers/iio/accel/adxl355.h b/drivers/iio/accel/adxl355.h
new file mode 100644
index 000000000..322b0abb8
--- /dev/null
+++ b/drivers/iio/accel/adxl355.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * ADXL355 3-Axis Digital Accelerometer
+ *
+ * Copyright (c) 2021 Puranjay Mohan <puranjay12@gmail.com>
+ */
+
+#ifndef _ADXL355_H_
+#define _ADXL355_H_
+
+#include <linux/regmap.h>
+
+extern const struct regmap_access_table adxl355_readable_regs_tbl;
+
+extern const struct regmap_access_table adxl355_writeable_regs_tbl;
+
+int adxl355_core_probe(struct device *dev, struct regmap *regmap,
+		       const char *name);
+#endif /* _ADXL355_H_ */
diff --git a/drivers/iio/accel/adxl355_core.c b/drivers/iio/accel/adxl355_core.c
new file mode 100644
index 000000000..47fbb31bc
--- /dev/null
+++ b/drivers/iio/accel/adxl355_core.c
@@ -0,0 +1,584 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ADXL355 3-Axis Digital Accelerometer IIO core driver
+ *
+ * Copyright (c) 2021 Puranjay Mohan <puranjay12@gmail.com>
+ *
+ * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adxl354_adxl355.pdf
+ */
+
+#include <asm/unaligned.h>
+#include <linux/bitfield.h>
+#include <linux/iio/iio.h>
+#include <linux/limits.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/regmap.h>
+
+#include "adxl355.h"
+
+/* ADXL355 Register Definitions */
+#define ADXL355_DEVID_AD_REG		0x00
+#define ADXL355_DEVID_MST_REG		0x01
+#define ADXL355_PARTID_REG		0x02
+#define ADXL355_STATUS_REG		0x04
+#define ADXL355_FIFO_ENTRIES_REG	0x05
+#define ADXL355_TEMP2_REG		0x06
+#define ADXL355_XDATA3_REG		0x08
+#define ADXL355_YDATA3_REG		0x0B
+#define ADXL355_ZDATA3_REG		0x0E
+#define ADXL355_FIFO_DATA_REG		0x11
+#define ADXL355_OFFSET_X_H_REG		0x1E
+#define ADXL355_OFFSET_Y_H_REG		0x20
+#define ADXL355_OFFSET_Z_H_REG		0x22
+#define ADXL355_ACT_EN_REG		0x24
+#define ADXL355_ACT_THRESH_H_REG	0x25
+#define ADXL355_ACT_THRESH_L_REG	0x26
+#define ADXL355_ACT_COUNT_REG		0x27
+#define ADXL355_FILTER_REG		0x28
+#define  ADXL355_FILTER_ODR_MSK GENMASK(3, 0)
+#define  ADXL355_FILTER_HPF_MSK	GENMASK(6, 4)
+#define ADXL355_FIFO_SAMPLES_REG	0x29
+#define ADXL355_INT_MAP_REG		0x2A
+#define ADXL355_SYNC_REG		0x2B
+#define ADXL355_RANGE_REG		0x2C
+#define ADXL355_POWER_CTL_REG		0x2D
+#define	 ADXL355_POWER_CTL_MODE_MSK	GENMASK(1, 0)
+#define ADXL355_SELF_TEST_REG		0x2E
+#define ADXL355_RESET_REG		0x2F
+
+#define ADXL355_DEVID_AD_VAL		0xAD
+#define ADXL355_DEVID_MST_VAL		0x1D
+#define ADXL355_PARTID_VAL		0xED
+#define ADXL355_RESET_CODE		0x52
+
+/*
+ * The datasheet defines an intercept of 1885 LSB at 25 degC
+ * and a slope of -9.05 LSB/C. The following formula can be used to find the
+ * temperature:
+ * Temp = ((RAW - 1885)/(-9.05)) + 25 but this doesn't follow the format of
+ * the IIO which is Temp = (RAW + OFFSET) * SCALE. Hence using some rearranging
+ * we get the scale as -110.49723 and offset as -2111.25
+ */
+#define TEMP_SCALE_VAL -110
+#define TEMP_SCALE_VAL2 497238
+#define TEMP_OFFSET_VAL -2111
+#define TEMP_OFFSET_VAL2 250000
+
+/*
+ * At +/- 2g with 20-bit resolution, scale is given in datasheet as
+ * 3.9ug/LSB = 0.0000039 * 9.80665 = 0.00003824593 m/s^2
+ */
+#define ADXL355_NSCALE	38245
+
+static const struct regmap_range adxl355_read_reg_range[] = {
+	regmap_reg_range(ADXL355_DEVID_AD_REG, ADXL355_FIFO_DATA_REG),
+	regmap_reg_range(ADXL355_OFFSET_X_H_REG, ADXL355_SELF_TEST_REG)
+};
+
+const struct regmap_access_table adxl355_readable_regs_tbl = {
+	.yes_ranges = adxl355_read_reg_range,
+	.n_yes_ranges = ARRAY_SIZE(adxl355_read_reg_range),
+};
+EXPORT_SYMBOL_GPL(adxl355_readable_regs_tbl);
+
+static const struct regmap_range adxl355_write_reg_range[] = {
+	regmap_reg_range(ADXL355_OFFSET_X_H_REG, ADXL355_RESET_REG)
+};
+
+const struct regmap_access_table adxl355_writeable_regs_tbl = {
+	.yes_ranges = adxl355_write_reg_range,
+	.n_yes_ranges = ARRAY_SIZE(adxl355_write_reg_range),
+};
+EXPORT_SYMBOL_GPL(adxl355_writeable_regs_tbl);
+
+enum adxl355_op_mode {
+	ADXL355_MEASUREMENT,
+	ADXL355_STANDBY,
+	ADXL355_TEMP_OFF
+};
+
+enum adxl355_odr {
+	ADXL355_ODR_4000HZ,
+	ADXL355_ODR_2000HZ,
+	ADXL355_ODR_1000HZ,
+	ADXL355_ODR_500HZ,
+	ADXL355_ODR_250HZ,
+	ADXL355_ODR_125HZ,
+	ADXL355_ODR_62_5HZ,
+	ADXL355_ODR_31_25HZ,
+	ADXL355_ODR_15_625HZ,
+	ADXL355_ODR_7_813HZ,
+	ADXL355_ODR_3_906HZ
+};
+
+enum adxl355_hpf_3db {
+	ADXL355_HPF_OFF,
+	ADXL355_HPF_24_7,
+	ADXL355_HPF_6_2084,
+	ADXL355_HPF_1_5545,
+	ADXL355_HPF_0_3862,
+	ADXL355_HPF_0_0954,
+	ADXL355_HPF_0_0238
+};
+
+static const int adxl355_odr_table[][2] = {
+	[0] = {4000, 0},
+	[1] = {2000, 0},
+	[2] = {1000, 0},
+	[3] = {500, 0},
+	[4] = {250, 0},
+	[5] = {125, 0},
+	[6] = {62, 500000},
+	[7] = {31, 250000},
+	[8] = {15, 625000},
+	[9] = {7, 813000},
+	[10] = {3, 906000}
+};
+
+static const int adxl355_hpf_3db_multipliers[] = {
+	0,
+	247000,
+	62084,
+	15545,
+	3862,
+	954,
+	238
+};
+
+enum adxl355_chans {
+	chan_x, chan_y, chan_z
+};
+
+struct adxl355_chan_info {
+	u8 data_reg;
+	u8 offset_reg;
+};
+
+static const struct adxl355_chan_info adxl355_chans[] = {
+	[chan_x] = {
+		.data_reg = ADXL355_XDATA3_REG,
+		.offset_reg = ADXL355_OFFSET_X_H_REG
+	},
+	[chan_y] = {
+		.data_reg = ADXL355_YDATA3_REG,
+		.offset_reg = ADXL355_OFFSET_Y_H_REG
+	},
+	[chan_z] = {
+		.data_reg = ADXL355_ZDATA3_REG,
+		.offset_reg = ADXL355_OFFSET_Z_H_REG
+	}
+};
+
+struct adxl355_data {
+	struct regmap *regmap;
+	struct device *dev;
+	struct mutex lock; /* lock to protect op_mode */
+	enum adxl355_op_mode op_mode;
+	enum adxl355_odr odr;
+	enum adxl355_hpf_3db hpf_3db;
+	int calibbias[3];
+	int adxl355_hpf_3db_table[7][2];
+	u8 transf_buf[3] ____cacheline_aligned;
+};
+
+static int adxl355_set_op_mode(struct adxl355_data *data,
+			       enum adxl355_op_mode op_mode)
+{
+	int ret;
+
+	if (data->op_mode == op_mode)
+		return 0;
+
+	ret = regmap_update_bits(data->regmap, ADXL355_POWER_CTL_REG,
+				 ADXL355_POWER_CTL_MODE_MSK, op_mode);
+	if (ret)
+		return ret;
+
+	data->op_mode = op_mode;
+
+	return ret;
+}
+
+static void adxl355_fill_3db_frequency_table(struct adxl355_data *data)
+{
+	int i;
+	u64 rem;
+	u64 div;
+	u32 multiplier;
+	u64 odr = mul_u64_u32_shr(adxl355_odr_table[data->odr][0], 1000000, 0) +
+				  adxl355_odr_table[data->odr][1];
+
+	for (i = 0; i < ARRAY_SIZE(adxl355_hpf_3db_multipliers); i++) {
+		multiplier = adxl355_hpf_3db_multipliers[i];
+		div = div64_u64_rem(mul_u64_u32_shr(odr, multiplier, 0),
+				    100000000000000UL, &rem);
+
+		data->adxl355_hpf_3db_table[i][0] = div;
+		data->adxl355_hpf_3db_table[i][1] = div_u64(rem, 100000000);
+	}
+}
+
+static int adxl355_setup(struct adxl355_data *data)
+{
+	unsigned int regval;
+	int ret;
+
+	ret = regmap_read(data->regmap, ADXL355_DEVID_AD_REG, &regval);
+	if (ret)
+		return ret;
+
+	if (regval != ADXL355_DEVID_AD_VAL) {
+		dev_err(data->dev, "Invalid ADI ID 0x%02x\n", regval);
+		return -ENODEV;
+	}
+
+	ret = regmap_read(data->regmap, ADXL355_DEVID_MST_REG, &regval);
+	if (ret)
+		return ret;
+
+	if (regval != ADXL355_DEVID_MST_VAL) {
+		dev_err(data->dev, "Invalid MEMS ID 0x%02x\n", regval);
+		return -ENODEV;
+	}
+
+	ret = regmap_read(data->regmap, ADXL355_PARTID_REG, &regval);
+	if (ret)
+		return ret;
+
+	if (regval != ADXL355_PARTID_VAL) {
+		dev_err(data->dev, "Invalid DEV ID 0x%02x\n", regval);
+		return -ENODEV;
+	}
+
+	/*
+	 * Perform a software reset to make sure the device is in a consistent
+	 * state after start up.
+	 */
+	ret = regmap_write(data->regmap, ADXL355_RESET_REG, ADXL355_RESET_CODE);
+	if (ret)
+		return ret;
+
+	adxl355_fill_3db_frequency_table(data);
+
+	return adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
+}
+
+static int adxl355_get_temp_data(struct adxl355_data *data, u8 addr)
+{
+	return regmap_bulk_read(data->regmap, addr, data->transf_buf, 2);
+}
+
+static int adxl355_read_axis(struct adxl355_data *data, u8 addr)
+{
+	int ret;
+
+	ret = regmap_bulk_read(data->regmap, addr, data->transf_buf, 3);
+	if (ret < 0)
+		return ret;
+
+	return get_unaligned_be24(data->transf_buf);
+}
+
+static int adxl355_find_match(const int (*freq_tbl)[2], const int n,
+			      const int val, const int val2)
+{
+	int i;
+
+	for (i = 0; i < n; i++) {
+		if (freq_tbl[i][0] == val && freq_tbl[i][1] == val2)
+			return i;
+	}
+
+	return -EINVAL;
+}
+
+static int adxl355_set_odr(struct adxl355_data *data,
+			   enum adxl355_odr odr)
+{
+	int ret = 0;
+
+	mutex_lock(&data->lock);
+
+	if (data->odr == odr)
+		goto out_unlock;
+
+	ret = adxl355_set_op_mode(data, ADXL355_STANDBY);
+	if (ret < 0)
+		goto out_unlock;
+
+	ret = regmap_update_bits(data->regmap, ADXL355_FILTER_REG,
+				 ADXL355_FILTER_ODR_MSK,
+				 FIELD_PREP(ADXL355_FILTER_ODR_MSK, odr));
+	if (ret < 0)
+		goto out_unlock;
+
+	data->odr = odr;
+	adxl355_fill_3db_frequency_table(data);
+
+out_unlock:
+	ret = adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
+	mutex_unlock(&data->lock);
+	return ret;
+}
+
+static int adxl355_set_hpf_3db(struct adxl355_data *data,
+			       enum adxl355_hpf_3db hpf)
+{
+	int ret = 0;
+
+	mutex_lock(&data->lock);
+
+	if (data->hpf_3db == hpf)
+		goto unlock;
+
+	ret = adxl355_set_op_mode(data, ADXL355_STANDBY);
+	if (ret < 0)
+		goto out_unlock;
+
+	ret = regmap_update_bits(data->regmap, ADXL355_FILTER_REG,
+				 ADXL355_FILTER_HPF_MSK,
+				 FIELD_PREP(ADXL355_FILTER_HPF_MSK, hpf));
+	if (!ret)
+		data->hpf_3db = hpf;
+
+out_unlock:
+	ret = adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
+unlock:
+	mutex_unlock(&data->lock);
+	return ret;
+}
+
+static int adxl355_set_calibbias(struct adxl355_data *data,
+				 enum adxl355_chans chan, int calibbias)
+{
+	int ret = 0;
+
+	put_unaligned_be16(calibbias, data->transf_buf);
+
+	mutex_lock(&data->lock);
+
+	ret = adxl355_set_op_mode(data, ADXL355_STANDBY);
+	if (ret < 0)
+		goto out_unlock;
+
+	ret = regmap_bulk_write(data->regmap,
+				adxl355_chans[chan].offset_reg,
+				data->transf_buf, 2);
+	if (ret)
+		goto out_unlock;
+
+	data->calibbias[chan] = calibbias;
+
+out_unlock:
+	ret = adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
+	mutex_unlock(&data->lock);
+	return ret;
+}
+
+static int adxl355_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct adxl355_data *data = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		switch (chan->type) {
+		case IIO_TEMP:
+			ret = adxl355_get_temp_data(data, chan->address);
+			if (ret < 0)
+				return ret;
+			*val = get_unaligned_be16(data->transf_buf);
+
+			return IIO_VAL_INT;
+		case IIO_ACCEL:
+			ret = adxl355_read_axis(data, adxl355_chans[
+						chan->address].data_reg);
+			if (ret < 0)
+				return ret;
+			*val = sign_extend32(ret >> chan->scan_type.shift,
+					     chan->scan_type.realbits - 1);
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->type) {
+		case IIO_TEMP:
+			*val = TEMP_SCALE_VAL;
+			*val2 = TEMP_SCALE_VAL2;
+			return IIO_VAL_INT_PLUS_MICRO;
+		case IIO_ACCEL:
+			*val = 0;
+			*val2 = ADXL355_NSCALE;
+			return IIO_VAL_INT_PLUS_NANO;
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_OFFSET:
+		*val = TEMP_OFFSET_VAL;
+		*val2 = TEMP_OFFSET_VAL2;
+		return IIO_VAL_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_CALIBBIAS:
+		*val = sign_extend32(data->calibbias[chan->address], 15);
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val = adxl355_odr_table[data->odr][0];
+		*val2 = adxl355_odr_table[data->odr][1];
+		return IIO_VAL_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY:
+		*val = data->adxl355_hpf_3db_table[data->hpf_3db][0];
+		*val2 = data->adxl355_hpf_3db_table[data->hpf_3db][1];
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int adxl355_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int val, int val2, long mask)
+{
+	struct adxl355_data *data = iio_priv(indio_dev);
+	int odr_idx, hpf_idx, calibbias;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		odr_idx = adxl355_find_match(adxl355_odr_table,
+					     ARRAY_SIZE(adxl355_odr_table),
+					     val, val2);
+		if (odr_idx < 0)
+			return odr_idx;
+
+		return adxl355_set_odr(data, odr_idx);
+	case IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY:
+		hpf_idx = adxl355_find_match(data->adxl355_hpf_3db_table,
+					ARRAY_SIZE(data->adxl355_hpf_3db_table),
+					     val, val2);
+		if (hpf_idx < 0)
+			return hpf_idx;
+
+		return adxl355_set_hpf_3db(data, hpf_idx);
+	case IIO_CHAN_INFO_CALIBBIAS:
+		calibbias = clamp_t(int, val, S16_MIN, S16_MAX);
+
+		return adxl355_set_calibbias(data, chan->address, calibbias);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int adxl355_read_avail(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      const int **vals, int *type, int *length,
+			      long mask)
+{
+	struct adxl355_data *data = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*vals = (const int *)adxl355_odr_table;
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		/* Values are stored in a 2D matrix */
+		*length = ARRAY_SIZE(adxl355_odr_table) * 2;
+
+		return IIO_AVAIL_LIST;
+	case IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY:
+		*vals = (const int *)data->adxl355_hpf_3db_table;
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		/* Values are stored in a 2D matrix */
+		*length = ARRAY_SIZE(data->adxl355_hpf_3db_table) * 2;
+
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info adxl355_info = {
+	.read_raw	= adxl355_read_raw,
+	.write_raw	= adxl355_write_raw,
+	.read_avail	= &adxl355_read_avail
+};
+
+#define ADXL355_ACCEL_CHANNEL(index, reg, axis) {			\
+	.type = IIO_ACCEL,						\
+	.address = reg,							\
+	.modified = 1,							\
+	.channel2 = IIO_MOD_##axis,					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
+			      BIT(IIO_CHAN_INFO_CALIBBIAS),		\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |		\
+				    BIT(IIO_CHAN_INFO_SAMP_FREQ) |	\
+		BIT(IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY),	\
+	.info_mask_shared_by_type_available =				\
+		BIT(IIO_CHAN_INFO_SAMP_FREQ) |				\
+		BIT(IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY),	\
+	.scan_type = {							\
+		.sign = 's',						\
+		.realbits = 20,						\
+		.storagebits = 32,					\
+		.shift = 4,						\
+		.endianness = IIO_BE,					\
+	}								\
+}
+
+static const struct iio_chan_spec adxl355_channels[] = {
+	ADXL355_ACCEL_CHANNEL(0, chan_x, X),
+	ADXL355_ACCEL_CHANNEL(1, chan_y, Y),
+	ADXL355_ACCEL_CHANNEL(2, chan_z, Z),
+	{
+		.type = IIO_TEMP,
+		.address = ADXL355_TEMP2_REG,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE) |
+				      BIT(IIO_CHAN_INFO_OFFSET),
+		.scan_type = {
+			.sign = 's',
+			.realbits = 12,
+			.storagebits = 16,
+			.endianness = IIO_BE,
+		},
+	}
+};
+
+int adxl355_core_probe(struct device *dev, struct regmap *regmap,
+		       const char *name)
+{
+	struct adxl355_data *data;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	data->regmap = regmap;
+	data->dev = dev;
+	data->op_mode = ADXL355_STANDBY;
+	mutex_init(&data->lock);
+
+	indio_dev->name = name;
+	indio_dev->info = &adxl355_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = adxl355_channels;
+	indio_dev->num_channels = ARRAY_SIZE(adxl355_channels);
+
+	ret = adxl355_setup(data);
+	if (ret < 0) {
+		dev_err(dev, "ADXL355 setup failed\n");
+		return ret;
+	}
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+EXPORT_SYMBOL_GPL(adxl355_core_probe);
+
+MODULE_AUTHOR("Puranjay Mohan <puranjay12@gmail.com>");
+MODULE_DESCRIPTION("ADXL355 3-Axis Digital Accelerometer core driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/accel/adxl355_i2c.c b/drivers/iio/accel/adxl355_i2c.c
new file mode 100644
index 000000000..e3070ee81
--- /dev/null
+++ b/drivers/iio/accel/adxl355_i2c.c
@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ADXL355 3-Axis Digital Accelerometer I2C driver
+ *
+ * Copyright (c) 2021 Puranjay Mohan <puranjay12@gmail.com>
+ */
+
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/regmap.h>
+
+#include "adxl355.h"
+
+static const struct regmap_config adxl355_i2c_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0x2F,
+	.rd_table = &adxl355_readable_regs_tbl,
+	.wr_table = &adxl355_writeable_regs_tbl
+};
+
+static int adxl355_i2c_probe(struct i2c_client *client)
+{
+	struct regmap *regmap;
+
+	regmap = devm_regmap_init_i2c(client, &adxl355_i2c_regmap_config);
+	if (IS_ERR(regmap)) {
+		dev_err(&client->dev, "Error initializing i2c regmap: %ld\n",
+			PTR_ERR(regmap));
+		return PTR_ERR(regmap);
+	}
+
+	return adxl355_core_probe(&client->dev, regmap, client->name);
+}
+
+static const struct i2c_device_id adxl355_i2c_id[] = {
+	{ "adxl355", 0 },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(i2c, adxl355_i2c_id);
+
+static const struct of_device_id adxl355_of_match[] = {
+	{ .compatible = "adi,adxl355" },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, adxl355_of_match);
+
+static struct i2c_driver adxl355_i2c_driver = {
+	.driver = {
+		.name	= "adxl355_i2c",
+		.of_match_table = adxl355_of_match,
+	},
+	.probe_new	= adxl355_i2c_probe,
+	.id_table	= adxl355_i2c_id,
+};
+
+module_i2c_driver(adxl355_i2c_driver);
+
+MODULE_AUTHOR("Puranjay Mohan <puranjay12@gmail.com>");
+MODULE_DESCRIPTION("ADXL355 3-Axis Digital Accelerometer I2C driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/accel/adxl355_spi.c b/drivers/iio/accel/adxl355_spi.c
new file mode 100644
index 000000000..a16bd1407
--- /dev/null
+++ b/drivers/iio/accel/adxl355_spi.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ADXL355 3-Axis Digital Accelerometer SPI driver
+ *
+ * Copyright (c) 2021 Puranjay Mohan <puranjay12@gmail.com>
+ */
+
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+
+#include "adxl355.h"
+
+static const struct regmap_config adxl355_spi_regmap_config = {
+	.reg_bits = 7,
+	.pad_bits = 1,
+	.val_bits = 8,
+	.read_flag_mask = BIT(0),
+	.max_register = 0x2F,
+	.rd_table = &adxl355_readable_regs_tbl,
+	.wr_table = &adxl355_writeable_regs_tbl
+};
+
+static int adxl355_spi_probe(struct spi_device *spi)
+{
+	const struct spi_device_id *id = spi_get_device_id(spi);
+	struct regmap *regmap;
+
+	regmap = devm_regmap_init_spi(spi, &adxl355_spi_regmap_config);
+	if (IS_ERR(regmap)) {
+		dev_err(&spi->dev, "Error initializing spi regmap: %ld\n",
+			PTR_ERR(regmap));
+		return PTR_ERR(regmap);
+	}
+
+	return adxl355_core_probe(&spi->dev, regmap, id->name);
+}
+
+static const struct spi_device_id adxl355_spi_id[] = {
+	{ "adxl355", 0 },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(spi, adxl355_spi_id);
+
+static const struct of_device_id adxl355_of_match[] = {
+	{ .compatible = "adi,adxl355" },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, adxl355_of_match);
+
+static struct spi_driver adxl355_spi_driver = {
+	.driver = {
+		.name	= "adxl355_spi",
+		.of_match_table = adxl355_of_match,
+	},
+	.probe		= adxl355_spi_probe,
+	.id_table	= adxl355_spi_id,
+};
+
+module_spi_driver(adxl355_spi_driver);
+
+MODULE_AUTHOR("Puranjay Mohan <puranjay12@gmail.com>");
+MODULE_DESCRIPTION("ADXL355 3-Axis Digital Accelerometer SPI driver");
+MODULE_LICENSE("GPL v2");
-- 
2.30.1

Re: [PATCH v8 2/3] iio: accel: Add driver support for ADXL355

[Alexandru Ardelean]

On Wed, Aug 4, 2021 at 5:05 PM Puranjay Mohan <puranjay12@gmail.com> wrote:
>
> ADXL355 is 3-axis MEMS Accelerometer. It offers low noise density,
> low 0g offset drift, low power with selectable measurement ranges.
> It also features programmable high-pass and low-pass filters.
>
> Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adxl354_adxl355.pdf

Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com>

> Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
> ---
>  MAINTAINERS                      |  10 +
>  drivers/iio/accel/Kconfig        |  29 ++
>  drivers/iio/accel/Makefile       |   3 +
>  drivers/iio/accel/adxl355.h      |  19 +
>  drivers/iio/accel/adxl355_core.c | 584 +++++++++++++++++++++++++++++++
>  drivers/iio/accel/adxl355_i2c.c  |  64 ++++
>  drivers/iio/accel/adxl355_spi.c  |  67 ++++
>  7 files changed, 776 insertions(+)
>  create mode 100644 drivers/iio/accel/adxl355.h
>  create mode 100644 drivers/iio/accel/adxl355_core.c
>  create mode 100644 drivers/iio/accel/adxl355_i2c.c
>  create mode 100644 drivers/iio/accel/adxl355_spi.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bd7aff0c1..704e70b86 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -586,6 +586,16 @@ W: http://ez.analog.com/community/linux-device-drivers
>  F:     Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml
>  F:     drivers/input/misc/adxl34x.c
>
> +ADXL355 THREE-AXIS DIGITAL ACCELEROMETER DRIVER
> +M:     Puranjay Mohan <puranjay12@gmail.com>
> +L:     linux-iio@vger.kernel.org
> +S:     Supported
> +F:     drivers/iio/accel/adxl355.h
> +F:     drivers/iio/accel/adxl355_core.c
> +F:     drivers/iio/accel/adxl355_i2c.c
> +F:     drivers/iio/accel/adxl355_spi.c
> +F:     Documentation/devicetree/bindings/iio/accel/adi,adxl355.yaml
> +
>  ADXL372 THREE-AXIS DIGITAL ACCELEROMETER DRIVER
>  M:     Michael Hennerich <michael.hennerich@analog.com>
>  S:     Supported
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index cceda3cec..d0c45c809 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -61,6 +61,35 @@ config ADXL345_SPI
>           will be called adxl345_spi and you will also get adxl345_core
>           for the core module.
>
> +config ADXL355
> +       tristate
> +
> +config ADXL355_I2C
> +       tristate "Analog Devices ADXL355 3-Axis Digital Accelerometer I2C Driver"
> +       depends on I2C
> +       select ADXL355
> +       select REGMAP_I2C
> +       help
> +         Say Y here if you want to build i2c support for the Analog Devices
> +         ADXL355 3-axis digital accelerometer.
> +
> +         To compile this driver as a module, choose M here: the module
> +         will be called adxl355_i2c and you will also get adxl355_core
> +         for the core module.
> +
> +config ADXL355_SPI
> +       tristate "Analog Devices ADXL355 3-Axis Digital Accelerometer SPI Driver"
> +       depends on SPI
> +       select ADXL355
> +       select REGMAP_SPI
> +       help
> +         Say Y here if you want to build spi support for the Analog Devices
> +         ADXL355 3-axis digital accelerometer.
> +
> +         To compile this driver as a module, choose M here: the module
> +         will be called adxl355_spi and you will also get adxl355_core
> +         for the core module.
> +
>  config ADXL372
>         tristate
>         select IIO_BUFFER
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index 32cd1342a..0e4721d2d 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -9,6 +9,9 @@ obj-$(CONFIG_ADIS16209) += adis16209.o
>  obj-$(CONFIG_ADXL345) += adxl345_core.o
>  obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o
>  obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o
> +obj-$(CONFIG_ADXL355) += adxl355_core.o
> +obj-$(CONFIG_ADXL355_I2C) += adxl355_i2c.o
> +obj-$(CONFIG_ADXL355_SPI) += adxl355_spi.o
>  obj-$(CONFIG_ADXL372) += adxl372.o
>  obj-$(CONFIG_ADXL372_I2C) += adxl372_i2c.o
>  obj-$(CONFIG_ADXL372_SPI) += adxl372_spi.o
> diff --git a/drivers/iio/accel/adxl355.h b/drivers/iio/accel/adxl355.h
> new file mode 100644
> index 000000000..322b0abb8
> --- /dev/null
> +++ b/drivers/iio/accel/adxl355.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * ADXL355 3-Axis Digital Accelerometer
> + *
> + * Copyright (c) 2021 Puranjay Mohan <puranjay12@gmail.com>
> + */
> +
> +#ifndef _ADXL355_H_
> +#define _ADXL355_H_
> +
> +#include <linux/regmap.h>
> +
> +extern const struct regmap_access_table adxl355_readable_regs_tbl;
> +
> +extern const struct regmap_access_table adxl355_writeable_regs_tbl;
> +
> +int adxl355_core_probe(struct device *dev, struct regmap *regmap,
> +                      const char *name);
> +#endif /* _ADXL355_H_ */
> diff --git a/drivers/iio/accel/adxl355_core.c b/drivers/iio/accel/adxl355_core.c
> new file mode 100644
> index 000000000..47fbb31bc
> --- /dev/null
> +++ b/drivers/iio/accel/adxl355_core.c
> @@ -0,0 +1,584 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * ADXL355 3-Axis Digital Accelerometer IIO core driver
> + *
> + * Copyright (c) 2021 Puranjay Mohan <puranjay12@gmail.com>
> + *
> + * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adxl354_adxl355.pdf
> + */
> +
> +#include <asm/unaligned.h>
> +#include <linux/bitfield.h>
> +#include <linux/iio/iio.h>
> +#include <linux/limits.h>
> +#include <linux/math64.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/regmap.h>
> +
> +#include "adxl355.h"
> +
> +/* ADXL355 Register Definitions */
> +#define ADXL355_DEVID_AD_REG           0x00
> +#define ADXL355_DEVID_MST_REG          0x01
> +#define ADXL355_PARTID_REG             0x02
> +#define ADXL355_STATUS_REG             0x04
> +#define ADXL355_FIFO_ENTRIES_REG       0x05
> +#define ADXL355_TEMP2_REG              0x06
> +#define ADXL355_XDATA3_REG             0x08
> +#define ADXL355_YDATA3_REG             0x0B
> +#define ADXL355_ZDATA3_REG             0x0E
> +#define ADXL355_FIFO_DATA_REG          0x11
> +#define ADXL355_OFFSET_X_H_REG         0x1E
> +#define ADXL355_OFFSET_Y_H_REG         0x20
> +#define ADXL355_OFFSET_Z_H_REG         0x22
> +#define ADXL355_ACT_EN_REG             0x24
> +#define ADXL355_ACT_THRESH_H_REG       0x25
> +#define ADXL355_ACT_THRESH_L_REG       0x26
> +#define ADXL355_ACT_COUNT_REG          0x27
> +#define ADXL355_FILTER_REG             0x28
> +#define  ADXL355_FILTER_ODR_MSK GENMASK(3, 0)
> +#define  ADXL355_FILTER_HPF_MSK        GENMASK(6, 4)
> +#define ADXL355_FIFO_SAMPLES_REG       0x29
> +#define ADXL355_INT_MAP_REG            0x2A
> +#define ADXL355_SYNC_REG               0x2B
> +#define ADXL355_RANGE_REG              0x2C
> +#define ADXL355_POWER_CTL_REG          0x2D
> +#define         ADXL355_POWER_CTL_MODE_MSK     GENMASK(1, 0)
> +#define ADXL355_SELF_TEST_REG          0x2E
> +#define ADXL355_RESET_REG              0x2F
> +
> +#define ADXL355_DEVID_AD_VAL           0xAD
> +#define ADXL355_DEVID_MST_VAL          0x1D
> +#define ADXL355_PARTID_VAL             0xED
> +#define ADXL355_RESET_CODE             0x52
> +
> +/*
> + * The datasheet defines an intercept of 1885 LSB at 25 degC
> + * and a slope of -9.05 LSB/C. The following formula can be used to find the
> + * temperature:
> + * Temp = ((RAW - 1885)/(-9.05)) + 25 but this doesn't follow the format of
> + * the IIO which is Temp = (RAW + OFFSET) * SCALE. Hence using some rearranging
> + * we get the scale as -110.49723 and offset as -2111.25
> + */
> +#define TEMP_SCALE_VAL -110
> +#define TEMP_SCALE_VAL2 497238
> +#define TEMP_OFFSET_VAL -2111
> +#define TEMP_OFFSET_VAL2 250000
> +
> +/*
> + * At +/- 2g with 20-bit resolution, scale is given in datasheet as
> + * 3.9ug/LSB = 0.0000039 * 9.80665 = 0.00003824593 m/s^2
> + */
> +#define ADXL355_NSCALE 38245
> +
> +static const struct regmap_range adxl355_read_reg_range[] = {
> +       regmap_reg_range(ADXL355_DEVID_AD_REG, ADXL355_FIFO_DATA_REG),
> +       regmap_reg_range(ADXL355_OFFSET_X_H_REG, ADXL355_SELF_TEST_REG)
> +};
> +
> +const struct regmap_access_table adxl355_readable_regs_tbl = {
> +       .yes_ranges = adxl355_read_reg_range,
> +       .n_yes_ranges = ARRAY_SIZE(adxl355_read_reg_range),
> +};
> +EXPORT_SYMBOL_GPL(adxl355_readable_regs_tbl);
> +
> +static const struct regmap_range adxl355_write_reg_range[] = {
> +       regmap_reg_range(ADXL355_OFFSET_X_H_REG, ADXL355_RESET_REG)
> +};
> +
> +const struct regmap_access_table adxl355_writeable_regs_tbl = {
> +       .yes_ranges = adxl355_write_reg_range,
> +       .n_yes_ranges = ARRAY_SIZE(adxl355_write_reg_range),
> +};
> +EXPORT_SYMBOL_GPL(adxl355_writeable_regs_tbl);
> +
> +enum adxl355_op_mode {
> +       ADXL355_MEASUREMENT,
> +       ADXL355_STANDBY,
> +       ADXL355_TEMP_OFF
> +};
> +
> +enum adxl355_odr {
> +       ADXL355_ODR_4000HZ,
> +       ADXL355_ODR_2000HZ,
> +       ADXL355_ODR_1000HZ,
> +       ADXL355_ODR_500HZ,
> +       ADXL355_ODR_250HZ,
> +       ADXL355_ODR_125HZ,
> +       ADXL355_ODR_62_5HZ,
> +       ADXL355_ODR_31_25HZ,
> +       ADXL355_ODR_15_625HZ,
> +       ADXL355_ODR_7_813HZ,
> +       ADXL355_ODR_3_906HZ
> +};
> +
> +enum adxl355_hpf_3db {
> +       ADXL355_HPF_OFF,
> +       ADXL355_HPF_24_7,
> +       ADXL355_HPF_6_2084,
> +       ADXL355_HPF_1_5545,
> +       ADXL355_HPF_0_3862,
> +       ADXL355_HPF_0_0954,
> +       ADXL355_HPF_0_0238
> +};
> +
> +static const int adxl355_odr_table[][2] = {
> +       [0] = {4000, 0},
> +       [1] = {2000, 0},
> +       [2] = {1000, 0},
> +       [3] = {500, 0},
> +       [4] = {250, 0},
> +       [5] = {125, 0},
> +       [6] = {62, 500000},
> +       [7] = {31, 250000},
> +       [8] = {15, 625000},
> +       [9] = {7, 813000},
> +       [10] = {3, 906000}
> +};
> +
> +static const int adxl355_hpf_3db_multipliers[] = {
> +       0,
> +       247000,
> +       62084,
> +       15545,
> +       3862,
> +       954,
> +       238
> +};
> +
> +enum adxl355_chans {
> +       chan_x, chan_y, chan_z
> +};
> +
> +struct adxl355_chan_info {
> +       u8 data_reg;
> +       u8 offset_reg;
> +};
> +
> +static const struct adxl355_chan_info adxl355_chans[] = {
> +       [chan_x] = {
> +               .data_reg = ADXL355_XDATA3_REG,
> +               .offset_reg = ADXL355_OFFSET_X_H_REG
> +       },
> +       [chan_y] = {
> +               .data_reg = ADXL355_YDATA3_REG,
> +               .offset_reg = ADXL355_OFFSET_Y_H_REG
> +       },
> +       [chan_z] = {
> +               .data_reg = ADXL355_ZDATA3_REG,
> +               .offset_reg = ADXL355_OFFSET_Z_H_REG
> +       }
> +};
> +
> +struct adxl355_data {
> +       struct regmap *regmap;
> +       struct device *dev;
> +       struct mutex lock; /* lock to protect op_mode */
> +       enum adxl355_op_mode op_mode;
> +       enum adxl355_odr odr;
> +       enum adxl355_hpf_3db hpf_3db;
> +       int calibbias[3];
> +       int adxl355_hpf_3db_table[7][2];
> +       u8 transf_buf[3] ____cacheline_aligned;
> +};
> +
> +static int adxl355_set_op_mode(struct adxl355_data *data,
> +                              enum adxl355_op_mode op_mode)
> +{
> +       int ret;
> +
> +       if (data->op_mode == op_mode)
> +               return 0;
> +
> +       ret = regmap_update_bits(data->regmap, ADXL355_POWER_CTL_REG,
> +                                ADXL355_POWER_CTL_MODE_MSK, op_mode);
> +       if (ret)
> +               return ret;
> +
> +       data->op_mode = op_mode;
> +
> +       return ret;
> +}
> +
> +static void adxl355_fill_3db_frequency_table(struct adxl355_data *data)
> +{
> +       int i;
> +       u64 rem;
> +       u64 div;
> +       u32 multiplier;
> +       u64 odr = mul_u64_u32_shr(adxl355_odr_table[data->odr][0], 1000000, 0) +
> +                                 adxl355_odr_table[data->odr][1];
> +
> +       for (i = 0; i < ARRAY_SIZE(adxl355_hpf_3db_multipliers); i++) {
> +               multiplier = adxl355_hpf_3db_multipliers[i];
> +               div = div64_u64_rem(mul_u64_u32_shr(odr, multiplier, 0),
> +                                   100000000000000UL, &rem);
> +
> +               data->adxl355_hpf_3db_table[i][0] = div;
> +               data->adxl355_hpf_3db_table[i][1] = div_u64(rem, 100000000);
> +       }
> +}
> +
> +static int adxl355_setup(struct adxl355_data *data)
> +{
> +       unsigned int regval;
> +       int ret;
> +
> +       ret = regmap_read(data->regmap, ADXL355_DEVID_AD_REG, &regval);
> +       if (ret)
> +               return ret;
> +
> +       if (regval != ADXL355_DEVID_AD_VAL) {
> +               dev_err(data->dev, "Invalid ADI ID 0x%02x\n", regval);
> +               return -ENODEV;
> +       }
> +
> +       ret = regmap_read(data->regmap, ADXL355_DEVID_MST_REG, &regval);
> +       if (ret)
> +               return ret;
> +
> +       if (regval != ADXL355_DEVID_MST_VAL) {
> +               dev_err(data->dev, "Invalid MEMS ID 0x%02x\n", regval);
> +               return -ENODEV;
> +       }
> +
> +       ret = regmap_read(data->regmap, ADXL355_PARTID_REG, &regval);
> +       if (ret)
> +               return ret;
> +
> +       if (regval != ADXL355_PARTID_VAL) {
> +               dev_err(data->dev, "Invalid DEV ID 0x%02x\n", regval);
> +               return -ENODEV;
> +       }
> +
> +       /*
> +        * Perform a software reset to make sure the device is in a consistent
> +        * state after start up.
> +        */
> +       ret = regmap_write(data->regmap, ADXL355_RESET_REG, ADXL355_RESET_CODE);
> +       if (ret)
> +               return ret;
> +
> +       adxl355_fill_3db_frequency_table(data);
> +
> +       return adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> +}
> +
> +static int adxl355_get_temp_data(struct adxl355_data *data, u8 addr)
> +{
> +       return regmap_bulk_read(data->regmap, addr, data->transf_buf, 2);
> +}
> +
> +static int adxl355_read_axis(struct adxl355_data *data, u8 addr)
> +{
> +       int ret;
> +
> +       ret = regmap_bulk_read(data->regmap, addr, data->transf_buf, 3);
> +       if (ret < 0)
> +               return ret;
> +
> +       return get_unaligned_be24(data->transf_buf);
> +}
> +
> +static int adxl355_find_match(const int (*freq_tbl)[2], const int n,
> +                             const int val, const int val2)
> +{
> +       int i;
> +
> +       for (i = 0; i < n; i++) {
> +               if (freq_tbl[i][0] == val && freq_tbl[i][1] == val2)
> +                       return i;
> +       }
> +
> +       return -EINVAL;
> +}
> +
> +static int adxl355_set_odr(struct adxl355_data *data,
> +                          enum adxl355_odr odr)
> +{
> +       int ret = 0;
> +
> +       mutex_lock(&data->lock);
> +
> +       if (data->odr == odr)
> +               goto out_unlock;
> +
> +       ret = adxl355_set_op_mode(data, ADXL355_STANDBY);
> +       if (ret < 0)
> +               goto out_unlock;
> +
> +       ret = regmap_update_bits(data->regmap, ADXL355_FILTER_REG,
> +                                ADXL355_FILTER_ODR_MSK,
> +                                FIELD_PREP(ADXL355_FILTER_ODR_MSK, odr));
> +       if (ret < 0)
> +               goto out_unlock;
> +
> +       data->odr = odr;
> +       adxl355_fill_3db_frequency_table(data);
> +
> +out_unlock:
> +       ret = adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> +       mutex_unlock(&data->lock);
> +       return ret;
> +}
> +
> +static int adxl355_set_hpf_3db(struct adxl355_data *data,
> +                              enum adxl355_hpf_3db hpf)
> +{
> +       int ret = 0;
> +
> +       mutex_lock(&data->lock);
> +
> +       if (data->hpf_3db == hpf)
> +               goto unlock;
> +
> +       ret = adxl355_set_op_mode(data, ADXL355_STANDBY);
> +       if (ret < 0)
> +               goto out_unlock;
> +
> +       ret = regmap_update_bits(data->regmap, ADXL355_FILTER_REG,
> +                                ADXL355_FILTER_HPF_MSK,
> +                                FIELD_PREP(ADXL355_FILTER_HPF_MSK, hpf));
> +       if (!ret)
> +               data->hpf_3db = hpf;
> +
> +out_unlock:
> +       ret = adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> +unlock:
> +       mutex_unlock(&data->lock);
> +       return ret;
> +}
> +
> +static int adxl355_set_calibbias(struct adxl355_data *data,
> +                                enum adxl355_chans chan, int calibbias)
> +{
> +       int ret = 0;
> +
> +       put_unaligned_be16(calibbias, data->transf_buf);
> +
> +       mutex_lock(&data->lock);
> +
> +       ret = adxl355_set_op_mode(data, ADXL355_STANDBY);
> +       if (ret < 0)
> +               goto out_unlock;
> +
> +       ret = regmap_bulk_write(data->regmap,
> +                               adxl355_chans[chan].offset_reg,
> +                               data->transf_buf, 2);
> +       if (ret)
> +               goto out_unlock;
> +
> +       data->calibbias[chan] = calibbias;
> +
> +out_unlock:
> +       ret = adxl355_set_op_mode(data, ADXL355_MEASUREMENT);
> +       mutex_unlock(&data->lock);
> +       return ret;
> +}
> +
> +static int adxl355_read_raw(struct iio_dev *indio_dev,
> +                           struct iio_chan_spec const *chan,
> +                           int *val, int *val2, long mask)
> +{
> +       struct adxl355_data *data = iio_priv(indio_dev);
> +       int ret;
> +
> +       switch (mask) {
> +       case IIO_CHAN_INFO_RAW:
> +               switch (chan->type) {
> +               case IIO_TEMP:
> +                       ret = adxl355_get_temp_data(data, chan->address);
> +                       if (ret < 0)
> +                               return ret;
> +                       *val = get_unaligned_be16(data->transf_buf);
> +
> +                       return IIO_VAL_INT;
> +               case IIO_ACCEL:
> +                       ret = adxl355_read_axis(data, adxl355_chans[
> +                                               chan->address].data_reg);
> +                       if (ret < 0)
> +                               return ret;
> +                       *val = sign_extend32(ret >> chan->scan_type.shift,
> +                                            chan->scan_type.realbits - 1);
> +                       return IIO_VAL_INT;
> +               default:
> +                       return -EINVAL;
> +               }
> +
> +       case IIO_CHAN_INFO_SCALE:
> +               switch (chan->type) {
> +               case IIO_TEMP:
> +                       *val = TEMP_SCALE_VAL;
> +                       *val2 = TEMP_SCALE_VAL2;
> +                       return IIO_VAL_INT_PLUS_MICRO;
> +               case IIO_ACCEL:
> +                       *val = 0;
> +                       *val2 = ADXL355_NSCALE;
> +                       return IIO_VAL_INT_PLUS_NANO;
> +               default:
> +                       return -EINVAL;
> +               }
> +       case IIO_CHAN_INFO_OFFSET:
> +               *val = TEMP_OFFSET_VAL;
> +               *val2 = TEMP_OFFSET_VAL2;
> +               return IIO_VAL_INT_PLUS_MICRO;
> +       case IIO_CHAN_INFO_CALIBBIAS:
> +               *val = sign_extend32(data->calibbias[chan->address], 15);
> +               return IIO_VAL_INT;
> +       case IIO_CHAN_INFO_SAMP_FREQ:
> +               *val = adxl355_odr_table[data->odr][0];
> +               *val2 = adxl355_odr_table[data->odr][1];
> +               return IIO_VAL_INT_PLUS_MICRO;
> +       case IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY:
> +               *val = data->adxl355_hpf_3db_table[data->hpf_3db][0];
> +               *val2 = data->adxl355_hpf_3db_table[data->hpf_3db][1];
> +               return IIO_VAL_INT_PLUS_MICRO;
> +       default:
> +               return -EINVAL;
> +       }
> +}
> +
> +static int adxl355_write_raw(struct iio_dev *indio_dev,
> +                            struct iio_chan_spec const *chan,
> +                            int val, int val2, long mask)
> +{
> +       struct adxl355_data *data = iio_priv(indio_dev);
> +       int odr_idx, hpf_idx, calibbias;
> +
> +       switch (mask) {
> +       case IIO_CHAN_INFO_SAMP_FREQ:
> +               odr_idx = adxl355_find_match(adxl355_odr_table,
> +                                            ARRAY_SIZE(adxl355_odr_table),
> +                                            val, val2);
> +               if (odr_idx < 0)
> +                       return odr_idx;
> +
> +               return adxl355_set_odr(data, odr_idx);
> +       case IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY:
> +               hpf_idx = adxl355_find_match(data->adxl355_hpf_3db_table,
> +                                       ARRAY_SIZE(data->adxl355_hpf_3db_table),
> +                                            val, val2);
> +               if (hpf_idx < 0)
> +                       return hpf_idx;
> +
> +               return adxl355_set_hpf_3db(data, hpf_idx);
> +       case IIO_CHAN_INFO_CALIBBIAS:
> +               calibbias = clamp_t(int, val, S16_MIN, S16_MAX);
> +
> +               return adxl355_set_calibbias(data, chan->address, calibbias);
> +       default:
> +               return -EINVAL;
> +       }
> +}
> +
> +static int adxl355_read_avail(struct iio_dev *indio_dev,
> +                             struct iio_chan_spec const *chan,
> +                             const int **vals, int *type, int *length,
> +                             long mask)
> +{
> +       struct adxl355_data *data = iio_priv(indio_dev);
> +
> +       switch (mask) {
> +       case IIO_CHAN_INFO_SAMP_FREQ:
> +               *vals = (const int *)adxl355_odr_table;
> +               *type = IIO_VAL_INT_PLUS_MICRO;
> +               /* Values are stored in a 2D matrix */
> +               *length = ARRAY_SIZE(adxl355_odr_table) * 2;
> +
> +               return IIO_AVAIL_LIST;
> +       case IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY:
> +               *vals = (const int *)data->adxl355_hpf_3db_table;
> +               *type = IIO_VAL_INT_PLUS_MICRO;
> +               /* Values are stored in a 2D matrix */
> +               *length = ARRAY_SIZE(data->adxl355_hpf_3db_table) * 2;
> +
> +               return IIO_AVAIL_LIST;
> +       default:
> +               return -EINVAL;
> +       }
> +}
> +
> +static const struct iio_info adxl355_info = {
> +       .read_raw       = adxl355_read_raw,
> +       .write_raw      = adxl355_write_raw,
> +       .read_avail     = &adxl355_read_avail
> +};
> +
> +#define ADXL355_ACCEL_CHANNEL(index, reg, axis) {                      \
> +       .type = IIO_ACCEL,                                              \
> +       .address = reg,                                                 \
> +       .modified = 1,                                                  \
> +       .channel2 = IIO_MOD_##axis,                                     \
> +       .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |                  \
> +                             BIT(IIO_CHAN_INFO_CALIBBIAS),             \
> +       .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |          \
> +                                   BIT(IIO_CHAN_INFO_SAMP_FREQ) |      \
> +               BIT(IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY),      \
> +       .info_mask_shared_by_type_available =                           \
> +               BIT(IIO_CHAN_INFO_SAMP_FREQ) |                          \
> +               BIT(IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY),      \
> +       .scan_type = {                                                  \
> +               .sign = 's',                                            \
> +               .realbits = 20,                                         \
> +               .storagebits = 32,                                      \
> +               .shift = 4,                                             \
> +               .endianness = IIO_BE,                                   \
> +       }                                                               \
> +}
> +
> +static const struct iio_chan_spec adxl355_channels[] = {
> +       ADXL355_ACCEL_CHANNEL(0, chan_x, X),
> +       ADXL355_ACCEL_CHANNEL(1, chan_y, Y),
> +       ADXL355_ACCEL_CHANNEL(2, chan_z, Z),
> +       {
> +               .type = IIO_TEMP,
> +               .address = ADXL355_TEMP2_REG,
> +               .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +                                     BIT(IIO_CHAN_INFO_SCALE) |
> +                                     BIT(IIO_CHAN_INFO_OFFSET),
> +               .scan_type = {
> +                       .sign = 's',
> +                       .realbits = 12,
> +                       .storagebits = 16,
> +                       .endianness = IIO_BE,
> +               },
> +       }
> +};
> +
> +int adxl355_core_probe(struct device *dev, struct regmap *regmap,
> +                      const char *name)
> +{
> +       struct adxl355_data *data;
> +       struct iio_dev *indio_dev;
> +       int ret;
> +
> +       indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> +       if (!indio_dev)
> +               return -ENOMEM;
> +
> +       data = iio_priv(indio_dev);
> +       data->regmap = regmap;
> +       data->dev = dev;
> +       data->op_mode = ADXL355_STANDBY;
> +       mutex_init(&data->lock);
> +
> +       indio_dev->name = name;
> +       indio_dev->info = &adxl355_info;
> +       indio_dev->modes = INDIO_DIRECT_MODE;
> +       indio_dev->channels = adxl355_channels;
> +       indio_dev->num_channels = ARRAY_SIZE(adxl355_channels);
> +
> +       ret = adxl355_setup(data);
> +       if (ret < 0) {
> +               dev_err(dev, "ADXL355 setup failed\n");
> +               return ret;
> +       }
> +
> +       return devm_iio_device_register(dev, indio_dev);
> +}
> +EXPORT_SYMBOL_GPL(adxl355_core_probe);
> +
> +MODULE_AUTHOR("Puranjay Mohan <puranjay12@gmail.com>");
> +MODULE_DESCRIPTION("ADXL355 3-Axis Digital Accelerometer core driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/accel/adxl355_i2c.c b/drivers/iio/accel/adxl355_i2c.c
> new file mode 100644
> index 000000000..e3070ee81
> --- /dev/null
> +++ b/drivers/iio/accel/adxl355_i2c.c
> @@ -0,0 +1,64 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * ADXL355 3-Axis Digital Accelerometer I2C driver
> + *
> + * Copyright (c) 2021 Puranjay Mohan <puranjay12@gmail.com>
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/regmap.h>
> +
> +#include "adxl355.h"
> +
> +static const struct regmap_config adxl355_i2c_regmap_config = {
> +       .reg_bits = 8,
> +       .val_bits = 8,
> +       .max_register = 0x2F,
> +       .rd_table = &adxl355_readable_regs_tbl,
> +       .wr_table = &adxl355_writeable_regs_tbl
> +};
> +
> +static int adxl355_i2c_probe(struct i2c_client *client)
> +{
> +       struct regmap *regmap;
> +
> +       regmap = devm_regmap_init_i2c(client, &adxl355_i2c_regmap_config);
> +       if (IS_ERR(regmap)) {
> +               dev_err(&client->dev, "Error initializing i2c regmap: %ld\n",
> +                       PTR_ERR(regmap));
> +               return PTR_ERR(regmap);
> +       }
> +
> +       return adxl355_core_probe(&client->dev, regmap, client->name);
> +}
> +
> +static const struct i2c_device_id adxl355_i2c_id[] = {
> +       { "adxl355", 0 },
> +       { }
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, adxl355_i2c_id);
> +
> +static const struct of_device_id adxl355_of_match[] = {
> +       { .compatible = "adi,adxl355" },
> +       { }
> +};
> +
> +MODULE_DEVICE_TABLE(of, adxl355_of_match);
> +
> +static struct i2c_driver adxl355_i2c_driver = {
> +       .driver = {
> +               .name   = "adxl355_i2c",
> +               .of_match_table = adxl355_of_match,
> +       },
> +       .probe_new      = adxl355_i2c_probe,
> +       .id_table       = adxl355_i2c_id,
> +};
> +
> +module_i2c_driver(adxl355_i2c_driver);
> +
> +MODULE_AUTHOR("Puranjay Mohan <puranjay12@gmail.com>");
> +MODULE_DESCRIPTION("ADXL355 3-Axis Digital Accelerometer I2C driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/accel/adxl355_spi.c b/drivers/iio/accel/adxl355_spi.c
> new file mode 100644
> index 000000000..a16bd1407
> --- /dev/null
> +++ b/drivers/iio/accel/adxl355_spi.c
> @@ -0,0 +1,67 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * ADXL355 3-Axis Digital Accelerometer SPI driver
> + *
> + * Copyright (c) 2021 Puranjay Mohan <puranjay12@gmail.com>
> + */
> +
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/regmap.h>
> +#include <linux/spi/spi.h>
> +
> +#include "adxl355.h"
> +
> +static const struct regmap_config adxl355_spi_regmap_config = {
> +       .reg_bits = 7,
> +       .pad_bits = 1,
> +       .val_bits = 8,
> +       .read_flag_mask = BIT(0),
> +       .max_register = 0x2F,
> +       .rd_table = &adxl355_readable_regs_tbl,
> +       .wr_table = &adxl355_writeable_regs_tbl
> +};
> +
> +static int adxl355_spi_probe(struct spi_device *spi)
> +{
> +       const struct spi_device_id *id = spi_get_device_id(spi);
> +       struct regmap *regmap;
> +
> +       regmap = devm_regmap_init_spi(spi, &adxl355_spi_regmap_config);
> +       if (IS_ERR(regmap)) {
> +               dev_err(&spi->dev, "Error initializing spi regmap: %ld\n",
> +                       PTR_ERR(regmap));
> +               return PTR_ERR(regmap);
> +       }
> +
> +       return adxl355_core_probe(&spi->dev, regmap, id->name);
> +}
> +
> +static const struct spi_device_id adxl355_spi_id[] = {
> +       { "adxl355", 0 },
> +       { }
> +};
> +
> +MODULE_DEVICE_TABLE(spi, adxl355_spi_id);
> +
> +static const struct of_device_id adxl355_of_match[] = {
> +       { .compatible = "adi,adxl355" },
> +       { }
> +};
> +
> +MODULE_DEVICE_TABLE(of, adxl355_of_match);
> +
> +static struct spi_driver adxl355_spi_driver = {
> +       .driver = {
> +               .name   = "adxl355_spi",
> +               .of_match_table = adxl355_of_match,
> +       },
> +       .probe          = adxl355_spi_probe,
> +       .id_table       = adxl355_spi_id,
> +};
> +
> +module_spi_driver(adxl355_spi_driver);
> +
> +MODULE_AUTHOR("Puranjay Mohan <puranjay12@gmail.com>");
> +MODULE_DESCRIPTION("ADXL355 3-Axis Digital Accelerometer SPI driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.30.1
>

Re: [PATCH v8 2/3] iio: accel: Add driver support for ADXL355

[Jonathan Cameron]

On Wed,  4 Aug 2021 19:33:08 +0530
Puranjay Mohan <puranjay12@gmail.com> wrote:

> ADXL355 is 3-axis MEMS Accelerometer. It offers low noise density,
> low 0g offset drift, low power with selectable measurement ranges.
> It also features programmable high-pass and low-pass filters.
> 
> Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adxl354_adxl355.pdf
> Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
Hi Puranjay,

One comment below, but it's a follow up on a discussion going on in the ADXL313 driver
review. I might just make that change whilst applying this, or leave it for
another day. Either way, no need for you to change anything unless you are
doing a v9 for another reason.

...

> +
> +/*
> + * The datasheet defines an intercept of 1885 LSB at 25 degC
> + * and a slope of -9.05 LSB/C. The following formula can be used to find the
> + * temperature:
> + * Temp = ((RAW - 1885)/(-9.05)) + 25 but this doesn't follow the format of
> + * the IIO which is Temp = (RAW + OFFSET) * SCALE. Hence using some rearranging
> + * we get the scale as -110.49723 and offset as -2111.25
> + */
> +#define TEMP_SCALE_VAL -110
> +#define TEMP_SCALE_VAL2 497238
> +#define TEMP_OFFSET_VAL -2111
> +#define TEMP_OFFSET_VAL2 250000
> +
> +/*
> + * At +/- 2g with 20-bit resolution, scale is given in datasheet as
> + * 3.9ug/LSB = 0.0000039 * 9.80665 = 0.00003824593 m/s^2
> + */
> +#define ADXL355_NSCALE	38245

This just came up in another review and wasn't something I've been picking
up on in reviews before now.

These scale defines would be better dropped and the values moved inline
(along with the comments).  It avoids possible unit misunderstandings etc
In short the key thing is, sometimes a number is just a number and
using a define to give it a name is not always helpful, particularly if
it's only used in one place in the code.

[PATCH v8 3/3] iio: accel: adxl355: Add triggered buffer support

[Puranjay Mohan]

Provide a way for continuous data capture by setting up buffer support. The
data ready signal exposed at the DRDY pin of the ADXL355 is exploited as
a hardware interrupt which triggers to fill the buffer.

Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
---
 drivers/iio/accel/Kconfig        |   4 ++
 drivers/iio/accel/adxl355.h      |   2 +-
 drivers/iio/accel/adxl355_core.c | 102 ++++++++++++++++++++++++++++++-
 drivers/iio/accel/adxl355_i2c.c  |   3 +-
 drivers/iio/accel/adxl355_spi.c  |   2 +-
 5 files changed, 108 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index d0c45c809..9c16c1841 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -69,6 +69,8 @@ config ADXL355_I2C
 	depends on I2C
 	select ADXL355
 	select REGMAP_I2C
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
 	help
 	  Say Y here if you want to build i2c support for the Analog Devices
 	  ADXL355 3-axis digital accelerometer.
@@ -82,6 +84,8 @@ config ADXL355_SPI
 	depends on SPI
 	select ADXL355
 	select REGMAP_SPI
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
 	help
 	  Say Y here if you want to build spi support for the Analog Devices
 	  ADXL355 3-axis digital accelerometer.
diff --git a/drivers/iio/accel/adxl355.h b/drivers/iio/accel/adxl355.h
index 322b0abb8..f0a376e6d 100644
--- a/drivers/iio/accel/adxl355.h
+++ b/drivers/iio/accel/adxl355.h
@@ -15,5 +15,5 @@ extern const struct regmap_access_table adxl355_readable_regs_tbl;
 extern const struct regmap_access_table adxl355_writeable_regs_tbl;
 
 int adxl355_core_probe(struct device *dev, struct regmap *regmap,
-		       const char *name);
+		       const char *name, int irq);
 #endif /* _ADXL355_H_ */
diff --git a/drivers/iio/accel/adxl355_core.c b/drivers/iio/accel/adxl355_core.c
index 47fbb31bc..f9dc2a530 100644
--- a/drivers/iio/accel/adxl355_core.c
+++ b/drivers/iio/accel/adxl355_core.c
@@ -9,6 +9,10 @@
 
 #include <asm/unaligned.h>
 #include <linux/bitfield.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
 #include <linux/iio/iio.h>
 #include <linux/limits.h>
 #include <linux/math64.h>
@@ -172,6 +176,7 @@ static const struct adxl355_chan_info adxl355_chans[] = {
 };
 
 struct adxl355_data {
+	int irq;
 	struct regmap *regmap;
 	struct device *dev;
 	struct mutex lock; /* lock to protect op_mode */
@@ -181,6 +186,12 @@ struct adxl355_data {
 	int calibbias[3];
 	int adxl355_hpf_3db_table[7][2];
 	u8 transf_buf[3] ____cacheline_aligned;
+	struct iio_trigger      *dready_trig;
+	/* Ensure correct alignment of timestamp when present */
+	struct {
+		__be32 channels[3];
+		s64 ts;
+	} buffer ____cacheline_aligned;
 };
 
 static int adxl355_set_op_mode(struct adxl355_data *data,
@@ -499,12 +510,46 @@ static int adxl355_read_avail(struct iio_dev *indio_dev,
 	}
 }
 
+static const unsigned long adxl355_avail_scan_masks[] = {
+	GENMASK(3, 0),
+	0
+};
+
 static const struct iio_info adxl355_info = {
 	.read_raw	= adxl355_read_raw,
 	.write_raw	= adxl355_write_raw,
 	.read_avail	= &adxl355_read_avail
 };
 
+static const struct iio_trigger_ops adxl355_trigger_ops = {
+	.validate_device = &iio_trigger_validate_own_device,
+};
+
+static irqreturn_t adxl355_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct adxl355_data *data = iio_priv(indio_dev);
+	int ret;
+
+	mutex_lock(&data->lock);
+
+	ret = regmap_bulk_read(data->regmap, ADXL355_XDATA3_REG,
+			       data->buffer.channels,
+			       9);
+	if (ret)
+		goto out_unlock_notify;
+
+	iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer,
+					   pf->timestamp);
+
+out_unlock_notify:
+	mutex_unlock(&data->lock);
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
 #define ADXL355_ACCEL_CHANNEL(index, reg, axis) {			\
 	.type = IIO_ACCEL,						\
 	.address = reg,							\
@@ -518,6 +563,7 @@ static const struct iio_info adxl355_info = {
 	.info_mask_shared_by_type_available =				\
 		BIT(IIO_CHAN_INFO_SAMP_FREQ) |				\
 		BIT(IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY),	\
+	.scan_index = index,						\
 	.scan_type = {							\
 		.sign = 's',						\
 		.realbits = 20,						\
@@ -537,17 +583,56 @@ static const struct iio_chan_spec adxl355_channels[] = {
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
 				      BIT(IIO_CHAN_INFO_SCALE) |
 				      BIT(IIO_CHAN_INFO_OFFSET),
+		.scan_index = 3,
 		.scan_type = {
 			.sign = 's',
 			.realbits = 12,
 			.storagebits = 16,
 			.endianness = IIO_BE,
 		},
-	}
+	},
+	IIO_CHAN_SOFT_TIMESTAMP(4)
 };
 
+static int adxl355_probe_trigger(struct iio_dev *indio_dev)
+{
+	struct adxl355_data *data = iio_priv(indio_dev);
+	int ret;
+
+	if (!data->irq) {
+		dev_info(data->dev, "no irq, using polling\n");
+		return 0;
+	}
+
+	data->dready_trig = devm_iio_trigger_alloc(data->dev, "%s-dev%d",
+						   indio_dev->name,
+						   indio_dev->id);
+	if (!data->dready_trig)
+		return -ENOMEM;
+
+	data->dready_trig->ops = &adxl355_trigger_ops;
+	iio_trigger_set_drvdata(data->dready_trig, indio_dev);
+
+	ret = devm_request_irq(data->dev, data->irq,
+			       &iio_trigger_generic_data_rdy_poll,
+			       IRQF_ONESHOT, "adxl355_irq", data->dready_trig);
+	if (ret < 0)
+		return dev_err_probe(data->dev, ret, "request irq %d failed\n",
+				     data->irq);
+
+	ret = devm_iio_trigger_register(data->dev, data->dready_trig);
+	if (ret) {
+		dev_err(data->dev, "iio trigger register failed\n");
+		return ret;
+	}
+
+	indio_dev->trig = iio_trigger_get(data->dready_trig);
+
+	return 0;
+}
+
 int adxl355_core_probe(struct device *dev, struct regmap *regmap,
-		       const char *name)
+		       const char *name, int irq)
 {
 	struct adxl355_data *data;
 	struct iio_dev *indio_dev;
@@ -560,6 +645,7 @@ int adxl355_core_probe(struct device *dev, struct regmap *regmap,
 	data = iio_priv(indio_dev);
 	data->regmap = regmap;
 	data->dev = dev;
+	data->irq = irq;
 	data->op_mode = ADXL355_STANDBY;
 	mutex_init(&data->lock);
 
@@ -568,6 +654,7 @@ int adxl355_core_probe(struct device *dev, struct regmap *regmap,
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = adxl355_channels;
 	indio_dev->num_channels = ARRAY_SIZE(adxl355_channels);
+	indio_dev->available_scan_masks = adxl355_avail_scan_masks;
 
 	ret = adxl355_setup(data);
 	if (ret < 0) {
@@ -575,6 +662,17 @@ int adxl355_core_probe(struct device *dev, struct regmap *regmap,
 		return ret;
 	}
 
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
+					      &iio_pollfunc_store_time,
+					      &adxl355_trigger_handler, NULL);
+	if (ret < 0)
+		return dev_err_probe(dev, ret,
+				     "iio triggered buffer setup failed\n");
+
+	ret = adxl355_probe_trigger(indio_dev);
+	if (ret < 0)
+		return ret;
+
 	return devm_iio_device_register(dev, indio_dev);
 }
 EXPORT_SYMBOL_GPL(adxl355_core_probe);
diff --git a/drivers/iio/accel/adxl355_i2c.c b/drivers/iio/accel/adxl355_i2c.c
index e3070ee81..c18521819 100644
--- a/drivers/iio/accel/adxl355_i2c.c
+++ b/drivers/iio/accel/adxl355_i2c.c
@@ -31,7 +31,8 @@ static int adxl355_i2c_probe(struct i2c_client *client)
 		return PTR_ERR(regmap);
 	}
 
-	return adxl355_core_probe(&client->dev, regmap, client->name);
+	return adxl355_core_probe(&client->dev, regmap, client->name,
+				  client->irq);
 }
 
 static const struct i2c_device_id adxl355_i2c_id[] = {
diff --git a/drivers/iio/accel/adxl355_spi.c b/drivers/iio/accel/adxl355_spi.c
index a16bd1407..f9ba153f6 100644
--- a/drivers/iio/accel/adxl355_spi.c
+++ b/drivers/iio/accel/adxl355_spi.c
@@ -34,7 +34,7 @@ static int adxl355_spi_probe(struct spi_device *spi)
 		return PTR_ERR(regmap);
 	}
 
-	return adxl355_core_probe(&spi->dev, regmap, id->name);
+	return adxl355_core_probe(&spi->dev, regmap, id->name, spi->irq);
 }
 
 static const struct spi_device_id adxl355_spi_id[] = {
-- 
2.30.1

Re: [PATCH v8 3/3] iio: accel: adxl355: Add triggered buffer support

[Jonathan Cameron]

On Wed,  4 Aug 2021 19:33:09 +0530
Puranjay Mohan <puranjay12@gmail.com> wrote:

> Provide a way for continuous data capture by setting up buffer support. The
> data ready signal exposed at the DRDY pin of the ADXL355 is exploited as
> a hardware interrupt which triggers to fill the buffer.
> 
> Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>

Hi Puranjay,

This device has a hardware fifo, so we need to be careful how we add
trigger support such that we can enable that smoothly.

There are a few things in here that suggest this isn't tested as much
as I'd expect for a submission.  If you are posting early, but need
to do more testing, please state that clearly.

Jonathan

> ---
>  drivers/iio/accel/Kconfig        |   4 ++
>  drivers/iio/accel/adxl355.h      |   2 +-
>  drivers/iio/accel/adxl355_core.c | 102 ++++++++++++++++++++++++++++++-
>  drivers/iio/accel/adxl355_i2c.c  |   3 +-
>  drivers/iio/accel/adxl355_spi.c  |   2 +-
>  5 files changed, 108 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index d0c45c809..9c16c1841 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -69,6 +69,8 @@ config ADXL355_I2C
>  	depends on I2C
>  	select ADXL355
>  	select REGMAP_I2C
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
>  	help
>  	  Say Y here if you want to build i2c support for the Analog Devices
>  	  ADXL355 3-axis digital accelerometer.
> @@ -82,6 +84,8 @@ config ADXL355_SPI
>  	depends on SPI
>  	select ADXL355
>  	select REGMAP_SPI
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
>  	help
>  	  Say Y here if you want to build spi support for the Analog Devices
>  	  ADXL355 3-axis digital accelerometer.
> diff --git a/drivers/iio/accel/adxl355.h b/drivers/iio/accel/adxl355.h
> index 322b0abb8..f0a376e6d 100644
> --- a/drivers/iio/accel/adxl355.h
> +++ b/drivers/iio/accel/adxl355.h
> @@ -15,5 +15,5 @@ extern const struct regmap_access_table adxl355_readable_regs_tbl;
>  extern const struct regmap_access_table adxl355_writeable_regs_tbl;
>  
>  int adxl355_core_probe(struct device *dev, struct regmap *regmap,
> -		       const char *name);
> +		       const char *name, int irq);
>  #endif /* _ADXL355_H_ */
> diff --git a/drivers/iio/accel/adxl355_core.c b/drivers/iio/accel/adxl355_core.c
> index 47fbb31bc..f9dc2a530 100644
> --- a/drivers/iio/accel/adxl355_core.c
> +++ b/drivers/iio/accel/adxl355_core.c
> @@ -9,6 +9,10 @@
>  
>  #include <asm/unaligned.h>
>  #include <linux/bitfield.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
>  #include <linux/iio/iio.h>
>  #include <linux/limits.h>
>  #include <linux/math64.h>
> @@ -172,6 +176,7 @@ static const struct adxl355_chan_info adxl355_chans[] = {
>  };
>  
>  struct adxl355_data {
> +	int irq;
>  	struct regmap *regmap;
>  	struct device *dev;
>  	struct mutex lock; /* lock to protect op_mode */
> @@ -181,6 +186,12 @@ struct adxl355_data {
>  	int calibbias[3];
>  	int adxl355_hpf_3db_table[7][2];
>  	u8 transf_buf[3] ____cacheline_aligned;
> +	struct iio_trigger      *dready_trig;
> +	/* Ensure correct alignment of timestamp when present */
> +	struct {
> +		__be32 channels[3];
> +		s64 ts;
> +	} buffer ____cacheline_aligned;

Interesting quirk here is that we need to ensure that any of the
buffers used for DMA don't share a cacheline with other data, but we don't
need to ensure they don't share with each other.  As such, this can
share fine with transf_buf, but definitely not with dready_trig (which
can't share with transf_buf like it is right now).

So reorder to put the various DMA safe buffers at the end, and ensure
the first one is cacheline aligned.

Even better, given you can only safely access one at a time anyway
(and I think you always hold the lock so that is fine), put them in a union
to make it explicit that only one is used at a time.

struct iio_trigger *dready_trig;
union {
	u8 trans_buf[3];
	struct {
		__be32 channels[3];
		s64 ts;
	} buffer;
} ___cacheline_algned;
or something like that.



>  };
>  
>  static int adxl355_set_op_mode(struct adxl355_data *data,
> @@ -499,12 +510,46 @@ static int adxl355_read_avail(struct iio_dev *indio_dev,
>  	}
>  }
>  
> +static const unsigned long adxl355_avail_scan_masks[] = {
> +	GENMASK(3, 0),
> +	0
> +};
> +
>  static const struct iio_info adxl355_info = {
>  	.read_raw	= adxl355_read_raw,
>  	.write_raw	= adxl355_write_raw,
>  	.read_avail	= &adxl355_read_avail
>  };
>  
> +static const struct iio_trigger_ops adxl355_trigger_ops = {
> +	.validate_device = &iio_trigger_validate_own_device,

No means of turning it on?

> +};
> +
> +static irqreturn_t adxl355_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct adxl355_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	mutex_lock(&data->lock);
> +
> +	ret = regmap_bulk_read(data->regmap, ADXL355_XDATA3_REG,
> +			       data->buffer.channels,
> +			       9);

That doesn't look right.  you are reading 9 bytes into an array of 3 __be32 channels.
You should check the data is turning up where it should.


> +	if (ret)
> +		goto out_unlock_notify;
> +
> +	iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer,
> +					   pf->timestamp);
> +
> +out_unlock_notify:
> +	mutex_unlock(&data->lock);
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
>  #define ADXL355_ACCEL_CHANNEL(index, reg, axis) {			\
>  	.type = IIO_ACCEL,						\
>  	.address = reg,							\
> @@ -518,6 +563,7 @@ static const struct iio_info adxl355_info = {
>  	.info_mask_shared_by_type_available =				\
>  		BIT(IIO_CHAN_INFO_SAMP_FREQ) |				\
>  		BIT(IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY),	\
> +	.scan_index = index,						\
>  	.scan_type = {							\
>  		.sign = 's',						\
>  		.realbits = 20,						\
> @@ -537,17 +583,56 @@ static const struct iio_chan_spec adxl355_channels[] = {
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>  				      BIT(IIO_CHAN_INFO_SCALE) |
>  				      BIT(IIO_CHAN_INFO_OFFSET),
> +		.scan_index = 3,
>  		.scan_type = {
>  			.sign = 's',
>  			.realbits = 12,
>  			.storagebits = 16,
>  			.endianness = IIO_BE,
>  		},
> -	}
> +	},

If you do end up respinning patch 2, add the comma in that patch as it'll reduced
noise here.

> +	IIO_CHAN_SOFT_TIMESTAMP(4)
>  };
>  
> +static int adxl355_probe_trigger(struct iio_dev *indio_dev)
> +{
> +	struct adxl355_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	if (!data->irq) {
> +		dev_info(data->dev, "no irq, using polling\n");
> +		return 0;
> +	}
> +
> +	data->dready_trig = devm_iio_trigger_alloc(data->dev, "%s-dev%d",
> +						   indio_dev->name,
> +						   indio_dev->id);
> +	if (!data->dready_trig)
> +		return -ENOMEM;
> +
> +	data->dready_trig->ops = &adxl355_trigger_ops;
> +	iio_trigger_set_drvdata(data->dready_trig, indio_dev);
> +
> +	ret = devm_request_irq(data->dev, data->irq,
> +			       &iio_trigger_generic_data_rdy_poll,
> +			       IRQF_ONESHOT, "adxl355_irq", data->dready_trig);
> +	if (ret < 0)
> +		return dev_err_probe(data->dev, ret, "request irq %d failed\n",
> +				     data->irq);
> +
> +	ret = devm_iio_trigger_register(data->dev, data->dready_trig);
> +	if (ret) {
> +		dev_err(data->dev, "iio trigger register failed\n");
> +		return ret;
> +	}
> +
> +	indio_dev->trig = iio_trigger_get(data->dready_trig);
> +
> +	return 0;
> +}
> +
>  int adxl355_core_probe(struct device *dev, struct regmap *regmap,
> -		       const char *name)
> +		       const char *name, int irq)
>  {
>  	struct adxl355_data *data;
>  	struct iio_dev *indio_dev;
> @@ -560,6 +645,7 @@ int adxl355_core_probe(struct device *dev, struct regmap *regmap,
>  	data = iio_priv(indio_dev);
>  	data->regmap = regmap;
>  	data->dev = dev;
> +	data->irq = irq;
>  	data->op_mode = ADXL355_STANDBY;
>  	mutex_init(&data->lock);
>  
> @@ -568,6 +654,7 @@ int adxl355_core_probe(struct device *dev, struct regmap *regmap,
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  	indio_dev->channels = adxl355_channels;
>  	indio_dev->num_channels = ARRAY_SIZE(adxl355_channels);
> +	indio_dev->available_scan_masks = adxl355_avail_scan_masks;
>  
>  	ret = adxl355_setup(data);
>  	if (ret < 0) {
> @@ -575,6 +662,17 @@ int adxl355_core_probe(struct device *dev, struct regmap *regmap,
>  		return ret;
>  	}
>  
> +	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
> +					      &iio_pollfunc_store_time,
> +					      &adxl355_trigger_handler, NULL);
> +	if (ret < 0)

if (ret) preferred, because it is more consistent with returning devm_iio_device_register()
below (where we are assuming negative or 0 only)

> +		return dev_err_probe(dev, ret,
> +				     "iio triggered buffer setup failed\n");
> +
> +	ret = adxl355_probe_trigger(indio_dev);
> +	if (ret < 0)
> +		return ret;
> +
>  	return devm_iio_device_register(dev, indio_dev);
>  }
>  EXPORT_SYMBOL_GPL(adxl355_core_probe);
> diff --git a/drivers/iio/accel/adxl355_i2c.c b/drivers/iio/accel/adxl355_i2c.c
> index e3070ee81..c18521819 100644
> --- a/drivers/iio/accel/adxl355_i2c.c
> +++ b/drivers/iio/accel/adxl355_i2c.c
> @@ -31,7 +31,8 @@ static int adxl355_i2c_probe(struct i2c_client *client)
>  		return PTR_ERR(regmap);
>  	}
>  
> -	return adxl355_core_probe(&client->dev, regmap, client->name);
> +	return adxl355_core_probe(&client->dev, regmap, client->name,
> +				  client->irq);

We can't just use the first interrupt (which this corresponds to) because
the device supports 3 and who knows if this is the right one?

Now to solve this, we run into a 'fun' issue.  We want to use the generic
firmware property interfaces (include/linux/property.h) but that doesn't
include a fwnode_irq_get_by_name()

We've had some recent discussion about this:
https://lore.kernel.org/linux-iio/20210724190614.3593dcd1@jic23-huawei/

If it is something you would like to take on, that would be great.
If not, for now use the of_irq_get_byname() method but add a comment
saying it would good to move to a generic version.

>  }
>  
>  static const struct i2c_device_id adxl355_i2c_id[] = {
> diff --git a/drivers/iio/accel/adxl355_spi.c b/drivers/iio/accel/adxl355_spi.c
> index a16bd1407..f9ba153f6 100644
> --- a/drivers/iio/accel/adxl355_spi.c
> +++ b/drivers/iio/accel/adxl355_spi.c
> @@ -34,7 +34,7 @@ static int adxl355_spi_probe(struct spi_device *spi)
>  		return PTR_ERR(regmap);
>  	}
>  
> -	return adxl355_core_probe(&spi->dev, regmap, id->name);
> +	return adxl355_core_probe(&spi->dev, regmap, id->name, spi->irq);
>  }
>  
>  static const struct spi_device_id adxl355_spi_id[] = {

Re: [PATCH v8 3/3] iio: accel: adxl355: Add triggered buffer support

[Jonathan Cameron]

On Sun, 8 Aug 2021 16:29:36 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Wed,  4 Aug 2021 19:33:09 +0530
> Puranjay Mohan <puranjay12@gmail.com> wrote:
> 
> > Provide a way for continuous data capture by setting up buffer support. The
> > data ready signal exposed at the DRDY pin of the ADXL355 is exploited as
> > a hardware interrupt which triggers to fill the buffer.
> > 
> > Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>  
> 
> Hi Puranjay,
> 
> This device has a hardware fifo, so we need to be careful how we add
> trigger support such that we can enable that smoothly.
Oops. I meant to come back and add some detail on this after taking a look
at the datasheet.

The device has an explicit data ready pin to indicate 'any data' not using
the fifo.  That's helpful for this mode.  I note that there is a way of
turning it off which is good and you should ensure it is off, except when
the trigger is enabled so that we aren't dealing with a stream of spurious
interrupts.

I would encourage you to also get the fifo mode up and running before sending
a new version of this patch.  It may well be the case that there is little
point in supporting this mode as it's just a special case of fifo mode with
the watermark set to 1.

> 
> There are a few things in here that suggest this isn't tested as much
> as I'd expect for a submission.  If you are posting early, but need
> to do more testing, please state that clearly.
> 
> Jonathan
> 
> > ---
> >  drivers/iio/accel/Kconfig        |   4 ++
> >  drivers/iio/accel/adxl355.h      |   2 +-
> >  drivers/iio/accel/adxl355_core.c | 102 ++++++++++++++++++++++++++++++-
> >  drivers/iio/accel/adxl355_i2c.c  |   3 +-
> >  drivers/iio/accel/adxl355_spi.c  |   2 +-
> >  5 files changed, 108 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> > index d0c45c809..9c16c1841 100644
> > --- a/drivers/iio/accel/Kconfig
> > +++ b/drivers/iio/accel/Kconfig
> > @@ -69,6 +69,8 @@ config ADXL355_I2C
> >  	depends on I2C
> >  	select ADXL355
> >  	select REGMAP_I2C
> > +	select IIO_BUFFER
> > +	select IIO_TRIGGERED_BUFFER
> >  	help
> >  	  Say Y here if you want to build i2c support for the Analog Devices
> >  	  ADXL355 3-axis digital accelerometer.
> > @@ -82,6 +84,8 @@ config ADXL355_SPI
> >  	depends on SPI
> >  	select ADXL355
> >  	select REGMAP_SPI
> > +	select IIO_BUFFER
> > +	select IIO_TRIGGERED_BUFFER
> >  	help
> >  	  Say Y here if you want to build spi support for the Analog Devices
> >  	  ADXL355 3-axis digital accelerometer.
> > diff --git a/drivers/iio/accel/adxl355.h b/drivers/iio/accel/adxl355.h
> > index 322b0abb8..f0a376e6d 100644
> > --- a/drivers/iio/accel/adxl355.h
> > +++ b/drivers/iio/accel/adxl355.h
> > @@ -15,5 +15,5 @@ extern const struct regmap_access_table adxl355_readable_regs_tbl;
> >  extern const struct regmap_access_table adxl355_writeable_regs_tbl;
> >  
> >  int adxl355_core_probe(struct device *dev, struct regmap *regmap,
> > -		       const char *name);
> > +		       const char *name, int irq);
> >  #endif /* _ADXL355_H_ */
> > diff --git a/drivers/iio/accel/adxl355_core.c b/drivers/iio/accel/adxl355_core.c
> > index 47fbb31bc..f9dc2a530 100644
> > --- a/drivers/iio/accel/adxl355_core.c
> > +++ b/drivers/iio/accel/adxl355_core.c
> > @@ -9,6 +9,10 @@
> >  
> >  #include <asm/unaligned.h>
> >  #include <linux/bitfield.h>
> > +#include <linux/iio/buffer.h>
> > +#include <linux/iio/trigger.h>
> > +#include <linux/iio/triggered_buffer.h>
> > +#include <linux/iio/trigger_consumer.h>
> >  #include <linux/iio/iio.h>
> >  #include <linux/limits.h>
> >  #include <linux/math64.h>
> > @@ -172,6 +176,7 @@ static const struct adxl355_chan_info adxl355_chans[] = {
> >  };
> >  
> >  struct adxl355_data {
> > +	int irq;
> >  	struct regmap *regmap;
> >  	struct device *dev;
> >  	struct mutex lock; /* lock to protect op_mode */
> > @@ -181,6 +186,12 @@ struct adxl355_data {
> >  	int calibbias[3];
> >  	int adxl355_hpf_3db_table[7][2];
> >  	u8 transf_buf[3] ____cacheline_aligned;
> > +	struct iio_trigger      *dready_trig;
> > +	/* Ensure correct alignment of timestamp when present */
> > +	struct {
> > +		__be32 channels[3];
> > +		s64 ts;
> > +	} buffer ____cacheline_aligned;  
> 
> Interesting quirk here is that we need to ensure that any of the
> buffers used for DMA don't share a cacheline with other data, but we don't
> need to ensure they don't share with each other.  As such, this can
> share fine with transf_buf, but definitely not with dready_trig (which
> can't share with transf_buf like it is right now).
> 
> So reorder to put the various DMA safe buffers at the end, and ensure
> the first one is cacheline aligned.
> 
> Even better, given you can only safely access one at a time anyway
> (and I think you always hold the lock so that is fine), put them in a union
> to make it explicit that only one is used at a time.
> 
> struct iio_trigger *dready_trig;
> union {
> 	u8 trans_buf[3];
> 	struct {
> 		__be32 channels[3];
> 		s64 ts;
> 	} buffer;
> } ___cacheline_algned;
> or something like that.
> 
> 
> 
> >  };
> >  
> >  static int adxl355_set_op_mode(struct adxl355_data *data,
> > @@ -499,12 +510,46 @@ static int adxl355_read_avail(struct iio_dev *indio_dev,
> >  	}
> >  }
> >  
> > +static const unsigned long adxl355_avail_scan_masks[] = {
> > +	GENMASK(3, 0),
> > +	0
> > +};
> > +
> >  static const struct iio_info adxl355_info = {
> >  	.read_raw	= adxl355_read_raw,
> >  	.write_raw	= adxl355_write_raw,
> >  	.read_avail	= &adxl355_read_avail
> >  };
> >  
> > +static const struct iio_trigger_ops adxl355_trigger_ops = {
> > +	.validate_device = &iio_trigger_validate_own_device,  
> 
> No means of turning it on?
> 
> > +};
> > +
> > +static irqreturn_t adxl355_trigger_handler(int irq, void *p)
> > +{
> > +	struct iio_poll_func *pf = p;
> > +	struct iio_dev *indio_dev = pf->indio_dev;
> > +	struct adxl355_data *data = iio_priv(indio_dev);
> > +	int ret;
> > +
> > +	mutex_lock(&data->lock);
> > +
> > +	ret = regmap_bulk_read(data->regmap, ADXL355_XDATA3_REG,
> > +			       data->buffer.channels,
> > +			       9);  
> 
> That doesn't look right.  you are reading 9 bytes into an array of 3 __be32 channels.
> You should check the data is turning up where it should.
> 
> 
> > +	if (ret)
> > +		goto out_unlock_notify;
> > +
> > +	iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer,
> > +					   pf->timestamp);
> > +
> > +out_unlock_notify:
> > +	mutex_unlock(&data->lock);
> > +	iio_trigger_notify_done(indio_dev->trig);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> >  #define ADXL355_ACCEL_CHANNEL(index, reg, axis) {			\
> >  	.type = IIO_ACCEL,						\
> >  	.address = reg,							\
> > @@ -518,6 +563,7 @@ static const struct iio_info adxl355_info = {
> >  	.info_mask_shared_by_type_available =				\
> >  		BIT(IIO_CHAN_INFO_SAMP_FREQ) |				\
> >  		BIT(IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY),	\
> > +	.scan_index = index,						\
> >  	.scan_type = {							\
> >  		.sign = 's',						\
> >  		.realbits = 20,						\
> > @@ -537,17 +583,56 @@ static const struct iio_chan_spec adxl355_channels[] = {
> >  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> >  				      BIT(IIO_CHAN_INFO_SCALE) |
> >  				      BIT(IIO_CHAN_INFO_OFFSET),
> > +		.scan_index = 3,
> >  		.scan_type = {
> >  			.sign = 's',
> >  			.realbits = 12,
> >  			.storagebits = 16,
> >  			.endianness = IIO_BE,
> >  		},
> > -	}
> > +	},  
> 
> If you do end up respinning patch 2, add the comma in that patch as it'll reduced
> noise here.
> 
> > +	IIO_CHAN_SOFT_TIMESTAMP(4)
> >  };
> >  
> > +static int adxl355_probe_trigger(struct iio_dev *indio_dev)
> > +{
> > +	struct adxl355_data *data = iio_priv(indio_dev);
> > +	int ret;
> > +
> > +	if (!data->irq) {
> > +		dev_info(data->dev, "no irq, using polling\n");
> > +		return 0;
> > +	}
> > +
> > +	data->dready_trig = devm_iio_trigger_alloc(data->dev, "%s-dev%d",
> > +						   indio_dev->name,
> > +						   indio_dev->id);
> > +	if (!data->dready_trig)
> > +		return -ENOMEM;
> > +
> > +	data->dready_trig->ops = &adxl355_trigger_ops;
> > +	iio_trigger_set_drvdata(data->dready_trig, indio_dev);
> > +
> > +	ret = devm_request_irq(data->dev, data->irq,
> > +			       &iio_trigger_generic_data_rdy_poll,
> > +			       IRQF_ONESHOT, "adxl355_irq", data->dready_trig);
> > +	if (ret < 0)
> > +		return dev_err_probe(data->dev, ret, "request irq %d failed\n",
> > +				     data->irq);
> > +
> > +	ret = devm_iio_trigger_register(data->dev, data->dready_trig);
> > +	if (ret) {
> > +		dev_err(data->dev, "iio trigger register failed\n");
> > +		return ret;
> > +	}
> > +
> > +	indio_dev->trig = iio_trigger_get(data->dready_trig);
> > +
> > +	return 0;
> > +}
> > +
> >  int adxl355_core_probe(struct device *dev, struct regmap *regmap,
> > -		       const char *name)
> > +		       const char *name, int irq)
> >  {
> >  	struct adxl355_data *data;
> >  	struct iio_dev *indio_dev;
> > @@ -560,6 +645,7 @@ int adxl355_core_probe(struct device *dev, struct regmap *regmap,
> >  	data = iio_priv(indio_dev);
> >  	data->regmap = regmap;
> >  	data->dev = dev;
> > +	data->irq = irq;
> >  	data->op_mode = ADXL355_STANDBY;
> >  	mutex_init(&data->lock);
> >  
> > @@ -568,6 +654,7 @@ int adxl355_core_probe(struct device *dev, struct regmap *regmap,
> >  	indio_dev->modes = INDIO_DIRECT_MODE;
> >  	indio_dev->channels = adxl355_channels;
> >  	indio_dev->num_channels = ARRAY_SIZE(adxl355_channels);
> > +	indio_dev->available_scan_masks = adxl355_avail_scan_masks;
> >  
> >  	ret = adxl355_setup(data);
> >  	if (ret < 0) {
> > @@ -575,6 +662,17 @@ int adxl355_core_probe(struct device *dev, struct regmap *regmap,
> >  		return ret;
> >  	}
> >  
> > +	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
> > +					      &iio_pollfunc_store_time,
> > +					      &adxl355_trigger_handler, NULL);
> > +	if (ret < 0)  
> 
> if (ret) preferred, because it is more consistent with returning devm_iio_device_register()
> below (where we are assuming negative or 0 only)
> 
> > +		return dev_err_probe(dev, ret,
> > +				     "iio triggered buffer setup failed\n");
> > +
> > +	ret = adxl355_probe_trigger(indio_dev);
> > +	if (ret < 0)
> > +		return ret;
> > +
> >  	return devm_iio_device_register(dev, indio_dev);
> >  }
> >  EXPORT_SYMBOL_GPL(adxl355_core_probe);
> > diff --git a/drivers/iio/accel/adxl355_i2c.c b/drivers/iio/accel/adxl355_i2c.c
> > index e3070ee81..c18521819 100644
> > --- a/drivers/iio/accel/adxl355_i2c.c
> > +++ b/drivers/iio/accel/adxl355_i2c.c
> > @@ -31,7 +31,8 @@ static int adxl355_i2c_probe(struct i2c_client *client)
> >  		return PTR_ERR(regmap);
> >  	}
> >  
> > -	return adxl355_core_probe(&client->dev, regmap, client->name);
> > +	return adxl355_core_probe(&client->dev, regmap, client->name,
> > +				  client->irq);  
> 
> We can't just use the first interrupt (which this corresponds to) because
> the device supports 3 and who knows if this is the right one?
> 
> Now to solve this, we run into a 'fun' issue.  We want to use the generic
> firmware property interfaces (include/linux/property.h) but that doesn't
> include a fwnode_irq_get_by_name()
> 
> We've had some recent discussion about this:
> https://lore.kernel.org/linux-iio/20210724190614.3593dcd1@jic23-huawei/
> 
> If it is something you would like to take on, that would be great.
> If not, for now use the of_irq_get_byname() method but add a comment
> saying it would good to move to a generic version.
> 
> >  }
> >  
> >  static const struct i2c_device_id adxl355_i2c_id[] = {
> > diff --git a/drivers/iio/accel/adxl355_spi.c b/drivers/iio/accel/adxl355_spi.c
> > index a16bd1407..f9ba153f6 100644
> > --- a/drivers/iio/accel/adxl355_spi.c
> > +++ b/drivers/iio/accel/adxl355_spi.c
> > @@ -34,7 +34,7 @@ static int adxl355_spi_probe(struct spi_device *spi)
> >  		return PTR_ERR(regmap);
> >  	}
> >  
> > -	return adxl355_core_probe(&spi->dev, regmap, id->name);
> > +	return adxl355_core_probe(&spi->dev, regmap, id->name, spi->irq);
> >  }
> >  
> >  static const struct spi_device_id adxl355_spi_id[] = {  
> 

Re: [PATCH v8 3/3] iio: accel: adxl355: Add triggered buffer support

[Puranjay Mohan]

On Sun, Aug 8, 2021 at 9:03 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Sun, 8 Aug 2021 16:29:36 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
>
> > On Wed,  4 Aug 2021 19:33:09 +0530
> > Puranjay Mohan <puranjay12@gmail.com> wrote:
> >
> > > Provide a way for continuous data capture by setting up buffer support. The
> > > data ready signal exposed at the DRDY pin of the ADXL355 is exploited as
> > > a hardware interrupt which triggers to fill the buffer.
> > >
> > > Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
> >
> > Hi Puranjay,
> >
> > This device has a hardware fifo, so we need to be careful how we add
> > trigger support such that we can enable that smoothly.
> Oops. I meant to come back and add some detail on this after taking a look
> at the datasheet.
>
> The device has an explicit data ready pin to indicate 'any data' not using
> the fifo.  That's helpful for this mode.  I note that there is a way of
> turning it off which is good and you should ensure it is off, except when
> the trigger is enabled so that we aren't dealing with a stream of spurious
> interrupts.

I have added a function in trigger_ops to enable this, and I have
turned this off in the setup.
>
> I would encourage you to also get the fifo mode up and running before sending
> a new version of this patch.  It may well be the case that there is little
> point in supporting this mode as it's just a special case of fifo mode with
> the watermark set to 1.
I am short on time now, will have to do the fifo later.
>
> >
> > There are a few things in here that suggest this isn't tested as much
> > as I'd expect for a submission.  If you are posting early, but need
> > to do more testing, please state that clearly.
> >
> > Jonathan
> >
> > > ---
> > >  drivers/iio/accel/Kconfig        |   4 ++
> > >  drivers/iio/accel/adxl355.h      |   2 +-
> > >  drivers/iio/accel/adxl355_core.c | 102 ++++++++++++++++++++++++++++++-
> > >  drivers/iio/accel/adxl355_i2c.c  |   3 +-
> > >  drivers/iio/accel/adxl355_spi.c  |   2 +-
> > >  5 files changed, 108 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> > > index d0c45c809..9c16c1841 100644
> > > --- a/drivers/iio/accel/Kconfig
> > > +++ b/drivers/iio/accel/Kconfig
> > > @@ -69,6 +69,8 @@ config ADXL355_I2C
> > >     depends on I2C
> > >     select ADXL355
> > >     select REGMAP_I2C
> > > +   select IIO_BUFFER
> > > +   select IIO_TRIGGERED_BUFFER
> > >     help
> > >       Say Y here if you want to build i2c support for the Analog Devices
> > >       ADXL355 3-axis digital accelerometer.
> > > @@ -82,6 +84,8 @@ config ADXL355_SPI
> > >     depends on SPI
> > >     select ADXL355
> > >     select REGMAP_SPI
> > > +   select IIO_BUFFER
> > > +   select IIO_TRIGGERED_BUFFER
> > >     help
> > >       Say Y here if you want to build spi support for the Analog Devices
> > >       ADXL355 3-axis digital accelerometer.
> > > diff --git a/drivers/iio/accel/adxl355.h b/drivers/iio/accel/adxl355.h
> > > index 322b0abb8..f0a376e6d 100644
> > > --- a/drivers/iio/accel/adxl355.h
> > > +++ b/drivers/iio/accel/adxl355.h
> > > @@ -15,5 +15,5 @@ extern const struct regmap_access_table adxl355_readable_regs_tbl;
> > >  extern const struct regmap_access_table adxl355_writeable_regs_tbl;
> > >
> > >  int adxl355_core_probe(struct device *dev, struct regmap *regmap,
> > > -                  const char *name);
> > > +                  const char *name, int irq);
> > >  #endif /* _ADXL355_H_ */
> > > diff --git a/drivers/iio/accel/adxl355_core.c b/drivers/iio/accel/adxl355_core.c
> > > index 47fbb31bc..f9dc2a530 100644
> > > --- a/drivers/iio/accel/adxl355_core.c
> > > +++ b/drivers/iio/accel/adxl355_core.c
> > > @@ -9,6 +9,10 @@
> > >
> > >  #include <asm/unaligned.h>
> > >  #include <linux/bitfield.h>
> > > +#include <linux/iio/buffer.h>
> > > +#include <linux/iio/trigger.h>
> > > +#include <linux/iio/triggered_buffer.h>
> > > +#include <linux/iio/trigger_consumer.h>
> > >  #include <linux/iio/iio.h>
> > >  #include <linux/limits.h>
> > >  #include <linux/math64.h>
> > > @@ -172,6 +176,7 @@ static const struct adxl355_chan_info adxl355_chans[] = {
> > >  };
> > >
> > >  struct adxl355_data {
> > > +   int irq;
> > >     struct regmap *regmap;
> > >     struct device *dev;
> > >     struct mutex lock; /* lock to protect op_mode */
> > > @@ -181,6 +186,12 @@ struct adxl355_data {
> > >     int calibbias[3];
> > >     int adxl355_hpf_3db_table[7][2];
> > >     u8 transf_buf[3] ____cacheline_aligned;
> > > +   struct iio_trigger      *dready_trig;
> > > +   /* Ensure correct alignment of timestamp when present */
> > > +   struct {
> > > +           __be32 channels[3];
> > > +           s64 ts;
> > > +   } buffer ____cacheline_aligned;
> >
> > Interesting quirk here is that we need to ensure that any of the
> > buffers used for DMA don't share a cacheline with other data, but we don't
> > need to ensure they don't share with each other.  As such, this can
> > share fine with transf_buf, but definitely not with dready_trig (which
> > can't share with transf_buf like it is right now).
> >
> > So reorder to put the various DMA safe buffers at the end, and ensure
> > the first one is cacheline aligned.
> >
> > Even better, given you can only safely access one at a time anyway
> > (and I think you always hold the lock so that is fine), put them in a union
> > to make it explicit that only one is used at a time.
> >
> > struct iio_trigger *dready_trig;
> > union {
> >       u8 trans_buf[3];
> >       struct {
> >               __be32 channels[3];
> >               s64 ts;
> >       } buffer;
> > } ___cacheline_algned;
> > or something like that.
Thanks for suggesting this snippet, it works perfectly.
> >
> >
> >
> > >  };
> > >
> > >  static int adxl355_set_op_mode(struct adxl355_data *data,
> > > @@ -499,12 +510,46 @@ static int adxl355_read_avail(struct iio_dev *indio_dev,
> > >     }
> > >  }
> > >
> > > +static const unsigned long adxl355_avail_scan_masks[] = {
> > > +   GENMASK(3, 0),
> > > +   0
> > > +};
> > > +
> > >  static const struct iio_info adxl355_info = {
> > >     .read_raw       = adxl355_read_raw,
> > >     .write_raw      = adxl355_write_raw,
> > >     .read_avail     = &adxl355_read_avail
> > >  };
> > >
> > > +static const struct iio_trigger_ops adxl355_trigger_ops = {
> > > +   .validate_device = &iio_trigger_validate_own_device,
> >
> > No means of turning it on?
added a set state function.
> >
> > > +};
> > > +
> > > +static irqreturn_t adxl355_trigger_handler(int irq, void *p)
> > > +{
> > > +   struct iio_poll_func *pf = p;
> > > +   struct iio_dev *indio_dev = pf->indio_dev;
> > > +   struct adxl355_data *data = iio_priv(indio_dev);
> > > +   int ret;
> > > +
> > > +   mutex_lock(&data->lock);
> > > +
> > > +   ret = regmap_bulk_read(data->regmap, ADXL355_XDATA3_REG,
> > > +                          data->buffer.channels,
> > > +                          9);
> >
> > That doesn't look right.  you are reading 9 bytes into an array of 3 __be32 channels.
> > You should check the data is turning up where it should.
I have fixed this in the next version,
> >
> >
> > > +   if (ret)
> > > +           goto out_unlock_notify;
> > > +
> > > +   iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer,
> > > +                                      pf->timestamp);
> > > +
> > > +out_unlock_notify:
> > > +   mutex_unlock(&data->lock);
> > > +   iio_trigger_notify_done(indio_dev->trig);
> > > +
> > > +   return IRQ_HANDLED;
> > > +}
> > > +
> > >  #define ADXL355_ACCEL_CHANNEL(index, reg, axis) {                  \
> > >     .type = IIO_ACCEL,                                              \
> > >     .address = reg,                                                 \
> > > @@ -518,6 +563,7 @@ static const struct iio_info adxl355_info = {
> > >     .info_mask_shared_by_type_available =                           \
> > >             BIT(IIO_CHAN_INFO_SAMP_FREQ) |                          \
> > >             BIT(IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY),      \
> > > +   .scan_index = index,                                            \
> > >     .scan_type = {                                                  \
> > >             .sign = 's',                                            \
> > >             .realbits = 20,                                         \
> > > @@ -537,17 +583,56 @@ static const struct iio_chan_spec adxl355_channels[] = {
> > >             .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > >                                   BIT(IIO_CHAN_INFO_SCALE) |
> > >                                   BIT(IIO_CHAN_INFO_OFFSET),
> > > +           .scan_index = 3,
> > >             .scan_type = {
> > >                     .sign = 's',
> > >                     .realbits = 12,
> > >                     .storagebits = 16,
> > >                     .endianness = IIO_BE,
> > >             },
> > > -   }
> > > +   },
> >
> > If you do end up respinning patch 2, add the comma in that patch as it'll reduced
> > noise here.
> >
> > > +   IIO_CHAN_SOFT_TIMESTAMP(4)
> > >  };
> > >
> > > +static int adxl355_probe_trigger(struct iio_dev *indio_dev)
> > > +{
> > > +   struct adxl355_data *data = iio_priv(indio_dev);
> > > +   int ret;
> > > +
> > > +   if (!data->irq) {
> > > +           dev_info(data->dev, "no irq, using polling\n");
> > > +           return 0;
> > > +   }
> > > +
> > > +   data->dready_trig = devm_iio_trigger_alloc(data->dev, "%s-dev%d",
> > > +                                              indio_dev->name,
> > > +                                              indio_dev->id);
> > > +   if (!data->dready_trig)
> > > +           return -ENOMEM;
> > > +
> > > +   data->dready_trig->ops = &adxl355_trigger_ops;
> > > +   iio_trigger_set_drvdata(data->dready_trig, indio_dev);
> > > +
> > > +   ret = devm_request_irq(data->dev, data->irq,
> > > +                          &iio_trigger_generic_data_rdy_poll,
> > > +                          IRQF_ONESHOT, "adxl355_irq", data->dready_trig);
> > > +   if (ret < 0)
> > > +           return dev_err_probe(data->dev, ret, "request irq %d failed\n",
> > > +                                data->irq);
> > > +
> > > +   ret = devm_iio_trigger_register(data->dev, data->dready_trig);
> > > +   if (ret) {
> > > +           dev_err(data->dev, "iio trigger register failed\n");
> > > +           return ret;
> > > +   }
> > > +
> > > +   indio_dev->trig = iio_trigger_get(data->dready_trig);
> > > +
> > > +   return 0;
> > > +}
> > > +
> > >  int adxl355_core_probe(struct device *dev, struct regmap *regmap,
> > > -                  const char *name)
> > > +                  const char *name, int irq)
> > >  {
> > >     struct adxl355_data *data;
> > >     struct iio_dev *indio_dev;
> > > @@ -560,6 +645,7 @@ int adxl355_core_probe(struct device *dev, struct regmap *regmap,
> > >     data = iio_priv(indio_dev);
> > >     data->regmap = regmap;
> > >     data->dev = dev;
> > > +   data->irq = irq;
> > >     data->op_mode = ADXL355_STANDBY;
> > >     mutex_init(&data->lock);
> > >
> > > @@ -568,6 +654,7 @@ int adxl355_core_probe(struct device *dev, struct regmap *regmap,
> > >     indio_dev->modes = INDIO_DIRECT_MODE;
> > >     indio_dev->channels = adxl355_channels;
> > >     indio_dev->num_channels = ARRAY_SIZE(adxl355_channels);
> > > +   indio_dev->available_scan_masks = adxl355_avail_scan_masks;
> > >
> > >     ret = adxl355_setup(data);
> > >     if (ret < 0) {
> > > @@ -575,6 +662,17 @@ int adxl355_core_probe(struct device *dev, struct regmap *regmap,
> > >             return ret;
> > >     }
> > >
> > > +   ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
> > > +                                         &iio_pollfunc_store_time,
> > > +                                         &adxl355_trigger_handler, NULL);
> > > +   if (ret < 0)
> >
> > if (ret) preferred, because it is more consistent with returning devm_iio_device_register()
> > below (where we are assuming negative or 0 only)
> >
> > > +           return dev_err_probe(dev, ret,
> > > +                                "iio triggered buffer setup failed\n");
> > > +
> > > +   ret = adxl355_probe_trigger(indio_dev);
> > > +   if (ret < 0)
> > > +           return ret;
> > > +
> > >     return devm_iio_device_register(dev, indio_dev);
> > >  }
> > >  EXPORT_SYMBOL_GPL(adxl355_core_probe);
> > > diff --git a/drivers/iio/accel/adxl355_i2c.c b/drivers/iio/accel/adxl355_i2c.c
> > > index e3070ee81..c18521819 100644
> > > --- a/drivers/iio/accel/adxl355_i2c.c
> > > +++ b/drivers/iio/accel/adxl355_i2c.c
> > > @@ -31,7 +31,8 @@ static int adxl355_i2c_probe(struct i2c_client *client)
> > >             return PTR_ERR(regmap);
> > >     }
> > >
> > > -   return adxl355_core_probe(&client->dev, regmap, client->name);
> > > +   return adxl355_core_probe(&client->dev, regmap, client->name,
> > > +                             client->irq);
> >
> > We can't just use the first interrupt (which this corresponds to) because
> > the device supports 3 and who knows if this is the right one?
> >
> > Now to solve this, we run into a 'fun' issue.  We want to use the generic
> > firmware property interfaces (include/linux/property.h) but that doesn't
> > include a fwnode_irq_get_by_name()
> >
> > We've had some recent discussion about this:
> > https://lore.kernel.org/linux-iio/20210724190614.3593dcd1@jic23-huawei/
> >
> > If it is something you would like to take on, that would be great.
> > If not, for now use the of_irq_get_byname() method but add a comment
> > saying it would good to move to a generic version.
I really want to work on fwnode_irq_get_by_name() but I am short on
time, I will work on it next month.
this would be a nice project for me, btw.
> >
> > >  }
> > >
> > >  static const struct i2c_device_id adxl355_i2c_id[] = {
> > > diff --git a/drivers/iio/accel/adxl355_spi.c b/drivers/iio/accel/adxl355_spi.c
> > > index a16bd1407..f9ba153f6 100644
> > > --- a/drivers/iio/accel/adxl355_spi.c
> > > +++ b/drivers/iio/accel/adxl355_spi.c
> > > @@ -34,7 +34,7 @@ static int adxl355_spi_probe(struct spi_device *spi)
> > >             return PTR_ERR(regmap);
> > >     }
> > >
> > > -   return adxl355_core_probe(&spi->dev, regmap, id->name);
> > > +   return adxl355_core_probe(&spi->dev, regmap, id->name, spi->irq);
> > >  }
> > >
> > >  static const struct spi_device_id adxl355_spi_id[] = {
> >
>


-- 
Thanks and Regards

Yours Truly,

Puranjay Mohan