Paul, Ulrik, Just adding my view to this discussion: > -----Original Message----- > From: linux-omap-owner@xxxxxxxxxxxxxxx [mailto:linux-omap- > owner@xxxxxxxxxxxxxxx] On Behalf Of Paul Walmsley > Sent: Tuesday, June 23, 2009 9:05 PM > To: Hald, Ulrik Bech > Cc: Pakaravoor, Jagadeesh; linux-omap@xxxxxxxxxxxxxxx > Subject: RE: [PATCH v2 1/1] i2c:OMAP3:Errata workaround for spurious RDR > event > > > > > > > > > + u8 stat2 = 0; > > > > > > + stat2 = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG); > > > > > > + if (stat2 & OMAP_I2C_STAT_BB) > > > > > > + return IRQ_HANDLED; > > > > > > > > > > Why use stat2? Why not just test stat again? > > > > > > > > Stat is read at the beginning of the ISR, what if BB bit gets > > > > cleared/set a while later, not along with RDR, as a corner case? > > > > > > If that is possible, then the comment in this patch needs to be > changed: > > > > > > > + /* 3430 I2C Errata 1.15 > > > > + * RDR could be set when the bus is busy, then > > > > + * ignore the interrupt, and clear the bit. > > > > + */ > > > > > > This implies that the state of the BB bit is important when the RDR > bit is > > > set. The closest sample we have for that is the contents of the > 'stat' > > > variable. > > > > > If I understand it correctly, you're concerned that the wording of the > > comment lets one think, that the state of the bus is critical at the > > moment the RDR interrupt is set? I guess you're right about that, since > > the wording could be a little misleading. > > My concern is that the comment and the code seem to conflict. > > > The point of the errata workaround is only to prevent handling of the > > RDR interrupt, if the bus is busy at the time when the RDR is to be > > handled. It doesn't matter if BB has been set/cleared before that. > > > > So maybe, the wording of the comment should be: > > > > /* 3430 I2C Errata 1.15, 2430 I2C Errata 1.67: > > * RDR should not be handled when bus is busy */ > > > > ? > > Yes, that is better. > > > > > If we keep it this way, re-reading the register; what could be the > > > > potential problem? > > > > > > It doesn't match the definition of the erratum as expressed in the > > > comment. Is it possible for the RDR bit to be erroneously set when BB > = > > > 0? > > > > Yes, the nature of the errata is that the RDR interrupt could be > > spurious. Although, if the bus is not busy and the RDR is set > > erroneously (there are no bytes in the FIFO to be drained), then the > > normal isr code would just leave and do nothing, which is what we also > > need in the errata work around. > > Okay. So it looks like there is a unfixable race here. Is it possible > for BB to be 0 during the stat2 read, then for BB to go to 1 immediately > afterwards? Then the rest of the RDR handler would be entered. If this > is possible, what will the ISR do -- will it hang? > > If this assessment of the problem is accurate, the stat2 read shrinks > the race window, and then indeed seems useful. > > a) why would bus be busy? - answer bus is busy before a transaction starts - each transaction is an atomic operation. If someone goofs around with signal while a master is sending/receiving data, there is AL(Arbitration Lost).. not Bus busy. b) Look at the flow in TRM figure 18-13 I2C Master Reciever Mode, interrupt mode -> where does BB get checked by the h/w? Not in the middle of the transaction.. Apologies if I am wrong, so am not entirely following the discussion here.. Regards, Nishanth Menon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
