All of lore.kernel.org
 help / color / mirror / Atom feed
From: Russell King <rmk+lkml@arm.linux.org.uk>
To: "George G. Davis" <gdavis@mvista.com>
Cc: Atsushi Nemoto <anemo@mba.ocn.ne.jp>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] serial: Add spin_lock_init() in 8250 early_serial_setup() to init port.lock
Date: Wed, 1 Feb 2006 23:00:13 +0000	[thread overview]
Message-ID: <20060201230013.GL3072@flint.arm.linux.org.uk> (raw)
In-Reply-To: <20060201154549.GE7405@mvista.com>

On Wed, Feb 01, 2006 at 10:45:49AM -0500, George G. Davis wrote:
> But uart_add_one_port() intentionally does not spin_lock_init() the
> port.lock of the serial console device under the assumption that
> it is already done.

Yes, and there's a bug in there atm...

> Here's the call sequence:
> 
> start_kernel()
> 	...
> 	console_init()
> 		...
> 		serial8250_console_init()
> 			serial8250_isa_init_ports()
> 				if (first)
> 					spin_lock_init() /* port.lock init */
> 				...
> 
> All 8250 serial port.locks are now initialised but no ports have
> been registered (on targets which do not register legacy serial ports
> via old_serial_port[]) at this point.

This initialisation is actually pointless here.

> 
> 			register_console()
> 				serial8250_console_setup() /* -ENODEV */
> 	...
> 	rest_init()
> 		...
> 		/* arch_initcalls */
> 		early_serial_setup()

This is where it goes wrong.  Don't call early_serial_setup() after
"early".  Use a platform device instead.

I absolutely detest the number of ways to initialise an 8250 port -
I'd like there to be only one way, but that doesn't satisfy everyone.
That one way is via platform devices.  Please use that method in
preference to everything else, _especially_ from architecture code.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

  parent reply	other threads:[~2006-02-01 23:00 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-01-26  3:24 [PATCH] serial: Add spin_lock_init() in 8250 early_serial_setup() to init port.lock George G. Davis
2006-01-30 15:39 ` Atsushi Nemoto
2006-02-01 15:45   ` George G. Davis
2006-02-01 16:01     ` Atsushi Nemoto
2006-02-01 17:00       ` George G. Davis
2006-02-01 23:00     ` Russell King [this message]
2006-02-02  1:25       ` George G. Davis

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=20060201230013.GL3072@flint.arm.linux.org.uk \
    --to=rmk+lkml@arm.linux.org.uk \
    --cc=anemo@mba.ocn.ne.jp \
    --cc=gdavis@mvista.com \
    --cc=linux-kernel@vger.kernel.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.