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

[Sergei Shtylylov <sshtylyov@ru.mvista.com>]

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