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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello.

Atsushi Nemoto wrote:
On Tue, 27 Dec 2005 01:24:52 +0300, Sergei Shtylylov <sshtylyov@xxxxxxxxxxxxx> said:

sshtylyov>         When a system console gets assigned to the UART
sshtylyov> located on the Toshiba GOKU-S PCI card, the port spinlock
sshtylyov> is not initialized at all -- uart_add_one_port() thinks
sshtylyov> it's been initialized by the console setup code which is
sshtylyov> called too early for being able to do that, before the PCI
sshtylyov> card is even detected by the driver, and therefore
sshtylyov> fails.

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

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

 Also,
if console setup failed, the console never enabled.  So the console
can not be used anyway.

   I'm able to use it with ths fix in 2.6.10 with 1.04 driver version backported.

I have an another fix.  Call register_console() again for PCI UART if
the console was not enabled.

   I disagree. Look at what uart_add_one_port() does closely.

 This fixes spin_lock_init issue and
makes PCI UART really usable as console.

Also, I have some other pending changes for this driver.

 * More strict check in verify_port.  Cleanup.
 * Do not insert a char caused previous overrun.
 * Fix some spin_locks.

Yeah, I was about to send the patch that deasl with the nested spinlocks as well... :-)

 * Call register_console again for PCI ports.

   This change doesn't look correct to me...

This is a patch against linux-mips GIT.

Signed-off-by: Atsushi Nemoto <anemo@xxxxxxxxxxxxx>


diff --git a/drivers/serial/serial_txx9.c b/drivers/serial/serial_txx9.c
index f10c86d..c24e0c3 100644
--- 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.
 	 */

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

@@ -1065,6 +1065,14 @@ static int __devinit serial_txx9_registe
 		uart->port.mapbase  = port->mapbase;
 		if (port->dev)
 			uart->port.dev = port->dev;
+#ifdef CONFIG_SERIAL_TXX9_CONSOLE
+		/*
+		 * The 'early' register_console fails for PCI ports.
+		 * Do it again.
+		 */
+		if (!(serial_txx9_console.flags & CON_ENABLED))
+			register_console(&serial_txx9_console);
+#endif
 		ret = uart_add_one_port(&serial_txx9_reg, &uart->port);
 		if (ret == 0)
 			ret = uart->port.line;

WBR, Sergei


[Index of Archives]     [Linux MIPS Home]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Linux]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux