lkml.org 
[lkml]   [2006]   [Feb]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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/

\
 
 \ /
  Last update: 2006-02-02 02:28    [W:0.587 / U:0.080 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site