On Fri, Oct 16, 2009 at 00:11, <[email protected]> wrote: > Revision 7660 Author sonicz Date 2009-10-16 00:11:41 -0400 (Fri, 16 Oct > 2009) > > Log Message > > Fix bug[#5275] Clear i2c int state earlier and loop till no ints left > > i2c event of next read/write byte may trigger before current int state > is cleared in the interrupt handler. So, this should be done at the > beginning of interrupt handler to avoid losing new i2c events. > > Modified: trunk/drivers/i2c/busses/i2c-bfin-twi.c (7659 => 7660) > > @@ -80,9 +80,9 @@ > {P_TWI1_SCL, P_TWI1_SDA, 0}, > }; > > -static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface) > +static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface, > + unsigned short twi_int_status) > { > - unsigned short twi_int_status = read_INT_STAT(iface); > unsigned short mast_stat = read_MASTER_STAT(iface); > > if (twi_int_status & XMTSERV) { > @@ -110,9 +110,6 @@ > (read_MASTER_CTL(iface) | RSTART) & > ~MDIR); > } > SSYNC(); > - /* Clear status */ > - write_INT_STAT(iface, XMTSERV); > - SSYNC(); > } > if (twi_int_status & RCVSERV) { > if (iface->readNum > 0) { > @@ -145,12 +142,8 @@ > (read_MASTER_CTL(iface) | RSTART) & > ~MDIR); > SSYNC(); > } > - /* Clear interrupt source */ > - write_INT_STAT(iface, RCVSERV); > - SSYNC(); > } > if (twi_int_status & MERR) { > - write_INT_STAT(iface, MERR); > write_INT_MASK(iface, 0); > write_MASTER_STAT(iface, 0x3e); > write_MASTER_CTL(iface, 0); > @@ -172,10 +165,6 @@ > * results. > */ > if (twi_int_status & MCOMP) { > - write_INT_STAT(iface, MCOMP); > - write_INT_MASK(iface, 0); > - write_MASTER_CTL(iface, 0); > - SSYNC(); > /* If it is a quick transfer, only address without data, > * not an err, return 1. > * If address is acknowledged return 1. > @@ -188,8 +177,6 @@ > return; > } > if (twi_int_status & MCOMP) { > - write_INT_STAT(iface, MCOMP); > - SSYNC(); > if (iface->cur_mode == TWI_I2C_MODE_COMBINED) { > if (iface->readNum == 0) { > /* set the read number to 1 and ask for manual > @@ -264,9 +251,18 @@ > { > struct bfin_twi_iface *iface = dev_id; > unsigned long flags; > + unsigned short twi_int_status; > > spin_lock_irqsave(&iface->lock, flags); > - bfin_twi_handle_interrupt(iface); > + while (1) { > + twi_int_status = read_INT_STAT(iface); > + if (!twi_int_status) > + break; > + /* Clear interrupt status */ > + write_INT_STAT(iface, twi_int_status); > + SSYNC(); > + bfin_twi_handle_interrupt(iface, twi_int_status); > + } > spin_unlock_irqrestore(&iface->lock, flags); > return IRQ_HANDLED;
i dont think we need to ssync here. it isnt like the INT_STAT being immediately cleared has any bearing on the rest of the interrupt handling code. why not unify the ssync's here by doing: - read INT_STAT - clear INT_STAT - call bfin_twi_handle_interrupt - do ssync then pretty much all of the ssync's in bfin_twi_handle_interrupt can be dropped too since it's expected that when the func returns, a ssync will be done also, i think we should declare a SSYNC_ATOMIC() macro so that we can avoid expanding anomalies related to interrupted ssyncs. it'd make sense to use it here for example. -mike _______________________________________________ Linux-kernel-commits mailing list [email protected] https://blackfin.uclinux.org/mailman/listinfo/linux-kernel-commits
