diff mbox series

dt-bindings: iio: accel: bma255: Fix bmc150/bmi055 compatible

Message ID 20201202083551.7753-1-stephan@gerhold.net (mailing list archive)
State New, archived
Headers show
Series dt-bindings: iio: accel: bma255: Fix bmc150/bmi055 compatible | expand

Commit Message

Stephan Gerhold Dec. 2, 2020, 8:35 a.m. UTC
The bmc150-accel-i2c.c driver has an "_accel" suffix for the
compatibles of BMC150 and BMI055. This is necessary because BMC150
contains both accelerometer (bosch,bmc150_accel) and magnetometer
(bosch,bmc150_magn) and therefore "bosch,bmc150" would be ambiguous.

However, the binding documentation suggests using "bosch,bmc150".
Add the "_accel" suffix for BMC150 and BMI055 so the binding docs
match what is expected by the driver.

Cc: Linus Walleij <linus.walleij@linaro.org>
Fixes: 496a39526fce8 ("iio: accel: bmc150-accel: Add DT bindings")
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Linus Walleij Dec. 2, 2020, 12:08 p.m. UTC | #1
On Wed, Dec 2, 2020 at 9:36 AM Stephan Gerhold <stephan@gerhold.net> wrote:

> The bmc150-accel-i2c.c driver has an "_accel" suffix for the
> compatibles of BMC150 and BMI055. This is necessary because BMC150
> contains both accelerometer (bosch,bmc150_accel) and magnetometer
> (bosch,bmc150_magn) and therefore "bosch,bmc150" would be ambiguous.
>
> However, the binding documentation suggests using "bosch,bmc150".
> Add the "_accel" suffix for BMC150 and BMI055 so the binding docs
> match what is expected by the driver.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Fixes: 496a39526fce8 ("iio: accel: bmc150-accel: Add DT bindings")
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>

I see this pattern elsewhere so by tradition:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

I suppose this is one of those situations where the two parts of the
component are on the same physical I2C bus, and phsycially inside
the same package, but accessed at two different I2C addresses?

These components are kind of ambiguous by nature. Technically
both devices could have the same compatible (by the label on the
package) but then we would need some other property on the node
to say which compatible is for which part of the component,
so tagging on "_function" like bmc150_accel and bmc150_magn
is one way to solve this, and I don't know anything better.

Yours,
Linus Walleij
Stephan Gerhold Dec. 2, 2020, 2:07 p.m. UTC | #2
On Wed, Dec 02, 2020 at 01:08:57PM +0100, Linus Walleij wrote:
> On Wed, Dec 2, 2020 at 9:36 AM Stephan Gerhold <stephan@gerhold.net> wrote:
> 
> > The bmc150-accel-i2c.c driver has an "_accel" suffix for the
> > compatibles of BMC150 and BMI055. This is necessary because BMC150
> > contains both accelerometer (bosch,bmc150_accel) and magnetometer
> > (bosch,bmc150_magn) and therefore "bosch,bmc150" would be ambiguous.
> >
> > However, the binding documentation suggests using "bosch,bmc150".
> > Add the "_accel" suffix for BMC150 and BMI055 so the binding docs
> > match what is expected by the driver.
> >
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Fixes: 496a39526fce8 ("iio: accel: bmc150-accel: Add DT bindings")
> > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> 
> I see this pattern elsewhere so by tradition:
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> 
> I suppose this is one of those situations where the two parts of the
> component are on the same physical I2C bus, and phsycially inside
> the same package, but accessed at two different I2C addresses?
> 

Yep, it looks like this (from
arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi):

&blsp_i2c2 {
	status = "okay";

	accelerometer@10 {
		compatible = "bosch,bmc150_accel";
		reg = <0x10>;
		interrupt-parent = <&msmgpio>;
		interrupts = <115 IRQ_TYPE_EDGE_RISING>;
	};

	magnetometer@12 {
		compatible = "bosch,bmc150_magn";
		reg = <0x12>;
	};
};

They look pretty much like separate components in the device tree.

> These components are kind of ambiguous by nature. Technically
> both devices could have the same compatible (by the label on the
> package) but then we would need some other property on the node
> to say which compatible is for which part of the component,
> so tagging on "_function" like bmc150_accel and bmc150_magn
> is one way to solve this, and I don't know anything better.
> 

The _accel and _magn compatibles are also actively used already, so
unless there is a significantly better option I think it's better to
keep existing uses working.

Thanks!
Stephan
Rob Herring (Arm) Dec. 9, 2020, 6:17 p.m. UTC | #3
On Wed, 02 Dec 2020 09:35:51 +0100, Stephan Gerhold wrote:
> The bmc150-accel-i2c.c driver has an "_accel" suffix for the
> compatibles of BMC150 and BMI055. This is necessary because BMC150
> contains both accelerometer (bosch,bmc150_accel) and magnetometer
> (bosch,bmc150_magn) and therefore "bosch,bmc150" would be ambiguous.
> 
> However, the binding documentation suggests using "bosch,bmc150".
> Add the "_accel" suffix for BMC150 and BMI055 so the binding docs
> match what is expected by the driver.
> 
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Fixes: 496a39526fce8 ("iio: accel: bmc150-accel: Add DT bindings")
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
>  Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>
Jonathan Cameron Dec. 13, 2020, 1:25 p.m. UTC | #4
On Wed, 9 Dec 2020 12:17:39 -0600
Rob Herring <robh@kernel.org> wrote:

> On Wed, 02 Dec 2020 09:35:51 +0100, Stephan Gerhold wrote:
> > The bmc150-accel-i2c.c driver has an "_accel" suffix for the
> > compatibles of BMC150 and BMI055. This is necessary because BMC150
> > contains both accelerometer (bosch,bmc150_accel) and magnetometer
> > (bosch,bmc150_magn) and therefore "bosch,bmc150" would be ambiguous.
> > 
> > However, the binding documentation suggests using "bosch,bmc150".
> > Add the "_accel" suffix for BMC150 and BMI055 so the binding docs
> > match what is expected by the driver.
> > 
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Fixes: 496a39526fce8 ("iio: accel: bmc150-accel: Add DT bindings")
> > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > ---
> >  Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >   
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
Applied to the fixes-togreg branch of iio.git which is now based on stuff
queued up for the merge window. I'll send a pull not long after rc1.

Thanks,

Jonathan
Jonathan Cameron Jan. 14, 2021, 8:59 p.m. UTC | #5
On Sun, 13 Dec 2020 13:25:14 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Wed, 9 Dec 2020 12:17:39 -0600
> Rob Herring <robh@kernel.org> wrote:
> 
> > On Wed, 02 Dec 2020 09:35:51 +0100, Stephan Gerhold wrote:  
> > > The bmc150-accel-i2c.c driver has an "_accel" suffix for the
> > > compatibles of BMC150 and BMI055. This is necessary because BMC150
> > > contains both accelerometer (bosch,bmc150_accel) and magnetometer
> > > (bosch,bmc150_magn) and therefore "bosch,bmc150" would be ambiguous.
> > > 
> > > However, the binding documentation suggests using "bosch,bmc150".
> > > Add the "_accel" suffix for BMC150 and BMI055 so the binding docs
> > > match what is expected by the driver.
> > > 
> > > Cc: Linus Walleij <linus.walleij@linaro.org>
> > > Fixes: 496a39526fce8 ("iio: accel: bmc150-accel: Add DT bindings")
> > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > > ---
> > >  Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >     
> > 
> > Reviewed-by: Rob Herring <robh@kernel.org>  
> Applied to the fixes-togreg branch of iio.git which is now based on stuff
> queued up for the merge window. I'll send a pull not long after rc1.
> 

Not sure why, but the Fixes tag above is invalid. 

As a result I need to rebase this branch anyway so I've also brought it forward
to staging/staging-linus as of today.

Hopefully I'll get pull request out tomorrow.  Sorry for the delay!

Jonathan


> Thanks,
> 
> Jonathan
> 
>
Stephan Gerhold Jan. 15, 2021, 8:37 a.m. UTC | #6
On Thu, Jan 14, 2021 at 08:59:37PM +0000, Jonathan Cameron wrote:
> On Sun, 13 Dec 2020 13:25:14 +0000
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > On Wed, 9 Dec 2020 12:17:39 -0600
> > Rob Herring <robh@kernel.org> wrote:
> > 
> > > On Wed, 02 Dec 2020 09:35:51 +0100, Stephan Gerhold wrote:  
> > > > The bmc150-accel-i2c.c driver has an "_accel" suffix for the
> > > > compatibles of BMC150 and BMI055. This is necessary because BMC150
> > > > contains both accelerometer (bosch,bmc150_accel) and magnetometer
> > > > (bosch,bmc150_magn) and therefore "bosch,bmc150" would be ambiguous.
> > > > 
> > > > However, the binding documentation suggests using "bosch,bmc150".
> > > > Add the "_accel" suffix for BMC150 and BMI055 so the binding docs
> > > > match what is expected by the driver.
> > > > 
> > > > Cc: Linus Walleij <linus.walleij@linaro.org>
> > > > Fixes: 496a39526fce8 ("iio: accel: bmc150-accel: Add DT bindings")
> > > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > > > ---
> > > >  Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >     
> > > 
> > > Reviewed-by: Rob Herring <robh@kernel.org>  
> > Applied to the fixes-togreg branch of iio.git which is now based on stuff
> > queued up for the merge window. I'll send a pull not long after rc1.
> > 
> 
> Not sure why, but the Fixes tag above is invalid. 
> 

I think we can call this a "race condition" :)

I sent the patch on Dec 02 and you rebased the patch on Dec 03 for
"iio-for-5.11b-take2" (because some sign offs were missing there).
My patch simply refers to the old commit hash.

Stephan
Jonathan Cameron Jan. 15, 2021, 1:56 p.m. UTC | #7
On Fri, 15 Jan 2021 09:37:42 +0100
Stephan Gerhold <stephan@gerhold.net> wrote:

> On Thu, Jan 14, 2021 at 08:59:37PM +0000, Jonathan Cameron wrote:
> > On Sun, 13 Dec 2020 13:25:14 +0000
> > Jonathan Cameron <jic23@kernel.org> wrote:
> >   
> > > On Wed, 9 Dec 2020 12:17:39 -0600
> > > Rob Herring <robh@kernel.org> wrote:
> > >   
> > > > On Wed, 02 Dec 2020 09:35:51 +0100, Stephan Gerhold wrote:    
> > > > > The bmc150-accel-i2c.c driver has an "_accel" suffix for the
> > > > > compatibles of BMC150 and BMI055. This is necessary because BMC150
> > > > > contains both accelerometer (bosch,bmc150_accel) and magnetometer
> > > > > (bosch,bmc150_magn) and therefore "bosch,bmc150" would be ambiguous.
> > > > > 
> > > > > However, the binding documentation suggests using "bosch,bmc150".
> > > > > Add the "_accel" suffix for BMC150 and BMI055 so the binding docs
> > > > > match what is expected by the driver.
> > > > > 
> > > > > Cc: Linus Walleij <linus.walleij@linaro.org>
> > > > > Fixes: 496a39526fce8 ("iio: accel: bmc150-accel: Add DT bindings")
> > > > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > > > > ---
> > > > >  Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml | 4 ++--
> > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > >       
> > > > 
> > > > Reviewed-by: Rob Herring <robh@kernel.org>    
> > > Applied to the fixes-togreg branch of iio.git which is now based on stuff
> > > queued up for the merge window. I'll send a pull not long after rc1.
> > >   
> > 
> > Not sure why, but the Fixes tag above is invalid. 
> >   
> 
> I think we can call this a "race condition" :)
> 
> I sent the patch on Dec 02 and you rebased the patch on Dec 03 for
> "iio-for-5.11b-take2" (because some sign offs were missing there).
> My patch simply refers to the old commit hash.
> 
> Stephan

That would do it! :)

I suspected as much but was being lazy so didn't bother looking :)

Jonathan
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
index 6eef3480ea8f..c2efbb813ca2 100644
--- a/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
+++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma255.yaml
@@ -16,8 +16,8 @@  description:
 properties:
   compatible:
     enum:
-      - bosch,bmc150
-      - bosch,bmi055
+      - bosch,bmc150_accel
+      - bosch,bmi055_accel
       - bosch,bma255
       - bosch,bma250e
       - bosch,bma222