* ep93xx irq-vic regression
@ 2013-03-15 21:46 H Hartley Sweeten
2013-03-15 23:11 ` Ryan Mallon
0 siblings, 1 reply; 8+ messages in thread
From: H Hartley Sweeten @ 2013-03-15 21:46 UTC (permalink / raw)
To: linux-arm-kernel
Hello all,
Yesterday I tried upgrading my ep93xx system from 3.6.6 to 3.8.3.
The kernel compiles fine but the machine would not boot. It just
hangs when the bootloader (RedBoot) tries to exec the kernel.
I then tried 3.7.0 and it works fine.
I did a bisect and got down to this commit:
$ git bisect bad
07c9249f1fa90cc8189bed44c0bcece664596a72 is the first bad commit
commit 07c9249f1fa90cc8189bed44c0bcece664596a72
Author: Linus Walleij <linus.walleij@linaro.org>
Date: Tue Oct 16 18:50:00 2012 +0100
ARM: 7554/1: VIC: use irq_domain_add_simple()
Any ideas on what might be the problem?
Regards,
Hartley
^ permalink raw reply [flat|nested] 8+ messages in thread* ep93xx irq-vic regression 2013-03-15 21:46 ep93xx irq-vic regression H Hartley Sweeten @ 2013-03-15 23:11 ` Ryan Mallon 2013-03-16 0:16 ` H Hartley Sweeten 0 siblings, 1 reply; 8+ messages in thread From: Ryan Mallon @ 2013-03-15 23:11 UTC (permalink / raw) To: linux-arm-kernel On 16/03/13 08:46, H Hartley Sweeten wrote: > Hello all, > > Yesterday I tried upgrading my ep93xx system from 3.6.6 to 3.8.3. > The kernel compiles fine but the machine would not boot. It just > hangs when the bootloader (RedBoot) tries to exec the kernel. > > I then tried 3.7.0 and it works fine. > > I did a bisect and got down to this commit: > > $ git bisect bad > 07c9249f1fa90cc8189bed44c0bcece664596a72 is the first bad commit > commit 07c9249f1fa90cc8189bed44c0bcece664596a72 > Author: Linus Walleij <linus.walleij@linaro.org> > Date: Tue Oct 16 18:50:00 2012 +0100 > > ARM: 7554/1: VIC: use irq_domain_add_simple() > > Any ideas on what might be the problem? I don't know the code very well, but looking at the change we go from being having the first vic bank being registered with irq_domain_add_legacy to irq_domain_add_linear (since first_irq is zero). It looks like the linear domain needs to have it's revmap initialised with a call to either irq_domain_associate_many or irq_create_strict_mappings, which I can't find a call to. Are we left with the first vic bank having no revmap? That's from a ten minute, uninformed glance at the code though, so I might be off base :-). ~Ryan ^ permalink raw reply [flat|nested] 8+ messages in thread
* ep93xx irq-vic regression 2013-03-15 23:11 ` Ryan Mallon @ 2013-03-16 0:16 ` H Hartley Sweeten 2013-03-16 13:49 ` Florian Fainelli 2013-03-18 20:21 ` Linus Walleij 0 siblings, 2 replies; 8+ messages in thread From: H Hartley Sweeten @ 2013-03-16 0:16 UTC (permalink / raw) To: linux-arm-kernel On Friday, March 15, 2013 4:12 PM, Ryan Mallon wrote: > On 16/03/13 08:46, H Hartley Sweeten wrote: > >> Hello all, >> >> Yesterday I tried upgrading my ep93xx system from 3.6.6 to 3.8.3. >> The kernel compiles fine but the machine would not boot. It just >> hangs when the bootloader (RedBoot) tries to exec the kernel. >> >> I then tried 3.7.0 and it works fine. >> >> I did a bisect and got down to this commit: >> >> $ git bisect bad >> 07c9249f1fa90cc8189bed44c0bcece664596a72 is the first bad commit >> commit 07c9249f1fa90cc8189bed44c0bcece664596a72 >> Author: Linus Walleij <linus.walleij@linaro.org> >> Date: Tue Oct 16 18:50:00 2012 +0100 >> >> ARM: 7554/1: VIC: use irq_domain_add_simple() >> >> Any ideas on what might be the problem? > > > I don't know the code very well, but looking at the change we go > from being having the first vic bank being registered with > irq_domain_add_legacy to irq_domain_add_linear (since first_irq > is zero). It looks like the linear domain needs to have it's > revmap initialised with a call to either irq_domain_associate_many > or irq_create_strict_mappings, which I can't find a call to. Are > we left with the first vic bank having no revmap? That's from a > ten minute, uninformed glance at the code though, so I might be > off base :-). Grrr... chasing my tail... It looks like the commit above _did_ have a couple bugs but they were fixed: commit 07c9249f1fa90cc8189bed44c0bcece664596a72 Author: Linus Walleij <linus.walleij@linaro.org> Date: Tue Oct 16 18:50:00 2012 +0100 ARM: 7554/1: VIC: use irq_domain_add_simple() commit 946c59a08a2497303750c0fee4367ca32009155c Author: Russell King <rmk+kernel@arm.linux.org.uk> Date: Thu Nov 8 10:48:30 2012 +0000 ARM: vic: fix build warning caused by previous commit commit 5ced33bc06e7e76cb8a236f15bff49eb6155b618 Author: Linus Walleij <linus.walleij@linaro.org> Date: Wed Dec 26 01:39:16 2012 +0100 ARM: 7611/1: VIC: fix bug in VIC irqdomain code It appears the reason my system was hanging is due to this commit: commit 210dce5faf89c9677ac1a6273bc53f130843539f Author: Florian Fainelli <florian@openwrt.org> Date: Mon Dec 10 22:21:19 2012 +0100 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. Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com> Cc: Ryan Mallon <rmallon@gmail.com> --- arch/arm/mach-ep93xx/include/mach/uncompress.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) 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); } -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* ep93xx irq-vic regression 2013-03-16 0:16 ` H Hartley Sweeten @ 2013-03-16 13:49 ` Florian Fainelli 2013-03-17 0:06 ` Ryan Mallon 2013-03-18 20:21 ` Linus Walleij 1 sibling, 1 reply; 8+ messages in thread From: Florian Fainelli @ 2013-03-16 13:49 UTC (permalink / raw) To: linux-arm-kernel 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. -- Florian ^ permalink raw reply [flat|nested] 8+ messages in thread
* ep93xx irq-vic regression 2013-03-16 13:49 ` Florian Fainelli @ 2013-03-17 0:06 ` Ryan Mallon 2013-03-17 14:48 ` Florian Fainelli 0 siblings, 1 reply; 8+ messages in thread From: Ryan Mallon @ 2013-03-17 0:06 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* ep93xx irq-vic regression 2013-03-17 0:06 ` Ryan Mallon @ 2013-03-17 14:48 ` Florian Fainelli 0 siblings, 0 replies; 8+ messages in thread From: Florian Fainelli @ 2013-03-17 14:48 UTC (permalink / raw) To: linux-arm-kernel 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! -- Florian ^ permalink raw reply [flat|nested] 8+ messages in thread
* ep93xx irq-vic regression 2013-03-16 0:16 ` H Hartley Sweeten 2013-03-16 13:49 ` Florian Fainelli @ 2013-03-18 20:21 ` Linus Walleij 2013-03-19 17:03 ` H Hartley Sweeten 1 sibling, 1 reply; 8+ messages in thread From: Linus Walleij @ 2013-03-18 20:21 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* ep93xx irq-vic regression 2013-03-18 20:21 ` Linus Walleij @ 2013-03-19 17:03 ` H Hartley Sweeten 0 siblings, 0 replies; 8+ messages in thread From: H Hartley Sweeten @ 2013-03-19 17:03 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-03-19 17:03 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-03-15 21:46 ep93xx irq-vic regression H Hartley Sweeten 2013-03-15 23:11 ` Ryan Mallon 2013-03-16 0:16 ` H Hartley Sweeten 2013-03-16 13:49 ` Florian Fainelli 2013-03-17 0:06 ` Ryan Mallon 2013-03-17 14:48 ` Florian Fainelli 2013-03-18 20:21 ` Linus Walleij 2013-03-19 17:03 ` H Hartley Sweeten
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.