linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Valentin Schneider <valentin.schneider@arm.com>
To: John Stultz <john.stultz@linaro.org>
Cc: lkml <linux-kernel@vger.kernel.org>,
	Russell King <linux@armlinux.org.uk>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jslaby@suse.com>,
	linux-serial@vger.kernel.org
Subject: Re: [RFC][PATCH] serial: amba-pl011: Make sure we initialize the port.lock spinlock
Date: Fri, 24 Apr 2020 00:14:46 +0100	[thread overview]
Message-ID: <jhj1rodyeu1.mognet@arm.com> (raw)
In-Reply-To: <20200423220056.29450-1-john.stultz@linaro.org>


Hi John,

On 23/04/20 23:00, John Stultz wrote:
> Which seems to be due to the fact that after allocating the uap
> structure, the pl011 code doesn't initialize the spinlock.
>
> This patch fixes it by initializing the spinlock and the warning
> has gone away.
>

Thanks for having a look. It does seem like the reasonable thing to do, and
I no longer get the warning on h960.

That said, I got more curious as this doesn't show up on my Juno (same
Image). Digging into it I see that uart_add_one_port() has a call to
uart_port_spin_lock_init() a few lines before uart_configure_port() (in
which the above warning gets triggered). That thing says:

 * Ensure that the serial console lock is initialised early.
 * If this port is a console, then the spinlock is already initialised.

Which requires me to ask: are we doing the right thing here?

> CC: Valentin Schneider <valentin.schneider@arm.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jiri Slaby <jslaby@suse.com>
> Cc: linux-serial@vger.kernel.org
> Reported-by: Valentin Schneider <valentin.schneider@arm.com>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>  drivers/tty/serial/amba-pl011.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 2296bb0f9578..458fc3d9d48c 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -2575,6 +2575,7 @@ static int pl011_setup_port(struct device *dev, struct uart_amba_port *uap,
>       uap->port.has_sysrq = IS_ENABLED(CONFIG_SERIAL_AMBA_PL011_CONSOLE);
>       uap->port.flags = UPF_BOOT_AUTOCONF;
>       uap->port.line = index;
> +	spin_lock_init(&uap->port.lock);
>
>       amba_ports[index] = uap;

  reply	other threads:[~2020-04-23 23:15 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-23 22:00 [RFC][PATCH] serial: amba-pl011: Make sure we initialize the port.lock spinlock John Stultz
2020-04-23 23:14 ` Valentin Schneider [this message]
2020-04-24  4:00   ` John Stultz
2020-04-25  4:04   ` John Stultz
2020-04-26 23:26     ` Valentin Schneider
2020-04-27  9:02       ` Andy Shevchenko
2020-04-28  8:54         ` Valentin Schneider
2020-04-28 10:03           ` Andy Shevchenko
2020-04-27  9:05 ` Andy Shevchenko

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=jhj1rodyeu1.mognet@arm.com \
    --to=valentin.schneider@arm.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=john.stultz@linaro.org \
    --cc=jslaby@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).