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

Ivaylo Dimitrov ivo.g.dimitrov.75 at gmail.com
Sat Oct 31 12:47:55 CET 2020



On 30.10.20 г. 9:24 ч., Heiko Schocher wrote:
> Hello Ivaylo,
> 
> Am 30.10.2020 um 08:00 schrieb Ivaylo Dimitrov:
>> Hi,
>>
>> On 29.10.20 г. 11:32 ч., Heiko Schocher wrote:
>>> Hello Ivaylo,
>>>
>>> Am 29.10.2020 um 08:51 schrieb Ivaylo Dimitrov:
>>>> Hi,
>>>>
>>>> On 28.10.20 г. 7:42 ч., Heiko Schocher wrote:
>>>>> Hello Pali,
>>>>>
>>>>> sorry for late response ...
>>>>>
>>>>> Am 26.10.2020 um 22:48 schrieb Pali Rohár:
>>>>>> 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 at 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 at 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)
>>
>> How is that trace even possible? I built and tested u-boot here on my 
>> devices and I see the same message, but unless I am becoming blind, 
>> i2c_read() is never called from within i2c_write(). This is really 
>> suspicious.
> 
> Puh..
> 

It turned out it is WD that kicks in.

>>
>>>>>>>>>>>>>>> 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 ...
>>>>>
>>>>> Yes, my first thought too!
>>>>>
>>>>> This board needs a complete cleanup.
>>>>>
>>>>>> I converted code to CONFIG_DM_I2C and nothing was changed, issue is
>>>>>> still there...
>>>>>
>>>>> Ok.
>>>>>
>>>>>>>>>>>>>>         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 at oevsv.at>
>>>>>>>>       Cc: Heiko Schocher <hs at 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.
>>>>>
>>>>> Ok, fine.
>>>>>
>>>>>> 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.
>>>>>
>>>>> Ok, this part definetely changes behaviour in timing...
>>>>>
>>>>>> 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.
>>>>>
>>>>
>>>> Correct, but this waiting seems to be described in the TRM (see 
>>>> Figure 18-46 and Figure 18-47), albeit for the polling mode. Though, 
>>>> in general flow diagrams (Figure 18-29 and Figure 18-31), there is 
>>>> no such loop.
>>>
>>> Could you provide a link to the TRM?
>>>
>>
>> https://www.ti.com/pdfs/wtbu/OMAP34xx_ES3.1x_PUBLIC_TRM_vZN.pdf
> 
> thanks! Seems I was blind ...
> 
>>>> However, by looking to __omap24_i2c_init(), it is not clear to me 
>>>> what mode uboot uses as it enables almost all interrupt bits in 
>>>> OMAP_I2C_IE_REG and loops waiting for events at the same time. Could 
>>>> someone confirm if uboot uses interrupt or polling mode? As this 
>>>> changes the things in regards to ARDY bit a lot, IIUC.
>>>
>>> I think, u-boot only polls the registers, while enabling all
>>> interrupt bits ...
>>>
>>>>> 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.
>>>>> """
>>>>>
>>>>
>>>> But it seems there is something weird about ARDY bit, at least in 
>>>> interrupt mode, see linux kernel commits cb527ede1b and 4cdbf7d346. 
>>>> So it seems ARDY bit shall be cleared twice.
>>>
>>> Hmm.. yes.
>>>
>>>>> 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.
>>>>>
>>>>
>>>> What looks wrong to me is that __omap24_i2c_init() enables all 
>>>> interrupts in OMAP_I2C_IE_REG, however, the precondition for polling 
>>>> mode according to Figure 18-29 is that all interrupts shall be 
>>>> disabled.
>>>>
>>>>> Hmm... so the question why does this bit not pop up on transfer
>>>>> end?
>>>>>
>>>>
>>>> It seems it never pops out. Moreover, if we look at the logs, the 
>>>> first wait_for_event() seems to 
>>>
>>> yes.
>>>
>>>> timeout with status=0100, that is with BF bit set. What looks 
>>>> suspicions here, is that the only bit we ever see set, is the bit we 
>>>> don't have interrupt bit enabled for.
>>>>
>>>> Pali, maybe you should try to comment the code that sets interrupt 
>>>> bits in __omap24_i2c_init() (the block that starts with "if (ip_rev 
>>>> == OMAP_I2C_REV_V1)") and see if it makes any difference. Also, 
>>>> maybe add more traces in __omap24_i2c_write() to see which exactly 
>>>> wait_for_event() call times out.
>>>
>>> May worth a try.
>>>
>>> More mystically is, that it works fine for Pali on bus 0 but
>>> makes problems on bus 1 ... !?
>>>
>>> Pali: Do you have also on bus 0 i2c writes?
>>>
>>>>> 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.
>>>>>
>>
>> And this is what happens, we have that once, as we write only one byte 
>> to bus 1.
>>
>>>>> 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?
>>>>>
>>
>> I can confirm I see it on the 2 devices I tested here.
>>
>> What is worse, is that writing on bus 1 does not fail every time. I 
>> increased I2C_TIMEOUT to 100000 (the value from the TRM) and it seems 
>> now after power-cycle, write succeeds almost every time, however, 
>> after a reset command from u-boot, it usually fails. And with that 
>> increased timeout,when it fails I see:
>>
>> Check if pads/pull-ups of bus are properly configured
>> i2c_read (addr phase): pads on bus probably not configured (status=0x10)
>>
>> message 5 times during the failing write.
>>
>> How we end up there, is a mystery to me.
> 
> Yes...
> 
> Ok, on page 2671 is described when this ARDY event is triggered, so
> if we wait for it, we must first check, if the prerequisite is met...
> 
> Hmm.. the ARDY bit is also for interrupt mode described, see page 2778
> Figure 18-31 ... so I think it is correct to check it ... but
> I do not see, why we should wait for it in a loop and print
> an error if bit does not come up
> 
> But I have no time to dig into this ...
> 

Looks like slave device is misbehaving after a reset command. We changed 
the write to not reset the slave, but instead to disable the LED engine 
and it seems there are no more timeouts/errors. However, I guess it 
still makes sense some more complicated logic to be implemented there 
(like, if we write only one byte, do not wait for ARDY), as I doubt 
lp5523 is the only device that misbehaves on reset.

Ivo


More information about the U-Boot mailing list