[PATCH v3 00/18] iio: accel: bma220 improvements

[PATCH v3 00/18] iio: accel: bma220 improvements

[Petre Rodan]

Series of patches that switch the driver to the regmap API and add
i2c connectivity.

Tested in I2C and SPI modes with two different sensors.

Event-related code was skipped since the patch series was getting too
large.

Contains fixes based on feedback from Krzysztof, David and Jonathan.

b4 lost track [1] of the patch series after the v2->v3 rebase.
probably my fault. expecting weirdness.

[1] CRITICAL: This is not a prep-managed branch.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
Petre Rodan (18):
      dt-bindings: iio: accel: bosch,bma220 cleanup typo
      dt-bindings: iio: accel: bosch,bma220 setup SPI clock mode
      dt-bindings: iio: accel: bosch,bma220 set irq type in example block
      iio: accel: bma220: split original driver
      iio: accel: bma220: add open firmware table
      iio: accel: bma220: turn power supplies on
      iio: accel: bma220: move bma220_power() fct
      iio: accel: bma220: reset registers during init stage
      iio: accel: bma220: relax constraints during probe()
      iio: accel: bma220: migrate to regmap API
      iio: accel: bma220: populate buffer ts in trigger handler
      iio: accel: bma220: use find_match_table fct
      iio: accel: bma220: add i2c module
      iio: accel: bma220: add i2c watchdog feature
      iio: accel: bma220: add interrupt trigger
      iio: accel: bma220: add LPF cut-off frequency mapping
      iio: accel: bma220: add debugfs reg access
      iio: accel: bma220: add maintainer

 .../bindings/iio/accel/bosch,bma220.yaml           |   9 +-
 MAINTAINERS                                        |   7 +
 drivers/iio/accel/Kconfig                          |  19 +-
 drivers/iio/accel/Makefile                         |   4 +-
 drivers/iio/accel/bma220.h                         |  20 +
 drivers/iio/accel/bma220_core.c                    | 611 +++++++++++++++++++++
 drivers/iio/accel/bma220_i2c.c                     |  58 ++
 drivers/iio/accel/bma220_spi.c                     | 318 +----------
 8 files changed, 749 insertions(+), 297 deletions(-)
---
base-commit: 661facba437e37c1685606825b9fd59be3f78771
change-id: 20250913-b4-bma220_improvements-ad0b3df025bb

Best regards,
-- 
Petre Rodan <petre.rodan@subdimension.ro>

[PATCH v3 01/18] dt-bindings: iio: accel: bosch,bma220 cleanup typo

[Petre Rodan]

Cleanup typo present in the title.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
ChangeLog:
- split out from a bigger patch file
---
 Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml
index ec643de031a34190af1bc2bffee7412ee2d3b902..da047258aca3d84e8b2cbe92a9c98309236fe7ae 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>

-- 
2.49.1

[PATCH v3 02/18] dt-bindings: iio: accel: bosch,bma220 setup SPI clock mode

[Petre Rodan]

Assert CPOL for a high-idle clock signal and CPHA for sampling on the
trailing (rising) edge.

Quoting from the datasheet:

 "During the transitions on CSB, SCK must be high. SDI and SDO are driven
 at the falling edge of SCK and should be captured at the rising edge of
 SCK."

The sensor does not function with the default SPI clock mode.

Fixes: 7dbd479425d2 ("dt-bindings:iio:accel:bosch,bma220 device tree binding documentation")
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1->v2
split out from a bigger patch file
v2->v3
add fixes tab, just in case (Jonathan)
---
 Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml
index da047258aca3d84e8b2cbe92a9c98309236fe7ae..0e27ec74065acca611e63309d6ae889b8a3134ce 100644
--- a/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml
+++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml
@@ -20,6 +20,9 @@ properties:
   interrupts:
     maxItems: 1
 
+  spi-cpha: true
+  spi-cpol: true
+
   vdda-supply: true
   vddd-supply: true
   vddio-supply: true
@@ -44,6 +47,8 @@ examples:
             compatible = "bosch,bma220";
             reg = <0>;
             spi-max-frequency = <2500000>;
+            spi-cpol;
+            spi-cpha;
             interrupt-parent = <&gpio0>;
             interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
         };

-- 
2.49.1

[PATCH v3 03/18] dt-bindings: iio: accel: bosch,bma220 set irq type in example block

[Petre Rodan]

Set the interrupt type to rising edge within the example block in order
to match the new driver.

The entry that got replaced was not in use by the original driver.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1->v2
split out from a bigger patch file (Krzysztof)
v2->v3
reword commit (Krzysztof)
---
 Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml
index 0e27ec74065acca611e63309d6ae889b8a3134ce..8c820c27f781e8001bc14b4ca6ab1f293bdb18ca 100644
--- a/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml
+++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml
@@ -50,7 +50,7 @@ examples:
             spi-cpol;
             spi-cpha;
             interrupt-parent = <&gpio0>;
-            interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+            interrupts = <0 IRQ_TYPE_EDGE_RISING>;
         };
     };
 ...

-- 
2.49.1

Re: [PATCH v3 03/18] dt-bindings: iio: accel: bosch,bma220 set irq type in example block

[Krzysztof Kozlowski]

On Sat, Sep 13, 2025 at 06:39:24PM +0300, Petre Rodan wrote:
> Set the interrupt type to rising edge within the example block in order
> to match the new driver.
> 
> The entry that got replaced was not in use by the original driver.
> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> ---

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v3 03/18] dt-bindings: iio: accel: bosch,bma220 set irq type in example block

[Jonathan Cameron]

On Thu, 18 Sep 2025 11:08:59 +0900
Krzysztof Kozlowski <krzk@kernel.org> wrote:

> On Sat, Sep 13, 2025 at 06:39:24PM +0300, Petre Rodan wrote:
> > Set the interrupt type to rising edge within the example block in order
> > to match the new driver.
> > 
> > The entry that got replaced was not in use by the original driver.
> > 
> > Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> > ---  
> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
In the interests of reducing what you need to post for v4, I've
picked up these first 3 patches and applied them to the testing
branch of iio.git.   That will get rebased on rc1 once available.

Thanks,

Jonathan

> Best regards,
> Krzysztof
> 

[PATCH v3 04/18] iio: accel: bma220: split original driver

[Petre Rodan]

In preparation for the i2c module, move the original code into multiple
source files without any other functional change.

Create the additional bma220_core module which currently is not
providing an abstracted bus type (this will change with the regmap
patch).
Fix checkpatch warning about GPL v2 license in bma220_spi.c.
Fix a few includes to make the change functional and prepare for the
next patches.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1->v2
- split out open firmware table modification into separate patch
- bma220_write_raw() exits without dev_err() based on similar feedback
  from David
- change includes in bma220.h (Jonathan)
- include bma220.h in bma220_core.c (Jonathan)
- add device.h, errno.h, mutex.h and pm.h includes to bma220_core.c
- cleanup struct spacing in bma220_spi.c
v2->v3
- keep depends on SPI (David)
- fix a few includes in bma220_spi.c (Jonathan)
- mention new includes in commit body (Jonathan)
---
 drivers/iio/accel/Kconfig       |   8 +-
 drivers/iio/accel/Makefile      |   3 +-
 drivers/iio/accel/bma220.h      |  18 +++
 drivers/iio/accel/bma220_core.c | 313 ++++++++++++++++++++++++++++++++++++++++
 drivers/iio/accel/bma220_spi.c  | 307 ++-------------------------------------
 5 files changed, 353 insertions(+), 296 deletions(-)

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 8c3f7cf55d5fa432a4d4662b184a46cd59c3ebca..4648be329917a3054615ed324156d67f53baefb6 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -221,12 +221,18 @@ config BMA220
 	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 ca8569e25aba31c3ae3437abf8506addbf5edffa..56a9f848f7f913633bc2a628c1ac5c9190774b9d 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 0000000000000000000000000000000000000000..dea34244b4fc4971841dc60d79db6c7db27bea20
--- /dev/null
+++ b/drivers/iio/accel/bma220.h
@@ -0,0 +1,18 @@
+/* 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/pm.h>
+
+extern const struct dev_pm_ops bma220_pm_ops;
+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 0000000000000000000000000000000000000000..6bc2e5c3fb6cebd50209acbcc2d5340630c27cd1
--- /dev/null
+++ b/drivers/iio/accel/bma220_core.c
@@ -0,0 +1,313 @@
+// 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/mutex.h>
+#include <linux/pm.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>
+
+#include "bma220.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)
+			return ret;
+		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 01592eebf05bb6b002d44c41cca1d2dd5f28350c..3ad5e43aae496d265a8cf198595bf824f8e73692 100644
--- a/drivers/iio/accel/bma220_spi.c
+++ b/drivers/iio/accel/bma220_spi.c
@@ -5,326 +5,45 @@
  * 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>
 #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
+#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)
+static int bma220_spi_probe(struct spi_device *spi)
 {
-	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);
-}
-
-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);
+	return bma220_common_probe(spi);
 }
-static DEFINE_SIMPLE_DEV_PM_OPS(bma220_pm_ops, bma220_suspend, bma220_resume);
 
 static const struct spi_device_id bma220_spi_id[] = {
-	{"bma220", 0},
+	{ "bma220", 0 },
 	{ }
 };
 
 static const struct acpi_device_id bma220_acpi_id[] = {
-	{"BMA0220", 0},
+	{ "BMA0220", 0 },
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, bma220_spi_id);
 
-static struct spi_driver bma220_driver = {
+static struct spi_driver bma220_spi_driver = {
 	.driver = {
 		.name = "bma220_spi",
 		.pm = pm_sleep_ptr(&bma220_pm_ops),
 		.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 v3 04/18] iio: accel: bma220: split original driver

[Andy Shevchenko]

On Sat, Sep 13, 2025 at 6:40 PM Petre Rodan <petre.rodan@subdimension.ro> wrote:
>
> In preparation for the i2c module, move the original code into multiple
> source files without any other functional change.
>
> Create the additional bma220_core module which currently is not
> providing an abstracted bus type (this will change with the regmap
> patch).
> Fix checkpatch warning about GPL v2 license in bma220_spi.c.

Wny not a prerequisite fix?

> Fix a few includes to make the change functional and prepare for the
> next patches.

Ditto?

...

>           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.

I'm not sure the last part is practical, as one needs to list all
buses next time a new one will be added.

...

> +++ b/drivers/iio/accel/bma220.h

> +#ifndef _BMA220_H
> +#define _BMA220_H
> +
> +#include <linux/pm.h>
> +
> +extern const struct dev_pm_ops bma220_pm_ops;

> +struct spi_device;

Besides the location of this (I would expect it to follow up include
linux/*) convert the existing driver to regmap first and remove this
unneeded churn.

> +int bma220_common_probe(struct spi_device *dev);
> +
> +#endif

...

> +#include <linux/bits.h>

> +#include <linux/kernel.h>

You should not use this header.

> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/pm.h>
> +#include <linux/types.h>
> +#include <linux/spi/spi.h>

...

> +       /**

Do not inherit problems from the past.

> +        * 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.
> +        */

...

I haven't reviewed the rest as I believe it's just ~1:1 copy of the
existing code, but I still think that the result will be better if
this series starts from small fixes, like kernel doc, and other
things, followed by the regmap conversion and only _after_ the split
is made.


--
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 04/18] iio: accel: bma220: split original driver

[Petre Rodan]

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


hi Andy,

On Sun, Sep 14, 2025 at 03:45:07PM +0300, Andy Shevchenko wrote:
> >           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.
> 
> I'm not sure the last part is practical, as one needs to list all
> buses next time a new one will be added.

it's just a friendly note to the user of how the modules are called.
the sensor supports only spi and i2c, so the list is short.

> > +++ b/drivers/iio/accel/bma220.h
> 
> > +#ifndef _BMA220_H
> > +#define _BMA220_H
> > +
> > +#include <linux/pm.h>
> > +
> > +extern const struct dev_pm_ops bma220_pm_ops;
> 
> > +struct spi_device;
> 
> Besides the location of this (I would expect it to follow up include
> linux/*) convert the existing driver to regmap first and remove this
> unneeded churn.

[..]

> I haven't reviewed the rest as I believe it's just ~1:1 copy of the
> existing code, but I still think that the result will be better if
> this series starts from small fixes, like kernel doc, and other
> things, followed by the regmap conversion and only _after_ the split
> is made.

either way you look at it it can be seen as churn.

scenario 1
    split is done early: ~300 lines from _spi.c have to move to _core.c.
    I tried to do a 1:1 copy so it can be easily diffed, but small tweaks are needed here and there.

scenario 2
   split is done after regmap: much more than 300 lines need to be moved AND the code would diverge way too much from my target.

as I see it scenario 2 is worse from a reviewer's perspective and a nightmare from my pov.

I prefer to stick with scenario 1, adding a few prerequisite patches if you so prefer.
It's much easier for me to cherry pick modifications and copy them from my target code once the file structure in the patches have a _core.c, _spi.c, etc.

this is what I had in mind for v4:
split prerequisites (2-4p) -> split -> regmap prerequisites (4p) -> regmap -> regmap cleanup (2p) -> i2c -> features (5p).

best regards,
petre rodan

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

[PATCH v3 05/18] iio: accel: bma220: add open firmware table

[Petre Rodan]

Add open firmware entry to the spi driver.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1->v2 split from larger patch (Jonathan)
v2->v3 no change
---
 drivers/iio/accel/bma220_spi.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/iio/accel/bma220_spi.c b/drivers/iio/accel/bma220_spi.c
index 3ad5e43aae496d265a8cf198595bf824f8e73692..78820d90e39119d9755b6266a8329e11ffd55723 100644
--- a/drivers/iio/accel/bma220_spi.c
+++ b/drivers/iio/accel/bma220_spi.c
@@ -32,10 +32,17 @@ static const struct acpi_device_id bma220_acpi_id[] = {
 };
 MODULE_DEVICE_TABLE(spi, bma220_spi_id);
 
+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_spi_probe,

-- 
2.49.1

[PATCH v3 06/18] iio: accel: bma220: turn power supplies on

[Petre Rodan]

Add devm_regulator_bulk_get_enable() to device probe().

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1->v2 split from larger patch
v2->v3 change commit desc (Jonathan)
---
 drivers/iio/accel/bma220_core.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index 6bc2e5c3fb6cebd50209acbcc2d5340630c27cd1..b6f1374a9cca52966c1055113710061a7284cf5a 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -11,6 +11,7 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/pm.h>
+#include <linux/regulator/consumer.h>
 #include <linux/types.h>
 #include <linux/spi/spi.h>
 
@@ -205,6 +206,13 @@ static const struct iio_info bma220_info = {
 static int bma220_init(struct spi_device *spi)
 {
 	int ret;
+	static const char * const regulator_names[] = { "vddd", "vddio", "vdda" };
+
+	ret = devm_regulator_bulk_get_enable(&spi->dev,
+					     ARRAY_SIZE(regulator_names),
+					     regulator_names);
+	if (ret)
+		return dev_err_probe(&spi->dev, ret, "Failed to get regulators\n");
 
 	ret = bma220_read_reg(spi, BMA220_REG_ID);
 	if (ret != BMA220_CHIP_ID)

-- 
2.49.1

Re: [PATCH v3 06/18] iio: accel: bma220: turn power supplies on

[Andy Shevchenko]

On Sat, Sep 13, 2025 at 6:40 PM Petre Rodan <petre.rodan@subdimension.ro> wrote:
>
> Add devm_regulator_bulk_get_enable() to device probe().

...

>  {
>         int ret;
> +       static const char * const regulator_names[] = { "vddd", "vddio", "vdda" };
> +
> +       ret = devm_regulator_bulk_get_enable(&spi->dev,
> +                                            ARRAY_SIZE(regulator_names),

Adding

  struct device *dev = &spi->dev;

will make this line and others shorter.

> +                                            regulator_names);
> +       if (ret)
> +               return dev_err_probe(&spi->dev, ret, "Failed to get regulators\n");

...

It's possible to do in a separate patch for all of the cases, though.


-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 07/18] iio: accel: bma220: move bma220_power() fct

[Petre Rodan]

Move bma220_power() fct before bma220_init() since it will make the
next patch cleaner.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v2->v3 split from bigger patch (Krzysztof, Jonathan)
---
 drivers/iio/accel/bma220_core.c | 48 ++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index b6f1374a9cca52966c1055113710061a7284cf5a..dc156fcc1435ace9bda5fabc87864f38ae7bee05 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -203,6 +203,30 @@ static const struct iio_info bma220_info = {
 	.read_avail		= bma220_read_avail,
 };
 
+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 int bma220_init(struct spi_device *spi)
 {
 	int ret;
@@ -230,30 +254,6 @@ static int bma220_init(struct spi_device *spi)
 	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);

-- 
2.49.1

Re: [PATCH v3 07/18] iio: accel: bma220: move bma220_power() fct

[Andy Shevchenko]

On Sat, Sep 13, 2025 at 6:40 PM Petre Rodan <petre.rodan@subdimension.ro> wrote:
>
> Move bma220_power() fct before bma220_init() since it will make the
> next patch cleaner.

...

> +static int bma220_power(struct spi_device *spi, bool up)
> +{
> +       int i, ret;
> +
> +       /**

It's not a kernel doc, do not inherit the mistakes from the past.

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


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 07/18] iio: accel: bma220: move bma220_power() fct

[Jonathan Cameron]

On Sun, 14 Sep 2025 15:05:47 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Sat, Sep 13, 2025 at 6:40 PM Petre Rodan <petre.rodan@subdimension.ro> wrote:
> >
> > Move bma220_power() fct before bma220_init() since it will make the
> > next patch cleaner.  
> 
> ...
> 
> > +static int bma220_power(struct spi_device *spi, bool up)
> > +{
> > +       int i, ret;
> > +
> > +       /**  
> 
> It's not a kernel doc, do not inherit the mistakes from the past.
Agreed.  To keep the series clean I'd do a cleanup of all incorrect instances
of this as a precursor patch.  Then these moves become cleaner.
Otherwise we have a whole load of patch descriptions with little
notes saying 'and fix incorrect kernel-doc markings' and that just
muddies the water!

Jonathan

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

[PATCH v3 08/18] iio: accel: bma220: reset registers during init stage

[Petre Rodan]

Bring all configuration registers to default values during device probe().
Remove trivial code duplication regarding bma220_power() in _init()

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v2->v3
- split from larger patch
- remove 20% innocent parentheses in conditional (David)
---
 drivers/iio/accel/bma220_core.c | 43 +++++++++++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index dc156fcc1435ace9bda5fabc87864f38ae7bee05..fad73bf6b8f29c0a33519c64c8888a01008f7579 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -29,12 +29,15 @@
 #define BMA220_REG_ACCEL_Z			0x04
 #define BMA220_REG_RANGE			0x11
 #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_SUSPEND_SLEEP			0xFF
 #define BMA220_SUSPEND_WAKE			0x00
+#define BMA220_RESET_MODE			0xFF
+#define BMA220_NONRESET_MODE			0x00
 
 #define BMA220_DEVICE_NAME			"bma220"
 
@@ -203,6 +206,30 @@ static const struct iio_info bma220_info = {
 	.read_avail		= bma220_read_avail,
 };
 
+static int bma220_reset(struct spi_device *spi, bool up)
+{
+	int i, ret;
+
+	/**
+	 * 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 = bma220_read_reg(spi, BMA220_REG_SOFTRESET);
+		if (ret < 0)
+			return ret;
+
+		if (up && ret == BMA220_RESET_MODE)
+			return 0;
+
+		if (!up && ret == BMA220_NONRESET_MODE)
+			return 0;
+	}
+
+	return -EBUSY;
+}
+
 static int bma220_power(struct spi_device *spi, bool up)
 {
 	int i, ret;
@@ -242,14 +269,14 @@ static int bma220_init(struct spi_device *spi)
 	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;
+	/* Make sure the chip is powered on and config registers are reset */
+	ret = bma220_power(spi, true);
+	if (ret)
+		return dev_err_probe(&spi->dev, ret, "Failed to power-on chip\n");
+
+	ret = bma220_reset(spi, true);
+	if (ret)
+		return dev_err_probe(&spi->dev, ret, "Failed to soft reset chip\n");
 
 	return 0;
 }

-- 
2.49.1

Re: [PATCH v3 08/18] iio: accel: bma220: reset registers during init stage

[Andy Shevchenko]

On Sat, Sep 13, 2025 at 6:40 PM Petre Rodan <petre.rodan@subdimension.ro> wrote:
>
> Bring all configuration registers to default values during device probe().
> Remove trivial code duplication regarding bma220_power() in _init()

...

> +static int bma220_reset(struct spi_device *spi, bool up)
> +{
> +       int i, ret;

Why is 'i' signed?

> +       /**

Same comment as per previous patch.

> +        * 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 = bma220_read_reg(spi, BMA220_REG_SOFTRESET);
> +               if (ret < 0)
> +                       return ret;
> +
> +               if (up && ret == BMA220_RESET_MODE)
> +                       return 0;
> +
> +               if (!up && ret == BMA220_NONRESET_MODE)
> +                       return 0;
> +       }
> +
> +       return -EBUSY;
> +}


-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 09/18] iio: accel: bma220: relax constraints during probe()

[Petre Rodan]

Allow compatible chips to work even if their identifier is different.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v2->v3 replace err with dev_info() (Jonathan)
---
 drivers/iio/accel/bma220_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index fad73bf6b8f29c0a33519c64c8888a01008f7579..7fdb83cedfa1809490cfb7f89532182123d580ba 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -267,7 +267,7 @@ static int bma220_init(struct spi_device *spi)
 
 	ret = bma220_read_reg(spi, BMA220_REG_ID);
 	if (ret != BMA220_CHIP_ID)
-		return -ENODEV;
+		dev_info(&spi->dev, "Unknown chip found: 0x%02x\n", ret);
 
 	/* Make sure the chip is powered on and config registers are reset */
 	ret = bma220_power(spi, true);

-- 
2.49.1

Re: [PATCH v3 09/18] iio: accel: bma220: relax constraints during probe()

[Andy Shevchenko]

On Sat, Sep 13, 2025 at 6:40 PM Petre Rodan <petre.rodan@subdimension.ro> wrote:
>
> Allow compatible chips to work even if their identifier is different.

This looks to me like a candidate to be moved closer to the beginning
of the series (before even split).

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 10/18] iio: accel: bma220: migrate to regmap API

[Petre Rodan]

Switch to regmap API.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1->v2
- split out into more patches (Jonathan)
v2->v3
- split out into even more patches (Jonathan)
- use dev_err_probe() (Jonathan)
---
 drivers/iio/accel/Kconfig       |   2 +
 drivers/iio/accel/bma220.h      |   5 +-
 drivers/iio/accel/bma220_core.c | 244 ++++++++++++++++++++++++++++------------
 drivers/iio/accel/bma220_spi.c  |  10 +-
 4 files changed, 183 insertions(+), 78 deletions(-)

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 4648be329917a3054615ed324156d67f53baefb6..988fe4b1f9a52c2e671ea30d8590f25ee1685ac7 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -219,6 +219,7 @@ config BMA180
 config BMA220
 	tristate "Bosch BMA220 3-Axis Accelerometer Driver"
 	depends on SPI
+	select REGMAP
 	select IIO_BUFFER
 	select IIO_TRIGGERED_BUFFER
 	select BMA220_SPI if SPI
@@ -232,6 +233,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 dea34244b4fc4971841dc60d79db6c7db27bea20..f9f4fa3daf33665f07f8bf073468dff070b46d74 100644
--- a/drivers/iio/accel/bma220.h
+++ b/drivers/iio/accel/bma220.h
@@ -9,10 +9,11 @@
 #define _BMA220_H
 
 #include <linux/pm.h>
+#include <linux/regmap.h>
 
+extern const struct regmap_config bma220_spi_regmap_config;
 extern const struct dev_pm_ops bma220_pm_ops;
-struct spi_device;
 
-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 7fdb83cedfa1809490cfb7f89532182123d580ba..73ce9a9511734961c2c6dadc99c21418070dce00 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -3,17 +3,21 @@
  * 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/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/pm.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>
@@ -24,16 +28,63 @@
 #include "bma220.h"
 
 #define BMA220_REG_ID				0x00
+#define BMA220_REG_REVISION_ID			0x01
 #define BMA220_REG_ACCEL_X			0x02
 #define BMA220_REG_ACCEL_Y			0x03
 #define BMA220_REG_ACCEL_Z			0x04
+#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_REG_CONF4			0x09
+#define BMA220_SLOPE_DUR_MSK			GENMASK(1, 0)
+#define BMA220_SLOPE_TH_MSK			GENMASK(5, 2)
+#define BMA220_REG_CONF5			0x0a
+#define BMA220_TIP_EN_MSK			BIT(4)
+#define BMA220_REG_IF0				0x0b
+#define BMA220_REG_IF1				0x0c
+#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)
+#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_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_RST_MSK			BIT(7)
+#define BMA220_REG_IE2				0x0f
+#define BMA220_REG_FILTER			0x10
+#define BMA220_FILTER_MASK			GENMASK(3, 0)
 #define BMA220_REG_RANGE			0x11
+#define BMA220_RANGE_MASK			GENMASK(1, 0)
+#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)
 #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_SUSPEND_SLEEP			0xFF
 #define BMA220_SUSPEND_WAKE			0x00
 #define BMA220_RESET_MODE			0xFF
@@ -69,14 +120,15 @@ 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 {
 		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[] = {
@@ -86,35 +138,57 @@ 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;
+	}
+}
+
+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_NONE,
+	.writeable_reg = bma220_is_writable_reg,
+};
+EXPORT_SYMBOL_NS_GPL(bma220_spi_regmap_config, "IIO_BOSCH_BMA220");
+
 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_trigger_notify_done(indio_dev->trig);
 
 	return IRQ_HANDLED;
@@ -125,24 +199,22 @@ 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;
 	}
 
@@ -158,6 +230,8 @@ static int bma220_write_raw(struct iio_dev *indio_dev,
 	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++)
@@ -169,14 +243,12 @@ static int bma220_write_raw(struct iio_dev *indio_dev,
 		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)
 			return ret;
-		mutex_unlock(&data->lock);
+		data->range_idx = index;
 
 		return 0;
 	}
@@ -206,9 +278,12 @@ static const struct iio_info bma220_info = {
 	.read_avail		= bma220_read_avail,
 };
 
-static int bma220_reset(struct spi_device *spi, bool up)
+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.
@@ -216,89 +291,112 @@ static int bma220_reset(struct spi_device *spi, bool up)
 	 * to make sure that the device is in the desired state.
 	 */
 	for (i = 0; i < 2; i++) {
-		ret = bma220_read_reg(spi, BMA220_REG_SOFTRESET);
+		ret = regmap_read(data->regmap, BMA220_REG_SOFTRESET, &val);
 		if (ret < 0)
 			return ret;
 
-		if (up && ret == BMA220_RESET_MODE)
+		if (up && val == BMA220_RESET_MODE)
 			return 0;
 
-		if (!up && ret == BMA220_NONRESET_MODE)
+		if (!up && val == BMA220_NONRESET_MODE)
 			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 int bma220_init(struct spi_device *spi)
+static int bma220_init(struct bma220_data *data)
 {
 	int ret;
+	unsigned int val;
+	struct device *dev = data->dev;
 	static const char * const regulator_names[] = { "vddd", "vddio", "vdda" };
 
-	ret = devm_regulator_bulk_get_enable(&spi->dev,
+	ret = devm_regulator_bulk_get_enable(dev,
 					     ARRAY_SIZE(regulator_names),
 					     regulator_names);
 	if (ret)
-		return dev_err_probe(&spi->dev, ret, "Failed to get regulators\n");
+		return dev_err_probe(dev, ret, "Failed to get regulators\n");
 
-	ret = bma220_read_reg(spi, BMA220_REG_ID);
-	if (ret != BMA220_CHIP_ID)
-		dev_info(&spi->dev, "Unknown chip found: 0x%02x\n", ret);
+	/* Try to read chip_id register. It should return 0xdd. */
+	ret = regmap_read(data->regmap, BMA220_REG_ID, &val);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Failed to read chip id register\n");
 
-	/* Make sure the chip is powered on and config registers are reset */
-	ret = bma220_power(spi, true);
+	if (val != BMA220_CHIP_ID)
+		dev_info(dev, "Unknown chip found: 0x%02x\n", val);
+
+	ret = bma220_power(data, true);
 	if (ret)
-		return dev_err_probe(&spi->dev, ret, "Failed to power-on chip\n");
+		return dev_err_probe(dev, ret, "Failed to power-on chip\n");
 
-	ret = bma220_reset(spi, true);
+	ret = bma220_reset(data, true);
 	if (ret)
-		return dev_err_probe(&spi->dev, ret, "Failed to soft reset chip\n");
+		return dev_err_probe(dev, ret, "Failed to soft reset chip\n");
 
 	return 0;
 }
 
-static void bma220_deinit(void *spi)
+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));
 }
 
-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;
+
+	ret = devm_mutex_init(dev, &data->lock);
+	if (ret)
+		return ret;
 
 	indio_dev->info = &bma220_info;
 	indio_dev->name = BMA220_DEVICE_NAME;
@@ -307,38 +405,34 @@ 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);
+	ret = devm_add_action_or_reset(dev, bma220_deinit, data);
 	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,
+	ret = devm_iio_triggered_buffer_setup(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;
-	}
+	if (ret < 0)
+		dev_err_probe(dev, ret, "iio triggered buffer setup failed\n");
 
-	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");
+EXPORT_SYMBOL_NS_GPL(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 78820d90e39119d9755b6266a8329e11ffd55723..7364428b3e363950d3be2a03b7657a2f0315fc5b 100644
--- a/drivers/iio/accel/bma220_spi.c
+++ b/drivers/iio/accel/bma220_spi.c
@@ -9,6 +9,7 @@
 #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>
 
@@ -18,7 +19,14 @@
 
 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))
+		return dev_err_probe(&spi->dev, PTR_ERR(regmap),
+				     "failed to create regmap\n");
+
+	return bma220_common_probe(&spi->dev, regmap, spi->irq);
 }
 
 static const struct spi_device_id bma220_spi_id[] = {

-- 
2.49.1

Re: [PATCH v3 10/18] iio: accel: bma220: migrate to regmap API

[Andy Shevchenko]

On Sat, Sep 13, 2025 at 6:40 PM Petre Rodan <petre.rodan@subdimension.ro> wrote:
>
> Switch to regmap API.

...

>  #define _BMA220_H
>
>  #include <linux/pm.h>
> +#include <linux/regmap.h>
>
> +extern const struct regmap_config bma220_spi_regmap_config;
>  extern const struct dev_pm_ops bma220_pm_ops;

> -struct spi_device;

So, you want

struct device;

instead.

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

...

>  #include <linux/bits.h>
> +#include <linux/bitfield.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/pm.h>
> +#include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/types.h>
> -#include <linux/spi/spi.h>

...

> +#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)

These do not look like bitfields, please use plain numbers (0, 2, 3).
I feel like I commented on this previous time and my comment was ignored...

...

>  struct bma220_data {
> -       struct spi_device *spi_device;
> +       struct device *dev;
> +       struct regmap *regmap;

Why do you need both? One can be derived from the other.

>         struct mutex lock;
> +       u8 range_idx;
>         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);
>  };

...

> +       struct device *dev = data->dev;
>         static const char * const regulator_names[] = { "vddd", "vddio", "vdda" };
>
> -       ret = devm_regulator_bulk_get_enable(&spi->dev,
> +       ret = devm_regulator_bulk_get_enable(dev,
>                                              ARRAY_SIZE(regulator_names),
>                                              regulator_names);
>         if (ret)
> -               return dev_err_probe(&spi->dev, ret, "Failed to get regulators\n");
> +               return dev_err_probe(dev, ret, "Failed to get regulators\n");

As I said... Do it early and you will have less unneeded churn in the future.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 10/18] iio: accel: bma220: migrate to regmap API

[Petre Rodan]

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


Hello Andy,

On Sun, Sep 14, 2025 at 03:21:30PM +0300, Andy Shevchenko wrote:
> > +#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)
> 
> These do not look like bitfields, please use plain numbers (0, 2, 3).

ok.

> I feel like I commented on this previous time and my comment was ignored...

I can't find your comment. got a link?

best regards,
peter

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

[PATCH v3 11/18] iio: accel: bma220: populate buffer ts in trigger handler

[Petre Rodan]

Populate buffer timestamps in trigger handler instead of in the
top half. Otherwise all timestamps read zero.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v2->v3 split out from bigger patch (Jonathan)
---
 drivers/iio/accel/bma220_core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index 73ce9a9511734961c2c6dadc99c21418070dce00..728bf08c2cfb250266be56e69b11af4b6c4a1347 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -188,7 +188,7 @@ static irqreturn_t bma220_trigger_handler(int irq, void *p)
 		return IRQ_NONE;
 
 	iio_push_to_buffers_with_ts(indio_dev, &data->scan, sizeof(data->scan),
-				    pf->timestamp);
+				    iio_get_time_ns(indio_dev));
 	iio_trigger_notify_done(indio_dev->trig);
 
 	return IRQ_HANDLED;
@@ -409,8 +409,7 @@ int bma220_common_probe(struct device *dev, struct regmap *regmap, int irq)
 	if (ret)
 		return ret;
 
-	ret = devm_iio_triggered_buffer_setup(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_probe(dev, ret, "iio triggered buffer setup failed\n");

-- 
2.49.1

Re: [PATCH v3 11/18] iio: accel: bma220: populate buffer ts in trigger handler

[Jonathan Cameron]

On Sat, 13 Sep 2025 18:39:32 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> Populate buffer timestamps in trigger handler instead of in the
> top half. Otherwise all timestamps read zero.
Say why.   Key here is that not all triggers can run the top half handler
that provides pf->timestamp.  Use an example of which ever one you are
testing this with.

> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> ---
> v2->v3 split out from bigger patch (Jonathan)
> ---
>  drivers/iio/accel/bma220_core.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
> index 73ce9a9511734961c2c6dadc99c21418070dce00..728bf08c2cfb250266be56e69b11af4b6c4a1347 100644
> --- a/drivers/iio/accel/bma220_core.c
> +++ b/drivers/iio/accel/bma220_core.c
> @@ -188,7 +188,7 @@ static irqreturn_t bma220_trigger_handler(int irq, void *p)
>  		return IRQ_NONE;
>  
>  	iio_push_to_buffers_with_ts(indio_dev, &data->scan, sizeof(data->scan),
> -				    pf->timestamp);
> +				    iio_get_time_ns(indio_dev));
>  	iio_trigger_notify_done(indio_dev->trig);
>  
>  	return IRQ_HANDLED;
> @@ -409,8 +409,7 @@ int bma220_common_probe(struct device *dev, struct regmap *regmap, int irq)
>  	if (ret)
>  		return ret;
>  
> -	ret = devm_iio_triggered_buffer_setup(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_probe(dev, ret, "iio triggered buffer setup failed\n");
> 

[PATCH v3 12/18] iio: accel: bma220: use find_match_table fct

[Petre Rodan]

Clean up the code a bit by using a find_match_table function.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v2->v3 split out from bigger patch (Jonathan)
---
 drivers/iio/accel/bma220_core.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index 728bf08c2cfb250266be56e69b11af4b6c4a1347..3252fb2d7fdcc84431ef19f5dbac8be01060e036 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -221,11 +221,23 @@ static int bma220_read_raw(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
+static int bma220_find_match_2dt(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);
@@ -234,12 +246,9 @@ static int bma220_write_raw(struct iio_dev *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;
-			}
+		index = bma220_find_match_2dt(bma220_scale_table,
+					      ARRAY_SIZE(bma220_scale_table),
+					      val, val2);
 		if (index < 0)
 			return -EINVAL;
 

-- 
2.49.1

[PATCH v3 13/18] iio: accel: bma220: add i2c module

[Petre Rodan]

Add the bma220_i2c module.

Note that this 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>
---
v1->v2 no change
v2->v3
- depends I2C || SPI (David)
- change extern const struct order (David)
- remove unused includes (David)
---
 drivers/iio/accel/Kconfig       | 11 ++++++--
 drivers/iio/accel/Makefile      |  1 +
 drivers/iio/accel/bma220.h      |  1 +
 drivers/iio/accel/bma220_core.c | 18 +++++++++++++
 drivers/iio/accel/bma220_i2c.c  | 58 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 988fe4b1f9a52c2e671ea30d8590f25ee1685ac7..76911278fb217bd3e429c427b873fb7eae26420e 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -218,10 +218,11 @@ config BMA180
 
 config BMA220
 	tristate "Bosch BMA220 3-Axis Accelerometer Driver"
-	depends on SPI
+	depends on I2C || SPI
 	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
@@ -229,7 +230,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 56a9f848f7f913633bc2a628c1ac5c9190774b9d..fa440a85928398fee927081f605595ba9fbc4ad9 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 f9f4fa3daf33665f07f8bf073468dff070b46d74..a5127e997b1f970a86bd3394ed566b3a92a8ee1c 100644
--- a/drivers/iio/accel/bma220.h
+++ b/drivers/iio/accel/bma220.h
@@ -11,6 +11,7 @@
 #include <linux/pm.h>
 #include <linux/regmap.h>
 
+extern const struct regmap_config bma220_i2c_regmap_config;
 extern const struct regmap_config bma220_spi_regmap_config;
 extern const struct dev_pm_ops bma220_pm_ops;
 
diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index 3252fb2d7fdcc84431ef19f5dbac8be01060e036..5eef47151501119ab792aab6a511938a16c1ade1 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -174,6 +174,24 @@ const struct regmap_config bma220_spi_regmap_config = {
 };
 EXPORT_SYMBOL_NS_GPL(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_NONE,
+	.writeable_reg = bma220_is_writable_reg,
+};
+EXPORT_SYMBOL_NS_GPL(bma220_i2c_regmap_config, "IIO_BOSCH_BMA220");
+
 static irqreturn_t bma220_trigger_handler(int irq, void *p)
 {
 	int ret;
diff --git a/drivers/iio/accel/bma220_i2c.c b/drivers/iio/accel/bma220_i2c.c
new file mode 100644
index 0000000000000000000000000000000000000000..5dc7c38f53b3173ff43ca29b63132ec44e672cde
--- /dev/null
+++ b/drivers/iio/accel/bma220_i2c.c
@@ -0,0 +1,58 @@
+// 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/errno.h>
+#include <linux/i2c.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/types.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))
+		return dev_err_probe(&client->dev, PTR_ERR(regmap),
+				     "failed to create regmap\n");
+
+	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

[PATCH v3 14/18] iio: accel: bma220: add i2c watchdog feature

[Petre Rodan]

Sometimes the sensor gets stuck and enters a condition in which it pulls
SDA low, thus making the entire i2c bus unusable.
This problem is mitigated by activating a 1ms watchdog implemented in
the sensor.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1->v2
- hardcode 1ms timer watchdog to any i2c-based sensor instead of
  configuring a dt-based property
- rename bma220_wdt() to bma220_set_wdt()
v2->v3 no change
---
 drivers/iio/accel/bma220_core.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index 5eef47151501119ab792aab6a511938a16c1ade1..425a8b981e141aa496351f29df0597c989aa4a0a 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -10,6 +10,7 @@
 #include <linux/bitfield.h>
 #include <linux/cleanup.h>
 #include <linux/device.h>
+#include <linux/i2c.h>
 #include <linux/kernel.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
@@ -358,6 +359,12 @@ static int bma220_power(struct bma220_data *data, bool up)
 	return -EBUSY;
 }
 
+static int bma220_set_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;
@@ -388,6 +395,13 @@ static int bma220_init(struct bma220_data *data)
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to soft reset chip\n");
 
+	if (i2c_verify_client(dev)) {
+		ret = bma220_set_wdt(data, BMA220_WDT_1MS);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Failed to set i2c watchdog\n");
+	}
+
 	return 0;
 }
 

-- 
2.49.1

[PATCH v3 15/18] iio: accel: bma220: add interrupt trigger

[Petre Rodan]

Add interrupt trigger.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1->v2 no change, just patch split
v2->v3 replace regmap_bulk_read with regmap_read (Jonathan)
 (I just realized BMA220_REG_IF0 is never used, even by future event
patches)
---
 drivers/iio/accel/bma220_core.c | 61 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index 425a8b981e141aa496351f29df0597c989aa4a0a..6297882bcf1b955291a2d8747984648bc6ee8512 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -23,6 +23,7 @@
 #include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/triggered_buffer.h>
 
@@ -125,6 +126,7 @@ struct bma220_data {
 	struct regmap *regmap;
 	struct mutex lock;
 	u8 range_idx;
+	struct iio_trigger *trig;
 	struct {
 		s8 chans[3];
 		/* Ensure timestamp is naturally aligned. */
@@ -193,6 +195,23 @@ const struct regmap_config bma220_i2c_regmap_config = {
 };
 EXPORT_SYMBOL_NS_GPL(bma220_i2c_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;
@@ -417,6 +436,24 @@ static void bma220_deinit(void *data_ptr)
 			 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;
+	unsigned int bma220_reg_if1;
+
+	guard(mutex)(&data->lock);
+	rv = regmap_read(data->regmap, BMA220_REG_IF1, &bma220_reg_if1);
+	if (rv)
+		return IRQ_NONE;
+
+	if (FIELD_GET(BMA220_IF_DRDY, bma220_reg_if1)) {
+		iio_trigger_poll_nested(data->trig);
+
+	return IRQ_HANDLED;
+}
+
 int bma220_common_probe(struct device *dev, struct regmap *regmap, int irq)
 {
 	int ret;
@@ -446,6 +483,30 @@ int bma220_common_probe(struct device *dev, struct regmap *regmap, int irq)
 	indio_dev->num_channels = ARRAY_SIZE(bma220_channels);
 	indio_dev->available_scan_masks = bma220_accel_scan_masks;
 
+	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(dev, data->trig);
+		if (ret)
+			return dev_err_probe(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(dev, ret,
+					     "request irq %d failed\n", irq);
+	}
+
 	ret = devm_add_action_or_reset(dev, bma220_deinit, data);
 	if (ret)
 		return ret;

-- 
2.49.1

Re: [PATCH v3 15/18] iio: accel: bma220: add interrupt trigger

[Petre Rodan]

On Sat, Sep 13, 2025 at 06:39:36PM +0300, Petre Rodan wrote:
> +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;
> +	unsigned int bma220_reg_if1;
> +
> +	guard(mutex)(&data->lock);
> +	rv = regmap_read(data->regmap, BMA220_REG_IF1, &bma220_reg_if1);
> +	if (rv)
> +		return IRQ_NONE;
> +
> +	if (FIELD_GET(BMA220_IF_DRDY, bma220_reg_if1)) {
> +		iio_trigger_poll_nested(data->trig);
> +
> +	return IRQ_HANDLED;
> +}

sorry, errant '{' in FIELD_GET line throws compilation off, will fix in next rev.

best regards,
peter

Re: [PATCH v3 15/18] iio: accel: bma220: add interrupt trigger

[Andy Shevchenko]

On Sat, Sep 13, 2025 at 7:32 PM Petre Rodan <petre.rodan@subdimension.ro> wrote:
> On Sat, Sep 13, 2025 at 06:39:36PM +0300, Petre Rodan wrote:

> > +     if (FIELD_GET(BMA220_IF_DRDY, bma220_reg_if1)) {
> > +             iio_trigger_poll_nested(data->trig);
> > +
> > +     return IRQ_HANDLED;
> > +}

> sorry, errant '{' in FIELD_GET line throws compilation off, will fix in next rev.

Please, slow down and test your patches carefully. If you haven't even
compiled this, I don't think it's ever possible to be tested on real
HW. So, this is NAK for the series. Also note, due to timing this will
be a material for the next cycle anyway (v6.19), so no rush with a new
version.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 15/18] iio: accel: bma220: add interrupt trigger

[kernel test robot]

Hi Petre,

kernel test robot noticed the following build errors:

[auto build test ERROR on 661facba437e37c1685606825b9fd59be3f78771]

url:    https://github.com/intel-lab-lkp/linux/commits/Petre-Rodan/dt-bindings-iio-accel-bosch-bma220-cleanup-typo/20250913-234451
base:   661facba437e37c1685606825b9fd59be3f78771
patch link:    https://lore.kernel.org/r/20250913-b4-bma220_improvements-v3-15-0b97279b4e45%40subdimension.ro
patch subject: [PATCH v3 15/18] iio: accel: bma220: add interrupt trigger
config: x86_64-buildonly-randconfig-002-20250913 (https://download.01.org/0day-ci/archive/20250914/202509140109.kB7kOKfZ-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250914/202509140109.kB7kOKfZ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509140109.kB7kOKfZ-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/linkage.h:7,
                    from arch/x86/include/asm/cache.h:5,
                    from include/vdso/cache.h:5,
                    from include/linux/cache.h:6,
                    from arch/x86/include/asm/current.h:10,
                    from include/linux/sched.h:12,
                    from include/linux/ratelimit.h:6,
                    from include/linux/dev_printk.h:16,
                    from include/linux/device.h:15,
                    from drivers/iio/accel/bma220_core.c:12:
   drivers/iio/accel/bma220_core.c: In function 'bma220_irq_handler':
>> drivers/iio/accel/bma220_core.c:521:22: error: non-static declaration of 'bma220_common_probe' follows static declaration
     521 | EXPORT_SYMBOL_NS_GPL(bma220_common_probe, "IIO_BOSCH_BMA220");
         |                      ^~~~~~~~~~~~~~~~~~~
   include/linux/export.h:76:28: note: in definition of macro '__EXPORT_SYMBOL'
      76 |         extern typeof(sym) sym;                                 \
         |                            ^~~
   drivers/iio/accel/bma220_core.c:521:1: note: in expansion of macro 'EXPORT_SYMBOL_NS_GPL'
     521 | EXPORT_SYMBOL_NS_GPL(bma220_common_probe, "IIO_BOSCH_BMA220");
         | ^~~~~~~~~~~~~~~~~~~~
   drivers/iio/accel/bma220_core.c:457:5: note: previous definition of 'bma220_common_probe' with type 'int(struct device *, struct regmap *, int)'
     457 | int bma220_common_probe(struct device *dev, struct regmap *regmap, int irq)
         |     ^~~~~~~~~~~~~~~~~~~
>> drivers/iio/accel/bma220_core.c:523:12: error: invalid storage class for function 'bma220_suspend'
     523 | static int bma220_suspend(struct device *dev)
         |            ^~~~~~~~~~~~~~
>> drivers/iio/accel/bma220_core.c:531:12: error: invalid storage class for function 'bma220_resume'
     531 | static int bma220_resume(struct device *dev)
         |            ^~~~~~~~~~~~~
>> drivers/iio/accel/bma220_core.c:538:29: error: extern declaration of 'bma220_pm_ops' follows declaration with no linkage
     538 | EXPORT_NS_SIMPLE_DEV_PM_OPS(bma220_pm_ops, bma220_suspend, bma220_resume,
         |                             ^~~~~~~~~~~~~
   include/linux/export.h:76:28: note: in definition of macro '__EXPORT_SYMBOL'
      76 |         extern typeof(sym) sym;                                 \
         |                            ^~~
   include/linux/pm.h:393:57: note: in expansion of macro '_EXPORT_PM_OPS'
     393 | #define _EXPORT_DEV_SLEEP_PM_OPS(name, license, ns)     _EXPORT_PM_OPS(name, license, ns)
         |                                                         ^~~~~~~~~~~~~~
   include/linux/pm.h:405:57: note: in expansion of macro '_EXPORT_DEV_SLEEP_PM_OPS'
     405 | #define EXPORT_NS_DEV_SLEEP_PM_OPS(name, ns)            _EXPORT_DEV_SLEEP_PM_OPS(name, "", #ns)
         |                                                         ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/pm.h:427:9: note: in expansion of macro 'EXPORT_NS_DEV_SLEEP_PM_OPS'
     427 |         EXPORT_NS_DEV_SLEEP_PM_OPS(name, ns) = { \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/accel/bma220_core.c:538:1: note: in expansion of macro 'EXPORT_NS_SIMPLE_DEV_PM_OPS'
     538 | EXPORT_NS_SIMPLE_DEV_PM_OPS(bma220_pm_ops, bma220_suspend, bma220_resume,
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/device.h:25:
   drivers/iio/accel/bma220_core.c:538:29: note: previous declaration of 'bma220_pm_ops' with type 'const struct dev_pm_ops'
     538 | EXPORT_NS_SIMPLE_DEV_PM_OPS(bma220_pm_ops, bma220_suspend, bma220_resume,
         |                             ^~~~~~~~~~~~~
   include/linux/pm.h:379:33: note: in definition of macro '_EXPORT_PM_OPS'
     379 |         const struct dev_pm_ops name;                                   \
         |                                 ^~~~
   include/linux/pm.h:405:57: note: in expansion of macro '_EXPORT_DEV_SLEEP_PM_OPS'
     405 | #define EXPORT_NS_DEV_SLEEP_PM_OPS(name, ns)            _EXPORT_DEV_SLEEP_PM_OPS(name, "", #ns)
         |                                                         ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/pm.h:427:9: note: in expansion of macro 'EXPORT_NS_DEV_SLEEP_PM_OPS'
     427 |         EXPORT_NS_DEV_SLEEP_PM_OPS(name, ns) = { \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/accel/bma220_core.c:538:1: note: in expansion of macro 'EXPORT_NS_SIMPLE_DEV_PM_OPS'
     538 | EXPORT_NS_SIMPLE_DEV_PM_OPS(bma220_pm_ops, bma220_suspend, bma220_resume,
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/iio/accel/bma220_core.c:538:29: error: declaration of 'bma220_pm_ops' with no linkage follows extern declaration
     538 | EXPORT_NS_SIMPLE_DEV_PM_OPS(bma220_pm_ops, bma220_suspend, bma220_resume,
         |                             ^~~~~~~~~~~~~
   include/linux/pm.h:381:33: note: in definition of macro '_EXPORT_PM_OPS'
     381 |         const struct dev_pm_ops name
         |                                 ^~~~
   include/linux/pm.h:405:57: note: in expansion of macro '_EXPORT_DEV_SLEEP_PM_OPS'
     405 | #define EXPORT_NS_DEV_SLEEP_PM_OPS(name, ns)            _EXPORT_DEV_SLEEP_PM_OPS(name, "", #ns)
         |                                                         ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/pm.h:427:9: note: in expansion of macro 'EXPORT_NS_DEV_SLEEP_PM_OPS'
     427 |         EXPORT_NS_DEV_SLEEP_PM_OPS(name, ns) = { \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/accel/bma220_core.c:538:1: note: in expansion of macro 'EXPORT_NS_SIMPLE_DEV_PM_OPS'
     538 | EXPORT_NS_SIMPLE_DEV_PM_OPS(bma220_pm_ops, bma220_suspend, bma220_resume,
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/accel/bma220_core.c:538:29: note: previous declaration of 'bma220_pm_ops' with type 'const struct dev_pm_ops'
     538 | EXPORT_NS_SIMPLE_DEV_PM_OPS(bma220_pm_ops, bma220_suspend, bma220_resume,
         |                             ^~~~~~~~~~~~~
   include/linux/export.h:76:28: note: in definition of macro '__EXPORT_SYMBOL'
      76 |         extern typeof(sym) sym;                                 \
         |                            ^~~
   include/linux/pm.h:393:57: note: in expansion of macro '_EXPORT_PM_OPS'
     393 | #define _EXPORT_DEV_SLEEP_PM_OPS(name, license, ns)     _EXPORT_PM_OPS(name, license, ns)
         |                                                         ^~~~~~~~~~~~~~
   include/linux/pm.h:405:57: note: in expansion of macro '_EXPORT_DEV_SLEEP_PM_OPS'
     405 | #define EXPORT_NS_DEV_SLEEP_PM_OPS(name, ns)            _EXPORT_DEV_SLEEP_PM_OPS(name, "", #ns)
         |                                                         ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/pm.h:427:9: note: in expansion of macro 'EXPORT_NS_DEV_SLEEP_PM_OPS'
     427 |         EXPORT_NS_DEV_SLEEP_PM_OPS(name, ns) = { \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/accel/bma220_core.c:538:1: note: in expansion of macro 'EXPORT_NS_SIMPLE_DEV_PM_OPS'
     538 | EXPORT_NS_SIMPLE_DEV_PM_OPS(bma220_pm_ops, bma220_suspend, bma220_resume,
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/iio/accel/bma220_core.c:543:1: error: expected declaration or statement at end of input
     543 | MODULE_LICENSE("GPL");
         | ^~~~~~~~~~~~~~
>> drivers/iio/accel/bma220_core.c:538:29: warning: unused variable 'bma220_pm_ops' [-Wunused-variable]
     538 | EXPORT_NS_SIMPLE_DEV_PM_OPS(bma220_pm_ops, bma220_suspend, bma220_resume,
         |                             ^~~~~~~~~~~~~
   include/linux/pm.h:381:33: note: in definition of macro '_EXPORT_PM_OPS'
     381 |         const struct dev_pm_ops name
         |                                 ^~~~
   include/linux/pm.h:405:57: note: in expansion of macro '_EXPORT_DEV_SLEEP_PM_OPS'
     405 | #define EXPORT_NS_DEV_SLEEP_PM_OPS(name, ns)            _EXPORT_DEV_SLEEP_PM_OPS(name, "", #ns)
         |                                                         ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/pm.h:427:9: note: in expansion of macro 'EXPORT_NS_DEV_SLEEP_PM_OPS'
     427 |         EXPORT_NS_DEV_SLEEP_PM_OPS(name, ns) = { \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/accel/bma220_core.c:538:1: note: in expansion of macro 'EXPORT_NS_SIMPLE_DEV_PM_OPS'
     538 | EXPORT_NS_SIMPLE_DEV_PM_OPS(bma220_pm_ops, bma220_suspend, bma220_resume,
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/bma220_common_probe +521 drivers/iio/accel/bma220_core.c

b019a92c1241f44 Petre Rodan 2025-09-13  456  
9216bea3069746d Petre Rodan 2025-09-13 @457  int bma220_common_probe(struct device *dev, struct regmap *regmap, int irq)
e51403e66843c16 Petre Rodan 2025-09-13  458  {
e51403e66843c16 Petre Rodan 2025-09-13  459  	int ret;
e51403e66843c16 Petre Rodan 2025-09-13  460  	struct iio_dev *indio_dev;
e51403e66843c16 Petre Rodan 2025-09-13  461  	struct bma220_data *data;
e51403e66843c16 Petre Rodan 2025-09-13  462  
9216bea3069746d Petre Rodan 2025-09-13  463  	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
e51403e66843c16 Petre Rodan 2025-09-13  464  	if (!indio_dev)
e51403e66843c16 Petre Rodan 2025-09-13  465  		return -ENOMEM;
e51403e66843c16 Petre Rodan 2025-09-13  466  
e51403e66843c16 Petre Rodan 2025-09-13  467  	data = iio_priv(indio_dev);
9216bea3069746d Petre Rodan 2025-09-13  468  	data->regmap = regmap;
9216bea3069746d Petre Rodan 2025-09-13  469  	data->dev = dev;
9216bea3069746d Petre Rodan 2025-09-13  470  
9216bea3069746d Petre Rodan 2025-09-13  471  	ret = bma220_init(data);
9216bea3069746d Petre Rodan 2025-09-13  472  	if (ret)
9216bea3069746d Petre Rodan 2025-09-13  473  		return ret;
9216bea3069746d Petre Rodan 2025-09-13  474  
9216bea3069746d Petre Rodan 2025-09-13  475  	ret = devm_mutex_init(dev, &data->lock);
9216bea3069746d Petre Rodan 2025-09-13  476  	if (ret)
9216bea3069746d Petre Rodan 2025-09-13  477  		return ret;
e51403e66843c16 Petre Rodan 2025-09-13  478  
e51403e66843c16 Petre Rodan 2025-09-13  479  	indio_dev->info = &bma220_info;
e51403e66843c16 Petre Rodan 2025-09-13  480  	indio_dev->name = BMA220_DEVICE_NAME;
e51403e66843c16 Petre Rodan 2025-09-13  481  	indio_dev->modes = INDIO_DIRECT_MODE;
e51403e66843c16 Petre Rodan 2025-09-13  482  	indio_dev->channels = bma220_channels;
e51403e66843c16 Petre Rodan 2025-09-13  483  	indio_dev->num_channels = ARRAY_SIZE(bma220_channels);
e51403e66843c16 Petre Rodan 2025-09-13  484  	indio_dev->available_scan_masks = bma220_accel_scan_masks;
e51403e66843c16 Petre Rodan 2025-09-13  485  
b019a92c1241f44 Petre Rodan 2025-09-13  486  	if (irq > 0) {
b019a92c1241f44 Petre Rodan 2025-09-13  487  		data->trig = devm_iio_trigger_alloc(dev, "%s-dev%d",
b019a92c1241f44 Petre Rodan 2025-09-13  488  						    indio_dev->name,
b019a92c1241f44 Petre Rodan 2025-09-13  489  						    iio_device_id(indio_dev));
b019a92c1241f44 Petre Rodan 2025-09-13  490  		if (!data->trig)
b019a92c1241f44 Petre Rodan 2025-09-13  491  			return -ENOMEM;
b019a92c1241f44 Petre Rodan 2025-09-13  492  
b019a92c1241f44 Petre Rodan 2025-09-13  493  		data->trig->ops = &bma220_trigger_ops;
b019a92c1241f44 Petre Rodan 2025-09-13  494  		iio_trigger_set_drvdata(data->trig, indio_dev);
b019a92c1241f44 Petre Rodan 2025-09-13  495  
b019a92c1241f44 Petre Rodan 2025-09-13  496  		ret = devm_iio_trigger_register(dev, data->trig);
b019a92c1241f44 Petre Rodan 2025-09-13  497  		if (ret)
b019a92c1241f44 Petre Rodan 2025-09-13  498  			return dev_err_probe(dev, ret,
b019a92c1241f44 Petre Rodan 2025-09-13  499  					     "iio trigger register fail\n");
b019a92c1241f44 Petre Rodan 2025-09-13  500  		indio_dev->trig = iio_trigger_get(data->trig);
b019a92c1241f44 Petre Rodan 2025-09-13  501  		ret = devm_request_threaded_irq(dev, irq, NULL,
b019a92c1241f44 Petre Rodan 2025-09-13  502  						&bma220_irq_handler,
b019a92c1241f44 Petre Rodan 2025-09-13  503  						IRQF_TRIGGER_RISING | IRQF_ONESHOT,
b019a92c1241f44 Petre Rodan 2025-09-13  504  						indio_dev->name, indio_dev);
b019a92c1241f44 Petre Rodan 2025-09-13  505  		if (ret)
b019a92c1241f44 Petre Rodan 2025-09-13  506  			return dev_err_probe(dev, ret,
b019a92c1241f44 Petre Rodan 2025-09-13  507  					     "request irq %d failed\n", irq);
b019a92c1241f44 Petre Rodan 2025-09-13  508  	}
b019a92c1241f44 Petre Rodan 2025-09-13  509  
9216bea3069746d Petre Rodan 2025-09-13  510  	ret = devm_add_action_or_reset(dev, bma220_deinit, data);
e51403e66843c16 Petre Rodan 2025-09-13  511  	if (ret)
e51403e66843c16 Petre Rodan 2025-09-13  512  		return ret;
e51403e66843c16 Petre Rodan 2025-09-13  513  
d1258c485cdab0a Petre Rodan 2025-09-13  514  	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
e51403e66843c16 Petre Rodan 2025-09-13  515  					      bma220_trigger_handler, NULL);
9216bea3069746d Petre Rodan 2025-09-13  516  	if (ret < 0)
9216bea3069746d Petre Rodan 2025-09-13  517  		dev_err_probe(dev, ret, "iio triggered buffer setup failed\n");
e51403e66843c16 Petre Rodan 2025-09-13  518  
9216bea3069746d Petre Rodan 2025-09-13  519  	return devm_iio_device_register(dev, indio_dev);
e51403e66843c16 Petre Rodan 2025-09-13  520  }
9216bea3069746d Petre Rodan 2025-09-13 @521  EXPORT_SYMBOL_NS_GPL(bma220_common_probe, "IIO_BOSCH_BMA220");
e51403e66843c16 Petre Rodan 2025-09-13  522  
e51403e66843c16 Petre Rodan 2025-09-13 @523  static int bma220_suspend(struct device *dev)
e51403e66843c16 Petre Rodan 2025-09-13  524  {
9216bea3069746d Petre Rodan 2025-09-13  525  	struct iio_dev *indio_dev = dev_get_drvdata(dev);
9216bea3069746d Petre Rodan 2025-09-13  526  	struct bma220_data *data = iio_priv(indio_dev);
e51403e66843c16 Petre Rodan 2025-09-13  527  
9216bea3069746d Petre Rodan 2025-09-13  528  	return bma220_power(data, false);
e51403e66843c16 Petre Rodan 2025-09-13  529  }
e51403e66843c16 Petre Rodan 2025-09-13  530  
e51403e66843c16 Petre Rodan 2025-09-13 @531  static int bma220_resume(struct device *dev)
e51403e66843c16 Petre Rodan 2025-09-13  532  {
9216bea3069746d Petre Rodan 2025-09-13  533  	struct iio_dev *indio_dev = dev_get_drvdata(dev);
9216bea3069746d Petre Rodan 2025-09-13  534  	struct bma220_data *data = iio_priv(indio_dev);
e51403e66843c16 Petre Rodan 2025-09-13  535  
9216bea3069746d Petre Rodan 2025-09-13  536  	return bma220_power(data, true);
e51403e66843c16 Petre Rodan 2025-09-13  537  }
e51403e66843c16 Petre Rodan 2025-09-13 @538  EXPORT_NS_SIMPLE_DEV_PM_OPS(bma220_pm_ops, bma220_suspend, bma220_resume,
e51403e66843c16 Petre Rodan 2025-09-13  539  			    IIO_BOSCH_BMA220);
e51403e66843c16 Petre Rodan 2025-09-13  540  
e51403e66843c16 Petre Rodan 2025-09-13  541  MODULE_AUTHOR("Tiberiu Breana <tiberiu.a.breana@intel.com>");
e51403e66843c16 Petre Rodan 2025-09-13  542  MODULE_DESCRIPTION("BMA220 acceleration sensor driver");
e51403e66843c16 Petre Rodan 2025-09-13 @543  MODULE_LICENSE("GPL");

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v3 15/18] iio: accel: bma220: add interrupt trigger

[Andy Shevchenko]

On Sat, Sep 13, 2025 at 6:40 PM Petre Rodan <petre.rodan@subdimension.ro> wrote:
>
> Add interrupt trigger.

...

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

Be consistent with the variable namings. This sounds like 'ret' to me.

> +       unsigned int bma220_reg_if1;
> +
> +       guard(mutex)(&data->lock);
> +       rv = regmap_read(data->regmap, BMA220_REG_IF1, &bma220_reg_if1);
> +       if (rv)
> +               return IRQ_NONE;
> +
> +       if (FIELD_GET(BMA220_IF_DRDY, bma220_reg_if1)) {
> +               iio_trigger_poll_nested(data->trig);
> +
> +       return IRQ_HANDLED;
> +}

...

> +               ret = devm_request_threaded_irq(dev, irq, NULL,
> +                                               &bma220_irq_handler,
> +                                               IRQF_TRIGGER_RISING | IRQF_ONESHOT,

Why is it okay to override firmware provided IRQ flags, please?

> +                                               indio_dev->name, indio_dev);
> +               if (ret)
> +                       return dev_err_probe(dev, ret,
> +                                            "request irq %d failed\n", irq);


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 15/18] iio: accel: bma220: add interrupt trigger

[Jonathan Cameron]

On Sat, 13 Sep 2025 18:39:36 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> Add interrupt trigger.
> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
Hi Petre

A few lock questions from a fresh look.
Sorry for delay - busy few weeks and a few days lain up in bed with
a cold.

> ---
> v1->v2 no change, just patch split
> v2->v3 replace regmap_bulk_read with regmap_read (Jonathan)
>  (I just realized BMA220_REG_IF0 is never used, even by future event
> patches)
> ---
>  drivers/iio/accel/bma220_core.c | 61 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
> index 425a8b981e141aa496351f29df0597c989aa4a0a..6297882bcf1b955291a2d8747984648bc6ee8512 100644
> --- a/drivers/iio/accel/bma220_core.c
> +++ b/drivers/iio/accel/bma220_core.c
> @@ -23,6 +23,7 @@
>  #include <linux/iio/buffer.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
> +#include <linux/iio/trigger.h>
>  #include <linux/iio/trigger_consumer.h>
>  #include <linux/iio/triggered_buffer.h>
>  
> @@ -125,6 +126,7 @@ struct bma220_data {
>  	struct regmap *regmap;
>  	struct mutex lock;
>  	u8 range_idx;
> +	struct iio_trigger *trig;
>  	struct {
>  		s8 chans[3];
>  		/* Ensure timestamp is naturally aligned. */
> @@ -193,6 +195,23 @@ const struct regmap_config bma220_i2c_regmap_config = {
>  };
>  EXPORT_SYMBOL_NS_GPL(bma220_i2c_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);

What is this lock protecting? 

> +	return regmap_update_bits(data->regmap, BMA220_REG_IE0,
> +				  BMA220_INT_EN_DRDY_MSK,
> +				  FIELD_PREP(BMA220_INT_EN_DRDY_MSK, state));
> +}

> @@ -417,6 +436,24 @@ static void bma220_deinit(void *data_ptr)
>  			 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;
> +	unsigned int bma220_reg_if1;
> +
> +	guard(mutex)(&data->lock);

What is the lock protecting here?  The internal locks in regmap
superficially look like they'd be enough given we only have a single
read.  I'd expect any necessary locking to be on the otherside
of that use of the iio trigger framework (so the poll function thread).

> +	rv = regmap_read(data->regmap, BMA220_REG_IF1, &bma220_reg_if1);
> +	if (rv)
> +		return IRQ_NONE;
> +
> +	if (FIELD_GET(BMA220_IF_DRDY, bma220_reg_if1)) {
> +		iio_trigger_poll_nested(data->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
>  int bma220_common_probe(struct device *dev, struct regmap *regmap, int irq)
>  {
>  	int ret;
> @@ -446,6 +483,30 @@ int bma220_common_probe(struct device *dev, struct regmap *regmap, int irq)
>  	indio_dev->num_channels = ARRAY_SIZE(bma220_channels);
>  	indio_dev->available_scan_masks = bma220_accel_scan_masks;
>  
> +	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(dev, data->trig);
> +		if (ret)
> +			return dev_err_probe(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,
Interrupt polarity should be coming from firmware.

It's a historically common mistake to encode it in the driver as it breaks
fairly standard things like using an inverter as a cheap level converter
in front of the interrupt pin.

So IRQF_ONEHSOT only should be all that is needed here.

> +						indio_dev->name, indio_dev);
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "request irq %d failed\n", irq);
> +	}
> +
>  	ret = devm_add_action_or_reset(dev, bma220_deinit, data);
>  	if (ret)
>  		return ret;
> 

[PATCH v3 16/18] 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>
---
v1->v2 rename variables to include unit capitalization (dB, Hz)
v2->v3 3->1 line comment (Jonathan)
---
 drivers/iio/accel/bma220_core.c | 59 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index 6297882bcf1b955291a2d8747984648bc6ee8512..508a1142a74b3fb330eea5222c4b0abc8fc37e49 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -94,13 +94,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',						\
@@ -125,6 +135,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 {
@@ -141,6 +152,16 @@ 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[] = {
+	[BMA220_COF_1000Hz] = 1000,
+	[BMA220_COF_500Hz] = 500,
+	[BMA220_COF_250Hz] = 250,
+	[BMA220_COF_125Hz] = 125,
+	[BMA220_COF_64Hz] = 64,
+	[BMA220_COF_32Hz] = 32,
+};
+
 static const unsigned long bma220_accel_scan_masks[] = {
 	BIT(AXIS_X) | BIT(AXIS_Y) | BIT(AXIS_Z),
 	0
@@ -254,6 +275,10 @@ 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];
+		return IIO_VAL_INT;
 	}
 
 	return -EINVAL;
@@ -272,6 +297,18 @@ static int bma220_find_match_2dt(const int (*tbl)[2], const int n,
 	return -EINVAL;
 }
 
+static int bma220_find_match(const int *arr, const int n, const int val)
+{
+	int i;
+
+	for (i = 0; i < n; i++) {
+		if (arr[i] == val)
+			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)
@@ -297,6 +334,21 @@ static int bma220_write_raw(struct iio_dev *indio_dev,
 			return ret;
 		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);
+		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)
+			return ret;
+		data->lpf_3dB_freq_idx = index;
+
 		return 0;
 	}
 
@@ -314,6 +366,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;
+		*length = ARRAY_SIZE(bma220_lpf_3dB_freq_Hz_table);
+		return IIO_AVAIL_LIST;
 	default:
 		return -EINVAL;
 	}

-- 
2.49.1

[PATCH v3 17/18] 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>
---
v1->v3 no change
---
 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 508a1142a74b3fb330eea5222c4b0abc8fc37e49..9bc7a3023de76b9b519d2a645535f12bb42f6f15 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -376,10 +376,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 v3 18/18] iio: accel: bma220: add maintainer

[Petre Rodan]

Add maintainer for this driver.

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

diff --git a/MAINTAINERS b/MAINTAINERS
index 75615d82593e173a9e07fd269163fecdc4711e8b..eb985bd06b7d960bbc25941a8ea7b420d83483b0 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