Messages in this thread | | | Date | Wed, 1 Feb 2006 20:25:43 -0500 | From | "George G. Davis" <> | Subject | Re: [PATCH] serial: Add spin_lock_init() in 8250 early_serial_setup() to init port.lock |
| |
On Wed, Feb 01, 2006 at 11:00:13PM +0000, Russell King wrote: > 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...
Which is resolved via:
ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.16-rc1/2.6.16-rc1-mm4/broken-out/serial-initialize-spinlock-for-port-failed-to-setup.patch
Since the above fixes the problem, this patch is withdrawn.
> > serial8250_console_init() > > serial8250_isa_init_ports() > > if (first) > > spin_lock_init() /* port.lock init */
> This initialisation is actually pointless here.
Agreed, when serial-initialize-spinlock-for-port-failed-to-setup.patch is applied, the above spin_lock_init() is no longer required.
> > 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.
Ok, this was perhaps a bad example, some machines call early_serial_setup() much earlier via setup_arch() or sooner. This case used an arch_initcall which is admittedly rather late to be calling early_* funcs. My bad...
> 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.
FWIW, I agree but had to use early_serial_setup() for other reasons (kgdb8250 vs. standard 8250 serial port fights in an earlier 2.6 kernel release). Well, aside from my brain damaged use of early_serial_setup(), there was a real bug initialising the serial console spinlock. So it wasn't a total waste of time. : )
> Please use that method in > preference to everything else, _especially_ from architecture code.
FWIW, I also prefer to see early_serial_setup() killed off and force current users to register 8250 ports via platform devices. But I'm stuck with early_serial_setup() in my current case.
Thanks for your comments.
-- Regards, George - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |