All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/10] iio: accel: bmc150: Add support for BMA253/BMA254
@ 2021-06-11  8:08 Stephan Gerhold
  2021-06-11  8:08 ` [PATCH v3 01/10] iio: accel: bmc150: Fix bma222 scale unit Stephan Gerhold
                   ` (10 more replies)
  0 siblings, 11 replies; 16+ messages in thread
From: Stephan Gerhold @ 2021-06-11  8:08 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Rob Herring, Linus Walleij, Peter Meerwald,
	linux-iio, devicetree, Bastien Nocera, Hans de Goede,
	Andy Shevchenko, ~postmarketos/upstreaming, Stephan Gerhold

The Bosch BMA253 accelerometer is very similar to both BMA254 and BMA255.
The current situation is very confusing: BMA254 is supported by the bma180
driver, but BMA255 is supported by the bmc150-accel driver.

It turns out the bma180 and bmc150-accel drivers have quite some overlap,
and BMA253/BMA254 would be a bit better supported in bmc150-accel
(which has support for the motion trigger/interrupt).

This series adds BMA253 support to bmc150-accel and also moves BMA254
over to bmc150, removing some unnecessary code from the bma180 driver.

In v2 I also sneaked in a small fix for the scale table of BMA222
to simplify backporting for the stable people.

---
Changes in v3:
  - Add new "iio: accel: bmc150: Clarify combo modules in Kconfig" patch
  - Sort "one-line" chip name lists as well, not just multi-line ones

v2: https://lore.kernel.org/linux-iio/20210610122126.50504-1-stephan@gerhold.net/
Changes in v2:
  - Add new "iio: accel: bmc150: Fix bma222 scale unit" patch at the
    beginning so the stable people can backport it without conflicts
  - Add Reviewed-by: from Hans and Andy for all previous patches
  - Add patch 3 and 4 to have all the chip lists in a consistent order
  - Fix last patch to also drop BMA254 from the file header in bma180.c

v1: https://lore.kernel.org/linux-iio/20210610095300.3613-1-stephan@gerhold.net/

Stephan Gerhold (10):
  iio: accel: bmc150: Fix bma222 scale unit
  iio: accel: bmc150: Clarify combo modules in Kconfig
  iio: accel: bmc150: Drop misleading/duplicate chip identifiers
  iio: accel: bmc150: Drop duplicated documentation of supported chips
  iio: accel: bmc150: Sort all chip names alphabetically / by chip ID
  dt-bindings: iio: accel: bma255: Document bosch,bma253
  iio: accel: bmc150: Add device IDs for BMA253
  dt-bindings: iio: bma255: Allow multiple interrupts
  dt-bindings: iio: accel: bma180/bma255: Move bma254 to bma255 schema
  iio: accel: bma180/bmc150: Move BMA254 to bmc150-accel driver

 .../bindings/iio/accel/bosch,bma180.yaml      |  3 +-
 .../bindings/iio/accel/bosch,bma255.yaml      |  9 +-
 drivers/iio/accel/Kconfig                     | 11 ++-
 drivers/iio/accel/bma180.c                    | 92 +++----------------
 drivers/iio/accel/bmc150-accel-core.c         | 87 ++++++------------
 drivers/iio/accel/bmc150-accel-i2c.c          | 52 +++++------
 drivers/iio/accel/bmc150-accel-spi.c          | 31 ++++---
 drivers/iio/accel/bmc150-accel.h              | 10 --
 8 files changed, 98 insertions(+), 197 deletions(-)

-- 
2.32.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3 01/10] iio: accel: bmc150: Fix bma222 scale unit
  2021-06-11  8:08 [PATCH v3 00/10] iio: accel: bmc150: Add support for BMA253/BMA254 Stephan Gerhold
@ 2021-06-11  8:08 ` Stephan Gerhold
  2021-06-11  8:08 ` [PATCH v3 02/10] iio: accel: bmc150: Clarify combo modules in Kconfig Stephan Gerhold
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Stephan Gerhold @ 2021-06-11  8:08 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Rob Herring, Linus Walleij, Peter Meerwald,
	linux-iio, devicetree, Bastien Nocera, Hans de Goede,
	Andy Shevchenko, ~postmarketos/upstreaming, Stephan Gerhold

According to sysfs-bus-iio documentation the unit for accelerometer
values after applying scale/offset should be m/s^2, not g, which explains
why the scale values for the other variants in bmc150-accel do not match
exactly the values given in the datasheet.

To get the correct values, we need to multiply the BMA222 scale values
by g = 9.80665 m/s^2.

Fixes: a1a210bf29a1 ("iio: accel: bmc150-accel: Add support for BMA222")
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
Added this patch in v2 to avoid conflicts later in case this
should be backported to a stable kernel.
---
 drivers/iio/accel/bmc150-accel-core.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index a3d08d713362..a80ee0fdabc5 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -1145,11 +1145,12 @@ static const struct bmc150_accel_chip_info bmc150_accel_chip_info_tbl[] = {
 		/*
 		 * The datasheet page 17 says:
 		 * 15.6, 31.3, 62.5 and 125 mg per LSB.
+		 * IIO unit is m/s^2 so multiply by g = 9.80665 m/s^2.
 		 */
-		.scale_table = { {156000, BMC150_ACCEL_DEF_RANGE_2G},
-				 {313000, BMC150_ACCEL_DEF_RANGE_4G},
-				 {625000, BMC150_ACCEL_DEF_RANGE_8G},
-				 {1250000, BMC150_ACCEL_DEF_RANGE_16G} },
+		.scale_table = { {152984, BMC150_ACCEL_DEF_RANGE_2G},
+				 {306948, BMC150_ACCEL_DEF_RANGE_4G},
+				 {612916, BMC150_ACCEL_DEF_RANGE_8G},
+				 {1225831, BMC150_ACCEL_DEF_RANGE_16G} },
 	},
 	[bma222e] = {
 		.name = "BMA222E",
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 02/10] iio: accel: bmc150: Clarify combo modules in Kconfig
  2021-06-11  8:08 [PATCH v3 00/10] iio: accel: bmc150: Add support for BMA253/BMA254 Stephan Gerhold
  2021-06-11  8:08 ` [PATCH v3 01/10] iio: accel: bmc150: Fix bma222 scale unit Stephan Gerhold
@ 2021-06-11  8:08 ` Stephan Gerhold
  2021-06-11  8:08 ` [PATCH v3 03/10] iio: accel: bmc150: Drop misleading/duplicate chip identifiers Stephan Gerhold
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Stephan Gerhold @ 2021-06-11  8:08 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Rob Herring, Linus Walleij, Peter Meerwald,
	linux-iio, devicetree, Bastien Nocera, Hans de Goede,
	Andy Shevchenko, ~postmarketos/upstreaming, Stephan Gerhold

The Kconfig option currently says that all Bosch accelerometers
supported by the bmc150-accel driver are combo chips with both
accelerometer and magnetometer. This is wrong: actually only BMC150
is such a combo. The BMA* variants only contain an accelerometer
and the BMI055 actually is a accelerometer + gyroscope combo.

Clarify this in the help text and also make the list of supported
variants complete and sorted for consistency.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
New patch in v3 after discussion in
[PATCH v2 4/9] iio: accel: bmc150: Sort all chip names alphabetically / by chip ID
---
 drivers/iio/accel/Kconfig | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 17f6bdcf1db3..cbca6ab7da88 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -143,9 +143,12 @@ config BMC150_ACCEL
 	select BMC150_ACCEL_SPI if SPI
 	help
 	  Say yes here to build support for the following Bosch accelerometers:
-	  BMC150, BMI055, BMA250E, BMA222E, BMA255, BMA280.
+	  BMA222, BMA222E, BMA250E, BMA255, BMA280, BMC150, BMI055.
+
+	  Note that some of these are combo modules:
+	    - BMC150: accelerometer and magnetometer
+	    - BMI055: accelerometer and gyroscope
 
-	  This is a combo module with both accelerometer and magnetometer.
 	  This driver is only implementing accelerometer part, which has
 	  its own address and register map.
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 03/10] iio: accel: bmc150: Drop misleading/duplicate chip identifiers
  2021-06-11  8:08 [PATCH v3 00/10] iio: accel: bmc150: Add support for BMA253/BMA254 Stephan Gerhold
  2021-06-11  8:08 ` [PATCH v3 01/10] iio: accel: bmc150: Fix bma222 scale unit Stephan Gerhold
  2021-06-11  8:08 ` [PATCH v3 02/10] iio: accel: bmc150: Clarify combo modules in Kconfig Stephan Gerhold
@ 2021-06-11  8:08 ` Stephan Gerhold
  2021-06-11  8:08 ` [PATCH v3 04/10] iio: accel: bmc150: Drop duplicated documentation of supported chips Stephan Gerhold
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Stephan Gerhold @ 2021-06-11  8:08 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Rob Herring, Linus Walleij, Peter Meerwald,
	linux-iio, devicetree, Bastien Nocera, Hans de Goede,
	Andy Shevchenko, ~postmarketos/upstreaming, Stephan Gerhold

Commit 0ad4bf370176 ("iio:accel:bmc150-accel: Use the chip ID to detect
sensor variant") stopped using the I2C/ACPI match data to look up the
bmc150_accel_chip_info. However, the bmc150_accel_chip_info_tbl remained
as-is, with multiple entries with the same chip_id (e.g. 0xFA for
BMC150/BMI055/BMA255). This is redundant now because actually the driver
will always select the first entry with a matching chip_id.

So even if a device probes e.g. with BMA0255 it will end up using the
chip_info for BMC150. And in general that's fine for now, the entries
for BMC150/BMI055/BMA255 were exactly the same anyway (except for the
name, which is replaced with the more accurate one later).

But in this case it's misleading because it suggests that one should
add even more entries with the same chip_id when adding support for
new variants. Let's make that more clear by removing the enum with
the chip identifiers entirely and instead have only one entry per
chip_id.

Note that we may need to bring back some mechanism to differentiate
between different chips with the same chip_id in the future.
For example, BMA250 (currently supported by the bma180 driver) has the
same chip_id = 0x03 as BMA222 even though they have different channel
sizes (8 bits vs 10 bits). But in any case, that mechanism would
need to look quite different from what we have right now.

Cc: Bastien Nocera <hadess@hadess.net>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
Changes in v3: Sort combined chip names in chip info table
---
 drivers/iio/accel/bmc150-accel-core.c | 34 ++++++---------------------
 drivers/iio/accel/bmc150-accel-i2c.c  | 30 +++++++++++------------
 drivers/iio/accel/bmc150-accel-spi.c  | 30 +++++++++++------------
 drivers/iio/accel/bmc150-accel.h      | 10 --------
 4 files changed, 37 insertions(+), 67 deletions(-)

diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index a80ee0fdabc5..9ecbd3769593 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -1097,28 +1097,8 @@ static const struct iio_chan_spec bma280_accel_channels[] =
 	BMC150_ACCEL_CHANNELS(14);
 
 static const struct bmc150_accel_chip_info bmc150_accel_chip_info_tbl[] = {
-	[bmc150] = {
-		.name = "BMC150A",
-		.chip_id = 0xFA,
-		.channels = bmc150_accel_channels,
-		.num_channels = ARRAY_SIZE(bmc150_accel_channels),
-		.scale_table = { {9610, BMC150_ACCEL_DEF_RANGE_2G},
-				 {19122, BMC150_ACCEL_DEF_RANGE_4G},
-				 {38344, BMC150_ACCEL_DEF_RANGE_8G},
-				 {76590, BMC150_ACCEL_DEF_RANGE_16G} },
-	},
-	[bmi055] = {
-		.name = "BMI055A",
-		.chip_id = 0xFA,
-		.channels = bmc150_accel_channels,
-		.num_channels = ARRAY_SIZE(bmc150_accel_channels),
-		.scale_table = { {9610, BMC150_ACCEL_DEF_RANGE_2G},
-				 {19122, BMC150_ACCEL_DEF_RANGE_4G},
-				 {38344, BMC150_ACCEL_DEF_RANGE_8G},
-				 {76590, BMC150_ACCEL_DEF_RANGE_16G} },
-	},
-	[bma255] = {
-		.name = "BMA0255",
+	{
+		.name = "BMA255/BMC150/BMI055",
 		.chip_id = 0xFA,
 		.channels = bmc150_accel_channels,
 		.num_channels = ARRAY_SIZE(bmc150_accel_channels),
@@ -1127,7 +1107,7 @@ static const struct bmc150_accel_chip_info bmc150_accel_chip_info_tbl[] = {
 				 {38344, BMC150_ACCEL_DEF_RANGE_8G},
 				 {76590, BMC150_ACCEL_DEF_RANGE_16G} },
 	},
-	[bma250e] = {
+	{
 		.name = "BMA250E",
 		.chip_id = 0xF9,
 		.channels = bma250e_accel_channels,
@@ -1137,7 +1117,7 @@ static const struct bmc150_accel_chip_info bmc150_accel_chip_info_tbl[] = {
 				 {153277, BMC150_ACCEL_DEF_RANGE_8G},
 				 {306457, BMC150_ACCEL_DEF_RANGE_16G} },
 	},
-	[bma222] = {
+	{
 		.name = "BMA222",
 		.chip_id = 0x03,
 		.channels = bma222e_accel_channels,
@@ -1152,7 +1132,7 @@ static const struct bmc150_accel_chip_info bmc150_accel_chip_info_tbl[] = {
 				 {612916, BMC150_ACCEL_DEF_RANGE_8G},
 				 {1225831, BMC150_ACCEL_DEF_RANGE_16G} },
 	},
-	[bma222e] = {
+	{
 		.name = "BMA222E",
 		.chip_id = 0xF8,
 		.channels = bma222e_accel_channels,
@@ -1162,8 +1142,8 @@ static const struct bmc150_accel_chip_info bmc150_accel_chip_info_tbl[] = {
 				 {612915, BMC150_ACCEL_DEF_RANGE_8G},
 				 {1225831, BMC150_ACCEL_DEF_RANGE_16G} },
 	},
-	[bma280] = {
-		.name = "BMA0280",
+	{
+		.name = "BMA280",
 		.chip_id = 0xFB,
 		.channels = bma280_accel_channels,
 		.num_channels = ARRAY_SIZE(bma280_accel_channels),
diff --git a/drivers/iio/accel/bmc150-accel-i2c.c b/drivers/iio/accel/bmc150-accel-i2c.c
index d34dddb850d9..b8bda0dfb495 100644
--- a/drivers/iio/accel/bmc150-accel-i2c.c
+++ b/drivers/iio/accel/bmc150-accel-i2c.c
@@ -221,14 +221,14 @@ static int bmc150_accel_remove(struct i2c_client *client)
 }
 
 static const struct acpi_device_id bmc150_accel_acpi_match[] = {
-	{"BSBA0150",	bmc150},
-	{"BMC150A",	bmc150},
-	{"BMI055A",	bmi055},
-	{"BMA0255",	bma255},
-	{"BMA250E",	bma250e},
-	{"BMA222",	bma222},
-	{"BMA222E",	bma222e},
-	{"BMA0280",	bma280},
+	{"BSBA0150"},
+	{"BMC150A"},
+	{"BMI055A"},
+	{"BMA0255"},
+	{"BMA250E"},
+	{"BMA222"},
+	{"BMA222E"},
+	{"BMA0280"},
 	{"BOSC0200"},
 	{"DUAL250E"},
 	{ },
@@ -236,13 +236,13 @@ static const struct acpi_device_id bmc150_accel_acpi_match[] = {
 MODULE_DEVICE_TABLE(acpi, bmc150_accel_acpi_match);
 
 static const struct i2c_device_id bmc150_accel_id[] = {
-	{"bmc150_accel",	bmc150},
-	{"bmi055_accel",	bmi055},
-	{"bma255",		bma255},
-	{"bma250e",		bma250e},
-	{"bma222",		bma222},
-	{"bma222e",		bma222e},
-	{"bma280",		bma280},
+	{"bmc150_accel"},
+	{"bmi055_accel"},
+	{"bma255"},
+	{"bma250e"},
+	{"bma222"},
+	{"bma222e"},
+	{"bma280"},
 	{}
 };
 
diff --git a/drivers/iio/accel/bmc150-accel-spi.c b/drivers/iio/accel/bmc150-accel-spi.c
index 74a8aee4f612..01b42fa6a015 100644
--- a/drivers/iio/accel/bmc150-accel-spi.c
+++ b/drivers/iio/accel/bmc150-accel-spi.c
@@ -34,26 +34,26 @@ static int bmc150_accel_remove(struct spi_device *spi)
 }
 
 static const struct acpi_device_id bmc150_accel_acpi_match[] = {
-	{"BSBA0150",	bmc150},
-	{"BMC150A",	bmc150},
-	{"BMI055A",	bmi055},
-	{"BMA0255",	bma255},
-	{"BMA250E",	bma250e},
-	{"BMA222",	bma222},
-	{"BMA222E",	bma222e},
-	{"BMA0280",	bma280},
+	{"BSBA0150"},
+	{"BMC150A"},
+	{"BMI055A"},
+	{"BMA0255"},
+	{"BMA250E"},
+	{"BMA222"},
+	{"BMA222E"},
+	{"BMA0280"},
 	{ },
 };
 MODULE_DEVICE_TABLE(acpi, bmc150_accel_acpi_match);
 
 static const struct spi_device_id bmc150_accel_id[] = {
-	{"bmc150_accel",	bmc150},
-	{"bmi055_accel",	bmi055},
-	{"bma255",		bma255},
-	{"bma250e",		bma250e},
-	{"bma222",		bma222},
-	{"bma222e",		bma222e},
-	{"bma280",		bma280},
+	{"bmc150_accel"},
+	{"bmi055_accel"},
+	{"bma255"},
+	{"bma250e"},
+	{"bma222"},
+	{"bma222e"},
+	{"bma280"},
 	{}
 };
 MODULE_DEVICE_TABLE(spi, bmc150_accel_id);
diff --git a/drivers/iio/accel/bmc150-accel.h b/drivers/iio/accel/bmc150-accel.h
index d67d6ed6ae77..47121f070fe9 100644
--- a/drivers/iio/accel/bmc150-accel.h
+++ b/drivers/iio/accel/bmc150-accel.h
@@ -68,16 +68,6 @@ struct bmc150_accel_data {
 	struct iio_mount_matrix orientation;
 };
 
-enum {
-	bmc150,
-	bmi055,
-	bma255,
-	bma250e,
-	bma222,
-	bma222e,
-	bma280,
-};
-
 int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq,
 			    const char *name, bool block_supported);
 int bmc150_accel_core_remove(struct device *dev);
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 04/10] iio: accel: bmc150: Drop duplicated documentation of supported chips
  2021-06-11  8:08 [PATCH v3 00/10] iio: accel: bmc150: Add support for BMA253/BMA254 Stephan Gerhold
                   ` (2 preceding siblings ...)
  2021-06-11  8:08 ` [PATCH v3 03/10] iio: accel: bmc150: Drop misleading/duplicate chip identifiers Stephan Gerhold
@ 2021-06-11  8:08 ` Stephan Gerhold
  2021-06-11  8:08 ` [PATCH v3 05/10] iio: accel: bmc150: Sort all chip names alphabetically / by chip ID Stephan Gerhold
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Stephan Gerhold @ 2021-06-11  8:08 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Rob Herring, Linus Walleij, Peter Meerwald,
	linux-iio, devicetree, Bastien Nocera, Hans de Goede,
	Andy Shevchenko, ~postmarketos/upstreaming, Stephan Gerhold

The chips supported by the bmc150-accel driver are clearly documented
in Kconfig, in the bmc150_accel_chip_info_tbl as well as in all the
device ID tables in the I2C/SPI drivers. It's easy to forget to update
the lists in the file header. Drop those entirely to reduce the amount
of changes required to add new chip variants.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
Changes in v3: Add Andy's Reviewed-by

New patch in v2. Originally I tried to reorder those too but then it
caused conflicts in all my following patches so I'm not convinced
it's worth to try and keep those up to date.
---
 drivers/iio/accel/bmc150-accel-core.c | 10 +---------
 drivers/iio/accel/bmc150-accel-i2c.c  | 10 +---------
 2 files changed, 2 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index 9ecbd3769593..4b5caa89a3b4 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -1,14 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * 3-axis accelerometer driver supporting following Bosch-Sensortec chips:
- *  - BMC150
- *  - BMI055
- *  - BMA255
- *  - BMA250E
- *  - BMA222
- *  - BMA222E
- *  - BMA280
- *
+ * 3-axis accelerometer driver supporting many Bosch-Sensortec chips
  * Copyright (c) 2014, Intel Corporation.
  */
 
diff --git a/drivers/iio/accel/bmc150-accel-i2c.c b/drivers/iio/accel/bmc150-accel-i2c.c
index b8bda0dfb495..a0e2782580b7 100644
--- a/drivers/iio/accel/bmc150-accel-i2c.c
+++ b/drivers/iio/accel/bmc150-accel-i2c.c
@@ -1,14 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * 3-axis accelerometer driver supporting following I2C Bosch-Sensortec chips:
- *  - BMC150
- *  - BMI055
- *  - BMA255
- *  - BMA250E
- *  - BMA222
- *  - BMA222E
- *  - BMA280
- *
+ * 3-axis accelerometer driver supporting many I2C Bosch-Sensortec chips
  * Copyright (c) 2014, Intel Corporation.
  */
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 05/10] iio: accel: bmc150: Sort all chip names alphabetically / by chip ID
  2021-06-11  8:08 [PATCH v3 00/10] iio: accel: bmc150: Add support for BMA253/BMA254 Stephan Gerhold
                   ` (3 preceding siblings ...)
  2021-06-11  8:08 ` [PATCH v3 04/10] iio: accel: bmc150: Drop duplicated documentation of supported chips Stephan Gerhold
@ 2021-06-11  8:08 ` Stephan Gerhold
  2021-06-11  8:08 ` [PATCH v3 06/10] dt-bindings: iio: accel: bma255: Document bosch,bma253 Stephan Gerhold
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Stephan Gerhold @ 2021-06-11  8:08 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Rob Herring, Linus Walleij, Peter Meerwald,
	linux-iio, devicetree, Bastien Nocera, Hans de Goede,
	Andy Shevchenko, ~postmarketos/upstreaming, Stephan Gerhold

Right now all the device IDs are listed in seemingly random order,
make this consistent by ordering those alphabetically. Also, order
bmc150_accel_chip_info_tbl by chip ID for the same reason.

Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
Changes in v3: Move Kconfig changes into extra patch to clarify
               which chips are combos
New patch in v2.
---
 drivers/iio/accel/bmc150-accel-core.c | 40 +++++++++++++--------------
 drivers/iio/accel/bmc150-accel-i2c.c  | 26 ++++++++---------
 drivers/iio/accel/bmc150-accel-spi.c  | 18 ++++++------
 3 files changed, 42 insertions(+), 42 deletions(-)

diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index 4b5caa89a3b4..5a19676bcd2d 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -1089,26 +1089,6 @@ static const struct iio_chan_spec bma280_accel_channels[] =
 	BMC150_ACCEL_CHANNELS(14);
 
 static const struct bmc150_accel_chip_info bmc150_accel_chip_info_tbl[] = {
-	{
-		.name = "BMA255/BMC150/BMI055",
-		.chip_id = 0xFA,
-		.channels = bmc150_accel_channels,
-		.num_channels = ARRAY_SIZE(bmc150_accel_channels),
-		.scale_table = { {9610, BMC150_ACCEL_DEF_RANGE_2G},
-				 {19122, BMC150_ACCEL_DEF_RANGE_4G},
-				 {38344, BMC150_ACCEL_DEF_RANGE_8G},
-				 {76590, BMC150_ACCEL_DEF_RANGE_16G} },
-	},
-	{
-		.name = "BMA250E",
-		.chip_id = 0xF9,
-		.channels = bma250e_accel_channels,
-		.num_channels = ARRAY_SIZE(bma250e_accel_channels),
-		.scale_table = { {38344, BMC150_ACCEL_DEF_RANGE_2G},
-				 {76590, BMC150_ACCEL_DEF_RANGE_4G},
-				 {153277, BMC150_ACCEL_DEF_RANGE_8G},
-				 {306457, BMC150_ACCEL_DEF_RANGE_16G} },
-	},
 	{
 		.name = "BMA222",
 		.chip_id = 0x03,
@@ -1134,6 +1114,26 @@ static const struct bmc150_accel_chip_info bmc150_accel_chip_info_tbl[] = {
 				 {612915, BMC150_ACCEL_DEF_RANGE_8G},
 				 {1225831, BMC150_ACCEL_DEF_RANGE_16G} },
 	},
+	{
+		.name = "BMA250E",
+		.chip_id = 0xF9,
+		.channels = bma250e_accel_channels,
+		.num_channels = ARRAY_SIZE(bma250e_accel_channels),
+		.scale_table = { {38344, BMC150_ACCEL_DEF_RANGE_2G},
+				 {76590, BMC150_ACCEL_DEF_RANGE_4G},
+				 {153277, BMC150_ACCEL_DEF_RANGE_8G},
+				 {306457, BMC150_ACCEL_DEF_RANGE_16G} },
+	},
+	{
+		.name = "BMA255/BMC150/BMI055",
+		.chip_id = 0xFA,
+		.channels = bmc150_accel_channels,
+		.num_channels = ARRAY_SIZE(bmc150_accel_channels),
+		.scale_table = { {9610, BMC150_ACCEL_DEF_RANGE_2G},
+				 {19122, BMC150_ACCEL_DEF_RANGE_4G},
+				 {38344, BMC150_ACCEL_DEF_RANGE_8G},
+				 {76590, BMC150_ACCEL_DEF_RANGE_16G} },
+	},
 	{
 		.name = "BMA280",
 		.chip_id = 0xFB,
diff --git a/drivers/iio/accel/bmc150-accel-i2c.c b/drivers/iio/accel/bmc150-accel-i2c.c
index a0e2782580b7..7db436ddbdce 100644
--- a/drivers/iio/accel/bmc150-accel-i2c.c
+++ b/drivers/iio/accel/bmc150-accel-i2c.c
@@ -213,41 +213,41 @@ static int bmc150_accel_remove(struct i2c_client *client)
 }
 
 static const struct acpi_device_id bmc150_accel_acpi_match[] = {
-	{"BSBA0150"},
-	{"BMC150A"},
-	{"BMI055A"},
 	{"BMA0255"},
-	{"BMA250E"},
+	{"BMA0280"},
 	{"BMA222"},
 	{"BMA222E"},
-	{"BMA0280"},
+	{"BMA250E"},
+	{"BMC150A"},
+	{"BMI055A"},
 	{"BOSC0200"},
+	{"BSBA0150"},
 	{"DUAL250E"},
 	{ },
 };
 MODULE_DEVICE_TABLE(acpi, bmc150_accel_acpi_match);
 
 static const struct i2c_device_id bmc150_accel_id[] = {
-	{"bmc150_accel"},
-	{"bmi055_accel"},
-	{"bma255"},
-	{"bma250e"},
 	{"bma222"},
 	{"bma222e"},
+	{"bma250e"},
+	{"bma255"},
 	{"bma280"},
+	{"bmc150_accel"},
+	{"bmi055_accel"},
 	{}
 };
 
 MODULE_DEVICE_TABLE(i2c, bmc150_accel_id);
 
 static const struct of_device_id bmc150_accel_of_match[] = {
-	{ .compatible = "bosch,bmc150_accel" },
-	{ .compatible = "bosch,bmi055_accel" },
-	{ .compatible = "bosch,bma255" },
-	{ .compatible = "bosch,bma250e" },
 	{ .compatible = "bosch,bma222" },
 	{ .compatible = "bosch,bma222e" },
+	{ .compatible = "bosch,bma250e" },
+	{ .compatible = "bosch,bma255" },
 	{ .compatible = "bosch,bma280" },
+	{ .compatible = "bosch,bmc150_accel" },
+	{ .compatible = "bosch,bmi055_accel" },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, bmc150_accel_of_match);
diff --git a/drivers/iio/accel/bmc150-accel-spi.c b/drivers/iio/accel/bmc150-accel-spi.c
index 01b42fa6a015..dc884fa18ad0 100644
--- a/drivers/iio/accel/bmc150-accel-spi.c
+++ b/drivers/iio/accel/bmc150-accel-spi.c
@@ -34,26 +34,26 @@ static int bmc150_accel_remove(struct spi_device *spi)
 }
 
 static const struct acpi_device_id bmc150_accel_acpi_match[] = {
-	{"BSBA0150"},
-	{"BMC150A"},
-	{"BMI055A"},
 	{"BMA0255"},
-	{"BMA250E"},
+	{"BMA0280"},
 	{"BMA222"},
 	{"BMA222E"},
-	{"BMA0280"},
+	{"BMA250E"},
+	{"BMC150A"},
+	{"BMI055A"},
+	{"BSBA0150"},
 	{ },
 };
 MODULE_DEVICE_TABLE(acpi, bmc150_accel_acpi_match);
 
 static const struct spi_device_id bmc150_accel_id[] = {
-	{"bmc150_accel"},
-	{"bmi055_accel"},
-	{"bma255"},
-	{"bma250e"},
 	{"bma222"},
 	{"bma222e"},
+	{"bma250e"},
+	{"bma255"},
 	{"bma280"},
+	{"bmc150_accel"},
+	{"bmi055_accel"},
 	{}
 };
 MODULE_DEVICE_TABLE(spi, bmc150_accel_id);
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 06/10] dt-bindings: iio: accel: bma255: Document bosch,bma253
  2021-06-11  8:08 [PATCH v3 00/10] iio: accel: bmc150: Add support for BMA253/BMA254 Stephan Gerhold
                   ` (4 preceding siblings ...)
  2021-06-11  8:08 ` [PATCH v3 05/10] iio: accel: bmc150: Sort all chip names alphabetically / by chip ID Stephan Gerhold
@ 2021-06-11  8:08 ` Stephan Gerhold
  2021-06-11  8:09 ` [PATCH v3 07/10] iio: accel: bmc150: Add device IDs for BMA253 Stephan Gerhold
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Stephan Gerhold @ 2021-06-11  8:08 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Rob Herring, Linus Walleij, Peter Meerwald,
	linux-iio, devicetree, Bastien Nocera, Hans de Goede,
	Andy Shevchenko, ~postmarketos/upstreaming, Stephan Gerhold

BMA253 is mostly like BMA255 that is already supported by the
bmc150-accel driver. Document an extra bosch,bma253 compatible for it.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
index c2efbb813ca2..8afb0fe8ef5c 100644
--- a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
+++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
@@ -18,6 +18,7 @@ properties:
     enum:
       - bosch,bmc150_accel
       - bosch,bmi055_accel
+      - bosch,bma253
       - bosch,bma255
       - bosch,bma250e
       - bosch,bma222
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 07/10] iio: accel: bmc150: Add device IDs for BMA253
  2021-06-11  8:08 [PATCH v3 00/10] iio: accel: bmc150: Add support for BMA253/BMA254 Stephan Gerhold
                   ` (5 preceding siblings ...)
  2021-06-11  8:08 ` [PATCH v3 06/10] dt-bindings: iio: accel: bma255: Document bosch,bma253 Stephan Gerhold
@ 2021-06-11  8:09 ` Stephan Gerhold
  2021-06-11  8:09 ` [PATCH v3 08/10] dt-bindings: iio: bma255: Allow multiple interrupts Stephan Gerhold
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Stephan Gerhold @ 2021-06-11  8:09 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Rob Herring, Linus Walleij, Peter Meerwald,
	linux-iio, devicetree, Bastien Nocera, Hans de Goede,
	Andy Shevchenko, ~postmarketos/upstreaming, Stephan Gerhold

BMA253 is mostly like BMA255 and has exactly the same register layout
as used by the bmc150-accel driver as far I can tell. Making it work
is as simple as adding new device IDs for it since it has the same
chip_id = 0xFA (250) as BMA255 and others.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 drivers/iio/accel/Kconfig             | 2 +-
 drivers/iio/accel/bmc150-accel-core.c | 2 +-
 drivers/iio/accel/bmc150-accel-i2c.c  | 2 ++
 drivers/iio/accel/bmc150-accel-spi.c  | 1 +
 4 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index cbca6ab7da88..f8dc5403397d 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -143,7 +143,7 @@ config BMC150_ACCEL
 	select BMC150_ACCEL_SPI if SPI
 	help
 	  Say yes here to build support for the following Bosch accelerometers:
-	  BMA222, BMA222E, BMA250E, BMA255, BMA280, BMC150, BMI055.
+	  BMA222, BMA222E, BMA250E, BMA253, BMA255, BMA280, BMC150, BMI055.
 
 	  Note that some of these are combo modules:
 	    - BMC150: accelerometer and magnetometer
diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index 5a19676bcd2d..3faa3de0dd4d 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -1125,7 +1125,7 @@ static const struct bmc150_accel_chip_info bmc150_accel_chip_info_tbl[] = {
 				 {306457, BMC150_ACCEL_DEF_RANGE_16G} },
 	},
 	{
-		.name = "BMA255/BMC150/BMI055",
+		.name = "BMA253/BMA255/BMC150/BMI055",
 		.chip_id = 0xFA,
 		.channels = bmc150_accel_channels,
 		.num_channels = ARRAY_SIZE(bmc150_accel_channels),
diff --git a/drivers/iio/accel/bmc150-accel-i2c.c b/drivers/iio/accel/bmc150-accel-i2c.c
index 7db436ddbdce..32ed07354a9a 100644
--- a/drivers/iio/accel/bmc150-accel-i2c.c
+++ b/drivers/iio/accel/bmc150-accel-i2c.c
@@ -231,6 +231,7 @@ static const struct i2c_device_id bmc150_accel_id[] = {
 	{"bma222"},
 	{"bma222e"},
 	{"bma250e"},
+	{"bma253"},
 	{"bma255"},
 	{"bma280"},
 	{"bmc150_accel"},
@@ -244,6 +245,7 @@ static const struct of_device_id bmc150_accel_of_match[] = {
 	{ .compatible = "bosch,bma222" },
 	{ .compatible = "bosch,bma222e" },
 	{ .compatible = "bosch,bma250e" },
+	{ .compatible = "bosch,bma253" },
 	{ .compatible = "bosch,bma255" },
 	{ .compatible = "bosch,bma280" },
 	{ .compatible = "bosch,bmc150_accel" },
diff --git a/drivers/iio/accel/bmc150-accel-spi.c b/drivers/iio/accel/bmc150-accel-spi.c
index dc884fa18ad0..54b8c9c8068b 100644
--- a/drivers/iio/accel/bmc150-accel-spi.c
+++ b/drivers/iio/accel/bmc150-accel-spi.c
@@ -50,6 +50,7 @@ static const struct spi_device_id bmc150_accel_id[] = {
 	{"bma222"},
 	{"bma222e"},
 	{"bma250e"},
+	{"bma253"},
 	{"bma255"},
 	{"bma280"},
 	{"bmc150_accel"},
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 08/10] dt-bindings: iio: bma255: Allow multiple interrupts
  2021-06-11  8:08 [PATCH v3 00/10] iio: accel: bmc150: Add support for BMA253/BMA254 Stephan Gerhold
                   ` (6 preceding siblings ...)
  2021-06-11  8:09 ` [PATCH v3 07/10] iio: accel: bmc150: Add device IDs for BMA253 Stephan Gerhold
@ 2021-06-11  8:09 ` Stephan Gerhold
  2021-06-11 17:59   ` Jonathan Cameron
  2021-06-11  8:09 ` [PATCH v3 09/10] dt-bindings: iio: accel: bma180/bma255: Move bma254 to bma255 schema Stephan Gerhold
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 16+ messages in thread
From: Stephan Gerhold @ 2021-06-11  8:09 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Rob Herring, Linus Walleij, Peter Meerwald,
	linux-iio, devicetree, Bastien Nocera, Hans de Goede,
	Andy Shevchenko, ~postmarketos/upstreaming, Stephan Gerhold

BMA253 has two interrupt pins (INT1 and INT2) that can be configured
independently. At the moment the bmc150-accel driver does not make use
of them but it might be able to in the future, so it's useful to already
specify all available interrupts in the device tree.

Set maxItems: 2 for interrupts to allow specifying a second one.
This is necessary as preparation to move the bosch,bma254 compatible
from bosch,bma180.yaml to bosch,bma255.yaml since bma180 allows two
interrupts, but BMA254 is better supported by the bmc150-accel driver.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 .../devicetree/bindings/iio/accel/bosch,bma255.yaml        | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
index 8afb0fe8ef5c..65b299a5619b 100644
--- a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
+++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
@@ -32,7 +32,12 @@ properties:
   vddio-supply: true
 
   interrupts:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
+    description: |
+      The first interrupt listed must be the one connected to the INT1 pin,
+      the second (optional) interrupt listed must be the one connected to the
+      INT2 pin (if available).
 
   mount-matrix:
     description: an optional 3x3 mounting rotation matrix.
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 09/10] dt-bindings: iio: accel: bma180/bma255: Move bma254 to bma255 schema
  2021-06-11  8:08 [PATCH v3 00/10] iio: accel: bmc150: Add support for BMA253/BMA254 Stephan Gerhold
                   ` (7 preceding siblings ...)
  2021-06-11  8:09 ` [PATCH v3 08/10] dt-bindings: iio: bma255: Allow multiple interrupts Stephan Gerhold
@ 2021-06-11  8:09 ` Stephan Gerhold
  2021-06-11  8:09 ` [PATCH v3 10/10] iio: accel: bma180/bmc150: Move BMA254 to bmc150-accel driver Stephan Gerhold
  2021-06-11 17:35 ` [PATCH v3 00/10] iio: accel: bmc150: Add support for BMA253/BMA254 Jonathan Cameron
  10 siblings, 0 replies; 16+ messages in thread
From: Stephan Gerhold @ 2021-06-11  8:09 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Rob Herring, Linus Walleij, Peter Meerwald,
	linux-iio, devicetree, Bastien Nocera, Hans de Goede,
	Andy Shevchenko, ~postmarketos/upstreaming, Stephan Gerhold

BMA254 is very similar to BMA253/BMA255 which are both supported by
the bmc150-accel driver. In general, there is quite some overlap between
the bma180 and bmc150-accel driver, but the bmc150-accel driver has a few
more features (e.g. motion trigger/interrupt).

Let's move bma254 over to the bma255 schema (bmc150-accel driver).

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 Documentation/devicetree/bindings/iio/accel/bosch,bma180.yaml | 3 +--
 Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma180.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma180.yaml
index 45b3abde298f..a7e84089cc3d 100644
--- a/Documentation/devicetree/bindings/iio/accel/bosch,bma180.yaml
+++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma180.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/iio/accel/bosch,bma180.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Bosch BMA023 / BMA150/ BMA180 / BMA25x / SMB380 triaxial accelerometers
+title: Bosch BMA023 / BMA150/ BMA180 / BMA250 / SMB380 triaxial accelerometers
 
 maintainers:
   - Jonathan Cameron <jic23@kernel.org>
@@ -21,7 +21,6 @@ properties:
       - bosch,bma150
       - bosch,bma180
       - bosch,bma250
-      - bosch,bma254
       - bosch,smb380
 
   reg:
diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
index 65b299a5619b..e830d5295b92 100644
--- a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
+++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
@@ -19,6 +19,7 @@ properties:
       - bosch,bmc150_accel
       - bosch,bmi055_accel
       - bosch,bma253
+      - bosch,bma254
       - bosch,bma255
       - bosch,bma250e
       - bosch,bma222
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 10/10] iio: accel: bma180/bmc150: Move BMA254 to bmc150-accel driver
  2021-06-11  8:08 [PATCH v3 00/10] iio: accel: bmc150: Add support for BMA253/BMA254 Stephan Gerhold
                   ` (8 preceding siblings ...)
  2021-06-11  8:09 ` [PATCH v3 09/10] dt-bindings: iio: accel: bma180/bma255: Move bma254 to bma255 schema Stephan Gerhold
@ 2021-06-11  8:09 ` Stephan Gerhold
  2021-06-11 17:35 ` [PATCH v3 00/10] iio: accel: bmc150: Add support for BMA253/BMA254 Jonathan Cameron
  10 siblings, 0 replies; 16+ messages in thread
From: Stephan Gerhold @ 2021-06-11  8:09 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Rob Herring, Linus Walleij, Peter Meerwald,
	linux-iio, devicetree, Bastien Nocera, Hans de Goede,
	Andy Shevchenko, ~postmarketos/upstreaming, Stephan Gerhold

Commit c1d1c4a62db5 ("iio: accel: bma180: BMA254 support") added
BMA254 support to the bma180 driver and changed some naming to BMA25x
to make it easier to add support for BMA253 and BMA255.

Unfortunately, there is quite some overlap between the bma180 driver
and the bmc150-accel driver. Back when the commit was made, the
bmc150-accel driver actually already had support for BMA255, and
adding support for BMA254 would have been as simple as adding a new
compatible to bmc150-accel.

The bmc150-accel driver is a bit better for BMA254 since it also
supports the motion trigger/interrupt functionality. Fortunately,
moving BMA254 support over to bmc150-accel is fairly simple because
the drivers have compatible device tree bindings.

Revert most of the changes for BMA254 support in bma180 and move
BMA254 over to bmc150-accel. This has the following advantages:

  - Support for motion trigger/interrupt
  - Fix incorrect scale values (BMA254 currently uses the same as
    BMA250 but actually they're different because of 10 vs 12 bits
    data size)
  - Less code than before :)

BMA250 could be potentially also moved but it's more complicated
because its chip_id conflicts with the one for BMA222 in bmc150-accel.
Perhaps there are also other register differences, I did not investigate
further yet (and I have no way to test it).

Cc: Peter Meerwald <pmeerw@pmeerw.net>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
Changes in v3: Sort chip list in BMA180 Kconfig text
Changes in v2: Also drop BMA254 comment from file header in bma180.c

Note that the similarity between bma180 and bmc150-accel has already
caused a lot of confusion in the past. See e.g. [1] where Hans
accidentally added BMA250E support to bma180 even though that was
already supported in bmc150-accel.

Having BMA250 in bma180 and BMA250E in bmc150-accel was also discussed
before in [2] but for some reason back then the conclusion was that
they are different enough to justify that.

Also note that this patch only applies cleanly on top of the
"iio: accel: bma180: Fix BMA25x bandwidth register values" patch [3]
that I sent earlier, which is not present in togreg/testing
at the moment (only in fixes-togreg).

[1]: https://lore.kernel.org/linux-iio/20170606203538.15250-1-hdegoede@redhat.com/
[2]: https://lore.kernel.org/linux-iio/alpine.DEB.2.01.1408281111250.3259@pmeerw.net/
[3]: https://lore.kernel.org/linux-iio/20210526094408.34298-2-stephan@gerhold.net/
---
 drivers/iio/accel/Kconfig             |  6 +-
 drivers/iio/accel/bma180.c            | 92 ++++-----------------------
 drivers/iio/accel/bmc150-accel-core.c |  2 +-
 drivers/iio/accel/bmc150-accel-i2c.c  |  2 +
 4 files changed, 18 insertions(+), 84 deletions(-)

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index f8dc5403397d..0e56ace61103 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -89,13 +89,13 @@ config ADXL372_I2C
 	  module will be called adxl372_i2c.
 
 config BMA180
-	tristate "Bosch BMA023/BMA1x0/BMA25x 3-Axis Accelerometer Driver"
+	tristate "Bosch BMA023/BMA1x0/BMA250 3-Axis Accelerometer Driver"
 	depends on I2C && INPUT_BMA150=n
 	select IIO_BUFFER
 	select IIO_TRIGGERED_BUFFER
 	help
 	  Say Y here if you want to build a driver for the Bosch BMA023, BMA150
-	  BMA180, SMB380, or BMA25x triaxial acceleration sensor.
+	  BMA180, BMA250 or SMB380 triaxial acceleration sensor.
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called bma180.
@@ -143,7 +143,7 @@ config BMC150_ACCEL
 	select BMC150_ACCEL_SPI if SPI
 	help
 	  Say yes here to build support for the following Bosch accelerometers:
-	  BMA222, BMA222E, BMA250E, BMA253, BMA255, BMA280, BMC150, BMI055.
+	  BMA222, BMA222E, BMA250E, BMA253, BMA254, BMA255, BMA280, BMC150, BMI055.
 
 	  Note that some of these are combo modules:
 	    - BMC150: accelerometer and magnetometer
diff --git a/drivers/iio/accel/bma180.c b/drivers/iio/accel/bma180.c
index e7c6b3096cb7..2edfcb4819b7 100644
--- a/drivers/iio/accel/bma180.c
+++ b/drivers/iio/accel/bma180.c
@@ -10,7 +10,6 @@
  * BMA023/BMA150/SMB380: 7-bit I2C slave address 0x38
  * BMA180: 7-bit I2C slave address 0x40 or 0x41
  * BMA250: 7-bit I2C slave address 0x18 or 0x19
- * BMA254: 7-bit I2C slave address 0x18 or 0x19
  */
 
 #include <linux/module.h>
@@ -38,7 +37,6 @@ enum chip_ids {
 	BMA150,
 	BMA180,
 	BMA250,
-	BMA254,
 };
 
 struct bma180_data;
@@ -59,7 +57,6 @@ struct bma180_part_info {
 	u8 scale_reg, scale_mask;
 	u8 power_reg, power_mask, lowpower_val;
 	u8 int_enable_reg, int_enable_mask;
-	u8 int_map_reg, int_enable_dataready_int1_mask;
 	u8 softreset_reg, softreset_val;
 
 	int (*chip_config)(struct bma180_data *data);
@@ -112,7 +109,6 @@ struct bma180_part_info {
 #define BMA023_ID_REG_VAL	0x02
 #define BMA180_ID_REG_VAL	0x03
 #define BMA250_ID_REG_VAL	0x03
-#define BMA254_ID_REG_VAL	0xfa /* 250 decimal */
 
 /* Chip power modes */
 #define BMA180_LOW_POWER	0x03
@@ -134,24 +130,6 @@ struct bma180_part_info {
 #define BMA250_INT1_DATA_MASK	BIT(0)
 #define BMA250_INT_RESET_MASK	BIT(7) /* Reset pending interrupts */
 
-#define BMA254_RANGE_REG	0x0f
-#define BMA254_BW_REG		0x10
-#define BMA254_POWER_REG	0x11
-#define BMA254_RESET_REG	0x14
-#define BMA254_INT_ENABLE_REG	0x17
-#define BMA254_INT_MAP_REG	0x1a
-#define BMA254_INT_RESET_REG	0x21
-
-#define BMA254_RANGE_MASK	GENMASK(3, 0) /* Range of accel values */
-#define BMA254_BW_MASK		GENMASK(4, 0) /* Accel bandwidth */
-#define BMA254_BW_OFFSET	8
-#define BMA254_SUSPEND_MASK	BIT(7) /* chip will sleep */
-#define BMA254_LOWPOWER_MASK	BIT(6)
-#define BMA254_DATA_INTEN_MASK	BIT(4)
-#define BMA254_INT2_DATA_MASK	BIT(7)
-#define BMA254_INT1_DATA_MASK	BIT(0)
-#define BMA254_INT_RESET_MASK	BIT(7) /* Reset pending interrupts */
-
 struct bma180_data {
 	struct regulator *vdd_supply;
 	struct regulator *vddio_supply;
@@ -184,8 +162,8 @@ static int bma023_scale_table[] = { 2452, 4903, 9709, };
 static int bma180_bw_table[] = { 10, 20, 40, 75, 150, 300 }; /* Hz */
 static int bma180_scale_table[] = { 1275, 1863, 2452, 3727, 4903, 9709, 19417 };
 
-static int bma25x_bw_table[] = { 8, 16, 31, 63, 125, 250, 500, 1000 }; /* Hz */
-static int bma25x_scale_table[] = { 0, 0, 0, 38344, 0, 76590, 0, 0, 153180, 0,
+static int bma250_bw_table[] = { 8, 16, 31, 63, 125, 250, 500, 1000 }; /* Hz */
+static int bma250_scale_table[] = { 0, 0, 0, 38344, 0, 76590, 0, 0, 153180, 0,
 	0, 0, 306458 };
 
 static int bma180_get_data_reg(struct bma180_data *data, enum bma180_chan chan)
@@ -432,7 +410,7 @@ static int bma180_chip_config(struct bma180_data *data)
 	return ret;
 }
 
-static int bma25x_chip_config(struct bma180_data *data)
+static int bma250_chip_config(struct bma180_data *data)
 {
 	int ret = bma180_chip_init(data);
 
@@ -451,8 +429,7 @@ static int bma25x_chip_config(struct bma180_data *data)
 	 * This enables dataready interrupt on the INT1 pin
 	 * FIXME: support using the INT2 pin
 	 */
-	ret = bma180_set_bits(data, data->part_info->int_map_reg,
-		data->part_info->int_enable_dataready_int1_mask, 1);
+	ret = bma180_set_bits(data, BMA250_INT_MAP_REG, BMA250_INT1_DATA_MASK, 1);
 	if (ret)
 		goto err;
 
@@ -489,7 +466,7 @@ static void bma180_chip_disable(struct bma180_data *data)
 	dev_err(&data->client->dev, "failed to disable the chip\n");
 }
 
-static void bma25x_chip_disable(struct bma180_data *data)
+static void bma250_chip_disable(struct bma180_data *data)
 {
 	if (bma180_set_new_data_intr_state(data, false))
 		goto err;
@@ -775,14 +752,6 @@ static const struct iio_chan_spec bma250_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(4),
 };
 
-static const struct iio_chan_spec bma254_channels[] = {
-	BMA180_ACC_CHANNEL(X, 12),
-	BMA180_ACC_CHANNEL(Y, 12),
-	BMA180_ACC_CHANNEL(Z, 12),
-	BMA180_TEMP_CHANNEL,
-	IIO_CHAN_SOFT_TIMESTAMP(4),
-};
-
 static const struct bma180_part_info bma180_part_info[] = {
 	[BMA023] = {
 		.chip_id = BMA023_ID_REG_VAL,
@@ -872,10 +841,10 @@ static const struct bma180_part_info bma180_part_info[] = {
 		.chip_id = BMA250_ID_REG_VAL,
 		.channels = bma250_channels,
 		.num_channels = ARRAY_SIZE(bma250_channels),
-		.scale_table = bma25x_scale_table,
-		.num_scales = ARRAY_SIZE(bma25x_scale_table),
-		.bw_table = bma25x_bw_table,
-		.num_bw = ARRAY_SIZE(bma25x_bw_table),
+		.scale_table = bma250_scale_table,
+		.num_scales = ARRAY_SIZE(bma250_scale_table),
+		.bw_table = bma250_bw_table,
+		.num_bw = ARRAY_SIZE(bma250_bw_table),
 		.temp_offset = 48, /* 0 LSB @ 24 degree C */
 		.int_reset_reg = BMA250_INT_RESET_REG,
 		.int_reset_mask = BMA250_INT_RESET_MASK,
@@ -891,42 +860,10 @@ static const struct bma180_part_info bma180_part_info[] = {
 		.lowpower_val = 1,
 		.int_enable_reg = BMA250_INT_ENABLE_REG,
 		.int_enable_mask = BMA250_DATA_INTEN_MASK,
-		.int_map_reg = BMA250_INT_MAP_REG,
-		.int_enable_dataready_int1_mask = BMA250_INT1_DATA_MASK,
 		.softreset_reg = BMA250_RESET_REG,
 		.softreset_val = BMA180_RESET_VAL,
-		.chip_config = bma25x_chip_config,
-		.chip_disable = bma25x_chip_disable,
-	},
-	[BMA254] = {
-		.chip_id = BMA254_ID_REG_VAL,
-		.channels = bma254_channels,
-		.num_channels = ARRAY_SIZE(bma254_channels),
-		.scale_table = bma25x_scale_table,
-		.num_scales = ARRAY_SIZE(bma25x_scale_table),
-		.bw_table = bma25x_bw_table,
-		.num_bw = ARRAY_SIZE(bma25x_bw_table),
-		.temp_offset = 46, /* 0 LSB @ 23 degree C */
-		.int_reset_reg = BMA254_INT_RESET_REG,
-		.int_reset_mask = BMA254_INT_RESET_MASK,
-		.sleep_reg = BMA254_POWER_REG,
-		.sleep_mask = BMA254_SUSPEND_MASK,
-		.bw_reg = BMA254_BW_REG,
-		.bw_mask = BMA254_BW_MASK,
-		.bw_offset = BMA254_BW_OFFSET,
-		.scale_reg = BMA254_RANGE_REG,
-		.scale_mask = BMA254_RANGE_MASK,
-		.power_reg = BMA254_POWER_REG,
-		.power_mask = BMA254_LOWPOWER_MASK,
-		.lowpower_val = 1,
-		.int_enable_reg = BMA254_INT_ENABLE_REG,
-		.int_enable_mask = BMA254_DATA_INTEN_MASK,
-		.int_map_reg = BMA254_INT_MAP_REG,
-		.int_enable_dataready_int1_mask = BMA254_INT1_DATA_MASK,
-		.softreset_reg = BMA254_RESET_REG,
-		.softreset_val = BMA180_RESET_VAL,
-		.chip_config = bma25x_chip_config,
-		.chip_disable = bma25x_chip_disable,
+		.chip_config = bma250_chip_config,
+		.chip_disable = bma250_chip_disable,
 	},
 };
 
@@ -1166,7 +1103,6 @@ static const struct i2c_device_id bma180_ids[] = {
 	{ "bma150", BMA150 },
 	{ "bma180", BMA180 },
 	{ "bma250", BMA250 },
-	{ "bma254", BMA254 },
 	{ "smb380", BMA150 },
 	{ }
 };
@@ -1190,10 +1126,6 @@ static const struct of_device_id bma180_of_match[] = {
 		.compatible = "bosch,bma250",
 		.data = (void *)BMA250
 	},
-	{
-		.compatible = "bosch,bma254",
-		.data = (void *)BMA254
-	},
 	{
 		.compatible = "bosch,smb380",
 		.data = (void *)BMA150
@@ -1217,5 +1149,5 @@ module_i2c_driver(bma180_driver);
 
 MODULE_AUTHOR("Kravchenko Oleksandr <x0199363@ti.com>");
 MODULE_AUTHOR("Texas Instruments, Inc.");
-MODULE_DESCRIPTION("Bosch BMA023/BMA1x0/BMA25x triaxial acceleration sensor");
+MODULE_DESCRIPTION("Bosch BMA023/BMA1x0/BMA250 triaxial acceleration sensor");
 MODULE_LICENSE("GPL");
diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index 3faa3de0dd4d..43aecd4bf3a4 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -1125,7 +1125,7 @@ static const struct bmc150_accel_chip_info bmc150_accel_chip_info_tbl[] = {
 				 {306457, BMC150_ACCEL_DEF_RANGE_16G} },
 	},
 	{
-		.name = "BMA253/BMA255/BMC150/BMI055",
+		.name = "BMA253/BMA254/BMA255/BMC150/BMI055",
 		.chip_id = 0xFA,
 		.channels = bmc150_accel_channels,
 		.num_channels = ARRAY_SIZE(bmc150_accel_channels),
diff --git a/drivers/iio/accel/bmc150-accel-i2c.c b/drivers/iio/accel/bmc150-accel-i2c.c
index 32ed07354a9a..999495f0669d 100644
--- a/drivers/iio/accel/bmc150-accel-i2c.c
+++ b/drivers/iio/accel/bmc150-accel-i2c.c
@@ -232,6 +232,7 @@ static const struct i2c_device_id bmc150_accel_id[] = {
 	{"bma222e"},
 	{"bma250e"},
 	{"bma253"},
+	{"bma254"},
 	{"bma255"},
 	{"bma280"},
 	{"bmc150_accel"},
@@ -246,6 +247,7 @@ static const struct of_device_id bmc150_accel_of_match[] = {
 	{ .compatible = "bosch,bma222e" },
 	{ .compatible = "bosch,bma250e" },
 	{ .compatible = "bosch,bma253" },
+	{ .compatible = "bosch,bma254" },
 	{ .compatible = "bosch,bma255" },
 	{ .compatible = "bosch,bma280" },
 	{ .compatible = "bosch,bmc150_accel" },
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 00/10] iio: accel: bmc150: Add support for BMA253/BMA254
  2021-06-11  8:08 [PATCH v3 00/10] iio: accel: bmc150: Add support for BMA253/BMA254 Stephan Gerhold
                   ` (9 preceding siblings ...)
  2021-06-11  8:09 ` [PATCH v3 10/10] iio: accel: bma180/bmc150: Move BMA254 to bmc150-accel driver Stephan Gerhold
@ 2021-06-11 17:35 ` Jonathan Cameron
  10 siblings, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2021-06-11 17:35 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: Lars-Peter Clausen, Rob Herring, Linus Walleij, Peter Meerwald,
	linux-iio, devicetree, Bastien Nocera, Hans de Goede,
	Andy Shevchenko, ~postmarketos/upstreaming

On Fri, 11 Jun 2021 10:08:53 +0200
Stephan Gerhold <stephan@gerhold.net> wrote:

> The Bosch BMA253 accelerometer is very similar to both BMA254 and BMA255.
> The current situation is very confusing: BMA254 is supported by the bma180
> driver, but BMA255 is supported by the bmc150-accel driver.
> 
> It turns out the bma180 and bmc150-accel drivers have quite some overlap,
> and BMA253/BMA254 would be a bit better supported in bmc150-accel
> (which has support for the motion trigger/interrupt).
> 
> This series adds BMA253 support to bmc150-accel and also moves BMA254
> over to bmc150, removing some unnecessary code from the bma180 driver.
> 
> In v2 I also sneaked in a small fix for the scale table of BMA222
> to simplify backporting for the stable people.
After cheating a bit and pulling the dependency across from my fixes-togreg
branch (along with the other patches that were there), I can now apply this.

So applied to the fixes-togreg branch of iio.git and pushed out as testing
for 0-day to have a quick poke at it before I let this going into linux-next.

Thanks,

Jonathan

> 
> ---
> Changes in v3:
>   - Add new "iio: accel: bmc150: Clarify combo modules in Kconfig" patch
>   - Sort "one-line" chip name lists as well, not just multi-line ones
> 
> v2: https://lore.kernel.org/linux-iio/20210610122126.50504-1-stephan@gerhold.net/
> Changes in v2:
>   - Add new "iio: accel: bmc150: Fix bma222 scale unit" patch at the
>     beginning so the stable people can backport it without conflicts
>   - Add Reviewed-by: from Hans and Andy for all previous patches
>   - Add patch 3 and 4 to have all the chip lists in a consistent order
>   - Fix last patch to also drop BMA254 from the file header in bma180.c
> 
> v1: https://lore.kernel.org/linux-iio/20210610095300.3613-1-stephan@gerhold.net/
> 
> Stephan Gerhold (10):
>   iio: accel: bmc150: Fix bma222 scale unit
>   iio: accel: bmc150: Clarify combo modules in Kconfig
>   iio: accel: bmc150: Drop misleading/duplicate chip identifiers
>   iio: accel: bmc150: Drop duplicated documentation of supported chips
>   iio: accel: bmc150: Sort all chip names alphabetically / by chip ID
>   dt-bindings: iio: accel: bma255: Document bosch,bma253
>   iio: accel: bmc150: Add device IDs for BMA253
>   dt-bindings: iio: bma255: Allow multiple interrupts
>   dt-bindings: iio: accel: bma180/bma255: Move bma254 to bma255 schema
>   iio: accel: bma180/bmc150: Move BMA254 to bmc150-accel driver
> 
>  .../bindings/iio/accel/bosch,bma180.yaml      |  3 +-
>  .../bindings/iio/accel/bosch,bma255.yaml      |  9 +-
>  drivers/iio/accel/Kconfig                     | 11 ++-
>  drivers/iio/accel/bma180.c                    | 92 +++----------------
>  drivers/iio/accel/bmc150-accel-core.c         | 87 ++++++------------
>  drivers/iio/accel/bmc150-accel-i2c.c          | 52 +++++------
>  drivers/iio/accel/bmc150-accel-spi.c          | 31 ++++---
>  drivers/iio/accel/bmc150-accel.h              | 10 --
>  8 files changed, 98 insertions(+), 197 deletions(-)
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 08/10] dt-bindings: iio: bma255: Allow multiple interrupts
  2021-06-11  8:09 ` [PATCH v3 08/10] dt-bindings: iio: bma255: Allow multiple interrupts Stephan Gerhold
@ 2021-06-11 17:59   ` Jonathan Cameron
  2021-06-11 18:21     ` Stephan Gerhold
  0 siblings, 1 reply; 16+ messages in thread
From: Jonathan Cameron @ 2021-06-11 17:59 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: Lars-Peter Clausen, Rob Herring, Linus Walleij, Peter Meerwald,
	linux-iio, devicetree, Bastien Nocera, Hans de Goede,
	Andy Shevchenko, ~postmarketos/upstreaming

On Fri, 11 Jun 2021 10:09:01 +0200
Stephan Gerhold <stephan@gerhold.net> wrote:

> BMA253 has two interrupt pins (INT1 and INT2) that can be configured
> independently. At the moment the bmc150-accel driver does not make use
> of them but it might be able to in the future, so it's useful to already
> specify all available interrupts in the device tree.
> 
> Set maxItems: 2 for interrupts to allow specifying a second one.
> This is necessary as preparation to move the bosch,bma254 compatible
> from bosch,bma180.yaml to bosch,bma255.yaml since bma180 allows two
> interrupts, but BMA254 is better supported by the bmc150-accel driver.
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
>  .../devicetree/bindings/iio/accel/bosch,bma255.yaml        | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> index 8afb0fe8ef5c..65b299a5619b 100644
> --- a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> +++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> @@ -32,7 +32,12 @@ properties:
>    vddio-supply: true
>  
>    interrupts:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
> +    description: |
> +      The first interrupt listed must be the one connected to the INT1 pin,
> +      the second (optional) interrupt listed must be the one connected to the
> +      INT2 pin (if available).

As this is a direct copy from the bma180 binding and we are moving devices
from one to the other, we need to support this as the default.
Longer term, from the bma253 datasheet, it look looks the two pins are equally
capable so if we get a board where only the int2 pin is connected then we will
need to use interrupt-names to distinguish the two (as we do in other drivers).

Another thing to note is that we don't have to have separate binding docs just
because we have separate drivers. At somepoint we might want to consider just
fusing the two docs.
 
Anyhow, work for another day!

Jonathan


>  
>    mount-matrix:
>      description: an optional 3x3 mounting rotation matrix.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 08/10] dt-bindings: iio: bma255: Allow multiple interrupts
  2021-06-11 17:59   ` Jonathan Cameron
@ 2021-06-11 18:21     ` Stephan Gerhold
  2021-06-11 18:26       ` Jonathan Cameron
  0 siblings, 1 reply; 16+ messages in thread
From: Stephan Gerhold @ 2021-06-11 18:21 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Rob Herring, Linus Walleij, Peter Meerwald,
	linux-iio, devicetree, Bastien Nocera, Hans de Goede,
	Andy Shevchenko, ~postmarketos/upstreaming

On Fri, Jun 11, 2021 at 06:59:41PM +0100, Jonathan Cameron wrote:
> On Fri, 11 Jun 2021 10:09:01 +0200
> Stephan Gerhold <stephan@gerhold.net> wrote:
> 
> > BMA253 has two interrupt pins (INT1 and INT2) that can be configured
> > independently. At the moment the bmc150-accel driver does not make use
> > of them but it might be able to in the future, so it's useful to already
> > specify all available interrupts in the device tree.
> > 
> > Set maxItems: 2 for interrupts to allow specifying a second one.
> > This is necessary as preparation to move the bosch,bma254 compatible
> > from bosch,bma180.yaml to bosch,bma255.yaml since bma180 allows two
> > interrupts, but BMA254 is better supported by the bmc150-accel driver.
> > 
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > ---
> >  .../devicetree/bindings/iio/accel/bosch,bma255.yaml        | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> > index 8afb0fe8ef5c..65b299a5619b 100644
> > --- a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> > +++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> > @@ -32,7 +32,12 @@ properties:
> >    vddio-supply: true
> >  
> >    interrupts:
> > -    maxItems: 1
> > +    minItems: 1
> > +    maxItems: 2
> > +    description: |
> > +      The first interrupt listed must be the one connected to the INT1 pin,
> > +      the second (optional) interrupt listed must be the one connected to the
> > +      INT2 pin (if available).
> 
> As this is a direct copy from the bma180 binding and we are moving devices
> from one to the other, we need to support this as the default.
> Longer term, from the bma253 datasheet, it look looks the two pins are equally
> capable so if we get a board where only the int2 pin is connected then we will
> need to use interrupt-names to distinguish the two (as we do in other drivers).
> 

This kind of sounds like a strange board layout in general. But what's
worse is that for some reason even Bosch thought this is a "good" idea
so they released the BMC156 [1]. It works just like the BMC150 but has
only a single interrupt pin. One would expect that would be INT1,
but nope, it's INT2 of course. :-)

I have a device with BMC156 where this is the case, so I guess I need to
make bmc150-accel use INT2 somehow (without specifying INT1). It might
be easiest if we treat this the same way as the case that you mentioned,
i.e. everyone with BMC156 would specify the interrupt-names explicitly
to have a consistent meaning of the device tree.

[1]: https://www.mouser.de/datasheet/2/783/BST-BMC156-DS000-1509546.pdf

> Another thing to note is that we don't have to have separate binding docs just
> because we have separate drivers. At somepoint we might want to consider just
> fusing the two docs.
>  

Good point! I might do that together with the BMC156 changes. :)

Thanks!
Stephan

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 08/10] dt-bindings: iio: bma255: Allow multiple interrupts
  2021-06-11 18:21     ` Stephan Gerhold
@ 2021-06-11 18:26       ` Jonathan Cameron
  2021-06-11 19:21         ` Stephan Gerhold
  0 siblings, 1 reply; 16+ messages in thread
From: Jonathan Cameron @ 2021-06-11 18:26 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: Lars-Peter Clausen, Rob Herring, Linus Walleij, Peter Meerwald,
	linux-iio, devicetree, Bastien Nocera, Hans de Goede,
	Andy Shevchenko, ~postmarketos/upstreaming

On Fri, 11 Jun 2021 20:21:05 +0200
Stephan Gerhold <stephan@gerhold.net> wrote:

> On Fri, Jun 11, 2021 at 06:59:41PM +0100, Jonathan Cameron wrote:
> > On Fri, 11 Jun 2021 10:09:01 +0200
> > Stephan Gerhold <stephan@gerhold.net> wrote:
> >   
> > > BMA253 has two interrupt pins (INT1 and INT2) that can be configured
> > > independently. At the moment the bmc150-accel driver does not make use
> > > of them but it might be able to in the future, so it's useful to already
> > > specify all available interrupts in the device tree.
> > > 
> > > Set maxItems: 2 for interrupts to allow specifying a second one.
> > > This is necessary as preparation to move the bosch,bma254 compatible
> > > from bosch,bma180.yaml to bosch,bma255.yaml since bma180 allows two
> > > interrupts, but BMA254 is better supported by the bmc150-accel driver.
> > > 
> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > > Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > > ---
> > >  .../devicetree/bindings/iio/accel/bosch,bma255.yaml        | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> > > index 8afb0fe8ef5c..65b299a5619b 100644
> > > --- a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> > > +++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> > > @@ -32,7 +32,12 @@ properties:
> > >    vddio-supply: true
> > >  
> > >    interrupts:
> > > -    maxItems: 1
> > > +    minItems: 1
> > > +    maxItems: 2
> > > +    description: |
> > > +      The first interrupt listed must be the one connected to the INT1 pin,
> > > +      the second (optional) interrupt listed must be the one connected to the
> > > +      INT2 pin (if available).  
> > 
> > As this is a direct copy from the bma180 binding and we are moving devices
> > from one to the other, we need to support this as the default.
> > Longer term, from the bma253 datasheet, it look looks the two pins are equally
> > capable so if we get a board where only the int2 pin is connected then we will
> > need to use interrupt-names to distinguish the two (as we do in other drivers).
> >   
> 
> This kind of sounds like a strange board layout in general. But what's
> worse is that for some reason even Bosch thought this is a "good" idea
> so they released the BMC156 [1]. It works just like the BMC150 but has
> only a single interrupt pin. One would expect that would be INT1,
> but nope, it's INT2 of course. :-)
> 
> I have a device with BMC156 where this is the case, so I guess I need to
> make bmc150-accel use INT2 somehow (without specifying INT1). It might
> be easiest if we treat this the same way as the case that you mentioned,
> i.e. everyone with BMC156 would specify the interrupt-names explicitly
> to have a consistent meaning of the device tree.

That will really confuse people who think they just have one interrupt so
why do they need the name!  You'll have to special case that delight in
the driver.

> 
> [1]: https://www.mouser.de/datasheet/2/783/BST-BMC156-DS000-1509546.pdf
> 
> > Another thing to note is that we don't have to have separate binding docs just
> > because we have separate drivers. At somepoint we might want to consider just
> > fusing the two docs.
> >    
> 
> Good point! I might do that together with the BMC156 changes. :)

Great.  Best of all you can drop me as maintainer the combined binding ;)

Jonathan

> 
> Thanks!
> Stephan


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 08/10] dt-bindings: iio: bma255: Allow multiple interrupts
  2021-06-11 18:26       ` Jonathan Cameron
@ 2021-06-11 19:21         ` Stephan Gerhold
  0 siblings, 0 replies; 16+ messages in thread
From: Stephan Gerhold @ 2021-06-11 19:21 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Rob Herring, Linus Walleij, Peter Meerwald,
	linux-iio, devicetree, Bastien Nocera, Hans de Goede,
	Andy Shevchenko, ~postmarketos/upstreaming

On Fri, Jun 11, 2021 at 07:26:39PM +0100, Jonathan Cameron wrote:
> On Fri, 11 Jun 2021 20:21:05 +0200
> Stephan Gerhold <stephan@gerhold.net> wrote:
> 
> > On Fri, Jun 11, 2021 at 06:59:41PM +0100, Jonathan Cameron wrote:
> > > On Fri, 11 Jun 2021 10:09:01 +0200
> > > Stephan Gerhold <stephan@gerhold.net> wrote:
> > >   
> > > > BMA253 has two interrupt pins (INT1 and INT2) that can be configured
> > > > independently. At the moment the bmc150-accel driver does not make use
> > > > of them but it might be able to in the future, so it's useful to already
> > > > specify all available interrupts in the device tree.
> > > > 
> > > > Set maxItems: 2 for interrupts to allow specifying a second one.
> > > > This is necessary as preparation to move the bosch,bma254 compatible
> > > > from bosch,bma180.yaml to bosch,bma255.yaml since bma180 allows two
> > > > interrupts, but BMA254 is better supported by the bmc150-accel driver.
> > > > 
> > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > > > Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > > > ---
> > > >  .../devicetree/bindings/iio/accel/bosch,bma255.yaml        | 7 ++++++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> > > > index 8afb0fe8ef5c..65b299a5619b 100644
> > > > --- a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> > > > +++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
> > > > @@ -32,7 +32,12 @@ properties:
> > > >    vddio-supply: true
> > > >  
> > > >    interrupts:
> > > > -    maxItems: 1
> > > > +    minItems: 1
> > > > +    maxItems: 2
> > > > +    description: |
> > > > +      The first interrupt listed must be the one connected to the INT1 pin,
> > > > +      the second (optional) interrupt listed must be the one connected to the
> > > > +      INT2 pin (if available).  
> > > 
> > > As this is a direct copy from the bma180 binding and we are moving devices
> > > from one to the other, we need to support this as the default.
> > > Longer term, from the bma253 datasheet, it look looks the two pins are equally
> > > capable so if we get a board where only the int2 pin is connected then we will
> > > need to use interrupt-names to distinguish the two (as we do in other drivers).
> > >   
> > 
> > This kind of sounds like a strange board layout in general. But what's
> > worse is that for some reason even Bosch thought this is a "good" idea
> > so they released the BMC156 [1]. It works just like the BMC150 but has
> > only a single interrupt pin. One would expect that would be INT1,
> > but nope, it's INT2 of course. :-)
> > 
> > I have a device with BMC156 where this is the case, so I guess I need to
> > make bmc150-accel use INT2 somehow (without specifying INT1). It might
> > be easiest if we treat this the same way as the case that you mentioned,
> > i.e. everyone with BMC156 would specify the interrupt-names explicitly
> > to have a consistent meaning of the device tree.
> 
> That will really confuse people who think they just have one interrupt so
> why do they need the name!  You'll have to special case that delight in
> the driver.
> 

Okay, good that we discussed this before then. :)
I was unsure if it would be better to special case it for BMC156 *or*
add support for the interrupt-names, but perhaps I'm just going to do
*both* then. (Special case it for BMC156, look at interrupt-names for
all others.)

Thanks!
Stephan

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-06-11 19:21 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-11  8:08 [PATCH v3 00/10] iio: accel: bmc150: Add support for BMA253/BMA254 Stephan Gerhold
2021-06-11  8:08 ` [PATCH v3 01/10] iio: accel: bmc150: Fix bma222 scale unit Stephan Gerhold
2021-06-11  8:08 ` [PATCH v3 02/10] iio: accel: bmc150: Clarify combo modules in Kconfig Stephan Gerhold
2021-06-11  8:08 ` [PATCH v3 03/10] iio: accel: bmc150: Drop misleading/duplicate chip identifiers Stephan Gerhold
2021-06-11  8:08 ` [PATCH v3 04/10] iio: accel: bmc150: Drop duplicated documentation of supported chips Stephan Gerhold
2021-06-11  8:08 ` [PATCH v3 05/10] iio: accel: bmc150: Sort all chip names alphabetically / by chip ID Stephan Gerhold
2021-06-11  8:08 ` [PATCH v3 06/10] dt-bindings: iio: accel: bma255: Document bosch,bma253 Stephan Gerhold
2021-06-11  8:09 ` [PATCH v3 07/10] iio: accel: bmc150: Add device IDs for BMA253 Stephan Gerhold
2021-06-11  8:09 ` [PATCH v3 08/10] dt-bindings: iio: bma255: Allow multiple interrupts Stephan Gerhold
2021-06-11 17:59   ` Jonathan Cameron
2021-06-11 18:21     ` Stephan Gerhold
2021-06-11 18:26       ` Jonathan Cameron
2021-06-11 19:21         ` Stephan Gerhold
2021-06-11  8:09 ` [PATCH v3 09/10] dt-bindings: iio: accel: bma180/bma255: Move bma254 to bma255 schema Stephan Gerhold
2021-06-11  8:09 ` [PATCH v3 10/10] iio: accel: bma180/bmc150: Move BMA254 to bmc150-accel driver Stephan Gerhold
2021-06-11 17:35 ` [PATCH v3 00/10] iio: accel: bmc150: Add support for BMA253/BMA254 Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.