AMD Alchemy: claim UART memory range

PATCH In review Sergei Shtylyov 2 participants 4 messages Apr 4, 2006 – Sep 4, 2006 3 messages not in the archive MIPS

lore

Share & tools

lore

Sergei Shtylyov

author

2006-04-04 18:38 UTC #0 lore raw

PATCH 1/1 AMD Alchemy: claim UART memory range

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 <[email protected]>

drivers/serial/8250.c +6 -0

index 674b15c..d641ac4 100644
@@ -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;

drivers/serial/8250_au1x00.c +2 -3

index 3d1bfd0..58015fd 100644
@@ -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[] = {

A message that is not in the archive (sent off-list or not yet received): [email protected]

Sergei Shtylyov

author

2006-09-04 17:26 UTC #4 lore raw

Hello.

Quoted 12 lines · Russell King

Quoted 6 lines
Quoted 4 lines
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.

Quoted 4 lines

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. :-)

Pantelis Antoniou

2006-09-04 17:39 UTC #5 lore raw

Quoted 30 lines · Sergei Shtylyov

Hello.
Quoted 12 lines · Russell King
Quoted 6 lines
Quoted 4 lines
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.
Quoted 4 lines
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...)

Quoted 3 lines

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

Sergei Shtylyov

author

2006-09-04 18:20 UTC #6 lore raw

Hello.

Quoted 26 lines · Pantelis Antoniou

Quoted 25 lines
Quoted 12 lines
Quoted 6 lines
Quoted 4 lines
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.
Quoted 4 lines
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...)

Quoted 3 lines

Quoted 1 line
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.

Quoted 4 lines

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)...

Quoted 1 line

Pantelis

WBR, Sergei