[PATCH] iio: gyro: mpu3050: fix chip ID reading

[PATCH] iio: gyro: mpu3050: fix chip ID reading

[Sergey Larin]

According to the datasheet, the last bit of CHIP_ID register controls
I2C bus, and the first one is unused. Handle this correctly.

Signed-off-by: Sergey Larin <cerg2010cerg2010@mail.ru>
---
 drivers/iio/gyro/mpu3050-core.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
index 77fac81a3adc..5ddebede31a6 100644
--- a/drivers/iio/gyro/mpu3050-core.c
+++ b/drivers/iio/gyro/mpu3050-core.c
@@ -29,7 +29,8 @@
 
 #include "mpu3050.h"
 
-#define MPU3050_CHIP_ID		0x69
+#define MPU3050_CHIP_ID		0x68
+#define MPU3050_CHIP_ID_MASK	0x7E
 
 /*
  * Register map: anything suffixed *_H is a big-endian high byte and always
@@ -1176,8 +1177,9 @@ int mpu3050_common_probe(struct device *dev,
 		goto err_power_down;
 	}
 
-	if (val != MPU3050_CHIP_ID) {
-		dev_err(dev, "unsupported chip id %02x\n", (u8)val);
+	if ((val & MPU3050_CHIP_ID_MASK) != MPU3050_CHIP_ID) {
+		dev_err(dev, "unsupported chip id %02x\n",
+				(u8)(val & MPU3050_CHIP_ID_MASK));
 		ret = -ENODEV;
 		goto err_power_down;
 	}
-- 
2.21.0

Re: [PATCH] iio: gyro: mpu3050: fix chip ID reading

[Jonathan Cameron]

On Sat,  2 Mar 2019 19:54:55 +0300
Sergey Larin <cerg2010cerg2010@mail.ru> wrote:

> According to the datasheet, the last bit of CHIP_ID register controls
> I2C bus, and the first one is unused. Handle this correctly.
> 
> Signed-off-by: Sergey Larin <cerg2010cerg2010@mail.ru>
Fix certainly seems to be correct, but seeing as we only support i2c for
this chip and that I assume the bottom bit is actually set (though
reserved), currently this is a tidy up rather than a fix.

Hence I'll take this the slow way rather than quickly as I would
a fix with actual effect.

Let me know if I've missed something!

Jonathan

> ---
>  drivers/iio/gyro/mpu3050-core.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
> index 77fac81a3adc..5ddebede31a6 100644
> --- a/drivers/iio/gyro/mpu3050-core.c
> +++ b/drivers/iio/gyro/mpu3050-core.c
> @@ -29,7 +29,8 @@
>  
>  #include "mpu3050.h"
>  
> -#define MPU3050_CHIP_ID		0x69
> +#define MPU3050_CHIP_ID		0x68
> +#define MPU3050_CHIP_ID_MASK	0x7E
>  
>  /*
>   * Register map: anything suffixed *_H is a big-endian high byte and always
> @@ -1176,8 +1177,9 @@ int mpu3050_common_probe(struct device *dev,
>  		goto err_power_down;
>  	}
>  
> -	if (val != MPU3050_CHIP_ID) {
> -		dev_err(dev, "unsupported chip id %02x\n", (u8)val);
> +	if ((val & MPU3050_CHIP_ID_MASK) != MPU3050_CHIP_ID) {
> +		dev_err(dev, "unsupported chip id %02x\n",
> +				(u8)(val & MPU3050_CHIP_ID_MASK));
>  		ret = -ENODEV;
>  		goto err_power_down;
>  	}

Re: [PATCH] iio: gyro: mpu3050: fix chip ID reading

[Sergey Larin]

On Sat, Mar 02, 2019 at 06:08:24PM +0000, Jonathan Cameron wrote:
> On Sat,  2 Mar 2019 19:54:55 +0300
> Sergey Larin <cerg2010cerg2010@mail.ru> wrote:
> 
> > According to the datasheet, the last bit of CHIP_ID register controls
> > I2C bus, and the first one is unused. Handle this correctly.
> > 
> > Signed-off-by: Sergey Larin <cerg2010cerg2010@mail.ru>
> Fix certainly seems to be correct, but seeing as we only support i2c for
> this chip and that I assume the bottom bit is actually set (though
> reserved), currently this is a tidy up rather than a fix.
> 
> Hence I'll take this the slow way rather than quickly as I would
> a fix with actual effect.
> 
> Let me know if I've missed something!
> 
> Jonathan

That's actually a fix for me. On my device, the bottom bit is set so driver
reports an error. With this patch, driver probes OK, prints chip info to
dmesg (die ID, revision, etc) and reports the data to sysfs, which looks
correct.

> 
> > ---
> >  drivers/iio/gyro/mpu3050-core.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
> > index 77fac81a3adc..5ddebede31a6 100644
> > --- a/drivers/iio/gyro/mpu3050-core.c
> > +++ b/drivers/iio/gyro/mpu3050-core.c
> > @@ -29,7 +29,8 @@
> >  
> >  #include "mpu3050.h"
> >  
> > -#define MPU3050_CHIP_ID		0x69
> > +#define MPU3050_CHIP_ID		0x68
> > +#define MPU3050_CHIP_ID_MASK	0x7E
> >  
> >  /*
> >   * Register map: anything suffixed *_H is a big-endian high byte and always
> > @@ -1176,8 +1177,9 @@ int mpu3050_common_probe(struct device *dev,
> >  		goto err_power_down;
> >  	}
> >  
> > -	if (val != MPU3050_CHIP_ID) {
> > -		dev_err(dev, "unsupported chip id %02x\n", (u8)val);
> > +	if ((val & MPU3050_CHIP_ID_MASK) != MPU3050_CHIP_ID) {
> > +		dev_err(dev, "unsupported chip id %02x\n",
> > +				(u8)(val & MPU3050_CHIP_ID_MASK));
> >  		ret = -ENODEV;
> >  		goto err_power_down;
> >  	}
> 

Re: [PATCH] iio: gyro: mpu3050: fix chip ID reading

[Linus Walleij]

On Sat, Mar 2, 2019 at 5:55 PM Sergey Larin <cerg2010cerg2010@mail.ru> wrote:

> According to the datasheet, the last bit of CHIP_ID register controls
> I2C bus, and the first one is unused. Handle this correctly.
>
> Signed-off-by: Sergey Larin <cerg2010cerg2010@mail.ru>

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

Yours,
Linus Walleij

Re: [PATCH] iio: gyro: mpu3050: fix chip ID reading

[Jonathan Cameron]

On Sat, 2 Mar 2019 21:48:30 +0300
Sergey Larin <cerg2010cerg2010@mail.ru> wrote:

> On Sat, Mar 02, 2019 at 06:08:24PM +0000, Jonathan Cameron wrote:
> > On Sat,  2 Mar 2019 19:54:55 +0300
> > Sergey Larin <cerg2010cerg2010@mail.ru> wrote:
> >   
> > > According to the datasheet, the last bit of CHIP_ID register controls
> > > I2C bus, and the first one is unused. Handle this correctly.
> > > 
> > > Signed-off-by: Sergey Larin <cerg2010cerg2010@mail.ru>  
> > Fix certainly seems to be correct, but seeing as we only support i2c for
> > this chip and that I assume the bottom bit is actually set (though
> > reserved), currently this is a tidy up rather than a fix.
> > 
> > Hence I'll take this the slow way rather than quickly as I would
> > a fix with actual effect.
> > 
> > Let me know if I've missed something!
> > 
> > Jonathan  
> 
> That's actually a fix for me. On my device, the bottom bit is set so driver
> reports an error. With this patch, driver probes OK, prints chip info to
> dmesg (die ID, revision, etc) and reports the data to sysfs, which looks
> correct.
Ah fair enough. Moved to the fixes-togreg branch with a note added
to the description that there are real parts out there where this matters.

Also added Linus' tag.

This will upstream sometime after rc1 in a few weeks time.

Thanks,

Jonathan
> 
> >   
> > > ---
> > >  drivers/iio/gyro/mpu3050-core.c | 8 +++++---
> > >  1 file changed, 5 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
> > > index 77fac81a3adc..5ddebede31a6 100644
> > > --- a/drivers/iio/gyro/mpu3050-core.c
> > > +++ b/drivers/iio/gyro/mpu3050-core.c
> > > @@ -29,7 +29,8 @@
> > >  
> > >  #include "mpu3050.h"
> > >  
> > > -#define MPU3050_CHIP_ID		0x69
> > > +#define MPU3050_CHIP_ID		0x68
> > > +#define MPU3050_CHIP_ID_MASK	0x7E
> > >  
> > >  /*
> > >   * Register map: anything suffixed *_H is a big-endian high byte and always
> > > @@ -1176,8 +1177,9 @@ int mpu3050_common_probe(struct device *dev,
> > >  		goto err_power_down;
> > >  	}
> > >  
> > > -	if (val != MPU3050_CHIP_ID) {
> > > -		dev_err(dev, "unsupported chip id %02x\n", (u8)val);
> > > +	if ((val & MPU3050_CHIP_ID_MASK) != MPU3050_CHIP_ID) {
> > > +		dev_err(dev, "unsupported chip id %02x\n",
> > > +				(u8)(val & MPU3050_CHIP_ID_MASK));
> > >  		ret = -ENODEV;
> > >  		goto err_power_down;
> > >  	}  
> >