earlycon issues in -next with amba-pl011 updates

earlycon issues in -next with amba-pl011 updates

[Leif Lindholm]

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

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 2af09ab..452dbba 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2348,13 +2348,14 @@ static struct console amba_console = {
 
 static void pl011_putc(struct uart_port *port, int c)
 {
-	struct uart_amba_port *uap =
-	    container_of(port, struct uart_amba_port, port);
+	struct uart_amba_port uap;
+	uap.port = *port;
+	uap.reg_lut = arm_reg;
 
-	while (pl011_readw(uap, REG_FR) & UART01x_FR_TXFF)
+	while (pl011_readw(&uap, REG_FR) & UART01x_FR_TXFF)
 		;
-	pl011_writeb(uap, c, REG_DR);
-	while (pl011_readw(uap, REG_FR) & uap->fr_busy)
+	pl011_writeb(&uap, c, REG_DR);
+	while (pl011_readw(&uap, REG_FR) & UART01x_FR_BUSY)
 		;
 }
 
-- 
2.1.4

Re: earlycon issues in -next with amba-pl011 updates

[Peter Hurley]

Hi Leif,

On 08/10/2015 07:23 PM, Leif Lindholm wrote:
> 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.


Thanks for the bug report and bisect, and apologies for the breakage;
I should have caught that on review.

Would you please test the patch below and see if that resolves the
problem for you?

Regards,
Peter Hurley

PS - _NOT_ even compile-tested, sorry.

--- >% ---
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.

Cc: Jun Nie <jun.nie@linaro.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/serial/amba-pl011.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 2af09ab..fd54991 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2348,13 +2348,10 @@ static struct console amba_console = {
 
 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 (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
 		;
-	pl011_writeb(uap, c, REG_DR);
-	while (pl011_readw(uap, REG_FR) & uap->fr_busy)
+	writeb(c, port->membase + UART01x_DR);
+	while (readl(port->membase + UART01x_FR) & UART01x_FR_BUSY)
 		;
 }
 
-- 
2.5.0

Re: earlycon issues in -next with amba-pl011 updates

[Leif Lindholm]

Hi Peter,

On Mon, Aug 10, 2015 at 08:31:03PM -0400, Peter Hurley wrote:
> > 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.
> 
> 
> Thanks for the bug report and bisect, and apologies for the breakage;
> I should have caught that on review.

I wouldn't have found it if I didn't have the boot failure to track
down, and I'm _not_ going to tell you how long I spent doing that.
 
> Would you please test the patch below and see if that resolves the
> problem for you?

Yeah, that's a more straightforward fix.

> PS - _NOT_ even compile-tested, sorry.
> 
> --- >% ---
> 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.
> 
> Cc: Jun Nie <jun.nie@linaro.org>
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> ---
>  drivers/tty/serial/amba-pl011.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 2af09ab..fd54991 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -2348,13 +2348,10 @@ static struct console amba_console = {
>  
>  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 (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
>  		;
> -	pl011_writeb(uap, c, REG_DR);
> -	while (pl011_readw(uap, REG_FR) & uap->fr_busy)
> +	writeb(c, port->membase + UART01x_DR);
> +	while (readl(port->membase + UART01x_FR) & UART01x_FR_BUSY)
>  		;
>  }
>  
> -- 
> 2.5.0

It works, but builds with:
drivers/tty/serial/amba-pl011.c:329:13: warning: ‘pl011_writeb’
defined but not used [-Wunused-function]
 static void pl011_writeb(struct uart_amba_port *uap, u8 val, int
 index)
             ^
I was dithering about whether having _relaxed accessors in post boot
code and plain ones in earlycon was an issue, but I guess it doesn't
matter much.

/
    Leif

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: earlycon issues in -next with amba-pl011 updates

[Russell King - ARM Linux]

On Mon, Aug 10, 2015 at 08:31:03PM -0400, Peter Hurley wrote:
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 2af09ab..fd54991 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -2348,13 +2348,10 @@ static struct console amba_console = {
>  
>  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 (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
>  		;
> -	pl011_writeb(uap, c, REG_DR);
> -	while (pl011_readw(uap, REG_FR) & uap->fr_busy)
> +	writeb(c, port->membase + UART01x_DR);
> +	while (readl(port->membase + UART01x_FR) & UART01x_FR_BUSY)
>  		;

These loops are wrong.  Kernel coding style is that loops like this
will use cpu_relax() in them, just like the driver used to.  This
was introduced by:

commit 0d3c673e7881e691991b2a4745bd4f149603baa2
Author: Rob Herring <robh@kernel.org>
Date:   Fri Apr 18 17:19:57 2014 -0500

    tty/serial: pl011: add generic earlycon support

Also, the above readl()s should be readl_relaxed(), we don't need
barriered reads or to hit the L2 cache on those reads in the above code.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Re: earlycon issues in -next with amba-pl011 updates

[Peter Hurley]

On 08/11/2015 08:40 AM, Russell King - ARM Linux wrote:
> On Mon, Aug 10, 2015 at 08:31:03PM -0400, Peter Hurley wrote:
>> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
>> index 2af09ab..fd54991 100644
>> --- a/drivers/tty/serial/amba-pl011.c
>> +++ b/drivers/tty/serial/amba-pl011.c
>> @@ -2348,13 +2348,10 @@ static struct console amba_console = {
>>  
>>  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 (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
>>  		;
>> -	pl011_writeb(uap, c, REG_DR);
>> -	while (pl011_readw(uap, REG_FR) & uap->fr_busy)
>> +	writeb(c, port->membase + UART01x_DR);
>> +	while (readl(port->membase + UART01x_FR) & UART01x_FR_BUSY)
>>  		;
> 
> These loops are wrong.  Kernel coding style is that loops like this
> will use cpu_relax() in them, just like the driver used to.  This
> was introduced by:
> 
> commit 0d3c673e7881e691991b2a4745bd4f149603baa2
> Author: Rob Herring <robh@kernel.org>
> Date:   Fri Apr 18 17:19:57 2014 -0500
> 
>     tty/serial: pl011: add generic earlycon support
> 
> Also, the above readl()s should be readl_relaxed(), we don't need
> barriered reads or to hit the L2 cache on those reads in the above code.

This patch just reverts pl011 earlycon back to the existing mainline
state.

The amba-pl011 driver has many places where cpu_relax() is not used,
so that seems like a good patch for -next during the 4.3-rc cycle.

Similarly for read_relaxed() use in earlycon.

Regards,
Peter Hurley

Re: earlycon issues in -next with amba-pl011 updates

[Jun Nie]

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.

Jun

>
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 2af09ab..452dbba 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -2348,13 +2348,14 @@ static struct console amba_console = {
>
>  static void pl011_putc(struct uart_port *port, int c)
>  {
> -       struct uart_amba_port *uap =
> -           container_of(port, struct uart_amba_port, port);
> +       struct uart_amba_port uap;
> +       uap.port = *port;
> +       uap.reg_lut = arm_reg;
>
> -       while (pl011_readw(uap, REG_FR) & UART01x_FR_TXFF)
> +       while (pl011_readw(&uap, REG_FR) & UART01x_FR_TXFF)
>                 ;
> -       pl011_writeb(uap, c, REG_DR);
> -       while (pl011_readw(uap, REG_FR) & uap->fr_busy)
> +       pl011_writeb(&uap, c, REG_DR);
> +       while (pl011_readw(&uap, REG_FR) & UART01x_FR_BUSY)
>                 ;
>  }
>
> --
> 2.1.4
>
>
>

Re: earlycon issues in -next with amba-pl011 updates

[Marc Zyngier]

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?


Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: earlycon issues in -next with amba-pl011 updates

[Greg Kroah-Hartman]

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.

thanks,

greg k-h

Re: earlycon issues in -next with amba-pl011 updates

[Marc Zyngier]

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:

8cd90e50d140 uart: pl011: Add support to ZTE ZX296702 uart
09dcc7dfc05b uart: pl011: Improve LCRH register access decision
2c096a9eedc6 uart: pl011: Introduce register look up table
7b753f318d14 uart: pl011: Introduce register accessor
534e14e2293d uart: pl011: Rename regs with enumeration

(7b753f318d14 being the one breaking everything, but it makes more sense
to revert the whole series until it is properly fixed and reviewed).

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: earlycon issues in -next with amba-pl011 updates

[Peter Hurley]

On 09/03/2015 12:08 PM, Marc Zyngier wrote:
> 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

Fix has been on list since Aug 10.

http://www.spinics.net/lists/linux-serial/msg18576.html


> that the discussion has
> stalled and that the author of the series is apparently away, the
> following patches should be reverted:
> 
> 8cd90e50d140 uart: pl011: Add support to ZTE ZX296702 uart
> 09dcc7dfc05b uart: pl011: Improve LCRH register access decision
> 2c096a9eedc6 uart: pl011: Introduce register look up table
> 7b753f318d14 uart: pl011: Introduce register accessor
> 534e14e2293d uart: pl011: Rename regs with enumeration
> 
> (7b753f318d14 being the one breaking everything, but it makes more sense
> to revert the whole series until it is properly fixed and reviewed).
> 
> Thanks,
> 
> 	M.
> 

Re: earlycon issues in -next with amba-pl011 updates

[Greg Kroah-Hartman]

On Thu, Sep 03, 2015 at 01:49:02PM -0400, Peter Hurley wrote:
> On 09/03/2015 12:08 PM, Marc Zyngier wrote:
> > 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
> 
> Fix has been on list since Aug 10.
> 
> http://www.spinics.net/lists/linux-serial/msg18576.html

Now if only someone would have at least compile tested it :)

Re: earlycon issues in -next with amba-pl011 updates

[Peter Hurley]

On 09/03/2015 01:53 PM, Greg Kroah-Hartman wrote:
> On Thu, Sep 03, 2015 at 01:49:02PM -0400, Peter Hurley wrote:
>> On 09/03/2015 12:08 PM, Marc Zyngier wrote:
>>> 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
>>
>> Fix has been on list since Aug 10.
>>
>> http://www.spinics.net/lists/linux-serial/msg18576.html
> 
> Now if only someone would have at least compile tested it :)

On 08/11/2015 06:07 AM, Leif Lindholm wrote:
> It works, but builds with:
> drivers/tty/serial/amba-pl011.c:329:13: warning: ‘pl011_writeb’
> defined but not used [-Wunused-function]
>  static void pl011_writeb(struct uart_amba_port *uap, u8 val, int
>  index)
>              ^
> I was dithering about whether having _relaxed accessors in post boot
> code and plain ones in earlycon was an issue, but I guess it doesn't
> matter much.

Russell wanted me to do a bunch of improvements, but I wrote that
should wait until 4.3-rc cycle.

Regards,
Peter Hurley



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: earlycon issues in -next with amba-pl011 updates

[Marc Zyngier]

On 03/09/15 19:03, Peter Hurley wrote:
> On 09/03/2015 01:53 PM, Greg Kroah-Hartman wrote:
>> On Thu, Sep 03, 2015 at 01:49:02PM -0400, Peter Hurley wrote:
>>> On 09/03/2015 12:08 PM, Marc Zyngier wrote:
>>>> 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
>>>
>>> Fix has been on list since Aug 10.
>>>
>>> http://www.spinics.net/lists/linux-serial/msg18576.html
>>
>> Now if only someone would have at least compile tested it :)
> 
> On 08/11/2015 06:07 AM, Leif Lindholm wrote:
>> It works, but builds with:
>> drivers/tty/serial/amba-pl011.c:329:13: warning: ‘pl011_writeb’
>> defined but not used [-Wunused-function]
>>  static void pl011_writeb(struct uart_amba_port *uap, u8 val, int
>>  index)
>>              ^
>> I was dithering about whether having _relaxed accessors in post boot
>> code and plain ones in earlycon was an issue, but I guess it doesn't
>> matter much.
> 
> Russell wanted me to do a bunch of improvements, but I wrote that
> should wait until 4.3-rc cycle.

It is well known that we merge "improvements" outside of the merge
window. And what about starting with a series that is actually bisectable?

It is quite apparent that this code hasn't been tested enough, hasn't
been reviewed enough, and breaks a wide range of platforms (almost
anything that sits on and under my desk, TBH).

That's really for Greg to decide, but I'm not overly confident with the
state of this series as it stands.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: earlycon issues in -next with amba-pl011 updates

[Peter Hurley]

On 09/03/2015 02:16 PM, Marc Zyngier wrote:
> On 03/09/15 19:03, Peter Hurley wrote:
>> On 09/03/2015 01:53 PM, Greg Kroah-Hartman wrote:
>>> On Thu, Sep 03, 2015 at 01:49:02PM -0400, Peter Hurley wrote:
>>>> On 09/03/2015 12:08 PM, Marc Zyngier wrote:
>>>>> 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
>>>>
>>>> Fix has been on list since Aug 10.
>>>>
>>>> http://www.spinics.net/lists/linux-serial/msg18576.html
>>>
>>> Now if only someone would have at least compile tested it :)
>>
>> On 08/11/2015 06:07 AM, Leif Lindholm wrote:
>>> It works, but builds with:
>>> drivers/tty/serial/amba-pl011.c:329:13: warning: ‘pl011_writeb’
>>> defined but not used [-Wunused-function]
>>>  static void pl011_writeb(struct uart_amba_port *uap, u8 val, int
>>>  index)
>>>              ^
>>> I was dithering about whether having _relaxed accessors in post boot
>>> code and plain ones in earlycon was an issue, but I guess it doesn't
>>> matter much.
>>
>> Russell wanted me to do a bunch of improvements, but I wrote that
>> should wait until 4.3-rc cycle.
> 
> It is well known that we merge "improvements" outside of the merge
> window. And what about starting with a series that is actually bisectable?
> 
> It is quite apparent that this code hasn't been tested enough, hasn't
> been reviewed enough, and breaks a wide range of platforms (almost
> anything that sits on and under my desk, TBH).
> 
> That's really for Greg to decide, but I'm not overly confident with the
> state of this series as it stands.

If you feel the code is not reliable enough/tested/not bisectable/whatever,
I have no objection to reverting and starting again.

But if the issue is only wrt earlycon, then my patch ought to address that.

Regards,
Peter Hurley


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: earlycon issues in -next with amba-pl011 updates

[Marc Zyngier]

On 03/09/15 18:49, Peter Hurley wrote:
> On 09/03/2015 12:08 PM, Marc Zyngier wrote:
>> 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
> 
> Fix has been on list since Aug 10.
> 
> http://www.spinics.net/lists/linux-serial/msg18576.html

... with both Leif and Russell outlining issues with this patch:

http://www.spinics.net/lists/linux-serial/msg18594.html
http://www.spinics.net/lists/linux-serial/msg18599.html

so maybe coming up with an updated series would have been a good move.

	M.
-- 
Jazz is not dead. It just smells funny...

Re: earlycon issues in -next with amba-pl011 updates

[Peter Hurley]

On 09/03/2015 02:00 PM, Marc Zyngier wrote:
> On 03/09/15 18:49, Peter Hurley wrote:
>> On 09/03/2015 12:08 PM, Marc Zyngier wrote:
>>> 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
>>
>> Fix has been on list since Aug 10.
>>
>> http://www.spinics.net/lists/linux-serial/msg18576.html
> 
> ... with both Leif and Russell outlining issues with this patch:
> 
> http://www.spinics.net/lists/linux-serial/msg18594.html
> http://www.spinics.net/lists/linux-serial/msg18599.html

Russell's suggestions were noted in my reply

http://www.spinics.net/lists/linux-serial/msg18650.html

> so maybe coming up with an updated series would have been a good move.

Feel free to work from where I left off.

Regards,
Peter Hurley

Re: earlycon issues in -next with amba-pl011 updates

[Will Deacon]

On Thu, Sep 03, 2015 at 05:08:53PM +0100, Marc Zyngier wrote:
> On 03/09/15 16:52, Greg Kroah-Hartman wrote:
> > On Thu, Sep 03, 2015 at 11:23:15AM +0100, Marc Zyngier wrote:
> >> 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:
> 
> 8cd90e50d140 uart: pl011: Add support to ZTE ZX296702 uart
> 09dcc7dfc05b uart: pl011: Improve LCRH register access decision
> 2c096a9eedc6 uart: pl011: Introduce register look up table
> 7b753f318d14 uart: pl011: Introduce register accessor
> 534e14e2293d uart: pl011: Rename regs with enumeration
> 
> (7b753f318d14 being the one breaking everything, but it makes more sense
> to revert the whole series until it is properly fixed and reviewed).

For the reverts (git revert 534e14e2293d~1..8cd90e50d140):

  Acked-by: Will Deacon <will.deacon@arm.com>
  Tested-by: Will Deacon <will.deacon@arm.com>

We can discuss what could've and should've happened 'til we're blue in
the face but, in the meantime, I'd like my serial console back.

Greg -- any chance you could send these for -rc1, please?

Will

Re: earlycon issues in -next with amba-pl011 updates

[Russell King - ARM Linux]

On Fri, Sep 04, 2015 at 12:06:52PM +0100, Will Deacon wrote:
> On Thu, Sep 03, 2015 at 05:08:53PM +0100, Marc Zyngier wrote:
> > On 03/09/15 16:52, Greg Kroah-Hartman wrote:
> > > On Thu, Sep 03, 2015 at 11:23:15AM +0100, Marc Zyngier wrote:
> > >> 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:
> > 
> > 8cd90e50d140 uart: pl011: Add support to ZTE ZX296702 uart
> > 09dcc7dfc05b uart: pl011: Improve LCRH register access decision
> > 2c096a9eedc6 uart: pl011: Introduce register look up table
> > 7b753f318d14 uart: pl011: Introduce register accessor
> > 534e14e2293d uart: pl011: Rename regs with enumeration
> > 
> > (7b753f318d14 being the one breaking everything, but it makes more sense
> > to revert the whole series until it is properly fixed and reviewed).
> 
> For the reverts (git revert 534e14e2293d~1..8cd90e50d140):
> 
>   Acked-by: Will Deacon <will.deacon@arm.com>
>   Tested-by: Will Deacon <will.deacon@arm.com>
> 
> We can discuss what could've and should've happened 'til we're blue in
> the face but, in the meantime, I'd like my serial console back.
> 
> Greg -- any chance you could send these for -rc1, please?

Given the number of reviews the patches went through, I eventually came
to the conclusion that it would be far better if I were to write the
set of patches for the driver - but I haven't had a slot to do that.
I basically stopped reviewing them because I decided it wasn't worth
spending the time anymore on the series.  That wasn't an implicit
"I'm happy with the patch set" but more a "I've given up with this
patch set, it's still not up to scratch."

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: earlycon issues in -next with amba-pl011 updates

[Jun Nie]

2015-09-04 19:13 GMT+08:00 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Fri, Sep 04, 2015 at 12:06:52PM +0100, Will Deacon wrote:
>> On Thu, Sep 03, 2015 at 05:08:53PM +0100, Marc Zyngier wrote:
>> > On 03/09/15 16:52, Greg Kroah-Hartman wrote:
>> > > On Thu, Sep 03, 2015 at 11:23:15AM +0100, Marc Zyngier wrote:
>> > >> 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:
>> >
>> > 8cd90e50d140 uart: pl011: Add support to ZTE ZX296702 uart
>> > 09dcc7dfc05b uart: pl011: Improve LCRH register access decision
>> > 2c096a9eedc6 uart: pl011: Introduce register look up table
>> > 7b753f318d14 uart: pl011: Introduce register accessor
>> > 534e14e2293d uart: pl011: Rename regs with enumeration
>> >
>> > (7b753f318d14 being the one breaking everything, but it makes more sense
>> > to revert the whole series until it is properly fixed and reviewed).
>>
>> For the reverts (git revert 534e14e2293d~1..8cd90e50d140):
>>
>>   Acked-by: Will Deacon <will.deacon@arm.com>
>>   Tested-by: Will Deacon <will.deacon@arm.com>
>>
>> We can discuss what could've and should've happened 'til we're blue in
>> the face but, in the meantime, I'd like my serial console back.
>>
>> Greg -- any chance you could send these for -rc1, please?
>
> Given the number of reviews the patches went through, I eventually came
> to the conclusion that it would be far better if I were to write the
> set of patches for the driver - but I haven't had a slot to do that.
> I basically stopped reviewing them because I decided it wasn't worth
> spending the time anymore on the series.  That wasn't an implicit
> "I'm happy with the patch set" but more a "I've given up with this
> patch set, it's still not up to scratch."

Russell,

Thanks for your time for reviewing that patch set. I did made some
mistake for the register naming for not understand your intention
clearly. I thought reviewing out of date patches also made you
desperate because I update patches frequently. Anyway, the existing
merged patches are made per your suggestion except the bugs. Could you
help comments for the below patch set even they are already merged? So
that I can polish existing code or re-prepare those changes. Or you
have other plan on supporting pl011 controller with different register
mapping? Thanks for you any comments!

http://www.spinics.net/lists/linux-serial/msg18363.html

Jun

>
> --
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.

Re: earlycon issues in -next with amba-pl011 updates

[Greg Kroah-Hartman]

On Fri, Sep 04, 2015 at 12:06:52PM +0100, Will Deacon wrote:
> On Thu, Sep 03, 2015 at 05:08:53PM +0100, Marc Zyngier wrote:
> > On 03/09/15 16:52, Greg Kroah-Hartman wrote:
> > > On Thu, Sep 03, 2015 at 11:23:15AM +0100, Marc Zyngier wrote:
> > >> 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:
> > 
> > 8cd90e50d140 uart: pl011: Add support to ZTE ZX296702 uart
> > 09dcc7dfc05b uart: pl011: Improve LCRH register access decision
> > 2c096a9eedc6 uart: pl011: Introduce register look up table
> > 7b753f318d14 uart: pl011: Introduce register accessor
> > 534e14e2293d uart: pl011: Rename regs with enumeration
> > 
> > (7b753f318d14 being the one breaking everything, but it makes more sense
> > to revert the whole series until it is properly fixed and reviewed).
> 
> For the reverts (git revert 534e14e2293d~1..8cd90e50d140):
> 
>   Acked-by: Will Deacon <will.deacon@arm.com>
>   Tested-by: Will Deacon <will.deacon@arm.com>
> 
> We can discuss what could've and should've happened 'til we're blue in
> the face but, in the meantime, I'd like my serial console back.
> 
> Greg -- any chance you could send these for -rc1, please?

Ok, now reverted, I'll send the pull request later today.

thanks,

greg k-h

Re: earlycon issues in -next with amba-pl011 updates

[Will Deacon]

On Fri, Sep 04, 2015 at 05:15:10PM +0100, Greg Kroah-Hartman wrote:
> On Fri, Sep 04, 2015 at 12:06:52PM +0100, Will Deacon wrote:
> > On Thu, Sep 03, 2015 at 05:08:53PM +0100, Marc Zyngier wrote:
> > > 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:
> > > 
> > > 8cd90e50d140 uart: pl011: Add support to ZTE ZX296702 uart
> > > 09dcc7dfc05b uart: pl011: Improve LCRH register access decision
> > > 2c096a9eedc6 uart: pl011: Introduce register look up table
> > > 7b753f318d14 uart: pl011: Introduce register accessor
> > > 534e14e2293d uart: pl011: Rename regs with enumeration
> > > 
> > > (7b753f318d14 being the one breaking everything, but it makes more sense
> > > to revert the whole series until it is properly fixed and reviewed).
> > 
> > For the reverts (git revert 534e14e2293d~1..8cd90e50d140):
> > 
> >   Acked-by: Will Deacon <will.deacon@arm.com>
> >   Tested-by: Will Deacon <will.deacon@arm.com>
> > 
> > We can discuss what could've and should've happened 'til we're blue in
> > the face but, in the meantime, I'd like my serial console back.
> > 
> > Greg -- any chance you could send these for -rc1, please?
> 
> Ok, now reverted, I'll send the pull request later today.

Brilliant, thanks Greg.

Will

Re: earlycon issues in -next with amba-pl011 updates

[Jun Nie]

[-- Attachment #1: Type: text/plain, Size: 3543 bytes --]

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!

Jun


>
> 8cd90e50d140 uart: pl011: Add support to ZTE ZX296702 uart
> 09dcc7dfc05b uart: pl011: Improve LCRH register access decision
> 2c096a9eedc6 uart: pl011: Introduce register look up table
> 7b753f318d14 uart: pl011: Introduce register accessor
> 534e14e2293d uart: pl011: Rename regs with enumeration
>
> (7b753f318d14 being the one breaking everything, but it makes more sense
> to revert the whole series until it is properly fixed and reviewed).
>
> Thanks,
>
>         M.
> --
> Jazz is not dead. It just smells funny...

[-- Attachment #2: 0001-serial-pl011-Fix-earlycon-register-LUT-breakage.patch --]
[-- Type: text/x-patch, Size: 1981 bytes --]

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


[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: earlycon issues in -next with amba-pl011 updates

[Greg Kroah-Hartman]

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

Re: earlycon issues in -next with amba-pl011 updates

[Jun Nie]

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