[PATCH 0/10] iio: accel: BMA220 improvements

[PATCH 0/10] iio: accel: BMA220 improvements

[Petre Rodan]

Series of patches that switch the driver to the regmap API, add i2c
connectivity, tap/low-g/high-g/slope event detection and a large number
of sysfs mappings while keeping backward compatibility.

Tested in I2C and SPI modes with two different sensors.

Petre Rodan (10):
  dt-bindings: iio: accel: bosch,BMA220 improvements
  iio: accel: BMA220 split original spi driver
  iio: accel: BMA220 migrate to regmap SPI
  iio: accel: BMA220 add i2c module
  iio: accel: BMA220 make use of the watchdog functionality
  iio: accel: BMA220 add LPF cut-off frequency mapping
  iio: accel: BMA220 add debugfs reg access
  iio: accel: BMA220 add events
  iio: accel: BMA220 add event attrs
  iio: accel: BMA220 add maintainer

 .../bindings/iio/accel/bosch,bma220.yaml      |   20 +-
 MAINTAINERS                                   |    7 +
 drivers/iio/accel/Kconfig                     |   18 +-
 drivers/iio/accel/Makefile                    |    4 +-
 drivers/iio/accel/bma220.h                    |   21 +
 drivers/iio/accel/bma220_core.c               | 1551 +++++++++++++++++
 drivers/iio/accel/bma220_i2c.c                |   62 +
 drivers/iio/accel/bma220_spi.c                |  313 +---
 8 files changed, 1702 insertions(+), 294 deletions(-)
 create mode 100644 drivers/iio/accel/bma220.h
 create mode 100644 drivers/iio/accel/bma220_core.c
 create mode 100644 drivers/iio/accel/bma220_i2c.c

--
2.49.1

[PATCH 01/10] dt-bindings: iio: accel: bosch,BMA220 improvements

[Petre Rodan]

 - fix title typo
 - add optional watchdog setting that recovers the sensor from a stuck-low
 SDA condition
 - set correct SPI phase and polarity
 - interrupt on rising edge. the level-based interrupt that is being
 replaced was not actually implemented in the driver.

This set of changes should not negatively affect existing users.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
 .../bindings/iio/accel/bosch,bma220.yaml      | 20 +++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml
index ec643de031a3..f71b2320b010 100644
--- a/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml
+++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/iio/accel/bosch,bma220.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#

-title: Bosch BMA220 Trixial Acceleration Sensor
+title: Bosch BMA220 Triaxial Acceleration Sensor

 maintainers:
   - Jonathan Cameron <Jonathan.Cameron@huawei.com>
@@ -20,6 +20,20 @@ properties:
   interrupts:
     maxItems: 1

+  bosch,watchdog:
+    description:
+      In order to prevent the built-in I2C slave to lock-up the I2C bus, a
+      watchdog timer is introduced. The WDT observes internal I2C signals and
+      resets the I2C interface if the bus is locked-up by the BMA220.
+      0 - off
+      1 - 1ms
+      2 - 10ms
+    enum: [0, 1, 2]
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  spi-cpha: true
+  spi-cpol: true
+
   vdda-supply: true
   vddd-supply: true
   vddio-supply: true
@@ -44,8 +58,10 @@ examples:
             compatible = "bosch,bma220";
             reg = <0>;
             spi-max-frequency = <2500000>;
+            spi-cpol;
+            spi-cpha;
             interrupt-parent = <&gpio0>;
-            interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+            interrupts = <0 IRQ_TYPE_EDGE_RISING>;
         };
     };
 ...
--
2.49.1

Re: [PATCH 01/10] dt-bindings: iio: accel: bosch,BMA220 improvements

[Krzysztof Kozlowski]

On 01/09/2025 21:47, Petre Rodan wrote:
>  - fix title typo
>  - add optional watchdog setting that recovers the sensor from a stuck-low
>  SDA condition
>  - set correct SPI phase and polarity
>  - interrupt on rising edge. the level-based interrupt that is being

Cleanup and new features must never be mixed together.

<form letter>
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.

You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time.

Please kindly resend and include all necessary To/Cc entries.
</form letter>


Best regards,
Krzysztof

Re: [PATCH 01/10] dt-bindings: iio: accel: bosch,BMA220 improvements

[Petre Rodan]

[-- Attachment #1: Type: text/plain, Size: 1460 bytes --]


hello,

On Tue, Sep 02, 2025 at 07:57:03AM +0200, Krzysztof Kozlowski wrote:
> <form letter>
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC. It might happen, that command when run on an older
> kernel, gives you outdated entries. Therefore please be sure you base
> your patches on recent Linux kernel.

I'm using the bleeding edge togreg branch of the iio tree, git pulled yesterday.
I indeed missed devicetree@ while manually copy-pasting from get_maintainer.pl on the bindings patch. I wish that script would provide a valid rfc822 email header instead of it's current verbose output.

> You missed at least devicetree list (maybe more), so this won't be
> tested by automated tooling. Performing review on untested code might be
> a waste of time.

the patch set had gone thru multiple static scans, unit tests, checkpatch.pl and the dt_binding_check on my end.
since devicetree@ is not emailed with driver code, I will still wait for any feedback to the driver part of my submission, I guess.

> >  - fix title typo
> >  - add optional watchdog setting that recovers the sensor from a stuck-low
> >  SDA condition
> >  - set correct SPI phase and polarity
> >  - interrupt on rising edge. the level-based interrupt that is being
> 
> Cleanup and new features must never be mixed together.

ok I will split up all the bindings changes in the next revision.

best regards.
peter

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 01/10] dt-bindings: iio: accel: bosch,BMA220 improvements

[David Lechner]

On 9/2/25 11:02 AM, Petre Rodan wrote:
> 
> hello,
> 
> On Tue, Sep 02, 2025 at 07:57:03AM +0200, Krzysztof Kozlowski wrote:
>> <form letter>
>> Please use scripts/get_maintainers.pl to get a list of necessary people
>> and lists to CC. It might happen, that command when run on an older
>> kernel, gives you outdated entries. Therefore please be sure you base
>> your patches on recent Linux kernel.
> 
> I'm using the bleeding edge togreg branch of the iio tree, git pulled yesterday.
> I indeed missed devicetree@ while manually copy-pasting from get_maintainer.pl on the bindings patch. I wish that script would provide a valid rfc822 email header instead of it's current verbose output.
> 
IIRC, the --no-rolestats option will fix that.

Suggest to use b4 or other tools to automate this.

Re: [PATCH 01/10] dt-bindings: iio: accel: bosch,BMA220 improvements

[Krzysztof Kozlowski]

On 02/09/2025 18:02, Petre Rodan wrote:
> 
> hello,
> 
> On Tue, Sep 02, 2025 at 07:57:03AM +0200, Krzysztof Kozlowski wrote:
>> <form letter>
>> Please use scripts/get_maintainers.pl to get a list of necessary people
>> and lists to CC. It might happen, that command when run on an older
>> kernel, gives you outdated entries. Therefore please be sure you base
>> your patches on recent Linux kernel.
> 
> I'm using the bleeding edge togreg branch of the iio tree, git pulled yesterday.
> I indeed missed devicetree@ while manually copy-pasting from get_maintainer.pl on the bindings patch. I wish that script would provide a valid rfc822 email header instead of it's current verbose output.

Recommended is to use b4. Simple wrapper like git_send_email() also
would work:
https://lore.kernel.org/all/?q=git_send_email


Best regards,
Krzysztof

Re: [PATCH 01/10] dt-bindings: iio: accel: bosch,BMA220 improvements

[David Lechner]

On 9/1/25 2:47 PM, Petre Rodan wrote:
>  - fix title typo
>  - add optional watchdog setting that recovers the sensor from a stuck-low
>  SDA condition
>  - set correct SPI phase and polarity
>  - interrupt on rising edge. the level-based interrupt that is being
>  replaced was not actually implemented in the driver.
> 
> This set of changes should not negatively affect existing users.
> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> ---
>  .../bindings/iio/accel/bosch,bma220.yaml      | 20 +++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml
> index ec643de031a3..f71b2320b010 100644
> --- a/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml
> +++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml
> @@ -4,7 +4,7 @@
>  $id: http://devicetree.org/schemas/iio/accel/bosch,bma220.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
> 
> -title: Bosch BMA220 Trixial Acceleration Sensor
> +title: Bosch BMA220 Triaxial Acceleration Sensor
> 
>  maintainers:
>    - Jonathan Cameron <Jonathan.Cameron@huawei.com>
> @@ -20,6 +20,20 @@ properties:
>    interrupts:
>      maxItems: 1
> 
> +  bosch,watchdog:
> +    description:
> +      In order to prevent the built-in I2C slave to lock-up the I2C bus, a
> +      watchdog timer is introduced. The WDT observes internal I2C signals and
> +      resets the I2C interface if the bus is locked-up by the BMA220.
> +      0 - off
> +      1 - 1ms
> +      2 - 10ms
> +    enum: [0, 1, 2]
> +    $ref: /schemas/types.yaml#/definitions/uint32

Why should this depend on how the chip is wired up? Normally, we don't have this
sort of control in devicetree. E.g. if it is useful, why shouldn't drivers just
always enable it?

If we can make the case that it belongs in the devicetree, it should use
standard units, e.g. property should be watchdog-timeout-ms with enum: [1, 10].
Maybe 0 for disabled is OK too - in that case should have default: 0.

> +
> +  spi-cpha: true
> +  spi-cpol: true
> +
>    vdda-supply: true
>    vddd-supply: true
>    vddio-supply: true
> @@ -44,8 +58,10 @@ examples:
>              compatible = "bosch,bma220";
>              reg = <0>;
>              spi-max-frequency = <2500000>;
> +            spi-cpol;
> +            spi-cpha;
>              interrupt-parent = <&gpio0>;
> -            interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> +            interrupts = <0 IRQ_TYPE_EDGE_RISING>;
>          };
>      };
>  ...
> --
> 2.49.1
> 

Re: [PATCH 01/10] dt-bindings: iio: accel: bosch,BMA220 improvements

[Petre Rodan]

Good morning.

Thank you for your feedback.

On Fri, Sep 05, 2025 at 03:15:55PM -0500, David Lechner wrote:
> On 9/1/25 2:47 PM, Petre Rodan wrote:
> > diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml

[..]

> > +  bosch,watchdog:
> > +    description:
> > +      In order to prevent the built-in I2C slave to lock-up the I2C bus, a
> > +      watchdog timer is introduced. The WDT observes internal I2C signals and
> > +      resets the I2C interface if the bus is locked-up by the BMA220.
> > +      0 - off
> > +      1 - 1ms
> > +      2 - 10ms
> > +    enum: [0, 1, 2]
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> 
> Why should this depend on how the chip is wired up? Normally, we don't have this
> sort of control in devicetree.

I was also unsure on how it would be best to implement the feature, bellow is my thought process.

The feature itself is definitely required for the i2c implementation of this chip. I have witnessed it pull sda low for no good reason twice over a 100h period and this would render not only the chip but the entire bus unusable until a power cycle.

I think from a driver perspective ideally WDT should be set very early - within bma220_common_probe() would be ideal.

> E.g. if it is useful, why shouldn't drivers just always enable it?

The registers holding the watchdog are all zeroed out after power on which mean it's off. I think the driver should also default on this setting. In my first implementation I had it hard-wired to 1ms, but I felt this would impose my point of view on users and it would be nicer to give them control over it.

If you guys think that the devicetree is not the place where the WDT should be set that is fine by me, would you recommend something like module_param() instead?

> If we can make the case that it belongs in the devicetree, it should use
> standard units, e.g. property should be watchdog-timeout-ms with enum: [1, 10].
> Maybe 0 for disabled is OK too - in that case should have default: 0.

Oh yes I can see it in bq256xx.yaml, to me this sounds absolutely perfect.


On a different note, from a reviewer's perspective would you prefer the next revision of this patch series to cover less ground? I was thinking about leaving everything event related for later since I might go past 15 separate patches if I split every modification into it's own separate entry.

thank you again,
peter

-- 
petre rodan

Re: [PATCH 01/10] dt-bindings: iio: accel: bosch,BMA220 improvements

[David Lechner]

On 9/5/25 9:46 PM, Petre Rodan wrote:
> 
> Good morning.
> 
> Thank you for your feedback.
> 
> On Fri, Sep 05, 2025 at 03:15:55PM -0500, David Lechner wrote:
>> On 9/1/25 2:47 PM, Petre Rodan wrote:
>>> diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml
> 
> [..]
> 
>>> +  bosch,watchdog:
>>> +    description:
>>> +      In order to prevent the built-in I2C slave to lock-up the I2C bus, a
>>> +      watchdog timer is introduced. The WDT observes internal I2C signals and
>>> +      resets the I2C interface if the bus is locked-up by the BMA220.
>>> +      0 - off
>>> +      1 - 1ms
>>> +      2 - 10ms
>>> +    enum: [0, 1, 2]
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>
>> Why should this depend on how the chip is wired up? Normally, we don't have this
>> sort of control in devicetree.
> 
> I was also unsure on how it would be best to implement the feature, bellow is my thought process.
> 
> The feature itself is definitely required for the i2c implementation of this chip. I have witnessed it pull sda low for no good reason twice over a 100h period and this would render not only the chip but the entire bus unusable until a power cycle.
> 
> I think from a driver perspective ideally WDT should be set very early - within bma220_common_probe() would be ideal.
> 
>> E.g. if it is useful, why shouldn't drivers just always enable it?
> 
> The registers holding the watchdog are all zeroed out after power on which mean it's off. I think the driver should also default on this setting. In my first implementation I had it hard-wired to 1ms, but I felt this would impose my point of view on users and it would be nicer to give them control over it.
> 
> If you guys think that the devicetree is not the place where the WDT should be set that is fine by me, would you recommend something like module_param() instead?

I wish I had a good answer, but I don't have the right kind of
experience with this sort of thing to know what works best.

We could start with just always enabling it and if we find it
actually does cause some problem for someone, then we would have
more information about that use case and could make a more
informed decision on how to handle it at that point in time.

> 
>> If we can make the case that it belongs in the devicetree, it should use
>> standard units, e.g. property should be watchdog-timeout-ms with enum: [1, 10].
>> Maybe 0 for disabled is OK too - in that case should have default: 0.
> 
> Oh yes I can see it in bq256xx.yaml, to me this sounds absolutely perfect.
> 
> 
> On a different note, from a reviewer's perspective would you prefer the next revision of this patch series to cover less ground? I was thinking about leaving everything event related for later since I might go past 15 separate patches if I split every modification into it's own separate entry.

Yes, smaller series will get more thorough reviews so I'm always
in favor of splitting things up like that.

> 
> thank you again,
> peter
> 

[PATCH 02/10] iio: accel: BMA220 split original spi driver

[Petre Rodan]

Split original driver from bma220_spi.c into bma220_core.c and bma220.h
with a minimal number of changes in preparation for the next patches.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
 drivers/iio/accel/Kconfig       |   9 +-
 drivers/iio/accel/Makefile      |   3 +-
 drivers/iio/accel/bma220.h      |  17 ++
 drivers/iio/accel/bma220_core.c | 310 ++++++++++++++++++++++++++++++++
 drivers/iio/accel/bma220_spi.c  | 309 ++-----------------------------
 5 files changed, 354 insertions(+), 294 deletions(-)
 create mode 100644 drivers/iio/accel/bma220.h
 create mode 100644 drivers/iio/accel/bma220_core.c

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 8c3f7cf55d5f..2cc3075e2688 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -218,15 +218,20 @@ config BMA180

 config BMA220
 	tristate "Bosch BMA220 3-Axis Accelerometer Driver"
-	depends on SPI
 	select IIO_BUFFER
 	select IIO_TRIGGERED_BUFFER
+	select BMA220_SPI if SPI
 	help
 	  Say yes here to add support for the Bosch BMA220 triaxial
 	  acceleration sensor.

 	  To compile this driver as a module, choose M here: the
-	  module will be called bma220_spi.
+	  module will be called bma220_core and you will also get
+	  bma220_spi if SPI is enabled.
+
+config BMA220_SPI
+	tristate
+	depends on BMA220

 config BMA400
 	tristate "Bosch BMA400 3-Axis Accelerometer Driver"
diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
index ca8569e25aba..56a9f848f7f9 100644
--- a/drivers/iio/accel/Makefile
+++ b/drivers/iio/accel/Makefile
@@ -25,7 +25,8 @@ obj-$(CONFIG_ADXL380) += adxl380.o
 obj-$(CONFIG_ADXL380_I2C) += adxl380_i2c.o
 obj-$(CONFIG_ADXL380_SPI) += adxl380_spi.o
 obj-$(CONFIG_BMA180) += bma180.o
-obj-$(CONFIG_BMA220) += bma220_spi.o
+obj-$(CONFIG_BMA220) += bma220_core.o
+obj-$(CONFIG_BMA220_SPI) += bma220_spi.o
 obj-$(CONFIG_BMA400) += bma400_core.o
 obj-$(CONFIG_BMA400_I2C) += bma400_i2c.o
 obj-$(CONFIG_BMA400_SPI) += bma400_spi.o
diff --git a/drivers/iio/accel/bma220.h b/drivers/iio/accel/bma220.h
new file mode 100644
index 000000000000..0606cf478f5f
--- /dev/null
+++ b/drivers/iio/accel/bma220.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Forward declarations needed by the bma220 sources.
+ *
+ * Copyright 2025 Petre Rodan <petre.rodan@subdimension.ro>
+ */
+
+#ifndef _BMA220_H
+#define _BMA220_H
+
+#include <linux/iio/iio.h>
+
+extern const struct dev_pm_ops bma220_pm_ops;
+
+int bma220_common_probe(struct spi_device *dev);
+
+#endif
diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
new file mode 100644
index 000000000000..60fd35637d2d
--- /dev/null
+++ b/drivers/iio/accel/bma220_core.c
@@ -0,0 +1,310 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * BMA220 Digital triaxial acceleration sensor driver
+ *
+ * Copyright (c) 2016,2020 Intel Corporation.
+ */
+
+#include <linux/bits.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/spi/spi.h>
+
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+
+#define BMA220_REG_ID				0x00
+#define BMA220_REG_ACCEL_X			0x02
+#define BMA220_REG_ACCEL_Y			0x03
+#define BMA220_REG_ACCEL_Z			0x04
+#define BMA220_REG_RANGE			0x11
+#define BMA220_REG_SUSPEND			0x18
+
+#define BMA220_CHIP_ID				0xDD
+#define BMA220_READ_MASK			BIT(7)
+#define BMA220_RANGE_MASK			GENMASK(1, 0)
+#define BMA220_SUSPEND_SLEEP			0xFF
+#define BMA220_SUSPEND_WAKE			0x00
+
+#define BMA220_DEVICE_NAME			"bma220"
+
+#define BMA220_ACCEL_CHANNEL(index, reg, axis) {			\
+	.type = IIO_ACCEL,						\
+	.address = reg,							\
+	.modified = 1,							\
+	.channel2 = IIO_MOD_##axis,					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
+	.scan_index = index,						\
+	.scan_type = {							\
+		.sign = 's',						\
+		.realbits = 6,						\
+		.storagebits = 8,					\
+		.shift = 2,						\
+		.endianness = IIO_CPU,					\
+	},								\
+}
+
+enum bma220_axis {
+	AXIS_X,
+	AXIS_Y,
+	AXIS_Z,
+};
+
+static const int bma220_scale_table[][2] = {
+	{0, 623000}, {1, 248000}, {2, 491000}, {4, 983000},
+};
+
+struct bma220_data {
+	struct spi_device *spi_device;
+	struct mutex lock;
+	struct {
+		s8 chans[3];
+		/* Ensure timestamp is naturally aligned. */
+		aligned_s64 timestamp;
+	} scan;
+	u8 tx_buf[2] __aligned(IIO_DMA_MINALIGN);
+};
+
+static const struct iio_chan_spec bma220_channels[] = {
+	BMA220_ACCEL_CHANNEL(0, BMA220_REG_ACCEL_X, X),
+	BMA220_ACCEL_CHANNEL(1, BMA220_REG_ACCEL_Y, Y),
+	BMA220_ACCEL_CHANNEL(2, BMA220_REG_ACCEL_Z, Z),
+	IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
+static inline int bma220_read_reg(struct spi_device *spi, u8 reg)
+{
+	return spi_w8r8(spi, reg | BMA220_READ_MASK);
+}
+
+static const unsigned long bma220_accel_scan_masks[] = {
+	BIT(AXIS_X) | BIT(AXIS_Y) | BIT(AXIS_Z),
+	0
+};
+
+static irqreturn_t bma220_trigger_handler(int irq, void *p)
+{
+	int ret;
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct bma220_data *data = iio_priv(indio_dev);
+	struct spi_device *spi = data->spi_device;
+
+	mutex_lock(&data->lock);
+	data->tx_buf[0] = BMA220_REG_ACCEL_X | BMA220_READ_MASK;
+	ret = spi_write_then_read(spi, data->tx_buf, 1, &data->scan.chans,
+				  ARRAY_SIZE(bma220_channels) - 1);
+	if (ret < 0)
+		goto err;
+
+	iio_push_to_buffers_with_ts(indio_dev, &data->scan, sizeof(data->scan),
+				    pf->timestamp);
+err:
+	mutex_unlock(&data->lock);
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+static int bma220_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	int ret;
+	u8 range_idx;
+	struct bma220_data *data = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = bma220_read_reg(data->spi_device, chan->address);
+		if (ret < 0)
+			return -EINVAL;
+		*val = sign_extend32(ret >> chan->scan_type.shift,
+				     chan->scan_type.realbits - 1);
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		ret = bma220_read_reg(data->spi_device, BMA220_REG_RANGE);
+		if (ret < 0)
+			return ret;
+		range_idx = ret & BMA220_RANGE_MASK;
+		*val = bma220_scale_table[range_idx][0];
+		*val2 = bma220_scale_table[range_idx][1];
+		return IIO_VAL_INT_PLUS_MICRO;
+	}
+
+	return -EINVAL;
+}
+
+static int bma220_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
+{
+	int i;
+	int ret;
+	int index = -1;
+	struct bma220_data *data = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		for (i = 0; i < ARRAY_SIZE(bma220_scale_table); i++)
+			if (val == bma220_scale_table[i][0] &&
+			    val2 == bma220_scale_table[i][1]) {
+				index = i;
+				break;
+			}
+		if (index < 0)
+			return -EINVAL;
+
+		mutex_lock(&data->lock);
+		data->tx_buf[0] = BMA220_REG_RANGE;
+		data->tx_buf[1] = index;
+		ret = spi_write(data->spi_device, data->tx_buf,
+				sizeof(data->tx_buf));
+		if (ret < 0)
+			dev_err(&data->spi_device->dev,
+				"failed to set measurement range\n");
+		mutex_unlock(&data->lock);
+
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int bma220_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     const int **vals, int *type, int *length,
+			     long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		*vals = (int *)bma220_scale_table;
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		*length = ARRAY_SIZE(bma220_scale_table) * 2;
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info bma220_info = {
+	.read_raw		= bma220_read_raw,
+	.write_raw		= bma220_write_raw,
+	.read_avail		= bma220_read_avail,
+};
+
+static int bma220_init(struct spi_device *spi)
+{
+	int ret;
+
+	ret = bma220_read_reg(spi, BMA220_REG_ID);
+	if (ret != BMA220_CHIP_ID)
+		return -ENODEV;
+
+	/* Make sure the chip is powered on */
+	ret = bma220_read_reg(spi, BMA220_REG_SUSPEND);
+	if (ret == BMA220_SUSPEND_WAKE)
+		ret = bma220_read_reg(spi, BMA220_REG_SUSPEND);
+	if (ret < 0)
+		return ret;
+	if (ret == BMA220_SUSPEND_WAKE)
+		return -EBUSY;
+
+	return 0;
+}
+
+static int bma220_power(struct spi_device *spi, bool up)
+{
+	int i, ret;
+
+	/**
+	 * The chip can be suspended/woken up by a simple register read.
+	 * So, we need up to 2 register reads of the suspend register
+	 * to make sure that the device is in the desired state.
+	 */
+	for (i = 0; i < 2; i++) {
+		ret = bma220_read_reg(spi, BMA220_REG_SUSPEND);
+		if (ret < 0)
+			return ret;
+
+		if (up && ret == BMA220_SUSPEND_SLEEP)
+			return 0;
+
+		if (!up && ret == BMA220_SUSPEND_WAKE)
+			return 0;
+	}
+
+	return -EBUSY;
+}
+
+static void bma220_deinit(void *spi)
+{
+	bma220_power(spi, false);
+}
+
+int bma220_common_probe(struct spi_device *spi)
+{
+	int ret;
+	struct iio_dev *indio_dev;
+	struct bma220_data *data;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	data->spi_device = spi;
+	mutex_init(&data->lock);
+
+	indio_dev->info = &bma220_info;
+	indio_dev->name = BMA220_DEVICE_NAME;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = bma220_channels;
+	indio_dev->num_channels = ARRAY_SIZE(bma220_channels);
+	indio_dev->available_scan_masks = bma220_accel_scan_masks;
+
+	ret = bma220_init(data->spi_device);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(&spi->dev, bma220_deinit, spi);
+	if (ret)
+		return ret;
+
+	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
+					      iio_pollfunc_store_time,
+					      bma220_trigger_handler, NULL);
+	if (ret < 0) {
+		dev_err(&spi->dev, "iio triggered buffer setup failed\n");
+		return ret;
+	}
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+EXPORT_SYMBOL_NS(bma220_common_probe, "IIO_BOSCH_BMA220");
+
+static int bma220_suspend(struct device *dev)
+{
+	struct spi_device *spi = to_spi_device(dev);
+
+	return bma220_power(spi, false);
+}
+
+static int bma220_resume(struct device *dev)
+{
+	struct spi_device *spi = to_spi_device(dev);
+
+	return bma220_power(spi, true);
+}
+EXPORT_NS_SIMPLE_DEV_PM_OPS(bma220_pm_ops, bma220_suspend, bma220_resume,
+			    IIO_BOSCH_BMA220);
+
+MODULE_AUTHOR("Tiberiu Breana <tiberiu.a.breana@intel.com>");
+MODULE_DESCRIPTION("BMA220 acceleration sensor driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/iio/accel/bma220_spi.c b/drivers/iio/accel/bma220_spi.c
index 01592eebf05b..be8348ad0a93 100644
--- a/drivers/iio/accel/bma220_spi.c
+++ b/drivers/iio/accel/bma220_spi.c
@@ -5,8 +5,8 @@
  * Copyright (c) 2016,2020 Intel Corporation.
  */

-#include <linux/bits.h>
-#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/errno.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/types.h>
@@ -14,295 +14,14 @@

 #include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>
-#include <linux/iio/sysfs.h>
-#include <linux/iio/trigger_consumer.h>
-#include <linux/iio/triggered_buffer.h>

-#define BMA220_REG_ID				0x00
-#define BMA220_REG_ACCEL_X			0x02
-#define BMA220_REG_ACCEL_Y			0x03
-#define BMA220_REG_ACCEL_Z			0x04
-#define BMA220_REG_RANGE			0x11
-#define BMA220_REG_SUSPEND			0x18
+#include "bma220.h"

-#define BMA220_CHIP_ID				0xDD
-#define BMA220_READ_MASK			BIT(7)
-#define BMA220_RANGE_MASK			GENMASK(1, 0)
-#define BMA220_SUSPEND_SLEEP			0xFF
-#define BMA220_SUSPEND_WAKE			0x00
-
-#define BMA220_DEVICE_NAME			"bma220"
-
-#define BMA220_ACCEL_CHANNEL(index, reg, axis) {			\
-	.type = IIO_ACCEL,						\
-	.address = reg,							\
-	.modified = 1,							\
-	.channel2 = IIO_MOD_##axis,					\
-	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
-	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
-	.scan_index = index,						\
-	.scan_type = {							\
-		.sign = 's',						\
-		.realbits = 6,						\
-		.storagebits = 8,					\
-		.shift = 2,						\
-		.endianness = IIO_CPU,					\
-	},								\
-}
-
-enum bma220_axis {
-	AXIS_X,
-	AXIS_Y,
-	AXIS_Z,
-};
-
-static const int bma220_scale_table[][2] = {
-	{0, 623000}, {1, 248000}, {2, 491000}, {4, 983000},
-};
-
-struct bma220_data {
-	struct spi_device *spi_device;
-	struct mutex lock;
-	struct {
-		s8 chans[3];
-		/* Ensure timestamp is naturally aligned. */
-		aligned_s64 timestamp;
-	} scan;
-	u8 tx_buf[2] __aligned(IIO_DMA_MINALIGN);
-};
-
-static const struct iio_chan_spec bma220_channels[] = {
-	BMA220_ACCEL_CHANNEL(0, BMA220_REG_ACCEL_X, X),
-	BMA220_ACCEL_CHANNEL(1, BMA220_REG_ACCEL_Y, Y),
-	BMA220_ACCEL_CHANNEL(2, BMA220_REG_ACCEL_Z, Z),
-	IIO_CHAN_SOFT_TIMESTAMP(3),
-};
-
-static inline int bma220_read_reg(struct spi_device *spi, u8 reg)
-{
-	return spi_w8r8(spi, reg | BMA220_READ_MASK);
-}
-
-static const unsigned long bma220_accel_scan_masks[] = {
-	BIT(AXIS_X) | BIT(AXIS_Y) | BIT(AXIS_Z),
-	0
-};
-
-static irqreturn_t bma220_trigger_handler(int irq, void *p)
-{
-	int ret;
-	struct iio_poll_func *pf = p;
-	struct iio_dev *indio_dev = pf->indio_dev;
-	struct bma220_data *data = iio_priv(indio_dev);
-	struct spi_device *spi = data->spi_device;
-
-	mutex_lock(&data->lock);
-	data->tx_buf[0] = BMA220_REG_ACCEL_X | BMA220_READ_MASK;
-	ret = spi_write_then_read(spi, data->tx_buf, 1, &data->scan.chans,
-				  ARRAY_SIZE(bma220_channels) - 1);
-	if (ret < 0)
-		goto err;
-
-	iio_push_to_buffers_with_ts(indio_dev, &data->scan, sizeof(data->scan),
-				    pf->timestamp);
-err:
-	mutex_unlock(&data->lock);
-	iio_trigger_notify_done(indio_dev->trig);
-
-	return IRQ_HANDLED;
-}
-
-static int bma220_read_raw(struct iio_dev *indio_dev,
-			   struct iio_chan_spec const *chan,
-			   int *val, int *val2, long mask)
-{
-	int ret;
-	u8 range_idx;
-	struct bma220_data *data = iio_priv(indio_dev);
-
-	switch (mask) {
-	case IIO_CHAN_INFO_RAW:
-		ret = bma220_read_reg(data->spi_device, chan->address);
-		if (ret < 0)
-			return -EINVAL;
-		*val = sign_extend32(ret >> chan->scan_type.shift,
-				     chan->scan_type.realbits - 1);
-		return IIO_VAL_INT;
-	case IIO_CHAN_INFO_SCALE:
-		ret = bma220_read_reg(data->spi_device, BMA220_REG_RANGE);
-		if (ret < 0)
-			return ret;
-		range_idx = ret & BMA220_RANGE_MASK;
-		*val = bma220_scale_table[range_idx][0];
-		*val2 = bma220_scale_table[range_idx][1];
-		return IIO_VAL_INT_PLUS_MICRO;
-	}
-
-	return -EINVAL;
-}
-
-static int bma220_write_raw(struct iio_dev *indio_dev,
-			    struct iio_chan_spec const *chan,
-			    int val, int val2, long mask)
-{
-	int i;
-	int ret;
-	int index = -1;
-	struct bma220_data *data = iio_priv(indio_dev);
-
-	switch (mask) {
-	case IIO_CHAN_INFO_SCALE:
-		for (i = 0; i < ARRAY_SIZE(bma220_scale_table); i++)
-			if (val == bma220_scale_table[i][0] &&
-			    val2 == bma220_scale_table[i][1]) {
-				index = i;
-				break;
-			}
-		if (index < 0)
-			return -EINVAL;
-
-		mutex_lock(&data->lock);
-		data->tx_buf[0] = BMA220_REG_RANGE;
-		data->tx_buf[1] = index;
-		ret = spi_write(data->spi_device, data->tx_buf,
-				sizeof(data->tx_buf));
-		if (ret < 0)
-			dev_err(&data->spi_device->dev,
-				"failed to set measurement range\n");
-		mutex_unlock(&data->lock);
-
-		return 0;
-	}
-
-	return -EINVAL;
-}
-
-static int bma220_read_avail(struct iio_dev *indio_dev,
-			     struct iio_chan_spec const *chan,
-			     const int **vals, int *type, int *length,
-			     long mask)
-{
-	switch (mask) {
-	case IIO_CHAN_INFO_SCALE:
-		*vals = (int *)bma220_scale_table;
-		*type = IIO_VAL_INT_PLUS_MICRO;
-		*length = ARRAY_SIZE(bma220_scale_table) * 2;
-		return IIO_AVAIL_LIST;
-	default:
-		return -EINVAL;
-	}
-}
-
-static const struct iio_info bma220_info = {
-	.read_raw		= bma220_read_raw,
-	.write_raw		= bma220_write_raw,
-	.read_avail		= bma220_read_avail,
-};
-
-static int bma220_init(struct spi_device *spi)
+static int bma220_spi_probe(struct spi_device *spi)
 {
-	int ret;
-
-	ret = bma220_read_reg(spi, BMA220_REG_ID);
-	if (ret != BMA220_CHIP_ID)
-		return -ENODEV;
-
-	/* Make sure the chip is powered on */
-	ret = bma220_read_reg(spi, BMA220_REG_SUSPEND);
-	if (ret == BMA220_SUSPEND_WAKE)
-		ret = bma220_read_reg(spi, BMA220_REG_SUSPEND);
-	if (ret < 0)
-		return ret;
-	if (ret == BMA220_SUSPEND_WAKE)
-		return -EBUSY;
-
-	return 0;
+	return bma220_common_probe(spi);
 }

-static int bma220_power(struct spi_device *spi, bool up)
-{
-	int i, ret;
-
-	/**
-	 * The chip can be suspended/woken up by a simple register read.
-	 * So, we need up to 2 register reads of the suspend register
-	 * to make sure that the device is in the desired state.
-	 */
-	for (i = 0; i < 2; i++) {
-		ret = bma220_read_reg(spi, BMA220_REG_SUSPEND);
-		if (ret < 0)
-			return ret;
-
-		if (up && ret == BMA220_SUSPEND_SLEEP)
-			return 0;
-
-		if (!up && ret == BMA220_SUSPEND_WAKE)
-			return 0;
-	}
-
-	return -EBUSY;
-}
-
-static void bma220_deinit(void *spi)
-{
-	bma220_power(spi, false);
-}
-
-static int bma220_probe(struct spi_device *spi)
-{
-	int ret;
-	struct iio_dev *indio_dev;
-	struct bma220_data *data;
-
-	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*data));
-	if (!indio_dev)
-		return -ENOMEM;
-
-	data = iio_priv(indio_dev);
-	data->spi_device = spi;
-	mutex_init(&data->lock);
-
-	indio_dev->info = &bma220_info;
-	indio_dev->name = BMA220_DEVICE_NAME;
-	indio_dev->modes = INDIO_DIRECT_MODE;
-	indio_dev->channels = bma220_channels;
-	indio_dev->num_channels = ARRAY_SIZE(bma220_channels);
-	indio_dev->available_scan_masks = bma220_accel_scan_masks;
-
-	ret = bma220_init(data->spi_device);
-	if (ret)
-		return ret;
-
-	ret = devm_add_action_or_reset(&spi->dev, bma220_deinit, spi);
-	if (ret)
-		return ret;
-
-	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
-					      iio_pollfunc_store_time,
-					      bma220_trigger_handler, NULL);
-	if (ret < 0) {
-		dev_err(&spi->dev, "iio triggered buffer setup failed\n");
-		return ret;
-	}
-
-	return devm_iio_device_register(&spi->dev, indio_dev);
-}
-
-static int bma220_suspend(struct device *dev)
-{
-	struct spi_device *spi = to_spi_device(dev);
-
-	return bma220_power(spi, false);
-}
-
-static int bma220_resume(struct device *dev)
-{
-	struct spi_device *spi = to_spi_device(dev);
-
-	return bma220_power(spi, true);
-}
-static DEFINE_SIMPLE_DEV_PM_OPS(bma220_pm_ops, bma220_suspend, bma220_resume);
-
 static const struct spi_device_id bma220_spi_id[] = {
 	{"bma220", 0},
 	{ }
@@ -314,17 +33,25 @@ static const struct acpi_device_id bma220_acpi_id[] = {
 };
 MODULE_DEVICE_TABLE(spi, bma220_spi_id);

-static struct spi_driver bma220_driver = {
+static const struct of_device_id bma220_of_spi_match[] = {
+	{ .compatible = "bosch,bma220" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, bma220_of_spi_match);
+
+static struct spi_driver bma220_spi_driver = {
 	.driver = {
 		.name = "bma220_spi",
 		.pm = pm_sleep_ptr(&bma220_pm_ops),
+		.of_match_table = bma220_of_spi_match,
 		.acpi_match_table = bma220_acpi_id,
 	},
-	.probe =            bma220_probe,
+	.probe =            bma220_spi_probe,
 	.id_table =         bma220_spi_id,
 };
-module_spi_driver(bma220_driver);
+module_spi_driver(bma220_spi_driver);

 MODULE_AUTHOR("Tiberiu Breana <tiberiu.a.breana@intel.com>");
-MODULE_DESCRIPTION("BMA220 acceleration sensor driver");
-MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("BMA220 triaxial acceleration sensor spi driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("IIO_BOSCH_BMA220");
--
2.49.1

Re: [PATCH 02/10] iio: accel: BMA220 split original spi driver

[Jonathan Cameron]

On Mon,  1 Sep 2025 22:47:28 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> Split original driver from bma220_spi.c into bma220_core.c and bma220.h
> with a minimal number of changes in preparation for the next patches.
> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
Hi Petre,

A few comments inline.

Jonathan

> diff --git a/drivers/iio/accel/bma220.h b/drivers/iio/accel/bma220.h
> new file mode 100644
> index 000000000000..0606cf478f5f
> --- /dev/null
> +++ b/drivers/iio/accel/bma220.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Forward declarations needed by the bma220 sources.
> + *
> + * Copyright 2025 Petre Rodan <petre.rodan@subdimension.ro>
> + */
> +
> +#ifndef _BMA220_H
> +#define _BMA220_H
> +
> +#include <linux/iio/iio.h>
Not used so far so don't include it in this header.
> +
> +extern const struct dev_pm_ops bma220_pm_ops;

Probably want a header for that, though I'm not 100% sure if needed
for an extern or not...

> +

struct spi_device;
> +int bma220_common_probe(struct spi_device *dev);
> +
> +#endif
> diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
> new file mode 100644
> index 000000000000..60fd35637d2d
> --- /dev/null
> +++ b/drivers/iio/accel/bma220_core.c
> @@ -0,0 +1,310 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * BMA220 Digital triaxial acceleration sensor driver
> + *
> + * Copyright (c) 2016,2020 Intel Corporation.
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/kernel.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/types.h>
> +#include <linux/spi/spi.h>
> +
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>

Would expect to see this including the new header.

> +
>  static const struct spi_device_id bma220_spi_id[] = {
>  	{"bma220", 0},
>  	{ }
> @@ -314,17 +33,25 @@ static const struct acpi_device_id bma220_acpi_id[] = {
>  };
>  MODULE_DEVICE_TABLE(spi, bma220_spi_id);
> 
> -static struct spi_driver bma220_driver = {
> +static const struct of_device_id bma220_of_spi_match[] = {

This looks like an unrelated change. Do this in a separate patch.

> +	{ .compatible = "bosch,bma220" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, bma220_of_spi_match);
> +
> +static struct spi_driver bma220_spi_driver = {
>  	.driver = {
>  		.name = "bma220_spi",
>  		.pm = pm_sleep_ptr(&bma220_pm_ops),
> +		.of_match_table = bma220_of_spi_match,
>  		.acpi_match_table = bma220_acpi_id,
>  	},
> -	.probe =            bma220_probe,
> +	.probe =            bma220_spi_probe,
>  	.id_table =         bma220_spi_id,
>  };
> -module_spi_driver(bma220_driver);
> +module_spi_driver(bma220_spi_driver);
> 
>  MODULE_AUTHOR("Tiberiu Breana <tiberiu.a.breana@intel.com>");
> -MODULE_DESCRIPTION("BMA220 acceleration sensor driver");
> -MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("BMA220 triaxial acceleration sensor spi driver");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("IIO_BOSCH_BMA220");
> --
> 2.49.1
> 
> 

[PATCH 03/10] iio: accel: BMA220 migrate to regmap API

[Petre Rodan]

Switch to regmap API.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
 drivers/iio/accel/Kconfig       |   2 +
 drivers/iio/accel/bma220.h      |   5 +-
 drivers/iio/accel/bma220_core.c | 417 ++++++++++++++++++++++++++------
 drivers/iio/accel/bma220_spi.c  |  12 +-
 4 files changed, 354 insertions(+), 82 deletions(-)

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 2cc3075e2688..9b6c35b75948 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -218,6 +218,7 @@ config BMA180

 config BMA220
 	tristate "Bosch BMA220 3-Axis Accelerometer Driver"
+	select REGMAP
 	select IIO_BUFFER
 	select IIO_TRIGGERED_BUFFER
 	select BMA220_SPI if SPI
@@ -231,6 +232,7 @@ config BMA220

 config BMA220_SPI
 	tristate
+	select REGMAP_SPI
 	depends on BMA220

 config BMA400
diff --git a/drivers/iio/accel/bma220.h b/drivers/iio/accel/bma220.h
index 0606cf478f5f..5eefa9749d33 100644
--- a/drivers/iio/accel/bma220.h
+++ b/drivers/iio/accel/bma220.h
@@ -8,10 +8,13 @@
 #ifndef _BMA220_H
 #define _BMA220_H

+#include <linux/regmap.h>
+
 #include <linux/iio/iio.h>

+extern const struct regmap_config bma220_spi_regmap_config;
 extern const struct dev_pm_ops bma220_pm_ops;

-int bma220_common_probe(struct spi_device *dev);
+int bma220_common_probe(struct device *dev, struct regmap *regmap, int irq);

 #endif
diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index 60fd35637d2d..e6dac2e1cf4d 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -3,31 +3,133 @@
  * BMA220 Digital triaxial acceleration sensor driver
  *
  * Copyright (c) 2016,2020 Intel Corporation.
+ * Copyright (c) 2025 Petre Rodan  <petre.rodan@subdimension.ro>
  */

 #include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/cleanup.h>
+#include <linux/device.h>
 #include <linux/kernel.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
 #include <linux/types.h>
-#include <linux/spi/spi.h>

-#include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/events.h>
 #include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/triggered_buffer.h>

+#include "bma220.h"
+
+/*
+ * Read-Only Registers
+ */
+
+/* ID registers */
 #define BMA220_REG_ID				0x00
+#define BMA220_REG_REVISION_ID			0x01
+
+/* Acceleration registers */
 #define BMA220_REG_ACCEL_X			0x02
 #define BMA220_REG_ACCEL_Y			0x03
 #define BMA220_REG_ACCEL_Z			0x04
+
+/*
+ * Read-write configuration registers
+ */
+#define BMA220_REG_CONF0			0x05
+#define BMA220_HIGH_DUR_MSK			GENMASK(5, 0)
+#define BMA220_HIGH_HY_MSK			GENMASK(7, 6)
+#define BMA220_REG_CONF1			0x06
+#define BMA220_HIGH_TH_MSK			GENMASK(3, 0)
+#define BMA220_LOW_TH_MSK			GENMASK(7, 4)
+#define BMA220_REG_CONF2			0x07
+#define BMA220_LOW_DUR_MSK			GENMASK(5, 0)
+#define BMA220_LOW_HY_MSK			GENMASK(7, 6)
+#define BMA220_REG_CONF3			0x08
+#define BMA220_TT_DUR_MSK			GENMASK(2, 0)
+#define BMA220_TT_TH_MSK			GENMASK(6, 3)
+#define BMA220_TT_FILT_MSK			BIT(7)
+#define BMA220_REG_CONF4			0x09
+#define BMA220_SLOPE_DUR_MSK			GENMASK(1, 0)
+#define BMA220_SLOPE_TH_MSK			GENMASK(5, 2)
+#define BMA220_SLOPE_FILT_MSK			BIT(6)
+#define BMA220_ORIENT_EX_MSK			BIT(7)
+#define BMA220_REG_CONF5			0x0a
+#define BMA220_TT_SAMP_MSK			GENMASK(1, 0)
+#define BMA220_ORIENT_BLOCKING_MSK		GENMASK(3, 2)
+#define BMA220_TIP_EN_MSK			BIT(4)
+
+/*
+ * Read-only interrupt flags
+ */
+#define BMA220_REG_IF0				0x0b
+/* interrupt flags */
+#define BMA220_IF_HIGH_SIGN			BIT(0)
+#define BMA220_IF_HIGH_FIRST_Z			BIT(1)
+#define BMA220_IF_HIGH_FIRST_Y			BIT(2)
+#define BMA220_IF_HIGH_FIRST_X			BIT(3)
+#define BMA220_IF_ORIENT_INT			BIT(7)
+
+#define BMA220_REG_IF1				0x0c
+/* interrupt flags */
+#define BMA220_IF_SLOPE				BIT(0)
+#define BMA220_IF_DRDY				BIT(1)
+#define BMA220_IF_HIGH				BIT(2)
+#define BMA220_IF_LOW				BIT(3)
+#define BMA220_IF_TT				BIT(4)
+
+/*
+ * Read-write interrupt enable configuration registers
+ */
+#define BMA220_REG_IE0				0x0d
+#define BMA220_INT_EN_TAP_Z_MSK			BIT(0)
+#define BMA220_INT_EN_TAP_Y_MSK			BIT(1)
+#define BMA220_INT_EN_TAP_X_MSK			BIT(2)
+#define BMA220_INT_EN_SLOPE_Z_MSK		BIT(3)
+#define BMA220_INT_EN_SLOPE_Y_MSK		BIT(4)
+#define BMA220_INT_EN_SLOPE_X_MSK		BIT(5)
+#define BMA220_INT_EN_ORIENT_MSK		BIT(6)
+#define BMA220_INT_EN_DRDY_MSK			BIT(7)
+#define BMA220_REG_IE1				0x0e
+#define BMA220_INT_EN_HIGH_Z_MSK		BIT(0)
+#define BMA220_INT_EN_HIGH_Y_MSK		BIT(1)
+#define BMA220_INT_EN_HIGH_X_MSK		BIT(2)
+#define BMA220_INT_EN_LOW_MSK			BIT(3)
+#define BMA220_INT_LATCH_MSK			GENMASK(6, 4)
+#define BMA220_INT_LATCH_MAX			0x7
+#define BMA220_INT_RST_MSK			BIT(7)
+#define BMA220_INT_LATCH_LEN			8
+#define BMA220_REG_IE2				0x0f
+
+/*
+ * Read-write configuration registers
+ */
+#define BMA220_REG_FILTER			0x10
 #define BMA220_REG_RANGE			0x11
+#define BMA220_REG_WDT				0x17
+#define BMA220_WDT_MASK				GENMASK(2, 1)
+#define BMA220_WDT_OFF				0x0
+#define BMA220_WDT_1MS				BIT(1)
+#define BMA220_WDT_10MS				GENMASK(1, 0)
+/*
+ * Read-only state change registers
+ */
 #define BMA220_REG_SUSPEND			0x18
+#define BMA220_REG_SOFTRESET			0x19

 #define BMA220_CHIP_ID				0xDD
-#define BMA220_READ_MASK			BIT(7)
 #define BMA220_RANGE_MASK			GENMASK(1, 0)
+#define BMA220_FILTER_MASK			GENMASK(3, 0)
 #define BMA220_SUSPEND_SLEEP			0xFF
 #define BMA220_SUSPEND_WAKE			0x00

@@ -61,14 +163,16 @@ static const int bma220_scale_table[][2] = {
 };

 struct bma220_data {
-	struct spi_device *spi_device;
+	struct device *dev;
+	struct regmap *regmap;
 	struct mutex lock;
+	u8 range_idx;
+	struct iio_trigger *trig;
 	struct {
 		s8 chans[3];
 		/* Ensure timestamp is naturally aligned. */
 		aligned_s64 timestamp;
-	} scan;
-	u8 tx_buf[2] __aligned(IIO_DMA_MINALIGN);
+	} scan __aligned(IIO_DMA_MINALIGN);
 };

 static const struct iio_chan_spec bma220_channels[] = {
@@ -78,35 +182,81 @@ static const struct iio_chan_spec bma220_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(3),
 };

-static inline int bma220_read_reg(struct spi_device *spi, u8 reg)
-{
-	return spi_w8r8(spi, reg | BMA220_READ_MASK);
-}
-
 static const unsigned long bma220_accel_scan_masks[] = {
 	BIT(AXIS_X) | BIT(AXIS_Y) | BIT(AXIS_Z),
 	0
 };

+static bool bma220_is_writable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case BMA220_REG_CONF0:
+	case BMA220_REG_CONF1:
+	case BMA220_REG_CONF2:
+	case BMA220_REG_CONF3:
+	case BMA220_REG_CONF4:
+	case BMA220_REG_CONF5:
+	case BMA220_REG_IE0:
+	case BMA220_REG_IE1:
+	case BMA220_REG_IE2:
+	case BMA220_REG_FILTER:
+	case BMA220_REG_RANGE:
+	case BMA220_REG_WDT:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool bma220_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+	/* Don't cache any registers. */
+	return true;
+}
+
+const struct regmap_config bma220_spi_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.read_flag_mask = BIT(7),
+	.max_register = BMA220_REG_SOFTRESET,
+	.cache_type = REGCACHE_MAPLE,
+	.writeable_reg = bma220_is_writable_reg,
+	.volatile_reg = bma220_is_volatile_reg,
+};
+EXPORT_SYMBOL_NS(bma220_spi_regmap_config, "IIO_BOSCH_BMA220");
+
+static int bma220_data_rdy_trigger_set_state(struct iio_trigger *trig,
+					     bool state)
+{
+	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
+	struct bma220_data *data = iio_priv(indio_dev);
+
+	guard(mutex)(&data->lock);
+	return regmap_update_bits(data->regmap, BMA220_REG_IE0,
+				  BMA220_INT_EN_DRDY_MSK,
+				  FIELD_PREP(BMA220_INT_EN_DRDY_MSK, state));
+}
+
+static const struct iio_trigger_ops bma220_trigger_ops = {
+	.set_trigger_state = &bma220_data_rdy_trigger_set_state,
+	.validate_device = &iio_trigger_validate_own_device,
+};
+
 static irqreturn_t bma220_trigger_handler(int irq, void *p)
 {
 	int ret;
 	struct iio_poll_func *pf = p;
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct bma220_data *data = iio_priv(indio_dev);
-	struct spi_device *spi = data->spi_device;

-	mutex_lock(&data->lock);
-	data->tx_buf[0] = BMA220_REG_ACCEL_X | BMA220_READ_MASK;
-	ret = spi_write_then_read(spi, data->tx_buf, 1, &data->scan.chans,
-				  ARRAY_SIZE(bma220_channels) - 1);
+	ret = regmap_bulk_read(data->regmap, BMA220_REG_ACCEL_X,
+			       &data->scan.chans,
+			       sizeof(data->scan.chans));
 	if (ret < 0)
-		goto err;
+		return IRQ_NONE;

 	iio_push_to_buffers_with_ts(indio_dev, &data->scan, sizeof(data->scan),
-				    pf->timestamp);
-err:
-	mutex_unlock(&data->lock);
+				    iio_get_time_ns(indio_dev));
 	iio_trigger_notify_done(indio_dev->trig);

 	return IRQ_HANDLED;
@@ -117,59 +267,66 @@ static int bma220_read_raw(struct iio_dev *indio_dev,
 			   int *val, int *val2, long mask)
 {
 	int ret;
-	u8 range_idx;
+	u8 index;
+	unsigned int reg;
 	struct bma220_data *data = iio_priv(indio_dev);

 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		ret = bma220_read_reg(data->spi_device, chan->address);
+		ret = regmap_read(data->regmap, chan->address, &reg);
 		if (ret < 0)
 			return -EINVAL;
-		*val = sign_extend32(ret >> chan->scan_type.shift,
+		*val = sign_extend32(reg >> chan->scan_type.shift,
 				     chan->scan_type.realbits - 1);
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
-		ret = bma220_read_reg(data->spi_device, BMA220_REG_RANGE);
-		if (ret < 0)
-			return ret;
-		range_idx = ret & BMA220_RANGE_MASK;
-		*val = bma220_scale_table[range_idx][0];
-		*val2 = bma220_scale_table[range_idx][1];
+		index = data->range_idx;
+		*val = bma220_scale_table[index][0];
+		*val2 = bma220_scale_table[index][1];
 		return IIO_VAL_INT_PLUS_MICRO;
 	}

 	return -EINVAL;
 }

+static int bma220_find_match(const int (*tbl)[2], const int n,
+			       const int val, const int val2)
+{
+	int i;
+
+	for (i = 0; i < n; i++) {
+		if (tbl[i][0] == val && tbl[i][1] == val2)
+			return i;
+	}
+
+	return -EINVAL;
+}
+
 static int bma220_write_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan,
 			    int val, int val2, long mask)
 {
-	int i;
 	int ret;
 	int index = -1;
 	struct bma220_data *data = iio_priv(indio_dev);

+	guard(mutex)(&data->lock);
+
 	switch (mask) {
 	case IIO_CHAN_INFO_SCALE:
-		for (i = 0; i < ARRAY_SIZE(bma220_scale_table); i++)
-			if (val == bma220_scale_table[i][0] &&
-			    val2 == bma220_scale_table[i][1]) {
-				index = i;
-				break;
-			}
+		index = bma220_find_match(bma220_scale_table,
+					  ARRAY_SIZE(bma220_scale_table),
+					  val, val2);
 		if (index < 0)
 			return -EINVAL;

-		mutex_lock(&data->lock);
-		data->tx_buf[0] = BMA220_REG_RANGE;
-		data->tx_buf[1] = index;
-		ret = spi_write(data->spi_device, data->tx_buf,
-				sizeof(data->tx_buf));
+		ret = regmap_update_bits(data->regmap, BMA220_REG_RANGE,
+					 BMA220_RANGE_MASK,
+					 FIELD_PREP(BMA220_RANGE_MASK, index));
 		if (ret < 0)
-			dev_err(&data->spi_device->dev,
+			dev_err(data->dev,
 				"failed to set measurement range\n");
-		mutex_unlock(&data->lock);
+		data->range_idx = index;

 		return 0;
 	}
@@ -199,69 +356,150 @@ static const struct iio_info bma220_info = {
 	.read_avail		= bma220_read_avail,
 };

-static int bma220_init(struct spi_device *spi)
+static int bma220_reset(struct bma220_data *data, bool up)
 {
-	int ret;
+	int i, ret;
+	unsigned int val;

-	ret = bma220_read_reg(spi, BMA220_REG_ID);
-	if (ret != BMA220_CHIP_ID)
-		return -ENODEV;
+	guard(mutex)(&data->lock);

-	/* Make sure the chip is powered on */
-	ret = bma220_read_reg(spi, BMA220_REG_SUSPEND);
-	if (ret == BMA220_SUSPEND_WAKE)
-		ret = bma220_read_reg(spi, BMA220_REG_SUSPEND);
-	if (ret < 0)
-		return ret;
-	if (ret == BMA220_SUSPEND_WAKE)
-		return -EBUSY;
+	/**
+	 * The chip can be reset by a simple register read.
+	 * We need up to 2 register reads of the softreset register
+	 * to make sure that the device is in the desired state.
+	 */
+	for (i = 0; i < 2; i++) {
+		ret = regmap_read(data->regmap, BMA220_REG_SOFTRESET, &val);
+		if (ret < 0)
+			return ret;

-	return 0;
+		if (up && (val == BMA220_SUSPEND_SLEEP))
+			return 0;
+
+		if (!up && (val == BMA220_SUSPEND_WAKE))
+			return 0;
+	}
+
+	return -EBUSY;
 }

-static int bma220_power(struct spi_device *spi, bool up)
+static int bma220_power(struct bma220_data *data, bool up)
 {
 	int i, ret;
+	unsigned int val;

+	guard(mutex)(&data->lock);
 	/**
 	 * The chip can be suspended/woken up by a simple register read.
 	 * So, we need up to 2 register reads of the suspend register
 	 * to make sure that the device is in the desired state.
 	 */
 	for (i = 0; i < 2; i++) {
-		ret = bma220_read_reg(spi, BMA220_REG_SUSPEND);
+		ret = regmap_read(data->regmap, BMA220_REG_SUSPEND, &val);
 		if (ret < 0)
 			return ret;

-		if (up && ret == BMA220_SUSPEND_SLEEP)
+		if (up && (val == BMA220_SUSPEND_SLEEP))
 			return 0;

-		if (!up && ret == BMA220_SUSPEND_WAKE)
+		if (!up && (val == BMA220_SUSPEND_WAKE))
 			return 0;
 	}

 	return -EBUSY;
 }

-static void bma220_deinit(void *spi)
+static int bma220_init(struct bma220_data *data)
+{
+	int ret;
+	unsigned int val;
+	static const char * const regulator_names[] = { "vddd", "vddio", "vdda" };
+
+	ret = devm_regulator_bulk_get_enable(data->dev,
+					     ARRAY_SIZE(regulator_names),
+					     regulator_names);
+	if (ret)
+		return dev_err_probe(data->dev, ret, "Failed to get regulators\n");
+
+	/* Try to read chip_id register. It must return 0xdd. */
+	ret = regmap_read(data->regmap, BMA220_REG_ID, &val);
+	if (ret) {
+		dev_err(data->dev, "Failed to read chip id register\n");
+		return ret;
+	}
+
+	if (val != BMA220_CHIP_ID)
+		return -ENODEV;
+
+	ret = bma220_power(data, true);
+	if (ret) {
+		dev_err(data->dev, "Failed to power-on chip\n");
+		return ret;
+	}
+
+	ret = bma220_reset(data, true);
+	if (ret) {
+		dev_err(data->dev, "Failed to soft reset chip\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static void bma220_deinit(void *data_ptr)
 {
-	bma220_power(spi, false);
+	struct bma220_data *data = data_ptr;
+	int ret;
+
+	ret = bma220_power(data, false);
+	if (ret)
+		dev_warn(data->dev,
+			 "Failed to put device into suspend mode (%pe)\n",
+			 ERR_PTR(ret));
+}
+
+static irqreturn_t bma220_irq_handler(int irq, void *private)
+{
+	struct iio_dev *indio_dev = private;
+	struct bma220_data *data = iio_priv(indio_dev);
+	int rv;
+	u8 bma220_reg_if[2];
+
+	guard(mutex)(&data->lock);
+	rv = regmap_bulk_read(data->regmap, BMA220_REG_IF0, bma220_reg_if,
+			      sizeof(bma220_reg_if));
+	if (rv)
+		return IRQ_NONE;
+
+	if (FIELD_GET(BMA220_IF_DRDY, bma220_reg_if[1])) {
+		iio_trigger_poll_nested(data->trig);
+		goto done;
+	}
+
+done:
+
+	return IRQ_HANDLED;
 }

-int bma220_common_probe(struct spi_device *spi)
+int bma220_common_probe(struct device *dev, struct regmap *regmap, int irq)
 {
 	int ret;
 	struct iio_dev *indio_dev;
 	struct bma220_data *data;

-	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*data));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
 	if (!indio_dev)
 		return -ENOMEM;

 	data = iio_priv(indio_dev);
-	data->spi_device = spi;
-	mutex_init(&data->lock);
+	data->regmap = regmap;
+	data->dev = dev;
+
+	ret = bma220_init(data);
+	if (ret)
+		return ret;

+	mutex_init(&data->lock);
 	indio_dev->info = &bma220_info;
 	indio_dev->name = BMA220_DEVICE_NAME;
 	indio_dev->modes = INDIO_DIRECT_MODE;
@@ -269,38 +507,59 @@ int bma220_common_probe(struct spi_device *spi)
 	indio_dev->num_channels = ARRAY_SIZE(bma220_channels);
 	indio_dev->available_scan_masks = bma220_accel_scan_masks;

-	ret = bma220_init(data->spi_device);
-	if (ret)
-		return ret;
+	if (irq > 0) {
+		data->trig = devm_iio_trigger_alloc(dev, "%s-dev%d",
+						    indio_dev->name,
+						    iio_device_id(indio_dev));
+		if (!data->trig)
+			return -ENOMEM;
+
+		data->trig->ops = &bma220_trigger_ops;
+		iio_trigger_set_drvdata(data->trig, indio_dev);
+
+		ret = devm_iio_trigger_register(data->dev, data->trig);
+		if (ret)
+			return dev_err_probe(data->dev, ret,
+					     "iio trigger register fail\n");
+		indio_dev->trig = iio_trigger_get(data->trig);
+		ret = devm_request_threaded_irq(dev, irq, NULL,
+						&bma220_irq_handler,
+						IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+						indio_dev->name, indio_dev);
+		if (ret)
+			return dev_err_probe(data->dev, ret,
+					     "request irq %d failed\n", irq);
+	}

-	ret = devm_add_action_or_reset(&spi->dev, bma220_deinit, spi);
+	ret = devm_add_action_or_reset(data->dev, bma220_deinit, data);
 	if (ret)
 		return ret;

-	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
-					      iio_pollfunc_store_time,
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
 					      bma220_trigger_handler, NULL);
 	if (ret < 0) {
-		dev_err(&spi->dev, "iio triggered buffer setup failed\n");
+		dev_err(dev, "iio triggered buffer setup failed\n");
 		return ret;
 	}

-	return devm_iio_device_register(&spi->dev, indio_dev);
+	return devm_iio_device_register(dev, indio_dev);
 }
 EXPORT_SYMBOL_NS(bma220_common_probe, "IIO_BOSCH_BMA220");

 static int bma220_suspend(struct device *dev)
 {
-	struct spi_device *spi = to_spi_device(dev);
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct bma220_data *data = iio_priv(indio_dev);

-	return bma220_power(spi, false);
+	return bma220_power(data, false);
 }

 static int bma220_resume(struct device *dev)
 {
-	struct spi_device *spi = to_spi_device(dev);
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct bma220_data *data = iio_priv(indio_dev);

-	return bma220_power(spi, true);
+	return bma220_power(data, true);
 }
 EXPORT_NS_SIMPLE_DEV_PM_OPS(bma220_pm_ops, bma220_suspend, bma220_resume,
 			    IIO_BOSCH_BMA220);
diff --git a/drivers/iio/accel/bma220_spi.c b/drivers/iio/accel/bma220_spi.c
index be8348ad0a93..00e3fba9436d 100644
--- a/drivers/iio/accel/bma220_spi.c
+++ b/drivers/iio/accel/bma220_spi.c
@@ -9,17 +9,25 @@
 #include <linux/errno.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
+#include <linux/regmap.h>
 #include <linux/types.h>
 #include <linux/spi/spi.h>

-#include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>

 #include "bma220.h"

 static int bma220_spi_probe(struct spi_device *spi)
 {
-	return bma220_common_probe(spi);
+	struct regmap *regmap;
+
+	regmap = devm_regmap_init_spi(spi, &bma220_spi_regmap_config);
+	if (IS_ERR(regmap)) {
+		dev_err(&spi->dev, "failed to create regmap\n");
+		return PTR_ERR(regmap);
+	}
+
+	return bma220_common_probe(&spi->dev, regmap, spi->irq);
 }

 static const struct spi_device_id bma220_spi_id[] = {
--
2.49.1

Re: [PATCH 03/10] iio: accel: BMA220 migrate to regmap API

[Jonathan Cameron]

On Mon,  1 Sep 2025 22:47:29 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> Switch to regmap API.
> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
Hi Petre,

Various comments inline.  Biggest one is that the addition of the stuff
for irqs doesn't belong in the patch adding regmap.


> diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
> index 60fd35637d2d..e6dac2e1cf4d 100644
> --- a/drivers/iio/accel/bma220_core.c
> +++ b/drivers/iio/accel/bma220_core.c
> @@ -3,31 +3,133 @@
>   * BMA220 Digital triaxial acceleration sensor driver
>   *
>   * Copyright (c) 2016,2020 Intel Corporation.
> + * Copyright (c) 2025 Petre Rodan  <petre.rodan@subdimension.ro>
>   */
> 
>  #include <linux/bits.h>
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/cleanup.h>
> +#include <linux/device.h>
>  #include <linux/kernel.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>

This feels like an unrelated change.
Good to fix up the headers but for this patch I'd just
expect to see regmap related ones.  Do a precursor patch
just before this one to add the others.

>  #include <linux/types.h>
> -#include <linux/spi/spi.h>

Can't you drop that in previous patch?

> 
> -#include <linux/iio/buffer.h>

Why move this?  We tend to keep these in alphabetical order.

>  #include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/events.h>
>  #include <linux/iio/sysfs.h>
> +#include <linux/iio/trigger.h>

This is related to the irq stuff that shouldn't be in this patch. 

>  #include <linux/iio/trigger_consumer.h>
>  #include <linux/iio/triggered_buffer.h>
> 
> +#include "bma220.h"
> +
> +/*
> + * Read-Only Registers
Here as well. Not seeing this as beneficial over the the look
up in the regmap callback that already tells use which are in what
state.

> + */
> +
> +/* ID registers */
>  #define BMA220_REG_ID				0x00
> +#define BMA220_REG_REVISION_ID			0x01
> +
> +/* Acceleration registers */
>  #define BMA220_REG_ACCEL_X			0x02
>  #define BMA220_REG_ACCEL_Y			0x03
>  #define BMA220_REG_ACCEL_Z			0x04
> +
> +/*
> + * Read-write configuration registers
I'm not sure we need the read-write part of these comments.
That should be obvious once the regmap config is in place.

> + */

> +static bool bma220_is_volatile_reg(struct device *dev, unsigned int reg)
> +{
> +	/* Don't cache any registers. */

I assume this changes in later patches as setting cache_type is a bit pointless
otherwise!

> +	return true;
> +}
> +
> +const struct regmap_config bma220_spi_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.read_flag_mask = BIT(7),
> +	.max_register = BMA220_REG_SOFTRESET,
> +	.cache_type = REGCACHE_MAPLE,
> +	.writeable_reg = bma220_is_writable_reg,
> +	.volatile_reg = bma220_is_volatile_reg,
> +};
> +EXPORT_SYMBOL_NS(bma220_spi_regmap_config, "IIO_BOSCH_BMA220");

Any reason not to go NS_GPL?  I'd prefer that ideally.

> @@ -199,69 +356,150 @@ static const struct iio_info bma220_info = {
>  	.read_avail		= bma220_read_avail,
>  };
> 
> -static int bma220_init(struct spi_device *spi)
> +static int bma220_reset(struct bma220_data *data, bool up)
>  {
> -	int ret;
> +	int i, ret;
> +	unsigned int val;
> 
> -	ret = bma220_read_reg(spi, BMA220_REG_ID);
> -	if (ret != BMA220_CHIP_ID)
> -		return -ENODEV;
> +	guard(mutex)(&data->lock);
> 
> -	/* Make sure the chip is powered on */
> -	ret = bma220_read_reg(spi, BMA220_REG_SUSPEND);
> -	if (ret == BMA220_SUSPEND_WAKE)
> -		ret = bma220_read_reg(spi, BMA220_REG_SUSPEND);
> -	if (ret < 0)
> -		return ret;
> -	if (ret == BMA220_SUSPEND_WAKE)
> -		return -EBUSY;
> +	/**
> +	 * The chip can be reset by a simple register read.
> +	 * We need up to 2 register reads of the softreset register

May need?  Given you return early if the first one succeeds. If you actually
need two drop the loop and only check values on second read.

> +	 * to make sure that the device is in the desired state.
> +	 */
> +	for (i = 0; i < 2; i++) {
> +		ret = regmap_read(data->regmap, BMA220_REG_SOFTRESET, &val);
> +		if (ret < 0)
> +			return ret;
> 
> -	return 0;
> +		if (up && (val == BMA220_SUSPEND_SLEEP))
> +			return 0;
> +
> +		if (!up && (val == BMA220_SUSPEND_WAKE))
> +			return 0;
> +	}
> +
> +	return -EBUSY;
>  }

> 
> -static void bma220_deinit(void *spi)
> +static int bma220_init(struct bma220_data *data)
> +{
> +	int ret;
> +	unsigned int val;
> +	static const char * const regulator_names[] = { "vddd", "vddio", "vdda" };
> +
> +	ret = devm_regulator_bulk_get_enable(data->dev,
> +					     ARRAY_SIZE(regulator_names),
> +					     regulator_names);
> +	if (ret)
> +		return dev_err_probe(data->dev, ret, "Failed to get regulators\n");
I'd have a local struct device *dev = data->dev;
just to shorten the various lines.
> +
> +	/* Try to read chip_id register. It must return 0xdd. */
> +	ret = regmap_read(data->regmap, BMA220_REG_ID, &val);
> +	if (ret) {
> +		dev_err(data->dev, "Failed to read chip id register\n");

Use return dev_err_probe().  For things that can't defer it just brings
prettier prints and simpler code.  Still worth having!

> +		return ret;
> +	}
> +
> +	if (val != BMA220_CHIP_ID)
> +		return -ENODEV;
> +
> +	ret = bma220_power(data, true);
> +	if (ret) {
> +		dev_err(data->dev, "Failed to power-on chip\n");
> +		return ret;

return dev_err_probe() here as well..

> +	}
> +
> +	ret = bma220_reset(data, true);
> +	if (ret) {
> +		dev_err(data->dev, "Failed to soft reset chip\n");
> +		return ret;
and here.

> +	}
> +
> +	return 0;
> +}
> +

> +static irqreturn_t bma220_irq_handler(int irq, void *private)
> +{
> +	struct iio_dev *indio_dev = private;
> +	struct bma220_data *data = iio_priv(indio_dev);
> +	int rv;
> +	u8 bma220_reg_if[2];
> +
> +	guard(mutex)(&data->lock);
> +	rv = regmap_bulk_read(data->regmap, BMA220_REG_IF0, bma220_reg_if,
> +			      sizeof(bma220_reg_if));
> +	if (rv)
> +		return IRQ_NONE;
> +
> +	if (FIELD_GET(BMA220_IF_DRDY, bma220_reg_if[1])) {
> +		iio_trigger_poll_nested(data->trig);
> +		goto done;
> +	}
> +
> +done:
> +
> +	return IRQ_HANDLED;
>  }
> 
> -int bma220_common_probe(struct spi_device *spi)
> +int bma220_common_probe(struct device *dev, struct regmap *regmap, int irq)
>  {
>  	int ret;
>  	struct iio_dev *indio_dev;
>  	struct bma220_data *data;
> 
> -	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*data));
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
>  	if (!indio_dev)
>  		return -ENOMEM;
> 
>  	data = iio_priv(indio_dev);
> -	data->spi_device = spi;
> -	mutex_init(&data->lock);
> +	data->regmap = regmap;
> +	data->dev = dev;
> +
> +	ret = bma220_init(data);
> +	if (ret)
> +		return ret;
> 
> +	mutex_init(&data->lock);
#Whilst you are here perhaps switch this to 
	ret = devm_mutex_init(dev, 7data->lock);
	if (ret)
		return ret;

It brings only a small benefit in lock debugging but doesn't cost much either
so I'm encouraging it's use in new code or code we are touching anyway.
Fine to just slip that in with this patch rather than spinning another one.

>  	indio_dev->info = &bma220_info;
>  	indio_dev->name = BMA220_DEVICE_NAME;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> @@ -269,38 +507,59 @@ int bma220_common_probe(struct spi_device *spi)
>  	indio_dev->num_channels = ARRAY_SIZE(bma220_channels);
>  	indio_dev->available_scan_masks = bma220_accel_scan_masks;
> 
> -	ret = bma220_init(data->spi_device);
> -	if (ret)
> -		return ret;
> +	if (irq > 0) {

This next block doesn't seem to have much to do with regmap API conversion.
Wrong patch?

> +		data->trig = devm_iio_trigger_alloc(dev, "%s-dev%d",
> +						    indio_dev->name,
> +						    iio_device_id(indio_dev));
> +		if (!data->trig)
> +			return -ENOMEM;
> +
> +		data->trig->ops = &bma220_trigger_ops;
> +		iio_trigger_set_drvdata(data->trig, indio_dev);
> +
> +		ret = devm_iio_trigger_register(data->dev, data->trig);
> +		if (ret)
> +			return dev_err_probe(data->dev, ret,
> +					     "iio trigger register fail\n");
> +		indio_dev->trig = iio_trigger_get(data->trig);
> +		ret = devm_request_threaded_irq(dev, irq, NULL,
> +						&bma220_irq_handler,
> +						IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> +						indio_dev->name, indio_dev);
> +		if (ret)
> +			return dev_err_probe(data->dev, ret,
> +					     "request irq %d failed\n", irq);
> +	}
> 
> -	ret = devm_add_action_or_reset(&spi->dev, bma220_deinit, spi);
> +	ret = devm_add_action_or_reset(data->dev, bma220_deinit, data);
>  	if (ret)
>  		return ret;
> 
> -	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
> -					      iio_pollfunc_store_time,
> +	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
>  					      bma220_trigger_handler, NULL);
>  	if (ret < 0) {
> -		dev_err(&spi->dev, "iio triggered buffer setup failed\n");
> +		dev_err(dev, "iio triggered buffer setup failed\n");
>  		return ret;
>  	}
> 
> -	return devm_iio_device_register(&spi->dev, indio_dev);
> +	return devm_iio_device_register(dev, indio_dev);
>  }
>  EXPORT_SYMBOL_NS(bma220_common_probe, "IIO_BOSCH_BMA220");

> diff --git a/drivers/iio/accel/bma220_spi.c b/drivers/iio/accel/bma220_spi.c
> index be8348ad0a93..00e3fba9436d 100644
> --- a/drivers/iio/accel/bma220_spi.c
> +++ b/drivers/iio/accel/bma220_spi.c
> @@ -9,17 +9,25 @@
>  #include <linux/errno.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/module.h>
> +#include <linux/regmap.h>
>  #include <linux/types.h>
>  #include <linux/spi/spi.h>
> 
> -#include <linux/iio/buffer.h>
>  #include <linux/iio/iio.h>
> 
>  #include "bma220.h"
> 
>  static int bma220_spi_probe(struct spi_device *spi)
>  {
> -	return bma220_common_probe(spi);
> +	struct regmap *regmap;
> +
> +	regmap = devm_regmap_init_spi(spi, &bma220_spi_regmap_config);
> +	if (IS_ERR(regmap)) {
> +		dev_err(&spi->dev, "failed to create regmap\n");

		return dev_err_probe(&spi->dev, PTR_ERR(regmap), "failed to create regmap\n");

If there are other similar cases in things only called from probe please
switch them to this interface as well (in a separate patch if touching
existing code)

Thanks,

Jonathan

> +		return PTR_ERR(regmap);
> +	}
> +
> +	return bma220_common_probe(&spi->dev, regmap, spi->irq);
>  }

Re: [PATCH 03/10] iio: accel: BMA220 migrate to regmap API

[Petre Rodan]

[-- Attachment #1: Type: text/plain, Size: 1485 bytes --]

On Sun, Sep 07, 2025 at 01:45:06PM +0100, Jonathan Cameron wrote:
> > +static int bma220_reset(struct bma220_data *data, bool up)
> >  {
> > +	int i, ret;
> > +	unsigned int val;
> > +	guard(mutex)(&data->lock);
> > 
> > +	/**
> > +	 * The chip can be reset by a simple register read.
> > +	 * We need up to 2 register reads of the softreset register
> 
> May need?  Given you return early if the first one succeeds. If you actually
> need two drop the loop and only check values on second read.
> 
> > +	 * to make sure that the device is in the desired state.
> > +	 */
> > +	for (i = 0; i < 2; i++) {
> > +		ret = regmap_read(data->regmap, BMA220_REG_SOFTRESET, &val);
> > +		if (ret < 0)
> > +			return ret;

I'm not sure how eloquently I can explain this. the sensor can be in

sleep state / non-sleep state
reset state / non-reset state
(these overlap)

the sensor toggles between these states when the master reads the suspend and
the soft_reset registers respectively.
based on the value read one can tell what was the previous state the sensor was in.

bma220_init() simply places the sensor in the non-sleep AND non-reset modes (and
resets all configuration registers so that we start from a known initial condition)

'may need' is used because the sensor might have been left in an unexpected mode
in the previous session.
we need at most two reads of a register to make sure bma220 ends up in the state we need.

best regards,
peter

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 03/10] iio: accel: BMA220 migrate to regmap API

[Jonathan Cameron]

On Mon, 8 Sep 2025 06:27:05 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> On Sun, Sep 07, 2025 at 01:45:06PM +0100, Jonathan Cameron wrote:
> > > +static int bma220_reset(struct bma220_data *data, bool up)
> > >  {
> > > +	int i, ret;
> > > +	unsigned int val;
> > > +	guard(mutex)(&data->lock);
> > > 
> > > +	/**
> > > +	 * The chip can be reset by a simple register read.
> > > +	 * We need up to 2 register reads of the softreset register  
> > 
> > May need?  Given you return early if the first one succeeds. If you actually
> > need two drop the loop and only check values on second read.
> >   
> > > +	 * to make sure that the device is in the desired state.
> > > +	 */
> > > +	for (i = 0; i < 2; i++) {
> > > +		ret = regmap_read(data->regmap, BMA220_REG_SOFTRESET, &val);
> > > +		if (ret < 0)
> > > +			return ret;  
> 
> I'm not sure how eloquently I can explain this. the sensor can be in
> 
> sleep state / non-sleep state
> reset state / non-reset state
> (these overlap)
> 
> the sensor toggles between these states when the master reads the suspend and
> the soft_reset registers respectively.
> based on the value read one can tell what was the previous state the sensor was in.
> 
> bma220_init() simply places the sensor in the non-sleep AND non-reset modes (and
> resets all configuration registers so that we start from a known initial condition)
> 
> 'may need' is used because the sensor might have been left in an unexpected mode
> in the previous session.
> we need at most two reads of a register to make sure bma220 ends up in the state we need.

Fair enough.  That is obscure and generally weird. 'may need' seems a valid short comment!

J
> 
> best regards,
> peter
> 

[PATCH 04/10] iio: accel: BMA220 add i2c module

[Petre Rodan]

Add i2c module.
Note that the kernel module transparently shifts all register addresses
1 bit to the left, so all functions will operate based on the SPI memory
map.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
 drivers/iio/accel/Kconfig       |  9 ++++-
 drivers/iio/accel/Makefile      |  1 +
 drivers/iio/accel/bma220.h      |  1 +
 drivers/iio/accel/bma220_core.c | 19 ++++++++++
 drivers/iio/accel/bma220_i2c.c  | 62 +++++++++++++++++++++++++++++++++
 5 files changed, 91 insertions(+), 1 deletion(-)
 create mode 100644 drivers/iio/accel/bma220_i2c.c

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 9b6c35b75948..b3c5b0b7a406 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -221,6 +221,7 @@ config BMA220
 	select REGMAP
 	select IIO_BUFFER
 	select IIO_TRIGGERED_BUFFER
+	select BMA220_I2C if I2C
 	select BMA220_SPI if SPI
 	help
 	  Say yes here to add support for the Bosch BMA220 triaxial
@@ -228,7 +229,13 @@ config BMA220

 	  To compile this driver as a module, choose M here: the
 	  module will be called bma220_core and you will also get
-	  bma220_spi if SPI is enabled.
+	  bma220_i2c if I2C is enabled and bma220_spi if SPI is
+	  enabled.
+
+config BMA220_I2C
+	tristate
+	select REGMAP_I2C
+	depends on BMA220

 config BMA220_SPI
 	tristate
diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
index 56a9f848f7f9..fa440a859283 100644
--- a/drivers/iio/accel/Makefile
+++ b/drivers/iio/accel/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_ADXL380_I2C) += adxl380_i2c.o
 obj-$(CONFIG_ADXL380_SPI) += adxl380_spi.o
 obj-$(CONFIG_BMA180) += bma180.o
 obj-$(CONFIG_BMA220) += bma220_core.o
+obj-$(CONFIG_BMA220_I2C) += bma220_i2c.o
 obj-$(CONFIG_BMA220_SPI) += bma220_spi.o
 obj-$(CONFIG_BMA400) += bma400_core.o
 obj-$(CONFIG_BMA400_I2C) += bma400_i2c.o
diff --git a/drivers/iio/accel/bma220.h b/drivers/iio/accel/bma220.h
index 5eefa9749d33..ae4b74514629 100644
--- a/drivers/iio/accel/bma220.h
+++ b/drivers/iio/accel/bma220.h
@@ -13,6 +13,7 @@
 #include <linux/iio/iio.h>

 extern const struct regmap_config bma220_spi_regmap_config;
+extern const struct regmap_config bma220_i2c_regmap_config;
 extern const struct dev_pm_ops bma220_pm_ops;

 int bma220_common_probe(struct device *dev, struct regmap *regmap, int irq);
diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index e6dac2e1cf4d..fae84823d52b 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -225,6 +225,25 @@ const struct regmap_config bma220_spi_regmap_config = {
 };
 EXPORT_SYMBOL_NS(bma220_spi_regmap_config, "IIO_BOSCH_BMA220");

+/*
+ * Based on the datasheet the memory map differs between the SPI and the I2C
+ * implementations. I2C register addresses are simply shifted to the left
+ * by 1 bit yet the register size remains unchanged.
+ * This driver employs the SPI memory map to correlate register names to
+ * addresses regardless of the bus type.
+ */
+
+const struct regmap_config bma220_i2c_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.reg_shift = -1,
+	.max_register = BMA220_REG_SOFTRESET,
+	.cache_type = REGCACHE_MAPLE,
+	.writeable_reg = bma220_is_writable_reg,
+	.volatile_reg = bma220_is_volatile_reg,
+};
+EXPORT_SYMBOL_NS(bma220_i2c_regmap_config, "IIO_BOSCH_BMA220");
+
 static int bma220_data_rdy_trigger_set_state(struct iio_trigger *trig,
 					     bool state)
 {
diff --git a/drivers/iio/accel/bma220_i2c.c b/drivers/iio/accel/bma220_i2c.c
new file mode 100644
index 000000000000..8a1644aac287
--- /dev/null
+++ b/drivers/iio/accel/bma220_i2c.c
@@ -0,0 +1,62 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Bosch triaxial acceleration sensor
+ *
+ * Copyright (c) 2025 Petre Rodan <petre.rodan@subdimension.ro>
+ *
+ * Datasheet: https://media.digikey.com/pdf/Data%20Sheets/Bosch/BMA220.pdf
+ * I2C address is either 0x0b or 0x0a depending on CSB (pin 10)
+ */
+
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+#include <linux/iio/iio.h>
+
+#include "bma220.h"
+
+static int bma220_i2c_probe(struct i2c_client *client)
+{
+	struct regmap *regmap;
+
+	regmap = devm_regmap_init_i2c(client, &bma220_i2c_regmap_config);
+	if (IS_ERR(regmap)) {
+		dev_err(&client->dev, "failed to create regmap\n");
+		return PTR_ERR(regmap);
+	}
+
+	return bma220_common_probe(&client->dev, regmap, client->irq);
+}
+
+static const struct of_device_id bma220_i2c_match[] = {
+	{ .compatible = "bosch,bma220" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, bma220_i2c_match);
+
+static const struct i2c_device_id bma220_i2c_id[] = {
+	{ "bma220" },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, bma220_i2c_id);
+
+static struct i2c_driver bma220_i2c_driver = {
+	.driver = {
+		.name = "bma220_i2c",
+		.pm = pm_sleep_ptr(&bma220_pm_ops),
+		.of_match_table = bma220_i2c_match,
+	},
+	.probe = bma220_i2c_probe,
+	.id_table = bma220_i2c_id,
+};
+module_i2c_driver(bma220_i2c_driver);
+
+MODULE_AUTHOR("Petre Rodan <petre.rodan@subdimension.ro>");
+MODULE_DESCRIPTION("Bosch triaxial acceleration sensor i2c driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("IIO_BOSCH_BMA220");
--
2.49.1

Re: [PATCH 04/10] iio: accel: BMA220 add i2c module

[Jonathan Cameron]

On Mon,  1 Sep 2025 22:47:30 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> Add i2c module.
> Note that the kernel module transparently shifts all register addresses
> 1 bit to the left, so all functions will operate based on the SPI memory
> map.
> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>

Trivial comments inline

> diff --git a/drivers/iio/accel/bma220_i2c.c b/drivers/iio/accel/bma220_i2c.c
> new file mode 100644
> index 000000000000..8a1644aac287
> --- /dev/null
> +++ b/drivers/iio/accel/bma220_i2c.c
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Bosch triaxial acceleration sensor
> + *
> + * Copyright (c) 2025 Petre Rodan <petre.rodan@subdimension.ro>
> + *
> + * Datasheet: https://media.digikey.com/pdf/Data%20Sheets/Bosch/BMA220.pdf
> + * I2C address is either 0x0b or 0x0a depending on CSB (pin 10)
> + */
> +
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/i2c.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/types.h>
> +
> +#include <linux/iio/iio.h>
> +
> +#include "bma220.h"
> +
> +static int bma220_i2c_probe(struct i2c_client *client)
> +{
> +	struct regmap *regmap;
> +
> +	regmap = devm_regmap_init_i2c(client, &bma220_i2c_regmap_config);
> +	if (IS_ERR(regmap)) {
> +		dev_err(&client->dev, "failed to create regmap\n");
> +		return PTR_ERR(regmap);

return dev_err_probe()

> +	}
> +
> +	return bma220_common_probe(&client->dev, regmap, client->irq);
> +}

> 

[PATCH 05/10] iio: accel: BMA220 make use of the watchdog functionality

[Petre Rodan]

Sometimes the sensor gets stuck and enters a condition in which it pulls
SDA low, thus making the entire i2c bus unusable.
The optional bosch,watchdog property mitigates this problem by clearing
the condition after a period of 1 or 10ms.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
 drivers/iio/accel/bma220_core.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index fae84823d52b..86347cf8ab1e 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -158,6 +158,12 @@ enum bma220_axis {
 	AXIS_Z,
 };

+enum bma220_prop_wdt {
+	BMA220_PROP_WDT_OFF,
+	BMA220_PROP_WDT_1MS,
+	BMA220_PROP_WDT_10MS,
+};
+
 static const int bma220_scale_table[][2] = {
 	{0, 623000}, {1, 248000}, {2, 491000}, {4, 983000},
 };
@@ -428,10 +434,17 @@ static int bma220_power(struct bma220_data *data, bool up)
 	return -EBUSY;
 }

+static int bma220_wdt(struct bma220_data *data, const u8 val)
+{
+	return regmap_update_bits(data->regmap, BMA220_REG_WDT, BMA220_WDT_MASK,
+				  FIELD_PREP(BMA220_WDT_MASK, val));
+}
+
 static int bma220_init(struct bma220_data *data)
 {
 	int ret;
 	unsigned int val;
+	u32 watchdog;
 	static const char * const regulator_names[] = { "vddd", "vddio", "vdda" };

 	ret = devm_regulator_bulk_get_enable(data->dev,
@@ -462,6 +475,25 @@ static int bma220_init(struct bma220_data *data)
 		return ret;
 	}

+	ret = device_property_read_u32(data->dev, "bosch,watchdog", &watchdog);
+	if (!ret) {
+		switch (watchdog) {
+		case BMA220_PROP_WDT_1MS:
+			ret = bma220_wdt(data, BMA220_WDT_1MS);
+			break;
+		case BMA220_PROP_WDT_10MS:
+			ret = bma220_wdt(data, BMA220_WDT_10MS);
+			break;
+		default:
+			ret = bma220_wdt(data, BMA220_WDT_OFF);
+			break;
+		}
+		if (ret) {
+			dev_err(data->dev, "Failed to set watchdog\n");
+			return ret;
+		}
+	}
+
 	return 0;
 }

--
2.49.1

Re: [PATCH 05/10] iio: accel: BMA220 make use of the watchdog functionality

[Jonathan Cameron]

On Mon,  1 Sep 2025 22:47:31 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> Sometimes the sensor gets stuck and enters a condition in which it pulls
> SDA low, thus making the entire i2c bus unusable.
> The optional bosch,watchdog property mitigates this problem by clearing
> the condition after a period of 1 or 10ms.

As I think was discussed with the binding, I'd turn this on by default
and we can figure out if we want to change that if it causes anyone
problems long run.

> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> ---
>  drivers/iio/accel/bma220_core.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
> index fae84823d52b..86347cf8ab1e 100644
> --- a/drivers/iio/accel/bma220_core.c
> +++ b/drivers/iio/accel/bma220_core.c
> @@ -158,6 +158,12 @@ enum bma220_axis {
>  	AXIS_Z,
>  };
> 
> +enum bma220_prop_wdt {
> +	BMA220_PROP_WDT_OFF,
> +	BMA220_PROP_WDT_1MS,
> +	BMA220_PROP_WDT_10MS,
> +};
> +
>  static const int bma220_scale_table[][2] = {
>  	{0, 623000}, {1, 248000}, {2, 491000}, {4, 983000},
>  };
> @@ -428,10 +434,17 @@ static int bma220_power(struct bma220_data *data, bool up)
>  	return -EBUSY;
>  }
> 
> +static int bma220_wdt(struct bma220_data *data, const u8 val)
> +{
> +	return regmap_update_bits(data->regmap, BMA220_REG_WDT, BMA220_WDT_MASK,
> +				  FIELD_PREP(BMA220_WDT_MASK, val));
> +}
> +
>  static int bma220_init(struct bma220_data *data)
>  {
>  	int ret;
>  	unsigned int val;
> +	u32 watchdog;
>  	static const char * const regulator_names[] = { "vddd", "vddio", "vdda" };
> 
>  	ret = devm_regulator_bulk_get_enable(data->dev,
> @@ -462,6 +475,25 @@ static int bma220_init(struct bma220_data *data)
>  		return ret;
>  	}
> 
> +	ret = device_property_read_u32(data->dev, "bosch,watchdog", &watchdog);
> +	if (!ret) {
> +		switch (watchdog) {
> +		case BMA220_PROP_WDT_1MS:
> +			ret = bma220_wdt(data, BMA220_WDT_1MS);
> +			break;
> +		case BMA220_PROP_WDT_10MS:
> +			ret = bma220_wdt(data, BMA220_WDT_10MS);
> +			break;
> +		default:
> +			ret = bma220_wdt(data, BMA220_WDT_OFF);
> +			break;
> +		}
> +		if (ret) {
> +			dev_err(data->dev, "Failed to set watchdog\n");

dev_err_probe()

> +			return ret;
> +		}
> +	}
> +
>  	return 0;
>  }
> 
> --
> 2.49.1
> 
> 

[PATCH 06/10] iio: accel: BMA220 add LPF cut-off frequency mapping

[Petre Rodan]

 - add mapping for the low pass filter cut-off frequency.
 - make valid values visible for both the cut-off frequency and the scale.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
 drivers/iio/accel/bma220_core.c | 60 ++++++++++++++++++++++++++++++++-
 1 file changed, 59 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index 86347cf8ab1e..e60acd62cf96 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -4,6 +4,15 @@
  *
  * Copyright (c) 2016,2020 Intel Corporation.
  * Copyright (c) 2025 Petre Rodan  <petre.rodan@subdimension.ro>
+ *
+ *
+ * Device register to IIO ABI mapping:
+ *
+ * Register                  | IIO ABI (sysfs)                | valid values
+ * --------------------------+--------------------------------+---------------
+ * scale (range)             | in_accel_scale                 | see _available
+ * cut-off freq (filt_config)| in_accel_filter_low_pass_...   | see _available
+ * ---------------------------------------------------------------------------
  */

 #include <linux/bits.h>
@@ -135,13 +144,23 @@

 #define BMA220_DEVICE_NAME			"bma220"

+#define BMA220_COF_1000HZ			0x0
+#define BMA220_COF_500HZ			0x1
+#define BMA220_COF_250HZ			0x2
+#define BMA220_COF_125HZ			0x3
+#define BMA220_COF_64HZ				0x4
+#define BMA220_COF_32HZ				0x5
+
 #define BMA220_ACCEL_CHANNEL(index, reg, axis) {			\
 	.type = IIO_ACCEL,						\
 	.address = reg,							\
 	.modified = 1,							\
 	.channel2 = IIO_MOD_##axis,					\
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
-	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |		\
+	    BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),		\
+	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SCALE) |\
+	    BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),		\
 	.scan_index = index,						\
 	.scan_type = {							\
 		.sign = 's',						\
@@ -172,6 +191,7 @@ struct bma220_data {
 	struct device *dev;
 	struct regmap *regmap;
 	struct mutex lock;
+	u8 lpf_3db_freq_idx;
 	u8 range_idx;
 	struct iio_trigger *trig;
 	struct {
@@ -188,6 +208,18 @@ static const struct iio_chan_spec bma220_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(3),
 };

+/*
+ * Available cut-off frequencies of the low pass filter in Hz.
+ */
+static const int bma220_lpf_3db_freq_hz_table[][2] = {
+	[BMA220_COF_1000HZ] = {1000, 0},
+	[BMA220_COF_500HZ] = {500, 0},
+	[BMA220_COF_250HZ] = {250, 0},
+	[BMA220_COF_125HZ] = {125, 0},
+	[BMA220_COF_64HZ] = {64, 0},
+	[BMA220_COF_32HZ] = {32, 0},
+};
+
 static const unsigned long bma220_accel_scan_masks[] = {
 	BIT(AXIS_X) | BIT(AXIS_Y) | BIT(AXIS_Z),
 	0
@@ -309,6 +341,11 @@ static int bma220_read_raw(struct iio_dev *indio_dev,
 		*val = bma220_scale_table[index][0];
 		*val2 = bma220_scale_table[index][1];
 		return IIO_VAL_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		index = data->lpf_3db_freq_idx;
+		*val = bma220_lpf_3db_freq_hz_table[index][0];
+		*val2 = bma220_lpf_3db_freq_hz_table[index][1];
+		return IIO_VAL_INT_PLUS_MICRO;
 	}

 	return -EINVAL;
@@ -353,6 +390,22 @@ static int bma220_write_raw(struct iio_dev *indio_dev,
 				"failed to set measurement range\n");
 		data->range_idx = index;

+		return 0;
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		index = bma220_find_match(bma220_lpf_3db_freq_hz_table,
+					  ARRAY_SIZE(bma220_lpf_3db_freq_hz_table),
+					  val, val2);
+		if (index < 0)
+			return -EINVAL;
+
+		ret = regmap_update_bits(data->regmap, BMA220_REG_FILTER,
+					 BMA220_FILTER_MASK,
+					 FIELD_PREP(BMA220_FILTER_MASK, index));
+		if (ret < 0)
+			dev_err(data->dev,
+				"failed to set low pass filter\n");
+		data->lpf_3db_freq_idx = index;
+
 		return 0;
 	}

@@ -370,6 +423,11 @@ static int bma220_read_avail(struct iio_dev *indio_dev,
 		*type = IIO_VAL_INT_PLUS_MICRO;
 		*length = ARRAY_SIZE(bma220_scale_table) * 2;
 		return IIO_AVAIL_LIST;
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		*vals = (const int *)bma220_lpf_3db_freq_hz_table;
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		*length = ARRAY_SIZE(bma220_lpf_3db_freq_hz_table) * 2;
+		return IIO_AVAIL_LIST;
 	default:
 		return -EINVAL;
 	}
--
2.49.1

Re: [PATCH 06/10] iio: accel: BMA220 add LPF cut-off frequency mapping

[David Lechner]

On 9/1/25 2:47 PM, Petre Rodan wrote:
>  - add mapping for the low pass filter cut-off frequency.
>  - make valid values visible for both the cut-off frequency and the scale.
> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> ---
>  drivers/iio/accel/bma220_core.c | 60 ++++++++++++++++++++++++++++++++-
>  1 file changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
> index 86347cf8ab1e..e60acd62cf96 100644
> --- a/drivers/iio/accel/bma220_core.c
> +++ b/drivers/iio/accel/bma220_core.c
> @@ -4,6 +4,15 @@
>   *
>   * Copyright (c) 2016,2020 Intel Corporation.
>   * Copyright (c) 2025 Petre Rodan  <petre.rodan@subdimension.ro>
> + *
> + *
> + * Device register to IIO ABI mapping:
> + *
> + * Register                  | IIO ABI (sysfs)                | valid values
> + * --------------------------+--------------------------------+---------------
> + * scale (range)             | in_accel_scale                 | see _available
> + * cut-off freq (filt_config)| in_accel_filter_low_pass_...   | see _available
> + * ---------------------------------------------------------------------------
>   */
> 
>  #include <linux/bits.h>
> @@ -135,13 +144,23 @@
> 
>  #define BMA220_DEVICE_NAME			"bma220"
> 
> +#define BMA220_COF_1000HZ			0x0
> +#define BMA220_COF_500HZ			0x1
> +#define BMA220_COF_250HZ			0x2
> +#define BMA220_COF_125HZ			0x3
> +#define BMA220_COF_64HZ				0x4
> +#define BMA220_COF_32HZ				0x5
> +
>  #define BMA220_ACCEL_CHANNEL(index, reg, axis) {			\
>  	.type = IIO_ACCEL,						\
>  	.address = reg,							\
>  	.modified = 1,							\
>  	.channel2 = IIO_MOD_##axis,					\
>  	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
> -	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |		\
> +	    BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),		\
> +	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SCALE) |\
> +	    BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),		\
>  	.scan_index = index,						\
>  	.scan_type = {							\
>  		.sign = 's',						\
> @@ -172,6 +191,7 @@ struct bma220_data {
>  	struct device *dev;
>  	struct regmap *regmap;
>  	struct mutex lock;
> +	u8 lpf_3db_freq_idx;
>  	u8 range_idx;
>  	struct iio_trigger *trig;
>  	struct {
> @@ -188,6 +208,18 @@ static const struct iio_chan_spec bma220_channels[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(3),
>  };
> 
> +/*
> + * Available cut-off frequencies of the low pass filter in Hz.
> + */
> +static const int bma220_lpf_3db_freq_hz_table[][2] = {
> +	[BMA220_COF_1000HZ] = {1000, 0},
> +	[BMA220_COF_500HZ] = {500, 0},
> +	[BMA220_COF_250HZ] = {250, 0},
> +	[BMA220_COF_125HZ] = {125, 0},
> +	[BMA220_COF_64HZ] = {64, 0},
> +	[BMA220_COF_32HZ] = {32, 0},

If all of these are integer values, why do we need 2-D table?

> +};
> +
>  static const unsigned long bma220_accel_scan_masks[] = {
>  	BIT(AXIS_X) | BIT(AXIS_Y) | BIT(AXIS_Z),
>  	0
> @@ -309,6 +341,11 @@ static int bma220_read_raw(struct iio_dev *indio_dev,
>  		*val = bma220_scale_table[index][0];
>  		*val2 = bma220_scale_table[index][1];
>  		return IIO_VAL_INT_PLUS_MICRO;
> +	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> +		index = data->lpf_3db_freq_idx;
> +		*val = bma220_lpf_3db_freq_hz_table[index][0];
> +		*val2 = bma220_lpf_3db_freq_hz_table[index][1];
> +		return IIO_VAL_INT_PLUS_MICRO;

Why not IIO_VAL_INT?

>  	}
> 
>  	return -EINVAL;
> @@ -353,6 +390,22 @@ static int bma220_write_raw(struct iio_dev *indio_dev,
>  				"failed to set measurement range\n");
>  		data->range_idx = index;
> 
> +		return 0;
> +	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> +		index = bma220_find_match(bma220_lpf_3db_freq_hz_table,
> +					  ARRAY_SIZE(bma220_lpf_3db_freq_hz_table),
> +					  val, val2);
> +		if (index < 0)
> +			return -EINVAL;
> +
> +		ret = regmap_update_bits(data->regmap, BMA220_REG_FILTER,
> +					 BMA220_FILTER_MASK,
> +					 FIELD_PREP(BMA220_FILTER_MASK, index));
> +		if (ret < 0)
> +			dev_err(data->dev,
> +				"failed to set low pass filter\n");

Should `return ret;` here rather than logging error.

> +		data->lpf_3db_freq_idx = index;
> +
>  		return 0;
>  	}
> 
> @@ -370,6 +423,11 @@ static int bma220_read_avail(struct iio_dev *indio_dev,
>  		*type = IIO_VAL_INT_PLUS_MICRO;
>  		*length = ARRAY_SIZE(bma220_scale_table) * 2;
>  		return IIO_AVAIL_LIST;
> +	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> +		*vals = (const int *)bma220_lpf_3db_freq_hz_table;
> +		*type = IIO_VAL_INT_PLUS_MICRO;
> +		*length = ARRAY_SIZE(bma220_lpf_3db_freq_hz_table) * 2;
> +		return IIO_AVAIL_LIST;
>  	default:
>  		return -EINVAL;
>  	}
> --
> 2.49.1
> 

Re: [PATCH 06/10] iio: accel: BMA220 add LPF cut-off frequency mapping

[Jonathan Cameron]

On Fri, 5 Sep 2025 14:59:27 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 9/1/25 2:47 PM, Petre Rodan wrote:
> >  - add mapping for the low pass filter cut-off frequency.
> >  - make valid values visible for both the cut-off frequency and the scale.
> > 
> > Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>

> > +/*
> > + * Available cut-off frequencies of the low pass filter in Hz.
> > + */
> > +static const int bma220_lpf_3db_freq_hz_table[][2] = {
> > +	[BMA220_COF_1000HZ] = {1000, 0},
> > +	[BMA220_COF_500HZ] = {500, 0},
> > +	[BMA220_COF_250HZ] = {250, 0},
> > +	[BMA220_COF_125HZ] = {125, 0},
> > +	[BMA220_COF_64HZ] = {64, 0},
> > +	[BMA220_COF_32HZ] = {32, 0},  
> 
> If all of these are integer values, why do we need 2-D table?
Style wise, for IIO we are going for 
{ 32, 0 }, 

That is extra spaces.

Mind you that's irrelevant if it's a 1D array ;)

[PATCH 07/10] iio: accel: BMA220 add debugfs reg access

[Petre Rodan]

Allow read/write access to sensor registers for use in unit-tests.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
 drivers/iio/accel/bma220_core.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index e60acd62cf96..41889cdcef76 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -433,10 +433,21 @@ static int bma220_read_avail(struct iio_dev *indio_dev,
 	}
 }

+static int bma220_reg_access(struct iio_dev *indio_dev, unsigned int reg,
+			     unsigned int writeval, unsigned int *readval)
+{
+	struct bma220_data *data = iio_priv(indio_dev);
+
+	if (readval)
+		return regmap_read(data->regmap, reg, readval);
+	return regmap_write(data->regmap, reg, writeval);
+}
+
 static const struct iio_info bma220_info = {
 	.read_raw		= bma220_read_raw,
 	.write_raw		= bma220_write_raw,
 	.read_avail		= bma220_read_avail,
+	.debugfs_reg_access	= &bma220_reg_access,
 };

 static int bma220_reset(struct bma220_data *data, bool up)
--
2.49.1

[PATCH 08/10] iio: accel: BMA220 add events

[Petre Rodan]

Add events for tap detection and low-g, high-g, slope conditions.
Ignored the 80-column rule for readability.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
 drivers/iio/accel/bma220_core.c | 686 ++++++++++++++++++++++++++++++++
 1 file changed, 686 insertions(+)

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index 41889cdcef76..c8da6cc2eaf3 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -13,6 +13,37 @@
  * scale (range)             | in_accel_scale                 | see _available
  * cut-off freq (filt_config)| in_accel_filter_low_pass_...   | see _available
  * ---------------------------------------------------------------------------
+ *
+ * Events:
+ * Register                  | IIO ABI (sysfs)                   | valid values
+ * --------------------------+-----------------------------------+------------
+ * high-g detection          |                                   |
+ *   enable/disable irq      | in_accel_*_thresh_rising_en       | 0/1
+ *   threshold (high_th)     | in_accel_thresh_rising_value      | 0-15
+ *   hysteresis (high_hy)    | in_accel_thresh_rising_hysteresis | 0-3
+ *   duration (high_dur)     | in_accel_thresh_rising_period     | 0-63
+ * low-g detection           |                                   |
+ *   enable/disable irq      | in_accel_*_thresh_falling_en      | 0/1
+ *   threshold (low_th)      | in_accel_thresh_falling_value     | 0-15
+ *   hysteresis (low_hy)     | in_accel_thresh_falling_hysteresis| 0-3
+ *   duration (low_dur)      | in_accel_thresh_falling_period    | 0-63
+ * slope detection           |                                   |
+ *   enable/disable irq      | in_accel_*_thresh_either_en       | 0/1
+ *   threshold (slope_th)    | in_accel_thresh_either_value      | 0-15
+ *   duration (slope_dur)    | in_accel_thresh_either_period     | 0-3
+ * tap sensing               |                                   |
+ *   enable/disable singletap| in_accel_*_gesture_singletap_en   | 0/1 [2]
+ *   enable/disable doubletap| in_accel_*_gesture_doubletap_en   | 0/1 [2]
+ *   threshold (tt_th)       | in_accel_gesture_singletap_value  | 0-15
+ *   duration (tt_dur)       | in_accel_gesture_doubletap_period | see [1]
+ * ----------------------------------------------------------------------------
+ *
+ * [1] The event related sysfs interface provides and expects raw register values
+ *         (unshifted bitfields) based on the chip specifications.
+ * [2] Do not mix singletap and doubletap interrupt enable flags.
+ *
+ * To be on the safe side do not enable two or more concurrent interrupt events
+ * of different types.
  */

 #include <linux/bits.h>
@@ -151,6 +182,48 @@
 #define BMA220_COF_64HZ				0x4
 #define BMA220_COF_32HZ				0x5

+#define BMA220_TAP_TYPE_DOUBLE			0x0
+#define BMA220_TAP_TYPE_SINGLE			0x1
+
+static const struct iio_event_spec bma220_events[] = {
+	{
+		.type = IIO_EV_TYPE_GESTURE,
+		.dir = IIO_EV_DIR_SINGLETAP,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE)
+	},
+	{
+		.type = IIO_EV_TYPE_GESTURE,
+		.dir = IIO_EV_DIR_DOUBLETAP,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
+				       BIT(IIO_EV_INFO_PERIOD),
+	},
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_shared_by_type = BIT(IIO_EV_INFO_ENABLE) |
+				       BIT(IIO_EV_INFO_VALUE) |
+				       BIT(IIO_EV_INFO_PERIOD) |
+				       BIT(IIO_EV_INFO_HYSTERESIS),
+	},
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
+				       BIT(IIO_EV_INFO_PERIOD) |
+				       BIT(IIO_EV_INFO_HYSTERESIS),
+	},
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_EITHER,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
+				       BIT(IIO_EV_INFO_PERIOD),
+	},
+};
+
 #define BMA220_ACCEL_CHANNEL(index, reg, axis) {			\
 	.type = IIO_ACCEL,						\
 	.address = reg,							\
@@ -162,6 +235,8 @@
 	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SCALE) |\
 	    BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),		\
 	.scan_index = index,						\
+	.event_spec = bma220_events,					\
+	.num_event_specs = ARRAY_SIZE(bma220_events),			\
 	.scan_type = {							\
 		.sign = 's',						\
 		.realbits = 6,						\
@@ -193,6 +268,7 @@ struct bma220_data {
 	struct mutex lock;
 	u8 lpf_3db_freq_idx;
 	u8 range_idx;
+	u8 tap_type;
 	struct iio_trigger *trig;
 	struct {
 		s8 chans[3];
@@ -299,6 +375,12 @@ static const struct iio_trigger_ops bma220_trigger_ops = {
 	.validate_device = &iio_trigger_validate_own_device,
 };

+static int bma220_reset_int(struct bma220_data *data)
+{
+	return regmap_update_bits(data->regmap, BMA220_REG_IE1, BMA220_INT_RST_MSK,
+				  FIELD_PREP(BMA220_INT_RST_MSK, 1));
+}
+
 static irqreturn_t bma220_trigger_handler(int irq, void *p)
 {
 	int ret;
@@ -433,6 +515,564 @@ static int bma220_read_avail(struct iio_dev *indio_dev,
 	}
 }

+static int bma220_is_tap_en(struct bma220_data *data,
+			    enum iio_modifier axis,
+			    int type,
+			    bool *en)
+{
+	unsigned int reg_val, val;
+	int ret;
+
+	ret = regmap_read(data->regmap, BMA220_REG_IE0, &reg_val);
+	if (ret)
+		return ret;
+
+	switch (axis) {
+	case IIO_MOD_X:
+		*en = FIELD_GET(BMA220_INT_EN_TAP_X_MSK, reg_val);
+		break;
+	case IIO_MOD_Y:
+		*en = FIELD_GET(BMA220_INT_EN_TAP_Y_MSK, reg_val);
+		break;
+	case IIO_MOD_Z:
+		*en = FIELD_GET(BMA220_INT_EN_TAP_Z_MSK, reg_val);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (*en) {
+		ret = regmap_read(data->regmap, BMA220_REG_CONF5, &reg_val);
+		if (ret)
+			return ret;
+		val = FIELD_GET(BMA220_TIP_EN_MSK, reg_val);
+		data->tap_type = val;
+/*
+ * the tip_en reg_flag  - if '1' it enables SingleTap, '0' DoubleTap
+ * truth table for the logic below, *en has to be switched to 0 in two cases:
+ *   ST  DT  reg_val  *en
+ *   1   0   0 (DT)    0
+ *   1   0   1 (ST)    1
+ *   0   1   0 (DT)    1
+ *   0   1   1 (ST)    0
+ */
+		if ((type == IIO_EV_DIR_DOUBLETAP) && val)
+			*en = false;
+		else if ((type == IIO_EV_DIR_SINGLETAP) && !val)
+			*en = false;
+	}
+
+	return 0;
+}
+
+static int bma220_is_slope_en(struct bma220_data *data,
+			      enum iio_modifier axis,
+			      bool *en)
+{
+	unsigned int reg_val;
+	int ret;
+
+	ret = regmap_read(data->regmap, BMA220_REG_IE0, &reg_val);
+	if (ret)
+		return ret;
+
+	switch (axis) {
+	case IIO_MOD_X:
+		*en = FIELD_GET(BMA220_INT_EN_SLOPE_X_MSK, reg_val);
+		break;
+	case IIO_MOD_Y:
+		*en = FIELD_GET(BMA220_INT_EN_SLOPE_Y_MSK, reg_val);
+		break;
+	case IIO_MOD_Z:
+		*en = FIELD_GET(BMA220_INT_EN_SLOPE_Z_MSK, reg_val);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int bma220_is_high_en(struct bma220_data *data,
+			      enum iio_modifier axis,
+			      bool *en)
+{
+	unsigned int reg_val;
+	int ret;
+
+	ret = regmap_read(data->regmap, BMA220_REG_IE1, &reg_val);
+	if (ret)
+		return ret;
+
+	switch (axis) {
+	case IIO_MOD_X:
+		*en = FIELD_GET(BMA220_INT_EN_HIGH_X_MSK, reg_val);
+		break;
+	case IIO_MOD_Y:
+		*en = FIELD_GET(BMA220_INT_EN_HIGH_Y_MSK, reg_val);
+		break;
+	case IIO_MOD_Z:
+		*en = FIELD_GET(BMA220_INT_EN_HIGH_Z_MSK, reg_val);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int bma220_set_tap_en(struct bma220_data *data,
+			     enum iio_modifier axis,
+			     int type,
+			     bool en)
+{
+	unsigned int flags = BMA220_TAP_TYPE_DOUBLE;
+	int ret;
+
+	switch (axis) {
+	case IIO_MOD_X:
+		ret = regmap_update_bits(data->regmap, BMA220_REG_IE0,
+					 BMA220_INT_EN_TAP_X_MSK,
+					 FIELD_PREP(BMA220_INT_EN_TAP_X_MSK, en));
+		break;
+	case IIO_MOD_Y:
+		ret = regmap_update_bits(data->regmap, BMA220_REG_IE0,
+					 BMA220_INT_EN_TAP_Y_MSK,
+					 FIELD_PREP(BMA220_INT_EN_TAP_Y_MSK, en));
+		break;
+	case IIO_MOD_Z:
+		ret = regmap_update_bits(data->regmap, BMA220_REG_IE0,
+					 BMA220_INT_EN_TAP_Z_MSK,
+					 FIELD_PREP(BMA220_INT_EN_TAP_Z_MSK, en));
+		break;
+	default:
+		return -EINVAL;
+	}
+	if (ret)
+		return ret;
+
+	/* tip_en must be 1 to select singletap detection */
+	if (type == IIO_EV_DIR_SINGLETAP)
+		flags = BMA220_TAP_TYPE_SINGLE;
+
+	ret = regmap_update_bits(data->regmap, BMA220_REG_CONF5,
+				 BMA220_TIP_EN_MSK,
+				 FIELD_PREP(BMA220_TIP_EN_MSK, flags));
+	if (ret)
+		return ret;
+
+	data->tap_type = flags;
+
+	return 0;
+}
+
+static int bma220_set_slope_en(struct bma220_data *data,
+			       enum iio_modifier axis,
+			       bool en)
+{
+	int ret;
+
+	switch (axis) {
+	case IIO_MOD_X:
+		ret = regmap_update_bits(data->regmap, BMA220_REG_IE0,
+					 BMA220_INT_EN_SLOPE_X_MSK,
+					 FIELD_PREP(BMA220_INT_EN_SLOPE_X_MSK, en));
+		break;
+	case IIO_MOD_Y:
+		ret = regmap_update_bits(data->regmap, BMA220_REG_IE0,
+					 BMA220_INT_EN_SLOPE_Y_MSK,
+					 FIELD_PREP(BMA220_INT_EN_SLOPE_Y_MSK, en));
+		break;
+	case IIO_MOD_Z:
+		ret = regmap_update_bits(data->regmap, BMA220_REG_IE0,
+					 BMA220_INT_EN_SLOPE_Z_MSK,
+					 FIELD_PREP(BMA220_INT_EN_SLOPE_Z_MSK, en));
+		break;
+	default:
+		return -EINVAL;
+	}
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int bma220_set_high_en(struct bma220_data *data,
+			      enum iio_modifier axis,
+			      bool en)
+{
+	int ret;
+
+	switch (axis) {
+	case IIO_MOD_X:
+		ret = regmap_update_bits(data->regmap, BMA220_REG_IE1,
+					 BMA220_INT_EN_HIGH_X_MSK,
+					 FIELD_PREP(BMA220_INT_EN_HIGH_X_MSK, en));
+		break;
+	case IIO_MOD_Y:
+		ret = regmap_update_bits(data->regmap, BMA220_REG_IE1,
+					 BMA220_INT_EN_HIGH_Y_MSK,
+					 FIELD_PREP(BMA220_INT_EN_HIGH_Y_MSK, en));
+		break;
+	case IIO_MOD_Z:
+		ret = regmap_update_bits(data->regmap, BMA220_REG_IE1,
+					 BMA220_INT_EN_HIGH_Z_MSK,
+					 FIELD_PREP(BMA220_INT_EN_HIGH_Z_MSK, en));
+		break;
+	default:
+		return -EINVAL;
+	}
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+// find out if the event is enabled
+static int bma220_read_event_config(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan,
+				    enum iio_event_type type,
+				    enum iio_event_direction dir)
+{
+	struct bma220_data *data = iio_priv(indio_dev);
+	bool int_en;
+	int ret;
+	unsigned int reg_val, val;
+
+	guard(mutex)(&data->lock);
+
+	switch (type) {
+	case IIO_EV_TYPE_GESTURE:
+		switch (dir) {
+		case IIO_EV_DIR_SINGLETAP:
+			ret = bma220_is_tap_en(data, chan->channel2,
+					       IIO_EV_DIR_SINGLETAP, &int_en);
+			if (ret)
+				return ret;
+			return int_en;
+		case IIO_EV_DIR_DOUBLETAP:
+			ret = bma220_is_tap_en(data, chan->channel2,
+					       IIO_EV_DIR_DOUBLETAP, &int_en);
+			if (ret)
+				return ret;
+			return int_en;
+		default:
+			return -EINVAL;
+		}
+	case IIO_EV_TYPE_THRESH:
+		switch (dir) {
+		case IIO_EV_DIR_EITHER:
+			ret = bma220_is_slope_en(data, chan->channel2, &int_en);
+			if (ret)
+				return ret;
+			return int_en;
+		case IIO_EV_DIR_RISING:
+			ret = bma220_is_high_en(data, chan->channel2, &int_en);
+			if (ret)
+				return ret;
+			return int_en;
+		case IIO_EV_DIR_FALLING:
+			ret = regmap_read(data->regmap, BMA220_REG_IE1,
+					  &reg_val);
+			if (ret)
+				return ret;
+			val = FIELD_GET(BMA220_INT_EN_LOW_MSK, reg_val);
+			return val;
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+// set if the event is enabled
+static int bma220_write_event_config(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir,
+				     bool state)
+{
+	struct bma220_data *data = iio_priv(indio_dev);
+	int ret = 0;
+
+	guard(mutex)(&data->lock);
+
+	switch (type) {
+	case IIO_EV_TYPE_GESTURE:
+		switch (dir) {
+		case IIO_EV_DIR_SINGLETAP:
+			ret = bma220_set_tap_en(data, chan->channel2,
+						IIO_EV_DIR_SINGLETAP, state);
+			break;
+		case IIO_EV_DIR_DOUBLETAP:
+			ret = bma220_set_tap_en(data, chan->channel2,
+						IIO_EV_DIR_DOUBLETAP, state);
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	case IIO_EV_TYPE_THRESH:
+		switch (dir) {
+		case IIO_EV_DIR_EITHER:
+			ret = bma220_set_slope_en(data, chan->channel2, state);
+			break;
+		case IIO_EV_DIR_RISING:
+			ret = bma220_set_high_en(data, chan->channel2, state);
+			break;
+		case IIO_EV_DIR_FALLING:
+			ret = regmap_update_bits(data->regmap, BMA220_REG_IE1,
+						 BMA220_INT_EN_LOW_MSK,
+						 FIELD_PREP(BMA220_INT_EN_LOW_MSK, state));
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (ret)
+		return ret;
+
+	return bma220_reset_int(data);
+}
+
+static int bma220_read_event_value(struct iio_dev *indio_dev,
+				   const struct iio_chan_spec *chan,
+				   enum iio_event_type type,
+				   enum iio_event_direction dir,
+				   enum iio_event_info info,
+				   int *val, int *val2)
+{
+	struct bma220_data *data = iio_priv(indio_dev);
+	unsigned int reg_val;
+	int ret;
+
+	guard(mutex)(&data->lock);
+
+	switch (type) {
+	case IIO_EV_TYPE_GESTURE:
+		switch (info) {
+		case IIO_EV_INFO_VALUE:
+			ret = regmap_read(data->regmap, BMA220_REG_CONF3, &reg_val);
+			*val = FIELD_GET(BMA220_TT_TH_MSK, reg_val);
+			break;
+		case IIO_EV_INFO_PERIOD:
+			ret = regmap_read(data->regmap, BMA220_REG_CONF3, &reg_val);
+			*val = FIELD_GET(BMA220_TT_DUR_MSK, reg_val);
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	case IIO_EV_TYPE_THRESH:
+		switch (dir) {
+		case IIO_EV_DIR_EITHER:
+			switch (info) {
+			case IIO_EV_INFO_VALUE:
+				ret = regmap_read(data->regmap, BMA220_REG_CONF4,
+						  &reg_val);
+				*val = FIELD_GET(BMA220_SLOPE_TH_MSK, reg_val);
+				break;
+			case IIO_EV_INFO_PERIOD:
+				ret = regmap_read(data->regmap, BMA220_REG_CONF4,
+						  &reg_val);
+				*val = FIELD_GET(BMA220_SLOPE_DUR_MSK, reg_val);
+				break;
+			default:
+				return -EINVAL;
+			}
+			break;
+		case IIO_EV_DIR_RISING:
+			switch (info) {
+			case IIO_EV_INFO_VALUE:
+				ret = regmap_read(data->regmap, BMA220_REG_CONF1,
+						  &reg_val);
+				*val = FIELD_GET(BMA220_HIGH_TH_MSK, reg_val);
+				break;
+			case IIO_EV_INFO_PERIOD:
+				ret = regmap_read(data->regmap, BMA220_REG_CONF0,
+						  &reg_val);
+				*val = FIELD_GET(BMA220_HIGH_DUR_MSK, reg_val);
+				break;
+			case IIO_EV_INFO_HYSTERESIS:
+				ret = regmap_read(data->regmap, BMA220_REG_CONF0,
+						  &reg_val);
+				*val = FIELD_GET(BMA220_HIGH_HY_MSK, reg_val);
+				break;
+			default:
+				return -EINVAL;
+			}
+			break;
+		case IIO_EV_DIR_FALLING:
+			switch (info) {
+			case IIO_EV_INFO_VALUE:
+				ret = regmap_read(data->regmap, BMA220_REG_CONF1,
+						  &reg_val);
+				*val = FIELD_GET(BMA220_LOW_TH_MSK, reg_val);
+				break;
+			case IIO_EV_INFO_PERIOD:
+				ret = regmap_read(data->regmap, BMA220_REG_CONF2,
+						  &reg_val);
+				*val = FIELD_GET(BMA220_LOW_DUR_MSK, reg_val);
+				break;
+			case IIO_EV_INFO_HYSTERESIS:
+				ret = regmap_read(data->regmap, BMA220_REG_CONF2,
+						  &reg_val);
+				*val = FIELD_GET(BMA220_LOW_HY_MSK, reg_val);
+				break;
+			default:
+				return -EINVAL;
+			}
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (ret)
+		return ret;
+
+	return IIO_VAL_INT;
+}
+
+static int bma220_write_event_value(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan,
+				    enum iio_event_type type,
+				    enum iio_event_direction dir,
+				    enum iio_event_info info,
+				    int val, int val2)
+{
+	struct bma220_data *data = iio_priv(indio_dev);
+	int ret;
+
+	guard(mutex)(&data->lock);
+
+	switch (type) {
+	case IIO_EV_TYPE_GESTURE:
+		switch (info) {
+		case IIO_EV_INFO_VALUE:
+			if (!FIELD_FIT(BMA220_TT_TH_MSK, val))
+				return -EINVAL;
+			ret = regmap_update_bits(data->regmap, BMA220_REG_CONF3,
+						 BMA220_TT_TH_MSK,
+						 FIELD_PREP(BMA220_TT_TH_MSK, val));
+			break;
+		case IIO_EV_INFO_PERIOD:
+			if (!FIELD_FIT(BMA220_TT_DUR_MSK, val))
+				return -EINVAL;
+			ret = regmap_update_bits(data->regmap, BMA220_REG_CONF3,
+						 BMA220_TT_DUR_MSK,
+						 FIELD_PREP(BMA220_TT_DUR_MSK, val));
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	case IIO_EV_TYPE_THRESH:
+		switch (dir) {
+		case IIO_EV_DIR_EITHER:
+			switch (info) {
+			case IIO_EV_INFO_VALUE:
+				if (!FIELD_FIT(BMA220_SLOPE_TH_MSK, val))
+					return -EINVAL;
+				ret = regmap_update_bits(data->regmap,
+							 BMA220_REG_CONF4,
+							 BMA220_SLOPE_TH_MSK,
+							 FIELD_PREP(BMA220_SLOPE_TH_MSK, val));
+				break;
+			case IIO_EV_INFO_PERIOD:
+				if (!FIELD_FIT(BMA220_SLOPE_DUR_MSK, val))
+					return -EINVAL;
+				ret = regmap_update_bits(data->regmap,
+							 BMA220_REG_CONF4,
+							 BMA220_SLOPE_DUR_MSK,
+							 FIELD_PREP(BMA220_SLOPE_DUR_MSK, val));
+				break;
+			default:
+				return -EINVAL;
+			}
+			break;
+		case IIO_EV_DIR_RISING:
+			switch (info) {
+			case IIO_EV_INFO_VALUE:
+				if (!FIELD_FIT(BMA220_HIGH_TH_MSK, val))
+					return -EINVAL;
+				ret = regmap_update_bits(data->regmap,
+							 BMA220_REG_CONF1,
+							 BMA220_HIGH_TH_MSK,
+							 FIELD_PREP(BMA220_HIGH_TH_MSK, val));
+				break;
+			case IIO_EV_INFO_PERIOD:
+				if (!FIELD_FIT(BMA220_HIGH_DUR_MSK, val))
+					return -EINVAL;
+				ret = regmap_update_bits(data->regmap,
+							 BMA220_REG_CONF0,
+							 BMA220_HIGH_DUR_MSK,
+							 FIELD_PREP(BMA220_HIGH_DUR_MSK, val));
+				break;
+			case IIO_EV_INFO_HYSTERESIS:
+				if (!FIELD_FIT(BMA220_HIGH_HY_MSK, val))
+					return -EINVAL;
+				ret = regmap_update_bits(data->regmap,
+							 BMA220_REG_CONF0,
+							 BMA220_HIGH_HY_MSK,
+							 FIELD_PREP(BMA220_HIGH_HY_MSK, val));
+				break;
+			default:
+				return -EINVAL;
+			}
+			break;
+		case IIO_EV_DIR_FALLING:
+			switch (info) {
+			case IIO_EV_INFO_VALUE:
+				if (!FIELD_FIT(BMA220_LOW_TH_MSK, val))
+					return -EINVAL;
+				ret = regmap_update_bits(data->regmap,
+							 BMA220_REG_CONF1,
+							 BMA220_LOW_TH_MSK,
+							 FIELD_PREP(BMA220_LOW_TH_MSK, val));
+				break;
+			case IIO_EV_INFO_PERIOD:
+				if (!FIELD_FIT(BMA220_LOW_DUR_MSK, val))
+					return -EINVAL;
+				ret = regmap_update_bits(data->regmap,
+							 BMA220_REG_CONF2,
+							 BMA220_LOW_DUR_MSK,
+							 FIELD_PREP(BMA220_LOW_DUR_MSK, val));
+				break;
+			case IIO_EV_INFO_HYSTERESIS:
+				if (!FIELD_FIT(BMA220_LOW_HY_MSK, val))
+					return -EINVAL;
+				ret = regmap_update_bits(data->regmap,
+							 BMA220_REG_CONF2,
+							 BMA220_LOW_HY_MSK,
+							 FIELD_PREP(BMA220_LOW_HY_MSK, val));
+				break;
+			default:
+				return -EINVAL;
+			}
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 static int bma220_reg_access(struct iio_dev *indio_dev, unsigned int reg,
 			     unsigned int writeval, unsigned int *readval)
 {
@@ -447,6 +1087,10 @@ static const struct iio_info bma220_info = {
 	.read_raw		= bma220_read_raw,
 	.write_raw		= bma220_write_raw,
 	.read_avail		= bma220_read_avail,
+	.read_event_config	= bma220_read_event_config,
+	.write_event_config	= bma220_write_event_config,
+	.read_event_value	= bma220_read_event_value,
+	.write_event_value	= bma220_write_event_value,
 	.debugfs_reg_access	= &bma220_reg_access,
 };

@@ -563,6 +1207,8 @@ static int bma220_init(struct bma220_data *data)
 		}
 	}

+	data->tap_type = BMA220_TAP_TYPE_DOUBLE;
+
 	return 0;
 }

@@ -584,6 +1230,7 @@ static irqreturn_t bma220_irq_handler(int irq, void *private)
 	struct bma220_data *data = iio_priv(indio_dev);
 	int rv;
 	u8 bma220_reg_if[2];
+	s64 timestamp = iio_get_time_ns(indio_dev);

 	guard(mutex)(&data->lock);
 	rv = regmap_bulk_read(data->regmap, BMA220_REG_IF0, bma220_reg_if,
@@ -596,6 +1243,45 @@ static irqreturn_t bma220_irq_handler(int irq, void *private)
 		goto done;
 	}

+	if (FIELD_GET(BMA220_IF_HIGH, bma220_reg_if[1]))
+		iio_push_event(indio_dev,
+			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+						  IIO_MOD_X_OR_Y_OR_Z,
+						  IIO_EV_TYPE_THRESH,
+						  IIO_EV_DIR_RISING),
+			       timestamp);
+	if (FIELD_GET(BMA220_IF_LOW, bma220_reg_if[1]))
+		iio_push_event(indio_dev,
+			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+						  IIO_MOD_X_OR_Y_OR_Z,
+						  IIO_EV_TYPE_THRESH,
+						  IIO_EV_DIR_FALLING),
+			       timestamp);
+	if (FIELD_GET(BMA220_IF_SLOPE, bma220_reg_if[1]))
+		iio_push_event(indio_dev,
+			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+						  IIO_MOD_X_OR_Y_OR_Z,
+						  IIO_EV_TYPE_THRESH,
+						  IIO_EV_DIR_EITHER),
+			       timestamp);
+	if (FIELD_GET(BMA220_IF_TT, bma220_reg_if[1])) {
+
+		if (data->tap_type == BMA220_TAP_TYPE_SINGLE)
+			iio_push_event(indio_dev,
+				       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+							  IIO_MOD_X_OR_Y_OR_Z,
+							  IIO_EV_TYPE_GESTURE,
+							  IIO_EV_DIR_SINGLETAP),
+				       timestamp);
+		else
+			iio_push_event(indio_dev,
+				       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+							  IIO_MOD_X_OR_Y_OR_Z,
+							  IIO_EV_TYPE_GESTURE,
+							  IIO_EV_DIR_DOUBLETAP),
+				       timestamp);
+	}
+
 done:

 	return IRQ_HANDLED;
--
2.49.1

Re: [PATCH 08/10] iio: accel: BMA220 add events

[Jonathan Cameron]

On Mon,  1 Sep 2025 22:47:34 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> Add events for tap detection and low-g, high-g, slope conditions.
> Ignored the 80-column rule for readability.
> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
It might be worth splitting this up into types of event just to reduce
the amount of code and allow some description of event types in the patch
intro.

> ---
>  drivers/iio/accel/bma220_core.c | 686 ++++++++++++++++++++++++++++++++
>  1 file changed, 686 insertions(+)
> 
> diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
> index 41889cdcef76..c8da6cc2eaf3 100644
> --- a/drivers/iio/accel/bma220_core.c
> +++ b/drivers/iio/accel/bma220_core.c
> @@ -13,6 +13,37 @@
>   * scale (range)             | in_accel_scale                 | see _available
>   * cut-off freq (filt_config)| in_accel_filter_low_pass_...   | see _available
>   * ---------------------------------------------------------------------------
> + *
> + * Events:
> + * Register                  | IIO ABI (sysfs)                   | valid values
> + * --------------------------+-----------------------------------+------------

This feels like documentation that ultimately may grow to the point
where it isn't useful enough to justify it's presence. Maybe we'll see what
this looks like once all the code is ready to merge and then consider if it
is worth having this.

> + * high-g detection          |                                   |
> + *   enable/disable irq      | in_accel_*_thresh_rising_en       | 0/1
> + *   threshold (high_th)     | in_accel_thresh_rising_value      | 0-15
> + *   hysteresis (high_hy)    | in_accel_thresh_rising_hysteresis | 0-3
> + *   duration (high_dur)     | in_accel_thresh_rising_period     | 0-63
> + * low-g detection           |                                   |
> + *   enable/disable irq      | in_accel_*_thresh_falling_en      | 0/1
> + *   threshold (low_th)      | in_accel_thresh_falling_value     | 0-15
> + *   hysteresis (low_hy)     | in_accel_thresh_falling_hysteresis| 0-3
> + *   duration (low_dur)      | in_accel_thresh_falling_period    | 0-63
> + * slope detection           |                                   |
> + *   enable/disable irq      | in_accel_*_thresh_either_en       | 0/1
> + *   threshold (slope_th)    | in_accel_thresh_either_value      | 0-15
> + *   duration (slope_dur)    | in_accel_thresh_either_period     | 0-3
> + * tap sensing               |                                   |
> + *   enable/disable singletap| in_accel_*_gesture_singletap_en   | 0/1 [2]
> + *   enable/disable doubletap| in_accel_*_gesture_doubletap_en   | 0/1 [2]
> + *   threshold (tt_th)       | in_accel_gesture_singletap_value  | 0-15
> + *   duration (tt_dur)       | in_accel_gesture_doubletap_period | see [1]
> + * ----------------------------------------------------------------------------
> + *
> + * [1] The event related sysfs interface provides and expects raw register values
> + *         (unshifted bitfields) based on the chip specifications.
> + * [2] Do not mix singletap and doubletap interrupt enable flags.
> + *
> + * To be on the safe side do not enable two or more concurrent interrupt events
> + * of different types.
>   */

> 
> +static int bma220_reset_int(struct bma220_data *data)
> +{
> +	return regmap_update_bits(data->regmap, BMA220_REG_IE1, BMA220_INT_RST_MSK,
> +				  FIELD_PREP(BMA220_INT_RST_MSK, 1));

regmap_set_bits() is handy in these cases.

> +}
>

> +
> +static int bma220_set_slope_en(struct bma220_data *data,
> +			       enum iio_modifier axis,
> +			       bool en)

Wrap closer to 80 chars.

> +
> +static int bma220_set_high_en(struct bma220_data *data,
> +			      enum iio_modifier axis,
> +			      bool en)
> +{
> +	int ret;
> +
> +	switch (axis) {
> +	case IIO_MOD_X:
> +		ret = regmap_update_bits(data->regmap, BMA220_REG_IE1,
> +					 BMA220_INT_EN_HIGH_X_MSK,
> +					 FIELD_PREP(BMA220_INT_EN_HIGH_X_MSK, en));
> +		break;
> +	case IIO_MOD_Y:
> +		ret = regmap_update_bits(data->regmap, BMA220_REG_IE1,
> +					 BMA220_INT_EN_HIGH_Y_MSK,
> +					 FIELD_PREP(BMA220_INT_EN_HIGH_Y_MSK, en));
> +		break;
> +	case IIO_MOD_Z:
> +		ret = regmap_update_bits(data->regmap, BMA220_REG_IE1,
> +					 BMA220_INT_EN_HIGH_Z_MSK,
> +					 FIELD_PREP(BMA220_INT_EN_HIGH_Z_MSK, en));
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	if (ret)

As below. Early returns simplify this.

> +		return ret;
> +
> +	return 0;
return ret;

> +}
>
> +// set if the event is enabled

All IIO comments are /* */ except the SPDX tags where appropriate.


> +static int bma220_write_event_config(struct iio_dev *indio_dev,
> +				     const struct iio_chan_spec *chan,
> +				     enum iio_event_type type,
> +				     enum iio_event_direction dir,
> +				     bool state)
> +{
> +	struct bma220_data *data = iio_priv(indio_dev);
> +	int ret = 0;
> +
> +	guard(mutex)(&data->lock);
> +
> +	switch (type) {
> +	case IIO_EV_TYPE_GESTURE:
> +		switch (dir) {
> +		case IIO_EV_DIR_SINGLETAP:
> +			ret = bma220_set_tap_en(data, chan->channel2,
> +						IIO_EV_DIR_SINGLETAP, state);
> +			break;
> +		case IIO_EV_DIR_DOUBLETAP:
> +			ret = bma220_set_tap_en(data, chan->channel2,
> +						IIO_EV_DIR_DOUBLETAP, state);
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	case IIO_EV_TYPE_THRESH:
> +		switch (dir) {
> +		case IIO_EV_DIR_EITHER:
> +			ret = bma220_set_slope_en(data, chan->channel2, state);
> +			break;
> +		case IIO_EV_DIR_RISING:
> +			ret = bma220_set_high_en(data, chan->channel2, state);
> +			break;
> +		case IIO_EV_DIR_FALLING:
> +			ret = regmap_update_bits(data->regmap, BMA220_REG_IE1,
> +						 BMA220_INT_EN_LOW_MSK,
> +						 FIELD_PREP(BMA220_INT_EN_LOW_MSK, state));
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (ret)
> +		return ret;
> +
> +	return bma220_reset_int(data);
> +}
> +
> +static int bma220_read_event_value(struct iio_dev *indio_dev,
> +				   const struct iio_chan_spec *chan,
> +				   enum iio_event_type type,
> +				   enum iio_event_direction dir,
> +				   enum iio_event_info info,
> +				   int *val, int *val2)
> +{
> +	struct bma220_data *data = iio_priv(indio_dev);
> +	unsigned int reg_val;
> +	int ret;
> +
> +	guard(mutex)(&data->lock);
> +
> +	switch (type) {
> +	case IIO_EV_TYPE_GESTURE:
> +		switch (info) {
> +		case IIO_EV_INFO_VALUE:
> +			ret = regmap_read(data->regmap, BMA220_REG_CONF3, &reg_val);
Whilst it is more code, convention would be to be side effect free on error so.
			if (ret)
				return ret;

			*val = FIELD_GET(B...);
			return IIO_VAL_INT;

> +			*val = FIELD_GET(BMA220_TT_TH_MSK, reg_val);
> +			break;
> +		case IIO_EV_INFO_PERIOD:
> +			ret = regmap_read(data->regmap, BMA220_REG_CONF3, &reg_val);
> +			*val = FIELD_GET(BMA220_TT_DUR_MSK, reg_val);
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	case IIO_EV_TYPE_THRESH:
> +		switch (dir) {
> +		case IIO_EV_DIR_EITHER:
> +			switch (info) {
> +			case IIO_EV_INFO_VALUE:
> +				ret = regmap_read(data->regmap, BMA220_REG_CONF4,
> +						  &reg_val);
> +				*val = FIELD_GET(BMA220_SLOPE_TH_MSK, reg_val);
> +				break;
> +			case IIO_EV_INFO_PERIOD:
> +				ret = regmap_read(data->regmap, BMA220_REG_CONF4,
> +						  &reg_val);
> +				*val = FIELD_GET(BMA220_SLOPE_DUR_MSK, reg_val);
> +				break;
> +			default:
> +				return -EINVAL;
> +			}
> +			break;
> +		case IIO_EV_DIR_RISING:
> +			switch (info) {
> +			case IIO_EV_INFO_VALUE:
> +				ret = regmap_read(data->regmap, BMA220_REG_CONF1,
> +						  &reg_val);
> +				*val = FIELD_GET(BMA220_HIGH_TH_MSK, reg_val);
> +				break;
> +			case IIO_EV_INFO_PERIOD:
> +				ret = regmap_read(data->regmap, BMA220_REG_CONF0,
> +						  &reg_val);
> +				*val = FIELD_GET(BMA220_HIGH_DUR_MSK, reg_val);
> +				break;
> +			case IIO_EV_INFO_HYSTERESIS:
> +				ret = regmap_read(data->regmap, BMA220_REG_CONF0,
> +						  &reg_val);
> +				*val = FIELD_GET(BMA220_HIGH_HY_MSK, reg_val);
> +				break;
> +			default:
> +				return -EINVAL;
> +			}
> +			break;
> +		case IIO_EV_DIR_FALLING:
> +			switch (info) {
> +			case IIO_EV_INFO_VALUE:
> +				ret = regmap_read(data->regmap, BMA220_REG_CONF1,
> +						  &reg_val);
> +				*val = FIELD_GET(BMA220_LOW_TH_MSK, reg_val);
> +				break;
> +			case IIO_EV_INFO_PERIOD:
> +				ret = regmap_read(data->regmap, BMA220_REG_CONF2,
> +						  &reg_val);
> +				*val = FIELD_GET(BMA220_LOW_DUR_MSK, reg_val);
> +				break;
> +			case IIO_EV_INFO_HYSTERESIS:
> +				ret = regmap_read(data->regmap, BMA220_REG_CONF2,
> +						  &reg_val);
> +				*val = FIELD_GET(BMA220_LOW_HY_MSK, reg_val);
> +				break;
> +			default:
> +				return -EINVAL;
> +			}
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (ret)
> +		return ret;
> +
> +	return IIO_VAL_INT;
As below. I'd do early returns.  you may end up with a few more checks
but the code will be easier to follow (slightly!)
> +}
> +
> +static int bma220_write_event_value(struct iio_dev *indio_dev,
> +				    const struct iio_chan_spec *chan,
> +				    enum iio_event_type type,
> +				    enum iio_event_direction dir,
> +				    enum iio_event_info info,
> +				    int val, int val2)
> +{
> +	struct bma220_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	guard(mutex)(&data->lock);
> +
> +	switch (type) {
> +	case IIO_EV_TYPE_GESTURE:
> +		switch (info) {
> +		case IIO_EV_INFO_VALUE:
> +			if (!FIELD_FIT(BMA220_TT_TH_MSK, val))
> +				return -EINVAL;
> +			ret = regmap_update_bits(data->regmap, BMA220_REG_CONF3,
> +						 BMA220_TT_TH_MSK,
> +						 FIELD_PREP(BMA220_TT_TH_MSK, val));
I'm a fan of returning early if there is nothing else to do.  Saves
scrolling up and down if trying to follow the code flow for a specific
set of inputs.
			return regmap_update_bits();

> +			break;
> +		case IIO_EV_INFO_PERIOD:
> +			if (!FIELD_FIT(BMA220_TT_DUR_MSK, val))
> +				return -EINVAL;
> +			ret = regmap_update_bits(data->regmap, BMA220_REG_CONF3,
> +						 BMA220_TT_DUR_MSK,
> +						 FIELD_PREP(BMA220_TT_DUR_MSK, val));
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +		break;

With early returns this will be dead code that should be dropped.

> +	case IIO_EV_TYPE_THRESH:
> +		switch (dir) {
> +		case IIO_EV_DIR_EITHER:
> +			switch (info) {
> +			case IIO_EV_INFO_VALUE:
> +				if (!FIELD_FIT(BMA220_SLOPE_TH_MSK, val))
> +					return -EINVAL;
> +				ret = regmap_update_bits(data->regmap,
> +							 BMA220_REG_CONF4,
> +							 BMA220_SLOPE_TH_MSK,
> +							 FIELD_PREP(BMA220_SLOPE_TH_MSK, val));
> +				break;
> +			case IIO_EV_INFO_PERIOD:
> +				if (!FIELD_FIT(BMA220_SLOPE_DUR_MSK, val))
> +					return -EINVAL;
> +				ret = regmap_update_bits(data->regmap,
> +							 BMA220_REG_CONF4,
> +							 BMA220_SLOPE_DUR_MSK,
> +							 FIELD_PREP(BMA220_SLOPE_DUR_MSK, val));
> +				break;
> +			default:
> +				return -EINVAL;
> +			}
> +			break;
> +		case IIO_EV_DIR_RISING:
> +			switch (info) {
> +			case IIO_EV_INFO_VALUE:
> +				if (!FIELD_FIT(BMA220_HIGH_TH_MSK, val))
> +					return -EINVAL;
> +				ret = regmap_update_bits(data->regmap,
> +							 BMA220_REG_CONF1,
> +							 BMA220_HIGH_TH_MSK,
> +							 FIELD_PREP(BMA220_HIGH_TH_MSK, val));
> +				break;
> +			case IIO_EV_INFO_PERIOD:
> +				if (!FIELD_FIT(BMA220_HIGH_DUR_MSK, val))
> +					return -EINVAL;
> +				ret = regmap_update_bits(data->regmap,
> +							 BMA220_REG_CONF0,
> +							 BMA220_HIGH_DUR_MSK,
> +							 FIELD_PREP(BMA220_HIGH_DUR_MSK, val));
> +				break;
> +			case IIO_EV_INFO_HYSTERESIS:
> +				if (!FIELD_FIT(BMA220_HIGH_HY_MSK, val))
> +					return -EINVAL;
> +				ret = regmap_update_bits(data->regmap,
> +							 BMA220_REG_CONF0,
> +							 BMA220_HIGH_HY_MSK,
> +							 FIELD_PREP(BMA220_HIGH_HY_MSK, val));
> +				break;
> +			default:
> +				return -EINVAL;
> +			}
> +			break;
> +		case IIO_EV_DIR_FALLING:
> +			switch (info) {
> +			case IIO_EV_INFO_VALUE:
> +				if (!FIELD_FIT(BMA220_LOW_TH_MSK, val))
> +					return -EINVAL;
> +				ret = regmap_update_bits(data->regmap,
> +							 BMA220_REG_CONF1,
> +							 BMA220_LOW_TH_MSK,
> +							 FIELD_PREP(BMA220_LOW_TH_MSK, val));
> +				break;
> +			case IIO_EV_INFO_PERIOD:
> +				if (!FIELD_FIT(BMA220_LOW_DUR_MSK, val))
> +					return -EINVAL;
> +				ret = regmap_update_bits(data->regmap,
> +							 BMA220_REG_CONF2,
> +							 BMA220_LOW_DUR_MSK,
> +							 FIELD_PREP(BMA220_LOW_DUR_MSK, val));
> +				break;
> +			case IIO_EV_INFO_HYSTERESIS:
> +				if (!FIELD_FIT(BMA220_LOW_HY_MSK, val))
> +					return -EINVAL;
> +				ret = regmap_update_bits(data->regmap,
> +							 BMA220_REG_CONF2,
> +							 BMA220_LOW_HY_MSK,
> +							 FIELD_PREP(BMA220_LOW_HY_MSK, val));
> +				break;
> +			default:
> +				return -EINVAL;
> +			}
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
With all early returns as above this isn't needed as we'll never get here.

> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}

> 
> @@ -584,6 +1230,7 @@ static irqreturn_t bma220_irq_handler(int irq, void *private)
>  	struct bma220_data *data = iio_priv(indio_dev);
>  	int rv;
>  	u8 bma220_reg_if[2];
> +	s64 timestamp = iio_get_time_ns(indio_dev);
> 
>  	guard(mutex)(&data->lock);
>  	rv = regmap_bulk_read(data->regmap, BMA220_REG_IF0, bma220_reg_if,
> @@ -596,6 +1243,45 @@ static irqreturn_t bma220_irq_handler(int irq, void *private)
>  		goto done;
>  	}
> 
> +	if (FIELD_GET(BMA220_IF_HIGH, bma220_reg_if[1]))
> +		iio_push_event(indio_dev,
> +			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
> +						  IIO_MOD_X_OR_Y_OR_Z,
> +						  IIO_EV_TYPE_THRESH,
> +						  IIO_EV_DIR_RISING),
> +			       timestamp);
> +	if (FIELD_GET(BMA220_IF_LOW, bma220_reg_if[1]))
> +		iio_push_event(indio_dev,
> +			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
> +						  IIO_MOD_X_OR_Y_OR_Z,
> +						  IIO_EV_TYPE_THRESH,
> +						  IIO_EV_DIR_FALLING),
> +			       timestamp);
> +	if (FIELD_GET(BMA220_IF_SLOPE, bma220_reg_if[1]))
> +		iio_push_event(indio_dev,

Sounds like a ROC event. (Rate of change) not a simple threshold.

> +			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
> +						  IIO_MOD_X_OR_Y_OR_Z,
> +						  IIO_EV_TYPE_THRESH,
> +						  IIO_EV_DIR_EITHER),
> +			       timestamp);
> +	if (FIELD_GET(BMA220_IF_TT, bma220_reg_if[1])) {
> +
> +		if (data->tap_type == BMA220_TAP_TYPE_SINGLE)
> +			iio_push_event(indio_dev,
> +				       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
> +							  IIO_MOD_X_OR_Y_OR_Z,
> +							  IIO_EV_TYPE_GESTURE,
> +							  IIO_EV_DIR_SINGLETAP),
> +				       timestamp);
> +		else
> +			iio_push_event(indio_dev,
> +				       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
> +							  IIO_MOD_X_OR_Y_OR_Z,
> +							  IIO_EV_TYPE_GESTURE,
> +							  IIO_EV_DIR_DOUBLETAP),
> +				       timestamp);
> +	}

[PATCH 09/10] iio: accel: BMA220 add event attrs

[Petre Rodan]

Add event attributes not directly covered by the IIO API.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
 drivers/iio/accel/bma220_core.c | 178 +++++++++++++++++++++++++++++++-
 1 file changed, 177 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index c8da6cc2eaf3..7a1064b408bb 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -31,11 +31,18 @@
  *   enable/disable irq      | in_accel_*_thresh_either_en       | 0/1
  *   threshold (slope_th)    | in_accel_thresh_either_value      | 0-15
  *   duration (slope_dur)    | in_accel_thresh_either_period     | 0-3
+ *   filter (slope_filt)     | in_accel_slope_filt               | 0/1
+ * orientation               |                                   |
+ *   mounting (orient_ex)    | in_accel_orient_ex                | 0/1
+ *   blocking                | in_accel_orient_blocking          | see [1]
  * tap sensing               |                                   |
  *   enable/disable singletap| in_accel_*_gesture_singletap_en   | 0/1 [2]
  *   enable/disable doubletap| in_accel_*_gesture_doubletap_en   | 0/1 [2]
  *   threshold (tt_th)       | in_accel_gesture_singletap_value  | 0-15
  *   duration (tt_dur)       | in_accel_gesture_doubletap_period | see [1]
+ *   sample cnt (tt_samp)    | in_accel_gesture_tap_sample_cnt   | see [1]
+ *   filter (tt_filt)        | in_accel_gesture_tap_filt         | 0/1
+ * latch time (lat_int)      | in_accel_latch_time               | see [1]
  * ----------------------------------------------------------------------------
  *
  * [1] The event related sysfs interface provides and expects raw register values
@@ -269,6 +276,7 @@ struct bma220_data {
 	u8 lpf_3db_freq_idx;
 	u8 range_idx;
 	u8 tap_type;
+	bool irq_needs_clear_if;
 	struct iio_trigger *trig;
 	struct {
 		s8 chans[3];
@@ -1083,7 +1091,167 @@ static int bma220_reg_access(struct iio_dev *indio_dev, unsigned int reg,
 	return regmap_write(data->regmap, reg, writeval);
 }

+/* Event related attributes not directly covered by the IIO API */
+enum bma220_attributes {
+	BMA220_ATTR_TT_FILT,
+	BMA220_ATTR_TT_SAMP,
+	BMA220_ATTR_SLOPE_FILT,
+	BMA220_ATTR_ORIENT_EX,
+	BMA220_ATTR_ORIENT_BLOCKING,
+	BMA220_ATTR_LATCH_TIME,
+};
+
+static ssize_t event_attr_reg_show(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf)
+{
+	struct bma220_data *data = iio_priv(dev_to_iio_dev(dev));
+	struct iio_dev_attr *iattr = to_iio_dev_attr(attr);
+	int ret = -EINVAL;
+	unsigned int reg_val, flags;
+
+	guard(mutex)(&data->lock);
+
+	switch (iattr->address) {
+	case BMA220_ATTR_TT_FILT:
+		ret = regmap_read(data->regmap, BMA220_REG_CONF3, &reg_val);
+		flags = FIELD_GET(BMA220_TT_FILT_MSK, reg_val);
+		break;
+	case BMA220_ATTR_TT_SAMP:
+		ret = regmap_read(data->regmap, BMA220_REG_CONF5, &reg_val);
+		flags = FIELD_GET(BMA220_TT_SAMP_MSK, reg_val);
+		break;
+	case BMA220_ATTR_SLOPE_FILT:
+		ret = regmap_read(data->regmap, BMA220_REG_CONF4, &reg_val);
+		flags = FIELD_GET(BMA220_SLOPE_FILT_MSK, reg_val);
+		break;
+	case BMA220_ATTR_ORIENT_EX:
+		ret = regmap_read(data->regmap, BMA220_REG_CONF4, &reg_val);
+		flags = FIELD_GET(BMA220_ORIENT_EX_MSK, reg_val);
+		break;
+	case BMA220_ATTR_ORIENT_BLOCKING:
+		ret = regmap_read(data->regmap, BMA220_REG_CONF5, &reg_val);
+		flags = FIELD_GET(BMA220_ORIENT_BLOCKING_MSK, reg_val);
+		break;
+	case BMA220_ATTR_LATCH_TIME:
+		ret = regmap_read(data->regmap, BMA220_REG_IE1, &reg_val);
+		flags = FIELD_GET(BMA220_INT_LATCH_MSK, reg_val);
+		break;
+	}
+
+	if (ret)
+		return ret;
+	return sysfs_emit(buf, "%d\n", flags);
+}
+
+static ssize_t event_attr_reg_store(struct device *dev,
+					    struct device_attribute *attr,
+					    const char *buf, size_t len)
+{
+	struct bma220_data *data = iio_priv(dev_to_iio_dev(dev));
+	struct iio_dev_attr *iattr = to_iio_dev_attr(attr);
+	int ret = -EINVAL;
+	int value;
+
+	if (kstrtoint(buf, 0, &value))
+		return -EINVAL;
+
+	guard(mutex)(&data->lock);
+
+	switch (iattr->address) {
+	case BMA220_ATTR_TT_FILT:
+		if (!FIELD_FIT(BMA220_TT_FILT_MSK, value))
+			return -EINVAL;
+		ret = regmap_update_bits(data->regmap, BMA220_REG_CONF3,
+					 BMA220_TT_FILT_MSK,
+					 FIELD_PREP(BMA220_TT_FILT_MSK, value));
+		break;
+	case BMA220_ATTR_TT_SAMP:
+		if (!FIELD_FIT(BMA220_TT_SAMP_MSK, value))
+			return -EINVAL;
+		ret = regmap_update_bits(data->regmap, BMA220_REG_CONF5,
+					 BMA220_TT_SAMP_MSK,
+					 FIELD_PREP(BMA220_TT_SAMP_MSK, value));
+		break;
+	case BMA220_ATTR_SLOPE_FILT:
+		if (!FIELD_FIT(BMA220_SLOPE_FILT_MSK, value))
+			return -EINVAL;
+		ret = regmap_update_bits(data->regmap, BMA220_REG_CONF4,
+					 BMA220_SLOPE_FILT_MSK,
+					 FIELD_PREP(BMA220_SLOPE_FILT_MSK, value));
+		break;
+	case BMA220_ATTR_ORIENT_EX:
+		if (!FIELD_FIT(BMA220_ORIENT_EX_MSK, value))
+			return -EINVAL;
+		ret = regmap_update_bits(data->regmap, BMA220_REG_CONF4,
+					 BMA220_ORIENT_EX_MSK,
+					 FIELD_PREP(BMA220_ORIENT_EX_MSK, value));
+		break;
+	case BMA220_ATTR_ORIENT_BLOCKING:
+		if (!FIELD_FIT(BMA220_ORIENT_BLOCKING_MSK, value))
+			return -EINVAL;
+		ret = regmap_update_bits(data->regmap, BMA220_REG_CONF5,
+					 BMA220_ORIENT_BLOCKING_MSK,
+					 FIELD_PREP(BMA220_ORIENT_BLOCKING_MSK,
+						    value));
+		break;
+	case BMA220_ATTR_LATCH_TIME:
+		if (!FIELD_FIT(BMA220_INT_LATCH_MSK, value))
+			return -EINVAL;
+		ret = regmap_update_bits(data->regmap, BMA220_REG_IE1,
+					 BMA220_INT_LATCH_MSK,
+					 FIELD_PREP(BMA220_INT_LATCH_MSK, value));
+		if (value == BMA220_INT_LATCH_MAX)
+			data->irq_needs_clear_if = true;
+		break;
+	}
+
+	if (ret < 0)
+		return -EINVAL;
+
+	return len;
+}
+
+static IIO_DEVICE_ATTR(in_accel_gesture_tap_filt, 0644,
+		       event_attr_reg_show, event_attr_reg_store,
+		       BMA220_ATTR_TT_FILT);
+
+static IIO_DEVICE_ATTR(in_accel_gesture_tap_sample_cnt, 0644,
+		       event_attr_reg_show, event_attr_reg_store,
+		       BMA220_ATTR_TT_SAMP);
+
+static IIO_DEVICE_ATTR(in_accel_thresh_either_filt, 0644,
+		       event_attr_reg_show, event_attr_reg_store,
+		       BMA220_ATTR_SLOPE_FILT);
+
+static IIO_DEVICE_ATTR(in_accel_orient_ex, 0644,
+		       event_attr_reg_show, event_attr_reg_store,
+		       BMA220_ATTR_ORIENT_EX);
+
+static IIO_DEVICE_ATTR(in_accel_orient_blocking, 0644,
+		       event_attr_reg_show, event_attr_reg_store,
+		       BMA220_ATTR_ORIENT_BLOCKING);
+
+static IIO_DEVICE_ATTR(in_accel_latch_time, 0644,
+		       event_attr_reg_show, event_attr_reg_store,
+		       BMA220_ATTR_LATCH_TIME);
+
+static struct attribute *bma220_event_attributes[] = {
+	&iio_dev_attr_in_accel_gesture_tap_filt.dev_attr.attr,
+	&iio_dev_attr_in_accel_gesture_tap_sample_cnt.dev_attr.attr,
+	&iio_dev_attr_in_accel_thresh_either_filt.dev_attr.attr,
+	&iio_dev_attr_in_accel_orient_ex.dev_attr.attr,
+	&iio_dev_attr_in_accel_orient_blocking.dev_attr.attr,
+	&iio_dev_attr_in_accel_latch_time.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group bma220_event_attribute_group = {
+	.attrs = bma220_event_attributes,
+};
+
 static const struct iio_info bma220_info = {
+	.event_attrs		= &bma220_event_attribute_group,
 	.read_raw		= bma220_read_raw,
 	.write_raw		= bma220_write_raw,
 	.read_avail		= bma220_read_avail,
@@ -1207,6 +1375,7 @@ static int bma220_init(struct bma220_data *data)
 		}
 	}

+	data->irq_needs_clear_if = false;
 	data->tap_type = BMA220_TAP_TYPE_DOUBLE;

 	return 0;
@@ -1228,7 +1397,7 @@ static irqreturn_t bma220_irq_handler(int irq, void *private)
 {
 	struct iio_dev *indio_dev = private;
 	struct bma220_data *data = iio_priv(indio_dev);
-	int rv;
+	int rv, ret;
 	u8 bma220_reg_if[2];
 	s64 timestamp = iio_get_time_ns(indio_dev);

@@ -1283,6 +1452,13 @@ static irqreturn_t bma220_irq_handler(int irq, void *private)
 	}

 done:
+	if (data->irq_needs_clear_if) {
+		ret = bma220_reset_int(data);
+		if (ret)
+			dev_warn(data->dev,
+				 "Failed to clear interrupt flag (%pe)\n",
+				 ERR_PTR(ret));
+	}

 	return IRQ_HANDLED;
 }
--
2.49.1

Re: [PATCH 09/10] iio: accel: BMA220 add event attrs

[Jonathan Cameron]

On Mon,  1 Sep 2025 22:47:35 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> Add event attributes not directly covered by the IIO API.

These must be accompanied by ABI documentation in
Documentation/ABI/testing/sysfs-bus-iio-...

We need to pull out of the datasheet generic descriptions of what
they are so we can consider if they make sense as general new ABI
or perhaps map to something existing.  In some cases it is more
appropriate just to set a reasonable default and not provide a
userspace ABI at all.

Key point is that custom ABI is effectively unused ABI because most
software is written against a bunch of devices and has no idea what
the new ABI is. 

Jonathan

> 

Re: [PATCH 09/10] iio: accel: BMA220 add event attrs

[Petre Rodan]

[-- Attachment #1: Type: text/plain, Size: 1847 bytes --]


Hello Jonathan,

On Sun, Sep 07, 2025 at 02:15:15PM +0100, Jonathan Cameron wrote:
> On Mon,  1 Sep 2025 22:47:35 +0300
> Petre Rodan <petre.rodan@subdimension.ro> wrote:
> 
> > Add event attributes not directly covered by the IIO API.
> 
> These must be accompanied by ABI documentation in
> Documentation/ABI/testing/sysfs-bus-iio-...
> 
> We need to pull out of the datasheet generic descriptions of what
> they are so we can consider if they make sense as general new ABI
> or perhaps map to something existing.  In some cases it is more
> appropriate just to set a reasonable default and not provide a
> userspace ABI at all.
> 
> Key point is that custom ABI is effectively unused ABI because most
> software is written against a bunch of devices and has no idea what
> the new ABI is. 

I see your point. From all ot those maybe just the interrupt latch functionality
is generic enough, I have seen something similar while writing other drivers.

from the datasheet:

" The interrupt controller can be used in two modes

 - Latched mode: Once one of the configured interrupt conditions applies, the INT pin is
asserted and must be reset by the external master through the digital interface.

 - Non-Latched mode: The interrupt controller clears the INT signal once the interrupt
condition no longer applies (default behaviour in our chip).

The interrupt output can be programmed by lat_int[2:0] to be either
  unlatched ('000') or
  latched permanently ('111')
  or have the latch time of 0.25s ('001')/0.5s('010')/1s('011')/2s('100')/4s('101')/8s('110').

The setting of these bits applies to all types of interrupts."

Many thanks for the detailed review, I will prepare a new set in b4 and skip
everything event related for now to keep the set smaller.

cheers,
peter

-- 
petre rodan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 09/10] iio: accel: BMA220 add event attrs

[Jonathan Cameron]

On Sun, 7 Sep 2025 16:28:17 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> Hello Jonathan,
> 
> On Sun, Sep 07, 2025 at 02:15:15PM +0100, Jonathan Cameron wrote:
> > On Mon,  1 Sep 2025 22:47:35 +0300
> > Petre Rodan <petre.rodan@subdimension.ro> wrote:
> >   
> > > Add event attributes not directly covered by the IIO API.  
> > 
> > These must be accompanied by ABI documentation in
> > Documentation/ABI/testing/sysfs-bus-iio-...
> > 
> > We need to pull out of the datasheet generic descriptions of what
> > they are so we can consider if they make sense as general new ABI
> > or perhaps map to something existing.  In some cases it is more
> > appropriate just to set a reasonable default and not provide a
> > userspace ABI at all.
> > 
> > Key point is that custom ABI is effectively unused ABI because most
> > software is written against a bunch of devices and has no idea what
> > the new ABI is.   
> 
> I see your point. From all ot those maybe just the interrupt latch functionality
> is generic enough, I have seen something similar while writing other drivers.
> 
> from the datasheet:
> 
> " The interrupt controller can be used in two modes
> 
>  - Latched mode: Once one of the configured interrupt conditions applies, the INT pin is
> asserted and must be reset by the external master through the digital interface.
> 
>  - Non-Latched mode: The interrupt controller clears the INT signal once the interrupt
> condition no longer applies (default behaviour in our chip).
> 
> The interrupt output can be programmed by lat_int[2:0] to be either
>   unlatched ('000') or
>   latched permanently ('111')
>   or have the latch time of 0.25s ('001')/0.5s('010')/1s('011')/2s('100')/4s('101')/8s('110').
> 
> The setting of these bits applies to all types of interrupts."
> 
> Many thanks for the detailed review, I will prepare a new set in b4 and skip
> everything event related for now to keep the set smaller.

Given you are skipping for now we can come back to this later...  But given we
are talking about it I couldn't resist replying ;)

So latched vs unlatched sounds like level signaled interrupt vs edge triggered.

The 'pulse' width cases are annoyingly hard to handle though with those lengths it
seems implausible we'd miss.  I hope for non-latched the sampling rate is slow enough
we still get a good pulse.

Anyhow, I'd suggest ignoring the latch time property and just select between
unlatched and latched based on the interrupt settings in firmware.

Unless you can think of a good usecase for changing these at runtime? I can only
come up with the theory they might be wired to some external circuitry.
Maybe a buzzer to be really annoying ;)

Jonathan

> 
> cheers,
> peter
> 

[PATCH 10/10] iio: accel: BMA220 add maintainer

[Petre Rodan]

Add myself as maintainer of this driver.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index af1c8d2bfb3d..dd94505fb9a0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4402,6 +4402,13 @@ F:	include/net/bond*
 F:	include/uapi/linux/if_bonding.h
 F:	tools/testing/selftests/drivers/net/bonding/

+BOSCH SENSORTEC BMA220 ACCELEROMETER IIO DRIVER
+M:	Petre Rodan <petre.rodan@subdimension.ro>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml
+F:	drivers/iio/accel/bma220*
+
 BOSCH SENSORTEC BMA400 ACCELEROMETER IIO DRIVER
 M:	Dan Robertson <dan@dlrobertson.com>
 L:	linux-iio@vger.kernel.org
--
2.49.1