[PATCH] Au1x00 8250 uart support.
Pantelis Antoniou
The following patch enables the use of the Au1x00 AMD SoC UARTs
through the standard 8250 serial driver.
The Au1x00 UARTs are weird in the following two ways:
1) The registers are not present at the normal offsets, so
a mapping function is used to transform them.
b) Modem status signals are not supported on all the members
of the Au1x00 family, so the modem status change interrupts
must not be enabled, or we die in an irq storm.
In order to not affect any standard 8250 parts, the code in
question is #ifdef'ed on CONFIG_SERIAL_8250_AU1X00. If you
deem the readability of the code is more important you may
removed them, without any effect on the functionality.
I've also considered abstracting the register mapping to a
couple of extra fields on plat_serial8250_port, but wasn't
sure how that would go.
Please comment.
Pantelis
----
Signed-off-by: Pantelis Antoniou <pant...@embeddedalley.ocm>
drivers/serial/8250.c | 63 ++++++++++++++++++++++++++++
drivers/serial/8250.h | 1
drivers/serial/8250_au1x00.c | 95
+++++++++++++++++++++++++++++++++++++++++++
drivers/serial/Kconfig | 8 +++
drivers/serial/Makefile | 1
drivers/serial/serial_core.c | 1
include/linux/serial_8250.h | 1
include/linux/serial_core.h | 4 +
8 files changed, 172 insertions(+), 2 deletions(-)
Christoph Hellwig
> {
> +#ifdef CONFIG_SERIAL_8250_AU1X00
> + if (up->port.iotype == UPIO_AU)
> + offset = au_io_in_map[offset];
> +#endif
All this ifdef stuff is rather messy. Allowing the driver to specity a map
in some structure might make more sense.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Pantelis Antoniou
> > static _INLINE_ unsigned int serial_in(struct uart_8250_port *up, int offset)
> > {
> > +#ifdef CONFIG_SERIAL_8250_AU1X00
> > + if (up->port.iotype == UPIO_AU)
> > + offset = au_io_in_map[offset];
> > +#endif
>
> All this ifdef stuff is rather messy. Allowing the driver to specity a map
> in some structure might make more sense.
>
>
Sure, I can do that.
But the check for the map existence will take a couple of instructions then,
for all architectures. If you're fine with that, it'd be no problem.
Regards
Pantelis
Chris Wedgwood
> Sure, I can do that.
>
> But the check for the map existence will take a couple of
> instructions then, for all architectures. If you're fine with that,
> it'd be no problem.
Or you an define an accessor #define foo_map(x,y) which would contain
*one* ifdef there and default to a NOP for all but au uart inflicted
platforms.
Russell King
> diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
> --- a/drivers/serial/8250.c
> +++ b/drivers/serial/8250.c
> @@ -249,10 +249,42 @@ static const struct serial8250_config ua
> .fcr = UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
> .flags = UART_CAP_FIFO | UART_CAP_UUE,
> },
> + [PORT_AU1X00] = {
> + .name = "Au1x00",
> + .fifo_size = 16,
> + .tx_loadsz = 16,
> + .fcr = UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
> + .flags = UART_CAP_FIFO | UART_CAP_NOMSR,
> + },
> +};
> +
> +#ifdef CONFIG_SERIAL_8250_AU1X00
> +static const u8 au_io_in_map[] = {
> + [UART_RX] = 0,
> + [UART_IER] = 2,
> + [UART_IIR] = 3,
> + [UART_LCR] = 5,
> + [UART_MCR] = 6,
> + [UART_LSR] = 7,
> + [UART_MSR] = 8,
> };
>
> +static const u8 au_io_out_map[] = {
> + [UART_TX] = 1,
> + [UART_IER] = 2,
> + [UART_FCR] = 4,
> + [UART_LCR] = 5,
> + [UART_MCR] = 6,
> +};
> +#endif
> +
> static _INLINE_ unsigned int serial_in(struct uart_8250_port *up, int offset)
> {
> +#ifdef CONFIG_SERIAL_8250_AU1X00
> + if (up->port.iotype == UPIO_AU)
> + offset = au_io_in_map[offset];
> +#endif
> offset <<= up->port.regshift;
>
> switch (up->port.iotype) {
> @@ -266,6 +298,11 @@ static _INLINE_ unsigned int serial_in(s
> 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
> +
At some point, we probably want to make this non-AU1x00 specific, but
it may as well wait until someone else has such quirkyness themselves.
It would be nice to pass the maps in somehow, but that's a fair re-jig
of the interfaces to other chunks of code elsewhere in the kernel tree.
Therefore, I think it's a change we should not make until there's a
reason to make it.
> default:
> return inb(up->port.iobase + offset);
> }
> @@ -274,6 +311,11 @@ static _INLINE_ unsigned int serial_in(s
> static _INLINE_ void
> serial_out(struct uart_8250_port *up, int offset, int value)
> {
> +#ifdef CONFIG_SERIAL_8250_AU1X00
> + if (up->port.iotype == UPIO_AU)
> + offset = au_io_out_map[offset];
> +#endif
> +
> offset <<= up->port.regshift;
>
> switch (up->port.iotype) {
> @@ -290,6 +332,12 @@ serial_out(struct uart_8250_port *up, in
> writel(value, up->port.membase + offset);
> break;
>
> +#ifdef CONFIG_SERIAL_8250_AU1X00
> + case UPIO_AU:
> + __raw_writel(value, up->port.membase + offset);
> + break;
> +#endif
> +
> default:
> outb(value, up->port.iobase + offset);
> }
> @@ -910,6 +958,15 @@ static void autoconfig(struct uart_8250_
> }
> }
> #endif
> +
> +#ifdef CONFIG_SERIAL_8250_AU1X00
> + /* if access method is AU, it is a AU */
> + if (up->port.type == PORT_16550A && up->port.iotype == UPIO_AU) {
> + up->port.type = PORT_AU1X00;
I don't really see the need for a new port type - it's a quirky
16550A with a silly register layout.
> + up->capabilities |= UART_CAP_NOMSR; /* to stop whine */
Since this is for a hardware quirk, this should be UART_BUG_xxx not
UART_CAP_xxx - it's not a capability as such.
> + }
> +#endif
> +
> serial_outp(up, UART_LCR, save_lcr);
>
> if (up->capabilities != uart_config[up->port.type].flags) {
> @@ -1057,6 +1114,10 @@ static void serial8250_enable_ms(struct
> {
> struct uart_8250_port *up = (struct uart_8250_port *)port;
>
> + /* no MSR capabilities */
> + if (up->capabilities & UART_CAP_NOMSR)
ditto.
> + return;
> +
> up->ier |= UART_IER_MSI;
> serial_out(up, UART_IER, up->ier);
> }
> @@ -1774,7 +1835,7 @@ serial8250_set_termios(struct uart_port
> * CTS flow control flag and modem status interrupts
> */
> up->ier &= ~UART_IER_MSI;
> - if (UART_ENABLE_MS(&up->port, termios->c_cflag))
> + if (!(up->capabilities & UART_CAP_NOMSR) && UART_ENABLE_MS(&up->port, termios->c_cflag))
ditto.
> up->ier |= UART_IER_MSI;
> if (up->capabilities & UART_CAP_UUE)
> up->ier |= UART_IER_UUE | UART_IER_RTOIE;
> diff --git a/drivers/serial/8250.h b/drivers/serial/8250.h
> --- a/drivers/serial/8250.h
> +++ b/drivers/serial/8250.h
> @@ -46,6 +46,7 @@ struct serial8250_config {
> #define UART_CAP_SLEEP (1 << 10) /* UART has IER sleep */
> #define UART_CAP_AFE (1 << 11) /* MCR-based hw flow control */
> #define UART_CAP_UUE (1 << 12) /* UART needs IER bit 6 set (Xscale) */
> +#define UART_CAP_NOMSR (1 << 13) /* UART has no modem status bits (Au1x00) */
ditto.
>
> #define UART_BUG_QUOT (1 << 0) /* UART has buggy quot LSB */
> #define UART_BUG_TXEN (1 << 1) /* UART has buggy TX IIR status */
> diff --git a/drivers/serial/8250_au1x00.c b/drivers/serial/8250_au1x00.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/serial/8250_au1x00.c
> @@ -0,0 +1,95 @@
> +/*
> + * Serial Device Initialisation for Au1x00
> + *
> + * (C) Copyright Embedded Alley Solutions, Inc 2005
> + * Author: Pantelis Antoniou <pant...@embeddedalley.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/ioport.h>
> +#include <linux/module.h>
> +#include <linux/serial_core.h>
> +#include <linux/signal.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +
> +#include <linux/serial_8250.h>
> +
> +#include <asm/mach-au1x00/au1000.h>
> +
> +#include "8250.h"
> +
> +#define PORT(_base, _irq) \
> + { \
> + .iobase = _base, \
> + .membase = (void __iomem *)_base,\
> + .mapbase = _base, \
> + .irq = _irq, \
> + .uartclk = 0, /* filled */ \
> + .regshift = 2, \
> + .iotype = UPIO_AU, \
> + .flags = UPF_SKIP_TEST | \
> + UPF_IOREMAP, \
> + }
> +
> +static struct plat_serial8250_port au1x00_data[] = {
> +#if defined(CONFIG_SOC_AU1000)
> + PORT(UART0_ADDR, AU1000_UART0_INT),
> + PORT(UART1_ADDR, AU1000_UART1_INT),
> + PORT(UART2_ADDR, AU1000_UART2_INT),
> + PORT(UART3_ADDR, AU1000_UART3_INT),
> +#elif defined(CONFIG_SOC_AU1500)
> + PORT(UART0_ADDR, AU1500_UART0_INT),
> + PORT(UART3_ADDR, AU1500_UART3_INT),
> +#elif defined(CONFIG_SOC_AU1100)
> + PORT(UART0_ADDR, AU1100_UART0_INT),
> + PORT(UART1_ADDR, AU1100_UART1_INT),
> + PORT(UART2_ADDR, AU1100_UART2_INT),
> + PORT(UART3_ADDR, AU1100_UART3_INT),
> +#elif defined(CONFIG_SOC_AU1550)
> + PORT(UART0_ADDR, AU1550_UART0_INT),
> + PORT(UART1_ADDR, AU1550_UART1_INT),
> + PORT(UART2_ADDR, AU1550_UART2_INT),
> + PORT(UART3_ADDR, AU1550_UART3_INT),
> +#elif defined(CONFIG_SOC_AU1200)
> + PORT(UART0_ADDR, AU1200_UART0_INT),
> + PORT(UART1_ADDR, AU1200_UART1_INT),
> +#endif
> + { },
> +};
> +
> +static struct platform_device au1x00_device = {
> + .name = "serial8250",
> + .id = PLAT8250_DEV_AU1X00,
> + .dev = {
> + .platform_data = au1x00_data,
> + },
> +};
> +
> +static int __init au1x00_init(void)
> +{
> + int i;
> + unsigned int uartclk;
> +
> + /* get uart clock */
> + uartclk = get_au1x00_uart_baud_base() * 16;
> +
> + /* fill up uartclk */
> + for (i = 0; au1x00_data[i].flags ; i++)
> + au1x00_data[i].uartclk = uartclk;
> +
> + return platform_device_register(&au1x00_device);
This seems to be capable of being a modular, yet it's registering a
static platform device structure, which will cause an oops if this
module is force-unloaded. Should it always be built in?
> +}
> +
> +module_init(au1x00_init);
> +
> +MODULE_AUTHOR("Pantelis Antoniou <pant...@embeddedalley.com>");
> +MODULE_DESCRIPTION("8250 serial probe module for Au1x000 cards");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -207,6 +207,14 @@ config SERIAL_8250_ACORN
> system, say Y to this option. The driver can handle 1, 2, or 3 port
> cards. If unsure, say N.
>
> +config SERIAL_8250_AU1X00
> + tristate "AU1X00 serial port support"
> + depends on SOC_AU1X00 && SERIAL_8250
> + help
> + If you have an Au1x00 board and want to use the serial port, say Y
> + to this option. The driver can handle 1 or 2 serial ports.
> + If unsure, say N.
> +
> comment "Non-8250 serial port support"
>
> config SERIAL_AMBA_PL010
> diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
> --- a/drivers/serial/Makefile
> +++ b/drivers/serial/Makefile
> @@ -22,6 +22,7 @@ obj-$(CONFIG_SERIAL_8250_ACCENT) += 8250
> obj-$(CONFIG_SERIAL_8250_BOCA) += 8250_boca.o
> obj-$(CONFIG_SERIAL_8250_HUB6) += 8250_hub6.o
> obj-$(CONFIG_SERIAL_8250_MCA) += 8250_mca.o
> +obj-$(CONFIG_SERIAL_8250_AU1X00) += 8250_au1x00.o
> obj-$(CONFIG_SERIAL_AMBA_PL010) += amba-pl010.o
> obj-$(CONFIG_SERIAL_AMBA_PL011) += amba-pl011.o
> obj-$(CONFIG_SERIAL_CLPS711X) += clps711x.o
> diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
> --- a/drivers/serial/serial_core.c
> +++ b/drivers/serial/serial_core.c
> @@ -1960,6 +1960,7 @@ uart_report_port(struct uart_driver *drv
> break;
> case UPIO_MEM:
> case UPIO_MEM32:
> + case UPIO_AU:
> snprintf(address, sizeof(address),
> "MMIO 0x%lx", port->mapbase);
> break;
> diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
> --- a/include/linux/serial_8250.h
> +++ b/include/linux/serial_8250.h
> @@ -42,6 +42,7 @@ enum {
> PLAT8250_DEV_BOCA,
> PLAT8250_DEV_HUB6,
> PLAT8250_DEV_MCA,
> + PLAT8250_DEV_AU1X00,
> };
>
> /*
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -39,7 +39,8 @@
> #define PORT_RSA 13
> #define PORT_NS16550A 14
> #define PORT_XSCALE 15
> -#define PORT_MAX_8250 15 /* max port ID */
> +#define PORT_AU1X00 16
> +#define PORT_MAX_8250 16 /* max port ID */
>
> /*
> * ARM specific type numbers. These are not currently guaranteed
> @@ -209,6 +210,7 @@ struct uart_port {
> #define UPIO_HUB6 (1)
> #define UPIO_MEM (2)
> #define UPIO_MEM32 (3)
> +#define UPIO_AU (4) /* Au1x00 type IO */
>
> unsigned int read_status_mask; /* driver specific */
> unsigned int ignore_status_mask; /* driver specific */
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
Russell King
> @@ -251,9 +251,60 @@ static const struct serial8250_config ua
> },
> };
>
> +#ifdef CONFIG_SERIAL_8250_AU1X00
> +
> +/* Au1x00 UART hardware has a weird register layout */
> +static const u8 au_io_in_map[] = {
> + [UART_RX] = 0,
> + [UART_IER] = 2,
> + [UART_IIR] = 3,
> + [UART_LCR] = 5,
> + [UART_MCR] = 6,
> + [UART_LSR] = 7,
> + [UART_MSR] = 8,
> +
> +static const u8 au_io_out_map[] = {
> + [UART_TX] = 1,
> + [UART_IER] = 2,
> + [UART_FCR] = 4,
> + [UART_LCR] = 5,
> + [UART_MCR] = 6,
> +};
> +
> +static inline int map_8250_in_reg(struct uart_8250_port *up, int offset)
> +{
> + if (up->port.iotype != UPIO_AU)
> + return offset;
> + return au_io_in_map[offset];
> +}
> +
> +static inline int map_8250_out_reg(struct uart_8250_port *up, int offset)
> +{
> + if (up->port.iotype != UPIO_AU)
> + return offset;
> + return au_io_out_map[offset];
> +}
> +
> +#else
> +
> +/* sane hardware needs no mapping */
> +static inline int map_8250_in_reg(struct uart_8250_port *up, int offset)
> +{
> + return offset;
> +}
> +
> +static inline int map_8250_out_reg(struct uart_8250_port *up, int offset)
> +{
> + return offset;
> +}
> +
> +#endif
I'm not sure this is any cleaner. Maybe:
#define map_8250_in_reg(port,offset) (offset)
#define map_8250_out_reg(port,offset) (offset)
would be better for the null case?
> @@ -1774,7 +1847,7 @@ serial8250_set_termios(struct uart_port
> * CTS flow control flag and modem status interrupts
> */
> up->ier &= ~UART_IER_MSI;
> - if (UART_ENABLE_MS(&up->port, termios->c_cflag))
> + if (!(up->bugs & UART_BUG_NOMSR) && UART_ENABLE_MS(&up->port, termios->c_cflag))
This should probably be wrapped thusly:
if (!(up->bugs & UART_BUG_NOMSR) &&
UART_ENABLE_MS(&up->port, termios->c_cflag))
> +static struct platform_device au1x00_device = {
> + .name = "serial8250",
> + .id = PLAT8250_DEV_AU1X00,
> + .dev = {
> + .platform_data = au1x00_data,
> + },
> +};
> +
> +static int __init au1x00_init(void)
> +{
> + int i;
> + unsigned int uartclk;
> +
> + /* get uart clock */
> + uartclk = get_au1x00_uart_baud_base() * 16;
> +
> + /* fill up uartclk */
> + for (i = 0; au1x00_data[i].flags ; i++)
> + au1x00_data[i].uartclk = uartclk;
> +
> + return platform_device_register(&au1x00_device);
> +}
> +
> +static void __exit au1x00_exit(void)
> +{
> + platform_device_unregister(&au1x00_device);
> +}
> +
> +module_init(au1x00_init);
> +module_exit(au1x00_exit);
This is also buggy - the problem this has is that although you're
unregistering the platform device, references can still remain after
the module has been unloaded. This means you have two options:
1. never allow code with statically allocated platform devices to be
modules.
2. use something like platform_device_register_simple() (which doesn't
currently exist in a useful form for devices with platform data.)
> diff --git a/include/asm-mips/mach-au1x00/au1000.h b/include/asm-mips/mach-au1x00/au1000.h
> --- a/include/asm-mips/mach-au1x00/au1000.h
> +++ b/include/asm-mips/mach-au1x00/au1000.h
> @@ -906,6 +906,8 @@ extern au1xxx_irq_map_t au1xxx_irq_map[]
> #define UART_BASE UART0_ADDR
> #define UART_DEBUG_BASE UART3_ADDR
>
> +#ifdef CONFIG_SERIAL_AU1X00
> +
> #define UART_RX 0 /* Receive buffer */
> #define UART_TX 4 /* Transmit buffer */
> #define UART_IER 8 /* Interrupt Enable Register */
> @@ -996,6 +998,8 @@ extern au1xxx_irq_map_t au1xxx_irq_map[]
> #define UART_MSR_DCTS 0x01 /* Delta CTS */
> #define UART_MSR_ANY_DELTA 0x0F /* Any of the delta bits! */
>
> +#endif
> +
Are these defines required anymore?
Pantelis Antoniou
Done.
> > @@ -1774,7 +1847,7 @@ serial8250_set_termios(struct uart_port
> > * CTS flow control flag and modem status interrupts
> > */
> > up->ier &= ~UART_IER_MSI;
> > - if (UART_ENABLE_MS(&up->port, termios->c_cflag))
> > + if (!(up->bugs & UART_BUG_NOMSR) && UART_ENABLE_MS(&up->port, termios->c_cflag))
>
> This should probably be wrapped thusly:
>
> if (!(up->bugs & UART_BUG_NOMSR) &&
> UART_ENABLE_MS(&up->port, termios->c_cflag))
>
Done.
Yes, this is stinking mess. I resorted to just not allowing a module build.
BTW, the current platform_device_register_simple suffers from the same bug.
! struct platform_device *platform_device_register_simple(char *name, unsigned int id,
! struct resource *res, unsigned int num)
!
later on in the function body we have...
! pobj->pdev.name = name;
It's clearly buggy, since the typical calling sequence passes a constant string as a 'name'
argument, which is located in the read only segment of the module (.text or .rodata).
To fix this space for the name must be allocated just like the resources...
> > diff --git a/include/asm-mips/mach-au1x00/au1000.h b/include/asm-mips/mach-au1x00/au1000.h
> > --- a/include/asm-mips/mach-au1x00/au1000.h
> > +++ b/include/asm-mips/mach-au1x00/au1000.h
> > @@ -906,6 +906,8 @@ extern au1xxx_irq_map_t au1xxx_irq_map[]
> > #define UART_BASE UART0_ADDR
> > #define UART_DEBUG_BASE UART3_ADDR
> >
> > +#ifdef CONFIG_SERIAL_AU1X00
> > +
> > #define UART_RX 0 /* Receive buffer */
> > #define UART_TX 4 /* Transmit buffer */
> > #define UART_IER 8 /* Interrupt Enable Register */
> > @@ -996,6 +998,8 @@ extern au1xxx_irq_map_t au1xxx_irq_map[]
> > #define UART_MSR_DCTS 0x01 /* Delta CTS */
> > #define UART_MSR_ANY_DELTA 0x0F /* Any of the delta bits! */
> >
> > +#endif
> > +
>
> Are these defines required anymore?
>
It's a MIPS tree problem. Removed from the patch, will be dealt through there.
Regards
Pantelis
Russell King
> > This is also buggy - the problem this has is that although you're
> > unregistering the platform device, references can still remain after
> > the module has been unloaded. This means you have two options:
> >
> > 1. never allow code with statically allocated platform devices to be
> > modules.
> > 2. use something like platform_device_register_simple() (which doesn't
> > currently exist in a useful form for devices with platform data.)
> >
>
> Yes, this is stinking mess. I resorted to just not allowing a module build.
>
> BTW, the current platform_device_register_simple suffers from the same bug.
>
> ! struct platform_device *platform_device_register_simple(char *name, unsigned int id,
> ! struct resource *res, unsigned int num)
> !
>
> later on in the function body we have...
>
> ! pobj->pdev.name = name;
>
> It's clearly buggy, since the typical calling sequence passes a constant
> string as a 'name' argument, which is located in the read only segment
> of the module (.text or .rodata).
>
> To fix this space for the name must be allocated just like the resources...
Is this true? platform_device_register uses pdev.name to construct the
bus id. pdev.name is also used when matching against drivers.
However, once a platform device is unregistered, it is no longer available
to be matched against drivers, so pdev.name is unused. Therefore, I don't
think this is a problem.
> +#ifdef CONFIG_SERIAL_8250_AU1X00_MODULE
> +#error This file can not yet be compiled as a module.
> +#endif
This isn't how we prevent modular builds...
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -207,6 +207,14 @@ config SERIAL_8250_ACORN
> system, say Y to this option. The driver can handle 1, 2, or 3 port
> cards. If unsure, say N.
>
> +config SERIAL_8250_AU1X00
> + tristate "AU1X00 serial port support"
We prevent them by setting this to 'bool'.
> + depends on SOC_AU1X00 && SERIAL_8250
It doens't really depend on SERIAL_8250 - it can live independently of it.
However, it can be built in when SERIAL_8250 isn't, so this dependency
should be SERIAL_8250 != n.
Pantelis Antoniou
It's awefully risky. At least a comment is in order.
> > +#ifdef CONFIG_SERIAL_8250_AU1X00_MODULE
> > +#error This file can not yet be compiled as a module.
> > +#endif
>
> This isn't how we prevent modular builds...
>
> > diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> > --- a/drivers/serial/Kconfig
> > +++ b/drivers/serial/Kconfig
> > @@ -207,6 +207,14 @@ config SERIAL_8250_ACORN
> > system, say Y to this option. The driver can handle 1, 2, or 3 port
> > cards. If unsure, say N.
> >
> > +config SERIAL_8250_AU1X00
> > + tristate "AU1X00 serial port support"
>
> We prevent them by setting this to 'bool'.
>
OK
> > + depends on SOC_AU1X00 && SERIAL_8250
>
> It doens't really depend on SERIAL_8250 - it can live independently of it.
> However, it can be built in when SERIAL_8250 isn't, so this dependency
> should be SERIAL_8250 != n.
>
OK
Russell King
Do you have a description and sign-off for this patch?
> diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
> --- a/drivers/serial/8250.c
> +++ b/drivers/serial/8250.c
> @@ -251,9 +251,53 @@ static const struct serial8250_config ua
> +#define map_8250_in_reg(up, offset) (offset)
> +#define map_8250_out_reg(up, offset) (offset)
> +
> +#endif
> +
> static _INLINE_ unsigned int serial_in(struct uart_8250_port *up, int offset)
> {
> - offset <<= up->port.regshift;
> + offset = map_8250_in_reg(up, offset) << up->port.regshift;
>
> switch (up->port.iotype) {
> case UPIO_HUB6:
> @@ -266,6 +310,11 @@ static _INLINE_ unsigned int serial_in(s
> 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
> +
> default:
> return inb(up->port.iobase + offset);
> }
> @@ -274,7 +323,7 @@ static _INLINE_ unsigned int serial_in(s
> static _INLINE_ void
> serial_out(struct uart_8250_port *up, int offset, int value)
> {
> - offset <<= up->port.regshift;
> + offset = map_8250_out_reg(up, offset) << up->port.regshift;
>
> switch (up->port.iotype) {
> case UPIO_HUB6:
> @@ -290,6 +339,12 @@ serial_out(struct uart_8250_port *up, in
> writel(value, up->port.membase + offset);
> break;
>
> +#ifdef CONFIG_SERIAL_8250_AU1X00
> + case UPIO_AU:
> + __raw_writel(value, up->port.membase + offset);
> + break;
> +#endif
> +
> default:
> outb(value, up->port.iobase + offset);
> }
> @@ -910,6 +965,13 @@ static void autoconfig(struct uart_8250_
> }
> }
> #endif
> +
> +#ifdef CONFIG_SERIAL_8250_AU1X00
> + /* if access method is AU, it is a 16550 with a quirk */
> + if (up->port.type == PORT_16550A && up->port.iotype == UPIO_AU)
> + up->bugs |= UART_BUG_NOMSR;
> +#endif
> +
> serial_outp(up, UART_LCR, save_lcr);
>
> if (up->capabilities != uart_config[up->port.type].flags) {
> @@ -1057,6 +1119,10 @@ static void serial8250_enable_ms(struct
> {
> struct uart_8250_port *up = (struct uart_8250_port *)port;
>
> + /* no MSR capabilities */
> + if (up->bugs & UART_BUG_NOMSR)
> + return;
> +
> up->ier |= UART_IER_MSI;
> serial_out(up, UART_IER, up->ier);
> }
> @@ -1774,7 +1840,8 @@ serial8250_set_termios(struct uart_port
> * CTS flow control flag and modem status interrupts
> */
> up->ier &= ~UART_IER_MSI;
> - if (UART_ENABLE_MS(&up->port, termios->c_cflag))
> + if (!(up->bugs & UART_BUG_NOMSR) &&
> + UART_ENABLE_MS(&up->port, termios->c_cflag))
> up->ier |= UART_IER_MSI;
> if (up->capabilities & UART_CAP_UUE)
> up->ier |= UART_IER_UUE | UART_IER_RTOIE;
> diff --git a/drivers/serial/8250.h b/drivers/serial/8250.h
> --- a/drivers/serial/8250.h
> +++ b/drivers/serial/8250.h
> @@ -49,6 +49,7 @@ struct serial8250_config {
>
> #define UART_BUG_QUOT (1 << 0) /* UART has buggy quot LSB */
> #define UART_BUG_TXEN (1 << 1) /* UART has buggy TX IIR status */
> +#define UART_BUG_NOMSR (1 << 2) /* UART has buggy MSR status bits (Au1x00) */
>
> #if defined(__i386__) && (defined(CONFIG_M386) || defined(CONFIG_M486))
> #define _INLINE_ inline
> diff --git a/drivers/serial/8250_au1x00.c b/drivers/serial/8250_au1x00.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/serial/8250_au1x00.c
> @@ -0,0 +1,102 @@
> +/* XXX: Yes, I know this doesn't yet work. */
> +static void __exit au1x00_exit(void)
> +{
> + platform_device_unregister(&au1x00_device);
> +}
> +
> +module_init(au1x00_init);
> +module_exit(au1x00_exit);
> +
> +MODULE_AUTHOR("Pantelis Antoniou <pant...@embeddedalley.com>");
> +MODULE_DESCRIPTION("8250 serial probe module for Au1x000 cards");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -207,6 +207,14 @@ config SERIAL_8250_ACORN
> system, say Y to this option. The driver can handle 1, 2, or 3 port
> cards. If unsure, say N.
>
> +config SERIAL_8250_AU1X00
> + bool "AU1X00 serial port support"
> + depends on SERIAL_8250 != n && SOC_AU1X00
> @@ -209,6 +209,7 @@ struct uart_port {
> #define UPIO_HUB6 (1)
> #define UPIO_MEM (2)
> #define UPIO_MEM32 (3)
> +#define UPIO_AU (4) /* Au1x00 type IO */
>
> unsigned int read_status_mask; /* driver specific */
> unsigned int ignore_status_mask; /* driver specific */
Pantelis Antoniou
> On Thu, Sep 22, 2005 at 10:11:21PM +0300, Pantelis Antoniou wrote:
> > Once more, with the MEM32 chunks out.
> >
> > Regards
> >
> > Pantelis
>
> Do you have a description and sign-off for this patch?
>
>
---
Support Au1x00 8250 UARTs using the generic 8250 driver.
The offsets of the registers are in a different place, and
some parts cannot handle a full set of modem control signals.
---
Signed-off-by: Pantelis Antoniou <pant...@embeddedalley.ocm>
----
Russell King
> On Sunday 06 November 2005 10:44, Russell King wrote:
> > Do you have a description and sign-off for this patch?
>
> ---
>
> Support Au1x00 8250 UARTs using the generic 8250 driver.
> The offsets of the registers are in a different place, and
> some parts cannot handle a full set of modem control signals.
>
> ---
>
> Signed-off-by: Pantelis Antoniou <pant...@embeddedalley.ocm>
Thanks, applied.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core