2023-12-19 15:05:44

by Jun Yan

[permalink] [raw]
Subject: [PATCH v2] iio: accel: bmi088: add i2c support for bmi088 accel driver

The BMI088, BMI085 and BMI090L accelerometer also support
I2C protocol, so let's add the missing I2C support.

The I2C interface of the {BMI085,BMI088,BMI090L} is compatible with
the I2C Specification UM10204 Rev. 03 (19 June 2007), available at
http://www.nxp.com. The {BMI085,BMI088,BMI090L} supports I2C standard
mode and fast mode, only 7-bit address mode is supported.[1][2][3]

[1]: https://www.bosch-sensortec.com/media/boschsensortec/downloads/datasheets/bst-bmi085-ds001.pdf
[2]: https://www.bosch-sensortec.com/media/boschsensortec/downloads/datasheets/bst-bmi088-ds001.pdf
[3]: https://mm.digikey.com/Volume0/opasdata/d220001/medias/docus/4807/BST-BMI090L-DS000-00.pdf

Signed-off-by: Jun Yan <[email protected]>

Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

---

Changes in v2:
- fix compile error
- Link to v1: https://lore.kernel.org/linux-iio/[email protected]/
- build-tested on linux-next

---
---
drivers/iio/accel/Kconfig | 8 +++-
drivers/iio/accel/Makefile | 1 +
drivers/iio/accel/bmi088-accel-i2c.c | 69 ++++++++++++++++++++++++++++
3 files changed, 76 insertions(+), 2 deletions(-)
create mode 100644 drivers/iio/accel/bmi088-accel-i2c.c

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 91adcac875a4..dc92cf599acb 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -254,11 +254,11 @@ config BMC150_ACCEL_SPI

config BMI088_ACCEL
tristate "Bosch BMI088 Accelerometer Driver"
- depends on SPI
select IIO_BUFFER
select IIO_TRIGGERED_BUFFER
select REGMAP
- select BMI088_ACCEL_SPI
+ select BMI088_ACCEL_SPI if SPI
+ select BMI088_ACCEL_I2C if I2C
help
Say yes here to build support for the following Bosch accelerometers:
BMI088, BMI085, BMI090L. Note that all of these are combo module that
@@ -267,6 +267,10 @@ config BMI088_ACCEL
This driver only implements the accelerometer part, which has its own
address and register map. BMG160 provides the gyroscope driver.

+config BMI088_ACCEL_I2C
+ tristate
+ select REGMAP_I2C
+
config BMI088_ACCEL_SPI
tristate
select REGMAP_SPI
diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
index 311ead9c3ef1..db90532ba24a 100644
--- a/drivers/iio/accel/Makefile
+++ b/drivers/iio/accel/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o
obj-$(CONFIG_BMC150_ACCEL_I2C) += bmc150-accel-i2c.o
obj-$(CONFIG_BMC150_ACCEL_SPI) += bmc150-accel-spi.o
obj-$(CONFIG_BMI088_ACCEL) += bmi088-accel-core.o
+obj-$(CONFIG_BMI088_ACCEL_I2C) += bmi088-accel-i2c.o
obj-$(CONFIG_BMI088_ACCEL_SPI) += bmi088-accel-spi.o
obj-$(CONFIG_DA280) += da280.o
obj-$(CONFIG_DA311) += da311.o
diff --git a/drivers/iio/accel/bmi088-accel-i2c.c b/drivers/iio/accel/bmi088-accel-i2c.c
new file mode 100644
index 000000000000..642dc2607943
--- /dev/null
+++ b/drivers/iio/accel/bmi088-accel-i2c.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * 3-axis accelerometer driver supporting following Bosch-Sensortec chips:
+ * - BMI088
+ * - BMI085
+ * - BMI090L
+ *
+ * Copyright 2023 Jun Yan <[email protected]>
+ */
+
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+
+#include "bmi088-accel.h"
+
+static int bmi088_accel_probe(struct i2c_client *i2c)
+{
+ struct regmap *regmap;
+ const struct i2c_device_id *id = i2c_client_get_device_id(i2c);
+
+ regmap = devm_regmap_init_i2c(i2c, &bmi088_regmap_conf);
+ if (IS_ERR(regmap)) {
+ dev_err(&i2c->dev, "Failed to initialize i2c regmap\n");
+ return PTR_ERR(regmap);
+ }
+
+ return bmi088_accel_core_probe(&i2c->dev, regmap, i2c->irq,
+ id->driver_data);
+}
+
+static void bmi088_accel_remove(struct i2c_client *i2c)
+{
+ bmi088_accel_core_remove(&i2c->dev);
+}
+
+static const struct of_device_id bmi088_of_match[] = {
+ { .compatible = "bosch,bmi085-accel" },
+ { .compatible = "bosch,bmi088-accel" },
+ { .compatible = "bosch,bmi090l-accel" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, bmi088_of_match);
+
+static const struct i2c_device_id bmi088_accel_id[] = {
+ {"bmi085-accel", BOSCH_BMI085},
+ {"bmi088-accel", BOSCH_BMI088},
+ {"bmi090l-accel", BOSCH_BMI090L},
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, bmi088_accel_id);
+
+static struct i2c_driver bmi088_accel_driver = {
+ .driver = {
+ .name = "bmi088_accel_i2c",
+ .pm = pm_ptr(&bmi088_accel_pm_ops),
+ .of_match_table = bmi088_of_match,
+ },
+ .probe = bmi088_accel_probe,
+ .remove = bmi088_accel_remove,
+ .id_table = bmi088_accel_id,
+};
+module_i2c_driver(bmi088_accel_driver);
+
+MODULE_AUTHOR("Jun Yan <[email protected]>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("BMI088 accelerometer driver (I2C)");
+MODULE_IMPORT_NS(IIO_BMI088);
--
2.43.0



2024-01-07 16:57:13

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2] iio: accel: bmi088: add i2c support for bmi088 accel driver

On Tue, 19 Dec 2023 23:04:40 +0800
Jun Yan <[email protected]> wrote:

> The BMI088, BMI085 and BMI090L accelerometer also support
> I2C protocol, so let's add the missing I2C support.
>
> The I2C interface of the {BMI085,BMI088,BMI090L} is compatible with
> the I2C Specification UM10204 Rev. 03 (19 June 2007), available at
> http://www.nxp.com. The {BMI085,BMI088,BMI090L} supports I2C standard
> mode and fast mode, only 7-bit address mode is supported.[1][2][3]
>
> [1]: https://www.bosch-sensortec.com/media/boschsensortec/downloads/datasheets/bst-bmi085-ds001.pdf
> [2]: https://www.bosch-sensortec.com/media/boschsensortec/downloads/datasheets/bst-bmi088-ds001.pdf
> [3]: https://mm.digikey.com/Volume0/opasdata/d220001/medias/docus/4807/BST-BMI090L-DS000-00.pdf
Prefer to have these in the tags block (so no blank line before the sign off) as
>
Datasheet: https://www.bosch-sensortec.com/media/boschsensortec/downloads/datasheets/bst-bmi085-ds001.pdf
Datasheet: https://www.bosch-sensortec.com/media/boschsensortec/downloads/datasheets/bst-bmi088-ds001.pdf
Datasheet: https://mm.digikey.com/Volume0/opasdata/d220001/medias/docus/4807/BST-BMI090L-DS000-00.pdf
> Signed-off-by: Jun Yan <[email protected]>
>
> Reported-by: kernel test robot <[email protected]>
> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
These two tags don't belong here. They are only appropriate for a stand alone fix patch for
code that has already gone upstream.

It's good to reflect the useful responses of 0-day but do it in the changes block, not up here.

Please take a look at the git log for a driver and include recent contributors.
This is a fairly new driver, so likely Mike will still be active for example.
(no problem if it turns out his address bounces and hence you've not included him
- I'll find out shortly ;)

Otherwise, only real comment is on some whitespace + an extra include I'd like to see.

Given the changes are so minor, I'll make them whilst applying.
Applied to the togreg branch of iio.git but I will be rebasing that on rc1 in a few weeks
time. So in meantime only pushed out as testing.

Thanks,

Jonathan

>
> ---
>
> Changes in v2:
> - fix compile error
> - Link to v1: https://lore.kernel.org/linux-iio/[email protected]/
> - build-tested on linux-next
>
> ---
> ---
> drivers/iio/accel/Kconfig | 8 +++-
> drivers/iio/accel/Makefile | 1 +
> drivers/iio/accel/bmi088-accel-i2c.c | 69 ++++++++++++++++++++++++++++
> 3 files changed, 76 insertions(+), 2 deletions(-)
> create mode 100644 drivers/iio/accel/bmi088-accel-i2c.c
>
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 91adcac875a4..dc92cf599acb 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -254,11 +254,11 @@ config BMC150_ACCEL_SPI
>
> config BMI088_ACCEL
> tristate "Bosch BMI088 Accelerometer Driver"
> - depends on SPI
> select IIO_BUFFER
> select IIO_TRIGGERED_BUFFER
> select REGMAP
> - select BMI088_ACCEL_SPI
> + select BMI088_ACCEL_SPI if SPI
> + select BMI088_ACCEL_I2C if I2C
> help
> Say yes here to build support for the following Bosch accelerometers:
> BMI088, BMI085, BMI090L. Note that all of these are combo module that
> @@ -267,6 +267,10 @@ config BMI088_ACCEL
> This driver only implements the accelerometer part, which has its own
> address and register map. BMG160 provides the gyroscope driver.
>
> +config BMI088_ACCEL_I2C
> + tristate
> + select REGMAP_I2C
> +
> config BMI088_ACCEL_SPI
> tristate
> select REGMAP_SPI
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index 311ead9c3ef1..db90532ba24a 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -30,6 +30,7 @@ obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o
> obj-$(CONFIG_BMC150_ACCEL_I2C) += bmc150-accel-i2c.o
> obj-$(CONFIG_BMC150_ACCEL_SPI) += bmc150-accel-spi.o
> obj-$(CONFIG_BMI088_ACCEL) += bmi088-accel-core.o
> +obj-$(CONFIG_BMI088_ACCEL_I2C) += bmi088-accel-i2c.o
> obj-$(CONFIG_BMI088_ACCEL_SPI) += bmi088-accel-spi.o
> obj-$(CONFIG_DA280) += da280.o
> obj-$(CONFIG_DA311) += da311.o
> diff --git a/drivers/iio/accel/bmi088-accel-i2c.c b/drivers/iio/accel/bmi088-accel-i2c.c
> new file mode 100644
> index 000000000000..642dc2607943
> --- /dev/null
> +++ b/drivers/iio/accel/bmi088-accel-i2c.c
> @@ -0,0 +1,69 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * 3-axis accelerometer driver supporting following Bosch-Sensortec chips:
> + * - BMI088
> + * - BMI085
> + * - BMI090L
> + *
> + * Copyright 2023 Jun Yan <[email protected]>
> + */
> +
#include <linux/mod_devicetable.h>

for the various id tables (missing from the spi driver)

In general we try to include what is used in a given file
directly. There are exceptions to that though so don't take it too strongly
as advice.


> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/i2c.h>
> +
> +#include "bmi088-accel.h"
> +
> +static int bmi088_accel_probe(struct i2c_client *i2c)
> +{
> + struct regmap *regmap;
> + const struct i2c_device_id *id = i2c_client_get_device_id(i2c);
> +
> + regmap = devm_regmap_init_i2c(i2c, &bmi088_regmap_conf);
> + if (IS_ERR(regmap)) {
> + dev_err(&i2c->dev, "Failed to initialize i2c regmap\n");
> + return PTR_ERR(regmap);
> + }
> +
> + return bmi088_accel_core_probe(&i2c->dev, regmap, i2c->irq,
> + id->driver_data);
> +}
> +
> +static void bmi088_accel_remove(struct i2c_client *i2c)
> +{
> + bmi088_accel_core_remove(&i2c->dev);
> +}
> +
> +static const struct of_device_id bmi088_of_match[] = {
> + { .compatible = "bosch,bmi085-accel" },
> + { .compatible = "bosch,bmi088-accel" },
> + { .compatible = "bosch,bmi090l-accel" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, bmi088_of_match);
> +
> +static const struct i2c_device_id bmi088_accel_id[] = {
> + {"bmi085-accel", BOSCH_BMI085},
> + {"bmi088-accel", BOSCH_BMI088},
> + {"bmi090l-accel", BOSCH_BMI090L},

Prefer spaces after { and before }
(though I can see the spi driver doesn't do this either!)


> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, bmi088_accel_id);
> +
> +static struct i2c_driver bmi088_accel_driver = {
> + .driver = {
> + .name = "bmi088_accel_i2c",
> + .pm = pm_ptr(&bmi088_accel_pm_ops),
> + .of_match_table = bmi088_of_match,
> + },
> + .probe = bmi088_accel_probe,
> + .remove = bmi088_accel_remove,
> + .id_table = bmi088_accel_id,
> +};
> +module_i2c_driver(bmi088_accel_driver);
> +
> +MODULE_AUTHOR("Jun Yan <[email protected]>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("BMI088 accelerometer driver (I2C)");
> +MODULE_IMPORT_NS(IIO_BMI088);