RE: [PATCH 5/7] i2c: img-scb: remove fifo EMPTYING interrupts handle

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi James,

> -----Original Message-----
> From: James Hogan
> Sent: 29 July 2015 17:01
> To: Sifan Naeem; Wolfram Sang; linux-i2c@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 5/7] i2c: img-scb: remove fifo EMPTYING interrupts
> handle
> 
> On 27/07/15 12:56, Sifan Naeem wrote:
> > This interrupt could have been useful for repeated start transfers as
> > the current transfer could be marked as complete while it's processing
> > the final byte of the transfer.
> > But having to use the transaction halt interrupt to safely control
> > repeated start transfers, means handling of the fifo EMPTYING
> > interrupts is no longer necessary.
> >
> > Handling this interrupt along with Transaction Halt interrupt can
> > cause erratic behaviour.
> 
> I'd be interested to know the cause of the erratic behaviour. It feels a little
> like we're just masking a real bug somewhere.
> 

EMPTYING interrupt indicates that the transfer is in its last byte, and in old ip versions it was safe to start a new transfer at this point.
The erratic behaviour I saw was due to how the latest IP handles Master Halt. In this IP a transaction is halted after each byte of a transfer.
Having to halt the transfer after the last byte means we can no longer service the EMPTYING interrupt, doing so may cause repeated start transfers to fails.

Thanks,
Sifan
> Cheers
> James
> 
> >
> > Signed-off-by: Sifan Naeem <sifan.naeem@xxxxxxxxxx>
> > ---
> >  drivers/i2c/busses/i2c-img-scb.c |    6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-img-scb.c
> > b/drivers/i2c/busses/i2c-img-scb.c
> > index efad4d7..10141a9 100644
> > --- a/drivers/i2c/busses/i2c-img-scb.c
> > +++ b/drivers/i2c/busses/i2c-img-scb.c
> > @@ -154,7 +154,6 @@
> >  #define INT_TIMING			BIT(18)
> >
> >  #define INT_FIFO_FULL_FILLING	(INT_FIFO_FULL  |
> INT_FIFO_FILLING)
> > -#define INT_FIFO_EMPTY_EMPTYING	(INT_FIFO_EMPTY |
> INT_FIFO_EMPTYING)
> >
> >  /* Level interrupts need clearing after handling instead of before */
> >  #define INT_LEVEL			0x01e00
> > @@ -176,8 +175,7 @@
> >  					 INT_WRITE_ACK_ERR    | \
> >  					 INT_FIFO_FULL        | \
> >  					 INT_FIFO_FILLING     | \
> > -					 INT_FIFO_EMPTY       | \
> > -					 INT_FIFO_EMPTYING)
> > +					 INT_FIFO_EMPTY)
> >
> >  #define INT_ENABLE_MASK_WAITSTOP	(INT_SLAVE_EVENT      | \
> >  					 INT_ADDR_ACK_ERR     | \
> > @@ -910,7 +908,7 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
> >  			}
> >  		}
> >  	} else {
> > -		if (int_status & INT_FIFO_EMPTY_EMPTYING) {
> > +		if (int_status & INT_FIFO_EMPTY) {
> >  			if (i2c->msg.len == 0) {
> >  				if (i2c->last_msg)
> >  					return ISR_WAITSTOP;
> >

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux