From: Peter Hurley <peter@hurleysoftware.com>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: jun.nie@linaro.org, tyler.baker@linaro.org,
linux-arm-kernel@lists.infradead.org,
Leif Lindholm <leif.lindholm@linaro.org>,
linux-serial@vger.kernel.org
Subject: Re: earlycon issues in -next with amba-pl011 updates
Date: Mon, 17 Aug 2015 12:04:16 -0400 [thread overview]
Message-ID: <55D20600.6050204@hurleysoftware.com> (raw)
In-Reply-To: <20150811124043.GZ7557@n2100.arm.linux.org.uk>
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
WARNING: multiple messages have this Message-ID (diff)
From: peter@hurleysoftware.com (Peter Hurley)
To: linux-arm-kernel@lists.infradead.org
Subject: earlycon issues in -next with amba-pl011 updates
Date: Mon, 17 Aug 2015 12:04:16 -0400 [thread overview]
Message-ID: <55D20600.6050204@hurleysoftware.com> (raw)
In-Reply-To: <20150811124043.GZ7557@n2100.arm.linux.org.uk>
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
next prev parent reply other threads:[~2015-08-17 16:04 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-10 23:23 earlycon issues in -next with amba-pl011 updates Leif Lindholm
2015-08-10 23:23 ` Leif Lindholm
2015-08-11 0:31 ` Peter Hurley
2015-08-11 0:31 ` Peter Hurley
2015-08-11 10:07 ` Leif Lindholm
2015-08-11 10:07 ` Leif Lindholm
2015-08-11 12:40 ` Russell King - ARM Linux
2015-08-11 12:40 ` Russell King - ARM Linux
2015-08-17 16:04 ` Peter Hurley [this message]
2015-08-17 16:04 ` Peter Hurley
2015-08-11 1:48 ` Jun Nie
2015-08-11 1:48 ` Jun Nie
2015-09-03 10:23 ` Marc Zyngier
2015-09-03 10:23 ` Marc Zyngier
2015-09-03 15:52 ` Greg Kroah-Hartman
2015-09-03 15:52 ` Greg Kroah-Hartman
2015-09-03 16:08 ` Marc Zyngier
2015-09-03 16:08 ` Marc Zyngier
2015-09-03 17:49 ` Peter Hurley
2015-09-03 17:49 ` Peter Hurley
2015-09-03 17:53 ` Greg Kroah-Hartman
2015-09-03 17:53 ` Greg Kroah-Hartman
2015-09-03 18:03 ` Peter Hurley
2015-09-03 18:03 ` Peter Hurley
2015-09-03 18:16 ` Marc Zyngier
2015-09-03 18:16 ` Marc Zyngier
2015-09-03 18:21 ` Peter Hurley
2015-09-03 18:21 ` Peter Hurley
2015-09-03 18:00 ` Marc Zyngier
2015-09-03 18:00 ` Marc Zyngier
2015-09-03 18:15 ` Peter Hurley
2015-09-03 18:15 ` Peter Hurley
2015-09-04 11:06 ` Will Deacon
2015-09-04 11:06 ` Will Deacon
2015-09-04 11:13 ` Russell King - ARM Linux
2015-09-04 11:13 ` Russell King - ARM Linux
2015-09-05 14:54 ` Jun Nie
2015-09-05 14:54 ` Jun Nie
2015-09-04 16:15 ` Greg Kroah-Hartman
2015-09-04 16:15 ` Greg Kroah-Hartman
2015-09-04 16:16 ` Will Deacon
2015-09-04 16:16 ` Will Deacon
2015-09-05 14:42 ` Jun Nie
2015-09-05 14:42 ` Jun Nie
2015-09-09 19:54 ` Greg Kroah-Hartman
2015-09-09 19:54 ` Greg Kroah-Hartman
[not found] ` <CAKU3ayWkXxmF1cp=fOAL6kNKY3wY1u=XhPROKiLqkx4oZ9xZ0g@mail.gmail.com>
2015-09-16 0:54 ` Jun Nie
2015-09-16 0:54 ` Jun Nie
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55D20600.6050204@hurleysoftware.com \
--to=peter@hurleysoftware.com \
--cc=jun.nie@linaro.org \
--cc=leif.lindholm@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-serial@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=tyler.baker@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.