Re: [PATCH v3 3/5] i2c:ocores: add polling interface

Federico Vaga
2019-02-11T08:20:58Z

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 <andrew@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