[PATCH v2 00/14] iio: accel: bma220 improvements

[PATCH v2 00/14] 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.

First time pushing with b4, crossing fingers.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
Petre Rodan (14):
      dt-bindings: iio: accel: bosch,bma220 cleanup typo
      dt-bindings: iio: accel: bosch,bma220 setup SPI clock mode
      dt-bindings: iio: accel: bosch,bma220 change irq type
      iio: accel: bma220: split original driver
      iio: accel: bma220: add open firmware table
      iio: accel: bma220: add get regulator check
      iio: accel: bma220: reset registers during init stage
      iio: accel: bma220: migrate to regmap API
      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                          |  18 +-
 drivers/iio/accel/Makefile                         |   4 +-
 drivers/iio/accel/bma220.h                         |  20 +
 drivers/iio/accel/bma220_core.c                    | 617 +++++++++++++++++++++
 drivers/iio/accel/bma220_i2c.c                     |  61 ++
 drivers/iio/accel/bma220_spi.c                     | 318 +----------
 8 files changed, 757 insertions(+), 297 deletions(-)
---
base-commit: 19dc57d72d2b9365ef185286886c432f980cff55
change-id: 20250907-bma220_improvements-e31641777e61

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

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

[Petre Rodan]

Cleanup typo present in the title.

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

Re: [PATCH v2 01/14] dt-bindings: iio: accel: bosch,bma220 cleanup typo

[Krzysztof Kozlowski]

On Wed, Sep 10, 2025 at 10:57:06AM +0300, Petre Rodan wrote:
> Cleanup typo present in the title.
> 
> 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(-)

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

Best regards,
Krzysztof

[PATCH v2 02/14] 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.

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

Re: [PATCH v2 02/14] dt-bindings: iio: accel: bosch,bma220 setup SPI clock mode

[Jonathan Cameron]

On Wed, 10 Sep 2025 10:57:07 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> 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.
> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>

Perhaps this one deserves a fixes tag? I'm not 100% sure for
dt-bindings that are broken like this.

> ---
> ChangeLog:
> - split out from a bigger patch file
> ---
>  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>;
>          };
> 

Re: [PATCH v2 02/14] dt-bindings: iio: accel: bosch,bma220 setup SPI clock mode

[Krzysztof Kozlowski]

On Wed, Sep 10, 2025 at 10:57:07AM +0300, Petre Rodan wrote:
> 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.
> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> ---
> ChangeLog:
> - split out from a bigger patch file

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

Best regards,
Krzysztof

[PATCH v2 03/14] dt-bindings: iio: accel: bosch,bma220 change irq type

[Petre Rodan]

Set the interrupt type to rising edge instead of high level.

Quoting from the datasheet:

 "If at least one of the configured conditions applies, an interrupt
 (logic ‘1’) is issued through the INT pin of the sensor."

The old code did not request an irq line via devm_request_threaded_irq()
so there is no backward compatibility that would be affected by this
change.

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 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 v2 03/14] dt-bindings: iio: accel: bosch,bma220 change irq type

[Krzysztof Kozlowski]

On Wed, Sep 10, 2025 at 10:57:08AM +0300, Petre Rodan wrote:
> Set the interrupt type to rising edge instead of high level.
> 
> Quoting from the datasheet:
> 
>  "If at least one of the configured conditions applies, an interrupt
>  (logic ‘1’) is issued through the INT pin of the sensor."

I don't see how this explains/suggests raising edge.

> 
> The old code did not request an irq line via devm_request_threaded_irq()
> so there is no backward compatibility that would be affected by this
> change.

This sentence is irrelevant. You are changing only example, which has
zero impact.

Rewrite the subject and commit msg to accurately say this - it is only
example - because now you suggest you actually change something...

Best regards,
Krzysztof

Re: [PATCH v2 03/14] dt-bindings: iio: accel: bosch,bma220 change irq type

[Petre Rodan]

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


Hi Krzysztof,

On Thu, Sep 11, 2025 at 09:33:25AM +0200, Krzysztof Kozlowski wrote:
> On Wed, Sep 10, 2025 at 10:57:08AM +0300, Petre Rodan wrote:
> > Set the interrupt type to rising edge instead of high level.
> > 
> > Quoting from the datasheet:
> > 
> >  "If at least one of the configured conditions applies, an interrupt
> >  (logic ???1???) is issued through the INT pin of the sensor."
> 
> I don't see how this explains/suggests raising edge.

well, I want my driver to react directly at the transition from lo to hi
(aka rising edge) of the INT signal.

why?
1. by default the latch register is disabled, thus the master (my driver) is not
expected to ack or clear any interrupt flag. the sensor controls when the INT line
deasserts. for instance during a tap interrupt the bottom half handles the rising
edge condition after 250-300us and the irq gets deasserted by the sensor 150-300ms
later without any other interaction from the driver.

which means that the time interval the trigger is asserted for can not be
controlled by the master (without tweaking the latch register).

2. there is no scenario in which an irq trigger that has been asserted before probe()
needs to be handled.

3. there is zero reason to handle one event multiple times.

these would be my reasons for gravitating toward edge triggering.

best regards,
peter

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

[PATCH v2 04/14] 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.
Fix checkpatch warning about GPL v2 license in bma220_spi.c.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
Changes:
- 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
- include bma220.h in bma220_core.c
- add mutex.h and pm.h includes to bma220_core.c
- cleanup struct spacing in bma220_spi.c
---
 drivers/iio/accel/Kconfig       |   9 +-
 drivers/iio/accel/Makefile      |   3 +-
 drivers/iio/accel/bma220.h      |  19 +++
 drivers/iio/accel/bma220_core.c | 313 ++++++++++++++++++++++++++++++++++++++++
 drivers/iio/accel/bma220_spi.c  | 307 ++-------------------------------------
 5 files changed, 354 insertions(+), 297 deletions(-)

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 8c3f7cf55d5fa432a4d4662b184a46cd59c3ebca..2cc3075e26883df60b5068c73b0551e1dd02c32e 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -218,15 +218,20 @@ config BMA180
 
 config BMA220
 	tristate "Bosch BMA220 3-Axis Accelerometer Driver"
-	depends on SPI
 	select IIO_BUFFER
 	select IIO_TRIGGERED_BUFFER
+	select BMA220_SPI if SPI
 	help
 	  Say yes here to add support for the Bosch BMA220 triaxial
 	  acceleration sensor.
 
 	  To compile this driver as a module, choose M here: the
-	  module will be called bma220_spi.
+	  module will be called bma220_core and you will also get
+	  bma220_spi if SPI is enabled.
+
+config BMA220_SPI
+	tristate
+	depends on BMA220
 
 config BMA400
 	tristate "Bosch BMA400 3-Axis Accelerometer Driver"
diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
index 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..eb311183ebfe37d1a75d858d435eac777efc4ed8
--- /dev/null
+++ b/drivers/iio/accel/bma220.h
@@ -0,0 +1,19 @@
+/* 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>
+#include <linux/spi/spi.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 v2 04/14] iio: accel: bma220: split original driver

[Jonathan Cameron]

On Wed, 10 Sep 2025 10:57:09 +0300
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.

I'd add a note here that you are not even making the interfaces
bus type independent as you are doing that in later regmap patch.
Feels odd to have spi calls in common code!

> 
> Create the additional bma220_core module.
> Fix checkpatch warning about GPL v2 license in bma220_spi.c.

Also mention fixing a few includes in bma220_spi.c whilst you
were here.

A few really trivial comments.

> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> ---
> Changes:
> - 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
> - include bma220.h in bma220_core.c
> - add mutex.h and pm.h includes to bma220_core.c
> - cleanup struct spacing in bma220_spi.c
> ---
>  drivers/iio/accel/Kconfig       |   9 +-
>  drivers/iio/accel/Makefile      |   3 +-
>  drivers/iio/accel/bma220.h      |  19 +++
>  drivers/iio/accel/bma220_core.c | 313 ++++++++++++++++++++++++++++++++++++++++
>  drivers/iio/accel/bma220_spi.c  | 307 ++-------------------------------------
>  5 files changed, 354 insertions(+), 297 deletions(-)
> 
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 8c3f7cf55d5fa432a4d4662b184a46cd59c3ebca..2cc3075e26883df60b5068c73b0551e1dd02c32e 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -218,15 +218,20 @@ config BMA180
>  
>  config BMA220
>  	tristate "Bosch BMA220 3-Axis Accelerometer Driver"
> -	depends on SPI
>  	select IIO_BUFFER
>  	select IIO_TRIGGERED_BUFFER
> +	select BMA220_SPI if SPI
>  	help
>  	  Say yes here to add support for the Bosch BMA220 triaxial
>  	  acceleration sensor.
>  
>  	  To compile this driver as a module, choose M here: the
> -	  module will be called bma220_spi.
> +	  module will be called bma220_core and you will also get
> +	  bma220_spi if SPI is enabled.
> +
> +config BMA220_SPI
> +	tristate
> +	depends on BMA220
>  
>  config BMA400
>  	tristate "Bosch BMA400 3-Axis Accelerometer Driver"
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index 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..eb311183ebfe37d1a75d858d435eac777efc4ed8
> --- /dev/null
> +++ b/drivers/iio/accel/bma220.h
> @@ -0,0 +1,19 @@
> +/* 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>
> +#include <linux/spi/spi.h>

Don't need spi.h here. The forward declaration of spi_device
deals with only reason it would otherwise be needed.

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

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

As mentioned above, these additions are good but I think not strictly
related to rest of the change.  That's fine. Just mention them in the commit description

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

Re: [PATCH v2 04/14] iio: accel: bma220: split original driver

[David Lechner]

On 9/10/25 2:57 AM, Petre Rodan 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.
> Fix checkpatch warning about GPL v2 license in bma220_spi.c.
> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> ---
> Changes:
> - 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
> - include bma220.h in bma220_core.c
> - add mutex.h and pm.h includes to bma220_core.c
> - cleanup struct spacing in bma220_spi.c
> ---
>  drivers/iio/accel/Kconfig       |   9 +-
>  drivers/iio/accel/Makefile      |   3 +-
>  drivers/iio/accel/bma220.h      |  19 +++
>  drivers/iio/accel/bma220_core.c | 313 ++++++++++++++++++++++++++++++++++++++++
>  drivers/iio/accel/bma220_spi.c  | 307 ++-------------------------------------
>  5 files changed, 354 insertions(+), 297 deletions(-)
> 
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 8c3f7cf55d5fa432a4d4662b184a46cd59c3ebca..2cc3075e26883df60b5068c73b0551e1dd02c32e 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -218,15 +218,20 @@ config BMA180
>  
>  config BMA220
>  	tristate "Bosch BMA220 3-Axis Accelerometer Driver"
> -	depends on SPI

I think we still want to keep `depends on SPI`. (And later add `|| I2C`)

There isn't much point in allowing this on systems that it can't
communicate with.


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

[PATCH v2 05/14] 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>
---
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 v2 06/14] iio: accel: bma220: add get regulator check

[Petre Rodan]

Add devm_regulator_bulk_get_enable() to device probe().

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
no change
---
 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 v2 06/14] iio: accel: bma220: add get regulator check

[Jonathan Cameron]

On Wed, 10 Sep 2025 10:57:11 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

I don't follow the 'add get regulator check' of the patch description.
This is ensuring they are powered up if necessary.  So I'd
just go with the vague: "turn power supplies on"


> Add devm_regulator_bulk_get_enable() to device probe().
> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> ---
> no change
> ---
>  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)
> 

Re: [PATCH v2 06/14] iio: accel: bma220: add get regulator check

[Petre Rodan]

Hello Jonathan,

thank you for the blazing fast feedback!

On Wed, Sep 10, 2025 at 06:58:41PM +0100, Jonathan Cameron wrote:
> On Wed, 10 Sep 2025 10:57:11 +0300
> Petre Rodan <petre.rodan@subdimension.ro> wrote:
> 
> I don't follow the 'add get regulator check' of the patch description.
> This is ensuring they are powered up if necessary.  So I'd
> just go with the vague: "turn power supplies on"

to be honest I added this just because I've seen the devm_regulator_bulk_get_enable() function used all over the place in iio code.
but looking at the linux/regulator/consumers.h header I was more puzzled.
just some forward declarations and then static functions doing exactly nothing.
I'm afraid to ask what that is all about. placeholder for a future API?

best regards,
peter

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

Re: [PATCH v2 06/14] iio: accel: bma220: add get regulator check

[Andy Shevchenko]

On Wed, Sep 10, 2025 at 9:52 PM Petre Rodan <petre.rodan@subdimension.ro> wrote:
> On Wed, Sep 10, 2025 at 06:58:41PM +0100, Jonathan Cameron wrote:
> > On Wed, 10 Sep 2025 10:57:11 +0300
> > Petre Rodan <petre.rodan@subdimension.ro> wrote:
> >
> > I don't follow the 'add get regulator check' of the patch description.
> > This is ensuring they are powered up if necessary.  So I'd
> > just go with the vague: "turn power supplies on"
>
> to be honest I added this just because I've seen the devm_regulator_bulk_get_enable() function used all over the place in iio code.
> but looking at the linux/regulator/consumers.h header I was more puzzled.
> just some forward declarations and then static functions doing exactly nothing.
> I'm afraid to ask what that is all about. placeholder for a future API?

You probably see two things in one place:
1/ declaration of the function that is implemented in the C-code
2/ stub for the case when implementation is switched off (usually via
kernel configuration).

I recommend using elixir.bootlin.com to browse the code, or locally
via cscope or similar.


-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 07/14] iio: accel: bma220: reset registers during init stage

[Petre Rodan]

Bring all configuration registers to default values during device probe().

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

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index b6f1374a9cca52966c1055113710061a7284cf5a..322df516c90a7c645eeca579cae9803eb31caad1 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,31 +206,28 @@ static const struct iio_info bma220_info = {
 	.read_avail		= bma220_read_avail,
 };
 
-static int bma220_init(struct spi_device *spi)
+static int bma220_reset(struct spi_device *spi, bool up)
 {
-	int ret;
-	static const char * const regulator_names[] = { "vddd", "vddio", "vdda" };
+	int i, ret;
 
-	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");
+	/**
+	 * 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;
 
-	ret = bma220_read_reg(spi, BMA220_REG_ID);
-	if (ret != BMA220_CHIP_ID)
-		return -ENODEV;
+		if (up && (ret == BMA220_RESET_MODE))
+			return 0;
 
-	/* 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;
+		if (!up && (ret == BMA220_NONRESET_MODE))
+			return 0;
+	}
 
-	return 0;
+	return -EBUSY;
 }
 
 static int bma220_power(struct spi_device *spi, bool up)
@@ -244,16 +244,43 @@ static int bma220_power(struct spi_device *spi, bool up)
 		if (ret < 0)
 			return ret;
 
-		if (up && ret == BMA220_SUSPEND_SLEEP)
+		if (up && (ret == BMA220_SUSPEND_SLEEP))
 			return 0;
 
-		if (!up && ret == BMA220_SUSPEND_WAKE)
+		if (!up && (ret == BMA220_SUSPEND_WAKE))
 			return 0;
 	}
 
 	return -EBUSY;
 }
 
+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)
+		return -ENODEV;
+
+	/* 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;
+}
+
 static void bma220_deinit(void *spi)
 {
 	bma220_power(spi, false);

-- 
2.49.1

Re: [PATCH v2 07/14] iio: accel: bma220: reset registers during init stage

[Jonathan Cameron]

On Wed, 10 Sep 2025 10:57:12 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> Bring all configuration registers to default values during device probe().
> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>

Hi Petre

One comment on code you happened to be moving.  The patch itself looks good
to me so this is a 'whilst we are here' type comment, though should be addressed
in a separate patch.

thanks,

Jonathan

> +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)
> +		return -ENODEV;

Not relevant to this patch as such, but we should relax this constraint so that
fallback compatibles work in device tree.  Convention is to just print a
dev_info() message if we get a wrong ID and carry on.

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

Re: [PATCH v2 07/14] iio: accel: bma220: reset registers during init stage

[Krzysztof Kozlowski]

On 10/09/2025 09:57, Petre Rodan wrote:
> Bring all configuration registers to default values during device probe().
> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> ---
>  drivers/iio/accel/bma220_core.c | 71 ++++++++++++++++++++++++++++-------------
>  1 file changed, 49 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
> index b6f1374a9cca52966c1055113710061a7284cf5a..322df516c90a7c645eeca579cae9803eb31caad1 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,31 +206,28 @@ static const struct iio_info bma220_info = {
>  	.read_avail		= bma220_read_avail,
>  };
>  
> -static int bma220_init(struct spi_device *spi)
> +static int bma220_reset(struct spi_device *spi, bool up)
>  {
> -	int ret;
> -	static const char * const regulator_names[] = { "vddd", "vddio", "vdda" };
> +	int i, ret;
>  
> -	ret = devm_regulator_bulk_get_enable(&spi->dev,


You just added this code in patch 6. Don't add code which immediately
you remove. I understand you re-add this later, so basically it is a
move, but such patch diff is still confusing.


Best regards,
Krzysztof

Re: [PATCH v2 07/14] iio: accel: bma220: reset registers during init stage

[Petre Rodan]

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


Hi Krzysztof,

On Thu, Sep 11, 2025 at 09:35:52AM +0200, Krzysztof Kozlowski wrote:
> On 10/09/2025 09:57, Petre Rodan wrote:
> > Bring all configuration registers to default values during device probe().
> > 
> > Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> > ---
> >  drivers/iio/accel/bma220_core.c | 71 ++++++++++++++++++++++++++++-------------
> >  1 file changed, 49 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
> > index b6f1374a9cca52966c1055113710061a7284cf5a..322df516c90a7c645eeca579cae9803eb31caad1 100644
> > --- a/drivers/iio/accel/bma220_core.c
> > -static int bma220_init(struct spi_device *spi)
> > +static int bma220_reset(struct spi_device *spi, bool up)
> >  {
> > -	int ret;
> > -	static const char * const regulator_names[] = { "vddd", "vddio", "vdda" };
> > +	int i, ret;
> >  
> > -	ret = devm_regulator_bulk_get_enable(&spi->dev,
> 
> 
> You just added this code in patch 6. Don't add code which immediately
> you remove. I understand you re-add this later, so basically it is a
> move, but such patch diff is still confusing.

sorry, but this is an artefact of 'git diff' I don't think I have no control of.

the bma220_reset() function was added to bma220_core.c with this patch and the
diff process merged lines from this new function with lines from bma220_init()
causing the apparent removal of the lines added in the previous patch.
if you look a few lines below your cut, the bma220_init() function contains the
code:

+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");
[..]

Just for my curiosity, do reviewers apply the patches one by one to (a branch of)
the tree itself or do they provide feedback directly based on the diffs?

best regards,
peter

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

Re: [PATCH v2 07/14] iio: accel: bma220: reset registers during init stage

[Krzysztof Kozlowski]

On 11/09/2025 14:36, Petre Rodan wrote:
> 
> Hi Krzysztof,
> 
> On Thu, Sep 11, 2025 at 09:35:52AM +0200, Krzysztof Kozlowski wrote:
>> On 10/09/2025 09:57, Petre Rodan wrote:
>>> Bring all configuration registers to default values during device probe().
>>>
>>> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
>>> ---
>>>  drivers/iio/accel/bma220_core.c | 71 ++++++++++++++++++++++++++++-------------
>>>  1 file changed, 49 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
>>> index b6f1374a9cca52966c1055113710061a7284cf5a..322df516c90a7c645eeca579cae9803eb31caad1 100644
>>> --- a/drivers/iio/accel/bma220_core.c
>>> -static int bma220_init(struct spi_device *spi)
>>> +static int bma220_reset(struct spi_device *spi, bool up)
>>>  {
>>> -	int ret;
>>> -	static const char * const regulator_names[] = { "vddd", "vddio", "vdda" };
>>> +	int i, ret;
>>>  
>>> -	ret = devm_regulator_bulk_get_enable(&spi->dev,
>>
>>
>> You just added this code in patch 6. Don't add code which immediately
>> you remove. I understand you re-add this later, so basically it is a
>> move, but such patch diff is still confusing.
> 
> sorry, but this is an artefact of 'git diff' I don't think I have no control of.


Don't think so. Before bma220_init() was above bma220_power(). After
your patch bma220_init() is BELOW bma220_power(), so that's a move.



> 
> the bma220_reset() function was added to bma220_core.c with this patch and the
> diff process merged lines from this new function with lines from bma220_init()
> causing the apparent removal of the lines added in the previous patch.
> if you look a few lines below your cut, the bma220_init() function contains the
> code:
> 
> +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");
> [..]
> 
> Just for my curiosity, do reviewers apply the patches one by one to (a branch of)
> the tree itself or do they provide feedback directly based on the diffs?


Each commit must stand on its own, is reviewed independently and entire
patchset must be 100% bisectable.


Best regards,
Krzysztof

Re: [PATCH v2 07/14] iio: accel: bma220: reset registers during init stage

[Petre Rodan]

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


Hello.

On Thu, Sep 11, 2025 at 03:07:05PM +0200, Krzysztof Kozlowski wrote:
> On 11/09/2025 14:36, Petre Rodan wrote:
> > 
> > Hi Krzysztof,
> > 
> > On Thu, Sep 11, 2025 at 09:35:52AM +0200, Krzysztof Kozlowski wrote:
> >> On 10/09/2025 09:57, Petre Rodan wrote:
> >>> Bring all configuration registers to default values during device probe().
> >>>
> >>> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> >>> ---
> >>>  drivers/iio/accel/bma220_core.c | 71 ++++++++++++++++++++++++++++-------------
> >>>  1 file changed, 49 insertions(+), 22 deletions(-)
> >>>
> >>> diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
> >>> index b6f1374a9cca52966c1055113710061a7284cf5a..322df516c90a7c645eeca579cae9803eb31caad1 100644
> >>> --- a/drivers/iio/accel/bma220_core.c
> >>> -static int bma220_init(struct spi_device *spi)
> >>> +static int bma220_reset(struct spi_device *spi, bool up)
> >>>  {
> >>> -	int ret;
> >>> -	static const char * const regulator_names[] = { "vddd", "vddio", "vdda" };
> >>> +	int i, ret;
> >>>  
> >>> -	ret = devm_regulator_bulk_get_enable(&spi->dev,
> >>
> >>
> >> You just added this code in patch 6. Don't add code which immediately
> >> you remove. I understand you re-add this later, so basically it is a
> >> move, but such patch diff is still confusing.
> > 
> > sorry, but this is an artefact of 'git diff' I don't think I have no control of.
> 
> 
> Don't think so. Before bma220_init() was above bma220_power(). After
> your patch bma220_init() is BELOW bma220_power(), so that's a move.

you are correct, these two functions did change places due to the fact that
_init() started using _power(). I preffered to do the move instead
of adding a forward declaration and leaving _power() between _init() and _deinit().
the code was optimized for how it will look at the end of all this patching.

I thought you ment the code that was added the previous patch was not removed per
se from _init(), which was not the case.

best regards,
peter

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

Re: [PATCH v2 07/14] iio: accel: bma220: reset registers during init stage

[Andy Shevchenko]

On Thu, Sep 11, 2025 at 4:52 PM Petre Rodan <petre.rodan@subdimension.ro> wrote:
> On Thu, Sep 11, 2025 at 03:07:05PM +0200, Krzysztof Kozlowski wrote:
> > On 11/09/2025 14:36, Petre Rodan wrote:
> > > On Thu, Sep 11, 2025 at 09:35:52AM +0200, Krzysztof Kozlowski wrote:
> > >> On 10/09/2025 09:57, Petre Rodan wrote:

...

> > >> You just added this code in patch 6. Don't add code which immediately
> > >> you remove. I understand you re-add this later, so basically it is a
> > >> move, but such patch diff is still confusing.
> > >
> > > sorry, but this is an artefact of 'git diff' I don't think I have no control of.
> >
> >
> > Don't think so. Before bma220_init() was above bma220_power(). After
> > your patch bma220_init() is BELOW bma220_power(), so that's a move.
>
> you are correct, these two functions did change places due to the fact that
> _init() started using _power(). I preffered to do the move instead
> of adding a forward declaration and leaving _power() between _init() and _deinit().
> the code was optimized for how it will look at the end of all this patching.

The idea is to balance between two, but for certain I agree with
Krzysztof, we need to avoid "ping-pong"ing the code in the same
series. If you need to move, create a no change patch that _just
moves_ one function up in the code.

> I thought you ment the code that was added the previous patch was not removed per
> se from _init(), which was not the case.



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 07/14] iio: accel: bma220: reset registers during init stage

[David Lechner]

On 9/11/25 7:36 AM, Petre Rodan wrote:
> 

...

> Just for my curiosity, do reviewers apply the patches one by one to (a branch of)
> the tree itself or do they provide feedback directly based on the diffs?
> 

I think most reviewers are like me and only are looking at the diff in the
email. It would take too much time to apply every single patch I look at. So
I only do that in very rare cases when I have a special interest in a patch.

Re: [PATCH v2 07/14] iio: accel: bma220: reset registers during init stage

[Jonathan Cameron]

On Thu, 11 Sep 2025 08:44:16 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 9/11/25 7:36 AM, Petre Rodan wrote:
> >   
> 
> ...
> 
> > Just for my curiosity, do reviewers apply the patches one by one to (a branch of)
> > the tree itself or do they provide feedback directly based on the diffs?
> >   
> 
> I think most reviewers are like me and only are looking at the diff in the
> email. It would take too much time to apply every single patch I look at. So
> I only do that in very rare cases when I have a special interest in a patch.

Likewise.

Jonathan

Re: [PATCH v2 07/14] iio: accel: bma220: reset registers during init stage

[David Lechner]

On 9/10/25 2:57 AM, Petre Rodan wrote:
> Bring all configuration registers to default values during device probe().
> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> ---

...

>  static int bma220_power(struct spi_device *spi, bool up)
> @@ -244,16 +244,43 @@ static int bma220_power(struct spi_device *spi, bool up)
>  		if (ret < 0)
>  			return ret;
>  
> -		if (up && ret == BMA220_SUSPEND_SLEEP)
> +		if (up && (ret == BMA220_SUSPEND_SLEEP))

Over 80% of existing kernel code doesn't have the unnecessary ()
on similar expressions so I think we should leave it that way.

(just seems like adding noise to me)

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

[PATCH v2 08/14] iio: accel: bma220: migrate to regmap API

[Petre Rodan]

Switch to regmap API.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
ChangeLog:
- cleanup patch by splitting out unrelated changes
---
 drivers/iio/accel/Kconfig       |   2 +
 drivers/iio/accel/bma220.h      |   6 +-
 drivers/iio/accel/bma220_core.c | 266 ++++++++++++++++++++++++++++------------
 drivers/iio/accel/bma220_spi.c  |  10 +-
 4 files changed, 200 insertions(+), 84 deletions(-)

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 2cc3075e26883df60b5068c73b0551e1dd02c32e..9b6c35b759481df5ff3c91856f8783357d25de80 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -218,6 +218,7 @@ config BMA180
 
 config BMA220
 	tristate "Bosch BMA220 3-Axis Accelerometer Driver"
+	select REGMAP
 	select IIO_BUFFER
 	select IIO_TRIGGERED_BUFFER
 	select BMA220_SPI if SPI
@@ -231,6 +232,7 @@ config BMA220
 
 config BMA220_SPI
 	tristate
+	select REGMAP_SPI
 	depends on BMA220
 
 config BMA400
diff --git a/drivers/iio/accel/bma220.h b/drivers/iio/accel/bma220.h
index eb311183ebfe37d1a75d858d435eac777efc4ed8..f9f4fa3daf33665f07f8bf073468dff070b46d74 100644
--- a/drivers/iio/accel/bma220.h
+++ b/drivers/iio/accel/bma220.h
@@ -9,11 +9,11 @@
 #define _BMA220_H
 
 #include <linux/pm.h>
-#include <linux/spi/spi.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 322df516c90a7c645eeca579cae9803eb31caad1..4d8b65ea737a2d5fe74f98da13a582a80874a5af 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,64 @@
 #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_LATCH_MAX			0x7
+#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 +121,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 +139,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_get_time_ns(indio_dev));
 	iio_trigger_notify_done(indio_dev->trig);
 
 	return IRQ_HANDLED;
@@ -125,58 +200,65 @@ static int bma220_read_raw(struct iio_dev *indio_dev,
 			   int *val, int *val2, long mask)
 {
 	int ret;
-	u8 range_idx;
+	u8 index;
+	unsigned int reg;
 	struct bma220_data *data = iio_priv(indio_dev);
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		ret = bma220_read_reg(data->spi_device, chan->address);
+		ret = regmap_read(data->regmap, chan->address, &reg);
 		if (ret < 0)
 			return -EINVAL;
-		*val = sign_extend32(ret >> chan->scan_type.shift,
+		*val = sign_extend32(reg >> chan->scan_type.shift,
 				     chan->scan_type.realbits - 1);
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
-		ret = bma220_read_reg(data->spi_device, BMA220_REG_RANGE);
-		if (ret < 0)
-			return ret;
-		range_idx = ret & BMA220_RANGE_MASK;
-		*val = bma220_scale_table[range_idx][0];
-		*val2 = bma220_scale_table[range_idx][1];
+		index = data->range_idx;
+		*val = bma220_scale_table[index][0];
+		*val2 = bma220_scale_table[index][1];
 		return IIO_VAL_INT_PLUS_MICRO;
 	}
 
 	return -EINVAL;
 }
 
+static int bma220_find_match_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);
 
+	guard(mutex)(&data->lock);
+
 	switch (mask) {
 	case IIO_CHAN_INFO_SCALE:
-		for (i = 0; i < ARRAY_SIZE(bma220_scale_table); i++)
-			if (val == bma220_scale_table[i][0] &&
-			    val2 == bma220_scale_table[i][1]) {
-				index = i;
-				break;
-			}
+		index = bma220_find_match_2dt(bma220_scale_table,
+					      ARRAY_SIZE(bma220_scale_table),
+					      val, val2);
 		if (index < 0)
 			return -EINVAL;
 
-		mutex_lock(&data->lock);
-		data->tx_buf[0] = BMA220_REG_RANGE;
-		data->tx_buf[1] = index;
-		ret = spi_write(data->spi_device, data->tx_buf,
-				sizeof(data->tx_buf));
+		ret = regmap_update_bits(data->regmap, BMA220_REG_RANGE,
+					 BMA220_RANGE_MASK,
+					 FIELD_PREP(BMA220_RANGE_MASK, index));
 		if (ret < 0)
 			return ret;
-		mutex_unlock(&data->lock);
+		data->range_idx = index;
 
 		return 0;
 	}
@@ -206,9 +288,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 +301,113 @@ 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)
+	/* Try to read chip_id register. It must 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");
+
+	if (val != BMA220_CHIP_ID)
 		return -ENODEV;
 
-	/* Make sure the chip is powered on and config registers are reset */
-	ret = bma220_power(spi, true);
+	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 +416,35 @@ 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,
-					      iio_pollfunc_store_time,
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
 					      bma220_trigger_handler, NULL);
 	if (ret < 0) {
-		dev_err(&spi->dev, "iio triggered buffer setup failed\n");
+		dev_err(dev, "iio triggered buffer setup failed\n");
 		return ret;
 	}
 
-	return devm_iio_device_register(&spi->dev, indio_dev);
+	return devm_iio_device_register(dev, indio_dev);
 }
-EXPORT_SYMBOL_NS(bma220_common_probe, "IIO_BOSCH_BMA220");
+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 v2 08/14] iio: accel: bma220: migrate to regmap API

[Jonathan Cameron]

On Wed, 10 Sep 2025 10:57:13 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

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

Hi Petre

There are a few things in here that seem unrelated to the regmap change
and should be in separate patches where we can review and discuss them more easily.

Thanks,

Jonathan

> diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
> index 322df516c90a7c645eeca579cae9803eb31caad1..4d8b65ea737a2d5fe74f98da13a582a80874a5af 100644
> --- a/drivers/iio/accel/bma220_core.c
> +++ b/drivers/iio/accel/bma220_core.c

> +#define BMA220_INT_LATCH_MSK			GENMASK(6, 4)
> +#define BMA220_INT_LATCH_MAX			0x7

Not particular important here but if this is used in later patches how I'd expect
you can use FIELD_FIT() to achieve the same without an extra define.


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

Why the move to grabbing timestamps in the thread rather than the top half?

I don't necessarily mind that change but doesn't feel appropriate to have
it in the same patch as the regmap change.

> -err:
> -	mutex_unlock(&data->lock);
> +				    iio_get_time_ns(indio_dev));
>  	iio_trigger_notify_done(indio_dev->trig);
>  
>  	return IRQ_HANDLED;


>  static int bma220_write_raw(struct iio_dev *indio_dev,
>  			    struct iio_chan_spec const *chan,
>  			    int val, int val2, long mask)
>  {
> -	int i;
>  	int ret;
>  	int index = -1;
>  	struct bma220_data *data = iio_priv(indio_dev);
>  
> +	guard(mutex)(&data->lock);
> +
>  	switch (mask) {
>  	case IIO_CHAN_INFO_SCALE:
> -		for (i = 0; i < ARRAY_SIZE(bma220_scale_table); i++)
> -			if (val == bma220_scale_table[i][0] &&
> -			    val2 == bma220_scale_table[i][1]) {
> -				index = i;
> -				break;
> -			}
> +		index = bma220_find_match_2dt(bma220_scale_table,
> +					      ARRAY_SIZE(bma220_scale_table),
> +					      val, val2);

This feels like an unrelated change that belongs in a different patch.

>  		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 +288,12 @@ static const struct iio_info bma220_info = {
>  
> -static int bma220_init(struct spi_device *spi)
> +

one blank line enough and will tidy up the diff a little.

> +static int bma220_init(struct bma220_data *data)
...

> -	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
> -					      iio_pollfunc_store_time,
> +	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
>  					      bma220_trigger_handler, NULL);

As above, I wasn't expecting the switch from iio_pollfunc_store_time in this
patch.

>  	if (ret < 0) {
> -		dev_err(&spi->dev, "iio triggered buffer setup failed\n");
> +		dev_err(dev, "iio triggered buffer setup failed\n");
>  		return ret;

Can use dev_err_probe() to shorten this a little.

>  	}
>  
> -	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");

Re: [PATCH v2 08/14] iio: accel: bma220: migrate to regmap API

[Petre Rodan]

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


Hi Jonathan,

On Wed, Sep 10, 2025 at 07:12:25PM +0100, Jonathan Cameron wrote:
> On Wed, 10 Sep 2025 10:57:13 +0300
> Petre Rodan <petre.rodan@subdimension.ro> wrote:
> 
> > Switch to regmap API.
> > 
> There are a few things in here that seem unrelated to the regmap change
> and should be in separate patches where we can review and discuss them more easily.
> 
> Thanks,
> Jonathan
> 
> > diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
> >>  static irqreturn_t bma220_trigger_handler(int irq, void *p)
> >  {
[..]
> >  	iio_push_to_buffers_with_ts(indio_dev, &data->scan, sizeof(data->scan),
> > -				    pf->timestamp);
> 
> Why the move to grabbing timestamps in the thread rather than the top half?
> 
> I don't necessarily mind that change but doesn't feel appropriate to have
> it in the same patch as the regmap change.

one of my unit tests [1] fails when using the original code (all timestamps are
0 when reading the IIO buffer with iio_generic_buffer)

would be easier to just split modification into new patch instead of debugging
the old code :)

[1]: https://codeberg.org/subDIMENSION/lkm_sandbox/src/branch/main/bosch_bma220/unit-tests/permanent_latch/test.sh

best regards,
peter

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

Re: [PATCH v2 08/14] iio: accel: bma220: migrate to regmap API

[Jonathan Cameron]

On Fri, 12 Sep 2025 17:54:30 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> Hi Jonathan,
> 
> On Wed, Sep 10, 2025 at 07:12:25PM +0100, Jonathan Cameron wrote:
> > On Wed, 10 Sep 2025 10:57:13 +0300
> > Petre Rodan <petre.rodan@subdimension.ro> wrote:
> >   
> > > Switch to regmap API.
> > >   
> > There are a few things in here that seem unrelated to the regmap change
> > and should be in separate patches where we can review and discuss them more easily.
> > 
> > Thanks,
> > Jonathan
> >   
> > > diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c  
> > >>  static irqreturn_t bma220_trigger_handler(int irq, void *p)  
> > >  {  
> [..]
> > >  	iio_push_to_buffers_with_ts(indio_dev, &data->scan, sizeof(data->scan),
> > > -				    pf->timestamp);  
> > 
> > Why the move to grabbing timestamps in the thread rather than the top half?
> > 
> > I don't necessarily mind that change but doesn't feel appropriate to have
> > it in the same patch as the regmap change.  
> 
> one of my unit tests [1] fails when using the original code (all timestamps are
> 0 when reading the IIO buffer with iio_generic_buffer)
> 
> would be easier to just split modification into new patch instead of debugging
> the old code :)
> 
> [1]: https://codeberg.org/subDIMENSION/lkm_sandbox/src/branch/main/bosch_bma220/unit-tests/permanent_latch/test.sh

Ah. So this is the problem we've had various attempts to fix of not all triggers
have a top half.  Definitely wants to be a separate patch with an explanation of
why you are making the change.  Hopefully someone will get time to finish of
fixing this up more generically but I don't mind papering over it in a particular
driver in the meantime because the generic fix is rather complex.

Jonathan

> 
> best regards,
> peter

[PATCH v2 09/14] 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>
---
no change
---
 drivers/iio/accel/Kconfig       |  9 +++++-
 drivers/iio/accel/Makefile      |  1 +
 drivers/iio/accel/bma220.h      |  1 +
 drivers/iio/accel/bma220_core.c | 18 ++++++++++++
 drivers/iio/accel/bma220_i2c.c  | 61 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 89 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 9b6c35b759481df5ff3c91856f8783357d25de80..b3c5b0b7a406ec0cec531a122af424cb8ec57703 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -221,6 +221,7 @@ config BMA220
 	select REGMAP
 	select IIO_BUFFER
 	select IIO_TRIGGERED_BUFFER
+	select BMA220_I2C if I2C
 	select BMA220_SPI if SPI
 	help
 	  Say yes here to add support for the Bosch BMA220 triaxial
@@ -228,7 +229,13 @@ config BMA220
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called bma220_core and you will also get
-	  bma220_spi if SPI is enabled.
+	  bma220_i2c if I2C is enabled and bma220_spi if SPI is
+	  enabled.
+
+config BMA220_I2C
+	tristate
+	select REGMAP_I2C
+	depends on BMA220
 
 config BMA220_SPI
 	tristate
diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
index 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..384557d10d5613b7d829c6666f3da06de219277a 100644
--- a/drivers/iio/accel/bma220.h
+++ b/drivers/iio/accel/bma220.h
@@ -12,6 +12,7 @@
 #include <linux/regmap.h>
 
 extern const struct regmap_config bma220_spi_regmap_config;
+extern const struct regmap_config bma220_i2c_regmap_config;
 extern const struct dev_pm_ops bma220_pm_ops;
 
 int bma220_common_probe(struct device *dev, struct regmap *regmap, int irq);
diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index 4d8b65ea737a2d5fe74f98da13a582a80874a5af..191074d8618ea2638f69283781b8677921876681 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -175,6 +175,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..2b63949ea64ee11421e76a2e7c868a922d1f9a12
--- /dev/null
+++ b/drivers/iio/accel/bma220_i2c.c
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Bosch triaxial acceleration sensor
+ *
+ * Copyright (c) 2025 Petre Rodan <petre.rodan@subdimension.ro>
+ *
+ * Datasheet: https://media.digikey.com/pdf/Data%20Sheets/Bosch/BMA220.pdf
+ * I2C address is either 0x0b or 0x0a depending on CSB (pin 10)
+ */
+
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+#include <linux/iio/iio.h>
+
+#include "bma220.h"
+
+static int bma220_i2c_probe(struct i2c_client *client)
+{
+	struct regmap *regmap;
+
+	regmap = devm_regmap_init_i2c(client, &bma220_i2c_regmap_config);
+	if (IS_ERR(regmap))
+		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

Re: [PATCH v2 09/14] iio: accel: bma220: add i2c module

[David Lechner]

On 9/10/25 2:57 AM, Petre Rodan wrote:
> 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>
> ---
> no change
> ---
>  drivers/iio/accel/Kconfig       |  9 +++++-
>  drivers/iio/accel/Makefile      |  1 +
>  drivers/iio/accel/bma220.h      |  1 +
>  drivers/iio/accel/bma220_core.c | 18 ++++++++++++
>  drivers/iio/accel/bma220_i2c.c  | 61 +++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 89 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 9b6c35b759481df5ff3c91856f8783357d25de80..b3c5b0b7a406ec0cec531a122af424cb8ec57703 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -221,6 +221,7 @@ config BMA220

	depends 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
> @@ -228,7 +229,13 @@ config BMA220
>  
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called bma220_core and you will also get
> -	  bma220_spi if SPI is enabled.
> +	  bma220_i2c if I2C is enabled and bma220_spi if SPI is
> +	  enabled.
> +
> +config BMA220_I2C
> +	tristate
> +	select REGMAP_I2C
> +	depends on BMA220
>  
>  config BMA220_SPI
>  	tristate
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index 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..384557d10d5613b7d829c6666f3da06de219277a 100644
> --- a/drivers/iio/accel/bma220.h
> +++ b/drivers/iio/accel/bma220.h
> @@ -12,6 +12,7 @@
>  #include <linux/regmap.h>
>  
>  extern const struct regmap_config bma220_spi_regmap_config;
> +extern const struct regmap_config bma220_i2c_regmap_config;

Up to now, i2c is before spi. So would be consistent to keep
doing that.

>  extern const struct dev_pm_ops bma220_pm_ops;
>  
>  int bma220_common_probe(struct device *dev, struct regmap *regmap, int irq);
> diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
> index 4d8b65ea737a2d5fe74f98da13a582a80874a5af..191074d8618ea2638f69283781b8677921876681 100644
> --- a/drivers/iio/accel/bma220_core.c
> +++ b/drivers/iio/accel/bma220_core.c
> @@ -175,6 +175,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..2b63949ea64ee11421e76a2e7c868a922d1f9a12
> --- /dev/null
> +++ b/drivers/iio/accel/bma220_i2c.c
> @@ -0,0 +1,61 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Bosch triaxial acceleration sensor
> + *
> + * Copyright (c) 2025 Petre Rodan <petre.rodan@subdimension.ro>
> + *
> + * Datasheet: https://media.digikey.com/pdf/Data%20Sheets/Bosch/BMA220.pdf
> + * I2C address is either 0x0b or 0x0a depending on CSB (pin 10)
> + */
> +
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/i2c.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/types.h>
> +
> +#include <linux/iio/iio.h>

It doesn't look like the iio header is used. Maybe not device.h either.

> +
> +#include "bma220.h"
> +

[PATCH v2 10/14] 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>
---
ChangeLog:
- hardcode 1ms timer watchdog to any i2c-based sensor instead of
  configuring a dt-based property
- rename bma220_wdt() to bma220_set_wdt()
---
 drivers/iio/accel/bma220_core.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index 191074d8618ea2638f69283781b8677921876681..b619f5a0bf713b4d386a5d4fc1d919e144798f02 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>
@@ -359,6 +360,11 @@ 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)
 {
@@ -390,6 +396,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 v2 11/14] iio: accel: bma220: add interrupt trigger

[Petre Rodan]

Add interrupt trigger.

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

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index b619f5a0bf713b4d386a5d4fc1d919e144798f02..69bfd7f9f2b35e67835c6e1e41e02258e6741e7b 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>
 
@@ -126,6 +127,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. */
@@ -194,6 +196,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;
@@ -418,6 +437,25 @@ 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;
+	u8 bma220_reg_if[2];
+
+	guard(mutex)(&data->lock);
+	rv = regmap_bulk_read(data->regmap, BMA220_REG_IF0, bma220_reg_if,
+			      sizeof(bma220_reg_if));
+	if (rv)
+		return IRQ_NONE;
+
+	if (FIELD_GET(BMA220_IF_DRDY, bma220_reg_if[1]))
+		iio_trigger_poll_nested(data->trig);
+
+	return IRQ_HANDLED;
+}
+
 int bma220_common_probe(struct device *dev, struct regmap *regmap, int irq)
 {
 	int ret;
@@ -447,6 +485,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 v2 11/14] iio: accel: bma220: add interrupt trigger

[Jonathan Cameron]

On Wed, 10 Sep 2025 10:57:16 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

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

One small thing inline,

Thanks,

Jonathan


>  static irqreturn_t bma220_trigger_handler(int irq, void *p)
>  {
>  	int ret;
> @@ -418,6 +437,25 @@ 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;
> +	u8 bma220_reg_if[2];
> +
> +	guard(mutex)(&data->lock);
> +	rv = regmap_bulk_read(data->regmap, BMA220_REG_IF0, bma220_reg_if,
> +			      sizeof(bma220_reg_if));

Given SPI needs DMA safe buffers and theory at least regmap could
have optimizations that don't bounce data in bulk accesses, please
use a DMA safe buffer here.  Put one at the end of iio_priv().
Can share a cacheline with the scan.

> +	if (rv)
> +		return IRQ_NONE;
> +
> +	if (FIELD_GET(BMA220_IF_DRDY, bma220_reg_if[1]))
> +		iio_trigger_poll_nested(data->trig);
> +
> +	return IRQ_HANDLED;
> +}

[PATCH v2 12/14] 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>
---
ChangeLog:
- rename variables to include unit capitalization (dB, Hz)
---
 drivers/iio/accel/bma220_core.c | 61 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 60 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index 69bfd7f9f2b35e67835c6e1e41e02258e6741e7b..16a1aac5aa178ec06dc26e7e632f8ec5f43b5f78 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -95,13 +95,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',						\
@@ -126,6 +136,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 {
@@ -142,6 +153,18 @@ static const struct iio_chan_spec bma220_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(3),
 };
 
+/*
+ * Available cut-off frequencies of the low pass filter in Hz.
+ */
+static const int bma220_lpf_3dB_freq_Hz_table[] = {
+	[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
@@ -255,6 +278,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;
@@ -273,6 +300,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)
@@ -298,6 +337,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;
 	}
 
@@ -315,6 +369,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

Re: [PATCH v2 12/14] iio: accel: bma220: add LPF cut-off frequency mapping

[Jonathan Cameron]

On Wed, 10 Sep 2025 10:57:17 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> Add mapping for the low pass filter cut-off frequency.
> Make valid values visible for both the cut-off frequency and the scale.
> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> ---
8 @@ static const struct iio_chan_spec bma220_channels[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(3),
>  };
>  
> +/*
> + * Available cut-off frequencies of the low pass filter in Hz.
> + */

Completely trivial, but stick to single line comments when it fits under 80 chars.

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

[PATCH v2 13/14] 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>
---
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 16a1aac5aa178ec06dc26e7e632f8ec5f43b5f78..981b0fdc306e936ce9fc45439f7c164f50de50c4 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -379,10 +379,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 v2 14/14] iio: accel: bma220: add maintainer

[Petre Rodan]

Add maintainer for this driver.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
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

Re: [PATCH v2 00/14] iio: accel: bma220 improvements

[Jonathan Cameron]

On Wed, 10 Sep 2025 10:57:05 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> 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.
> 
> First time pushing with b4, crossing fingers.
Worked nicely as far as I can see.  I should try it myself at some point.
I've been using magic finger memory to format patches for far too long :)

Anyhow, this is coming together nicely.  Don't rush a new version out
for a few days though to give others time to take a look.

thanks,

Jonathan

> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> ---
> Petre Rodan (14):
>       dt-bindings: iio: accel: bosch,bma220 cleanup typo
>       dt-bindings: iio: accel: bosch,bma220 setup SPI clock mode
>       dt-bindings: iio: accel: bosch,bma220 change irq type
>       iio: accel: bma220: split original driver
>       iio: accel: bma220: add open firmware table
>       iio: accel: bma220: add get regulator check
>       iio: accel: bma220: reset registers during init stage
>       iio: accel: bma220: migrate to regmap API
>       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                          |  18 +-
>  drivers/iio/accel/Makefile                         |   4 +-
>  drivers/iio/accel/bma220.h                         |  20 +
>  drivers/iio/accel/bma220_core.c                    | 617 +++++++++++++++++++++
>  drivers/iio/accel/bma220_i2c.c                     |  61 ++
>  drivers/iio/accel/bma220_spi.c                     | 318 +----------
>  8 files changed, 757 insertions(+), 297 deletions(-)
> ---
> base-commit: 19dc57d72d2b9365ef185286886c432f980cff55
> change-id: 20250907-bma220_improvements-e31641777e61
> 
> Best regards,