U-Boot i2c bus num 1 is broken on Nokia N900

["Pali Rohár" <pali@kernel.org>]

On Wednesday 28 October 2020 06:42:39 Heiko Schocher wrote:
> Am 26.10.2020 um 22:48 schrieb Pali Roh?r:
> > On Monday 27 April 2020 09:03:13 Heiko Schocher wrote:
> > > Am 26.04.2020 um 01:54 schrieb Pali Roh?r:
> > > > On Saturday 25 April 2020 14:11:32 Pali Roh?r wrote:
> > > > > On Saturday 25 April 2020 07:00:58 Adam Ford wrote:
> > > > > > On Sat, Apr 25, 2020 at 6:50 AM Pali Roh?r <pali@kernel.org> wrote:
> > > > > > > On Saturday 25 April 2020 06:36:58 Adam Ford wrote:
> > > > > > > > On Sat, Apr 25, 2020 at 5:42 AM Pali Roh?r <pali@kernel.org> wrote:
> > > > > > > > > On Thursday 02 April 2020 20:42:31 Pali Roh?r wrote:
> > > > > > > > > > On Wednesday 01 April 2020 12:32:29 Merlijn Wajer wrote:
> > > > > > > > > > > MMC:   OMAP SD/MMC: 0, OMAP SD/MMC: 1
> > > > > > > > > > > In:    vga
> > > > > > > > > > > Out:   vga
> > > > > > > > > > > Err:   vga
> > > > > > > > > > > Timed out in wait_for_event: status=0100
> > > > > > > > > > > Check if pads/pull-ups of bus are properly configured
> > > > > > > > > > > Timed out in wait_for_event: status=0000
> > > > > > > > > > > Check if pads/pull-ups of bus are properly configured
> > > > > > > > > > > Timed out in wait_for_event: status=0000
> > > > > > > > > > > Check if pads/pull-ups of bus are properly configured
> > > > > > > > > > > Timed out in wait_for_event: status=0000
> > > > > > > > > > > Check if pads/pull-ups of bus are properly configured
> > > > > > > > > > > Timed out in wait_for_event: status=0000
> > > > > > > > > > > Check if pads/pull-ups of bus are properly configured
> > > > > > > > > > > Timed out in wait_for_event: status=0000
> > > > > > > > > > > Check if pads/pull-ups of bus are properly configured
> > > > > > > > > > > Timed out in wait_for_event: status=0000
> > > > > > > > > > > Check if pads/pull-ups of bus are properly configured
> > > > > > > > > > > Timed out in wait_for_event: status=0000
> > > > > > > > > > > Check if pads/pull-ups of bus are properly configured
> > > > > > > > > > > Timed out in wait_for_event: status=0000
> > > > > > > > > > > Check if pads/pull-ups of bus are properly configured
> > > > > > > > > > > Timed out in wait_for_event: status=0000
> > > > > > > > > > > Check if pads/pull-ups of bus are properly configured
> > > > > > > > > > > Timed out in wait_for_event: status=0000
> > > > > > > > > > > Check if pads/pull-ups of bus are properly configured
> > > > > > > > > > > Timed out in wait_for_event: status=0000
> > > > > > > > > > > Check if pads/pull-ups of bus are properly configured
> > > > > > > > > > > Timed out in wait_for_event: status=0000
> > > > > > > > > > > Check if pads/pull-ups of bus are properly configured
> > > > > > > > > > > Timed out in wait_for_event: status=0000
> > > > > > > > > > > Check if pads/pull-ups of bus are properly configured
> > > > > > > > > > > Timed out in wait_for_event: status=0000
> > > > > > > > > > > Check if pads/pull-ups of bus are properly configured
> > > > > > > > > > > Timed out in wait_for_event: status=0000
> > > > > > > > > > > Check if pads/pull-ups of bus are properly configured
> > > > > > > > > > > Timed out in wait_for_event: status=0000
> > > > > > > > > > > Check if pads/pull-ups of bus are properly configured
> > > > > > > > > > > i2c_read (addr phase): pads on bus probably not configured (status=0x10)
> > > > > > > > > > > i2c_write: timed out writig last byte!
...
> > > > Now I was able to find commit which is causing above i2c problems:
> > > > "Check if pads/pull-ups of bus are properly configured"
> > > > 
> > > > It is d5243359e1afc957acd373dbbde1cf6c70ee5485:
> > > > 
> > > >       OMAP24xx I2C: Add support for set-speed
> > > >       Adds support for set-speed on the OMAP24xx I2C Adapter.
> > > >       Changes to omap24_i2c_write(...) for polling ARDY Bit from IRQ-Status.
> > > >       Otherwise on a subsequent call the transfer of last byte from the
> > > >       predecessor is aborted and therefore lost. For exmaple when
> > > >       i2c_write(...) is followed by a i2c_setspeed(...) (which has to
> > > >       deactivate and activate master for changing psc,...).
> > > >       Minor cosmetical changes.
> > > >       Signed-off-by: Hannes Petermaier <oe5hpm@oevsv.at>
> > > >       Cc: Heiko Schocher <hs@denx.de>
> > > > 
> > > > U-Boot version prior this command does not report those i2c errors.
...
> > I applied revert of above change on top of the master u-boot branch and
> > i2c bus num 1 (second) started working on N900 hw:
> > 
> > diff --git a/drivers/i2c/omap24xx_i2c.c b/drivers/i2c/omap24xx_i2c.c
> > index 0af4e333c4..a49cf89712 100644
> > --- a/drivers/i2c/omap24xx_i2c.c
> > +++ b/drivers/i2c/omap24xx_i2c.c
> > @@ -820,16 +820,6 @@ static int __omap24_i2c_write(void __iomem *i2c_base, int ip_rev, int waitdelay,
> >   		}
> >   	}
> > -	/*
> > -	 * poll ARDY bit for making sure that last byte really has been
> > -	 * transferred on the bus.
> > -	 */
> > -	do {
> > -		status = wait_for_event(i2c_base, ip_rev, waitdelay);
> > -	} while (!(status & I2C_STAT_ARDY) && timeout--);
> > -	if (timeout <= 0)
> > -		printf("i2c_write: timed out writig last byte!\n");
> > -
> >   wr_exit:
> >   	flush_fifo(i2c_base, ip_rev);
> >   	omap_i2c_write_reg(i2c_base, ip_rev, 0xFFFF, OMAP_I2C_STAT_REG);
> > 
> > I have looked into i2c-omap.c linux kernel driver and its transfer
> > function does not have any such code for waiting ARDY bit.
> 
> Ok...
> 
> > Why it is there? I have not able to find any information and that
> > comment is strange... it looks like it was incomplete/broken? workaround
> > about other issue.
> 
> Hmm.. ARDY bit means:
> """
> The current transaction is finished and the module registers
> can be accessed.
> """
> 
> Seems not to bad to me to check this bit! ... but may missing
> on transaction start some prerequisite is missing for triggering
> this bit? And so, this additional check only leads in a loop
> going into timeout?
> 
> > As you can see in log, at the first call status flags contains value
> > 0x0100 and on all other calls it contains just 0x000 status flags.
> > 
> > Therefore ARDY bit is never set, but i2c transfer works fine.
> 
> Hmm... so the question why does this bit not pop up on transfer
> end?

I do not know. I even do not have documentation for this i2c bus IP.
More suspicious is that this happens only for bus num 1.

Bus num 0 does not cause any issue and is working with and also without
that patch.

> I just can speculate that adding this polling ARDY bit loop
> changes the timing... and fixed an underlying bug, yes...
> 
> but if this bit never pop up, there must come the printf
> "i2c_write: timed out writig last byte!" for each i2c transfer.

Yes, it is for every bus num 1 transfer. But in N900 code there is just
one write on i2c bus 1 (to reset LED). All other devices which are
accessed from U-Boot on N900 are on i2c bus 0.

> Hannes may you can check if this is the case for you?
> 
> why does nobody claimed that this message pops up in the last 5 years?
> 
> or reverse asked, why does this bit may not pop up for you Pali?

Nokia N900 is Omap3430 HS device. I do not know how many people are
using latest U-Boot on HS devices and maybe Omap3430 has some i2c bugs?

But my main question is, why kernel's i2c driver does not do that check
for ARDY bit? And U-Boot is doing it?

> I have not yet time to check this... also I am unsure if I have a hw
> where I can try ...
> 
> bye,
> Heiko
> -- 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de