Re: [PATCH] 8250_early console support for au1x00 (again)

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

Hello.

Rodolfo Giometti wrote:

>>   Sorry for follwing up 2 month ago, I just happen to stumble on some 

    I was going to type "3 months ago" since 3 months have apssed indeed. :-<

>>   issues addresses by these patches as well. I assume you haven't tried 
>>sending them to Russel King?

> Not yet, I just waiting for some comments. :)

    Now you have some at last. :-)

>>>+	case UPIO_MEM32:
>>>+	case UPIO_AU:
>>>+		return readl(port->membase + offset);

>>   NAK. readl() can't be used to read from Alchemy SOC peripherals because 
>>it'll break in BE mode.  Alchemy automagically handles byteswap for the SOC 
>>peripherals.

> Ok. I'm going to fix it by using au_readl() but in this case I have to
> add an ifdef with au1xxx include file. Can it be acceptable?

    I think so. But it's Russel who will decide. :-)

>>>+			(port->iotype == UPIO_MEM) ? "MMIO" : \
>>>+			(port->iotype == UPIO_AU)  ? "AU"   : "I/O port",
>>>+			(port->iotype == UPIO_MEM) || \
>>>+			(port->iotype == UPIO_AU) ? port->mapbase :
>>>			    (unsigned long) port->iobase);

>>   I'd simply map UPIO_AU to "MMIO" in the message because it's memory 
>>   mapped UART after all...

> Yes, but in the kernel command line we must supply "au"... That's why
> I used different string, so the user can verify whatever he/she passed
> to the kernel.

    I can also suggest something like "Au1xx0 MMIO"... :-)

>>>index 17839e7..9e27aee 100644
>>>--- a/drivers/serial/serial_core.c
>>>+++ b/drivers/serial/serial_core.c
>>>@@ -2367,6 +2367,7 @@ int uart_match_port(struct uart_port *po
>>>		return (port1->iobase == port2->iobase) &&
>>>		       (port1->hub6   == port2->hub6);
>>>	case UPIO_MEM:
>>>+	case UPIO_AU:

>>   Also needs cases for UPIO_MEM32 and UPIO_TSI.

> I just added the code for au1xxx. Why should I consider those cases
> also?

    It seems that you can remove this hunk altogether now -- Russel has 
accepted my patch adding cases for UPIO_MEM32, UPIO_AU and UPIO_TSI...

>>>-#ifdef CONFIG_SERIAL_8250_AU1X00
>>>	case UPIO_AU:
>>>-		__raw_writel(value, up->port.membase + offset);
>>>+		writel(value, up->port.membase + offset);
>>>		break;
>>>-#endif

>>   Ditto writel().

> Is __raw_writel() correct?

    It should be.

> Thanks for your suggestions.

> Ciao,

> Rodolfo

WBR, Sergei