* [PATCH 0/4] iio: accel: bmc150: Add support for INT2 and BMC156 @ 2021-07-19 11:21 Stephan Gerhold 2021-07-19 11:21 ` [PATCH 1/4] dt-bindings: iio: accel: bma255: Add interrupt-names Stephan Gerhold ` (4 more replies) 0 siblings, 5 replies; 27+ messages in thread From: Stephan Gerhold @ 2021-07-19 11:21 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/ 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 | 27 +++++++ drivers/iio/accel/Kconfig | 5 +- drivers/iio/accel/bmc150-accel-core.c | 77 +++++++++++++++---- drivers/iio/accel/bmc150-accel-i2c.c | 10 ++- drivers/iio/accel/bmc150-accel-spi.c | 10 ++- drivers/iio/accel/bmc150-accel.h | 9 ++- 6 files changed, 118 insertions(+), 20 deletions(-) -- 2.32.0 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 1/4] dt-bindings: iio: accel: bma255: Add interrupt-names 2021-07-19 11:21 [PATCH 0/4] iio: accel: bmc150: Add support for INT2 and BMC156 Stephan Gerhold @ 2021-07-19 11:21 ` Stephan Gerhold 2021-07-19 13:57 ` Linus Walleij 2021-07-24 16:00 ` Jonathan Cameron 2021-07-19 11:21 ` [PATCH 2/4] dt-bindings: iio: accel: bma255: Add bosch,bmc156_accel Stephan Gerhold ` (3 subsequent siblings) 4 siblings, 2 replies; 27+ messages in thread From: Stephan Gerhold @ 2021-07-19 11:21 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. Signed-off-by: Stephan Gerhold <stephan@gerhold.net> --- .../devicetree/bindings/iio/accel/bosch,bma255.yaml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml index 5b35856b1942..897a1d808ef5 100644 --- a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml +++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml @@ -49,6 +49,14 @@ properties: 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 +81,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] 27+ messages in thread
* Re: [PATCH 1/4] dt-bindings: iio: accel: bma255: Add interrupt-names 2021-07-19 11:21 ` [PATCH 1/4] dt-bindings: iio: accel: bma255: Add interrupt-names Stephan Gerhold @ 2021-07-19 13:57 ` Linus Walleij 2021-07-24 16:00 ` Jonathan Cameron 1 sibling, 0 replies; 27+ messages in thread From: Linus Walleij @ 2021-07-19 13:57 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, Jul 19, 2021 at 1:26 PM Stephan Gerhold <stephan@gerhold.net> 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. > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] dt-bindings: iio: accel: bma255: Add interrupt-names 2021-07-19 11:21 ` [PATCH 1/4] dt-bindings: iio: accel: bma255: Add interrupt-names Stephan Gerhold 2021-07-19 13:57 ` Linus Walleij @ 2021-07-24 16:00 ` Jonathan Cameron 1 sibling, 0 replies; 27+ messages in thread From: Jonathan Cameron @ 2021-07-24 16:00 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, 19 Jul 2021 13:21:53 +0200 Stephan Gerhold <stephan@gerhold.net> 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. > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> > --- > .../devicetree/bindings/iio/accel/bosch,bma255.yaml | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml > index 5b35856b1942..897a1d808ef5 100644 > --- a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml > +++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml > @@ -49,6 +49,14 @@ properties: > the second (optional) interrupt listed must be the one connected to the > INT2 pin (if available). The type should be IRQ_TYPE_EDGE_RISING. Looks like this comment needs updating to say the ordering is only true if interrupt-names is not present. Jonathan > > + interrupt-names: > + minItems: 1 > + maxItems: 2 > + items: > + enum: > + - INT1 > + - INT2 > + > mount-matrix: > description: an optional 3x3 mounting rotation matrix. > > @@ -73,6 +81,7 @@ examples: > vddio-supply = <&vddio>; > vdd-supply = <&vdd>; > interrupts = <57 IRQ_TYPE_EDGE_RISING>; > + interrupt-names = "INT1"; > }; > }; > - | ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 2/4] dt-bindings: iio: accel: bma255: Add bosch,bmc156_accel 2021-07-19 11:21 [PATCH 0/4] iio: accel: bmc150: Add support for INT2 and BMC156 Stephan Gerhold 2021-07-19 11:21 ` [PATCH 1/4] dt-bindings: iio: accel: bma255: Add interrupt-names Stephan Gerhold @ 2021-07-19 11:21 ` Stephan Gerhold 2021-07-19 13:58 ` Linus Walleij ` (2 more replies) 2021-07-19 11:21 ` [PATCH 3/4] iio: accel: bmc150: Make it possible to configure INT2 instead of INT1 Stephan Gerhold ` (2 subsequent siblings) 4 siblings, 3 replies; 27+ messages in thread From: Stephan Gerhold @ 2021-07-19 11:21 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 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 was crazy and 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. 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 897a1d808ef5..f7848e4a7b29 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 @@ -49,6 +50,9 @@ properties: the second (optional) interrupt listed must be 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 @@ -84,6 +88,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] 27+ messages in thread
* Re: [PATCH 2/4] dt-bindings: iio: accel: bma255: Add bosch,bmc156_accel 2021-07-19 11:21 ` [PATCH 2/4] dt-bindings: iio: accel: bma255: Add bosch,bmc156_accel Stephan Gerhold @ 2021-07-19 13:58 ` Linus Walleij 2021-07-24 16:03 ` Jonathan Cameron 2021-07-29 19:10 ` Rob Herring 2 siblings, 0 replies; 27+ messages in thread From: Linus Walleij @ 2021-07-19 13:58 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, Jul 19, 2021 at 1:26 PM Stephan Gerhold <stephan@gerhold.net> wrote: > 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 was crazy and 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. > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/4] dt-bindings: iio: accel: bma255: Add bosch,bmc156_accel 2021-07-19 11:21 ` [PATCH 2/4] dt-bindings: iio: accel: bma255: Add bosch,bmc156_accel Stephan Gerhold 2021-07-19 13:58 ` Linus Walleij @ 2021-07-24 16:03 ` Jonathan Cameron 2021-07-29 19:10 ` Rob Herring 2021-07-29 19:10 ` Rob Herring 2 siblings, 1 reply; 27+ messages in thread From: Jonathan Cameron @ 2021-07-24 16:03 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, 19 Jul 2021 13:21:54 +0200 Stephan Gerhold <stephan@gerhold.net> wrote: > 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 was crazy and 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. > > 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 897a1d808ef5..f7848e4a7b29 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 > @@ -49,6 +50,9 @@ properties: > the second (optional) interrupt listed must be 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. We 'could' enforce any name present for the bmc156 to be INT2, but we probably still want the fallback you have in the driver to handle the case of not interrupt-name provided. Rob, do you think it's worth the complexity for this corner case? Jonathan > + > interrupt-names: > minItems: 1 > maxItems: 2 > @@ -84,6 +88,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 { ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/4] dt-bindings: iio: accel: bma255: Add bosch,bmc156_accel 2021-07-24 16:03 ` Jonathan Cameron @ 2021-07-29 19:10 ` Rob Herring 0 siblings, 0 replies; 27+ messages in thread From: Rob Herring @ 2021-07-29 19:10 UTC (permalink / raw) To: Jonathan Cameron Cc: Stephan Gerhold, Lars-Peter Clausen, Linus Walleij, linux-iio, devicetree, Hans de Goede, Andy Shevchenko, ~postmarketos/upstreaming, Nikita Travkin On Sat, Jul 24, 2021 at 05:03:18PM +0100, Jonathan Cameron wrote: > On Mon, 19 Jul 2021 13:21:54 +0200 > Stephan Gerhold <stephan@gerhold.net> wrote: > > > 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 was crazy and 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. > > > > 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 897a1d808ef5..f7848e4a7b29 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 > > @@ -49,6 +50,9 @@ properties: > > the second (optional) interrupt listed must be 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. > > We 'could' enforce any name present for the bmc156 to be INT2, but we probably > still want the fallback you have in the driver to handle the case of > not interrupt-name provided. > > Rob, do you think it's worth the complexity for this corner case? No. Rob ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/4] dt-bindings: iio: accel: bma255: Add bosch,bmc156_accel 2021-07-19 11:21 ` [PATCH 2/4] dt-bindings: iio: accel: bma255: Add bosch,bmc156_accel Stephan Gerhold 2021-07-19 13:58 ` Linus Walleij 2021-07-24 16:03 ` Jonathan Cameron @ 2021-07-29 19:10 ` Rob Herring 2 siblings, 0 replies; 27+ messages in thread From: Rob Herring @ 2021-07-29 19:10 UTC (permalink / raw) To: Stephan Gerhold Cc: Hans de Goede, devicetree, Jonathan Cameron, Andy Shevchenko, Nikita Travkin, Linus Walleij, ~postmarketos/upstreaming, Lars-Peter Clausen, linux-iio, Rob Herring On Mon, 19 Jul 2021 13:21:54 +0200, Stephan Gerhold wrote: > 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 was crazy and 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. > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> > --- > .../bindings/iio/accel/bosch,bma255.yaml | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > Reviewed-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 3/4] iio: accel: bmc150: Make it possible to configure INT2 instead of INT1 2021-07-19 11:21 [PATCH 0/4] iio: accel: bmc150: Add support for INT2 and BMC156 Stephan Gerhold 2021-07-19 11:21 ` [PATCH 1/4] dt-bindings: iio: accel: bma255: Add interrupt-names Stephan Gerhold 2021-07-19 11:21 ` [PATCH 2/4] dt-bindings: iio: accel: bma255: Add bosch,bmc156_accel Stephan Gerhold @ 2021-07-19 11:21 ` Stephan Gerhold 2021-07-19 14:07 ` Linus Walleij 2021-07-19 11:21 ` [PATCH 4/4] iio: accel: bmc150: Add support for BMC156 Stephan Gerhold 2021-07-19 12:34 ` [PATCH 0/4] iio: accel: bmc150: Add support for INT2 and BMC156 Andy Shevchenko 4 siblings, 1 reply; 27+ messages in thread From: Stephan Gerhold @ 2021-07-19 11:21 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] 27+ messages in thread
* Re: [PATCH 3/4] iio: accel: bmc150: Make it possible to configure INT2 instead of INT1 2021-07-19 11:21 ` [PATCH 3/4] iio: accel: bmc150: Make it possible to configure INT2 instead of INT1 Stephan Gerhold @ 2021-07-19 14:07 ` Linus Walleij 2021-07-19 15:01 ` Andy Shevchenko 0 siblings, 1 reply; 27+ messages in thread From: Linus Walleij @ 2021-07-19 14:07 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, Jul 19, 2021 at 1:26 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> > #include <linux/acpi.h> > +#include <linux/of_irq.h> (...) > + irq_info = bmc150_accel_interrupts_int1; > + if (irq == of_irq_get_byname(dev->of_node, "INT2")) > + irq_info = bmc150_accel_interrupts_int2; This looks a bit DT-specific, but I don't see that ACPI has named IRQs so I don't know what to do about it either. What does platform_get_irq_byname() do on ACPI systems? If there is no obvious fix I would leave it like this until the first ACPI used needing this comes along, but I think maybe Andy has suggestions. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] iio: accel: bmc150: Make it possible to configure INT2 instead of INT1 2021-07-19 14:07 ` Linus Walleij @ 2021-07-19 15:01 ` Andy Shevchenko 2021-07-19 15:10 ` Stephan Gerhold 0 siblings, 1 reply; 27+ messages in thread From: Andy Shevchenko @ 2021-07-19 15:01 UTC (permalink / raw) To: Linus Walleij Cc: Stephan Gerhold, Jonathan Cameron, Lars-Peter Clausen, Rob Herring, linux-iio, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Hans de Goede, 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, Jul 19, 2021 at 5:07 PM Linus Walleij <linus.walleij@linaro.org> wrote: > On Mon, Jul 19, 2021 at 1:26 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> > > > #include <linux/acpi.h> > > +#include <linux/of_irq.h> > (...) > > + irq_info = bmc150_accel_interrupts_int1; > > + if (irq == of_irq_get_byname(dev->of_node, "INT2")) > > + irq_info = bmc150_accel_interrupts_int2; > > This looks a bit DT-specific, but I don't see that ACPI has > named IRQs so I don't know what to do about it either. Yeah, we only have so far the (de facto) established way of naming GPIO based IRQs, and not IOxAPIC ones. > What does platform_get_irq_byname() do on ACPI systems? See above. > If there is no obvious fix I would leave it like this until the > first ACPI used needing this comes along, but I think maybe > Andy has suggestions. The platform_get_irq_byname() should do something similar that has been done in platform_get_irq() WRT ACPI. Here for sure the platform_get_irq_byname() or its optional variant should be used. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] iio: accel: bmc150: Make it possible to configure INT2 instead of INT1 2021-07-19 15:01 ` Andy Shevchenko @ 2021-07-19 15:10 ` Stephan Gerhold 2021-07-19 16:19 ` Andy Shevchenko 0 siblings, 1 reply; 27+ messages in thread From: Stephan Gerhold @ 2021-07-19 15:10 UTC (permalink / raw) To: Andy Shevchenko Cc: Linus Walleij, Jonathan Cameron, Lars-Peter Clausen, Rob Herring, linux-iio, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Hans de Goede, 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, Jul 19, 2021 at 06:01:01PM +0300, Andy Shevchenko wrote: > On Mon, Jul 19, 2021 at 5:07 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > On Mon, Jul 19, 2021 at 1:26 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> > > > > > #include <linux/acpi.h> > > > +#include <linux/of_irq.h> > > (...) > > > + irq_info = bmc150_accel_interrupts_int1; > > > + if (irq == of_irq_get_byname(dev->of_node, "INT2")) > > > + irq_info = bmc150_accel_interrupts_int2; > > > > This looks a bit DT-specific, but I don't see that ACPI has > > named IRQs so I don't know what to do about it either. > > Yeah, we only have so far the (de facto) established way of naming > GPIO based IRQs, and not IOxAPIC ones. > > > What does platform_get_irq_byname() do on ACPI systems? > > See above. > > > If there is no obvious fix I would leave it like this until the > > first ACPI used needing this comes along, but I think maybe > > Andy has suggestions. > > The platform_get_irq_byname() should do something similar that has > been done in platform_get_irq() WRT ACPI. > Here for sure the platform_get_irq_byname() or its optional variant > should be used. > I don't think there is a platform device here, we only have the i2c_client or spi_device. That's why I didn't use platform_get_irq_byname(). :) Is there something equivalent for I2C/SPI drivers? Thanks! Stephan ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] iio: accel: bmc150: Make it possible to configure INT2 instead of INT1 2021-07-19 15:10 ` Stephan Gerhold @ 2021-07-19 16:19 ` Andy Shevchenko 2021-07-19 17:26 ` Stephan Gerhold 0 siblings, 1 reply; 27+ messages in thread From: Andy Shevchenko @ 2021-07-19 16:19 UTC (permalink / raw) To: Stephan Gerhold Cc: Linus Walleij, Jonathan Cameron, Lars-Peter Clausen, Rob Herring, linux-iio, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Hans de Goede, 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, Jul 19, 2021 at 6:11 PM Stephan Gerhold <stephan@gerhold.net> wrote: > On Mon, Jul 19, 2021 at 06:01:01PM +0300, Andy Shevchenko wrote: > > On Mon, Jul 19, 2021 at 5:07 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > > On Mon, Jul 19, 2021 at 1:26 PM Stephan Gerhold <stephan@gerhold.net> wrote: ... > > > > #include <linux/acpi.h> > > > > +#include <linux/of_irq.h> > > > (...) > > > > + irq_info = bmc150_accel_interrupts_int1; > > > > + if (irq == of_irq_get_byname(dev->of_node, "INT2")) > > > > + irq_info = bmc150_accel_interrupts_int2; > > > > > > This looks a bit DT-specific, but I don't see that ACPI has > > > named IRQs so I don't know what to do about it either. > > > > Yeah, we only have so far the (de facto) established way of naming > > GPIO based IRQs, and not IOxAPIC ones. > > > > > What does platform_get_irq_byname() do on ACPI systems? > > > > See above. > > > > > If there is no obvious fix I would leave it like this until the > > > first ACPI used needing this comes along, but I think maybe > > > Andy has suggestions. > > > > The platform_get_irq_byname() should do something similar that has > > been done in platform_get_irq() WRT ACPI. > > Here for sure the platform_get_irq_byname() or its optional variant > > should be used. > > I don't think there is a platform device here, we only have the > i2c_client or spi_device. That's why I didn't use > platform_get_irq_byname(). :) > > Is there something equivalent for I2C/SPI drivers? Not yet. You probably need to supply some code there to allow multi-IRQ devices (in resource provider agnostic way). You need to provide fwnode_get_irq_byname() to be similar with https://elixir.bootlin.com/linux/latest/source/drivers/base/property.c#L1010 Then use it in the drivers. And/or integrate into frameworks somehow (something in between the lines: https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-core-base.c#L461). -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] iio: accel: bmc150: Make it possible to configure INT2 instead of INT1 2021-07-19 16:19 ` Andy Shevchenko @ 2021-07-19 17:26 ` Stephan Gerhold 2021-07-19 18:05 ` Andy Shevchenko 0 siblings, 1 reply; 27+ messages in thread From: Stephan Gerhold @ 2021-07-19 17:26 UTC (permalink / raw) To: Andy Shevchenko Cc: Linus Walleij, Jonathan Cameron, Lars-Peter Clausen, Rob Herring, linux-iio, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Hans de Goede, 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, Jul 19, 2021 at 07:19:05PM +0300, Andy Shevchenko wrote: > On Mon, Jul 19, 2021 at 6:11 PM Stephan Gerhold <stephan@gerhold.net> wrote: > > On Mon, Jul 19, 2021 at 06:01:01PM +0300, Andy Shevchenko wrote: > > > On Mon, Jul 19, 2021 at 5:07 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > > > On Mon, Jul 19, 2021 at 1:26 PM Stephan Gerhold <stephan@gerhold.net> wrote: > > ... > > > > > > #include <linux/acpi.h> > > > > > +#include <linux/of_irq.h> > > > > (...) > > > > > + irq_info = bmc150_accel_interrupts_int1; > > > > > + if (irq == of_irq_get_byname(dev->of_node, "INT2")) > > > > > + irq_info = bmc150_accel_interrupts_int2; > > > > > > > > This looks a bit DT-specific, but I don't see that ACPI has > > > > named IRQs so I don't know what to do about it either. > > > > > > Yeah, we only have so far the (de facto) established way of naming > > > GPIO based IRQs, and not IOxAPIC ones. > > > > > > > What does platform_get_irq_byname() do on ACPI systems? > > > > > > See above. > > > > > > > If there is no obvious fix I would leave it like this until the > > > > first ACPI used needing this comes along, but I think maybe > > > > Andy has suggestions. > > > > > > The platform_get_irq_byname() should do something similar that has > > > been done in platform_get_irq() WRT ACPI. > > > Here for sure the platform_get_irq_byname() or its optional variant > > > should be used. > > > > I don't think there is a platform device here, we only have the > > i2c_client or spi_device. That's why I didn't use > > platform_get_irq_byname(). :) > > > > Is there something equivalent for I2C/SPI drivers? > > Not yet. You probably need to supply some code there to allow > multi-IRQ devices (in resource provider agnostic way). > > You need to provide fwnode_get_irq_byname() to be similar with > https://elixir.bootlin.com/linux/latest/source/drivers/base/property.c#L1010 > > Then use it in the drivers. > > And/or integrate into frameworks somehow (something in between the > lines: https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-core-base.c#L461). > Well, I don't think anyone has an ACPI use case for this right now so it's probably better if this is done by someone who actually needs this and can test it somewhere. :) I actually just "copied" this approach from some other IIO drivers where this is done similarly (and additionally checked the source code to make sure this won't break anything for ACPI platforms). Stephan ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] iio: accel: bmc150: Make it possible to configure INT2 instead of INT1 2021-07-19 17:26 ` Stephan Gerhold @ 2021-07-19 18:05 ` Andy Shevchenko 2021-07-19 18:36 ` Stephan Gerhold 2021-07-24 18:06 ` Jonathan Cameron 0 siblings, 2 replies; 27+ messages in thread From: Andy Shevchenko @ 2021-07-19 18:05 UTC (permalink / raw) To: Stephan Gerhold Cc: Linus Walleij, Jonathan Cameron, Lars-Peter Clausen, Rob Herring, linux-iio, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Hans de Goede, 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, Jul 19, 2021 at 8:29 PM Stephan Gerhold <stephan@gerhold.net> wrote: > > On Mon, Jul 19, 2021 at 07:19:05PM +0300, Andy Shevchenko wrote: > > On Mon, Jul 19, 2021 at 6:11 PM Stephan Gerhold <stephan@gerhold.net> wrote: > > > On Mon, Jul 19, 2021 at 06:01:01PM +0300, Andy Shevchenko wrote: > > > > On Mon, Jul 19, 2021 at 5:07 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > > > > On Mon, Jul 19, 2021 at 1:26 PM Stephan Gerhold <stephan@gerhold.net> wrote: ... > > > > > > #include <linux/acpi.h> > > > > > > +#include <linux/of_irq.h> > > > > > (...) > > > > > > + irq_info = bmc150_accel_interrupts_int1; > > > > > > + if (irq == of_irq_get_byname(dev->of_node, "INT2")) > > > > > > + irq_info = bmc150_accel_interrupts_int2; > > > > > > > > > > This looks a bit DT-specific, but I don't see that ACPI has > > > > > named IRQs so I don't know what to do about it either. > > > > > > > > Yeah, we only have so far the (de facto) established way of naming > > > > GPIO based IRQs, and not IOxAPIC ones. > > > > > > > > > What does platform_get_irq_byname() do on ACPI systems? > > > > > > > > See above. > > > > > > > > > If there is no obvious fix I would leave it like this until the > > > > > first ACPI used needing this comes along, but I think maybe > > > > > Andy has suggestions. > > > > > > > > The platform_get_irq_byname() should do something similar that has > > > > been done in platform_get_irq() WRT ACPI. > > > > Here for sure the platform_get_irq_byname() or its optional variant > > > > should be used. > > > > > > I don't think there is a platform device here, we only have the > > > i2c_client or spi_device. That's why I didn't use > > > platform_get_irq_byname(). :) > > > > > > Is there something equivalent for I2C/SPI drivers? > > > > Not yet. You probably need to supply some code there to allow > > multi-IRQ devices (in resource provider agnostic way). > > > > You need to provide fwnode_get_irq_byname() to be similar with > > https://elixir.bootlin.com/linux/latest/source/drivers/base/property.c#L1010 > > > > Then use it in the drivers. > > > > And/or integrate into frameworks somehow (something in between the > > lines: https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-core-base.c#L461). > > > > Well, I don't think anyone has an ACPI use case for this right now so > it's probably better if this is done by someone who actually needs this > and can test it somewhere. :) > > I actually just "copied" this approach from some other IIO drivers where > this is done similarly (and additionally checked the source code to make > sure this won't break anything for ACPI platforms). I see in today's Linux Next snapshot: drivers/iio/accel/fxls8962af-core.c:774: irq = of_irq_get_byname(of_node, "INT2"); drivers/iio/accel/mma8452.c:1616: irq2 = of_irq_get_byname(client->dev.of_node, "INT2"); drivers/iio/gyro/fxas21002c_core.c:834: irq1 = of_irq_get_byname(np, "INT1"); drivers/iio/imu/adis16480.c:1265: irq = of_irq_get_byname(of_node, adis16480_int_pin_names[i]); drivers/iio/imu/bmi160/bmi160_core.c:655: irq = of_irq_get_byname(of_node, "INT1"); drivers/iio/imu/bmi160/bmi160_core.c:661: irq = of_irq_get_byname(of_node, "INT2"); I believe we may stop distributing this and actually start using a common API. I don't want this to be spread again over all IIO. Btw, I have LSM9DS0, which supports two INT pins for IMU and currently it uses hard coded pin mapping. Side note to Jonathan, I believe the below may be (lazily) converted to fwnode / device properties APIs. drivers/iio/adc/ab8500-gpadc.c:1041: nchans = of_get_available_child_count(np); drivers/iio/adc/ad7124.c:747: st->num_channels = of_get_available_child_count(np); drivers/iio/adc/berlin2-adc.c:287: struct device_node *parent_np = of_get_parent(pdev->dev.of_node); drivers/iio/adc/qcom-pm8xxx-xoadc.c:830: adc->nchans = of_get_available_child_count(np); drivers/iio/adc/qcom-spmi-adc5.c:650: channel_name = of_get_property(node, drivers/iio/adc/qcom-spmi-adc5.c:813: adc->nchannels = of_get_available_child_count(node); drivers/iio/adc/qcom-spmi-vadc.c:743: vadc->nchannels = of_get_available_child_count(node); drivers/iio/adc/stm32-adc.c:1630:static int stm32_adc_of_get_resolution(struct iio_dev *indio_dev) drivers/iio/adc/stm32-adc.c:1935: ret = stm32_adc_of_get_resolution(indio_dev); drivers/iio/adc/xilinx-xadc-core.c:1248: chan_node = of_get_child_by_name(np, "xlnx,channels"); drivers/iio/imu/adis16480.c:1293:static int adis16480_of_get_ext_clk_pin(struct adis16480 *st, drivers/iio/imu/adis16480.c:1328: pin = adis16480_of_get_ext_clk_pin(st, of_node); drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c:68: mux_node = of_get_child_by_name(dev->of_node, "i2c-gate") ; drivers/iio/industrialio-core.c:1877: label = of_get_property(indio_dev->dev.of_node, "label", NULL); drivers/iio/inkern.c:228: if (np && !of_get_property(np, "io-channel-ranges", NULL)) drivers/iio/temperature/ltc2983.c:1275: st->num_channels = of_get_available_child_count(dev->of_node); -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] iio: accel: bmc150: Make it possible to configure INT2 instead of INT1 2021-07-19 18:05 ` Andy Shevchenko @ 2021-07-19 18:36 ` Stephan Gerhold 2021-07-20 15:04 ` Andy Shevchenko 2021-07-24 18:06 ` Jonathan Cameron 1 sibling, 1 reply; 27+ messages in thread From: Stephan Gerhold @ 2021-07-19 18:36 UTC (permalink / raw) To: Andy Shevchenko Cc: Linus Walleij, Jonathan Cameron, Lars-Peter Clausen, Rob Herring, linux-iio, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Hans de Goede, 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, Jul 19, 2021 at 09:05:48PM +0300, Andy Shevchenko wrote: > On Mon, Jul 19, 2021 at 8:29 PM Stephan Gerhold <stephan@gerhold.net> wrote: > > > > On Mon, Jul 19, 2021 at 07:19:05PM +0300, Andy Shevchenko wrote: > > > On Mon, Jul 19, 2021 at 6:11 PM Stephan Gerhold <stephan@gerhold.net> wrote: > > > > On Mon, Jul 19, 2021 at 06:01:01PM +0300, Andy Shevchenko wrote: > > > > > On Mon, Jul 19, 2021 at 5:07 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > > > > > On Mon, Jul 19, 2021 at 1:26 PM Stephan Gerhold <stephan@gerhold.net> wrote: > > ... > > > > > > > > #include <linux/acpi.h> > > > > > > > +#include <linux/of_irq.h> > > > > > > (...) > > > > > > > + irq_info = bmc150_accel_interrupts_int1; > > > > > > > + if (irq == of_irq_get_byname(dev->of_node, "INT2")) > > > > > > > + irq_info = bmc150_accel_interrupts_int2; > > > > > > > > > > > > This looks a bit DT-specific, but I don't see that ACPI has > > > > > > named IRQs so I don't know what to do about it either. > > > > > > > > > > Yeah, we only have so far the (de facto) established way of naming > > > > > GPIO based IRQs, and not IOxAPIC ones. > > > > > > > > > > > What does platform_get_irq_byname() do on ACPI systems? > > > > > > > > > > See above. > > > > > > > > > > > If there is no obvious fix I would leave it like this until the > > > > > > first ACPI used needing this comes along, but I think maybe > > > > > > Andy has suggestions. > > > > > > > > > > The platform_get_irq_byname() should do something similar that has > > > > > been done in platform_get_irq() WRT ACPI. > > > > > Here for sure the platform_get_irq_byname() or its optional variant > > > > > should be used. > > > > > > > > I don't think there is a platform device here, we only have the > > > > i2c_client or spi_device. That's why I didn't use > > > > platform_get_irq_byname(). :) > > > > > > > > Is there something equivalent for I2C/SPI drivers? > > > > > > Not yet. You probably need to supply some code there to allow > > > multi-IRQ devices (in resource provider agnostic way). > > > > > > You need to provide fwnode_get_irq_byname() to be similar with > > > https://elixir.bootlin.com/linux/latest/source/drivers/base/property.c#L1010 > > > > > > Then use it in the drivers. > > > > > > And/or integrate into frameworks somehow (something in between the > > > lines: https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-core-base.c#L461). > > > > > > > Well, I don't think anyone has an ACPI use case for this right now so > > it's probably better if this is done by someone who actually needs this > > and can test it somewhere. :) > > > > I actually just "copied" this approach from some other IIO drivers where > > this is done similarly (and additionally checked the source code to make > > sure this won't break anything for ACPI platforms). > > I see in today's Linux Next snapshot: > > drivers/iio/accel/fxls8962af-core.c:774: irq = > of_irq_get_byname(of_node, "INT2"); > drivers/iio/accel/mma8452.c:1616: irq2 = > of_irq_get_byname(client->dev.of_node, "INT2"); > drivers/iio/gyro/fxas21002c_core.c:834: irq1 = of_irq_get_byname(np, "INT1"); > drivers/iio/imu/adis16480.c:1265: irq = > of_irq_get_byname(of_node, adis16480_int_pin_names[i]); > drivers/iio/imu/bmi160/bmi160_core.c:655: irq = > of_irq_get_byname(of_node, "INT1"); > drivers/iio/imu/bmi160/bmi160_core.c:661: irq = > of_irq_get_byname(of_node, "INT2"); > > I believe we may stop distributing this and actually start using a > common API. I don't want this to be spread again over all IIO. Btw, I > have LSM9DS0, which supports two INT pins for IMU and currently it > uses hard coded pin mapping. > Hm, I'm not quite sure how to implement this. Could you prepare a patch that would implement such a common API? I would be happy to test it for the device tree and make use of it in this patch. To be honest, I mainly implemented support for the interrupt-names because Jonathan mentioned this would be nice to have [1] and it kind of fit well together with the BMC156 patch that needs the INT2 support. I actually just use the if (data->type == BOSCH_BMC156) part from PATCH 4/4 which does not depend on of_irq_get_byname(). Thanks, Stephan [1]: https://lore.kernel.org/linux-iio/20210611185941.3487efc6@jic23-huawei/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] iio: accel: bmc150: Make it possible to configure INT2 instead of INT1 2021-07-19 18:36 ` Stephan Gerhold @ 2021-07-20 15:04 ` Andy Shevchenko 2021-07-24 16:19 ` Jonathan Cameron 0 siblings, 1 reply; 27+ messages in thread From: Andy Shevchenko @ 2021-07-20 15:04 UTC (permalink / raw) To: Stephan Gerhold Cc: Linus Walleij, Jonathan Cameron, Lars-Peter Clausen, Rob Herring, linux-iio, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Hans de Goede, 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, Jul 19, 2021 at 9:37 PM Stephan Gerhold <stephan@gerhold.net> wrote: > On Mon, Jul 19, 2021 at 09:05:48PM +0300, Andy Shevchenko wrote: > > On Mon, Jul 19, 2021 at 8:29 PM Stephan Gerhold <stephan@gerhold.net> wrote: > > > On Mon, Jul 19, 2021 at 07:19:05PM +0300, Andy Shevchenko wrote: > > > > On Mon, Jul 19, 2021 at 6:11 PM Stephan Gerhold <stephan@gerhold.net> wrote: > > > > > On Mon, Jul 19, 2021 at 06:01:01PM +0300, Andy Shevchenko wrote: > > > > > > On Mon, Jul 19, 2021 at 5:07 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > > > > > > On Mon, Jul 19, 2021 at 1:26 PM Stephan Gerhold <stephan@gerhold.net> wrote: ... > > > > > > > > + irq_info = bmc150_accel_interrupts_int1; > > > > > > > > + if (irq == of_irq_get_byname(dev->of_node, "INT2")) > > > > > > > > + irq_info = bmc150_accel_interrupts_int2; > > > > > > > > > > > > > > This looks a bit DT-specific, but I don't see that ACPI has > > > > > > > named IRQs so I don't know what to do about it either. > > > > > > > > > > > > Yeah, we only have so far the (de facto) established way of naming > > > > > > GPIO based IRQs, and not IOxAPIC ones. > > > > > > > > > > > > > What does platform_get_irq_byname() do on ACPI systems? > > > > > > > > > > > > See above. > > > > > > > > > > > > > If there is no obvious fix I would leave it like this until the > > > > > > > first ACPI used needing this comes along, but I think maybe > > > > > > > Andy has suggestions. > > > > > > > > > > > > The platform_get_irq_byname() should do something similar that has > > > > > > been done in platform_get_irq() WRT ACPI. > > > > > > Here for sure the platform_get_irq_byname() or its optional variant > > > > > > should be used. > > > > > > > > > > I don't think there is a platform device here, we only have the > > > > > i2c_client or spi_device. That's why I didn't use > > > > > platform_get_irq_byname(). :) > > > > > > > > > > Is there something equivalent for I2C/SPI drivers? > > > > > > > > Not yet. You probably need to supply some code there to allow > > > > multi-IRQ devices (in resource provider agnostic way). > > > > > > > > You need to provide fwnode_get_irq_byname() to be similar with > > > > https://elixir.bootlin.com/linux/latest/source/drivers/base/property.c#L1010 > > > > > > > > Then use it in the drivers. > > > > > > > > And/or integrate into frameworks somehow (something in between the > > > > lines: https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-core-base.c#L461). > > > > > > > > > > Well, I don't think anyone has an ACPI use case for this right now so > > > it's probably better if this is done by someone who actually needs this > > > and can test it somewhere. :) > > > > > > I actually just "copied" this approach from some other IIO drivers where > > > this is done similarly (and additionally checked the source code to make > > > sure this won't break anything for ACPI platforms). > > > > I see in today's Linux Next snapshot: > > > > drivers/iio/accel/fxls8962af-core.c:774: irq = > > of_irq_get_byname(of_node, "INT2"); > > drivers/iio/accel/mma8452.c:1616: irq2 = > > of_irq_get_byname(client->dev.of_node, "INT2"); > > drivers/iio/gyro/fxas21002c_core.c:834: irq1 = of_irq_get_byname(np, "INT1"); > > drivers/iio/imu/adis16480.c:1265: irq = > > of_irq_get_byname(of_node, adis16480_int_pin_names[i]); > > drivers/iio/imu/bmi160/bmi160_core.c:655: irq = > > of_irq_get_byname(of_node, "INT1"); > > drivers/iio/imu/bmi160/bmi160_core.c:661: irq = > > of_irq_get_byname(of_node, "INT2"); > > > > I believe we may stop distributing this and actually start using a > > common API. I don't want this to be spread again over all IIO. Btw, I > > have LSM9DS0, which supports two INT pins for IMU and currently it > > uses hard coded pin mapping. > > > > Hm, I'm not quite sure how to implement this. Could you prepare a patch > that would implement such a common API? I would be happy to test it for > the device tree and make use of it in this patch. Unfortunately I have no time to fulfil the required process. The idea in general is like this: if (is_of_node(...)) return of_irq_get_byname(...); if (is_acpi_node(...)) return acpi_gpio_irq_get_byname(...); Everything else is quite similar to fwnode_irq_get(). > To be honest, I mainly implemented support for the interrupt-names > because Jonathan mentioned this would be nice to have [1] and it kind of > fit well together with the BMC156 patch that needs the INT2 support. > I actually just use the if (data->type == BOSCH_BMC156) part from > PATCH 4/4 which does not depend on of_irq_get_byname(). Then I leave it to Jonathan and other maintainers. > [1]: https://lore.kernel.org/linux-iio/20210611185941.3487efc6@jic23-huawei/ -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] iio: accel: bmc150: Make it possible to configure INT2 instead of INT1 2021-07-20 15:04 ` Andy Shevchenko @ 2021-07-24 16:19 ` Jonathan Cameron 0 siblings, 0 replies; 27+ messages in thread From: Jonathan Cameron @ 2021-07-24 16:19 UTC (permalink / raw) To: Andy Shevchenko Cc: Stephan Gerhold, Linus Walleij, Lars-Peter Clausen, Rob Herring, linux-iio, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Hans de Goede, 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 Tue, 20 Jul 2021 18:04:22 +0300 Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Mon, Jul 19, 2021 at 9:37 PM Stephan Gerhold <stephan@gerhold.net> wrote: > > On Mon, Jul 19, 2021 at 09:05:48PM +0300, Andy Shevchenko wrote: > > > On Mon, Jul 19, 2021 at 8:29 PM Stephan Gerhold <stephan@gerhold.net> wrote: > > > > On Mon, Jul 19, 2021 at 07:19:05PM +0300, Andy Shevchenko wrote: > > > > > On Mon, Jul 19, 2021 at 6:11 PM Stephan Gerhold <stephan@gerhold.net> wrote: > > > > > > On Mon, Jul 19, 2021 at 06:01:01PM +0300, Andy Shevchenko wrote: > > > > > > > On Mon, Jul 19, 2021 at 5:07 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > > > > > > > On Mon, Jul 19, 2021 at 1:26 PM Stephan Gerhold <stephan@gerhold.net> wrote: > > ... > > > > > > > > > > + irq_info = bmc150_accel_interrupts_int1; > > > > > > > > > + if (irq == of_irq_get_byname(dev->of_node, "INT2")) > > > > > > > > > + irq_info = bmc150_accel_interrupts_int2; > > > > > > > > > > > > > > > > This looks a bit DT-specific, but I don't see that ACPI has > > > > > > > > named IRQs so I don't know what to do about it either. > > > > > > > > > > > > > > Yeah, we only have so far the (de facto) established way of naming > > > > > > > GPIO based IRQs, and not IOxAPIC ones. > > > > > > > > > > > > > > > What does platform_get_irq_byname() do on ACPI systems? > > > > > > > > > > > > > > See above. > > > > > > > > > > > > > > > If there is no obvious fix I would leave it like this until the > > > > > > > > first ACPI used needing this comes along, but I think maybe > > > > > > > > Andy has suggestions. > > > > > > > > > > > > > > The platform_get_irq_byname() should do something similar that has > > > > > > > been done in platform_get_irq() WRT ACPI. > > > > > > > Here for sure the platform_get_irq_byname() or its optional variant > > > > > > > should be used. > > > > > > > > > > > > I don't think there is a platform device here, we only have the > > > > > > i2c_client or spi_device. That's why I didn't use > > > > > > platform_get_irq_byname(). :) > > > > > > > > > > > > Is there something equivalent for I2C/SPI drivers? > > > > > > > > > > Not yet. You probably need to supply some code there to allow > > > > > multi-IRQ devices (in resource provider agnostic way). > > > > > > > > > > You need to provide fwnode_get_irq_byname() to be similar with > > > > > https://elixir.bootlin.com/linux/latest/source/drivers/base/property.c#L1010 > > > > > > > > > > Then use it in the drivers. > > > > > > > > > > And/or integrate into frameworks somehow (something in between the > > > > > lines: https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-core-base.c#L461). > > > > > > > > > > > > > Well, I don't think anyone has an ACPI use case for this right now so > > > > it's probably better if this is done by someone who actually needs this > > > > and can test it somewhere. :) > > > > > > > > I actually just "copied" this approach from some other IIO drivers where > > > > this is done similarly (and additionally checked the source code to make > > > > sure this won't break anything for ACPI platforms). > > > > > > I see in today's Linux Next snapshot: > > > > > > drivers/iio/accel/fxls8962af-core.c:774: irq = > > > of_irq_get_byname(of_node, "INT2"); > > > drivers/iio/accel/mma8452.c:1616: irq2 = > > > of_irq_get_byname(client->dev.of_node, "INT2"); > > > drivers/iio/gyro/fxas21002c_core.c:834: irq1 = of_irq_get_byname(np, "INT1"); > > > drivers/iio/imu/adis16480.c:1265: irq = > > > of_irq_get_byname(of_node, adis16480_int_pin_names[i]); > > > drivers/iio/imu/bmi160/bmi160_core.c:655: irq = > > > of_irq_get_byname(of_node, "INT1"); > > > drivers/iio/imu/bmi160/bmi160_core.c:661: irq = > > > of_irq_get_byname(of_node, "INT2"); > > > > > > I believe we may stop distributing this and actually start using a > > > common API. I don't want this to be spread again over all IIO. Btw, I > > > have LSM9DS0, which supports two INT pins for IMU and currently it > > > uses hard coded pin mapping. > > > > > > > Hm, I'm not quite sure how to implement this. Could you prepare a patch > > that would implement such a common API? I would be happy to test it for > > the device tree and make use of it in this patch. > > Unfortunately I have no time to fulfil the required process. The idea > in general is like this: > > if (is_of_node(...)) > return of_irq_get_byname(...); > if (is_acpi_node(...)) > return acpi_gpio_irq_get_byname(...); > > Everything else is quite similar to fwnode_irq_get(). > > > To be honest, I mainly implemented support for the interrupt-names > > because Jonathan mentioned this would be nice to have [1] and it kind of > > fit well together with the BMC156 patch that needs the INT2 support. > > I actually just use the if (data->type == BOSCH_BMC156) part from > > PATCH 4/4 which does not depend on of_irq_get_byname(). > > Then I leave it to Jonathan and other maintainers. I'd be rather nervous about this one myself unless I have a test setup where I can poke all the paths. My current qemu hack setup doesn't do full enough ACPI so whilst I'd take a look at this myself it might take me a little while to hack in the ACPI tables needed to bring up a suitable bus on that device. I'll get to it if no one else does, but I'm not keen to block any drivers that just use the of route in the meantime. Should be easier to do a sweep of the ones Andy has highlighted + this one when we have the support ready. The patch looks fine to me otherwise. Thanks, Jonathan > > > [1]: https://lore.kernel.org/linux-iio/20210611185941.3487efc6@jic23-huawei/ > ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] iio: accel: bmc150: Make it possible to configure INT2 instead of INT1 2021-07-19 18:05 ` Andy Shevchenko 2021-07-19 18:36 ` Stephan Gerhold @ 2021-07-24 18:06 ` Jonathan Cameron 1 sibling, 0 replies; 27+ messages in thread From: Jonathan Cameron @ 2021-07-24 18:06 UTC (permalink / raw) To: Andy Shevchenko Cc: Stephan Gerhold, Linus Walleij, Lars-Peter Clausen, Rob Herring, linux-iio, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Hans de Goede, 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, 19 Jul 2021 21:05:48 +0300 Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Mon, Jul 19, 2021 at 8:29 PM Stephan Gerhold <stephan@gerhold.net> wrote: > > > > On Mon, Jul 19, 2021 at 07:19:05PM +0300, Andy Shevchenko wrote: > > > On Mon, Jul 19, 2021 at 6:11 PM Stephan Gerhold <stephan@gerhold.net> wrote: > > > > On Mon, Jul 19, 2021 at 06:01:01PM +0300, Andy Shevchenko wrote: > > > > > On Mon, Jul 19, 2021 at 5:07 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > > > > > On Mon, Jul 19, 2021 at 1:26 PM Stephan Gerhold <stephan@gerhold.net> wrote: > > ... > > > > > > > > #include <linux/acpi.h> > > > > > > > +#include <linux/of_irq.h> > > > > > > (...) > > > > > > > + irq_info = bmc150_accel_interrupts_int1; > > > > > > > + if (irq == of_irq_get_byname(dev->of_node, "INT2")) > > > > > > > + irq_info = bmc150_accel_interrupts_int2; > > > > > > > > > > > > This looks a bit DT-specific, but I don't see that ACPI has > > > > > > named IRQs so I don't know what to do about it either. > > > > > > > > > > Yeah, we only have so far the (de facto) established way of naming > > > > > GPIO based IRQs, and not IOxAPIC ones. > > > > > > > > > > > What does platform_get_irq_byname() do on ACPI systems? > > > > > > > > > > See above. > > > > > > > > > > > If there is no obvious fix I would leave it like this until the > > > > > > first ACPI used needing this comes along, but I think maybe > > > > > > Andy has suggestions. > > > > > > > > > > The platform_get_irq_byname() should do something similar that has > > > > > been done in platform_get_irq() WRT ACPI. > > > > > Here for sure the platform_get_irq_byname() or its optional variant > > > > > should be used. > > > > > > > > I don't think there is a platform device here, we only have the > > > > i2c_client or spi_device. That's why I didn't use > > > > platform_get_irq_byname(). :) > > > > > > > > Is there something equivalent for I2C/SPI drivers? > > > > > > Not yet. You probably need to supply some code there to allow > > > multi-IRQ devices (in resource provider agnostic way). > > > > > > You need to provide fwnode_get_irq_byname() to be similar with > > > https://elixir.bootlin.com/linux/latest/source/drivers/base/property.c#L1010 > > > > > > Then use it in the drivers. > > > > > > And/or integrate into frameworks somehow (something in between the > > > lines: https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-core-base.c#L461). > > > > > > > Well, I don't think anyone has an ACPI use case for this right now so > > it's probably better if this is done by someone who actually needs this > > and can test it somewhere. :) > > > > I actually just "copied" this approach from some other IIO drivers where > > this is done similarly (and additionally checked the source code to make > > sure this won't break anything for ACPI platforms). > > I see in today's Linux Next snapshot: > > drivers/iio/accel/fxls8962af-core.c:774: irq = > of_irq_get_byname(of_node, "INT2"); > drivers/iio/accel/mma8452.c:1616: irq2 = > of_irq_get_byname(client->dev.of_node, "INT2"); > drivers/iio/gyro/fxas21002c_core.c:834: irq1 = of_irq_get_byname(np, "INT1"); > drivers/iio/imu/adis16480.c:1265: irq = > of_irq_get_byname(of_node, adis16480_int_pin_names[i]); > drivers/iio/imu/bmi160/bmi160_core.c:655: irq = > of_irq_get_byname(of_node, "INT1"); > drivers/iio/imu/bmi160/bmi160_core.c:661: irq = > of_irq_get_byname(of_node, "INT2"); > > I believe we may stop distributing this and actually start using a > common API. I don't want this to be spread again over all IIO. Btw, I > have LSM9DS0, which supports two INT pins for IMU and currently it > uses hard coded pin mapping. I'm definitely keen to tidy this up, though I'd also rather not tie it to this particular series. > > Side note to Jonathan, I believe the below may be (lazily) converted > to fwnode / device properties APIs. Yup. I know we still have a bunch of these to tidy up. Might take a while to get to them though! > > drivers/iio/adc/ab8500-gpadc.c:1041: nchans = > of_get_available_child_count(np); > drivers/iio/adc/ad7124.c:747: st->num_channels = > of_get_available_child_count(np); > drivers/iio/adc/berlin2-adc.c:287: struct device_node *parent_np > = of_get_parent(pdev->dev.of_node); > drivers/iio/adc/qcom-pm8xxx-xoadc.c:830: adc->nchans = > of_get_available_child_count(np); > drivers/iio/adc/qcom-spmi-adc5.c:650: channel_name = of_get_property(node, > drivers/iio/adc/qcom-spmi-adc5.c:813: adc->nchannels = > of_get_available_child_count(node); > drivers/iio/adc/qcom-spmi-vadc.c:743: vadc->nchannels = > of_get_available_child_count(node); > drivers/iio/adc/stm32-adc.c:1630:static int > stm32_adc_of_get_resolution(struct iio_dev *indio_dev) > drivers/iio/adc/stm32-adc.c:1935: ret = > stm32_adc_of_get_resolution(indio_dev); > drivers/iio/adc/xilinx-xadc-core.c:1248: chan_node = > of_get_child_by_name(np, "xlnx,channels"); > drivers/iio/imu/adis16480.c:1293:static int > adis16480_of_get_ext_clk_pin(struct adis16480 *st, > drivers/iio/imu/adis16480.c:1328: pin = > adis16480_of_get_ext_clk_pin(st, of_node); > drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c:68: mux_node = > of_get_child_by_name(dev->of_node, "i2c-gate") > ; > drivers/iio/industrialio-core.c:1877: label = > of_get_property(indio_dev->dev.of_node, "label", NULL); > drivers/iio/inkern.c:228: if (np && !of_get_property(np, > "io-channel-ranges", NULL)) > drivers/iio/temperature/ltc2983.c:1275: st->num_channels = > of_get_available_child_count(dev->of_node); > > ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 4/4] iio: accel: bmc150: Add support for BMC156 2021-07-19 11:21 [PATCH 0/4] iio: accel: bmc150: Add support for INT2 and BMC156 Stephan Gerhold ` (2 preceding siblings ...) 2021-07-19 11:21 ` [PATCH 3/4] iio: accel: bmc150: Make it possible to configure INT2 instead of INT1 Stephan Gerhold @ 2021-07-19 11:21 ` Stephan Gerhold 2021-07-19 14:08 ` Linus Walleij 2021-07-24 16:12 ` Jonathan Cameron 2021-07-19 12:34 ` [PATCH 0/4] iio: accel: bmc150: Add support for INT2 and BMC156 Andy Shevchenko 4 siblings, 2 replies; 27+ messages in thread From: Stephan Gerhold @ 2021-07-19 11:21 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 was crazy and 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 Signed-off-by: Stephan Gerhold <stephan@gerhold.net> --- drivers/iio/accel/Kconfig | 5 +++-- drivers/iio/accel/bmc150-accel-core.c | 8 +++++--- drivers/iio/accel/bmc150-accel-i2c.c | 10 ++++++++-- drivers/iio/accel/bmc150-accel-spi.c | 10 +++++++++- drivers/iio/accel/bmc150-accel.h | 9 ++++++++- 5 files changed, 33 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..a5d321e878ef 100644 --- a/drivers/iio/accel/bmc150-accel-core.c +++ b/drivers/iio/accel/bmc150-accel-core.c @@ -553,7 +553,7 @@ 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 +1174,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 +1661,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 +1677,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..a3f4905e48a3 100644 --- a/drivers/iio/accel/bmc150-accel.h +++ b/drivers/iio/accel/bmc150-accel.h @@ -13,6 +13,11 @@ struct i2c_client; struct bmc150_accel_chip_info; struct bmc150_accel_interrupt_info; +enum bmc150_type { + BOSCH_UNKNOWN, + BOSCH_BMC156, +}; + struct bmc150_accel_interrupt { const struct bmc150_accel_interrupt_info *info; atomic_t users; @@ -62,6 +67,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 +75,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] 27+ messages in thread
* Re: [PATCH 4/4] iio: accel: bmc150: Add support for BMC156 2021-07-19 11:21 ` [PATCH 4/4] iio: accel: bmc150: Add support for BMC156 Stephan Gerhold @ 2021-07-19 14:08 ` Linus Walleij 2021-07-24 16:12 ` Jonathan Cameron 1 sibling, 0 replies; 27+ messages in thread From: Linus Walleij @ 2021-07-19 14:08 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, Jul 19, 2021 at 1:26 PM Stephan Gerhold <stephan@gerhold.net> wrote: > 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 was crazy and 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 > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/4] iio: accel: bmc150: Add support for BMC156 2021-07-19 11:21 ` [PATCH 4/4] iio: accel: bmc150: Add support for BMC156 Stephan Gerhold 2021-07-19 14:08 ` Linus Walleij @ 2021-07-24 16:12 ` Jonathan Cameron 2021-07-27 18:32 ` Stephan Gerhold 1 sibling, 1 reply; 27+ messages in thread From: Jonathan Cameron @ 2021-07-24 16:12 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, 19 Jul 2021 13:21:56 +0200 Stephan Gerhold <stephan@gerhold.net> wrote: > 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 was crazy and 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 > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> A few really minor things inline. Thanks, Jonathan > --- > drivers/iio/accel/Kconfig | 5 +++-- > drivers/iio/accel/bmc150-accel-core.c | 8 +++++--- > drivers/iio/accel/bmc150-accel-i2c.c | 10 ++++++++-- > drivers/iio/accel/bmc150-accel-spi.c | 10 +++++++++- > drivers/iio/accel/bmc150-accel.h | 9 ++++++++- > 5 files changed, 33 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..a5d321e878ef 100644 > --- a/drivers/iio/accel/bmc150-accel-core.c > +++ b/drivers/iio/accel/bmc150-accel-core.c > @@ -553,7 +553,7 @@ 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")) It is still preferred to keep line lengths under 80 chars unless it hurts readability to do so. So please wrap this one. > irq_info = bmc150_accel_interrupts_int2; > > for (i = 0; i < BMC150_ACCEL_INTERRUPTS; i++) > @@ -1174,7 +1174,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 +1661,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 +1677,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..a3f4905e48a3 100644 > --- a/drivers/iio/accel/bmc150-accel.h > +++ b/drivers/iio/accel/bmc150-accel.h > @@ -13,6 +13,11 @@ struct i2c_client; > struct bmc150_accel_chip_info; > struct bmc150_accel_interrupt_info; > > +enum bmc150_type { > + BOSCH_UNKNOWN, > + BOSCH_BMC156, Whilst we only need to distinguish this one at the moment, the unknown naming implies we don't know the type when often we actually do. Probably better to just have all the known device types in the enum and set in the id table. > +}; > + > struct bmc150_accel_interrupt { > const struct bmc150_accel_interrupt_info *info; > atomic_t users; > @@ -62,6 +67,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 +75,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; ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/4] iio: accel: bmc150: Add support for BMC156 2021-07-24 16:12 ` Jonathan Cameron @ 2021-07-27 18:32 ` Stephan Gerhold 2021-07-31 18:04 ` Jonathan Cameron 0 siblings, 1 reply; 27+ messages in thread From: Stephan Gerhold @ 2021-07-27 18:32 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 Hi Jonathan! On Sat, Jul 24, 2021 at 05:12:46PM +0100, Jonathan Cameron wrote: > On Mon, 19 Jul 2021 13:21:56 +0200 > Stephan Gerhold <stephan@gerhold.net> wrote: > > > 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 was crazy and 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 > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> > A few really minor things inline. > > Thanks, > > Jonathan > > > --- > > drivers/iio/accel/Kconfig | 5 +++-- > > drivers/iio/accel/bmc150-accel-core.c | 8 +++++--- > > drivers/iio/accel/bmc150-accel-i2c.c | 10 ++++++++-- > > drivers/iio/accel/bmc150-accel-spi.c | 10 +++++++++- > > drivers/iio/accel/bmc150-accel.h | 9 ++++++++- > > 5 files changed, 33 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..a5d321e878ef 100644 > > --- a/drivers/iio/accel/bmc150-accel-core.c > > +++ b/drivers/iio/accel/bmc150-accel-core.c > > @@ -553,7 +553,7 @@ 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")) > > It is still preferred to keep line lengths under 80 chars unless it hurts > readability to do so. So please wrap this one. > OK, will fix this in v2. :) > > irq_info = bmc150_accel_interrupts_int2; > > > > for (i = 0; i < BMC150_ACCEL_INTERRUPTS; i++) > > @@ -1174,7 +1174,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 +1661,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 +1677,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..a3f4905e48a3 100644 > > --- a/drivers/iio/accel/bmc150-accel.h > > +++ b/drivers/iio/accel/bmc150-accel.h > > @@ -13,6 +13,11 @@ struct i2c_client; > > struct bmc150_accel_chip_info; > > struct bmc150_accel_interrupt_info; > > > > +enum bmc150_type { > > + BOSCH_UNKNOWN, > > + BOSCH_BMC156, > Whilst we only need to distinguish this one at the moment, the unknown naming > implies we don't know the type when often we actually do. > Hm, actually this is exactly what I want to imply! We do have seemingly obvious names listed in the ID tables, but unfortunately I don't think we can assume them to be accurate. The original reason why we no longer rely on the type implied by the ID is that there are some ACPI devices that specify an ID like "BMA250E" when they actually have a BMA222E, see commit 0ad4bf37017 [1] ("iio:accel:bmc150-accel: Use the chip ID to detect sensor variant"). And this was the motivation for my commit c06a6aba6835 [2] ("iio: accel: bmc150: Drop misleading/duplicate chip identifiers"). I removed them because they were not used. Also, we cannot make use of them in the general case because they are not reliable thanks to those ACPI devices. We could perhaps add it only for the of_device_ids. However, even there it's easy to specify some similar compatible only because Bosch has so many compatible parts. For example, the device where these BMC156 changes were tested on was using "bosch,bmc150_accel" so far simply because this was working fine (without interrupts) and we weren't actually aware that it has a BMC156 instead of BMC150. Im my opinion, adding type information to all of them would imply that it can be used reliably, which is not the case unfortunately. Perhaps I should instead add a comment to this enum to make this more clear? What do you think? Thanks! Stephan [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0ad4bf37017621e25fe157fa095fd8849779a873 [2]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c06a6aba6835946bcccb9909c98ec110949ea630 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/4] iio: accel: bmc150: Add support for BMC156 2021-07-27 18:32 ` Stephan Gerhold @ 2021-07-31 18:04 ` Jonathan Cameron 0 siblings, 0 replies; 27+ messages in thread From: Jonathan Cameron @ 2021-07-31 18:04 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 Tue, 27 Jul 2021 20:32:12 +0200 Stephan Gerhold <stephan@gerhold.net> wrote: > Hi Jonathan! > > On Sat, Jul 24, 2021 at 05:12:46PM +0100, Jonathan Cameron wrote: > > On Mon, 19 Jul 2021 13:21:56 +0200 > > Stephan Gerhold <stephan@gerhold.net> wrote: > > > > > 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 was crazy and 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 > > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> > > A few really minor things inline. > > > > Thanks, > > > > Jonathan > > > > > --- > > > drivers/iio/accel/Kconfig | 5 +++-- > > > drivers/iio/accel/bmc150-accel-core.c | 8 +++++--- > > > drivers/iio/accel/bmc150-accel-i2c.c | 10 ++++++++-- > > > drivers/iio/accel/bmc150-accel-spi.c | 10 +++++++++- > > > drivers/iio/accel/bmc150-accel.h | 9 ++++++++- > > > 5 files changed, 33 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..a5d321e878ef 100644 > > > --- a/drivers/iio/accel/bmc150-accel-core.c > > > +++ b/drivers/iio/accel/bmc150-accel-core.c > > > @@ -553,7 +553,7 @@ 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")) > > > > It is still preferred to keep line lengths under 80 chars unless it hurts > > readability to do so. So please wrap this one. > > > > OK, will fix this in v2. :) > > > > irq_info = bmc150_accel_interrupts_int2; > > > > > > for (i = 0; i < BMC150_ACCEL_INTERRUPTS; i++) > > > @@ -1174,7 +1174,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 +1661,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 +1677,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..a3f4905e48a3 100644 > > > --- a/drivers/iio/accel/bmc150-accel.h > > > +++ b/drivers/iio/accel/bmc150-accel.h > > > @@ -13,6 +13,11 @@ struct i2c_client; > > > struct bmc150_accel_chip_info; > > > struct bmc150_accel_interrupt_info; > > > > > > +enum bmc150_type { > > > + BOSCH_UNKNOWN, > > > + BOSCH_BMC156, > > Whilst we only need to distinguish this one at the moment, the unknown naming > > implies we don't know the type when often we actually do. > > > > Hm, actually this is exactly what I want to imply! We do have seemingly > obvious names listed in the ID tables, but unfortunately I don't think > we can assume them to be accurate. > > The original reason why we no longer rely on the type implied by the ID > is that there are some ACPI devices that specify an ID like "BMA250E" > when they actually have a BMA222E, see commit 0ad4bf37017 [1] > ("iio:accel:bmc150-accel: Use the chip ID to detect sensor variant"). > > And this was the motivation for my commit c06a6aba6835 [2] > ("iio: accel: bmc150: Drop misleading/duplicate chip identifiers"). > I removed them because they were not used. Also, we cannot make use of > them in the general case because they are not reliable thanks to those > ACPI devices. Gah! > > We could perhaps add it only for the of_device_ids. However, even there > it's easy to specify some similar compatible only because Bosch has so > many compatible parts. For example, the device where these BMC156 > changes were tested on was using "bosch,bmc150_accel" so far simply > because this was working fine (without interrupts) and we weren't > actually aware that it has a BMC156 instead of BMC150. > > Im my opinion, adding type information to all of them would imply that > it can be used reliably, which is not the case unfortunately. Perhaps > I should instead add a comment to this enum to make this more clear? > > What do you think? A comment seems a good solution given the situation. Thanks, Jonathan > > Thanks! > Stephan > > [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0ad4bf37017621e25fe157fa095fd8849779a873 > [2]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c06a6aba6835946bcccb9909c98ec110949ea630 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/4] iio: accel: bmc150: Add support for INT2 and BMC156 2021-07-19 11:21 [PATCH 0/4] iio: accel: bmc150: Add support for INT2 and BMC156 Stephan Gerhold ` (3 preceding siblings ...) 2021-07-19 11:21 ` [PATCH 4/4] iio: accel: bmc150: Add support for BMC156 Stephan Gerhold @ 2021-07-19 12:34 ` Andy Shevchenko 2021-07-19 12:42 ` Stephan Gerhold 4 siblings, 1 reply; 27+ messages in thread From: Andy Shevchenko @ 2021-07-19 12:34 UTC (permalink / raw) To: Stephan Gerhold Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring, Linus Walleij, linux-iio, devicetree, Hans de Goede, ~postmarketos/upstreaming, Nikita Travkin On Mon, Jul 19, 2021 at 2:26 PM 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/ I forgot the story, but the series sounds to me like déjà-vu. Please, remind me if it was sent once before? If yes, then this one misses version bumping and/or changelog. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/4] iio: accel: bmc150: Add support for INT2 and BMC156 2021-07-19 12:34 ` [PATCH 0/4] iio: accel: bmc150: Add support for INT2 and BMC156 Andy Shevchenko @ 2021-07-19 12:42 ` Stephan Gerhold 0 siblings, 0 replies; 27+ messages in thread From: Stephan Gerhold @ 2021-07-19 12:42 UTC (permalink / raw) To: Andy Shevchenko Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring, Linus Walleij, linux-iio, devicetree, Hans de Goede, ~postmarketos/upstreaming, Nikita Travkin On Mon, Jul 19, 2021 at 03:34:50PM +0300, Andy Shevchenko wrote: > On Mon, Jul 19, 2021 at 2:26 PM 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/ > > I forgot the story, but the series sounds to me like déjà-vu. Please, > remind me if it was sent once before? If yes, then this one misses > version bumping and/or changelog. > Hm, no I didn't send this one before. :) Perhaps you are confusing it with the patch series I sent for BMA253 support recently [1] which is where I mentioned I would work on BMC156 support as well as follow-up series (see link above). :) Thanks! Stephan [1]: https://lore.kernel.org/linux-iio/20210611080903.14384-1-stephan@gerhold.net/ ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2021-07-31 18:01 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-19 11:21 [PATCH 0/4] iio: accel: bmc150: Add support for INT2 and BMC156 Stephan Gerhold 2021-07-19 11:21 ` [PATCH 1/4] dt-bindings: iio: accel: bma255: Add interrupt-names Stephan Gerhold 2021-07-19 13:57 ` Linus Walleij 2021-07-24 16:00 ` Jonathan Cameron 2021-07-19 11:21 ` [PATCH 2/4] dt-bindings: iio: accel: bma255: Add bosch,bmc156_accel Stephan Gerhold 2021-07-19 13:58 ` Linus Walleij 2021-07-24 16:03 ` Jonathan Cameron 2021-07-29 19:10 ` Rob Herring 2021-07-29 19:10 ` Rob Herring 2021-07-19 11:21 ` [PATCH 3/4] iio: accel: bmc150: Make it possible to configure INT2 instead of INT1 Stephan Gerhold 2021-07-19 14:07 ` Linus Walleij 2021-07-19 15:01 ` Andy Shevchenko 2021-07-19 15:10 ` Stephan Gerhold 2021-07-19 16:19 ` Andy Shevchenko 2021-07-19 17:26 ` Stephan Gerhold 2021-07-19 18:05 ` Andy Shevchenko 2021-07-19 18:36 ` Stephan Gerhold 2021-07-20 15:04 ` Andy Shevchenko 2021-07-24 16:19 ` Jonathan Cameron 2021-07-24 18:06 ` Jonathan Cameron 2021-07-19 11:21 ` [PATCH 4/4] iio: accel: bmc150: Add support for BMC156 Stephan Gerhold 2021-07-19 14:08 ` Linus Walleij 2021-07-24 16:12 ` Jonathan Cameron 2021-07-27 18:32 ` Stephan Gerhold 2021-07-31 18:04 ` Jonathan Cameron 2021-07-19 12:34 ` [PATCH 0/4] iio: accel: bmc150: Add support for INT2 and BMC156 Andy Shevchenko 2021-07-19 12:42 ` Stephan Gerhold
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).