Re: [PATCH] serial: Add spin_lock_init() in 8250 early_serial_setup() to init port.lock

[Russell King]

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/