| Message ID | ADE657CA350FB648AAC2C43247A983F0020980106CB7@AUSP01VMBX24.collaborationhost.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello, Le samedi 16 mars 2013 01:16:52, H Hartley Sweeten a écrit : [snip] > > ARM: ep93xx: properly wait for UART FIFO to be empty > > Reverting that one allows my system to boot with 3.9.0-rc2. > > Instead of removing the wait loop completely I propose this > patch instead. I can repost this patch in a new thread if > necessary. > > Regards, > Hartley > > --- > > From: H Hartley Sweeten <hsweeten@visionengravers.com> > Subject: [PATCH] arm: ep93xx: fix wait for UART FIFO to be empty > > commit 210dce5f "ARM: ep93xx: properly wait for UART FIFO to be empty" > > Removed the timeout loop while waiting for the uart transmit fifo to > empty. Some bootloaders leave the uart in a state where there might > be bytes in the uart that are not transmitted when execution is handed > over to the kernel. This results in a deadlocked system while waiting > for the fifo to empty. > > Add back the timeout wait to prevent the deadlock. > > Increase the wait time to hopefully prevent the decompressor corruption > that lead to commit 210dce5f. This corruption was probably due to a > slow uart baudrate. The 10* increase in the wait time should be enough > for all cases. Ok, your solution seems like it would work, when I come accross this bug I initially ended up doing the same thing and incrementing the number of iterations in the loop. I was not quite happy with that as it would still be highly depending on the clocking. Anyway, sorry for breaking your system with this commit.
On 17/03/13 00:49, Florian Fainelli wrote: > Hello, > > Le samedi 16 mars 2013 01:16:52, H Hartley Sweeten a écrit : > [snip] > >> >> ARM: ep93xx: properly wait for UART FIFO to be empty >> >> Reverting that one allows my system to boot with 3.9.0-rc2. >> >> Instead of removing the wait loop completely I propose this >> patch instead. I can repost this patch in a new thread if >> necessary. >> >> Regards, >> Hartley >> >> --- >> >> From: H Hartley Sweeten <hsweeten@visionengravers.com> >> Subject: [PATCH] arm: ep93xx: fix wait for UART FIFO to be empty >> >> commit 210dce5f "ARM: ep93xx: properly wait for UART FIFO to be empty" >> >> Removed the timeout loop while waiting for the uart transmit fifo to >> empty. Some bootloaders leave the uart in a state where there might >> be bytes in the uart that are not transmitted when execution is handed >> over to the kernel. This results in a deadlocked system while waiting >> for the fifo to empty. >> >> Add back the timeout wait to prevent the deadlock. >> >> Increase the wait time to hopefully prevent the decompressor corruption >> that lead to commit 210dce5f. This corruption was probably due to a >> slow uart baudrate. The 10* increase in the wait time should be enough >> for all cases. > > Ok, your solution seems like it would work, when I come accross this bug I > initially ended up doing the same thing and incrementing the number of > iterations in the loop. I was not quite happy with that as it would still be > highly depending on the clocking. Anyway, sorry for breaking your system with > this commit. Do you want to add an Acked/Reviewed-by Florian? I'll queue this up in my tree if you are happy with it. ~Ryan
On Sunday 17 March 2013 11:06:29 Ryan Mallon wrote: [snip] > >> From: H Hartley Sweeten <hsweeten@visionengravers.com> > >> Subject: [PATCH] arm: ep93xx: fix wait for UART FIFO to be empty > >> > >> commit 210dce5f "ARM: ep93xx: properly wait for UART FIFO to be empty" > >> > >> Removed the timeout loop while waiting for the uart transmit fifo to > >> empty. Some bootloaders leave the uart in a state where there might > >> be bytes in the uart that are not transmitted when execution is handed > >> over to the kernel. This results in a deadlocked system while waiting > >> for the fifo to empty. > >> > >> Add back the timeout wait to prevent the deadlock. > >> > >> Increase the wait time to hopefully prevent the decompressor corruption > >> that lead to commit 210dce5f. This corruption was probably due to a > >> slow uart baudrate. The 10* increase in the wait time should be enough > >> for all cases. > > > > Ok, your solution seems like it would work, when I come accross this bug I > > initially ended up doing the same thing and incrementing the number of > > iterations in the loop. I was not quite happy with that as it would still be > > highly depending on the clocking. Anyway, sorry for breaking your system with > > this commit. > > > Do you want to add an Acked/Reviewed-by Florian? I'll queue this > up in my tree if you are happy with it. Yes, I just re-tested with 10000 as a loop value, and it fixes the corruption I was seeing, you can add my Acked-by: Florian Fainelli <florian@openwrt.org> Thanks to both of you!
On Sat, Mar 16, 2013 at 1:16 AM, H Hartley Sweeten <hartleys@visionengravers.com> wrote: > It looks like the commit above _did_ have a couple bugs but they were > fixed: Oh thanks, I already was scared I had broken something again. The irqdomain stuff is a bit fragile, everytime I refactor something it blows up in my face. I hope there are no remaining problems. Yours, Linus Walleij
On Monday, March 18, 2013 1:22 PM, Linus Walleij wrote: > On Sat, Mar 16, 2013 at 1:16 AM, H Hartley Sweeten <hartleys@visionengravers.com> wrote: > >> It looks like the commit above _did_ have a couple bugs but they were >> fixed: > > Oh thanks, I already was scared I had broken something again. > The irqdomain stuff is a bit fragile, everytime I refactor something > it blows up in my face. I hope there are no remaining problems. Linus, Sorry about not putting you in the loop with this. The vic driver works fine on the ep93xx. As I stated, there _was_ a couple bugs in the driver but they have been fixed. It appears I was chasing a red herring with the vic driver. It ends up that my system was not booting due to a patch to the ep93xx uncompress.h header that was causing a deadlock in the early printk support. This has been fixed and Ryan Mallon has merged the patch into his ep93xx-fixes branch. Regards, Hartley
diff --git a/arch/arm/mach-ep93xx/include/mach/uncompress.h b/arch/arm/mach-ep93xx/include/mach/uncompress.h index d2afb4d..b5cc77d 100644 --- a/arch/arm/mach-ep93xx/include/mach/uncompress.h +++ b/arch/arm/mach-ep93xx/include/mach/uncompress.h @@ -47,9 +47,13 @@ static void __raw_writel(unsigned int value, unsigned int ptr) static inline void putc(int c) { - /* Transmit fifo not full? */ - while (__raw_readb(PHYS_UART_FLAG) & UART_FLAG_TXFF) - ; + int i; + + for (i = 0; i < 10000; i++) { + /* Transmit fifo not full? */ + if (!(__raw_readb(PHYS_UART_FLAG) & UART_FLAG_TXFF)) + break; + } __raw_writeb(c, PHYS_UART_DATA); }