All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] iio: accel: bmc150: Add support for INT2 and BMC156
@ 2021-08-02 15:56 Stephan Gerhold
  2021-08-02 15:56 ` [PATCH v2 1/4] dt-bindings: iio: accel: bma255: Add interrupt-names Stephan Gerhold
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Stephan Gerhold @ 2021-08-02 15:56 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Rob Herring, Linus Walleij, linux-iio,
	devicetree, Hans de Goede, Andy Shevchenko,
	~postmarketos/upstreaming, Nikita Travkin, Stephan Gerhold

This series makes it possible to set up interrupts with the BMC150 driver
on boards where only the INT2 pin is connected (and not INT1). This is
particularly always the case for BMC156 since for some reason it only
has the INT2 pin and not the INT1 pin.

These changes were already partially discussed here:
https://lore.kernel.org/linux-iio/YMOphuXSoODIVX06@gerhold.net/

Changes in v2:
  - PATCH 1/4: Clarify order of "interrupts" with "interrupt-names"
  - PATCH 4/4: Wrap a long line, clarify BOSCH_UNKNOWN with a comment

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

Stephan Gerhold (4):
  dt-bindings: iio: accel: bma255: Add interrupt-names
  dt-bindings: iio: accel: bma255: Add bosch,bmc156_accel
  iio: accel: bmc150: Make it possible to configure INT2 instead of INT1
  iio: accel: bmc150: Add support for BMC156

 .../bindings/iio/accel/bosch,bma255.yaml      | 34 +++++++-
 drivers/iio/accel/Kconfig                     |  5 +-
 drivers/iio/accel/bmc150-accel-core.c         | 78 +++++++++++++++----
 drivers/iio/accel/bmc150-accel-i2c.c          | 10 ++-
 drivers/iio/accel/bmc150-accel-spi.c          | 10 ++-
 drivers/iio/accel/bmc150-accel.h              | 20 ++++-
 6 files changed, 134 insertions(+), 23 deletions(-)

-- 
2.32.0


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

* [PATCH v2 1/4] dt-bindings: iio: accel: bma255: Add interrupt-names
  2021-08-02 15:56 [PATCH v2 0/4] iio: accel: bmc150: Add support for INT2 and BMC156 Stephan Gerhold
@ 2021-08-02 15:56 ` Stephan Gerhold
  2021-08-06 21:49   ` Rob Herring
  2021-08-02 15:56 ` [PATCH v2 2/4] dt-bindings: iio: accel: bma255: Add bosch,bmc156_accel Stephan Gerhold
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Stephan Gerhold @ 2021-08-02 15:56 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Rob Herring, Linus Walleij, linux-iio,
	devicetree, Hans de Goede, Andy Shevchenko,
	~postmarketos/upstreaming, Nikita Travkin, Stephan Gerhold

The binding already allows specifying both interrupt pins, but there
is currently no way to describe a board where (for whatever reason)
only INT2 is connected. Make it possible to use "interrupt-names"
to make it explicit which interrupt pin is meant in the interrupts.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
Changes in v2:
  - Add "Without interrupt-names, ..." to "interrupts" description
    to clarify that ordering of interrupts can be relaxed if
    interrupt-names is used.
---
 .../bindings/iio/accel/bosch,bma255.yaml         | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
index 5b35856b1942..253b2051d0b1 100644
--- a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
+++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
@@ -45,9 +45,18 @@ properties:
     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). The type should be IRQ_TYPE_EDGE_RISING.
+      Without interrupt-names, 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). The type should be
+      IRQ_TYPE_EDGE_RISING.
+
+  interrupt-names:
+    minItems: 1
+    maxItems: 2
+    items:
+      enum:
+        - INT1
+        - INT2
 
   mount-matrix:
     description: an optional 3x3 mounting rotation matrix.
@@ -73,6 +82,7 @@ examples:
             vddio-supply = <&vddio>;
             vdd-supply = <&vdd>;
             interrupts = <57 IRQ_TYPE_EDGE_RISING>;
+            interrupt-names = "INT1";
         };
     };
   - |
-- 
2.32.0


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

* [PATCH v2 2/4] dt-bindings: iio: accel: bma255: Add bosch,bmc156_accel
  2021-08-02 15:56 [PATCH v2 0/4] iio: accel: bmc150: Add support for INT2 and BMC156 Stephan Gerhold
  2021-08-02 15:56 ` [PATCH v2 1/4] dt-bindings: iio: accel: bma255: Add interrupt-names Stephan Gerhold
@ 2021-08-02 15:56 ` Stephan Gerhold
  2021-08-02 15:56 ` [PATCH v2 3/4] iio: accel: bmc150: Make it possible to configure INT2 instead of INT1 Stephan Gerhold
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Stephan Gerhold @ 2021-08-02 15:56 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Rob Herring, Linus Walleij, linux-iio,
	devicetree, Hans de Goede, Andy Shevchenko,
	~postmarketos/upstreaming, Nikita Travkin, Stephan Gerhold,
	Rob Herring

BMC156 is very smilar to BMC150, but it has only one accelerometer
interrupt pin. It would make sense if only INT1 was exposed but someone
at Bosch decided to only have an INT2 pin.

In this case, it does not make sense if the first interrupt pin is
treated as INT1 (since that pin does not exist). Add a note to the
bindings that the first interrupt pin is treated as INT2 for BMC156.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 .../bindings/iio/accel/bosch,bma255.yaml       | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
index 253b2051d0b1..478e75ae0885 100644
--- a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
+++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
@@ -26,6 +26,7 @@ properties:
       - bosch,bma255
       - bosch,bma280
       - bosch,bmc150_accel
+      - bosch,bmc156_accel
       - bosch,bmi055_accel
 
       # bma180 driver in Linux
@@ -50,6 +51,9 @@ properties:
       the one connected to the INT2 pin (if available). The type should be
       IRQ_TYPE_EDGE_RISING.
 
+      BMC156 does not have an INT1 pin, therefore the first interrupt pin is
+      always treated as INT2.
+
   interrupt-names:
     minItems: 1
     maxItems: 2
@@ -85,6 +89,20 @@ examples:
             interrupt-names = "INT1";
         };
     };
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        accelerometer@10 {
+            compatible = "bosch,bmc156_accel";
+            reg = <0x10>;
+            vddio-supply = <&vddio>;
+            vdd-supply = <&vdd>;
+            interrupts = <116 IRQ_TYPE_EDGE_RISING>;
+            interrupt-names = "INT2";
+        };
+    };
   - |
     # include <dt-bindings/interrupt-controller/irq.h>
     spi {
-- 
2.32.0


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

* [PATCH v2 3/4] iio: accel: bmc150: Make it possible to configure INT2 instead of INT1
  2021-08-02 15:56 [PATCH v2 0/4] iio: accel: bmc150: Add support for INT2 and BMC156 Stephan Gerhold
  2021-08-02 15:56 ` [PATCH v2 1/4] dt-bindings: iio: accel: bma255: Add interrupt-names Stephan Gerhold
  2021-08-02 15:56 ` [PATCH v2 2/4] dt-bindings: iio: accel: bma255: Add bosch,bmc156_accel Stephan Gerhold
@ 2021-08-02 15:56 ` Stephan Gerhold
  2021-08-03 23:34   ` Linus Walleij
  2021-08-02 15:56 ` [PATCH v2 4/4] iio: accel: bmc150: Add support for BMC156 Stephan Gerhold
  2021-08-08 15:51 ` [PATCH v2 0/4] iio: accel: bmc150: Add support for INT2 and BMC156 Jonathan Cameron
  4 siblings, 1 reply; 8+ messages in thread
From: Stephan Gerhold @ 2021-08-02 15:56 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Rob Herring, Linus Walleij, linux-iio,
	devicetree, Hans de Goede, Andy Shevchenko,
	~postmarketos/upstreaming, Nikita Travkin, Stephan Gerhold

Some Bosch accelerometers have two interrupt pins (INT1 and INT2).
At the moment, the driver uses only the first one, which is fine for
most situations. However, some boards might only have INT2 connected
for some reason.

Add the necessary bits and configuration to set up INT2. Then try
to detect this situation at least for device tree setups by checking
if the first interrupt (the one picked by the I2C/SPI core) is actually
named "INT2" using the interrupt-names property.

of_irq_get_byname() returns either 0 or some error code in case
the driver probed without device tree, so in all other cases we fall
back to configuring INT1 as before.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 drivers/iio/accel/bmc150-accel-core.c | 71 ++++++++++++++++++++++-----
 1 file changed, 59 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index 5ce384ebe6c7..8d3dd3c2bcc2 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -10,6 +10,7 @@
 #include <linux/delay.h>
 #include <linux/slab.h>
 #include <linux/acpi.h>
+#include <linux/of_irq.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/iio/iio.h>
@@ -57,12 +58,18 @@
 #define BMC150_ACCEL_RESET_VAL			0xB6
 
 #define BMC150_ACCEL_REG_INT_MAP_0		0x19
-#define BMC150_ACCEL_INT_MAP_0_BIT_SLOPE	BIT(2)
+#define BMC150_ACCEL_INT_MAP_0_BIT_INT1_SLOPE	BIT(2)
 
 #define BMC150_ACCEL_REG_INT_MAP_1		0x1A
-#define BMC150_ACCEL_INT_MAP_1_BIT_DATA		BIT(0)
-#define BMC150_ACCEL_INT_MAP_1_BIT_FWM		BIT(1)
-#define BMC150_ACCEL_INT_MAP_1_BIT_FFULL	BIT(2)
+#define BMC150_ACCEL_INT_MAP_1_BIT_INT1_DATA	BIT(0)
+#define BMC150_ACCEL_INT_MAP_1_BIT_INT1_FWM	BIT(1)
+#define BMC150_ACCEL_INT_MAP_1_BIT_INT1_FFULL	BIT(2)
+#define BMC150_ACCEL_INT_MAP_1_BIT_INT2_FFULL	BIT(5)
+#define BMC150_ACCEL_INT_MAP_1_BIT_INT2_FWM	BIT(6)
+#define BMC150_ACCEL_INT_MAP_1_BIT_INT2_DATA	BIT(7)
+
+#define BMC150_ACCEL_REG_INT_MAP_2		0x1B
+#define BMC150_ACCEL_INT_MAP_2_BIT_INT2_SLOPE	BIT(2)
 
 #define BMC150_ACCEL_REG_INT_RST_LATCH		0x21
 #define BMC150_ACCEL_INT_MODE_LATCH_RESET	0x80
@@ -81,6 +88,7 @@
 
 #define BMC150_ACCEL_REG_INT_OUT_CTRL		0x20
 #define BMC150_ACCEL_INT_OUT_CTRL_INT1_LVL	BIT(0)
+#define BMC150_ACCEL_INT_OUT_CTRL_INT2_LVL	BIT(2)
 
 #define BMC150_ACCEL_REG_INT_5			0x27
 #define BMC150_ACCEL_SLOPE_DUR_MASK		0x03
@@ -476,21 +484,24 @@ static bool bmc150_apply_acpi_orientation(struct device *dev,
 }
 #endif
 
-static const struct bmc150_accel_interrupt_info {
+struct bmc150_accel_interrupt_info {
 	u8 map_reg;
 	u8 map_bitmask;
 	u8 en_reg;
 	u8 en_bitmask;
-} bmc150_accel_interrupts[BMC150_ACCEL_INTERRUPTS] = {
+};
+
+static const struct bmc150_accel_interrupt_info
+bmc150_accel_interrupts_int1[BMC150_ACCEL_INTERRUPTS] = {
 	{ /* data ready interrupt */
 		.map_reg = BMC150_ACCEL_REG_INT_MAP_1,
-		.map_bitmask = BMC150_ACCEL_INT_MAP_1_BIT_DATA,
+		.map_bitmask = BMC150_ACCEL_INT_MAP_1_BIT_INT1_DATA,
 		.en_reg = BMC150_ACCEL_REG_INT_EN_1,
 		.en_bitmask = BMC150_ACCEL_INT_EN_BIT_DATA_EN,
 	},
 	{  /* motion interrupt */
 		.map_reg = BMC150_ACCEL_REG_INT_MAP_0,
-		.map_bitmask = BMC150_ACCEL_INT_MAP_0_BIT_SLOPE,
+		.map_bitmask = BMC150_ACCEL_INT_MAP_0_BIT_INT1_SLOPE,
 		.en_reg = BMC150_ACCEL_REG_INT_EN_0,
 		.en_bitmask =  BMC150_ACCEL_INT_EN_BIT_SLP_X |
 			BMC150_ACCEL_INT_EN_BIT_SLP_Y |
@@ -498,19 +509,55 @@ static const struct bmc150_accel_interrupt_info {
 	},
 	{ /* fifo watermark interrupt */
 		.map_reg = BMC150_ACCEL_REG_INT_MAP_1,
-		.map_bitmask = BMC150_ACCEL_INT_MAP_1_BIT_FWM,
+		.map_bitmask = BMC150_ACCEL_INT_MAP_1_BIT_INT1_FWM,
+		.en_reg = BMC150_ACCEL_REG_INT_EN_1,
+		.en_bitmask = BMC150_ACCEL_INT_EN_BIT_FWM_EN,
+	},
+};
+
+static const struct bmc150_accel_interrupt_info
+bmc150_accel_interrupts_int2[BMC150_ACCEL_INTERRUPTS] = {
+	{ /* data ready interrupt */
+		.map_reg = BMC150_ACCEL_REG_INT_MAP_1,
+		.map_bitmask = BMC150_ACCEL_INT_MAP_1_BIT_INT2_DATA,
+		.en_reg = BMC150_ACCEL_REG_INT_EN_1,
+		.en_bitmask = BMC150_ACCEL_INT_EN_BIT_DATA_EN,
+	},
+	{  /* motion interrupt */
+		.map_reg = BMC150_ACCEL_REG_INT_MAP_2,
+		.map_bitmask = BMC150_ACCEL_INT_MAP_2_BIT_INT2_SLOPE,
+		.en_reg = BMC150_ACCEL_REG_INT_EN_0,
+		.en_bitmask =  BMC150_ACCEL_INT_EN_BIT_SLP_X |
+			BMC150_ACCEL_INT_EN_BIT_SLP_Y |
+			BMC150_ACCEL_INT_EN_BIT_SLP_Z
+	},
+	{ /* fifo watermark interrupt */
+		.map_reg = BMC150_ACCEL_REG_INT_MAP_1,
+		.map_bitmask = BMC150_ACCEL_INT_MAP_1_BIT_INT2_FWM,
 		.en_reg = BMC150_ACCEL_REG_INT_EN_1,
 		.en_bitmask = BMC150_ACCEL_INT_EN_BIT_FWM_EN,
 	},
 };
 
 static void bmc150_accel_interrupts_setup(struct iio_dev *indio_dev,
-					  struct bmc150_accel_data *data)
+					  struct bmc150_accel_data *data, int irq)
 {
+	const struct bmc150_accel_interrupt_info *irq_info = NULL;
+	struct device *dev = regmap_get_device(data->regmap);
 	int i;
 
+	/*
+	 * For now we map all interrupts to the same output pin.
+	 * However, some boards may have just INT2 (and not INT1) connected,
+	 * so we try to detect which IRQ it is based on the interrupt-names.
+	 * Without interrupt-names, we assume the irq belongs to INT1.
+	 */
+	irq_info = bmc150_accel_interrupts_int1;
+	if (irq == of_irq_get_byname(dev->of_node, "INT2"))
+		irq_info = bmc150_accel_interrupts_int2;
+
 	for (i = 0; i < BMC150_ACCEL_INTERRUPTS; i++)
-		data->interrupts[i].info = &bmc150_accel_interrupts[i];
+		data->interrupts[i].info = &irq_info[i];
 }
 
 static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data, int i,
@@ -1714,7 +1761,7 @@ int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq,
 			goto err_buffer_cleanup;
 		}
 
-		bmc150_accel_interrupts_setup(indio_dev, data);
+		bmc150_accel_interrupts_setup(indio_dev, data, irq);
 
 		ret = bmc150_accel_triggers_setup(indio_dev, data);
 		if (ret)
-- 
2.32.0


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

* [PATCH v2 4/4] iio: accel: bmc150: Add support for BMC156
  2021-08-02 15:56 [PATCH v2 0/4] iio: accel: bmc150: Add support for INT2 and BMC156 Stephan Gerhold
                   ` (2 preceding siblings ...)
  2021-08-02 15:56 ` [PATCH v2 3/4] iio: accel: bmc150: Make it possible to configure INT2 instead of INT1 Stephan Gerhold
@ 2021-08-02 15:56 ` Stephan Gerhold
  2021-08-08 15:51 ` [PATCH v2 0/4] iio: accel: bmc150: Add support for INT2 and BMC156 Jonathan Cameron
  4 siblings, 0 replies; 8+ messages in thread
From: Stephan Gerhold @ 2021-08-02 15:56 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Rob Herring, Linus Walleij, linux-iio,
	devicetree, Hans de Goede, Andy Shevchenko,
	~postmarketos/upstreaming, Nikita Travkin, Stephan Gerhold

BMC156 is another accelerometer that works just fine with the bmc150-accel
driver. It's very similar to BMC150 (also a accelerometer + magnetometer
combo) but with only one accelerometer interrupt pin. It would make sense
if only INT1 was exposed but someone at Bosch decided to only have an
INT2 pin.

Try to deal with this by making use of the INT2 support introduced
in the previous commit and force using INT2 for BMC156. To detect
that we need to bring up a simplified version of the previous type IDs.

Note that unlike the type IDs removed in commit c06a6aba6835
("iio: accel: bmc150: Drop misleading/duplicate chip identifiers")
here I only add one for the special case of BMC156. Everything else
still happens by reading the CHIP_ID register since the chip type
information often is not accurate in ACPI tables.

Tested-by: Nikita Travkin <nikita@trvn.ru> # BMC156
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
Changes in v2:
  - Wrap line in bmc150_accel_interrupts_setup() for better readability
  - Clarify reason for BOSCH_UNKNOWN with a comment
---
 drivers/iio/accel/Kconfig             |  5 +++--
 drivers/iio/accel/bmc150-accel-core.c |  9 ++++++---
 drivers/iio/accel/bmc150-accel-i2c.c  | 10 ++++++++--
 drivers/iio/accel/bmc150-accel-spi.c  | 10 +++++++++-
 drivers/iio/accel/bmc150-accel.h      | 20 +++++++++++++++++++-
 5 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 0e56ace61103..2f0c0d512ae7 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -143,10 +143,11 @@ 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, BMA254, BMA255, BMA280, BMC150, BMI055.
+	  BMA222, BMA222E, BMA250E, BMA253, BMA254, BMA255, BMA280, BMC150, BMC156
+	  BMI055.
 
 	  Note that some of these are combo modules:
-	    - BMC150: accelerometer and magnetometer
+	    - BMC150/BMC156: accelerometer and magnetometer
 	    - BMI055: accelerometer and gyroscope
 
 	  This driver is only implementing accelerometer part, which has
diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index 8d3dd3c2bcc2..e8693a42ad46 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -553,7 +553,8 @@ static void bmc150_accel_interrupts_setup(struct iio_dev *indio_dev,
 	 * Without interrupt-names, we assume the irq belongs to INT1.
 	 */
 	irq_info = bmc150_accel_interrupts_int1;
-	if (irq == of_irq_get_byname(dev->of_node, "INT2"))
+	if (data->type == BOSCH_BMC156 ||
+	    irq == of_irq_get_byname(dev->of_node, "INT2"))
 		irq_info = bmc150_accel_interrupts_int2;
 
 	for (i = 0; i < BMC150_ACCEL_INTERRUPTS; i++)
@@ -1174,7 +1175,7 @@ static const struct bmc150_accel_chip_info bmc150_accel_chip_info_tbl[] = {
 				 {306458, BMC150_ACCEL_DEF_RANGE_16G} },
 	},
 	{
-		.name = "BMA253/BMA254/BMA255/BMC150/BMI055",
+		.name = "BMA253/BMA254/BMA255/BMC150/BMC156/BMI055",
 		.chip_id = 0xFA,
 		.channels = bmc150_accel_channels,
 		.num_channels = ARRAY_SIZE(bmc150_accel_channels),
@@ -1661,7 +1662,8 @@ static int bmc150_accel_chip_init(struct bmc150_accel_data *data)
 }
 
 int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq,
-			    const char *name, bool block_supported)
+			    enum bmc150_type type, const char *name,
+			    bool block_supported)
 {
 	const struct attribute **fifo_attrs;
 	struct bmc150_accel_data *data;
@@ -1676,6 +1678,7 @@ int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq,
 	dev_set_drvdata(dev, indio_dev);
 
 	data->regmap = regmap;
+	data->type = type;
 
 	if (!bmc150_apply_acpi_orientation(dev, &data->orientation)) {
 		ret = iio_read_mount_matrix(dev, &data->orientation);
diff --git a/drivers/iio/accel/bmc150-accel-i2c.c b/drivers/iio/accel/bmc150-accel-i2c.c
index 999495f0669d..88bd8a25f142 100644
--- a/drivers/iio/accel/bmc150-accel-i2c.c
+++ b/drivers/iio/accel/bmc150-accel-i2c.c
@@ -176,6 +176,7 @@ static int bmc150_accel_probe(struct i2c_client *client,
 {
 	struct regmap *regmap;
 	const char *name = NULL;
+	enum bmc150_type type = BOSCH_UNKNOWN;
 	bool block_supported =
 		i2c_check_functionality(client->adapter, I2C_FUNC_I2C) ||
 		i2c_check_functionality(client->adapter,
@@ -188,10 +189,13 @@ static int bmc150_accel_probe(struct i2c_client *client,
 		return PTR_ERR(regmap);
 	}
 
-	if (id)
+	if (id) {
 		name = id->name;
+		type = id->driver_data;
+	}
 
-	ret = bmc150_accel_core_probe(&client->dev, regmap, client->irq, name, block_supported);
+	ret = bmc150_accel_core_probe(&client->dev, regmap, client->irq,
+				      type, name, block_supported);
 	if (ret)
 		return ret;
 
@@ -236,6 +240,7 @@ static const struct i2c_device_id bmc150_accel_id[] = {
 	{"bma255"},
 	{"bma280"},
 	{"bmc150_accel"},
+	{"bmc156_accel", BOSCH_BMC156},
 	{"bmi055_accel"},
 	{}
 };
@@ -251,6 +256,7 @@ static const struct of_device_id bmc150_accel_of_match[] = {
 	{ .compatible = "bosch,bma255" },
 	{ .compatible = "bosch,bma280" },
 	{ .compatible = "bosch,bmc150_accel" },
+	{ .compatible = "bosch,bmc156_accel" },
 	{ .compatible = "bosch,bmi055_accel" },
 	{ },
 };
diff --git a/drivers/iio/accel/bmc150-accel-spi.c b/drivers/iio/accel/bmc150-accel-spi.c
index 54b8c9c8068b..191e312dc91a 100644
--- a/drivers/iio/accel/bmc150-accel-spi.c
+++ b/drivers/iio/accel/bmc150-accel-spi.c
@@ -16,6 +16,8 @@
 static int bmc150_accel_probe(struct spi_device *spi)
 {
 	struct regmap *regmap;
+	const char *name = NULL;
+	enum bmc150_type type = BOSCH_UNKNOWN;
 	const struct spi_device_id *id = spi_get_device_id(spi);
 
 	regmap = devm_regmap_init_spi(spi, &bmc150_regmap_conf);
@@ -24,7 +26,12 @@ static int bmc150_accel_probe(struct spi_device *spi)
 		return PTR_ERR(regmap);
 	}
 
-	return bmc150_accel_core_probe(&spi->dev, regmap, spi->irq, id->name,
+	if (id) {
+		name = id->name;
+		type = id->driver_data;
+	}
+
+	return bmc150_accel_core_probe(&spi->dev, regmap, spi->irq, type, name,
 				       true);
 }
 
@@ -54,6 +61,7 @@ static const struct spi_device_id bmc150_accel_id[] = {
 	{"bma255"},
 	{"bma280"},
 	{"bmc150_accel"},
+	{"bmc156_accel", BOSCH_BMC156},
 	{"bmi055_accel"},
 	{}
 };
diff --git a/drivers/iio/accel/bmc150-accel.h b/drivers/iio/accel/bmc150-accel.h
index 47121f070fe9..1bb5023e8ed9 100644
--- a/drivers/iio/accel/bmc150-accel.h
+++ b/drivers/iio/accel/bmc150-accel.h
@@ -13,6 +13,22 @@ struct i2c_client;
 struct bmc150_accel_chip_info;
 struct bmc150_accel_interrupt_info;
 
+/*
+ * We can often guess better than "UNKNOWN" based on the device IDs
+ * but unfortunately this information is not always accurate. There are some
+ * devices where ACPI firmware specifies an ID like "BMA250E" when the device
+ * actually has a BMA222E. The driver attempts to detect those by reading the
+ * chip ID from the registers but this information is not always enough either.
+ *
+ * Therefore, this enum should be only used when the chip ID detection is not
+ * enough and we can be reasonably sure that the device IDs are reliable
+ * in practice (e.g. for device tree platforms).
+ */
+enum bmc150_type {
+	BOSCH_UNKNOWN,
+	BOSCH_BMC156,
+};
+
 struct bmc150_accel_interrupt {
 	const struct bmc150_accel_interrupt_info *info;
 	atomic_t users;
@@ -62,6 +78,7 @@ struct bmc150_accel_data {
 	int ev_enable_state;
 	int64_t timestamp, old_timestamp; /* Only used in hw fifo mode. */
 	const struct bmc150_accel_chip_info *chip_info;
+	enum bmc150_type type;
 	struct i2c_client *second_device;
 	void (*resume_callback)(struct device *dev);
 	struct delayed_work resume_work;
@@ -69,7 +86,8 @@ struct bmc150_accel_data {
 };
 
 int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq,
-			    const char *name, bool block_supported);
+			    enum bmc150_type type, const char *name,
+			    bool block_supported);
 int bmc150_accel_core_remove(struct device *dev);
 extern const struct dev_pm_ops bmc150_accel_pm_ops;
 extern const struct regmap_config bmc150_regmap_conf;
-- 
2.32.0


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

* Re: [PATCH v2 3/4] iio: accel: bmc150: Make it possible to configure INT2 instead of INT1
  2021-08-02 15:56 ` [PATCH v2 3/4] iio: accel: bmc150: Make it possible to configure INT2 instead of INT1 Stephan Gerhold
@ 2021-08-03 23:34   ` Linus Walleij
  0 siblings, 0 replies; 8+ messages in thread
From: Linus Walleij @ 2021-08-03 23:34 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring, linux-iio,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Hans de Goede, Andy Shevchenko,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
	<devicetree@vger.kernel.org>,
	Hans de Goede <hdegoede@redhat.com>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,,
	Nikita Travkin

On Mon, Aug 2, 2021 at 5:57 PM Stephan Gerhold <stephan@gerhold.net> wrote:

> Some Bosch accelerometers have two interrupt pins (INT1 and INT2).
> At the moment, the driver uses only the first one, which is fine for
> most situations. However, some boards might only have INT2 connected
> for some reason.
>
> Add the necessary bits and configuration to set up INT2. Then try
> to detect this situation at least for device tree setups by checking
> if the first interrupt (the one picked by the I2C/SPI core) is actually
> named "INT2" using the interrupt-names property.
>
> of_irq_get_byname() returns either 0 or some error code in case
> the driver probed without device tree, so in all other cases we fall
> back to configuring INT1 as before.
>
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>

Looks good
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

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

* Re: [PATCH v2 1/4] dt-bindings: iio: accel: bma255: Add interrupt-names
  2021-08-02 15:56 ` [PATCH v2 1/4] dt-bindings: iio: accel: bma255: Add interrupt-names Stephan Gerhold
@ 2021-08-06 21:49   ` Rob Herring
  0 siblings, 0 replies; 8+ messages in thread
From: Rob Herring @ 2021-08-06 21:49 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: devicetree, Jonathan Cameron, ~postmarketos/upstreaming,
	Rob Herring, Andy Shevchenko, Lars-Peter Clausen, linux-iio,
	Nikita Travkin, Hans de Goede, Linus Walleij

On Mon, 02 Aug 2021 17:56:54 +0200, Stephan Gerhold wrote:
> The binding already allows specifying both interrupt pins, but there
> is currently no way to describe a board where (for whatever reason)
> only INT2 is connected. Make it possible to use "interrupt-names"
> to make it explicit which interrupt pin is meant in the interrupts.
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
> Changes in v2:
>   - Add "Without interrupt-names, ..." to "interrupts" description
>     to clarify that ordering of interrupts can be relaxed if
>     interrupt-names is used.
> ---
>  .../bindings/iio/accel/bosch,bma255.yaml         | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH v2 0/4] iio: accel: bmc150: Add support for INT2 and BMC156
  2021-08-02 15:56 [PATCH v2 0/4] iio: accel: bmc150: Add support for INT2 and BMC156 Stephan Gerhold
                   ` (3 preceding siblings ...)
  2021-08-02 15:56 ` [PATCH v2 4/4] iio: accel: bmc150: Add support for BMC156 Stephan Gerhold
@ 2021-08-08 15:51 ` Jonathan Cameron
  4 siblings, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2021-08-08 15:51 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: Lars-Peter Clausen, Rob Herring, Linus Walleij, linux-iio,
	devicetree, Hans de Goede, Andy Shevchenko,
	~postmarketos/upstreaming, Nikita Travkin

On Mon,  2 Aug 2021 17:56:53 +0200
Stephan Gerhold <stephan@gerhold.net> wrote:

> This series makes it possible to set up interrupts with the BMC150 driver
> on boards where only the INT2 pin is connected (and not INT1). This is
> particularly always the case for BMC156 since for some reason it only
> has the INT2 pin and not the INT1 pin.
> 
> These changes were already partially discussed here:
> https://lore.kernel.org/linux-iio/YMOphuXSoODIVX06@gerhold.net/

Hopefully one of us or someone else will come back to this and
figure out a clean solution to generic fw support for getting named IRQs.
In the meantime this will be fine for this particular driver.

Some fun to look forwards to ;)

Applied to the togreg branch of iio.git and pushed out as testing for 0-day
to poke at it and see what we missed.

Thanks,

Jonathan

> 
> Changes in v2:
>   - PATCH 1/4: Clarify order of "interrupts" with "interrupt-names"
>   - PATCH 4/4: Wrap a long line, clarify BOSCH_UNKNOWN with a comment
> 
> v1: https://lore.kernel.org/linux-iio/20210719112156.27087-1-stephan@gerhold.net/
> 
> Stephan Gerhold (4):
>   dt-bindings: iio: accel: bma255: Add interrupt-names
>   dt-bindings: iio: accel: bma255: Add bosch,bmc156_accel
>   iio: accel: bmc150: Make it possible to configure INT2 instead of INT1
>   iio: accel: bmc150: Add support for BMC156
> 
>  .../bindings/iio/accel/bosch,bma255.yaml      | 34 +++++++-
>  drivers/iio/accel/Kconfig                     |  5 +-
>  drivers/iio/accel/bmc150-accel-core.c         | 78 +++++++++++++++----
>  drivers/iio/accel/bmc150-accel-i2c.c          | 10 ++-
>  drivers/iio/accel/bmc150-accel-spi.c          | 10 ++-
>  drivers/iio/accel/bmc150-accel.h              | 20 ++++-
>  6 files changed, 134 insertions(+), 23 deletions(-)
> 


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

end of thread, other threads:[~2021-08-08 15:49 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-02 15:56 [PATCH v2 0/4] iio: accel: bmc150: Add support for INT2 and BMC156 Stephan Gerhold
2021-08-02 15:56 ` [PATCH v2 1/4] dt-bindings: iio: accel: bma255: Add interrupt-names Stephan Gerhold
2021-08-06 21:49   ` Rob Herring
2021-08-02 15:56 ` [PATCH v2 2/4] dt-bindings: iio: accel: bma255: Add bosch,bmc156_accel Stephan Gerhold
2021-08-02 15:56 ` [PATCH v2 3/4] iio: accel: bmc150: Make it possible to configure INT2 instead of INT1 Stephan Gerhold
2021-08-03 23:34   ` Linus Walleij
2021-08-02 15:56 ` [PATCH v2 4/4] iio: accel: bmc150: Add support for BMC156 Stephan Gerhold
2021-08-08 15:51 ` [PATCH v2 0/4] iio: accel: bmc150: Add support for INT2 and BMC156 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.