i2c: at91: disable TXRDY interrupt after sending data
diff mbox series

Message ID cbd93920a225e2e32c7f43ff417f301af57c4e6c.1563820695.git.mirq-linux@rere.qmqm.pl
State Accepted
Headers show
Series
  • i2c: at91: disable TXRDY interrupt after sending data
Related show

Commit Message

Michał Mirosław July 22, 2019, 6:55 p.m. UTC
Driver was not disabling TXRDY interrupt after last TX byte.
This caused interrupt storm until transfer timeouts for slow
or broken device on the bus. The patch fixes the interrupt storm
on my SAMA5D2-based board.

Cc: stable@vger.kernel.org # 5.2.x
[v5.2 introduced file split; the patch should apply to i2c-at91.c before the split]
Fixes: fac368a0404 ("i2c: at91: add new driver")
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

---
 drivers/i2c/busses/i2c-at91-master.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Ludovic Desroches July 31, 2019, 12:37 p.m. UTC | #1
Hi,

On Mon, Jul 22, 2019 at 08:55:27PM +0200, Michał Mirosław wrote:
> 
> Driver was not disabling TXRDY interrupt after last TX byte.
> This caused interrupt storm until transfer timeouts for slow
> or broken device on the bus. The patch fixes the interrupt storm
> on my SAMA5D2-based board.
> 
> Cc: stable@vger.kernel.org # 5.2.x
> [v5.2 introduced file split; the patch should apply to i2c-at91.c before the split]
> Fixes: fac368a0404 ("i2c: at91: add new driver")
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com> 

Sounds good to me as AT91_TWI_TXRDY is only used to send the next byte.

Raag, you added you in the copy list as it seems close to your issue. I am
really sorry, I didn't have time to go further with your issue. Is this
patch solves your issue?

Thanks for this patch.

Ludovic

> ---
>  drivers/i2c/busses/i2c-at91-master.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-at91-master.c b/drivers/i2c/busses/i2c-at91-master.c
> index e87232f2e708..a3fcc35ffd3b 100644
> --- a/drivers/i2c/busses/i2c-at91-master.c
> +++ b/drivers/i2c/busses/i2c-at91-master.c
> @@ -122,9 +122,11 @@ static void at91_twi_write_next_byte(struct at91_twi_dev *dev)
>  	writeb_relaxed(*dev->buf, dev->base + AT91_TWI_THR);
>  
>  	/* send stop when last byte has been written */
> -	if (--dev->buf_len == 0)
> +	if (--dev->buf_len == 0) {
>  		if (!dev->use_alt_cmd)
>  			at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_STOP);
> +		at91_twi_write(dev, AT91_TWI_IDR, AT91_TWI_TXRDY);
> +	}
>  
>  	dev_dbg(dev->dev, "wrote 0x%x, to go %zu\n", *dev->buf, dev->buf_len);
>  
> @@ -542,9 +544,8 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
>  		} else {
>  			at91_twi_write_next_byte(dev);
>  			at91_twi_write(dev, AT91_TWI_IER,
> -				       AT91_TWI_TXCOMP |
> -				       AT91_TWI_NACK |
> -				       AT91_TWI_TXRDY);
> +				       AT91_TWI_TXCOMP | AT91_TWI_NACK |
> +				       (dev->buf_len ? AT91_TWI_TXRDY : 0));
>  		}
>  	}
>  
> -- 
> 2.20.1
>
Wolfram Sang Aug. 1, 2019, 12:54 p.m. UTC | #2
On Mon, Jul 22, 2019 at 08:55:27PM +0200, Michał Mirosław wrote:
> Driver was not disabling TXRDY interrupt after last TX byte.
> This caused interrupt storm until transfer timeouts for slow
> or broken device on the bus. The patch fixes the interrupt storm
> on my SAMA5D2-based board.
> 
> Cc: stable@vger.kernel.org # 5.2.x
> [v5.2 introduced file split; the patch should apply to i2c-at91.c before the split]
> Fixes: fac368a0404 ("i2c: at91: add new driver")
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> 

Applied to for-current, thanks!
Raag Jadav Aug. 1, 2019, 5:05 p.m. UTC | #3
On Wed, Jul 31, 2019 at 02:37:35PM +0200, Ludovic Desroches wrote:
> Hi,
> 
> On Mon, Jul 22, 2019 at 08:55:27PM +0200, Michał Mirosław wrote:
> > 
> > Driver was not disabling TXRDY interrupt after last TX byte.
> > This caused interrupt storm until transfer timeouts for slow
> > or broken device on the bus. The patch fixes the interrupt storm
> > on my SAMA5D2-based board.
> > 
> > Cc: stable@vger.kernel.org # 5.2.x
> > [v5.2 introduced file split; the patch should apply to i2c-at91.c before the split]
> > Fixes: fac368a0404 ("i2c: at91: add new driver")
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com> 

Tested-by: Raag Jadav <raagjadav@gmail.com>

> 
> Sounds good to me as AT91_TWI_TXRDY is only used to send the next byte.
> 
> Raag, you added you in the copy list as it seems close to your issue. I am
> really sorry, I didn't have time to go further with your issue. Is this
> patch solves your issue?

Yes, although I had a v2 ready to send out with similar changes,
this patch does the job as well.

Cheers,
Raag

> 
> Thanks for this patch.
> 
> Ludovic
> 
> > ---
> >  drivers/i2c/busses/i2c-at91-master.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-at91-master.c b/drivers/i2c/busses/i2c-at91-master.c
> > index e87232f2e708..a3fcc35ffd3b 100644
> > --- a/drivers/i2c/busses/i2c-at91-master.c
> > +++ b/drivers/i2c/busses/i2c-at91-master.c
> > @@ -122,9 +122,11 @@ static void at91_twi_write_next_byte(struct at91_twi_dev *dev)
> >  	writeb_relaxed(*dev->buf, dev->base + AT91_TWI_THR);
> >  
> >  	/* send stop when last byte has been written */
> > -	if (--dev->buf_len == 0)
> > +	if (--dev->buf_len == 0) {
> >  		if (!dev->use_alt_cmd)
> >  			at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_STOP);
> > +		at91_twi_write(dev, AT91_TWI_IDR, AT91_TWI_TXRDY);
> > +	}
> >  
> >  	dev_dbg(dev->dev, "wrote 0x%x, to go %zu\n", *dev->buf, dev->buf_len);
> >  
> > @@ -542,9 +544,8 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
> >  		} else {
> >  			at91_twi_write_next_byte(dev);
> >  			at91_twi_write(dev, AT91_TWI_IER,
> > -				       AT91_TWI_TXCOMP |
> > -				       AT91_TWI_NACK |
> > -				       AT91_TWI_TXRDY);
> > +				       AT91_TWI_TXCOMP | AT91_TWI_NACK |
> > +				       (dev->buf_len ? AT91_TWI_TXRDY : 0));
> >  		}
> >  	}
> >  
> > -- 
> > 2.20.1
> >
Wolfram Sang Aug. 1, 2019, 8:28 p.m. UTC | #4
> > > [v5.2 introduced file split; the patch should apply to i2c-at91.c before the split]
> > > Fixes: fac368a0404 ("i2c: at91: add new driver")
> > > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com> 
> 
> Tested-by: Raag Jadav <raagjadav@gmail.com>

Thanks for testing! Because I needed to change this commit message
anyhow, I added your tag as well while at it.
Ludovic Desroches Aug. 2, 2019, 6:22 a.m. UTC | #5
On Thu, Aug 01, 2019 at 10:35:52PM +0530, Raag Jadav wrote:
> External E-Mail
> 
> 
> On Wed, Jul 31, 2019 at 02:37:35PM +0200, Ludovic Desroches wrote:
> > Hi,
> > 
> > On Mon, Jul 22, 2019 at 08:55:27PM +0200, Michał Mirosław wrote:
> > > 
> > > Driver was not disabling TXRDY interrupt after last TX byte.
> > > This caused interrupt storm until transfer timeouts for slow
> > > or broken device on the bus. The patch fixes the interrupt storm
> > > on my SAMA5D2-based board.
> > > 
> > > Cc: stable@vger.kernel.org # 5.2.x
> > > [v5.2 introduced file split; the patch should apply to i2c-at91.c before the split]
> > > Fixes: fac368a0404 ("i2c: at91: add new driver")
> > > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com> 
> 
> Tested-by: Raag Jadav <raagjadav@gmail.com>
> 
> > 
> > Sounds good to me as AT91_TWI_TXRDY is only used to send the next byte.
> > 
> > Raag, you added you in the copy list as it seems close to your issue. I am
> > really sorry, I didn't have time to go further with your issue. Is this
> > patch solves your issue?
> 
> Yes, although I had a v2 ready to send out with similar changes,
> this patch does the job as well.

Great, thanks for the test Raag.

Regards

Ludovic

> 
> Cheers,
> Raag
> 
> > 
> > Thanks for this patch.
> > 
> > Ludovic
> > 
> > > ---
> > >  drivers/i2c/busses/i2c-at91-master.c | 9 +++++----
> > >  1 file changed, 5 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/i2c/busses/i2c-at91-master.c b/drivers/i2c/busses/i2c-at91-master.c
> > > index e87232f2e708..a3fcc35ffd3b 100644
> > > --- a/drivers/i2c/busses/i2c-at91-master.c
> > > +++ b/drivers/i2c/busses/i2c-at91-master.c
> > > @@ -122,9 +122,11 @@ static void at91_twi_write_next_byte(struct at91_twi_dev *dev)
> > >  	writeb_relaxed(*dev->buf, dev->base + AT91_TWI_THR);
> > >  
> > >  	/* send stop when last byte has been written */
> > > -	if (--dev->buf_len == 0)
> > > +	if (--dev->buf_len == 0) {
> > >  		if (!dev->use_alt_cmd)
> > >  			at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_STOP);
> > > +		at91_twi_write(dev, AT91_TWI_IDR, AT91_TWI_TXRDY);
> > > +	}
> > >  
> > >  	dev_dbg(dev->dev, "wrote 0x%x, to go %zu\n", *dev->buf, dev->buf_len);
> > >  
> > > @@ -542,9 +544,8 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
> > >  		} else {
> > >  			at91_twi_write_next_byte(dev);
> > >  			at91_twi_write(dev, AT91_TWI_IER,
> > > -				       AT91_TWI_TXCOMP |
> > > -				       AT91_TWI_NACK |
> > > -				       AT91_TWI_TXRDY);
> > > +				       AT91_TWI_TXCOMP | AT91_TWI_NACK |
> > > +				       (dev->buf_len ? AT91_TWI_TXRDY : 0));
> > >  		}
> > >  	}
> > >  
> > > -- 
> > > 2.20.1
> > >

Patch
diff mbox series

diff --git a/drivers/i2c/busses/i2c-at91-master.c b/drivers/i2c/busses/i2c-at91-master.c
index e87232f2e708..a3fcc35ffd3b 100644
--- a/drivers/i2c/busses/i2c-at91-master.c
+++ b/drivers/i2c/busses/i2c-at91-master.c
@@ -122,9 +122,11 @@  static void at91_twi_write_next_byte(struct at91_twi_dev *dev)
 	writeb_relaxed(*dev->buf, dev->base + AT91_TWI_THR);
 
 	/* send stop when last byte has been written */
-	if (--dev->buf_len == 0)
+	if (--dev->buf_len == 0) {
 		if (!dev->use_alt_cmd)
 			at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_STOP);
+		at91_twi_write(dev, AT91_TWI_IDR, AT91_TWI_TXRDY);
+	}
 
 	dev_dbg(dev->dev, "wrote 0x%x, to go %zu\n", *dev->buf, dev->buf_len);
 
@@ -542,9 +544,8 @@  static int at91_do_twi_transfer(struct at91_twi_dev *dev)
 		} else {
 			at91_twi_write_next_byte(dev);
 			at91_twi_write(dev, AT91_TWI_IER,
-				       AT91_TWI_TXCOMP |
-				       AT91_TWI_NACK |
-				       AT91_TWI_TXRDY);
+				       AT91_TWI_TXCOMP | AT91_TWI_NACK |
+				       (dev->buf_len ? AT91_TWI_TXRDY : 0));
 		}
 	}