* [U-Boot] mpc512x: Trouble migrating from 2012.07 to 2013.01
@ 2013-01-23 15:24 Mats Kärrman
2013-01-23 21:25 ` Wolfgang Denk
0 siblings, 1 reply; 11+ messages in thread
From: Mats Kärrman @ 2013-01-23 15:24 UTC (permalink / raw)
To: u-boot
> Now when I'm trying to migrate to U-Boot 2013.01 I face problems with a sudden hang. I see console output until the "Flash:" from arch/powerpc/lib/board.c::board_init_r() but nothing more until the board reboots on a watchdog reset after 30s.
Found that it was looping endlessly in arch/powerpc/lib/ticks.S::wait_ticks(). Reverting commit "ppc: Create a stack frame for wait_ticks()" made everything work again.
I am not fluent enough in powerpc assembly or gcc stack frames for the same to see exactly what is wrong. Perhaps someone with the necessary mileage could take a look?
BR // Mats
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] mpc512x: Trouble migrating from 2012.07 to 2013.01 2013-01-23 15:24 [U-Boot] mpc512x: Trouble migrating from 2012.07 to 2013.01 Mats Kärrman @ 2013-01-23 21:25 ` Wolfgang Denk 2013-01-23 21:58 ` Mats Kärrman 0 siblings, 1 reply; 11+ messages in thread From: Wolfgang Denk @ 2013-01-23 21:25 UTC (permalink / raw) To: u-boot Dear Mats K?rrman, In message <ED3E0BCACD909541BA94A34C4A164D4C427B4AA5@post.tritech.se> you wrote: > > > Found that it was looping endlessly in arch/powerpc/lib/ticks.S::wait_ticks(). Reverting commit "ppc: Create a stack frame for wait_ticks()" made everything work again. This makes no sense to me - especially as it works on all other systems. > I am not fluent enough in powerpc assembly or gcc stack frames for the same to see exactly what is wrong. Perhaps someone with the necessary mileage could take a look? Well, you mentioned you were using a MPC5125 processor which is not supported in mainline U-Boot, and which is known to have enough differences in it's register layout that anything might be the cause for such issues. Is it correct to assume that you have a _lot_ of local modifiucations in your tree? I bet the problem lies there... Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Anything free is worth what you pay for it. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] mpc512x: Trouble migrating from 2012.07 to 2013.01 2013-01-23 21:25 ` Wolfgang Denk @ 2013-01-23 21:58 ` Mats Kärrman 2013-01-24 8:40 ` Joakim Tjernlund [not found] ` <OFD3C888A1.560BEE31-ONC1257AFD.002DB246-C1257AFD.002FAD45@LocalDomain> 0 siblings, 2 replies; 11+ messages in thread From: Mats Kärrman @ 2013-01-23 21:58 UTC (permalink / raw) To: u-boot Dear Wolfgang Denk, >> Found that it was looping endlessly in arch/powerpc/lib/ticks.S::wait_ticks(). Reverting commit "ppc: Create a stack frame for wait_ticks()" made everything work again. > > This makes no sense to me - especially as it works on all other > systems. If you say it works, it makes no sense to me either. I thought it might be some subtle difference in the way the e300c4 core or GCC works compared to other PPC cores. If there are no such differences, then a patch with no intended functional difference beyond keeping gdb happy should not make any difference, good or bad. > Well, you mentioned you were using a MPC5125 processor which is not > supported in mainline U-Boot, and which is known to have enough > differences in it's register layout that anything might be the cause > for such issues. > > Is it correct to assume that you have a _lot_ of local modifiucations > in your tree? I bet the problem lies there... I wouldn't say a lot. I'm using only the serial console to be able to download and flash images (using CFI flash) and to boot Linux so the primary patches are the serial driver and the DRAM initialization. On top of that there are some minimal I/O initialization and of course I went through and updated the register mappings for the relevant peripherals. It worked fine with 2009.03 and with 2012.07 but of course I might have overlooked something. I will report back when / if I find out something more. I'm not much for betting though ;) Best regards, Mats ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] mpc512x: Trouble migrating from 2012.07 to 2013.01 2013-01-23 21:58 ` Mats Kärrman @ 2013-01-24 8:40 ` Joakim Tjernlund [not found] ` <OFD3C888A1.560BEE31-ONC1257AFD.002DB246-C1257AFD.002FAD45@LocalDomain> 1 sibling, 0 replies; 11+ messages in thread From: Joakim Tjernlund @ 2013-01-24 8:40 UTC (permalink / raw) To: u-boot Mats K?rrman <Mats.Karrman@tritech.se> wrote on 2013/01/23 22:58:56: > > Dear Wolfgang Denk, > > >> Found that it was looping endlessly in arch/powerpc/lib/ticks.S::wait_ticks > (). Reverting commit "ppc: Create a stack frame for wait_ticks()" made > everything work again. > > > > This makes no sense to me - especially as it works on all other > > systems. > Me neither, there is not a lot details. I do recall having other problems with wait_ticks from time to time, sometimes the TB counter(mtfbu, mftb in get_ticks) would not increment so one would just loop forever in wait_ticks. This problem had nothing to do with my patch though. Never got around to finding out what caused it, we ended up blaming unstable HW. Some ideas though: - Perhaps you have some alignment problem, try adding nop's here and there. - My patch uses r0(which is what one should use to make gdb happy) instead of r8 to stash the LR. Possibly you have some odd dependency on r0, try using r8 again. None of the above is a real fix tough. > If you say it works, it makes no sense to me either. I thought it might be > some subtle difference in the way the e300c4 core or GCC works compared to > other PPC cores. If there are no such differences, then a patch with no > intended functional difference beyond keeping gdb happy should not make any > difference, good or bad. hmm, we use e300c2 with no problem. Jocke ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <OFD3C888A1.560BEE31-ONC1257AFD.002DB246-C1257AFD.002FAD45@LocalDomain>]
* [U-Boot] mpc512x: Trouble migrating from 2012.07 to 2013.01 [not found] ` <OFD3C888A1.560BEE31-ONC1257AFD.002DB246-C1257AFD.002FAD45@LocalDomain> @ 2013-01-24 8:58 ` Joakim Tjernlund [not found] ` <OF5A9C9414.751DE82C-ONC1257AFD.0030970D-C1257AFD.00314F3A@LocalDomain> 1 sibling, 0 replies; 11+ messages in thread From: Joakim Tjernlund @ 2013-01-24 8:58 UTC (permalink / raw) To: u-boot Joakim Tjernlund/Transmode wrote on 2013/01/24 09:40:45: > From: Joakim Tjernlund/Transmode > To: Mats K?rrman <Mats.Karrman@tritech.se>, > Cc: "u-boot at lists.denx.de" <u-boot@lists.denx.de>, Wolfgang Denk <wd@denx.de> > Date: 2013/01/24 09:40 > Subject: RE: [U-Boot] mpc512x: Trouble migrating from 2012.07 to 2013.01 > > Mats K?rrman <Mats.Karrman@tritech.se> wrote on 2013/01/23 22:58:56: > > > > Dear Wolfgang Denk, > > > > >> Found that it was looping endlessly in arch/powerpc/lib/ticks.S::wait_ticks > > (). Reverting commit "ppc: Create a stack frame for wait_ticks()" made > > everything work again. > > > > > > This makes no sense to me - especially as it works on all other > > > systems. > > > > Me neither, there is not a lot details. I do recall having other problems with > wait_ticks from time to time, sometimes the TB counter(mtfbu, mftb in get_ticks) > would not increment so one would just loop forever in wait_ticks. This problem > had nothing to do with my patch though. > Never got around to finding out what caused it, we ended up blaming unstable HW. > > Some ideas though: > - Perhaps you have some alignment problem, try adding nop's here and there. > - My patch uses r0(which is what one should use to make gdb happy) instead of r8 > to stash the LR. Possibly you have some odd dependency on r0, try using r8 again. Try getting LR from stack instead of trusting r0 to be valid: - mtlr r0 /* restore link register */ + lwz r0, 20(r1) /* restore link register */ + mtlr r0 This is what one should do but as we have complete control of r0 here we don't need to. hmm, do you have WATCHDOG_RESET defined? does it use r0? I guess the above patch would make wait_ticks safer from accidental use by WATCHDOG_RESET, if it works for you, please send a proper patch to u-boot. Jocke ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <OF5A9C9414.751DE82C-ONC1257AFD.0030970D-C1257AFD.00314F3A@LocalDomain>]
* [U-Boot] mpc512x: Trouble migrating from 2012.07 to 2013.01 [not found] ` <OF5A9C9414.751DE82C-ONC1257AFD.0030970D-C1257AFD.00314F3A@LocalDomain> @ 2013-01-24 9:21 ` Joakim Tjernlund 2013-01-24 11:54 ` Mats Kärrman 2013-01-24 13:31 ` Mats Kärrman 0 siblings, 2 replies; 11+ messages in thread From: Joakim Tjernlund @ 2013-01-24 9:21 UTC (permalink / raw) To: u-boot Joakim Tjernlund/Transmode wrote on 2013/01/24 09:58:35: > > Joakim Tjernlund/Transmode wrote on 2013/01/24 09:40:45: > > > From: Joakim Tjernlund/Transmode > > To: Mats K?rrman <Mats.Karrman@tritech.se>, > > Cc: "u-boot at lists.denx.de" <u-boot@lists.denx.de>, Wolfgang Denk <wd@denx.de> > > Date: 2013/01/24 09:40 > > Subject: RE: [U-Boot] mpc512x: Trouble migrating from 2012.07 to 2013.01 > > > > Mats K?rrman <Mats.Karrman@tritech.se> wrote on 2013/01/23 22:58:56: > > > > > > Dear Wolfgang Denk, > > > > > > >> Found that it was looping endlessly in arch/powerpc/lib/ticks.S::wait_ticks > > > (). Reverting commit "ppc: Create a stack frame for wait_ticks()" made > > > everything work again. > > > > > > > > This makes no sense to me - especially as it works on all other > > > > systems. > > > > > > > Me neither, there is not a lot details. I do recall having other problems with > > wait_ticks from time to time, sometimes the TB counter(mtfbu, mftb in get_ticks) > > would not increment so one would just loop forever in wait_ticks. This problem > > had nothing to do with my patch though. > > Never got around to finding out what caused it, we ended up blaming unstable HW. > > > > Some ideas though: > > - Perhaps you have some alignment problem, try adding nop's here and there. > > - My patch uses r0(which is what one should use to make gdb happy) instead of r8 > > to stash the LR. Possibly you have some odd dependency on r0, try using r8 again. > > Try getting LR from stack instead of trusting r0 to be valid: > - mtlr r0 /* restore link register */ > + lwz r0, 20(r1) /* restore link register */ > + mtlr r0 > This is what one should do but as we have complete control of r0 here we don't need to. > > hmm, do you have WATCHDOG_RESET defined? does it use r0? > I guess the above patch would make wait_ticks safer from accidental use by > WATCHDOG_RESET, > if it works for you, please send a proper patch to u-boot. Looking at the watchdog impl. I see it can be normal C code. This makes wait_ticks unsafe (even before my patch) as wait_ticks relies on r6 and r7 (and with my patch r0 too) to be unmodified. This MIGHT be the fix: --- a/arch/powerpc/lib/ticks.S +++ b/arch/powerpc/lib/ticks.S @@ -56,13 +56,17 @@ wait_ticks: /* Calculate end time */ addc r7, r4, r7 /* Compute end time lower */ addze r6, r3 /* and end time upper */ - + stw r7, 4(r1) /* Stash r6 and r7 */ + stw r6, 8(r1) WATCHDOG_RESET /* Trigger watchdog, if needed */ + lwz r7, 4(r1) + lwz r6, 8(r1) 1: bl get_ticks /* Get current time */ subfc r4, r4, r7 /* Subtract current time from end time */ subfe. r3, r3, r6 bge 1b /* Loop until time expired */ - mtlr r0 /* restore link register */ + lwz r0, 20(r1) /* restore link register */ + mtlr r0 addi r1,r1,16 blr Not sure about the 4 and 8 offsets though Jocke ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] mpc512x: Trouble migrating from 2012.07 to 2013.01 2013-01-24 9:21 ` Joakim Tjernlund @ 2013-01-24 11:54 ` Mats Kärrman 2013-01-24 13:31 ` Mats Kärrman 1 sibling, 0 replies; 11+ messages in thread From: Mats Kärrman @ 2013-01-24 11:54 UTC (permalink / raw) To: u-boot Joakim Tjernlund/Transmode wrote on 2013/01/24 09:58:35: > > > > > > Me neither, there is not a lot details. I do recall having other > problems with > > > wait_ticks from time to time, sometimes the TB counter(mtfbu, mftb in > get_ticks) > > > would not increment so one would just loop forever in wait_ticks. This > problem > > > had nothing to do with my patch though. > > > Never got around to finding out what caused it, we ended up blaming > unstable HW. > > > > > > Some ideas though: > > > - Perhaps you have some alignment problem, try adding nop's here and > there. > > > - My patch uses r0(which is what one should use to make gdb happy) > instead of r8 > > > to stash the LR. Possibly you have some odd dependency on r0, try > using r8 again. > > > > Try getting LR from stack instead of trusting r0 to be valid: > > - mtlr r0 /* restore link register */ > > + lwz r0, 20(r1) /* restore link register */ > > + mtlr r0 > > This is what one should do but as we have complete control of r0 here we > don't need to. > > > > hmm, do you have WATCHDOG_RESET defined? does it use r0? > > I guess the above patch would make wait_ticks safer from accidental use > by > > WATCHDOG_RESET, > > if it works for you, please send a proper patch to u-boot. > > Looking at the watchdog impl. I see it can be normal C code. This makes > wait_ticks unsafe > (even before my patch) as wait_ticks relies on r6 and r7 (and with my > patch r0 too) > to be unmodified. > > This MIGHT be the fix: > --- a/arch/powerpc/lib/ticks.S > +++ b/arch/powerpc/lib/ticks.S > @@ -56,13 +56,17 @@ wait_ticks: > /* Calculate end time */ > addc r7, r4, r7 /* Compute end time lower */ > addze r6, r3 /* and end time upper */ > - > + stw r7, 4(r1) /* Stash r6 and r7 */ > + stw r6, 8(r1) > WATCHDOG_RESET /* Trigger watchdog, if needed */ > + lwz r7, 4(r1) > + lwz r6, 8(r1) > 1: bl get_ticks /* Get current time */ > subfc r4, r4, r7 /* Subtract current time from end time */ > subfe. r3, r3, r6 > bge 1b /* Loop until time expired */ > > - mtlr r0 /* restore link register */ > + lwz r0, 20(r1) /* restore link register */ > + mtlr r0 > addi r1,r1,16 > blr > > Not sure about the 4 and 8 offsets though > > Jocke Thanks for feedback! I will look into this ASAP (maybe not until tomorrow). I'll report back as soon as I have a result. BR, Mats ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] mpc512x: Trouble migrating from 2012.07 to 2013.01 2013-01-24 9:21 ` Joakim Tjernlund 2013-01-24 11:54 ` Mats Kärrman @ 2013-01-24 13:31 ` Mats Kärrman 2013-01-24 13:48 ` Joakim Tjernlund 1 sibling, 1 reply; 11+ messages in thread From: Mats Kärrman @ 2013-01-24 13:31 UTC (permalink / raw) To: u-boot Joakim Tjernlund/Transmode wrote on 2013/01/24 09:21: > Looking at the watchdog impl. I see it can be normal C code. This makes > wait_ticks unsafe > (even before my patch) as wait_ticks relies on r6 and r7 (and with my > patch r0 too) > to be unmodified. Yes! I can see in the assembly from my watchdog_reset() (arch/powerpc/cpu/mpc5125/cpu.c) that it does not touch r7 and r8 but indeed r0 -- for storing lr in fact so the endless loop is explained! Many thanks! I'll prepare a patch as soon as I've got one tested. BR // Mats ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] mpc512x: Trouble migrating from 2012.07 to 2013.01 2013-01-24 13:31 ` Mats Kärrman @ 2013-01-24 13:48 ` Joakim Tjernlund 0 siblings, 0 replies; 11+ messages in thread From: Joakim Tjernlund @ 2013-01-24 13:48 UTC (permalink / raw) To: u-boot Mats K?rrman <Mats.Karrman@tritech.se> wrote on 2013/01/24 14:31:02: > > Joakim Tjernlund/Transmode wrote on 2013/01/24 09:21: > > Looking at the watchdog impl. I see it can be normal C code. This makes > > wait_ticks unsafe > > (even before my patch) as wait_ticks relies on r6 and r7 (and with my > > patch r0 too) > > to be unmodified. > > Yes! > I can see in the assembly from my watchdog_reset() (arch/powerpc/cpu/mpc5125/ > cpu.c) that it does not touch r7 and r8 but indeed r0 -- for storing lr in > fact so the endless loop is explained! > > Many thanks! > I'll prepare a patch as soon as I've got one tested. Great, just one thing though. I think the 4 resp.8 stack offsets should be 8 resp. 12 instead. Jocke ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] mpc512x: Trouble migrating from 2012.07 to 2013.01 @ 2013-01-23 13:17 Mats Kärrman 2013-01-23 20:40 ` Wolfgang Denk 0 siblings, 1 reply; 11+ messages in thread From: Mats Kärrman @ 2013-01-23 13:17 UTC (permalink / raw) To: u-boot Hi! I have a system using a MPC5125 processor working fine using U-Boot 2012.07. I have based my board adaption etc on the mpc5121ads board and also patched the necessary parts of the serial port driver. Now when I'm trying to migrate to U-Boot 2013.01 I face problems with a sudden hang. I see console output until the "Flash:" from arch/powerpc/lib/board.c::board_init_r() but nothing more until the board reboots on a watchdog reset after 30s. I have looked for relevant changes in the U-Boot code and tried some printf debugging. So far I have been unable to pin down exactly where things go wrong. Question: To help me decide where to look I would like to know if anyone has built the mpc5121ads board configuration using U-Boot 2013.01 and actually tested that it is still working? Thanks & best regards, Mats ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] mpc512x: Trouble migrating from 2012.07 to 2013.01 2013-01-23 13:17 Mats Kärrman @ 2013-01-23 20:40 ` Wolfgang Denk 0 siblings, 0 replies; 11+ messages in thread From: Wolfgang Denk @ 2013-01-23 20:40 UTC (permalink / raw) To: u-boot Dear Mats, In message <ED3E0BCACD909541BA94A34C4A164D4C427B39B4@post.tritech.se> you wrote: > > Question: To help me decide where to look I would like to know if anyone has built the mpc5121ads board configuration using U-Boot 2013.01 and actually tested that it is still working? Yes, v2012.01 is working on the MPC5121ADS board: U-Boot 2013.01 (Jan 23 2013 - 21:37:51)MPC512X CPU: MPC5121e rev. 2.0, Core e300c4 at 400 MHz, CSB at 200 MHz (RSR=0x1003) Board: MPC5121ADS rev. 0x0400 (CPLD rev. 0x05) I2C: ready DRAM: 512 MiB Flash: 64 MiB NAND: 1024 MiB PCI: Bus Dev VenId DevId Class Int In: serial Out: serial Err: serial Net: FEC IDE: Bus 0: ............................................................** Timeout ** Type run flash_nfs to mount root filesystem over NFS Hit any key to stop autoboot: 0 => Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de In any group of employed individuals the only naturally early riser is _always_ the office manager, who will _always_ leave reproachful little notes ... on the desks of their subordinates. - Terry Pratchett, _Lords and Ladies_ ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-01-24 13:48 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-01-23 15:24 [U-Boot] mpc512x: Trouble migrating from 2012.07 to 2013.01 Mats Kärrman 2013-01-23 21:25 ` Wolfgang Denk 2013-01-23 21:58 ` Mats Kärrman 2013-01-24 8:40 ` Joakim Tjernlund [not found] ` <OFD3C888A1.560BEE31-ONC1257AFD.002DB246-C1257AFD.002FAD45@LocalDomain> 2013-01-24 8:58 ` Joakim Tjernlund [not found] ` <OF5A9C9414.751DE82C-ONC1257AFD.0030970D-C1257AFD.00314F3A@LocalDomain> 2013-01-24 9:21 ` Joakim Tjernlund 2013-01-24 11:54 ` Mats Kärrman 2013-01-24 13:31 ` Mats Kärrman 2013-01-24 13:48 ` Joakim Tjernlund -- strict thread matches above, loose matches on Subject: below -- 2013-01-23 13:17 Mats Kärrman 2013-01-23 20:40 ` Wolfgang Denk
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.