Re: [PATCH] 8250_early console support for au1x00 (again)
From: Sergei Shtylyov <hidden>
Date: 2006-08-28 14:50:16
Hello. Rodolfo Giometti wrote:
Here, attached and published on my site, my two new proposal for serial line and early console for au1x00.
The first patch:
http://ftp.enneenne.com/pub/misc/au1100-patches/linux/patch-au1x00-early-console
adds early console support for au1x00. In this version I decided to add the "AU" serial type in all kernel messages.
The second patch:
http://ftp.enneenne.com/pub/misc/au1100-patches/linux/patch-au1x00-serial-phys-addr
fixes serial address space by using only physical addresses. Please, note that this change allow to simply file "drivers/serial/8250.c" since functions __raw_readl()/__raw_writel() can be replaced by functions readl()/writel().
Sorry for follwing up 2 month ago, I just happen to stumble on some issues
addresses by these patches as well. I assume you haven't tried sending them to
Russel King?
Below are my comments:
quoted hunk ↗ jump to hunk
------------------------------------------------------------------------diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c index bcf1b10..78dcded 100644 --- a/arch/mips/kernel/setup.c +++ b/arch/mips/kernel/setup.c@@ -32,6 +32,7 @@ #include <linux/kdev_t.h> #include <linux/root_dev.h> #include <linux/highmem.h> +#include <linux/serial.h> #include <linux/console.h> #include <linux/mmzone.h> #include <linux/pfn.h>@@ -517,6 +518,10 @@ void __init setup_arch(char **cmdline_p) *cmdline_p = command_line; +#ifdef CONFIG_SERIAL_8250_CONSOLE + early_serial_console_init(*cmdline_p); +#endif + parse_cmdline_early(); bootmem_init(); sparse_init();diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c index bbf78aa..784d9d3 100644 --- a/drivers/serial/8250.c +++ b/drivers/serial/8250.c@@ -2336,8 +2336,11 @@ int __init serial8250_start_console(stru add_preferred_console("ttyS", line, options); printk("Adding console on ttyS%d at %s 0x%lx (options '%s')\n", - line, port->iotype == UPIO_MEM ? "MMIO" : "I/O port", - port->iotype == UPIO_MEM ? (unsigned long) port->mapbase : + line, + (port->iotype == UPIO_MEM) ? "MMIO" : \ + (port->iotype == UPIO_AU) ? "AU" : "I/O port", + (port->iotype == UPIO_MEM) || \ + (port->iotype == UPIO_AU) ? (unsigned long) port->mapbase : (unsigned long) port->iobase, options); if (!(serial8250_console.flags & CON_ENABLED)) { serial8250_console.flags &= ~CON_PRINTBUFFER;diff --git a/drivers/serial/8250_early.c b/drivers/serial/8250_early.c index 7e51119..319baa9 100644 --- a/drivers/serial/8250_early.c +++ b/drivers/serial/8250_early.c
[...]
quoted hunk ↗ jump to hunk
@@ -44,22 +52,133 @@ struct early_uart_device {
[...]
+static unsigned int serial_in(struct uart_port *port, int offset)
{
- if (port->iotype == UPIO_MEM)
+ offset = map_in_reg(port, offset) << port->regshift;
+
+ switch (port->iotype) {
+ case UPIO_HUB6:
+ outb(port->hub6 - 1 + offset, port->iobase);
+ return inb(port->iobase + 1);
This module can't handle HAB6 otherwise, so whay add it here?
+ case UPIO_MEM: return readb(port->membase + offset); - else + + 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.
+
+ default:
return inb(port->iobase + offset);
+ }
}
-static void __init serial_out(struct uart_port *port, int offset, int value)
+static void
+serial_out(struct uart_port *port, int offset, int value)
{
- if (port->iotype == UPIO_MEM)
+ offset = map_out_reg(port, offset) << port->regshift;
+
+ switch (port->iotype) {
+ case UPIO_HUB6:
+ outb(port->hub6 - 1 + offset, port->iobase);
+ outb(value, port->iobase + 1);
+ break;
+
+ case UPIO_MEM:
writeb(value, port->membase + offset);
- else
+ break;
+
+ case UPIO_MEM32:
+ case UPIO_AU:
+ writel(value, port->membase + offset);
+ break;
+
The same NAK with writel()...
quoted hunk ↗ jump to hunk
+ default: outb(value, port->iobase + offset); + } +} + +/* Uart divisor latch read */ +static inline int _serial_dl_read(struct uart_port *port) +{ + return serial_in(port, UART_DLL) | serial_in(port, UART_DLM) << 8; +} + +/* Uart divisor latch write */ +static inline void _serial_dl_write(struct uart_port *port, int value) +{ + serial_out(port, UART_DLL, value & 0xff); + serial_out(port, UART_DLM, value >> 8 & 0xff); +} + +#ifdef CONFIG_SERIAL_8250_AU1X00 +/* Au1x00 haven't got a standard divisor latch */ +static int serial_dl_read(struct uart_port *port) +{ + if (port->iotype == UPIO_AU) + return readl(port->membase + 0x28); + else + return _serial_dl_read(port); } +static void serial_dl_write(struct uart_port *port, int value) +{ + if (port->iotype == UPIO_AU) + writel(value, port->membase + 0x28); + else + _serial_dl_write(port, value); +} +#else +#define serial_dl_read(port) _serial_dl_read(port) +#define serial_dl_write(port, value) _serial_dl_write(port, value) +#endif + #define BOTH_EMPTY (UART_LSR_TEMT | UART_LSR_THRE) static void __init wait_for_xmitr(struct uart_port *port)@@ -98,16 +217,17 @@ static void __init early_uart_write(stru static unsigned int __init probe_baud(struct uart_port *port) { - unsigned char lcr, dll, dlm; + unsigned char lcr; unsigned int quot; + int sd = ((int)(__raw_readl(SYS_POWERCTRL)&0x03)) + 2; /* Au1x00 SD */ lcr = serial_in(port, UART_LCR); serial_out(port, UART_LCR, lcr | UART_LCR_DLAB); - dll = serial_in(port, UART_DLL); - dlm = serial_in(port, UART_DLM); + quot = serial_dl_read(port); serial_out(port, UART_LCR, lcr); - quot = (dlm << 8) | dll; + if (port->iotype == UPIO_AU) + return CPU_SPEED / (sd * quot * 32); return (port->uartclk / 16) / quot; }@@ -142,6 +266,7 @@ static int __init parse_options(struct e port->uartclk = BASE_BAUD * 16; if (!strncmp(options, "mmio,", 5)) { port->iotype = UPIO_MEM; + port->regshift = 0;
Maybe regshift even deserves the separate (but optional) field in the boot
option...
[...]quoted hunk ↗ jump to hunk
@@ -169,8 +309,7 @@ static int __init parse_options(struct e } printk(KERN_INFO "Early serial console at %s 0x%lx (options '%s')\n", - mmio ? "MMIO" : "I/O port", - mmio ? port->mapbase : (unsigned long) port->iobase, + buf, mmio ? port->mapbase : (unsigned long) port->iobase, device->options); return 0; }@@ -227,22 +366,23 @@ static int __init early_uart_console_swi { struct early_uart_device *device = &early_device; struct uart_port *port = &device->port; - int mmio, line; + int line; if (!(early_uart_console.flags & CON_ENABLED)) return 0; /* Try to start the normal driver on a matching line. */ - mmio = (port->iotype == UPIO_MEM); line = serial8250_start_console(port, device->options); if (line < 0) printk("No ttyS device at %s 0x%lx for console\n", - mmio ? "MMIO" : "I/O port", - mmio ? port->mapbase : + (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...
quoted hunk ↗ jump to hunk
diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c 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.
return (port1->mapbase == port2->mapbase); } return 0;
quoted hunk ↗ jump to hunk
------------------------------------------------------------------------diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c index 784d9d3..40c9097 100644 --- a/drivers/serial/8250.c +++ b/drivers/serial/8250.c@@ -311,12 +311,8 @@ static unsigned int serial_in(struct uar return readb(up->port.membase + offset); case UPIO_MEM32: - return readl(up->port.membase + offset); - -#ifdef CONFIG_SERIAL_8250_AU1X00 case UPIO_AU: - return __raw_readl(up->port.membase + offset); -#endif + return readl(up->port.membase + offset);
NAK for the reason already stated.
quoted hunk ↗ jump to hunk
@@ -339,14 +335,9 @@ serial_out(struct uart_8250_port *up, in break; case UPIO_MEM32: - writel(value, up->port.membase + offset); - break; - -#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().
quoted hunk ↗ jump to hunk
@@ -380,7 +371,7 @@ static inline void _serial_dl_write(stru static int serial_dl_read(struct uart_8250_port *up) { if (up->port.iotype == UPIO_AU) - return __raw_readl(up->port.membase + 0x28); + return readl(up->port.membase + 0x28); else return _serial_dl_read(up); }@@ -388,7 +379,7 @@ static int serial_dl_read(struct uart_82 static void serial_dl_write(struct uart_8250_port *up, int value) { if (up->port.iotype == UPIO_AU) - __raw_writel(value, up->port.membase + 0x28); + writel(value, up->port.membase + 0x28); else _serial_dl_write(up, value); }
Ditto here.
quoted hunk ↗ jump to hunk
diff --git a/drivers/serial/8250_au1x00.c b/drivers/serial/8250_au1x00.c index 58015fd..9d0f1be 100644 --- a/drivers/serial/8250_au1x00.c +++ b/drivers/serial/8250_au1x00.c
The reason this file came into being at all is doubtful to me, BTW...
The platform devices should be registered in the platform setup code.
WBR, Sergei