linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] iio: imu: st_lsm6dsx: Fix selection of ST_LSM6DS3_ID
@ 2019-12-09 17:05 Stephan Gerhold
  2019-12-09 21:48 ` Lorenzo Bianconi
  2019-12-15 17:23 ` Jonathan Cameron
  0 siblings, 2 replies; 8+ messages in thread
From: Stephan Gerhold @ 2019-12-09 17:05 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lorenzo Bianconi, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, linux-iio, Stephan Gerhold

At the moment, attempting to probe a device with ST_LSM6DS3_ID
(e.g. using the st,lsm6ds3 compatible) fails with:

    st_lsm6dsx_i2c 1-006b: unsupported whoami [69]

... even though 0x69 is the whoami listed for ST_LSM6DS3_ID.

This happens because st_lsm6dsx_check_whoami() also attempts
to match unspecified (zero-initialized) entries in the "id" array.
ST_LSM6DS3_ID = 0 will therefore match any entry in
st_lsm6dsx_sensor_settings (here: the first), because none of them
actually have all 12 entries listed in the "id" array.

Avoid this by additionally checking if "name" is set,
which is only set for valid entries in the "id" array.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index a7d40c02ce6b..b921dd9e108f 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -1301,7 +1301,8 @@ static int st_lsm6dsx_check_whoami(struct st_lsm6dsx_hw *hw, int id,
 
 	for (i = 0; i < ARRAY_SIZE(st_lsm6dsx_sensor_settings); i++) {
 		for (j = 0; j < ST_LSM6DSX_MAX_ID; j++) {
-			if (id == st_lsm6dsx_sensor_settings[i].id[j].hw_id)
+			if (st_lsm6dsx_sensor_settings[i].id[j].name &&
+			    id == st_lsm6dsx_sensor_settings[i].id[j].hw_id)
 				break;
 		}
 		if (j < ST_LSM6DSX_MAX_ID)
-- 
2.24.0


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

* Re: [PATCH] iio: imu: st_lsm6dsx: Fix selection of ST_LSM6DS3_ID
  2019-12-09 17:05 [PATCH] iio: imu: st_lsm6dsx: Fix selection of ST_LSM6DS3_ID Stephan Gerhold
@ 2019-12-09 21:48 ` Lorenzo Bianconi
  2019-12-10  8:22   ` Stephan Gerhold
  2019-12-15 17:23 ` Jonathan Cameron
  1 sibling, 1 reply; 8+ messages in thread
From: Lorenzo Bianconi @ 2019-12-09 21:48 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: Jonathan Cameron, Lorenzo Bianconi, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio

[-- Attachment #1: Type: text/plain, Size: 1740 bytes --]

> At the moment, attempting to probe a device with ST_LSM6DS3_ID
> (e.g. using the st,lsm6ds3 compatible) fails with:
> 
>     st_lsm6dsx_i2c 1-006b: unsupported whoami [69]
> 
> ... even though 0x69 is the whoami listed for ST_LSM6DS3_ID.
> 

Hi Stephan,

thx for working on this. I guess we can skip 'void' iterations defining the
array real size, do you agree?

Regards,
Lorenzo

> This happens because st_lsm6dsx_check_whoami() also attempts
> to match unspecified (zero-initialized) entries in the "id" array.
> ST_LSM6DS3_ID = 0 will therefore match any entry in
> st_lsm6dsx_sensor_settings (here: the first), because none of them
> actually have all 12 entries listed in the "id" array.
> 
> Avoid this by additionally checking if "name" is set,
> which is only set for valid entries in the "id" array.
> 
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index a7d40c02ce6b..b921dd9e108f 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -1301,7 +1301,8 @@ static int st_lsm6dsx_check_whoami(struct st_lsm6dsx_hw *hw, int id,
>  
>  	for (i = 0; i < ARRAY_SIZE(st_lsm6dsx_sensor_settings); i++) {
>  		for (j = 0; j < ST_LSM6DSX_MAX_ID; j++) {
> -			if (id == st_lsm6dsx_sensor_settings[i].id[j].hw_id)
> +			if (st_lsm6dsx_sensor_settings[i].id[j].name &&
> +			    id == st_lsm6dsx_sensor_settings[i].id[j].hw_id)
>  				break;
>  		}
>  		if (j < ST_LSM6DSX_MAX_ID)
> -- 
> 2.24.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH] iio: imu: st_lsm6dsx: Fix selection of ST_LSM6DS3_ID
  2019-12-09 21:48 ` Lorenzo Bianconi
@ 2019-12-10  8:22   ` Stephan Gerhold
  2019-12-11 14:10     ` Lorenzo Bianconi
  0 siblings, 1 reply; 8+ messages in thread
From: Stephan Gerhold @ 2019-12-10  8:22 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: Jonathan Cameron, Lorenzo Bianconi, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio

On Mon, Dec 09, 2019 at 10:48:52PM +0100, Lorenzo Bianconi wrote:
> > At the moment, attempting to probe a device with ST_LSM6DS3_ID
> > (e.g. using the st,lsm6ds3 compatible) fails with:
> > 
> >     st_lsm6dsx_i2c 1-006b: unsupported whoami [69]
> > 
> > ... even though 0x69 is the whoami listed for ST_LSM6DS3_ID.
> > 
> 
> Hi Stephan,
> 
> thx for working on this. I guess we can skip 'void' iterations defining the
> array real size, do you agree?
> 

I'm not sure I understand you correctly.
Do you mean having something like:

struct st_lsm6dsx_settings {
	u8 wai;
	/* ... */
	struct {
		enum st_lsm6dsx_hw_id hw_id;
		const char *name;
	} id[ST_LSM6DSX_MAX_ID];
	int id_num; /* Add this field */
	/* ... */
};

And then change the loop to use .id_num instead?

I think it is pretty easy to forget to update "id_num"
when adding new entries. Right now there is no need to worry about that.

Thanks,
Stephan

> Regards,
> Lorenzo
> 
> > This happens because st_lsm6dsx_check_whoami() also attempts
> > to match unspecified (zero-initialized) entries in the "id" array.
> > ST_LSM6DS3_ID = 0 will therefore match any entry in
> > st_lsm6dsx_sensor_settings (here: the first), because none of them
> > actually have all 12 entries listed in the "id" array.
> > 
> > Avoid this by additionally checking if "name" is set,
> > which is only set for valid entries in the "id" array.
> > 
> > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > ---
> >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > index a7d40c02ce6b..b921dd9e108f 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > @@ -1301,7 +1301,8 @@ static int st_lsm6dsx_check_whoami(struct st_lsm6dsx_hw *hw, int id,
> >  
> >  	for (i = 0; i < ARRAY_SIZE(st_lsm6dsx_sensor_settings); i++) {
> >  		for (j = 0; j < ST_LSM6DSX_MAX_ID; j++) {
> > -			if (id == st_lsm6dsx_sensor_settings[i].id[j].hw_id)
> > +			if (st_lsm6dsx_sensor_settings[i].id[j].name &&
> > +			    id == st_lsm6dsx_sensor_settings[i].id[j].hw_id)
> >  				break;
> >  		}
> >  		if (j < ST_LSM6DSX_MAX_ID)
> > -- 
> > 2.24.0
> > 



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

* Re: [PATCH] iio: imu: st_lsm6dsx: Fix selection of ST_LSM6DS3_ID
  2019-12-10  8:22   ` Stephan Gerhold
@ 2019-12-11 14:10     ` Lorenzo Bianconi
  0 siblings, 0 replies; 8+ messages in thread
From: Lorenzo Bianconi @ 2019-12-11 14:10 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: Jonathan Cameron, Lorenzo Bianconi, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio

[-- Attachment #1: Type: text/plain, Size: 2717 bytes --]

> On Mon, Dec 09, 2019 at 10:48:52PM +0100, Lorenzo Bianconi wrote:
> > > At the moment, attempting to probe a device with ST_LSM6DS3_ID
> > > (e.g. using the st,lsm6ds3 compatible) fails with:
> > > 
> > >     st_lsm6dsx_i2c 1-006b: unsupported whoami [69]
> > > 
> > > ... even though 0x69 is the whoami listed for ST_LSM6DS3_ID.
> > > 
> > 
> > Hi Stephan,
> > 
> > thx for working on this. I guess we can skip 'void' iterations defining the
> > array real size, do you agree?
> > 
> 
> I'm not sure I understand you correctly.
> Do you mean having something like:
> 
> struct st_lsm6dsx_settings {
> 	u8 wai;
> 	/* ... */
> 	struct {
> 		enum st_lsm6dsx_hw_id hw_id;
> 		const char *name;
> 	} id[ST_LSM6DSX_MAX_ID];
> 	int id_num; /* Add this field */
> 	/* ... */
> };
> 
> And then change the loop to use .id_num instead?
> 
> I think it is pretty easy to forget to update "id_num"
> when adding new entries. Right now there is no need to worry about that.

Uhm..this approach is even more safe if someone forgets to set name for a given
device. So for the patch:

Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>

Regards,
Lorenzo

> 
> Thanks,
> Stephan
> 
> > Regards,
> > Lorenzo
> > 
> > > This happens because st_lsm6dsx_check_whoami() also attempts
> > > to match unspecified (zero-initialized) entries in the "id" array.
> > > ST_LSM6DS3_ID = 0 will therefore match any entry in
> > > st_lsm6dsx_sensor_settings (here: the first), because none of them
> > > actually have all 12 entries listed in the "id" array.
> > > 
> > > Avoid this by additionally checking if "name" is set,
> > > which is only set for valid entries in the "id" array.
> > > 
> > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > > ---
> > >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > > index a7d40c02ce6b..b921dd9e108f 100644
> > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > > @@ -1301,7 +1301,8 @@ static int st_lsm6dsx_check_whoami(struct st_lsm6dsx_hw *hw, int id,
> > >  
> > >  	for (i = 0; i < ARRAY_SIZE(st_lsm6dsx_sensor_settings); i++) {
> > >  		for (j = 0; j < ST_LSM6DSX_MAX_ID; j++) {
> > > -			if (id == st_lsm6dsx_sensor_settings[i].id[j].hw_id)
> > > +			if (st_lsm6dsx_sensor_settings[i].id[j].name &&
> > > +			    id == st_lsm6dsx_sensor_settings[i].id[j].hw_id)
> > >  				break;
> > >  		}
> > >  		if (j < ST_LSM6DSX_MAX_ID)
> > > -- 
> > > 2.24.0
> > > 
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH] iio: imu: st_lsm6dsx: Fix selection of ST_LSM6DS3_ID
  2019-12-09 17:05 [PATCH] iio: imu: st_lsm6dsx: Fix selection of ST_LSM6DS3_ID Stephan Gerhold
  2019-12-09 21:48 ` Lorenzo Bianconi
@ 2019-12-15 17:23 ` Jonathan Cameron
  2020-01-13 22:12   ` Jonathan Cameron
  1 sibling, 1 reply; 8+ messages in thread
From: Jonathan Cameron @ 2019-12-15 17:23 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: Lorenzo Bianconi, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, linux-iio

On Mon,  9 Dec 2019 18:05:41 +0100
Stephan Gerhold <stephan@gerhold.net> wrote:

> At the moment, attempting to probe a device with ST_LSM6DS3_ID
> (e.g. using the st,lsm6ds3 compatible) fails with:
> 
>     st_lsm6dsx_i2c 1-006b: unsupported whoami [69]
> 
> ... even though 0x69 is the whoami listed for ST_LSM6DS3_ID.
> 
> This happens because st_lsm6dsx_check_whoami() also attempts
> to match unspecified (zero-initialized) entries in the "id" array.
> ST_LSM6DS3_ID = 0 will therefore match any entry in
> st_lsm6dsx_sensor_settings (here: the first), because none of them
> actually have all 12 entries listed in the "id" array.
> 
> Avoid this by additionally checking if "name" is set,
> which is only set for valid entries in the "id" array.
> 
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
Definitely sounds like this wants backporting. 

If you can figure out a fixes tag that would be great!

Thanks,

Jonathan

> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index a7d40c02ce6b..b921dd9e108f 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -1301,7 +1301,8 @@ static int st_lsm6dsx_check_whoami(struct st_lsm6dsx_hw *hw, int id,
>  
>  	for (i = 0; i < ARRAY_SIZE(st_lsm6dsx_sensor_settings); i++) {
>  		for (j = 0; j < ST_LSM6DSX_MAX_ID; j++) {
> -			if (id == st_lsm6dsx_sensor_settings[i].id[j].hw_id)
> +			if (st_lsm6dsx_sensor_settings[i].id[j].name &&
> +			    id == st_lsm6dsx_sensor_settings[i].id[j].hw_id)
>  				break;
>  		}
>  		if (j < ST_LSM6DSX_MAX_ID)


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

* Re: [PATCH] iio: imu: st_lsm6dsx: Fix selection of ST_LSM6DS3_ID
  2019-12-15 17:23 ` Jonathan Cameron
@ 2020-01-13 22:12   ` Jonathan Cameron
  2020-01-14  9:06     ` Stephan Gerhold
  0 siblings, 1 reply; 8+ messages in thread
From: Jonathan Cameron @ 2020-01-13 22:12 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: Lorenzo Bianconi, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, linux-iio

On Sun, 15 Dec 2019 17:23:42 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Mon,  9 Dec 2019 18:05:41 +0100
> Stephan Gerhold <stephan@gerhold.net> wrote:
> 
> > At the moment, attempting to probe a device with ST_LSM6DS3_ID
> > (e.g. using the st,lsm6ds3 compatible) fails with:
> > 
> >     st_lsm6dsx_i2c 1-006b: unsupported whoami [69]
> > 
> > ... even though 0x69 is the whoami listed for ST_LSM6DS3_ID.
> > 
> > This happens because st_lsm6dsx_check_whoami() also attempts
> > to match unspecified (zero-initialized) entries in the "id" array.
> > ST_LSM6DS3_ID = 0 will therefore match any entry in
> > st_lsm6dsx_sensor_settings (here: the first), because none of them
> > actually have all 12 entries listed in the "id" array.
> > 
> > Avoid this by additionally checking if "name" is set,
> > which is only set for valid entries in the "id" array.
> > 
> > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>  
> Definitely sounds like this wants backporting. 
> 
> If you can figure out a fixes tag that would be great!
I've taken a stab at working out when this got introduced and
came up with:

81956a93b522 "iio: imu: st_lsm6dsx: get device name from st_lsm6dsx_sensor_settings"

If that's wrong please let me know asap.

Applied to the togreg branch of iio.git as we are near the merge
window opening and marked for stable.

Thanks,

Jonathan
> 
> Thanks,
> 
> Jonathan
> 
> > ---
> >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > index a7d40c02ce6b..b921dd9e108f 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > @@ -1301,7 +1301,8 @@ static int st_lsm6dsx_check_whoami(struct st_lsm6dsx_hw *hw, int id,
> >  
> >  	for (i = 0; i < ARRAY_SIZE(st_lsm6dsx_sensor_settings); i++) {
> >  		for (j = 0; j < ST_LSM6DSX_MAX_ID; j++) {
> > -			if (id == st_lsm6dsx_sensor_settings[i].id[j].hw_id)
> > +			if (st_lsm6dsx_sensor_settings[i].id[j].name &&
> > +			    id == st_lsm6dsx_sensor_settings[i].id[j].hw_id)
> >  				break;
> >  		}
> >  		if (j < ST_LSM6DSX_MAX_ID)  
> 


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

* Re: [PATCH] iio: imu: st_lsm6dsx: Fix selection of ST_LSM6DS3_ID
  2020-01-13 22:12   ` Jonathan Cameron
@ 2020-01-14  9:06     ` Stephan Gerhold
  2020-01-14  9:12       ` Jonathan Cameron
  0 siblings, 1 reply; 8+ messages in thread
From: Stephan Gerhold @ 2020-01-14  9:06 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lorenzo Bianconi, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, linux-iio

On Mon, Jan 13, 2020 at 10:12:11PM +0000, Jonathan Cameron wrote:
> On Sun, 15 Dec 2019 17:23:42 +0000
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > On Mon,  9 Dec 2019 18:05:41 +0100
> > Stephan Gerhold <stephan@gerhold.net> wrote:
> > 
> > > At the moment, attempting to probe a device with ST_LSM6DS3_ID
> > > (e.g. using the st,lsm6ds3 compatible) fails with:
> > > 
> > >     st_lsm6dsx_i2c 1-006b: unsupported whoami [69]
> > > 
> > > ... even though 0x69 is the whoami listed for ST_LSM6DS3_ID.
> > > 
> > > This happens because st_lsm6dsx_check_whoami() also attempts
> > > to match unspecified (zero-initialized) entries in the "id" array.
> > > ST_LSM6DS3_ID = 0 will therefore match any entry in
> > > st_lsm6dsx_sensor_settings (here: the first), because none of them
> > > actually have all 12 entries listed in the "id" array.
> > > 
> > > Avoid this by additionally checking if "name" is set,
> > > which is only set for valid entries in the "id" array.
> > > 
> > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>  
> > Definitely sounds like this wants backporting. 
> > 
> > If you can figure out a fixes tag that would be great!
> I've taken a stab at working out when this got introduced and
> came up with:
> 
> 81956a93b522 "iio: imu: st_lsm6dsx: get device name from st_lsm6dsx_sensor_settings"
> 
> If that's wrong please let me know asap.
> 
> Applied to the togreg branch of iio.git as we are near the merge
> window opening and marked for stable.

You have already applied the v2 I sent with the fixes/cc stable tags :)
It's part of your "Second set of IIO fixes for the 5.5 cycle." pull request:
https://lore.kernel.org/linux-iio/20200105110051.445c9a95@archlinux/

> 
> Thanks,
> 
> Jonathan
> > 
> > Thanks,
> > 
> > Jonathan
> > 
> > > ---
> > >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > > index a7d40c02ce6b..b921dd9e108f 100644
> > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > > @@ -1301,7 +1301,8 @@ static int st_lsm6dsx_check_whoami(struct st_lsm6dsx_hw *hw, int id,
> > >  
> > >  	for (i = 0; i < ARRAY_SIZE(st_lsm6dsx_sensor_settings); i++) {
> > >  		for (j = 0; j < ST_LSM6DSX_MAX_ID; j++) {
> > > -			if (id == st_lsm6dsx_sensor_settings[i].id[j].hw_id)
> > > +			if (st_lsm6dsx_sensor_settings[i].id[j].name &&
> > > +			    id == st_lsm6dsx_sensor_settings[i].id[j].hw_id)
> > >  				break;
> > >  		}
> > >  		if (j < ST_LSM6DSX_MAX_ID)  
> > 
> 


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

* Re: [PATCH] iio: imu: st_lsm6dsx: Fix selection of ST_LSM6DS3_ID
  2020-01-14  9:06     ` Stephan Gerhold
@ 2020-01-14  9:12       ` Jonathan Cameron
  0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2020-01-14  9:12 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: Jonathan Cameron, Lorenzo Bianconi, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio

On Tue, 14 Jan 2020 10:06:52 +0100
Stephan Gerhold <stephan@gerhold.net> wrote:

> On Mon, Jan 13, 2020 at 10:12:11PM +0000, Jonathan Cameron wrote:
> > On Sun, 15 Dec 2019 17:23:42 +0000
> > Jonathan Cameron <jic23@kernel.org> wrote:
> >   
> > > On Mon,  9 Dec 2019 18:05:41 +0100
> > > Stephan Gerhold <stephan@gerhold.net> wrote:
> > >   
> > > > At the moment, attempting to probe a device with ST_LSM6DS3_ID
> > > > (e.g. using the st,lsm6ds3 compatible) fails with:
> > > > 
> > > >     st_lsm6dsx_i2c 1-006b: unsupported whoami [69]
> > > > 
> > > > ... even though 0x69 is the whoami listed for ST_LSM6DS3_ID.
> > > > 
> > > > This happens because st_lsm6dsx_check_whoami() also attempts
> > > > to match unspecified (zero-initialized) entries in the "id" array.
> > > > ST_LSM6DS3_ID = 0 will therefore match any entry in
> > > > st_lsm6dsx_sensor_settings (here: the first), because none of them
> > > > actually have all 12 entries listed in the "id" array.
> > > > 
> > > > Avoid this by additionally checking if "name" is set,
> > > > which is only set for valid entries in the "id" array.
> > > > 
> > > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>    
> > > Definitely sounds like this wants backporting. 
> > > 
> > > If you can figure out a fixes tag that would be great!  
> > I've taken a stab at working out when this got introduced and
> > came up with:
> > 
> > 81956a93b522 "iio: imu: st_lsm6dsx: get device name from st_lsm6dsx_sensor_settings"
> > 
> > If that's wrong please let me know asap.
> > 
> > Applied to the togreg branch of iio.git as we are near the merge
> > window opening and marked for stable.  
> 
> You have already applied the v2 I sent with the fixes/cc stable tags :)
> It's part of your "Second set of IIO fixes for the 5.5 cycle." pull request:
> https://lore.kernel.org/linux-iio/20200105110051.445c9a95@archlinux/

Gah!  I clearly wasn't on top form last night.   Will drop this one again.

Thanks.

Jonathan

> 
> > 
> > Thanks,
> > 
> > Jonathan  
> > > 
> > > Thanks,
> > > 
> > > Jonathan
> > >   
> > > > ---
> > > >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > > > index a7d40c02ce6b..b921dd9e108f 100644
> > > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > > > @@ -1301,7 +1301,8 @@ static int st_lsm6dsx_check_whoami(struct st_lsm6dsx_hw *hw, int id,
> > > >  
> > > >  	for (i = 0; i < ARRAY_SIZE(st_lsm6dsx_sensor_settings); i++) {
> > > >  		for (j = 0; j < ST_LSM6DSX_MAX_ID; j++) {
> > > > -			if (id == st_lsm6dsx_sensor_settings[i].id[j].hw_id)
> > > > +			if (st_lsm6dsx_sensor_settings[i].id[j].name &&
> > > > +			    id == st_lsm6dsx_sensor_settings[i].id[j].hw_id)
> > > >  				break;
> > > >  		}
> > > >  		if (j < ST_LSM6DSX_MAX_ID)    
> > >   
> >   
> 



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

end of thread, other threads:[~2020-01-14  9:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-09 17:05 [PATCH] iio: imu: st_lsm6dsx: Fix selection of ST_LSM6DS3_ID Stephan Gerhold
2019-12-09 21:48 ` Lorenzo Bianconi
2019-12-10  8:22   ` Stephan Gerhold
2019-12-11 14:10     ` Lorenzo Bianconi
2019-12-15 17:23 ` Jonathan Cameron
2020-01-13 22:12   ` Jonathan Cameron
2020-01-14  9:06     ` Stephan Gerhold
2020-01-14  9:12       ` Jonathan Cameron

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).