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

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

On Monday 27 April 2020 09:03:13 Heiko Schocher wrote:
> Hello Pali,
> 
> Am 26.04.2020 um 01:54 schrieb Pali Roh?r:
> > Adding Hannes and Heiko to the loop, please look at this problem.
> > 
> > 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:
> > > > > > > > > Hi,
> > > > > > > > > 
> > > > > > > > > On 01/04/2020 00:42, Pali Roh?r wrote:
> > > > > > > > > > On Wednesday 01 April 2020 00:35:07 Pali Roh?r wrote:
> > > > > > > > > > > This patch series contain fixes for Nokia RX-51 board (aka N900).
> > > > > > > > > > > After these changes it is possible to run U-Boot in qemu emulator again.
> > > > > > > > > > > And U-Boot can boot kernel image from RAM, eMMC or OneNAND memory without
> > > > > > > > > > > problem.
> > > > > > > > > > 
> > > > > > > > > > But on real Nokia N900 device is U-Boot crashing in reboot loop.
> > > > > > > > > > 
> > > > > > > > > > I do not have serial console for Nokia N900 to debug this issue, but
> > > > > > > > > > seems that it is related to OMAP I2C and OMAP HS MMC code. Problem is
> > > > > > > > > > that there is no crash and even no error in qemu emulator so I cannot
> > > > > > > > > > debug this issue.
> > > > > > > > > > 
> > > > > > > > > > First problem is around /* reset lp5523 led */ code in rx51.c. On real
> > > > > > > > > > N900 device it generates repeating messages:
> > > > > > > > > > 
> > > > > > > > > >    Check if pads/pull-ups of bus are properly configured
> > > > > > > > > >    Timed out in wait_for_event: status=0000
> > > > > > > > > > 
> > > > > > > > > > When I commented that few lines all these messages disappeared. So
> > > > > > > > > > problem is with OMAP I2C.
> > ...
> > > > > > > > > > I remember that somebody had serial jig for Nokia N900, could somebody
> > > > > > > > > > look at this reboot loop problem?
> > > > > > > > > > 
> > > > > > > > > > And any idea how should be OMAP I2C configured in U-Boot to correctly
> > > > > > > > > > work?
> > > > > > > > > > 
> > > > > > > > > > Maybe I will try to find some time to git bisect which change broke
> > > > > > > > > > U-Boot on real N900 hardware.
> > > > > > > > > 
> > > > > > > > > Took latest u-boot master, applied patches and this is the result on
> > > > > > > > > serial (first part is NOLO booting, I think that can be ignored) [1].
> > > > > > > > 
> > > > > > > > ...
> > > > > > > > 
> > > > > > > > > U-Boot 2020.04-rc4-00033-g7dbafe0634-dirty (Apr 01 2020 - 12:15:47 +0200)
> > > > > > > > > 
> > > > > > > > > OMAP3530-HS ES3.1, CPU-OPP2, L3-165MHz, Max CPU Clock 600 MHz
> > > > > > > > > Nokia RX-51 + LPDDR/OneNAND
> > > > > > > > > I2C:   ready
> > > > > > > > > DRAM:  256 MiB
> > > > > > > > > NAND:  0 Bytes
> > > > > > > > 
> > > > > > > > Looks like that something with NAND is broken.
> 
> The board code in U-Boot is in a very old state... :-(
> 
> > > > > > > > 
> > > > > > > > > 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!
> > > > > > > > 
> > > > > > > > These i2c errors are caused by
> > > > > > > > 
> > > > > > > >        /* reset lp5523 led */
> > > > > > > >        i2c_set_bus_num(1);
> 
> deprecated ... the board code needs cleanup ...

I converted code to CONFIG_DM_I2C and nothing was changed, issue is
still there...

> > > > > > > >        state = 0xff;
> > > > > > > >        i2c_write(0x32, 0x3d, 1, &state, 1);
> > > > > > > >        i2c_set_bus_num(0);
> > > > > > > > 
> > > > > > > > Is there anything which needs to be done to initialize i2c bus 1?
> > > > > > > > Because this code is working fine on older U-Boot version.
> > > > > > > 
> > > > > > > Above code worked fine for U-Boot 2013.04, but in git version from
> > > > > > > January 2015 it prints above error messages.
> > > > > > > 
> > > > > > > On on internet forums I see these error messages also from other OMAP3
> > > > > > > board, e.g. beagle board.
> > > > > > > 
> > > > > > > Has somebody some working OMAP3 board? And can test if it works with
> > > > > > > recent version of U-Boot? I guess that above i2c problem would happen
> > > > > > > also on other OMAP3 boards.
> > > > > > 
> > > > > > There was a conversion a while ago to dm_i2c, and I converted my board
> > > > > > to support using the device tree during the SPL phase, and whenever I
> > > > > > am aware any driver has driver model (DM) support, I try to convert my
> > > > > > board.
> > > > > > 
> > > > > > I have a DM3730 and the last check I did was 2020.04-rc1, and it was working
> > > > > 
> > > > > Ok, so it either OMAP3430 specific problem or N900 board specific
> > > > > problem. N900 does not use driver model.
> > > > 
> > > > i have an OMAP3530 which is basically a 3430, and it works too.  I am
> > > > guessing the issue is unique to the N900 or the fact that it's
> > > > high-security.  Neither of my boards are HS parts.  They are both GP.
> > > 
> > > N900 is HS device, but I guess that should be caused by GP vs HS
> > > difference. Working i2c bus 0 and non-working i2c bus 1 could not be
> > > caused by GP vs HS difference. Also I guess that omap hs mmc would be
> > > same on GP and HS boards.
> > ...
> > > > > Before calling i2c_write(0x32, ...) I tried to call i2c_probe(0x32) and
> > > > > it returned error.
> > > > > 
> > > > > If I tried to call "i2c dev 1" in U-Boot console, I got tons of errors
> > > > > and basically U-Boot stopped responding.
> > > > > 
> > > > > So by above observation it looks like I2C bus num 1 does not work, but
> > > > > I2C bus num 0 works fine.
> > > > > 
> > > > > Do I need to call i2c_probe(...) before calling i2c_write(...)?
> > > > > 
> > > > > And is something special needed for initializing omap i2c bus num 1?
> > > > > Because from my above observation it looks like that something is
> > > > > missing for bus 1 which in older u-boot version was not needed.
> > 
> > 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.
> > 
> > Hannes, any idea how your patch could broke omap i2c i2c bus num 1 on
> > Nokia N900?
> 
> Hard to say here anything useful, as I do not have the hardware...
> 
> The above commit changes:
> 
> -               udelay(I2C_WAIT);
> +               udelay(adap->waitdelay);
> 
> May you can check, if adap->waitdelay is the same as I2C_WAIT ?

Yes, it is the same value.

Anyway, I have deeply looked at that commit again and it just adds
support for omap24_i2c_setspeed and into omap24_i2c_write adds following
change:

@@ -464,6 +502,15 @@ static int omap24_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,
 			goto wr_exit;
 		}
 	}
+	/*
+	 * poll ARDY bit for making sure that last byte really has been
+	 * transferred on the bus.
+	 */
+	do {
+		status = wait_for_event(adap);
+	} while (!(status & I2C_STAT_ARDY) && timeout--);
+	if (timeout <= 0)
+		printf("i2c_write: timed out writig last byte!\n");
 
 wr_exit:
 	flush_fifo(adap);

And this change is causing that non-functional i2c bus.

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.

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.

As you can see in log,@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.

> 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