| Message ID | CABymUCP4s+ePs-sphJkQ4b61EcNT51FQat2ADWoy1Tgww35GMg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sat, Sep 05, 2015 at 10:42:03PM +0800, Jun Nie wrote: > 2015-09-04 0:08 GMT+08:00 Marc Zyngier <marc.zyngier@arm.com>: > > On 03/09/15 16:52, Greg Kroah-Hartman wrote: > >> On Thu, Sep 03, 2015 at 11:23:15AM +0100, Marc Zyngier wrote: > >>> On 11/08/15 02:48, Jun Nie wrote: > >>>> 2015-08-11 7:23 GMT+08:00 Leif Lindholm <leif.lindholm@linaro.org>: > >>>>> Hi all, > >>>>> > >>>>> The kernelci.org bot picked up a complete boot failure (no output past > >>>>> UEFI stub) with next-20150806 and Tyler bisected it down to somewhere > >>>>> in > >>>>> 8cd90e5 uart: pl011: Add support to ZTE ZX296702 uart > >>>>> 09dcc7d uart: pl011: Improve LCRH register access decision > >>>>> 2c096a9 uart: pl011: Introduce register look up table > >>>>> 7b753f3 uart: pl011: Introduce register accessor > >>>>> > >>>>> The issue only appears with earlycon on command line, for pl011 > >>>>> consoles. > >>>>> > >>>>> Some investigation shows that the cause lies with > >>>>> commit 7b753f318d14 ("uart: pl011: Introduce register accessor") > >>>>> and > >>>>> commit 2c096a9eedc6 ("uart: pl011: Introduce register look up table") > >>>>> > >>>>> Specifically, the changes to pl011_putc() are incorrect: > >>>>> The new pl011_ accessors take a (struct uart_amba_port *) input, but > >>>>> pl011_putc() directly uses the incoming (struct uart_port *) for this. > >>>>> > >>>>> Apart from ending up with an unintended/incorrect UART base address, > >>>>> the introduction of the lookup table for register offsets also means > >>>>> the accessors try to dereference (struct uart_amba_port *)->reg_lut. > >>>>> > >>>>> The below is a hack that shows/resolves the issue, but some > >>>>> refactoring of the original patches might be in order. > >>>>> > >>>>> / > >>>>> Leif > >>>> > >>>> Leif, > >>>> > >>>> Sorry for the inconvenience. I do not have idea of early console till > >>>> now and I always have debug console for early panic debug. Learned > >>>> more from this issue. > >>>> > >>>> Suppose Peter's patch will resolve your issue. > >>> > >>> [+ Greg KH] > >>> > >>> So -next has now been broken for a while on a number of ARM platforms > >>> because of this (they simply cannot boot), and no progress has been made > >>> towards resolving this problem. > >>> > >>> Can we please drop this series (at least commits 7b753f3 and following) > >>> from -next until is has been reworked and reviewed? > >> > >> I don't have any patches in my -next tree, everything is in Linus's tree > >> now. So if I've missed something, or need to revert something, please > >> let me know specifcally what to do. > > > > Gahhh... Given that there is no obvious fix, that the discussion has > > stalled and that the author of the series is apparently away, the > > following patches should be reverted: > > Marc, > > There had a patch from Peter that revert early console part, which I > thought will be merged to mainline. It's my fault for not fix the bug > on time. Also sorry for late response to you as I took three days > leave. > > Could you help test attached patch? Thank you in advance! > > > Greg, > > Would you please hold the merging of revert patches for one or two > days so that the fix can be verified? Thank you! Given that this took way more than a few days, Linus now has the reverts in his tree. Please feel free to resend the series, in a form that does not break people's machines, for inclusion in 4.4-rc1. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2015-09-16 7:20 GMT+08:00 Peter Hurley <peter@hurleysoftware.com>: > On Wed, Sep 9, 2015 at 3:54 PM, Greg Kroah-Hartman <greg@kroah.com> wrote: >> >> On Sat, Sep 05, 2015 at 10:42:03PM +0800, Jun Nie wrote: >> > 2015-09-04 0:08 GMT+08:00 Marc Zyngier <marc.zyngier@arm.com>: >> > > On 03/09/15 16:52, Greg Kroah-Hartman wrote: >> > >> On Thu, Sep 03, 2015 at 11:23:15AM +0100, Marc Zyngier wrote: >> > >>> On 11/08/15 02:48, Jun Nie wrote: >> > >>>> 2015-08-11 7:23 GMT+08:00 Leif Lindholm <leif.lindholm@linaro.org>: >> > >>>>> Hi all, >> > >>>>> >> > >>>>> The kernelci.org bot picked up a complete boot failure (no output >> > >>>>> past >> > >>>>> UEFI stub) with next-20150806 and Tyler bisected it down to >> > >>>>> somewhere >> > >>>>> in >> > >>>>> 8cd90e5 uart: pl011: Add support to ZTE ZX296702 uart >> > >>>>> 09dcc7d uart: pl011: Improve LCRH register access decision >> > >>>>> 2c096a9 uart: pl011: Introduce register look up table >> > >>>>> 7b753f3 uart: pl011: Introduce register accessor >> > >>>>> >> > >>>>> The issue only appears with earlycon on command line, for pl011 >> > >>>>> consoles. >> > >>>>> >> > >>>>> Some investigation shows that the cause lies with >> > >>>>> commit 7b753f318d14 ("uart: pl011: Introduce register accessor") >> > >>>>> and >> > >>>>> commit 2c096a9eedc6 ("uart: pl011: Introduce register look up >> > >>>>> table") >> > >>>>> >> > >>>>> Specifically, the changes to pl011_putc() are incorrect: >> > >>>>> The new pl011_ accessors take a (struct uart_amba_port *) input, >> > >>>>> but >> > >>>>> pl011_putc() directly uses the incoming (struct uart_port *) for >> > >>>>> this. >> > >>>>> >> > >>>>> Apart from ending up with an unintended/incorrect UART base >> > >>>>> address, >> > >>>>> the introduction of the lookup table for register offsets also >> > >>>>> means >> > >>>>> the accessors try to dereference (struct uart_amba_port >> > >>>>> *)->reg_lut. >> > >>>>> >> > >>>>> The below is a hack that shows/resolves the issue, but some >> > >>>>> refactoring of the original patches might be in order. >> > >>>>> >> > >>>>> / >> > >>>>> Leif >> > >>>> >> > >>>> Leif, >> > >>>> >> > >>>> Sorry for the inconvenience. I do not have idea of early console >> > >>>> till >> > >>>> now and I always have debug console for early panic debug. Learned >> > >>>> more from this issue. >> > >>>> >> > >>>> Suppose Peter's patch will resolve your issue. >> > >>> >> > >>> [+ Greg KH] >> > >>> >> > >>> So -next has now been broken for a while on a number of ARM >> > >>> platforms >> > >>> because of this (they simply cannot boot), and no progress has been >> > >>> made >> > >>> towards resolving this problem. >> > >>> >> > >>> Can we please drop this series (at least commits 7b753f3 and >> > >>> following) >> > >>> from -next until is has been reworked and reviewed? >> > >> >> > >> I don't have any patches in my -next tree, everything is in Linus's >> > >> tree >> > >> now. So if I've missed something, or need to revert something, >> > >> please >> > >> let me know specifcally what to do. >> > > >> > > Gahhh... Given that there is no obvious fix, that the discussion has >> > > stalled and that the author of the series is apparently away, the >> > > following patches should be reverted: >> > >> > Marc, >> > >> > There had a patch from Peter that revert early console part, which I >> > thought will be merged to mainline. It's my fault for not fix the bug >> > on time. Also sorry for late response to you as I took three days >> > leave. >> > >> > Could you help test attached patch? Thank you in advance! >> > >> > >> > Greg, >> > >> > Would you please hold the merging of revert patches for one or two >> > days so that the fix can be verified? Thank you! >> >> Given that this took way more than a few days, Linus now has the reverts >> in his tree. Please feel free to resend the series, in a form that does >> not break people's machines, for inclusion in 4.4-rc1. > > > Jun, > > I suggest you keep the series largely as is, while leaving the earlycon > support unmodified. > > Be sure to address the series to all in this email so they have adequate > opportunity to test your driver changes, prior to inclusion in mainline. > > Regards, > Peter Hurley Will do in one month due to occupied in internal tasks. Thanks for your suggestion! Best Regards, Jun -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From fdf1ed2346b12904bcd65a16e33ce12c7d84a2c7 Mon Sep 17 00:00:00 2001
From: Jun Nie <jun.nie@linaro.org>
Date: Sat, 5 Sep 2015 22:32:40 +0800
Subject: [PATCH] serial: pl011: Fix earlycon register LUT breakage
Commit 7b753f318d14 ("uart: pl011: Introduce register accessor")
mistakenly used the register LUT i/o accessors for the pl011
earlycon. Since the port has not been probed at earlycon time,
the struct uart_amba_port (and register LUTs) are uninitialized.
Use direct register addressing for pl011 earlycon; other h/w supported
by the amba-pl011 driver should declare an alternate earlycon.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
drivers/tty/serial/amba-pl011.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 2af09ab..22893d0 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -326,12 +326,6 @@ static void pl011_writew(struct uart_amba_port *uap, int val, int index)
writew_relaxed(val, uap->port.membase + uap->reg_lut[index]);
}
-static void pl011_writeb(struct uart_amba_port *uap, u8 val, int index)
-{
- WARN_ON(index > REG_NR);
- writeb_relaxed(val, uap->port.membase + uap->reg_lut[index]);
-}
-
/*
* Reads up to 256 characters from the FIFO or until it's empty and
* inserts them into the TTY layer. Returns the number of characters
@@ -2351,10 +2345,10 @@ static void pl011_putc(struct uart_port *port, int c)
struct uart_amba_port *uap =
container_of(port, struct uart_amba_port, port);
- while (pl011_readw(uap, REG_FR) & UART01x_FR_TXFF)
+ while (readw_relaxed(port->membase + UART01x_FR) & UART01x_FR_TXFF)
;
- pl011_writeb(uap, c, REG_DR);
- while (pl011_readw(uap, REG_FR) & uap->fr_busy)
+ writeb_relaxed(c, port->membase + UART01x_DR);
+ while (readw_relaxed(port->membase + UART01x_FR) & uap->fr_busy)
;
}
--
1.9.1