[v2] serial: pl011: Fix earlycon register LUT breakage

Message ID 1441468654-6923-1-git-send-email-jun.nie@linaro.org
State New
Headers show

Commit Message

Jun Nie Sept. 5, 2015, 3:57 p.m.
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 | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

Comments

Jun Nie Sept. 8, 2015, 2:23 a.m. | #1
2015-09-05 23:57 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
> 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 | 15 +++------------
>  1 file changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 2af09ab..cce41e0 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
> @@ -2348,13 +2342,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 (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) & UART01x_FR_BUSY)
>                 ;
>  }
>
> --
> 1.9.1
>

Could anybody help test this earlycon patch on hardware with standard
pl011 serial? Your help is appreciated and valuable no matter the
previous patch set is carried on or reverted.
Thank you very much!

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
Jun Nie Sept. 8, 2015, 7:52 a.m. | #2
2015-09-08 10:23 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
> 2015-09-05 23:57 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
>> 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 | 15 +++------------
>>  1 file changed, 3 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
>> index 2af09ab..cce41e0 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
>> @@ -2348,13 +2342,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 (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) & UART01x_FR_BUSY)
>>                 ;
>>  }
>>
>> --
>> 1.9.1
>>
>
> Could anybody help test this earlycon patch on hardware with standard
> pl011 serial? Your help is appreciated and valuable no matter the
> previous patch set is carried on or reverted.
> Thank you very much!
>
> Jun

cc linux-arm-kernel@lists.infradead.org in case someone may have
resource to help have a test. Thank you!

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
Jun Nie Sept. 8, 2015, 7:53 a.m. | #3
2015-09-08 15:52 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
> 2015-09-08 10:23 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
>> 2015-09-05 23:57 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
>>> 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 | 15 +++------------
>>>  1 file changed, 3 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
>>> index 2af09ab..cce41e0 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
>>> @@ -2348,13 +2342,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 (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) & UART01x_FR_BUSY)
>>>                 ;
>>>  }
>>>
>>> --
>>> 1.9.1
>>>
>>
>> Could anybody help test this earlycon patch on hardware with standard
>> pl011 serial? Your help is appreciated and valuable no matter the
>> previous patch set is carried on or reverted.
>> Thank you very much!
>>
>> Jun

cc linux-arm-kernel@lists.infradead.org in case someone may have
resource to help have a test. Thank you!

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
Huang Shijie Sept. 8, 2015, 8:31 a.m. | #4
On Tue, Sep 08, 2015 at 03:53:28PM +0800, Jun Nie wrote:
> 2015-09-08 15:52 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
> > 2015-09-08 10:23 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
> >> 2015-09-05 23:57 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
> >>> 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 | 15 +++------------
> >>>  1 file changed, 3 insertions(+), 12 deletions(-)
> >>>
> >>> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> >>> index 2af09ab..cce41e0 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
> >>> @@ -2348,13 +2342,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 (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) & UART01x_FR_BUSY)
> >>>                 ;
> >>>  }
> >>>
> >>> --
> >>> 1.9.1
> >>>
> >>
> >> Could anybody help test this earlycon patch on hardware with standard
> >> pl011 serial? Your help is appreciated and valuable no matter the
> >> previous patch set is carried on or reverted.
> >> Thank you very much!
> >>
> >> Jun
> 
> cc linux-arm-kernel@lists.infradead.org in case someone may have
> resource to help have a test. Thank you!
I tested this patch with the ARM Juno-r1 board, it works.

Tested-by: Huang Shijie <shijie.huang@arm.com>

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

Patch hide | download patch | download mbox

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 2af09ab..cce41e0 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
@@ -2348,13 +2342,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 (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) & UART01x_FR_BUSY)
 		;
 }