On Saturday, February 9, 2019 10:33:53 PM CET Andrew Lunn wrote: > > +static int ocores_poll_wait(struct ocores_i2c *i2c) > > +{ > > + u8 mask; > > + int err; > > + > > + if (i2c->state == STATE_DONE || i2c->state == STATE_ERROR) { > > + /* transfer is over */ > > + mask = OCI2C_STAT_BUSY; > > + } else { > > + /* on going transfer */ > > + mask = OCI2C_STAT_TIP; > > + udelay((8 * 1000) / i2c->bus_clock_khz); > > + } > > + > > + /* > > + * once we are here we expect to get the expected result immediately > > + * so if after 1ms we timeout then something is broken. > > + */ > > + err = ocores_wait(i2c, OCI2C_STATUS, mask, 0, msecs_to_jiffies(1)); > > Hi Federico > > I did some timing tests for this. On my box, we request a udelay of > 80uS. The kernel actually delays for about 79uS. We then spin in > ocores_wait() for an additional 10-11uS, which is 3 to 4 iterations. > > There are actually 9 bits on the wire, not 8, since there is an > ACK/NACK bit after the actual data transfer. So i changed the delay to > (9 * 1000) / i2c->bus_clock_khz. That resulted in ocores_wait() mostly > not looping at all. But for reading an 4K AT24 EEPROM, it increased > the read time by 10ms, from 424ms to 434ms. So we should probably keep > with 8. > > Tested-by: Andrew Lunn <and...@lunn.ch>
I had a similar experience. I will add a comment in the code to explain that 8 is not a mistake but a conscious decision. Then I will add what you wrote here in the patch changelog > > Andrew