From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ivaylo Dimitrov Date: Fri, 30 Oct 2020 09:00:46 +0200 Subject: U-Boot i2c bus num 1 is broken on Nokia N900 In-Reply-To: References: <20200331224254.nzaotf5abj6w7n65@pali> <3c7dda52-10b3-e8c3-a382-785c80f124e7@wizzup.org> <20200402184231.jzhy36qfbi4drnli@pali> <20200425104217.pzp5zkhhft5a2he5@pali> <20200425115045.w45tb62cnwtq5umk@pali> <20200425121132.vnlurfxti6y433pq@pali> <20200425235459.bcoskmnux45x4iqs@pali> <948158e4-1c2c-9895-3c04-2a53198402f9@denx.de> <20201026214858.rowljynsc7vh2bjk@pali> Message-ID: <4fb91240-2a17-05ba-acdc-6c184ddfb250@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 wrote: >>>>>>>>> >>>>>>>>> On Saturday 25 April 2020 06:36:58 Adam Ford wrote: >>>>>>>>>> On Sat, Apr 25, 2020 at 5:42 AM Pali Roh?r >>>>>>>>>> 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. >>>>>>>>>>>>> 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 >>>>>> ????? Cc: Heiko Schocher >>>>>> >>>>>> 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 >> 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. Regards, Ivo