> 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@xxxxxxxxxx>
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@xxxxxxxxxxx>
> > > ---
> > > 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:
signature.asc
Description: PGP signature
