linux-mips
[Top] [All Lists]

Re: [PATCH] serial_txx9: forcibly init the spinlock for PCI UART used as

To: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
Subject: Re: [PATCH] serial_txx9: forcibly init the spinlock for PCI UART used as a console
From: Sergei Shtylylov <sshtylyov@ru.mvista.com>
Date: Tue, 27 Dec 2005 19:29:07 +0300
Cc: rmk+serial@arm.linux.org.uk, linux-mips@linux-mips.org, ralf@linux-mips.org
In-reply-to: <20051228.003457.74752441.anemo@mba.ocn.ne.jp>
Organization: MostaVista Software Inc.
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <43B06DB4.409@ru.mvista.com> <20051227.144551.79070832.nemoto@toshiba-tops.co.jp> <43B143EE.6070700@ru.mvista.com> <20051228.003457.74752441.anemo@mba.ocn.ne.jp>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mozilla/5.0 (X11; U; Linux i686; rv:1.7.2) Gecko/20040803
Hello.

Atsushi Nemoto wrote:

On Tue, 27 Dec 2005 16:38:54 +0300, Sergei Shtylylov <sshtylyov@ru.mvista.com> 
said:

The problem is not just only spin_lock_init.  The parameters of
"console=" option (baudrate, etc.) are not passed for PCI UART.

sshtylyov>     They are -- uart_add_one_port() calls console setup
sshtylyov> once more when registering PCI UART with serial code.

Yes, you are right.  I missed the register_console call in
uart_add_one_port().  So your patch will fix the problem.  But I
suppose the spinlock should be initialized in serial_core.  How about
this?

--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -2233,7 +2233,7 @@ int uart_add_one_port(struct uart_driver
         * If this port is a console, then the spinlock is already
         * initialised.
         */
-       if (!uart_console(port))
+       if (!(uart_console(port) && (port->cons->flags & CON_ENABLED)))
                spin_lock_init(&port->lock);
uart_configure_port(drv, state, port);

   I wouldn't object.

--- a/drivers/serial/serial_txx9.c
+++ b/drivers/serial/serial_txx9.c
@@ -937,11 +942,6 @@ static int serial_txx9_console_setup(str
return -ENODEV;

/*
-        * Temporary fix.
-        */
-       spin_lock_init(&port->lock);
-
-       /*
*       Disable UART interrupts, set DTR and RTS high
*       and set speed.
*/

sshtylyov> Can you tell me, how this is supposed to work with TX49xx
sshtylyov> SOC UARTs? When that spinlock will be init'ed for the
sshtylyov> console port? uart_add_one_port() won't do it, and your
sshtylyov> added code below won't do it either, so I disagree with
sshtylyov> this change (though with "empty" spinlock it will no doubt
sshtylyov> work) since there's nothing to init.

The spinlock is initialized in uart_set_options() which is called
from console setup function.

   I'm sorry, haven't dug that deep. :-)
   Thought explicit spinlock init was really necessary there...

WBR, Sergei


<Prev in Thread] Current Thread [Next in Thread>