Re: [PATCH v2] iio: imu: st_lsm6dsx: disable I3C support during device reset

[Lorenzo Bianconi <lorenzo@kernel.org>]

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

> > From: Lorenzo Bianconi <lorenzo@kernel.org>
> > Date: Mon, Mar 09, 2020 at 16:07:14
> > 
> > > > From: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > Date: Mon, Mar 09, 2020 at 15:10:35
> > > > 
> > > > > > From: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > > > Date: Mon, Mar 09, 2020 at 15:01:01
> > > > > > 
> > > > > > > > Hi Lorenzo,
> > > > > > > > 
> > > > > > > > From: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > > > > > Date: Sun, Mar 08, 2020 at 00:06:03
> > > > > > > > 
> > > > > > > > > Disable MIPI I3C during device reset in order to avoid
> > > > > > > > > possible races on interrupt line 1. If the first interrupt
> > > > > > > > > line is asserted during hw reset the device will work in
> > > > > > > > > I3C-only mode
> > > > > > > > > 
> > > > > > > 
> > > > > > > [...]
> > > > > > > 
> > > > > > > > > +
> > > > > > > > 
> > > > > > > > After disable the i3c interface the dynamic address is no more accessible 
> > > > > > > > and fails the initialization.
> > > > > > > > 
> > > > > > > 
> > > > > > > Hi Vitor,
> > > > > > > 
> > > > > > > thx for testing it. What do you mean here?
> > > > > > > Is int1 set to vdd in your test?
> > > > > > > 
> > > > > > > Regards,
> > > > > > > Lorenzo
> > > > > > 
> > > > > > Yes, according with figure 14 of lsm6dso datasheet.
> > > > > 
> > > > > uhm..probably we should do this configuration if the device is not in I3C-only
> > > > > mode. Are you able to test it without setting the int1 to vdd?
> > > > > Unfortunately I have no devices with an I3C controller yet.
> > > > > 
> > > > > Regards,
> > > > > Lorenzo
> > > > > 
> > > > 
> > > > Yes, I can test but I suspect we will have the same issue because it lost 
> > > > the dynamic address. I would say to add a flag during the probe to 
> > > > indicate the interface and bypass this if in I3C mode.
> > > 
> > > I am not an i3c expert but I think the dynamic address is reset during the boot
> > > procedure of the sensor (this is done even if you do not disable i3c).
> > 
> > It can't because the dynamic address assignment (1) and the sensor boot 
> > (2) are made in different steps.
> >   1. probe of i3c master driver;
> >   2. probe of sensor driver;
> > 
> > > Re-thinking about it, we should avoid it if the device if working in i3c-only
> > > (int1 set to vdd) but I think it would be necessary in i3c-mixed (int1 set 0
> > > gnd). Could you please test it in the latter case? Thanks.
> > > 
> > > Regards,
> > > Lorenzo
> > 
> > I test and get the same behavior. I thought it goes back to I2C mode but 
> > not and neither the I3C DA is addressable.
> 
> Hi Vitor,
> 
> thx a lot for testing :) Maybe this would be important just when DSO/DSR are
> working in i2c. We need some info from st I guess
> 
> + st folks

Mario suggested me we can fix the issue even flushing the FIFO before the reset.
I will post a v3 using this approach.

Regards,
Lorenzo

> 
> Regards,
> Lorenzo
> 
> > 
> > Best regards,
> > Vitor Soares
> > 
> > > 
> > > > 
> > > > This may be useful when address the In-band interrupts.
> > > > 
> > > > Best regards,
> > > > Vitor Soares
> > > > 
> > > > > > 
> > > > > > Is there any way to clear the INT1 before the hw reset?
> > > > > > 
> > > > > > Best regards,
> > > > > > Vitor Soares
> > > > > > 
> > > > > > > 
> > > > > > > > Best regards,
> > > > > > > > Vitor Soares
> > > > > > > > 
> > > > > > > > >  	/* device sw reset */
> > > > > > > > >  	reg = &hw->settings->reset;
> > > > > > > > >  	err = regmap_update_bits(hw->regmap, reg->addr, reg->mask,
> > > > > > > > > @@ -2059,6 +2081,15 @@ static int st_lsm6dsx_init_device(struct st_lsm6dsx_hw *hw)
> > > > > > > > >  
> > > > > > > > >  	msleep(50);
> > > > > > > > >  
> > > > > > > > > +	/* enable MIPI I3C */
> > > > > > > > > +	if (hw->settings->i3c_disable.addr) {
> > > > > > > > > +		reg = &hw->settings->i3c_disable;
> > > > > > > > > +		err = regmap_update_bits(hw->regmap, reg->addr, reg->mask,
> > > > > > > > > +					 ST_LSM6DSX_SHIFT_VAL(0, reg->mask));
> > > > > > > > > +		if (err < 0)
> > > > > > > > > +			return err;
> > > > > > > > > +	}
> > > > > > > > > +
> > > > > > > > >  	/* enable Block Data Update */
> > > > > > > > >  	reg = &hw->settings->bdu;
> > > > > > > > >  	err = regmap_update_bits(hw->regmap, reg->addr, reg->mask,
> > > > > > > > > -- 
> > > > > > > > > 2.24.1
> > > > > > > > 
> > > > > > > > 
> > > > > > 
> > > > > > 
> > > > 
> > > > 
> > 
> > 



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