Re: [PATCH] Xilinx UART Lite 2.6.18 driver

[Peter Korsgaard <jacmet@sunsite.dk>]

>>>>> "Peter" == Peter Korsgaard <jacmet@sunsite.dk> writes:

Peter> I'll test and take a closer look at your patch when I have
Peter> access to hw again on Monday.

Sorry, it got a bit later than promised, but I've now had a closer
look at your patch ..

--- uartlite/arch/ppc/boot/simple/misc.c	2006-10-15 14:09:47.000000000 -0700
+++ uartlite-mod/arch/ppc/boot/simple/misc.c	2006-10-15 13:58:51.000000000 -0700
@@ -48,7 +48,8 @@
 #if (defined(CONFIG_SERIAL_8250_CONSOLE) \
 	|| defined(CONFIG_VGA_CONSOLE) \
 	|| defined(CONFIG_SERIAL_MPC52xx_CONSOLE) \
-	|| defined(CONFIG_SERIAL_MPSC_CONSOLE)) \
+	|| defined(CONFIG_SERIAL_MPSC_CONSOLE) \
+	|| defined(CONFIG_SERIAL_UARTLITE_CONSOLE)) \
 	&& !defined(CONFIG_GEMINI)
 #define INTERACTIVE_CONSOLE	1
 #endif

The Xilinx boards use misc-embedded.c not misc.c - Why is this needed?

+unsigned long serial_init(int chan, void *ignored)
+{
+	switch (chan)  {
+	#ifdef XPAR_XUL_UART_0_BASEADDR
+		case 0:
+			return XPAR_XUL_UART_0_BASEADDR;
+	#endif
+	#ifdef XPAR_XUL_UART_1_BASEADDR
+		case 1:		
+			return XPAR_XUL_UART_1_BASEADDR;
+	#endif
+	#ifdef XPAR_XUL_UART_2_BASEADDR
+		case 2:
+			return XPAR_XUL_UART_2_BASEADDR;
+	#endif
+	#ifdef XPAR_XUL_UART_3_BASEADDR
+		case 3:
+			return XPAR_XUL_UART_3_BASEADDR;
+	#endif

This doesn't help much as you don't use the com_port argument in the
other functions.

Where did you get the XPAR_XUL_UART_ defines from? Our xparameters.h
seem to contain XPAR_UARTLITE_ defines instead.

@@ -131,12 +115,16 @@
 	struct uart_port *port = (struct uart_port *)dev_id;
 	int busy;

+	spin_lock(&port->lock); /* Lock the port in case of printk */
+	

In an interrupt handler? Why? The console_write does a spin_lock_irqsave.

 static int __init ulite_console_setup(struct console *co, char *options)
 {
+	int i, ret;
 	struct uart_port *port;
-
+	struct platform_device *pdev;
+	
 	if (co->index < 0 || co->index >= ULITE_NR_UARTS)
 		return -EINVAL;

 	port = &ports[co->index];

 	/* not initialized yet? */
-	if (!port->membase)
-		return -ENODEV;
+	if (!port->membase) {
+		/* We might be early console */

Is this really necessary? The platform probe get's called quite early, E.G.:

Breakpoint 2, ulite_probe (pdev=0xc01c84d0) at drivers/serial/uartlite.c:392
392     {
(gdb) print __log_buf 
$1 = "<5>Linux version 2.6.19-rc3 (peko@sleipner) (gcc version 3.4.5) #19 Fri Oct 27 16:39:00 CEST 2006\n<6>Barco ThinLite (V2P) <peter.korsgaard@barco.com>\n<7>Entering add_active_range(0, 0, 15360) 0 entrie"...

You can always use the ppc_md.progress() stuff for really early
debugging if needed. I would prefer to keep this workaround out of
uartlite.c if it isn't needed.

-- 
Bye, Peter Korsgaard