linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: John Stultz <john.stultz@linaro.org>
To: Valentin Schneider <valentin.schneider@arm.com>
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 21:04:50 -0700	[thread overview]
Message-ID: <CALAqxLW+CBMxj_5gCF5yLcX8dhM7Fg6oOL-zot0ZZT6PW6R04g@mail.gmail.com> (raw)
In-Reply-To: <jhj1rodyeu1.mognet@arm.com>

On Thu, Apr 23, 2020 at 4:14 PM Valentin Schneider
<valentin.schneider@arm.com> wrote:
> 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?

So I got a little bit of time to look at this before I got pulled off
to other things (and now its Friday night, so I figured I'd reply
before I forget it on Monday).

I did check and lockdep is tripping when we add ttyAMA6 which is the
serial console on the board. I wasn't able to trace back to why we
hadn't already called spin_lock_init() in the console code, but it
seems we haven't.

Also I checked on HiKey as well, and there I'm seeing the same lockdep
splat and this fix seems to resolve it. So more digging is needed. If
anyone has a better idea of what might be awry or if the lock does
need to be initialized in the driver (it's a bit inconsistent, I see
some drivers do but others don't), let me know.

thanks
-john

  parent reply	other threads:[~2020-04-25  4:05 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
2020-04-24  4:00   ` John Stultz
2020-04-25  4:04   ` John Stultz [this message]
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=CALAqxLW+CBMxj_5gCF5yLcX8dhM7Fg6oOL-zot0ZZT6PW6R04g@mail.gmail.com \
    --to=john.stultz@linaro.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jslaby@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=valentin.schneider@arm.com \
    /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).