In preparation for the i2c module, move the original code into multiple
source files without any other functional change.
Create the additional bma220_core module which currently is not
providing an abstracted bus type (this will change with the regmap
patch).
Fix checkpatch warning about GPL v2 license in bma220_spi.c.
Fix a few includes to make the change functional and prepare for the
next patches.
Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1->v2
- split out open firmware table modification into separate patch
- bma220_write_raw() exits without dev_err() based on similar feedback
from David
- change includes in bma220.h (Jonathan)
- include bma220.h in bma220_core.c (Jonathan)
- add device.h, errno.h, mutex.h and pm.h includes to bma220_core.c
- cleanup struct spacing in bma220_spi.c
v2->v3
- keep depends on SPI (David)
- fix a few includes in bma220_spi.c (Jonathan)
- mention new includes in commit body (Jonathan)
---
drivers/iio/accel/Kconfig | 8 +-
drivers/iio/accel/Makefile | 3 +-
drivers/iio/accel/bma220.h | 18 +++
drivers/iio/accel/bma220_core.c | 313 ++++++++++++++++++++++++++++++++++++++++
drivers/iio/accel/bma220_spi.c | 307 ++-------------------------------------
5 files changed, 353 insertions(+), 296 deletions(-)
diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 8c3f7cf55d5fa432a4d4662b184a46cd59c3ebca..4648be329917a3054615ed324156d67f53baefb6 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -221,12 +221,18 @@ config BMA220
depends on SPI
select IIO_BUFFER
select IIO_TRIGGERED_BUFFER
+ select BMA220_SPI if SPI
help
Say yes here to add support for the Bosch BMA220 triaxial
acceleration sensor.
To compile this driver as a module, choose M here: the
- module will be called bma220_spi.
+ module will be called bma220_core and you will also get
+ bma220_spi if SPI is enabled.
+
+config BMA220_SPI
+ tristate
+ depends on BMA220
config BMA400
tristate "Bosch BMA400 3-Axis Accelerometer Driver"
diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
index ca8569e25aba31c3ae3437abf8506addbf5edffa..56a9f848f7f913633bc2a628c1ac5c9190774b9d 100644
--- a/drivers/iio/accel/Makefile
+++ b/drivers/iio/accel/Makefile
@@ -25,7 +25,8 @@ obj-$(CONFIG_ADXL380) += adxl380.o
obj-$(CONFIG_ADXL380_I2C) += adxl380_i2c.o
obj-$(CONFIG_ADXL380_SPI) += adxl380_spi.o
obj-$(CONFIG_BMA180) += bma180.o
-obj-$(CONFIG_BMA220) += bma220_spi.o
+obj-$(CONFIG_BMA220) += bma220_core.o
+obj-$(CONFIG_BMA220_SPI) += bma220_spi.o
obj-$(CONFIG_BMA400) += bma400_core.o
obj-$(CONFIG_BMA400_I2C) += bma400_i2c.o
obj-$(CONFIG_BMA400_SPI) += bma400_spi.o
diff --git a/drivers/iio/accel/bma220.h b/drivers/iio/accel/bma220.h
new file mode 100644
index 0000000000000000000000000000000000000000..dea34244b4fc4971841dc60d79db6c7db27bea20
--- /dev/null
+++ b/drivers/iio/accel/bma220.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Forward declarations needed by the bma220 sources.
+ *
+ * Copyright 2025 Petre Rodan <petre.rodan@subdimension.ro>
+ */
+
+#ifndef _BMA220_H
+#define _BMA220_H
+
+#include <linux/pm.h>
+
+extern const struct dev_pm_ops bma220_pm_ops;
+struct spi_device;
+
+int bma220_common_probe(struct spi_device *dev);
+
+#endif
diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
new file mode 100644
index 0000000000000000000000000000000000000000..6bc2e5c3fb6cebd50209acbcc2d5340630c27cd1
--- /dev/null
+++ b/drivers/iio/accel/bma220_core.c
@@ -0,0 +1,313 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * BMA220 Digital triaxial acceleration sensor driver
+ *
+ * Copyright (c) 2016,2020 Intel Corporation.
+ */
+
+#include <linux/bits.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pm.h>
+#include <linux/types.h>
+#include <linux/spi/spi.h>
+
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+
+#include "bma220.h"
+
+#define BMA220_REG_ID 0x00
+#define BMA220_REG_ACCEL_X 0x02
+#define BMA220_REG_ACCEL_Y 0x03
+#define BMA220_REG_ACCEL_Z 0x04
+#define BMA220_REG_RANGE 0x11
+#define BMA220_REG_SUSPEND 0x18
+
+#define BMA220_CHIP_ID 0xDD
+#define BMA220_READ_MASK BIT(7)
+#define BMA220_RANGE_MASK GENMASK(1, 0)
+#define BMA220_SUSPEND_SLEEP 0xFF
+#define BMA220_SUSPEND_WAKE 0x00
+
+#define BMA220_DEVICE_NAME "bma220"
+
+#define BMA220_ACCEL_CHANNEL(index, reg, axis) { \
+ .type = IIO_ACCEL, \
+ .address = reg, \
+ .modified = 1, \
+ .channel2 = IIO_MOD_##axis, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+ .scan_index = index, \
+ .scan_type = { \
+ .sign = 's', \
+ .realbits = 6, \
+ .storagebits = 8, \
+ .shift = 2, \
+ .endianness = IIO_CPU, \
+ }, \
+}
+
+enum bma220_axis {
+ AXIS_X,
+ AXIS_Y,
+ AXIS_Z,
+};
+
+static const int bma220_scale_table[][2] = {
+ {0, 623000}, {1, 248000}, {2, 491000}, {4, 983000},
+};
+
+struct bma220_data {
+ struct spi_device *spi_device;
+ struct mutex lock;
+ struct {
+ s8 chans[3];
+ /* Ensure timestamp is naturally aligned. */
+ aligned_s64 timestamp;
+ } scan;
+ u8 tx_buf[2] __aligned(IIO_DMA_MINALIGN);
+};
+
+static const struct iio_chan_spec bma220_channels[] = {
+ BMA220_ACCEL_CHANNEL(0, BMA220_REG_ACCEL_X, X),
+ BMA220_ACCEL_CHANNEL(1, BMA220_REG_ACCEL_Y, Y),
+ BMA220_ACCEL_CHANNEL(2, BMA220_REG_ACCEL_Z, Z),
+ IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
+static inline int bma220_read_reg(struct spi_device *spi, u8 reg)
+{
+ return spi_w8r8(spi, reg | BMA220_READ_MASK);
+}
+
+static const unsigned long bma220_accel_scan_masks[] = {
+ BIT(AXIS_X) | BIT(AXIS_Y) | BIT(AXIS_Z),
+ 0
+};
+
+static irqreturn_t bma220_trigger_handler(int irq, void *p)
+{
+ int ret;
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct bma220_data *data = iio_priv(indio_dev);
+ struct spi_device *spi = data->spi_device;
+
+ mutex_lock(&data->lock);
+ data->tx_buf[0] = BMA220_REG_ACCEL_X | BMA220_READ_MASK;
+ ret = spi_write_then_read(spi, data->tx_buf, 1, &data->scan.chans,
+ ARRAY_SIZE(bma220_channels) - 1);
+ if (ret < 0)
+ goto err;
+
+ iio_push_to_buffers_with_ts(indio_dev, &data->scan, sizeof(data->scan),
+ pf->timestamp);
+err:
+ mutex_unlock(&data->lock);
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
+static int bma220_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ int ret;
+ u8 range_idx;
+ struct bma220_data *data = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ ret = bma220_read_reg(data->spi_device, chan->address);
+ if (ret < 0)
+ return -EINVAL;
+ *val = sign_extend32(ret >> chan->scan_type.shift,
+ chan->scan_type.realbits - 1);
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ ret = bma220_read_reg(data->spi_device, BMA220_REG_RANGE);
+ if (ret < 0)
+ return ret;
+ range_idx = ret & BMA220_RANGE_MASK;
+ *val = bma220_scale_table[range_idx][0];
+ *val2 = bma220_scale_table[range_idx][1];
+ return IIO_VAL_INT_PLUS_MICRO;
+ }
+
+ return -EINVAL;
+}
+
+static int bma220_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ int i;
+ int ret;
+ int index = -1;
+ struct bma220_data *data = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_SCALE:
+ for (i = 0; i < ARRAY_SIZE(bma220_scale_table); i++)
+ if (val == bma220_scale_table[i][0] &&
+ val2 == bma220_scale_table[i][1]) {
+ index = i;
+ break;
+ }
+ if (index < 0)
+ return -EINVAL;
+
+ mutex_lock(&data->lock);
+ data->tx_buf[0] = BMA220_REG_RANGE;
+ data->tx_buf[1] = index;
+ ret = spi_write(data->spi_device, data->tx_buf,
+ sizeof(data->tx_buf));
+ if (ret < 0)
+ return ret;
+ mutex_unlock(&data->lock);
+
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static int bma220_read_avail(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ const int **vals, int *type, int *length,
+ long mask)
+{
+ switch (mask) {
+ case IIO_CHAN_INFO_SCALE:
+ *vals = (int *)bma220_scale_table;
+ *type = IIO_VAL_INT_PLUS_MICRO;
+ *length = ARRAY_SIZE(bma220_scale_table) * 2;
+ return IIO_AVAIL_LIST;
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct iio_info bma220_info = {
+ .read_raw = bma220_read_raw,
+ .write_raw = bma220_write_raw,
+ .read_avail = bma220_read_avail,
+};
+
+static int bma220_init(struct spi_device *spi)
+{
+ int ret;
+
+ ret = bma220_read_reg(spi, BMA220_REG_ID);
+ if (ret != BMA220_CHIP_ID)
+ return -ENODEV;
+
+ /* Make sure the chip is powered on */
+ ret = bma220_read_reg(spi, BMA220_REG_SUSPEND);
+ if (ret == BMA220_SUSPEND_WAKE)
+ ret = bma220_read_reg(spi, BMA220_REG_SUSPEND);
+ if (ret < 0)
+ return ret;
+ if (ret == BMA220_SUSPEND_WAKE)
+ return -EBUSY;
+
+ return 0;
+}
+
+static int bma220_power(struct spi_device *spi, bool up)
+{
+ int i, ret;
+
+ /**
+ * The chip can be suspended/woken up by a simple register read.
+ * So, we need up to 2 register reads of the suspend register
+ * to make sure that the device is in the desired state.
+ */
+ for (i = 0; i < 2; i++) {
+ ret = bma220_read_reg(spi, BMA220_REG_SUSPEND);
+ if (ret < 0)
+ return ret;
+
+ if (up && ret == BMA220_SUSPEND_SLEEP)
+ return 0;
+
+ if (!up && ret == BMA220_SUSPEND_WAKE)
+ return 0;
+ }
+
+ return -EBUSY;
+}
+
+static void bma220_deinit(void *spi)
+{
+ bma220_power(spi, false);
+}
+
+int bma220_common_probe(struct spi_device *spi)
+{
+ int ret;
+ struct iio_dev *indio_dev;
+ struct bma220_data *data;
+
+ indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*data));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ data = iio_priv(indio_dev);
+ data->spi_device = spi;
+ mutex_init(&data->lock);
+
+ indio_dev->info = &bma220_info;
+ indio_dev->name = BMA220_DEVICE_NAME;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->channels = bma220_channels;
+ indio_dev->num_channels = ARRAY_SIZE(bma220_channels);
+ indio_dev->available_scan_masks = bma220_accel_scan_masks;
+
+ ret = bma220_init(data->spi_device);
+ if (ret)
+ return ret;
+
+ ret = devm_add_action_or_reset(&spi->dev, bma220_deinit, spi);
+ if (ret)
+ return ret;
+
+ ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
+ iio_pollfunc_store_time,
+ bma220_trigger_handler, NULL);
+ if (ret < 0) {
+ dev_err(&spi->dev, "iio triggered buffer setup failed\n");
+ return ret;
+ }
+
+ return devm_iio_device_register(&spi->dev, indio_dev);
+}
+EXPORT_SYMBOL_NS(bma220_common_probe, "IIO_BOSCH_BMA220");
+
+static int bma220_suspend(struct device *dev)
+{
+ struct spi_device *spi = to_spi_device(dev);
+
+ return bma220_power(spi, false);
+}
+
+static int bma220_resume(struct device *dev)
+{
+ struct spi_device *spi = to_spi_device(dev);
+
+ return bma220_power(spi, true);
+}
+EXPORT_NS_SIMPLE_DEV_PM_OPS(bma220_pm_ops, bma220_suspend, bma220_resume,
+ IIO_BOSCH_BMA220);
+
+MODULE_AUTHOR("Tiberiu Breana <tiberiu.a.breana@intel.com>");
+MODULE_DESCRIPTION("BMA220 acceleration sensor driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/iio/accel/bma220_spi.c b/drivers/iio/accel/bma220_spi.c
index 01592eebf05bb6b002d44c41cca1d2dd5f28350c..3ad5e43aae496d265a8cf198595bf824f8e73692 100644
--- a/drivers/iio/accel/bma220_spi.c
+++ b/drivers/iio/accel/bma220_spi.c
@@ -5,326 +5,45 @@
* Copyright (c) 2016,2020 Intel Corporation.
*/
-#include <linux/bits.h>
-#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/errno.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/types.h>
#include <linux/spi/spi.h>
-#include <linux/iio/buffer.h>
#include <linux/iio/iio.h>
-#include <linux/iio/sysfs.h>
-#include <linux/iio/trigger_consumer.h>
-#include <linux/iio/triggered_buffer.h>
-#define BMA220_REG_ID 0x00
-#define BMA220_REG_ACCEL_X 0x02
-#define BMA220_REG_ACCEL_Y 0x03
-#define BMA220_REG_ACCEL_Z 0x04
-#define BMA220_REG_RANGE 0x11
-#define BMA220_REG_SUSPEND 0x18
+#include "bma220.h"
-#define BMA220_CHIP_ID 0xDD
-#define BMA220_READ_MASK BIT(7)
-#define BMA220_RANGE_MASK GENMASK(1, 0)
-#define BMA220_SUSPEND_SLEEP 0xFF
-#define BMA220_SUSPEND_WAKE 0x00
-
-#define BMA220_DEVICE_NAME "bma220"
-
-#define BMA220_ACCEL_CHANNEL(index, reg, axis) { \
- .type = IIO_ACCEL, \
- .address = reg, \
- .modified = 1, \
- .channel2 = IIO_MOD_##axis, \
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
- .scan_index = index, \
- .scan_type = { \
- .sign = 's', \
- .realbits = 6, \
- .storagebits = 8, \
- .shift = 2, \
- .endianness = IIO_CPU, \
- }, \
-}
-
-enum bma220_axis {
- AXIS_X,
- AXIS_Y,
- AXIS_Z,
-};
-
-static const int bma220_scale_table[][2] = {
- {0, 623000}, {1, 248000}, {2, 491000}, {4, 983000},
-};
-
-struct bma220_data {
- struct spi_device *spi_device;
- struct mutex lock;
- struct {
- s8 chans[3];
- /* Ensure timestamp is naturally aligned. */
- aligned_s64 timestamp;
- } scan;
- u8 tx_buf[2] __aligned(IIO_DMA_MINALIGN);
-};
-
-static const struct iio_chan_spec bma220_channels[] = {
- BMA220_ACCEL_CHANNEL(0, BMA220_REG_ACCEL_X, X),
- BMA220_ACCEL_CHANNEL(1, BMA220_REG_ACCEL_Y, Y),
- BMA220_ACCEL_CHANNEL(2, BMA220_REG_ACCEL_Z, Z),
- IIO_CHAN_SOFT_TIMESTAMP(3),
-};
-
-static inline int bma220_read_reg(struct spi_device *spi, u8 reg)
-{
- return spi_w8r8(spi, reg | BMA220_READ_MASK);
-}
-
-static const unsigned long bma220_accel_scan_masks[] = {
- BIT(AXIS_X) | BIT(AXIS_Y) | BIT(AXIS_Z),
- 0
-};
-
-static irqreturn_t bma220_trigger_handler(int irq, void *p)
-{
- int ret;
- struct iio_poll_func *pf = p;
- struct iio_dev *indio_dev = pf->indio_dev;
- struct bma220_data *data = iio_priv(indio_dev);
- struct spi_device *spi = data->spi_device;
-
- mutex_lock(&data->lock);
- data->tx_buf[0] = BMA220_REG_ACCEL_X | BMA220_READ_MASK;
- ret = spi_write_then_read(spi, data->tx_buf, 1, &data->scan.chans,
- ARRAY_SIZE(bma220_channels) - 1);
- if (ret < 0)
- goto err;
-
- iio_push_to_buffers_with_ts(indio_dev, &data->scan, sizeof(data->scan),
- pf->timestamp);
-err:
- mutex_unlock(&data->lock);
- iio_trigger_notify_done(indio_dev->trig);
-
- return IRQ_HANDLED;
-}
-
-static int bma220_read_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- int *val, int *val2, long mask)
-{
- int ret;
- u8 range_idx;
- struct bma220_data *data = iio_priv(indio_dev);
-
- switch (mask) {
- case IIO_CHAN_INFO_RAW:
- ret = bma220_read_reg(data->spi_device, chan->address);
- if (ret < 0)
- return -EINVAL;
- *val = sign_extend32(ret >> chan->scan_type.shift,
- chan->scan_type.realbits - 1);
- return IIO_VAL_INT;
- case IIO_CHAN_INFO_SCALE:
- ret = bma220_read_reg(data->spi_device, BMA220_REG_RANGE);
- if (ret < 0)
- return ret;
- range_idx = ret & BMA220_RANGE_MASK;
- *val = bma220_scale_table[range_idx][0];
- *val2 = bma220_scale_table[range_idx][1];
- return IIO_VAL_INT_PLUS_MICRO;
- }
-
- return -EINVAL;
-}
-
-static int bma220_write_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- int val, int val2, long mask)
+static int bma220_spi_probe(struct spi_device *spi)
{
- int i;
- int ret;
- int index = -1;
- struct bma220_data *data = iio_priv(indio_dev);
-
- switch (mask) {
- case IIO_CHAN_INFO_SCALE:
- for (i = 0; i < ARRAY_SIZE(bma220_scale_table); i++)
- if (val == bma220_scale_table[i][0] &&
- val2 == bma220_scale_table[i][1]) {
- index = i;
- break;
- }
- if (index < 0)
- return -EINVAL;
-
- mutex_lock(&data->lock);
- data->tx_buf[0] = BMA220_REG_RANGE;
- data->tx_buf[1] = index;
- ret = spi_write(data->spi_device, data->tx_buf,
- sizeof(data->tx_buf));
- if (ret < 0)
- dev_err(&data->spi_device->dev,
- "failed to set measurement range\n");
- mutex_unlock(&data->lock);
-
- return 0;
- }
-
- return -EINVAL;
-}
-
-static int bma220_read_avail(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- const int **vals, int *type, int *length,
- long mask)
-{
- switch (mask) {
- case IIO_CHAN_INFO_SCALE:
- *vals = (int *)bma220_scale_table;
- *type = IIO_VAL_INT_PLUS_MICRO;
- *length = ARRAY_SIZE(bma220_scale_table) * 2;
- return IIO_AVAIL_LIST;
- default:
- return -EINVAL;
- }
-}
-
-static const struct iio_info bma220_info = {
- .read_raw = bma220_read_raw,
- .write_raw = bma220_write_raw,
- .read_avail = bma220_read_avail,
-};
-
-static int bma220_init(struct spi_device *spi)
-{
- int ret;
-
- ret = bma220_read_reg(spi, BMA220_REG_ID);
- if (ret != BMA220_CHIP_ID)
- return -ENODEV;
-
- /* Make sure the chip is powered on */
- ret = bma220_read_reg(spi, BMA220_REG_SUSPEND);
- if (ret == BMA220_SUSPEND_WAKE)
- ret = bma220_read_reg(spi, BMA220_REG_SUSPEND);
- if (ret < 0)
- return ret;
- if (ret == BMA220_SUSPEND_WAKE)
- return -EBUSY;
-
- return 0;
-}
-
-static int bma220_power(struct spi_device *spi, bool up)
-{
- int i, ret;
-
- /**
- * The chip can be suspended/woken up by a simple register read.
- * So, we need up to 2 register reads of the suspend register
- * to make sure that the device is in the desired state.
- */
- for (i = 0; i < 2; i++) {
- ret = bma220_read_reg(spi, BMA220_REG_SUSPEND);
- if (ret < 0)
- return ret;
-
- if (up && ret == BMA220_SUSPEND_SLEEP)
- return 0;
-
- if (!up && ret == BMA220_SUSPEND_WAKE)
- return 0;
- }
-
- return -EBUSY;
-}
-
-static void bma220_deinit(void *spi)
-{
- bma220_power(spi, false);
-}
-
-static int bma220_probe(struct spi_device *spi)
-{
- int ret;
- struct iio_dev *indio_dev;
- struct bma220_data *data;
-
- indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*data));
- if (!indio_dev)
- return -ENOMEM;
-
- data = iio_priv(indio_dev);
- data->spi_device = spi;
- mutex_init(&data->lock);
-
- indio_dev->info = &bma220_info;
- indio_dev->name = BMA220_DEVICE_NAME;
- indio_dev->modes = INDIO_DIRECT_MODE;
- indio_dev->channels = bma220_channels;
- indio_dev->num_channels = ARRAY_SIZE(bma220_channels);
- indio_dev->available_scan_masks = bma220_accel_scan_masks;
-
- ret = bma220_init(data->spi_device);
- if (ret)
- return ret;
-
- ret = devm_add_action_or_reset(&spi->dev, bma220_deinit, spi);
- if (ret)
- return ret;
-
- ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
- iio_pollfunc_store_time,
- bma220_trigger_handler, NULL);
- if (ret < 0) {
- dev_err(&spi->dev, "iio triggered buffer setup failed\n");
- return ret;
- }
-
- return devm_iio_device_register(&spi->dev, indio_dev);
-}
-
-static int bma220_suspend(struct device *dev)
-{
- struct spi_device *spi = to_spi_device(dev);
-
- return bma220_power(spi, false);
-}
-
-static int bma220_resume(struct device *dev)
-{
- struct spi_device *spi = to_spi_device(dev);
-
- return bma220_power(spi, true);
+ return bma220_common_probe(spi);
}
-static DEFINE_SIMPLE_DEV_PM_OPS(bma220_pm_ops, bma220_suspend, bma220_resume);
static const struct spi_device_id bma220_spi_id[] = {
- {"bma220", 0},
+ { "bma220", 0 },
{ }
};
static const struct acpi_device_id bma220_acpi_id[] = {
- {"BMA0220", 0},
+ { "BMA0220", 0 },
{ }
};
MODULE_DEVICE_TABLE(spi, bma220_spi_id);
-static struct spi_driver bma220_driver = {
+static struct spi_driver bma220_spi_driver = {
.driver = {
.name = "bma220_spi",
.pm = pm_sleep_ptr(&bma220_pm_ops),
.acpi_match_table = bma220_acpi_id,
},
- .probe = bma220_probe,
+ .probe = bma220_spi_probe,
.id_table = bma220_spi_id,
};
-module_spi_driver(bma220_driver);
+module_spi_driver(bma220_spi_driver);
MODULE_AUTHOR("Tiberiu Breana <tiberiu.a.breana@intel.com>");
-MODULE_DESCRIPTION("BMA220 acceleration sensor driver");
-MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("BMA220 triaxial acceleration sensor spi driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("IIO_BOSCH_BMA220");
--
2.49.1On Sat, Sep 13, 2025 at 6:40 PM Petre Rodan <petre.rodan@subdimension.ro> wrote: > > In preparation for the i2c module, move the original code into multiple > source files without any other functional change. > > Create the additional bma220_core module which currently is not > providing an abstracted bus type (this will change with the regmap > patch). > Fix checkpatch warning about GPL v2 license in bma220_spi.c. Wny not a prerequisite fix? > Fix a few includes to make the change functional and prepare for the > next patches. Ditto? ... > To compile this driver as a module, choose M here: the > - module will be called bma220_spi. > + module will be called bma220_core and you will also get > + bma220_spi if SPI is enabled. I'm not sure the last part is practical, as one needs to list all buses next time a new one will be added. ... > +++ b/drivers/iio/accel/bma220.h > +#ifndef _BMA220_H > +#define _BMA220_H > + > +#include <linux/pm.h> > + > +extern const struct dev_pm_ops bma220_pm_ops; > +struct spi_device; Besides the location of this (I would expect it to follow up include linux/*) convert the existing driver to regmap first and remove this unneeded churn. > +int bma220_common_probe(struct spi_device *dev); > + > +#endif ... > +#include <linux/bits.h> > +#include <linux/kernel.h> You should not use this header. > +#include <linux/mod_devicetable.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/pm.h> > +#include <linux/types.h> > +#include <linux/spi/spi.h> ... > + /** Do not inherit problems from the past. > + * The chip can be suspended/woken up by a simple register read. > + * So, we need up to 2 register reads of the suspend register > + * to make sure that the device is in the desired state. > + */ ... I haven't reviewed the rest as I believe it's just ~1:1 copy of the existing code, but I still think that the result will be better if this series starts from small fixes, like kernel doc, and other things, followed by the regmap conversion and only _after_ the split is made. -- With Best Regards, Andy Shevchenko
hi Andy,
On Sun, Sep 14, 2025 at 03:45:07PM +0300, Andy Shevchenko wrote:
> > To compile this driver as a module, choose M here: the
> > - module will be called bma220_spi.
> > + module will be called bma220_core and you will also get
> > + bma220_spi if SPI is enabled.
>
> I'm not sure the last part is practical, as one needs to list all
> buses next time a new one will be added.
it's just a friendly note to the user of how the modules are called.
the sensor supports only spi and i2c, so the list is short.
> > +++ b/drivers/iio/accel/bma220.h
>
> > +#ifndef _BMA220_H
> > +#define _BMA220_H
> > +
> > +#include <linux/pm.h>
> > +
> > +extern const struct dev_pm_ops bma220_pm_ops;
>
> > +struct spi_device;
>
> Besides the location of this (I would expect it to follow up include
> linux/*) convert the existing driver to regmap first and remove this
> unneeded churn.
[..]
> I haven't reviewed the rest as I believe it's just ~1:1 copy of the
> existing code, but I still think that the result will be better if
> this series starts from small fixes, like kernel doc, and other
> things, followed by the regmap conversion and only _after_ the split
> is made.
either way you look at it it can be seen as churn.
scenario 1
split is done early: ~300 lines from _spi.c have to move to _core.c.
I tried to do a 1:1 copy so it can be easily diffed, but small tweaks are needed here and there.
scenario 2
split is done after regmap: much more than 300 lines need to be moved AND the code would diverge way too much from my target.
as I see it scenario 2 is worse from a reviewer's perspective and a nightmare from my pov.
I prefer to stick with scenario 1, adding a few prerequisite patches if you so prefer.
It's much easier for me to cherry pick modifications and copy them from my target code once the file structure in the patches have a _core.c, _spi.c, etc.
this is what I had in mind for v4:
split prerequisites (2-4p) -> split -> regmap prerequisites (4p) -> regmap -> regmap cleanup (2p) -> i2c -> features (5p).
best regards,
petre rodan
© 2016 - 2025 Red Hat, Inc.