[PATCH] AMD Alchemy: claim UART memory range

[PATCH] AMD Alchemy: claim UART memory range

[Sergei Shtylyov]

[-- Attachment #1: Type: text/plain, Size: 266 bytes --]

Hello.

      I've noticed that the 8250/Au1x00 driver (drivers/serial/8250_au1x00.c)
doesn't claim UART memory ranges and uses wrong (KSEG1-based) UART addresses
instead of the physical ones.

WBR, Sergei

Sighed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>



[-- Attachment #2: Au1xx0-claim-UART-range.patch --]
[-- Type: text/plain, Size: 1184 bytes --]

diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 674b15c..d641ac4 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -1906,6 +1906,9 @@ static int serial8250_request_std_resour
 	int ret = 0;
 
 	switch (up->port.iotype) {
+	case UPIO_AU:
+		size = 0x100000;
+		/* fall thru */
 	case UPIO_MEM:
 		if (!up->port.mapbase)
 			break;
@@ -1938,6 +1941,9 @@ static void serial8250_release_std_resou
 	unsigned int size = 8 << up->port.regshift;
 
 	switch (up->port.iotype) {
+	case UPIO_AU:
+		size = 0x100000;
+		/* fall thru */
 	case UPIO_MEM:
 		if (!up->port.mapbase)
 			break;
diff --git a/drivers/serial/8250_au1x00.c b/drivers/serial/8250_au1x00.c
index 3d1bfd0..58015fd 100644
--- a/drivers/serial/8250_au1x00.c
+++ b/drivers/serial/8250_au1x00.c
@@ -30,13 +30,12 @@
 	{						\
 		.iobase		= _base,		\
 		.membase	= (void __iomem *)_base,\
-		.mapbase	= _base,		\
+		.mapbase	= CPHYSADDR(_base),	\
 		.irq		= _irq,			\
 		.uartclk	= 0,	/* filled */	\
 		.regshift	= 2,			\
 		.iotype		= UPIO_AU,		\
-		.flags		= UPF_SKIP_TEST | 	\
-				  UPF_IOREMAP,		\
+		.flags		= UPF_SKIP_TEST 	\
 	}
 
 static struct plat_serial8250_port au1x00_data[] = {

Re: [PATCH] AMD Alchemy: claim UART memory range

[Sergei Shtylyov]

Hello.

Russell King wrote:

>>>  BTW, can anybody enlighten me why 8250_au1x00.c came into being  
>>>at all?
>>>Its only function seems to register the UART platform devices, the  
>>>thing
>>>that is usually done in the board setup code, i. e. I'd rather have  
>>>it in arch/mips/au1000/common/platform.c (however, 8250.c should  
>>>have been able to filter out ports with UPIO_AU in case  
>>>CONFIG_SERIAL_8250_AU1X00 undefined)...

>>Seemed like a good idea at the moment to follow the already existing  
>>convention.

> Already existing convention is as per Sergei's mail actually - to have the
> platform device registration in arch/*.  The others which you thought were
> convention there (accent, boca, fourport, hub6, mca) are all for add-in
> cards and aren't architecture specific.

> Hence, they can't live in arch/*.

> So yes, 8250_au1x00.c breaks the established convention because it isn't
> an add-in card.

    Thanks for clarification.

    Now another question to Pantelis: IIUC, the Alchemy UART platform devices 
have UPF_SKIP_TEST set because of the Alchemy docs claiming that UARTs other 
than UART3 don't have MCR/MSR and only UART3 does have the full set of the 
modem control/status lines?  Were they indeed failing the loopback test for 
you? Asking because on DBAu1550 board all (enabled) UARTs do pass the loopback 
test if I get rid of this flag (however, Au1550 datasheet says MCR/MSR exists 
on all UARTs, just no modem pins exist on UART0, and only RTS-/CTS- pair on 
UART1 -- and the bits having no correspoding pins seem to be tied high 
internally).
    If I'm correct, the driver seems inconsistent in how it handles 
UART_BUG_NOMSR flag, only checking it when deciding whether to enable the 
modem status interrupts or not while actually it should have been checked in 
serial8250_set_mctrl() and check_modem_status() as well...
    It also looks like the driver doesn't use Alchemy UARTs to their full 
potential currently: UART3 has not only full set of modem lines, but also is 
capable of the auto flow control (UART1 on Au1550 also is).  (Making use of 
these features howewer are complicated by the auto flow control being only 
available in the late steppings of Au1500 and UART3 modem pins being 
multiplexed with GPIO...)

WBR, Sergei

PS: CCing linux-mips to keep people here informed. :-)

Re: [PATCH] AMD Alchemy: claim UART memory range

[Pantelis Antoniou]

On 04 Σεπ 2006, at 8:28 ΜΜ, Sergei Shtylyov wrote:

> Hello.
>
> Russell King wrote:
>
>>>>  BTW, can anybody enlighten me why 8250_au1x00.c came into  
>>>> being  at all?
>>>> Its only function seems to register the UART platform devices,  
>>>> the  thing
>>>> that is usually done in the board setup code, i. e. I'd rather  
>>>> have  it in arch/mips/au1000/common/platform.c (however, 8250.c  
>>>> should  have been able to filter out ports with UPIO_AU in case   
>>>> CONFIG_SERIAL_8250_AU1X00 undefined)...
>
>>> Seemed like a good idea at the moment to follow the already  
>>> existing  convention.
>
>> Already existing convention is as per Sergei's mail actually - to  
>> have the
>> platform device registration in arch/*.  The others which you  
>> thought were
>> convention there (accent, boca, fourport, hub6, mca) are all for  
>> add-in
>> cards and aren't architecture specific.
>
>> Hence, they can't live in arch/*.
>
>> So yes, 8250_au1x00.c breaks the established convention because it  
>> isn't
>> an add-in card.
>
>    Thanks for clarification.
>
>    Now another question to Pantelis: IIUC, the Alchemy UART  
> platform devices have UPF_SKIP_TEST set because of the Alchemy docs  
> claiming that UARTs other than UART3 don't have MCR/MSR and only  
> UART3 does have the full set of the modem control/status lines?   
> Were they indeed failing the loopback test for you? Asking because  
> on DBAu1550 board all (enabled) UARTs do pass the loopback test if  
> I get rid of this flag (however, Au1550 datasheet says MCR/MSR  
> exists on all UARTs, just no modem pins exist on UART0, and only  
> RTS-/CTS- pair on UART1 -- and the bits having no correspoding pins  
> seem to be tied high internally).
>    If I'm correct, the driver seems inconsistent in how it handles  
> UART_BUG_NOMSR flag, only checking it when deciding whether to  
> enable the modem status interrupts or not while actually it should  
> have been checked in serial8250_set_mctrl() and check_modem_status 
> () as well...
>    It also looks like the driver doesn't use Alchemy UARTs to their  
> full potential currently: UART3 has not only full set of modem  
> lines, but also is capable of the auto flow control (UART1 on  
> Au1550 also is).  (Making use of these features howewer are  
> complicated by the auto flow control being only available in the  
> late steppings of Au1500 and UART3 modem pins being multiplexed  
> with GPIO...)
>
> WBR, Sergei
>
> PS: CCing linux-mips to keep people here informed. :-)

Hi Sergei,

Yes, 1550 has proper UARTs on all port, but not 1200 ;)

Somehow I thought that hacking 8250 to support two different Au's  
(1550 & 1200)
wouldn't go down well; so I chickened out & settled for a subset that  
would work on
both. Feel free to fight your way through to support all the  
functionality you
require.

Pantelis

Re: [PATCH] AMD Alchemy: claim UART memory range

[Sergei Shtylyov]

Hello.

Pantelis Antoniou wrote:

>>>>>  BTW, can anybody enlighten me why 8250_au1x00.c came into  being  
>>>>> at all?
>>>>> Its only function seems to register the UART platform devices,  
>>>>> the  thing
>>>>> that is usually done in the board setup code, i. e. I'd rather  
>>>>> have  it in arch/mips/au1000/common/platform.c (however, 8250.c  
>>>>> should  have been able to filter out ports with UPIO_AU in case   
>>>>> CONFIG_SERIAL_8250_AU1X00 undefined)...

>>>> Seemed like a good idea at the moment to follow the already  
>>>> existing  convention.

>>> Already existing convention is as per Sergei's mail actually - to  
>>> have the
>>> platform device registration in arch/*.  The others which you  
>>> thought were
>>> convention there (accent, boca, fourport, hub6, mca) are all for  add-in
>>> cards and aren't architecture specific.

>>> Hence, they can't live in arch/*.

>>> So yes, 8250_au1x00.c breaks the established convention because it  
>>> isn't
>>> an add-in card.

>>    Thanks for clarification.

>>    Now another question to Pantelis: IIUC, the Alchemy UART  platform 
>> devices have UPF_SKIP_TEST set because of the Alchemy docs  claiming 
>> that UARTs other than UART3 don't have MCR/MSR and only  UART3 does 
>> have the full set of the modem control/status lines?   Were they 
>> indeed failing the loopback test for you? Asking because  on DBAu1550 
>> board all (enabled) UARTs do pass the loopback test if  I get rid of 
>> this flag (however, Au1550 datasheet says MCR/MSR  exists on all 
>> UARTs, just no modem pins exist on UART0, and only  RTS-/CTS- pair on 
>> UART1 -- and the bits having no correspoding pins  seem to be tied 
>> high internally).
>>    If I'm correct, the driver seems inconsistent in how it handles  
>> UART_BUG_NOMSR flag, only checking it when deciding whether to  enable 
>> the modem status interrupts or not while actually it should  have been 
>> checked in serial8250_set_mctrl() and check_modem_status () as well...
>>    It also looks like the driver doesn't use Alchemy UARTs to their  
>> full potential currently: UART3 has not only full set of modem  lines, 
>> but also is capable of the auto flow control (UART1 on  Au1550 also 
>> is).  (Making use of these features howewer are  complicated by the 
>> auto flow control being only available in the  late steppings of 
>> Au1500 and UART3 modem pins being multiplexed  with GPIO...)

>> PS: CCing linux-mips to keep people here informed. :-)

> Yes, 1550 has proper UARTs on all port, but not 1200 ;)

    No, it doesn't have "proper" UARTs on all ports (like all the other 
Alchemies), it's just said it has MCR/MSR on UART0/1 as well as on UART3. 
Actually, Au1200 also does, according to its datasheet.

> Somehow I thought that hacking 8250 to support two different Au's  (1550 
> & 1200)
> wouldn't go down well; so I chickened out & settled for a subset that  
> would work on
> both. Feel free to fight your way through to support all the  
> functionality you
> require.

    Well, now I certainly have no time for enabling any features, even for 
fixing buglets. So, if anybody of the linux-mips readers cares enough, it's 
their call... :-)
    At least UART_BUG_NOMSR handling should be extended if MCR/MSR are indeed 
missing on some SOCs.
    And since 0 in the bit 7 (U3) bit of sys_pinfunc determines if UART3 modem 
control/status are used for GPIO, this is also worth checking somewhere (if 
one wants to support the full set of the modem lines)...

> Pantelis

WBR, Sergei