| Message ID | 20190430014825.30553-16-marek.behun@nic.cz |
|---|---|
| State | Superseded |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | Fixes for Turris Omnia | expand |
Hello Marek, Am 30.04.2019 um 03:48 schrieb Marek Behún: > The twsi_wait function reads the control register for interrupt flag, > and if interrupt flag is present, it immediately reads status register. > > On our device this sometimes causes bad value being read from status > register, as if the value was not yet updated. > > My theory is that the controller does approximately this: > 1. sets interrupt flag in control register, > 2. sets the value of status register, > 3. causes an interrupt > > In U-Boot we do not use interrupts, so I think that it is possible that > sometimes the status register in the twsi_wait function is read between > points 1 and 2. > > The bug does not appear if I add a small delay before reading status > register. Strange. > Wait 100ns (which in U-Boot currently means 1 us, because ndelay(i) > function calls udelay(DIV_ROUND_UP(i, 1000))) before reading the status > register. > > Signed-off-by: Marek Behún <marek.behun@nic.cz> > Cc: Mario Six <mario.six@gdsys.cc> > Cc: Stefan Roese <sr@denx.de> > Cc: Baruch Siach <baruch@tkos.co.il> > Cc: Heiko Schocher <hs@denx.de> > --- > drivers/i2c/mvtwsi.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c > index 74ac0a4aa7..483a71ba12 100644 > --- a/drivers/i2c/mvtwsi.c > +++ b/drivers/i2c/mvtwsi.c > @@ -271,6 +271,7 @@ static int twsi_wait(struct mvtwsi_registers *twsi, int expected_status, > do { > control = readl(&twsi->control); > if (control & MVTWSI_CONTROL_IFLG) { > + ndelay(100); > status = readl(&twsi->status); > if (status == expected_status) > return 0; > Can you please add a comment in the code too? Beside of that: Acked-by: Heiko Schocher <hs@denx.de> bye, Heiko
diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c index 74ac0a4aa7..483a71ba12 100644 --- a/drivers/i2c/mvtwsi.c +++ b/drivers/i2c/mvtwsi.c @@ -271,6 +271,7 @@ static int twsi_wait(struct mvtwsi_registers *twsi, int expected_status, do { control = readl(&twsi->control); if (control & MVTWSI_CONTROL_IFLG) { + ndelay(100); status = readl(&twsi->status); if (status == expected_status) return 0;
The twsi_wait function reads the control register for interrupt flag, and if interrupt flag is present, it immediately reads status register. On our device this sometimes causes bad value being read from status register, as if the value was not yet updated. My theory is that the controller does approximately this: 1. sets interrupt flag in control register, 2. sets the value of status register, 3. causes an interrupt In U-Boot we do not use interrupts, so I think that it is possible that sometimes the status register in the twsi_wait function is read between points 1 and 2. The bug does not appear if I add a small delay before reading status register. Wait 100ns (which in U-Boot currently means 1 us, because ndelay(i) function calls udelay(DIV_ROUND_UP(i, 1000))) before reading the status register. Signed-off-by: Marek Behún <marek.behun@nic.cz> Cc: Mario Six <mario.six@gdsys.cc> Cc: Stefan Roese <sr@denx.de> Cc: Baruch Siach <baruch@tkos.co.il> Cc: Heiko Schocher <hs@denx.de> --- drivers/i2c/mvtwsi.c | 1 + 1 file changed, 1 insertion(+)