| Message ID | 1632151292-18503-3-git-send-email-alain.volmat@foss.st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | i2c: stm32: various fixes & dmaengine updates | expand |
Hi Alain Look good to me Reviewed-by: Pierre-Yves MORDRET <pierre-yves.mordret@foss.st.com> Regards On 9/20/21 5:21 PM, Alain Volmat wrote: > When getting an access timeout, ensure that the bus is in a proper > state prior to returning the error. > > Fixes: aeb068c57214 ("i2c: i2c-stm32f7: add driver") > > Signed-off-by: Alain Volmat <alain.volmat@foss.st.com> > --- > drivers/i2c/busses/i2c-stm32f7.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c > index ed977b6f7ab6..ad3459a3bc5e 100644 > --- a/drivers/i2c/busses/i2c-stm32f7.c > +++ b/drivers/i2c/busses/i2c-stm32f7.c > @@ -1712,6 +1712,7 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap, > i2c_dev->msg->addr); > if (i2c_dev->use_dma) > dmaengine_terminate_all(dma->chan_using); > + stm32f7_i2c_wait_free_bus(i2c_dev); > ret = -ETIMEDOUT; > } > > @@ -1769,6 +1770,7 @@ static int stm32f7_i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, > dev_dbg(dev, "Access to slave 0x%x timed out\n", f7_msg->addr); > if (i2c_dev->use_dma) > dmaengine_terminate_all(dma->chan_using); > + stm32f7_i2c_wait_free_bus(i2c_dev); > ret = -ETIMEDOUT; > goto pm_free; > } >
> + stm32f7_i2c_wait_free_bus(i2c_dev);
This does only a controller reset, not a bus recovery with 9 toggling
pulses, or?
Hi Wolfram On Mon, Nov 29, 2021 at 01:17:21PM +0100, Wolfram Sang wrote: > > > + stm32f7_i2c_wait_free_bus(i2c_dev); > > This does only a controller reset, not a bus recovery with 9 toggling > pulses, or? indeed. I might better rework this and at the same time introduce the bus recovery mechanism via the bus recovery callback in this driver. Please don't merge this patch and I will rework that.
Hi Alain, > > > + stm32f7_i2c_wait_free_bus(i2c_dev); > > > > This does only a controller reset, not a bus recovery with 9 toggling > > pulses, or? > > indeed. I might better rework this and at the same time introduce the > bus recovery mechanism via the bus recovery callback in this driver. > Please don't merge this patch and I will rework that. Wait a sec. Resetting a controller at the end of a failed transfer might make sense if the controller is otherwise in an confused state. Full bus recovery (9 pulses) should be done at the beginning of a transfer when SDA is low, though. So, I'd actually suggest to apply this patch and add full bus recovery based on SDA low at the beginning of a transfer seperately. What do you think? All the best, Wolfram
Hi Alain, On Mon, Nov 29, 2021 at 01:52:30PM +0100, Wolfram Sang wrote: > Hi Alain, > > > > > + stm32f7_i2c_wait_free_bus(i2c_dev); > > > > > > This does only a controller reset, not a bus recovery with 9 toggling > > > pulses, or? > > > > indeed. I might better rework this and at the same time introduce the > > bus recovery mechanism via the bus recovery callback in this driver. > > Please don't merge this patch and I will rework that. > > Wait a sec. Resetting a controller at the end of a failed transfer might > make sense if the controller is otherwise in an confused state. > > Full bus recovery (9 pulses) should be done at the beginning of a > transfer when SDA is low, though. > > So, I'd actually suggest to apply this patch and add full bus recovery > based on SDA low at the beginning of a transfer seperately. > > What doo you think? I just checked again. Indeed, this patch is here to handle cases when communication went bad with a device leading to controller being left in a confused state. This is done to put it back in a working state. I agree with you on the fact to decouple this with the 9 pulses bus recovery and first apply this one first. Thanks. Alain > > All the best, > > Wolfram >
> I agree with you on the fact to decouple this with the 9 pulses bus > recovery and first apply this one first. Good. I noticed something in your driver on the way. Will send an RFC in some minutes.
On Mon, Sep 20, 2021 at 05:21:30PM +0200, Alain Volmat wrote: > When getting an access timeout, ensure that the bus is in a proper > state prior to returning the error. > > Fixes: aeb068c57214 ("i2c: i2c-stm32f7: add driver") > > Signed-off-by: Alain Volmat <alain.volmat@foss.st.com> Applied to for-current, thanks!
diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c index ed977b6f7ab6..ad3459a3bc5e 100644 --- a/drivers/i2c/busses/i2c-stm32f7.c +++ b/drivers/i2c/busses/i2c-stm32f7.c @@ -1712,6 +1712,7 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap, i2c_dev->msg->addr); if (i2c_dev->use_dma) dmaengine_terminate_all(dma->chan_using); + stm32f7_i2c_wait_free_bus(i2c_dev); ret = -ETIMEDOUT; } @@ -1769,6 +1770,7 @@ static int stm32f7_i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, dev_dbg(dev, "Access to slave 0x%x timed out\n", f7_msg->addr); if (i2c_dev->use_dma) dmaengine_terminate_all(dma->chan_using); + stm32f7_i2c_wait_free_bus(i2c_dev); ret = -ETIMEDOUT; goto pm_free; }
When getting an access timeout, ensure that the bus is in a proper state prior to returning the error. Fixes: aeb068c57214 ("i2c: i2c-stm32f7: add driver") Signed-off-by: Alain Volmat <alain.volmat@foss.st.com> --- drivers/i2c/busses/i2c-stm32f7.c | 2 ++ 1 file changed, 2 insertions(+)