[PATCH 0/7] serial/8250: I/O accessor cleanups

[PATCH 0/7] serial/8250: I/O accessor cleanups

[Arnd Bergmann]

Hi Greg,

This series of patches cleans up the part of the 8250 device
driver that is responsible for accessing the hardware registers.
The driver defines all sorts of methods to do that right now,
when it really should only support memory mapped and programmed
I/O by default, and the latter only on PC-compatible platforms
including those that have ISA/PCMCIA/PCI buses.

The series shrinks the 8250 driver by about 10% in both binary
and source code size, hopefully with no loss of functionality,
and it allows platforms to no longer define bogus inb/outb
functions when they don't provide CONFIG_HAS_IOPORT. I've
build-tested for x86 and ARM with and without HAS_IOPORT.

Hopefully Ralf can provide some feedback about the three
MIPS platforms that have code changed by this.

	Arnd

Arnd Bergmann (7):
  serial/8250: remove obsolete RM9000 port type
  serial/8250: move alchemy I/O handler to platform code
  serial/8250: move UPIO_TSI to powerpc
  serial/8250: move DWAP support to arch/mips
  serial/8250: remove obsolete and broken PORT_RSA support
  serial/8250: sanitize fourport handling
  serial/8250: make PIO support optional

 arch/mips/Kconfig                         |    7 -
 arch/mips/alchemy/common/platform.c       |   50 +++
 arch/mips/pmc-sierra/msp71xx/msp_serial.c |   32 ++-
 arch/powerpc/kernel/legacy_serial.c       |   24 ++
 drivers/tty/serial/8250.c                 |  472 +++--------------------------
 drivers/tty/serial/8250.h                 |    8 +
 drivers/tty/serial/8250_hub6.c            |   17 +
 drivers/tty/serial/Kconfig                |   15 -
 drivers/tty/serial/serial_core.c          |    4 -
 drivers/tty/serial/sunsu.c                |   93 ------
 include/linux/serial.h                    |    2 +-
 include/linux/serial_core.h               |   10 +-
 include/linux/serial_reg.h                |   51 ---
 13 files changed, 171 insertions(+), 614 deletions(-)

Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-serial@vger.kernel.org
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>

-- 
1.7.5.4

[PATCH 1/7] serial/8250: remove obsolete RM9000 port type

[Arnd Bergmann]

This code was only used on the MIPS eXcite platform, which got
removed in 2009. Since no code ever sets UPIO_RM9000, none of
the code is being used any more now.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Thomas Koeller <thomas.koeller@baslerweb.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: linux-serial@vger.kernel.org
---
 arch/mips/Kconfig           |    7 ----
 drivers/tty/serial/8250.c   |   68 +------------------------------------------
 drivers/tty/serial/Kconfig  |    9 ------
 include/linux/serial_core.h |    3 +-
 4 files changed, 2 insertions(+), 85 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 653da62..a07f7b8 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -1045,10 +1045,6 @@ config PCI_GT64XXX_PCI0
 config NO_EXCEPT_FILL
 	bool
 
-config MIPS_RM9122
-	bool
-	select SERIAL_RM9000
-
 config SOC_EMMA2RH
 	bool
 	select CEVT_R4K
@@ -1094,9 +1090,6 @@ config SOC_PNX8550
 config SWAP_IO_SPACE
 	bool
 
-config SERIAL_RM9000
-	bool
-
 config SGI_HAS_INDYDOG
 	bool
 
diff --git a/drivers/tty/serial/8250.c b/drivers/tty/serial/8250.c
index b40f7b9..cf19b26 100644
--- a/drivers/tty/serial/8250.c
+++ b/drivers/tty/serial/8250.c
@@ -273,13 +273,6 @@ static const struct serial8250_config uart_config[] = {
 		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
 		.flags		= UART_CAP_FIFO | UART_CAP_UUE | UART_CAP_RTOIE,
 	},
-	[PORT_RM9000] = {
-		.name		= "RM9000",
-		.fifo_size	= 16,
-		.tx_loadsz	= 16,
-		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
-		.flags		= UART_CAP_FIFO,
-	},
 	[PORT_OCTEON] = {
 		.name		= "OCTEON",
 		.fifo_size	= 64,
@@ -347,44 +340,6 @@ static inline int map_8250_out_reg(struct uart_port *p, int offset)
 	return au_io_out_map[offset];
 }
 
-#elif defined(CONFIG_SERIAL_8250_RM9K)
-
-static const u8
-	regmap_in[8] = {
-		[UART_RX]	= 0x00,
-		[UART_IER]	= 0x0c,
-		[UART_IIR]	= 0x14,
-		[UART_LCR]	= 0x1c,
-		[UART_MCR]	= 0x20,
-		[UART_LSR]	= 0x24,
-		[UART_MSR]	= 0x28,
-		[UART_SCR]	= 0x2c
-	},
-	regmap_out[8] = {
-		[UART_TX] 	= 0x04,
-		[UART_IER]	= 0x0c,
-		[UART_FCR]	= 0x18,
-		[UART_LCR]	= 0x1c,
-		[UART_MCR]	= 0x20,
-		[UART_LSR]	= 0x24,
-		[UART_MSR]	= 0x28,
-		[UART_SCR]	= 0x2c
-	};
-
-static inline int map_8250_in_reg(struct uart_port *p, int offset)
-{
-	if (p->iotype != UPIO_RM9000)
-		return offset;
-	return regmap_in[offset];
-}
-
-static inline int map_8250_out_reg(struct uart_port *p, int offset)
-{
-	if (p->iotype != UPIO_RM9000)
-		return offset;
-	return regmap_out[offset];
-}
-
 #else
 
 /* sane hardware needs no mapping */
@@ -524,7 +479,6 @@ static void set_io_from_upio(struct uart_port *p)
 		p->serial_out = mem_serial_out;
 		break;
 
-	case UPIO_RM9000:
 	case UPIO_MEM32:
 		p->serial_in = mem32_serial_in;
 		p->serial_out = mem32_serial_out;
@@ -620,24 +574,6 @@ static void serial_dl_write(struct uart_8250_port *up, int value)
 	else
 		_serial_dl_write(up, value);
 }
-#elif defined(CONFIG_SERIAL_8250_RM9K)
-static int serial_dl_read(struct uart_8250_port *up)
-{
-	return	(up->port.iotype == UPIO_RM9000) ?
-		(((__raw_readl(up->port.membase + 0x10) << 8) |
-		(__raw_readl(up->port.membase + 0x08) & 0xff)) & 0xffff) :
-		_serial_dl_read(up);
-}
-
-static void serial_dl_write(struct uart_8250_port *up, int value)
-{
-	if (up->port.iotype == UPIO_RM9000) {
-		__raw_writel(value, up->port.membase + 0x08);
-		__raw_writel(value >> 8, up->port.membase + 0x10);
-	} else {
-		_serial_dl_write(up, value);
-	}
-}
 #else
 #define serial_dl_read(up) _serial_dl_read(up)
 #define serial_dl_write(up, value) _serial_dl_write(up, value)
@@ -1394,9 +1330,7 @@ static void serial8250_start_tx(struct uart_port *port)
 			unsigned char lsr;
 			lsr = serial_in(up, UART_LSR);
 			up->lsr_saved_flags |= lsr & LSR_SAVE_FLAGS;
-			if ((up->port.type == PORT_RM9000) ?
-				(lsr & UART_LSR_THRE) :
-				(lsr & UART_LSR_TEMT))
+			if (lsr & UART_LSR_TEMT)
 				transmit_chars(up);
 		}
 	}
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 636144c..ea12815 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -258,15 +258,6 @@ 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_RM9K
-	bool "Support for MIPS RM9xxx integrated serial port"
-	depends on SERIAL_8250 != n && SERIAL_RM9000
-	select SERIAL_8250_SHARE_IRQ
-	help
-	  Selecting this option will add support for the integrated serial
-	  port hardware found on MIPS RM9122 and similar processors.
-	  If unsure, say N.
-
 comment "Non-8250 serial port support"
 
 config SERIAL_AMBA_PL010
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index a5c3114..df295e0 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -41,7 +41,7 @@
 #define PORT_RSA	13
 #define PORT_NS16550A	14
 #define PORT_XSCALE	15
-#define PORT_RM9000	16	/* PMC-Sierra RM9xxx internal UART */
+/* #define PORT_RM9000	16 */	/* was: PMC-Sierra RM9xxx internal UART */
 #define PORT_OCTEON	17	/* Cavium OCTEON internal UART */
 #define PORT_AR7	18	/* Texas Instruments AR7 internal UART */
 #define PORT_U6_16550A	19	/* ST-Ericsson U6xxx internal UART */
@@ -318,7 +318,6 @@ struct uart_port {
 #define UPIO_AU			(4)			/* Au1x00 type IO */
 #define UPIO_TSI		(5)			/* Tsi108/109 type IO */
 #define UPIO_DWAPB		(6)			/* DesignWare APB UART */
-#define UPIO_RM9000		(7)			/* RM9000 type IO */
 #define UPIO_DWAPB32		(8)			/* DesignWare APB UART (32 bit accesses) */
 
 	unsigned int		read_status_mask;	/* driver specific */
-- 
1.7.5.4

[PATCH 2/7] serial/8250: move alchemy I/O handler to platform code

[Arnd Bergmann]

Only one platform ever sets the UPIO_AU iotype, so it's
cleaner to define the handlers in the code that actually
requires it, rather than building the same logic into
every 8250 driver.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Manuel Lauss <manuel.lauss@googlemail.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: linux-serial@vger.kernel.org
---
 arch/mips/alchemy/common/platform.c |   50 ++++++++++++++++++++++++++++++
 drivers/tty/serial/8250.c           |   58 -----------------------------------
 2 files changed, 50 insertions(+), 58 deletions(-)

diff --git a/arch/mips/alchemy/common/platform.c b/arch/mips/alchemy/common/platform.c
index 3b2c18b..750441f 100644
--- a/arch/mips/alchemy/common/platform.c
+++ b/arch/mips/alchemy/common/platform.c
@@ -45,6 +45,54 @@ static void alchemy_8250_pm(struct uart_port *port, unsigned int state,
 #endif
 }
 
+/* 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,
+};
+
+/* sane hardware needs no mapping */
+static inline int map_8250_in_reg(struct uart_port *p, int offset)
+{
+	if (p->iotype != UPIO_AU)
+		return offset;
+	return au_io_in_map[offset];
+}
+
+static inline int map_8250_out_reg(struct uart_port *p, int offset)
+{
+	if (p->iotype != UPIO_AU)
+		return offset;
+	return au_io_out_map[offset];
+}
+
+/* sane hardware needs no mapping */
+static unsigned int au_serial_in(struct uart_port *p, int offset)
+{
+	offset = map_8250_in_reg(p, offset) << p->regshift;
+	return __raw_readl(p->membase + offset);
+}
+
+static void au_serial_out(struct uart_port *p, int offset, int value)
+{
+	offset = map_8250_out_reg(p, offset) << p->regshift;
+	__raw_writel(value, p->membase + offset);
+}
+
+
 #define PORT(_base, _irq)					\
 	{							\
 		.mapbase	= _base,			\
@@ -55,6 +103,8 @@ static void alchemy_8250_pm(struct uart_port *port, unsigned int state,
 				  UPF_FIXED_TYPE,		\
 		.type		= PORT_16550A,			\
 		.pm		= alchemy_8250_pm,		\
+		.serial_in	= au_serial_in,			\
+		.serial_out	= au_serial_out			\
 	}
 
 static struct plat_serial8250_port au1x00_uart_data[][4] __initdata = {
diff --git a/drivers/tty/serial/8250.c b/drivers/tty/serial/8250.c
index cf19b26..c8f107e 100644
--- a/drivers/tty/serial/8250.c
+++ b/drivers/tty/serial/8250.c
@@ -304,50 +304,9 @@ static const struct serial8250_config uart_config[] = {
 	},
 };
 
-#if defined(CONFIG_MIPS_ALCHEMY)
-
-/* 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,
-};
-
-/* sane hardware needs no mapping */
-static inline int map_8250_in_reg(struct uart_port *p, int offset)
-{
-	if (p->iotype != UPIO_AU)
-		return offset;
-	return au_io_in_map[offset];
-}
-
-static inline int map_8250_out_reg(struct uart_port *p, int offset)
-{
-	if (p->iotype != UPIO_AU)
-		return offset;
-	return au_io_out_map[offset];
-}
-
-#else
-
-/* sane hardware needs no mapping */
 #define map_8250_in_reg(up, offset) (offset)
 #define map_8250_out_reg(up, offset) (offset)
 
-#endif
-
 static unsigned int hub6_serial_in(struct uart_port *p, int offset)
 {
 	offset = map_8250_in_reg(p, offset) << p->regshift;
@@ -386,18 +345,6 @@ static unsigned int mem32_serial_in(struct uart_port *p, int offset)
 	return readl(p->membase + offset);
 }
 
-static unsigned int au_serial_in(struct uart_port *p, int offset)
-{
-	offset = map_8250_in_reg(p, offset) << p->regshift;
-	return __raw_readl(p->membase + offset);
-}
-
-static void au_serial_out(struct uart_port *p, int offset, int value)
-{
-	offset = map_8250_out_reg(p, offset) << p->regshift;
-	__raw_writel(value, p->membase + offset);
-}
-
 static unsigned int tsi_serial_in(struct uart_port *p, int offset)
 {
 	unsigned int tmp;
@@ -484,11 +431,6 @@ static void set_io_from_upio(struct uart_port *p)
 		p->serial_out = mem32_serial_out;
 		break;
 
-	case UPIO_AU:
-		p->serial_in = au_serial_in;
-		p->serial_out = au_serial_out;
-		break;
-
 	case UPIO_TSI:
 		p->serial_in = tsi_serial_in;
 		p->serial_out = tsi_serial_out;
-- 
1.7.5.4

Re: [PATCH 2/7] serial/8250: move alchemy I/O handler to platform code

[Manuel Lauss]

Hi Arnd,

On Mon, Jun 27, 2011 at 11:45 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> Only one platform ever sets the UPIO_AU iotype, so it's
> cleaner to define the handlers in the code that actually
> requires it, rather than building the same logic into
> every 8250 driver.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Manuel Lauss <manuel.lauss@googlemail.com>
> Cc: Greg Kroah-Hartman <gregkh@suse.de>
> Cc: linux-serial@vger.kernel.org
> ---
>  arch/mips/alchemy/common/platform.c |   50 ++++++++++++++++++++++++++++++
>  drivers/tty/serial/8250.c           |   58 -----------------------------------
>  2 files changed, 50 insertions(+), 58 deletions(-)

This breaks serial console on all my Alchemy test system.  I'll try to
figure out why.

Manuel
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/7] serial/8250: move alchemy I/O handler to platform code

[Manuel Lauss]

On Mon, Jun 27, 2011 at 11:45 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> Only one platform ever sets the UPIO_AU iotype, so it's
> cleaner to define the handlers in the code that actually
> requires it, rather than building the same logic into
> every 8250 driver.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Manuel Lauss <manuel.lauss@googlemail.com>
> Cc: Greg Kroah-Hartman <gregkh@suse.de>
> Cc: linux-serial@vger.kernel.org
> ---
>  arch/mips/alchemy/common/platform.c |   50 ++++++++++++++++++++++++++++++
>  drivers/tty/serial/8250.c           |   58 -----------------------------------
>  2 files changed, 50 insertions(+), 58 deletions(-)
>
> diff --git a/arch/mips/alchemy/common/platform.c b/arch/mips/alchemy/common/platform.c
> index 3b2c18b..750441f 100644
> --- a/arch/mips/alchemy/common/platform.c
> +++ b/arch/mips/alchemy/common/platform.c
[...]
> @@ -55,6 +103,8 @@ static void alchemy_8250_pm(struct uart_port *port, unsigned int state,
>                                  UPF_FIXED_TYPE,               \
>                .type           = PORT_16550A,                  \
>                .pm             = alchemy_8250_pm,              \
> +               .serial_in      = au_serial_in,                 \
> +               .serial_out     = au_serial_out                 \
>        }

This is very strange:  Just this part alone (leaving 8250.c intact)
screws everything up.
The assembly for au_serial_in/out is identical in both 8250.o and
arch/mips/alchemy/common/platform.o (renamed the functions here obviously)
I have no idea what's wrong...

Manuel
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/7] serial/8250: move alchemy I/O handler to platform code

[Manuel Lauss]

On Tue, Jun 28, 2011 at 1:29 PM, Manuel Lauss
<manuel.lauss@googlemail.com> wrote:
> On Mon, Jun 27, 2011 at 11:45 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> Only one platform ever sets the UPIO_AU iotype, so it's
>> cleaner to define the handlers in the code that actually
>> requires it, rather than building the same logic into
>> every 8250 driver.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> Cc: Ralf Baechle <ralf@linux-mips.org>
>> Cc: Manuel Lauss <manuel.lauss@googlemail.com>
>> Cc: Greg Kroah-Hartman <gregkh@suse.de>
>> Cc: linux-serial@vger.kernel.org
>> ---
>>  arch/mips/alchemy/common/platform.c |   50 ++++++++++++++++++++++++++++++
>>  drivers/tty/serial/8250.c           |   58 -----------------------------------
>>  2 files changed, 50 insertions(+), 58 deletions(-)
>>
>> diff --git a/arch/mips/alchemy/common/platform.c b/arch/mips/alchemy/common/platform.c
>> index 3b2c18b..750441f 100644
>> --- a/arch/mips/alchemy/common/platform.c
>> +++ b/arch/mips/alchemy/common/platform.c
> [...]
>> @@ -55,6 +103,8 @@ static void alchemy_8250_pm(struct uart_port *port, unsigned int state,
>>                                  UPF_FIXED_TYPE,               \
>>                .type           = PORT_16550A,                  \
>>                .pm             = alchemy_8250_pm,              \
>> +               .serial_in      = au_serial_in,                 \
>> +               .serial_out     = au_serial_out                 \
>>        }
>
> This is very strange:  Just this part alone (leaving 8250.c intact)
> screws everything up.
> The assembly for au_serial_in/out is identical in both 8250.o and
> arch/mips/alchemy/common/platform.o (renamed the functions here obviously)
> I have no idea what's wrong...


Okay, found it.  the Alchemy headers have definitions for UART_RX/TX/... with
different values; they can be deleted safely since nothing depends on them.
Here's an updated and tested patch.  I took the liberty to rename the functions
a bit while at it.

diff --git a/arch/mips/alchemy/common/platform.c
b/arch/mips/alchemy/common/platform.c
index 932f697..301ccb7 100644
--- a/arch/mips/alchemy/common/platform.c
+++ b/arch/mips/alchemy/common/platform.c
@@ -16,6 +16,7 @@
 #include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/serial_8250.h>
+#include <linux/serial_reg.h>
 #include <linux/slab.h>

 #include <asm/mach-au1x00/au1xxx.h>
@@ -45,6 +46,54 @@ static void alchemy_8250_pm(struct uart_port *port,
unsigned int state,
 #endif
 }

+/* Au1x00 UART hardware has a weird register layout */
+static const u8 alchemy_8250_inmap[] = {
+	[UART_RX]  = 0,
+	[UART_IER] = 2,
+	[UART_IIR] = 3,
+	[UART_LCR] = 5,
+	[UART_MCR] = 6,
+	[UART_LSR] = 7,
+	[UART_MSR] = 8,
+};
+
+static const u8 alchemy_8250_outmap[] = {
+	[UART_TX]  = 1,
+	[UART_IER] = 2,
+	[UART_FCR] = 4,
+	[UART_LCR] = 5,
+	[UART_MCR] = 6,
+};
+
+/* sane hardware needs no mapping */
+static inline int map_8250_in_reg(struct uart_port *p, int offset)
+{
+	if (p->iotype != UPIO_AU)
+		return offset;
+	return alchemy_8250_inmap[offset];
+}
+
+static inline int map_8250_out_reg(struct uart_port *p, int offset)
+{
+	if (p->iotype != UPIO_AU)
+		return offset;
+	return alchemy_8250_outmap[offset];
+}
+
+/* sane hardware needs no mapping */
+static unsigned int alchemy_8250_in(struct uart_port *p, int offset)
+{
+	offset = map_8250_in_reg(p, offset) << p->regshift;
+	return __raw_readl(p->membase + offset);
+}
+
+static void alchemy_8250_out(struct uart_port *p, int offset, int value)
+{
+	offset = map_8250_out_reg(p, offset) << p->regshift;
+	__raw_writel(value, p->membase + offset);
+}
+
+
 #define PORT(_base, _irq)					\
 	{							\
 		.mapbase	= _base,			\
@@ -55,6 +104,8 @@ static void alchemy_8250_pm(struct uart_port *port,
unsigned int state,
 				  UPF_FIXED_TYPE,		\
 		.type		= PORT_16550A,			\
 		.pm		= alchemy_8250_pm,		\
+		.serial_in	= alchemy_8250_in,		\
+		.serial_out	= alchemy_8250_out,		\
 	}

 static struct plat_serial8250_port au1x00_uart_data[][4] __initdata = {
diff --git a/arch/mips/include/asm/mach-au1x00/au1000.h
b/arch/mips/include/asm/mach-au1x00/au1000.h
index 610cc06..666de8e 100644
--- a/arch/mips/include/asm/mach-au1x00/au1000.h
+++ b/arch/mips/include/asm/mach-au1x00/au1000.h
@@ -1172,18 +1172,6 @@ enum soc_au1200_ints {
 #define MAC_RX_BUFF3_STATUS	0x30
 #define MAC_RX_BUFF3_ADDR	0x34

-#define UART_RX		0	/* Receive buffer */
-#define UART_TX		4	/* Transmit buffer */
-#define UART_IER	8	/* Interrupt Enable Register */
-#define UART_IIR	0xC	/* Interrupt ID Register */
-#define UART_FCR	0x10	/* FIFO Control Register */
-#define UART_LCR	0x14	/* Line Control Register */
-#define UART_MCR	0x18	/* Modem Control Register */
-#define UART_LSR	0x1C	/* Line Status Register */
-#define UART_MSR	0x20	/* Modem Status Register */
-#define UART_CLK	0x28	/* Baud Rate Clock Divider */
-#define UART_MOD_CNTRL	0x100	/* Module Control */
-
 /* SSIO */
 #define SSI0_STATUS		0xB1600000
 #  define SSI_STATUS_BF 	(1 << 4)
diff --git a/drivers/tty/serial/8250.c b/drivers/tty/serial/8250.c
index cf19b26..c8f107e 100644
--- a/drivers/tty/serial/8250.c
+++ b/drivers/tty/serial/8250.c
@@ -304,50 +304,9 @@ static const struct serial8250_config uart_config[] = {
 	},
 };

-#if defined(CONFIG_MIPS_ALCHEMY)
-
-/* 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,
-};
-
-/* sane hardware needs no mapping */
-static inline int map_8250_in_reg(struct uart_port *p, int offset)
-{
-	if (p->iotype != UPIO_AU)
-		return offset;
-	return au_io_in_map[offset];
-}
-
-static inline int map_8250_out_reg(struct uart_port *p, int offset)
-{
-	if (p->iotype != UPIO_AU)
-		return offset;
-	return au_io_out_map[offset];
-}
-
-#else
-
-/* sane hardware needs no mapping */
 #define map_8250_in_reg(up, offset) (offset)
 #define map_8250_out_reg(up, offset) (offset)

-#endif
-
 static unsigned int hub6_serial_in(struct uart_port *p, int offset)
 {
 	offset = map_8250_in_reg(p, offset) << p->regshift;
@@ -386,18 +345,6 @@ static unsigned int mem32_serial_in(struct
uart_port *p, int offset)
 	return readl(p->membase + offset);
 }

-static unsigned int au_serial_in(struct uart_port *p, int offset)
-{
-	offset = map_8250_in_reg(p, offset) << p->regshift;
-	return __raw_readl(p->membase + offset);
-}
-
-static void au_serial_out(struct uart_port *p, int offset, int value)
-{
-	offset = map_8250_out_reg(p, offset) << p->regshift;
-	__raw_writel(value, p->membase + offset);
-}
-
 static unsigned int tsi_serial_in(struct uart_port *p, int offset)
 {
 	unsigned int tmp;
@@ -484,11 +431,6 @@ static void set_io_from_upio(struct uart_port *p)
 		p->serial_out = mem32_serial_out;
 		break;

-	case UPIO_AU:
-		p->serial_in = au_serial_in;
-		p->serial_out = au_serial_out;
-		break;
-
 	case UPIO_TSI:
 		p->serial_in = tsi_serial_in;
 		p->serial_out = tsi_serial_out;
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/7] serial/8250: move alchemy I/O handler to platform code

[Arnd Bergmann]

On Tuesday 28 June 2011, Manuel Lauss wrote:
> Okay, found it.  the Alchemy headers have definitions for UART_RX/TX/... with
> different values; they can be deleted safely since nothing depends on them.
> Here's an updated and tested patch.  I took the liberty to rename the functions
> a bit while at it.

Cool, thanks so much for tracking that down!

Please send me (in private) the Signed-off-by that you want me to add to the
patch, apparently you missed that.

I'll put your version in the series then.

	Arnd

[PATCH 3/7] serial/8250: move UPIO_TSI to powerpc

[Arnd Bergmann]

This iotype is only used by the legacy_serial code in powerpc, so the
code should live there, rather than be compiled in for every 8250
driver.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: linux-serial@vger.kernel.org
---
 arch/powerpc/kernel/legacy_serial.c |   24 ++++++++++++++++++++++++
 drivers/tty/serial/8250.c           |   23 -----------------------
 2 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/arch/powerpc/kernel/legacy_serial.c b/arch/powerpc/kernel/legacy_serial.c
index 2b97b80..b229e1e 100644
--- a/arch/powerpc/kernel/legacy_serial.c
+++ b/arch/powerpc/kernel/legacy_serial.c
@@ -47,6 +47,24 @@ static struct __initdata of_device_id legacy_serial_parents[] = {
 static unsigned int legacy_serial_count;
 static int legacy_serial_console = -1;
 
+static unsigned int tsi_serial_in(struct uart_port *p, int offset)
+{
+	unsigned int tmp;
+	offset = offset << p->regshift;
+	if (offset == UART_IIR) {
+		tmp = readl(p->membase + (UART_IIR & ~3));
+		return (tmp >> 16) & 0xff; /* UART_IIR % 4 == 2 */
+	} else
+		return readb(p->membase + offset);
+}
+
+static void tsi_serial_out(struct uart_port *p, int offset, int value)
+{
+	offset = offset << p->regshift;
+	if (!((offset == UART_IER) && (value & UART_IER_UUE)))
+		writeb(value, p->membase + offset);
+}
+
 static int __init add_legacy_port(struct device_node *np, int want_index,
 				  int iotype, phys_addr_t base,
 				  phys_addr_t taddr, unsigned long irq,
@@ -102,6 +120,7 @@ static int __init add_legacy_port(struct device_node *np, int want_index,
 		legacy_serial_ports[index].iobase = base;
 	else
 		legacy_serial_ports[index].mapbase = base;
+
 	legacy_serial_ports[index].iotype = iotype;
 	legacy_serial_ports[index].uartclk = clock;
 	legacy_serial_ports[index].irq = irq;
@@ -112,6 +131,11 @@ static int __init add_legacy_port(struct device_node *np, int want_index,
 	legacy_serial_infos[index].speed = spd ? be32_to_cpup(spd) : 0;
 	legacy_serial_infos[index].irq_check_parent = irq_check_parent;
 
+	if (iotype == UPIO_TSI) {
+		legacy_serial_ports[index].serial_in = tsi_serial_in;
+		legacy_serial_ports[index].serial_out = tsi_serial_out;
+	}
+
 	printk(KERN_DEBUG "Found legacy serial port %d for %s\n",
 	       index, np->full_name);
 	printk(KERN_DEBUG "  %s=%llx, taddr=%llx, irq=%lx, clk=%d, speed=%d\n",
diff --git a/drivers/tty/serial/8250.c b/drivers/tty/serial/8250.c
index c8f107e..d575ccb 100644
--- a/drivers/tty/serial/8250.c
+++ b/drivers/tty/serial/8250.c
@@ -345,24 +345,6 @@ static unsigned int mem32_serial_in(struct uart_port *p, int offset)
 	return readl(p->membase + offset);
 }
 
-static unsigned int tsi_serial_in(struct uart_port *p, int offset)
-{
-	unsigned int tmp;
-	offset = map_8250_in_reg(p, offset) << p->regshift;
-	if (offset == UART_IIR) {
-		tmp = readl(p->membase + (UART_IIR & ~3));
-		return (tmp >> 16) & 0xff; /* UART_IIR % 4 == 2 */
-	} else
-		return readb(p->membase + offset);
-}
-
-static void tsi_serial_out(struct uart_port *p, int offset, int value)
-{
-	offset = map_8250_out_reg(p, offset) << p->regshift;
-	if (!((offset == UART_IER) && (value & UART_IER_UUE)))
-		writeb(value, p->membase + offset);
-}
-
 /* Save the LCR value so it can be re-written when a Busy Detect IRQ occurs. */
 static inline void dwapb_save_out_value(struct uart_port *p, int offset,
 					int value)
@@ -431,11 +413,6 @@ static void set_io_from_upio(struct uart_port *p)
 		p->serial_out = mem32_serial_out;
 		break;
 
-	case UPIO_TSI:
-		p->serial_in = tsi_serial_in;
-		p->serial_out = tsi_serial_out;
-		break;
-
 	case UPIO_DWAPB:
 		p->serial_in = mem_serial_in;
 		p->serial_out = dwapb_serial_out;
-- 
1.7.5.4

Re: [PATCH 3/7] serial/8250: move UPIO_TSI to powerpc

[David Daney]

On 06/27/2011 02:45 PM, Arnd Bergmann wrote:
> This iotype is only used by the legacy_serial code in powerpc, so the
> code should live there, rather than be compiled in for every 8250
> driver.
>
> Signed-off-by: Arnd Bergmann<arnd@arndb.de>
> Cc: Benjamin Herrenschmidt<benh@kernel.crashing.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: Greg Kroah-Hartman<gregkh@suse.de>
> Cc: linux-serial@vger.kernel.org
> ---
>   arch/powerpc/kernel/legacy_serial.c |   24 ++++++++++++++++++++++++
>   drivers/tty/serial/8250.c           |   23 -----------------------
>   2 files changed, 24 insertions(+), 23 deletions(-)
>

This seems vaguely familiar:

https://lkml.org/lkml/2008/10/6/297

So just for the hell of it...

Acked-by: David Daney <david.daney@cavium.com>

Re: [PATCH 3/7] serial/8250: move UPIO_TSI to powerpc

[Benjamin Herrenschmidt]

On Mon, 2011-06-27 at 23:45 +0200, Arnd Bergmann wrote:
> This iotype is only used by the legacy_serial code in powerpc, so the
> code should live there, rather than be compiled in for every 8250
> driver.

Obviously untested :-)

Misses the definition of UART_IIR etc...

I'm committing a fixed version in my tree

Cheers.
Ben.

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: Greg Kroah-Hartman <gregkh@suse.de>
> Cc: linux-serial@vger.kernel.org
> ---
>  arch/powerpc/kernel/legacy_serial.c |   24 ++++++++++++++++++++++++
>  drivers/tty/serial/8250.c           |   23 -----------------------
>  2 files changed, 24 insertions(+), 23 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/legacy_serial.c b/arch/powerpc/kernel/legacy_serial.c
> index 2b97b80..b229e1e 100644
> --- a/arch/powerpc/kernel/legacy_serial.c
> +++ b/arch/powerpc/kernel/legacy_serial.c
> @@ -47,6 +47,24 @@ static struct __initdata of_device_id legacy_serial_parents[] = {
>  static unsigned int legacy_serial_count;
>  static int legacy_serial_console = -1;
>  
> +static unsigned int tsi_serial_in(struct uart_port *p, int offset)
> +{
> +	unsigned int tmp;
> +	offset = offset << p->regshift;
> +	if (offset == UART_IIR) {
> +		tmp = readl(p->membase + (UART_IIR & ~3));
> +		return (tmp >> 16) & 0xff; /* UART_IIR % 4 == 2 */
> +	} else
> +		return readb(p->membase + offset);
> +}
> +
> +static void tsi_serial_out(struct uart_port *p, int offset, int value)
> +{
> +	offset = offset << p->regshift;
> +	if (!((offset == UART_IER) && (value & UART_IER_UUE)))
> +		writeb(value, p->membase + offset);
> +}
> +
>  static int __init add_legacy_port(struct device_node *np, int want_index,
>  				  int iotype, phys_addr_t base,
>  				  phys_addr_t taddr, unsigned long irq,
> @@ -102,6 +120,7 @@ static int __init add_legacy_port(struct device_node *np, int want_index,
>  		legacy_serial_ports[index].iobase = base;
>  	else
>  		legacy_serial_ports[index].mapbase = base;
> +
>  	legacy_serial_ports[index].iotype = iotype;
>  	legacy_serial_ports[index].uartclk = clock;
>  	legacy_serial_ports[index].irq = irq;
> @@ -112,6 +131,11 @@ static int __init add_legacy_port(struct device_node *np, int want_index,
>  	legacy_serial_infos[index].speed = spd ? be32_to_cpup(spd) : 0;
>  	legacy_serial_infos[index].irq_check_parent = irq_check_parent;
>  
> +	if (iotype == UPIO_TSI) {
> +		legacy_serial_ports[index].serial_in = tsi_serial_in;
> +		legacy_serial_ports[index].serial_out = tsi_serial_out;
> +	}
> +
>  	printk(KERN_DEBUG "Found legacy serial port %d for %s\n",
>  	       index, np->full_name);
>  	printk(KERN_DEBUG "  %s=%llx, taddr=%llx, irq=%lx, clk=%d, speed=%d\n",
> diff --git a/drivers/tty/serial/8250.c b/drivers/tty/serial/8250.c
> index c8f107e..d575ccb 100644
> --- a/drivers/tty/serial/8250.c
> +++ b/drivers/tty/serial/8250.c
> @@ -345,24 +345,6 @@ static unsigned int mem32_serial_in(struct uart_port *p, int offset)
>  	return readl(p->membase + offset);
>  }
>  
> -static unsigned int tsi_serial_in(struct uart_port *p, int offset)
> -{
> -	unsigned int tmp;
> -	offset = map_8250_in_reg(p, offset) << p->regshift;
> -	if (offset == UART_IIR) {
> -		tmp = readl(p->membase + (UART_IIR & ~3));
> -		return (tmp >> 16) & 0xff; /* UART_IIR % 4 == 2 */
> -	} else
> -		return readb(p->membase + offset);
> -}
> -
> -static void tsi_serial_out(struct uart_port *p, int offset, int value)
> -{
> -	offset = map_8250_out_reg(p, offset) << p->regshift;
> -	if (!((offset == UART_IER) && (value & UART_IER_UUE)))
> -		writeb(value, p->membase + offset);
> -}
> -
>  /* Save the LCR value so it can be re-written when a Busy Detect IRQ occurs. */
>  static inline void dwapb_save_out_value(struct uart_port *p, int offset,
>  					int value)
> @@ -431,11 +413,6 @@ static void set_io_from_upio(struct uart_port *p)
>  		p->serial_out = mem32_serial_out;
>  		break;
>  
> -	case UPIO_TSI:
> -		p->serial_in = tsi_serial_in;
> -		p->serial_out = tsi_serial_out;
> -		break;
> -
>  	case UPIO_DWAPB:
>  		p->serial_in = mem_serial_in;
>  		p->serial_out = dwapb_serial_out;

Re: [PATCH 3/7] serial/8250: move UPIO_TSI to powerpc

[Arnd Bergmann]

On Thursday 01 September 2011 16:02:06 Benjamin Herrenschmidt wrote:
> On Mon, 2011-06-27 at 23:45 +0200, Arnd Bergmann wrote:
> > This iotype is only used by the legacy_serial code in powerpc, so the
> > code should live there, rather than be compiled in for every 8250
> > driver.
> 
> Obviously untested 
> 
> Misses the definition of UART_IIR etc...
> 
> I'm committing a fixed version in my tree

Ok, thank you!

	Arnd

[PATCH 4/7] serial/8250: move DWAP support to arch/mips

[Arnd Bergmann]

Only one board uses the UPIO_DWAP iotype, and nothing uses
the UPIO_DWAP32 iotype. It seems much cleaner to handle the
DWAP quirk as a new port type, and move the serial_out
accessor into the platform code. If more platforms start
using the DWAP port, that function can be moved into a
common location.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Jamie Iles <jamie@jamieiles.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: linux-serial@vger.kernel.org
---
 arch/mips/pmc-sierra/msp71xx/msp_serial.c |   32 ++++++++++-
 drivers/tty/serial/8250.c                 |   87 +++--------------------------
 drivers/tty/serial/8250_hub6.c            |   17 ++++++
 drivers/tty/serial/serial_core.c          |    4 -
 include/linux/serial_core.h               |    5 +-
 5 files changed, 57 insertions(+), 88 deletions(-)

diff --git a/arch/mips/pmc-sierra/msp71xx/msp_serial.c b/arch/mips/pmc-sierra/msp71xx/msp_serial.c
index f726162..6c9f1d3 100644
--- a/arch/mips/pmc-sierra/msp71xx/msp_serial.c
+++ b/arch/mips/pmc-sierra/msp71xx/msp_serial.c
@@ -38,6 +38,33 @@
 #include <msp_int.h>
 #include <msp_regs.h>
 
+/* Save the LCR value so it can be re-written when a Busy Detect IRQ occurs. */
+static inline void dwapb_save_out_value(struct uart_port *p, int offset,
+					int value)
+{
+	struct uart_8250_port *up =
+		container_of(p, struct uart_8250_port, port);
+
+	if (offset == UART_LCR)
+		up->lcr = value;
+}
+
+/* Read the IER to ensure any interrupt is cleared before returning from ISR. */
+static inline void dwapb_check_clear_ier(struct uart_port *p, int offset)
+{
+	if (offset == UART_TX || offset == UART_IER)
+		p->serial_in(p, UART_IER);
+}
+
+static void dwapb_serial_out(struct uart_port *p, int offset, int value)
+{
+	int save_offset = offset;
+	offset = map_8250_out_reg(p, offset) << p->regshift;
+	dwapb_save_out_value(p, save_offset, value);
+	writeb(value, p->membase + offset);
+	dwapb_check_clear_ier(p, save_offset);
+}
+
 void __init msp_serial_setup(void)
 {
 	char    *s;
@@ -59,9 +86,10 @@ void __init msp_serial_setup(void)
 	up.irq          = MSP_INT_UART0;
 	up.uartclk      = uartclk;
 	up.regshift     = 2;
-	up.iotype       = UPIO_DWAPB; /* UPIO_MEM like */
+	up.iotype       = UPIO_MEM;
+	up.serial_out	= dwapb_serial_out;
 	up.flags        = ASYNC_BOOT_AUTOCONF | ASYNC_SKIP_TEST;
-	up.type         = PORT_16550A;
+	up.type         = PORT_DWAP;
 	up.line         = 0;
 	up.private_data		= (void*)UART0_STATUS_REG;
 	if (early_serial_setup(&up))
diff --git a/drivers/tty/serial/8250.c b/drivers/tty/serial/8250.c
index d575ccb..5698157 100644
--- a/drivers/tty/serial/8250.c
+++ b/drivers/tty/serial/8250.c
@@ -100,12 +100,6 @@ static unsigned int skip_txen_test; /* force skip of txen test at init time */
 #define CONFIG_SERIAL_MANY_PORTS 1
 #endif
 
-/*
- * HUB6 is always on.  This will be removed once the header
- * files have been cleaned.
- */
-#define CONFIG_HUB6 1
-
 #include <asm/serial.h>
 /*
  * SERIAL_PORT_DFNS tells us about built-in ports that have no
@@ -302,25 +296,18 @@ static const struct serial8250_config uart_config[] = {
 				  UART_FCR_T_TRIG_01,
 		.flags		= UART_CAP_FIFO | UART_CAP_RTOIE,
 	},
+	[PORT_DWAP] = {
+		.name		= "16550A",
+		.fifo_size	= 16,
+		.tx_loadsz	= 16,
+		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
+		.flags		= UART_CAP_FIFO,
+	},
 };
 
 #define map_8250_in_reg(up, offset) (offset)
 #define map_8250_out_reg(up, offset) (offset)
 
-static unsigned int hub6_serial_in(struct uart_port *p, int offset)
-{
-	offset = map_8250_in_reg(p, offset) << p->regshift;
-	outb(p->hub6 - 1 + offset, p->iobase);
-	return inb(p->iobase + 1);
-}
-
-static void hub6_serial_out(struct uart_port *p, int offset, int value)
-{
-	offset = map_8250_out_reg(p, offset) << p->regshift;
-	outb(p->hub6 - 1 + offset, p->iobase);
-	outb(value, p->iobase + 1);
-}
-
 static unsigned int mem_serial_in(struct uart_port *p, int offset)
 {
 	offset = map_8250_in_reg(p, offset) << p->regshift;
@@ -345,42 +332,6 @@ static unsigned int mem32_serial_in(struct uart_port *p, int offset)
 	return readl(p->membase + offset);
 }
 
-/* Save the LCR value so it can be re-written when a Busy Detect IRQ occurs. */
-static inline void dwapb_save_out_value(struct uart_port *p, int offset,
-					int value)
-{
-	struct uart_8250_port *up =
-		container_of(p, struct uart_8250_port, port);
-
-	if (offset == UART_LCR)
-		up->lcr = value;
-}
-
-/* Read the IER to ensure any interrupt is cleared before returning from ISR. */
-static inline void dwapb_check_clear_ier(struct uart_port *p, int offset)
-{
-	if (offset == UART_TX || offset == UART_IER)
-		p->serial_in(p, UART_IER);
-}
-
-static void dwapb_serial_out(struct uart_port *p, int offset, int value)
-{
-	int save_offset = offset;
-	offset = map_8250_out_reg(p, offset) << p->regshift;
-	dwapb_save_out_value(p, save_offset, value);
-	writeb(value, p->membase + offset);
-	dwapb_check_clear_ier(p, save_offset);
-}
-
-static void dwapb32_serial_out(struct uart_port *p, int offset, int value)
-{
-	int save_offset = offset;
-	offset = map_8250_out_reg(p, offset) << p->regshift;
-	dwapb_save_out_value(p, save_offset, value);
-	writel(value, p->membase + offset);
-	dwapb_check_clear_ier(p, save_offset);
-}
-
 static unsigned int io_serial_in(struct uart_port *p, int offset)
 {
 	offset = map_8250_in_reg(p, offset) << p->regshift;
@@ -398,11 +349,6 @@ static void set_io_from_upio(struct uart_port *p)
 	struct uart_8250_port *up =
 		container_of(p, struct uart_8250_port, port);
 	switch (p->iotype) {
-	case UPIO_HUB6:
-		p->serial_in = hub6_serial_in;
-		p->serial_out = hub6_serial_out;
-		break;
-
 	case UPIO_MEM:
 		p->serial_in = mem_serial_in;
 		p->serial_out = mem_serial_out;
@@ -413,16 +359,6 @@ static void set_io_from_upio(struct uart_port *p)
 		p->serial_out = mem32_serial_out;
 		break;
 
-	case UPIO_DWAPB:
-		p->serial_in = mem_serial_in;
-		p->serial_out = dwapb_serial_out;
-		break;
-
-	case UPIO_DWAPB32:
-		p->serial_in = mem32_serial_in;
-		p->serial_out = dwapb32_serial_out;
-		break;
-
 	default:
 		p->serial_in = io_serial_in;
 		p->serial_out = io_serial_out;
@@ -440,8 +376,6 @@ serial_out_sync(struct uart_8250_port *up, int offset, int value)
 	case UPIO_MEM:
 	case UPIO_MEM32:
 	case UPIO_AU:
-	case UPIO_DWAPB:
-	case UPIO_DWAPB32:
 		p->serial_out(p, offset, value);
 		p->serial_in(p, UART_LCR);	/* safe, no side-effects */
 		break;
@@ -1512,8 +1446,7 @@ static irqreturn_t serial8250_interrupt(int irq, void *dev_id)
 			handled = 1;
 
 			end = NULL;
-		} else if ((up->port.iotype == UPIO_DWAPB ||
-			    up->port.iotype == UPIO_DWAPB32) &&
+		} else if ((up->port.type == PORT_DWAP) &&
 			  (iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
 			/* The DesignWare APB UART has an Busy Detect (0x07)
 			 * interrupt meaning an LCR write attempt occurred while the
@@ -2421,8 +2354,6 @@ static int serial8250_request_std_resource(struct uart_8250_port *up)
 	case UPIO_TSI:
 	case UPIO_MEM32:
 	case UPIO_MEM:
-	case UPIO_DWAPB:
-	case UPIO_DWAPB32:
 		if (!up->port.mapbase)
 			break;
 
@@ -2459,8 +2390,6 @@ static void serial8250_release_std_resource(struct uart_8250_port *up)
 	case UPIO_TSI:
 	case UPIO_MEM32:
 	case UPIO_MEM:
-	case UPIO_DWAPB:
-	case UPIO_DWAPB32:
 		if (!up->port.mapbase)
 			break;
 
diff --git a/drivers/tty/serial/8250_hub6.c b/drivers/tty/serial/8250_hub6.c
index a5c778e..fd1361e 100644
--- a/drivers/tty/serial/8250_hub6.c
+++ b/drivers/tty/serial/8250_hub6.c
@@ -9,6 +9,21 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/serial_8250.h>
+#include <linux/io.h>
+
+static unsigned int hub6_serial_in(struct uart_port *p, int offset)
+{
+	offset = offset << p->regshift;
+	outb(p->hub6 - 1 + offset, p->iobase);
+	return inb(p->iobase + 1);
+}
+
+static void hub6_serial_out(struct uart_port *p, int offset, int value)
+{
+	offset = offset << p->regshift;
+	outb(p->hub6 - 1 + offset, p->iobase);
+	outb(value, p->iobase + 1);
+}
 
 #define HUB6(card,port)							\
 	{								\
@@ -18,6 +33,8 @@
 		.iotype		= UPIO_HUB6,				\
 		.flags		= UPF_BOOT_AUTOCONF,			\
 		.hub6		= (card) << 6 | (port) << 3 | 1,	\
+		.serial_in	= hub6_serial_in,			\
+		.serial_out	= hub6_serial_out,			\
 	}
 
 static struct plat_serial8250_port hub6_data[] = {
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index db7912c..e5a0cae 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2063,8 +2063,6 @@ uart_report_port(struct uart_driver *drv, struct uart_port *port)
 	case UPIO_MEM32:
 	case UPIO_AU:
 	case UPIO_TSI:
-	case UPIO_DWAPB:
-	case UPIO_DWAPB32:
 		snprintf(address, sizeof(address),
 			 "MMIO 0x%llx", (unsigned long long)port->mapbase);
 		break;
@@ -2484,8 +2482,6 @@ int uart_match_port(struct uart_port *port1, struct uart_port *port2)
 	case UPIO_MEM32:
 	case UPIO_AU:
 	case UPIO_TSI:
-	case UPIO_DWAPB:
-	case UPIO_DWAPB32:
 		return (port1->mapbase == port2->mapbase);
 	}
 	return 0;
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index df295e0..598992f 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -46,7 +46,8 @@
 #define PORT_AR7	18	/* Texas Instruments AR7 internal UART */
 #define PORT_U6_16550A	19	/* ST-Ericsson U6xxx internal UART */
 #define PORT_TEGRA	20	/* NVIDIA Tegra internal UART */
-#define PORT_MAX_8250	20	/* max port ID */
+#define PORT_DWAP	21	/* Designware */
+#define PORT_MAX_8250	21	/* max port ID */
 
 /*
  * ARM specific type numbers.  These are not currently guaranteed
@@ -317,8 +318,6 @@ struct uart_port {
 #define UPIO_MEM32		(3)
 #define UPIO_AU			(4)			/* Au1x00 type IO */
 #define UPIO_TSI		(5)			/* Tsi108/109 type IO */
-#define UPIO_DWAPB		(6)			/* DesignWare APB UART */
-#define UPIO_DWAPB32		(8)			/* DesignWare APB UART (32 bit accesses) */
 
 	unsigned int		read_status_mask;	/* driver specific */
 	unsigned int		ignore_status_mask;	/* driver specific */
-- 
1.7.5.4

Re: [PATCH 4/7] serial/8250: move DWAP support to arch/mips

[Jamie Iles]

Hi Arnd,

On Mon, Jun 27, 2011 at 11:45:17PM +0200, Arnd Bergmann wrote:
> Only one board uses the UPIO_DWAP iotype, and nothing uses
> the UPIO_DWAP32 iotype. It seems much cleaner to handle the
> DWAP quirk as a new port type, and move the serial_out
> accessor into the platform code. If more platforms start
> using the DWAP port, that function can be moved into a
> common location.

I posted a series[1] a couple of weeks back that does this for 
DWAPB/DWAPB32 in a slightly different way which also removes the extra 
interrupt handling into the platform code.

Incidentally, I've been trying to mainline a platform (picoxcell) that 
has a DWAPB (with 32-bit access requirement) but struggled to get 
feedback on the SoC port but hope to resubmit as a device tree based 
platform.

1. http://marc.info/?l=linux-serial&m=130821602729595&w=2

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Jamie Iles <jamie@jamieiles.com>
> Cc: Greg Kroah-Hartman <gregkh@suse.de>
> Cc: linux-serial@vger.kernel.org
> ---
>  arch/mips/pmc-sierra/msp71xx/msp_serial.c |   32 ++++++++++-
>  drivers/tty/serial/8250.c                 |   87 +++--------------------------
>  drivers/tty/serial/8250_hub6.c            |   17 ++++++

Should the hub6 stuff be in this patch?

>  drivers/tty/serial/serial_core.c          |    4 -
>  include/linux/serial_core.h               |    5 +-
>  5 files changed, 57 insertions(+), 88 deletions(-)
> 
> diff --git a/arch/mips/pmc-sierra/msp71xx/msp_serial.c b/arch/mips/pmc-sierra/msp71xx/msp_serial.c
> index f726162..6c9f1d3 100644
> --- a/arch/mips/pmc-sierra/msp71xx/msp_serial.c
> +++ b/arch/mips/pmc-sierra/msp71xx/msp_serial.c
> @@ -38,6 +38,33 @@
>  #include <msp_int.h>
>  #include <msp_regs.h>
>  
> +/* Save the LCR value so it can be re-written when a Busy Detect IRQ occurs. */
> +static inline void dwapb_save_out_value(struct uart_port *p, int offset,
> +					int value)
> +{
> +	struct uart_8250_port *up =
> +		container_of(p, struct uart_8250_port, port);

I couldn't see that struct uart_8250_port is moved out of 8250.c, so I'm 
not sure that this would work in here?

Jamie

Re: [PATCH 4/7] serial/8250: move DWAP support to arch/mips

[Arnd Bergmann]

On Tuesday 28 June 2011 00:15:41 Jamie Iles wrote:
> On Mon, Jun 27, 2011 at 11:45:17PM +0200, Arnd Bergmann wrote:
> > Only one board uses the UPIO_DWAP iotype, and nothing uses
> > the UPIO_DWAP32 iotype. It seems much cleaner to handle the
> > DWAP quirk as a new port type, and move the serial_out
> > accessor into the platform code. If more platforms start
> > using the DWAP port, that function can be moved into a
> > common location.
> 
> I posted a series[1] a couple of weeks back that does this for 
> DWAPB/DWAPB32 in a slightly different way which also removes the extra 
> interrupt handling into the platform code.

Yes, your series looks better than my patch (and works!), so let's
take that instead.

> Incidentally, I've been trying to mainline a platform (picoxcell) that 
> has a DWAPB (with 32-bit access requirement) but struggled to get 
> feedback on the SoC port but hope to resubmit as a device tree based 
> platform.

Ok. Please have a look at the xilinx zynq and the csr SiRFprimaII
submissions then. I'm planning to merge new platforms in 3.1, but
only ones that have been fully converted to the device tree and
require no board specific files. I have to admit that I mostly ignored
the picoxcell submission at the time, but I'll definitely review the
patches when you resubmit them.

> >  arch/mips/pmc-sierra/msp71xx/msp_serial.c |   32 ++++++++++-
> >  drivers/tty/serial/8250.c                 |   87 +++--------------------------
> >  drivers/tty/serial/8250_hub6.c            |   17 ++++++
> 
> Should the hub6 stuff be in this patch?

No, I must have accidentally folded that. I'll do a new patch for that.

> > +/* Save the LCR value so it can be re-written when a Busy Detect IRQ occurs. */
> > +static inline void dwapb_save_out_value(struct uart_port *p, int offset,
> > +					int value)
> > +{
> > +	struct uart_8250_port *up =
> > +		container_of(p, struct uart_8250_port, port);
> 
> I couldn't see that struct uart_8250_port is moved out of 8250.c, so I'm 
> not sure that this would work in here?

Right, I missed that as well.

Thanks!

	Arnd

Re: [PATCH 4/7] serial/8250: move DWAP support to arch/mips

[Alan Cox]

On Mon, 27 Jun 2011 23:45:17 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> Only one board uses the UPIO_DWAP iotype, and nothing uses
> the UPIO_DWAP32 iotype. It seems much cleaner to handle the
> DWAP quirk as a new port type, and move the serial_out
> accessor into the platform code. If more platforms start
> using the DWAP port, that function can be moved into a
> common location.

Half of this patch appears to be unrelated work on HUB6

Both halves look right but did you accidentally fold two patches
together ?

Re: [PATCH 4/7] serial/8250: move DWAP support to arch/mips

[Arnd Bergmann]

On Tuesday 28 June 2011, Alan Cox wrote:
> On Mon, 27 Jun 2011 23:45:17 +0200
> Arnd Bergmann <arnd@arndb.de> wrote:
> 
> > Only one board uses the UPIO_DWAP iotype, and nothing uses
> > the UPIO_DWAP32 iotype. It seems much cleaner to handle the
> > DWAP quirk as a new port type, and move the serial_out
> > accessor into the platform code. If more platforms start
> > using the DWAP port, that function can be moved into a
> > common location.
> 
> Half of this patch appears to be unrelated work on HUB6
> 
> Both halves look right but did you accidentally fold two patches
> together ?

Right, Jamie also pointed this out. When I re-send, I'll use Jamie's
version of the patch and make a proper patch for HUB6.

	Arnd

[PATCH 5/7] serial/8250: remove obsolete and broken PORT_RSA support

[Arnd Bergmann]

For all I can tell, the RSA support in the 8250 driver
is not functional and has not been for a long time.

The RSA-DV II/S ISA card[1] was sold by IODATA in the
1990s as a faster alternative to regular ISA serial ports,
and it was discontinued in 2002. Linux support was added
by Kiyokazu SUTO in 2000.

The driver defines an I/O port base address of -8 (negative
eight), which should always get refused by request_region
on all architectures for being outside of the valid
resource range.

Further, it seems that a patch to setserial would be
required in order to set PORT_RSA when not using
autoconfiguration.

This patch removes all code related to the RSA support, with
the exception of the user-visible PORT_RSA definition.
Some of the code has been copied into the sunsu.c driver,
so remove it from there as well.

[1] http://translate.google.com/translate?hl=en&sl=ja&u=http://www.iodata.jp/products/network/rsadv2s.htm

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Kiyokazu SUTO <suto@ks-and-ks.ne.jp>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: linux-serial@vger.kernel.org
---
 drivers/tty/serial/8250.c   |  200 ++-----------------------------------------
 drivers/tty/serial/Kconfig  |    6 --
 drivers/tty/serial/sunsu.c  |   93 --------------------
 include/linux/serial.h      |    2 +-
 include/linux/serial_core.h |    2 +-
 include/linux/serial_reg.h  |   51 -----------
 6 files changed, 10 insertions(+), 344 deletions(-)

diff --git a/drivers/tty/serial/8250.c b/drivers/tty/serial/8250.c
index 5698157..0ddba34 100644
--- a/drivers/tty/serial/8250.c
+++ b/drivers/tty/serial/8250.c
@@ -116,13 +116,6 @@ static const struct old_serial_port old_serial_port[] = {
 
 #define UART_NR	CONFIG_SERIAL_8250_NR_UARTS
 
-#ifdef CONFIG_SERIAL_8250_RSA
-
-#define PORT_RSA_MAX 4
-static unsigned long probe_rsa[PORT_RSA_MAX];
-static unsigned int probe_rsa_count;
-#endif /* CONFIG_SERIAL_8250_RSA  */
-
 struct uart_8250_port {
 	struct uart_port	port;
 	struct timer_list	timer;		/* "no irq" timer */
@@ -246,13 +239,6 @@ static const struct serial8250_config uart_config[] = {
 		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
 		.flags		= UART_CAP_FIFO | UART_CAP_EFR | UART_CAP_SLEEP,
 	},
-	[PORT_RSA] = {
-		.name		= "RSA",
-		.fifo_size	= 2048,
-		.tx_loadsz	= 2048,
-		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_11,
-		.flags		= UART_CAP_FIFO,
-	},
 	[PORT_NS16550A] = {
 		.name		= "NS16550A",
 		.fifo_size	= 16,
@@ -488,75 +474,6 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep)
 	}
 }
 
-#ifdef CONFIG_SERIAL_8250_RSA
-/*
- * Attempts to turn on the RSA FIFO.  Returns zero on failure.
- * We set the port uart clock rate if we succeed.
- */
-static int __enable_rsa(struct uart_8250_port *up)
-{
-	unsigned char mode;
-	int result;
-
-	mode = serial_inp(up, UART_RSA_MSR);
-	result = mode & UART_RSA_MSR_FIFO;
-
-	if (!result) {
-		serial_outp(up, UART_RSA_MSR, mode | UART_RSA_MSR_FIFO);
-		mode = serial_inp(up, UART_RSA_MSR);
-		result = mode & UART_RSA_MSR_FIFO;
-	}
-
-	if (result)
-		up->port.uartclk = SERIAL_RSA_BAUD_BASE * 16;
-
-	return result;
-}
-
-static void enable_rsa(struct uart_8250_port *up)
-{
-	if (up->port.type == PORT_RSA) {
-		if (up->port.uartclk != SERIAL_RSA_BAUD_BASE * 16) {
-			spin_lock_irq(&up->port.lock);
-			__enable_rsa(up);
-			spin_unlock_irq(&up->port.lock);
-		}
-		if (up->port.uartclk == SERIAL_RSA_BAUD_BASE * 16)
-			serial_outp(up, UART_RSA_FRR, 0);
-	}
-}
-
-/*
- * Attempts to turn off the RSA FIFO.  Returns zero on failure.
- * It is unknown why interrupts were disabled in here.  However,
- * the caller is expected to preserve this behaviour by grabbing
- * the spinlock before calling this function.
- */
-static void disable_rsa(struct uart_8250_port *up)
-{
-	unsigned char mode;
-	int result;
-
-	if (up->port.type == PORT_RSA &&
-	    up->port.uartclk == SERIAL_RSA_BAUD_BASE * 16) {
-		spin_lock_irq(&up->port.lock);
-
-		mode = serial_inp(up, UART_RSA_MSR);
-		result = !(mode & UART_RSA_MSR_FIFO);
-
-		if (!result) {
-			serial_outp(up, UART_RSA_MSR, mode & ~UART_RSA_MSR_FIFO);
-			mode = serial_inp(up, UART_RSA_MSR);
-			result = !(mode & UART_RSA_MSR_FIFO);
-		}
-
-		if (result)
-			up->port.uartclk = SERIAL_RSA_BAUD_BASE_LO * 16;
-		spin_unlock_irq(&up->port.lock);
-	}
-}
-#endif /* CONFIG_SERIAL_8250_RSA */
-
 /*
  * This is a quickie test to see how big the FIFO is.
  * It doesn't work at all the time, more's the pity.
@@ -1042,23 +959,6 @@ static void autoconfig(struct uart_8250_port *up, unsigned int probeflags)
 		break;
 	}
 
-#ifdef CONFIG_SERIAL_8250_RSA
-	/*
-	 * Only probe for RSA ports if we got the region.
-	 */
-	if (up->port.type == PORT_16550A && probeflags & PROBE_RSA) {
-		int i;
-
-		for (i = 0 ; i < probe_rsa_count; ++i) {
-			if (probe_rsa[i] == up->port.iobase &&
-			    __enable_rsa(up)) {
-				up->port.type = PORT_RSA;
-				break;
-			}
-		}
-	}
-#endif
-
 	serial_outp(up, UART_LCR, save_lcr);
 
 	if (up->capabilities != uart_config[up->port.type].flags) {
@@ -1078,10 +978,6 @@ static void autoconfig(struct uart_8250_port *up, unsigned int probeflags)
 	/*
 	 * Reset the UART.
 	 */
-#ifdef CONFIG_SERIAL_8250_RSA
-	if (up->port.type == PORT_RSA)
-		serial_outp(up, UART_RSA_FRR, 0);
-#endif
 	serial_outp(up, UART_MCR, save_mcr);
 	serial8250_clear_fifos(up);
 	serial_in(up, UART_RX);
@@ -1836,14 +1732,6 @@ static int serial8250_startup(struct uart_port *port)
 		serial_outp(up, UART_LCR, 0);
 	}
 
-#ifdef CONFIG_SERIAL_8250_RSA
-	/*
-	 * If this is an RSA port, see if we can kick it up to the
-	 * higher speed clock.
-	 */
-	enable_rsa(up);
-#endif
-
 	/*
 	 * Clear the FIFO buffers and disable them.
 	 * (they will be reenabled in set_termios())
@@ -2067,13 +1955,6 @@ static void serial8250_shutdown(struct uart_port *port)
 	serial_out(up, UART_LCR, serial_inp(up, UART_LCR) & ~UART_LCR_SBC);
 	serial8250_clear_fifos(up);
 
-#ifdef CONFIG_SERIAL_8250_RSA
-	/*
-	 * Reset the RSA board back to 115kbps compat mode.
-	 */
-	disable_rsa(up);
-#endif
-
 	/*
 	 * Read data port to reset things, and then unlink from
 	 * the IRQ chain.
@@ -2344,8 +2225,10 @@ static unsigned int serial8250_port_size(struct uart_8250_port *pt)
 /*
  * Resource handling.
  */
-static int serial8250_request_std_resource(struct uart_8250_port *up)
+static int serial8250_request_port(struct uart_port *port)
 {
+	struct uart_8250_port *up =
+		container_of(port, struct uart_8250_port, port);
 	unsigned int size = serial8250_port_size(up);
 	int ret = 0;
 
@@ -2381,8 +2264,10 @@ static int serial8250_request_std_resource(struct uart_8250_port *up)
 	return ret;
 }
 
-static void serial8250_release_std_resource(struct uart_8250_port *up)
+static void serial8250_release_port(struct uart_port *port)
 {
+	struct uart_8250_port *up =
+		container_of(port, struct uart_8250_port, port);
 	unsigned int size = serial8250_port_size(up);
 
 	switch (up->port.iotype) {
@@ -2408,65 +2293,6 @@ static void serial8250_release_std_resource(struct uart_8250_port *up)
 	}
 }
 
-static int serial8250_request_rsa_resource(struct uart_8250_port *up)
-{
-	unsigned long start = UART_RSA_BASE << up->port.regshift;
-	unsigned int size = 8 << up->port.regshift;
-	int ret = -EINVAL;
-
-	switch (up->port.iotype) {
-	case UPIO_HUB6:
-	case UPIO_PORT:
-		start += up->port.iobase;
-		if (request_region(start, size, "serial-rsa"))
-			ret = 0;
-		else
-			ret = -EBUSY;
-		break;
-	}
-
-	return ret;
-}
-
-static void serial8250_release_rsa_resource(struct uart_8250_port *up)
-{
-	unsigned long offset = UART_RSA_BASE << up->port.regshift;
-	unsigned int size = 8 << up->port.regshift;
-
-	switch (up->port.iotype) {
-	case UPIO_HUB6:
-	case UPIO_PORT:
-		release_region(up->port.iobase + offset, size);
-		break;
-	}
-}
-
-static void serial8250_release_port(struct uart_port *port)
-{
-	struct uart_8250_port *up =
-		container_of(port, struct uart_8250_port, port);
-
-	serial8250_release_std_resource(up);
-	if (up->port.type == PORT_RSA)
-		serial8250_release_rsa_resource(up);
-}
-
-static int serial8250_request_port(struct uart_port *port)
-{
-	struct uart_8250_port *up =
-		container_of(port, struct uart_8250_port, port);
-	int ret = 0;
-
-	ret = serial8250_request_std_resource(up);
-	if (ret == 0 && up->port.type == PORT_RSA) {
-		ret = serial8250_request_rsa_resource(up);
-		if (ret < 0)
-			serial8250_release_std_resource(up);
-	}
-
-	return ret;
-}
-
 static void serial8250_config_port(struct uart_port *port, int flags)
 {
 	struct uart_8250_port *up =
@@ -2478,14 +2304,10 @@ static void serial8250_config_port(struct uart_port *port, int flags)
 	 * Find the region that we can probe for.  This in turn
 	 * tells us whether we can probe for the type of port.
 	 */
-	ret = serial8250_request_std_resource(up);
+	ret = serial8250_request_port(port);
 	if (ret < 0)
 		return;
 
-	ret = serial8250_request_rsa_resource(up);
-	if (ret < 0)
-		probeflags &= ~PROBE_RSA;
-
 	if (up->port.iotype != up->cur_iotype)
 		set_io_from_upio(port);
 
@@ -2499,10 +2321,8 @@ static void serial8250_config_port(struct uart_port *port, int flags)
 	if (up->port.type != PORT_UNKNOWN && flags & UART_CONFIG_IRQ)
 		autoconfig_irq(up);
 
-	if (up->port.type != PORT_RSA && probeflags & PROBE_RSA)
-		serial8250_release_rsa_resource(up);
 	if (up->port.type == PORT_UNKNOWN)
-		serial8250_release_std_resource(up);
+		serial8250_release_port(port);
 }
 
 static int
@@ -3198,8 +3018,4 @@ MODULE_PARM_DESC(nr_uarts, "Maximum number of UARTs supported. (1-" __MODULE_STR
 module_param(skip_txen_test, uint, 0644);
 MODULE_PARM_DESC(skip_txen_test, "Skip checking for the TXEN bug at init time");
 
-#ifdef CONFIG_SERIAL_8250_RSA
-module_param_array(probe_rsa, ulong, &probe_rsa_count, 0444);
-MODULE_PARM_DESC(probe_rsa, "Probe I/O ports for RSA");
-#endif
 MODULE_ALIAS_CHARDEV_MAJOR(TTY_MAJOR);
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index ea12815..3ec604f 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -235,12 +235,6 @@ config SERIAL_8250_DETECT_IRQ
 
 	  If unsure, say N.
 
-config SERIAL_8250_RSA
-	bool "Support RSA serial ports"
-	depends on SERIAL_8250_EXTENDED
-	help
-	  ::: To be written :::
-
 config SERIAL_8250_MCA
 	tristate "Support 8250-type ports on MCA buses"
 	depends on SERIAL_8250 != n && MCA
diff --git a/drivers/tty/serial/sunsu.c b/drivers/tty/serial/sunsu.c
index 92aa545..a52af85 100644
--- a/drivers/tty/serial/sunsu.c
+++ b/drivers/tty/serial/sunsu.c
@@ -75,7 +75,6 @@ static const struct serial_uart_config uart_config[PORT_MAX_8250+1] = {
 	{ "16C950/954",	128,	UART_CLEAR_FIFO | UART_USE_FIFO },
 	{ "ST16654",	64,	UART_CLEAR_FIFO | UART_USE_FIFO | UART_STARTECH },
 	{ "XR16850",	128,	UART_CLEAR_FIFO | UART_USE_FIFO | UART_STARTECH },
-	{ "RSA",	2048,	UART_CLEAR_FIFO | UART_USE_FIFO }
 };
 
 struct uart_sunsu_port {
@@ -179,75 +178,6 @@ static unsigned int serial_icr_read(struct uart_sunsu_port *up, int offset)
 }
 #endif
 
-#ifdef CONFIG_SERIAL_8250_RSA
-/*
- * Attempts to turn on the RSA FIFO.  Returns zero on failure.
- * We set the port uart clock rate if we succeed.
- */
-static int __enable_rsa(struct uart_sunsu_port *up)
-{
-	unsigned char mode;
-	int result;
-
-	mode = serial_inp(up, UART_RSA_MSR);
-	result = mode & UART_RSA_MSR_FIFO;
-
-	if (!result) {
-		serial_outp(up, UART_RSA_MSR, mode | UART_RSA_MSR_FIFO);
-		mode = serial_inp(up, UART_RSA_MSR);
-		result = mode & UART_RSA_MSR_FIFO;
-	}
-
-	if (result)
-		up->port.uartclk = SERIAL_RSA_BAUD_BASE * 16;
-
-	return result;
-}
-
-static void enable_rsa(struct uart_sunsu_port *up)
-{
-	if (up->port.type == PORT_RSA) {
-		if (up->port.uartclk != SERIAL_RSA_BAUD_BASE * 16) {
-			spin_lock_irq(&up->port.lock);
-			__enable_rsa(up);
-			spin_unlock_irq(&up->port.lock);
-		}
-		if (up->port.uartclk == SERIAL_RSA_BAUD_BASE * 16)
-			serial_outp(up, UART_RSA_FRR, 0);
-	}
-}
-
-/*
- * Attempts to turn off the RSA FIFO.  Returns zero on failure.
- * It is unknown why interrupts were disabled in here.  However,
- * the caller is expected to preserve this behaviour by grabbing
- * the spinlock before calling this function.
- */
-static void disable_rsa(struct uart_sunsu_port *up)
-{
-	unsigned char mode;
-	int result;
-
-	if (up->port.type == PORT_RSA &&
-	    up->port.uartclk == SERIAL_RSA_BAUD_BASE * 16) {
-		spin_lock_irq(&up->port.lock);
-
-		mode = serial_inp(up, UART_RSA_MSR);
-		result = !(mode & UART_RSA_MSR_FIFO);
-
-		if (!result) {
-			serial_outp(up, UART_RSA_MSR, mode & ~UART_RSA_MSR_FIFO);
-			mode = serial_inp(up, UART_RSA_MSR);
-			result = !(mode & UART_RSA_MSR_FIFO);
-		}
-
-		if (result)
-			up->port.uartclk = SERIAL_RSA_BAUD_BASE_LO * 16;
-		spin_unlock_irq(&up->port.lock);
-	}
-}
-#endif /* CONFIG_SERIAL_8250_RSA */
-
 static inline void __stop_tx(struct uart_sunsu_port *p)
 {
 	if (p->ier & UART_IER_THRI) {
@@ -626,14 +556,6 @@ static int sunsu_startup(struct uart_port *port)
 		serial_outp(up, UART_LCR, 0);
 	}
 
-#ifdef CONFIG_SERIAL_8250_RSA
-	/*
-	 * If this is an RSA port, see if we can kick it up to the
-	 * higher speed clock.
-	 */
-	enable_rsa(up);
-#endif
-
 	/*
 	 * Clear the FIFO buffers and disable them.
 	 * (they will be reenabled in set_termios())
@@ -748,13 +670,6 @@ static void sunsu_shutdown(struct uart_port *port)
 				  UART_FCR_CLEAR_XMIT);
 	serial_outp(up, UART_FCR, 0);
 
-#ifdef CONFIG_SERIAL_8250_RSA
-	/*
-	 * Reset the RSA board back to 115kbps compat mode.
-	 */
-	disable_rsa(up);
-#endif
-
 	/*
 	 * Read data port to reset things.
 	 */
@@ -810,10 +725,6 @@ sunsu_change_speed(struct uart_port *port, unsigned int cflag,
 	if (uart_config[up->port.type].flags & UART_USE_FIFO) {
 		if ((up->port.uartclk / quot) < (2400 * 16))
 			fcr = UART_FCR_ENABLE_FIFO | UART_FCR_TRIGGER_1;
-#ifdef CONFIG_SERIAL_8250_RSA
-		else if (up->port.type == PORT_RSA)
-			fcr = UART_FCR_ENABLE_FIFO | UART_FCR_TRIGGER_14;
-#endif
 		else
 			fcr = UART_FCR_ENABLE_FIFO | UART_FCR_TRIGGER_8;
 	}
@@ -1156,10 +1067,6 @@ static void sunsu_autoconfig(struct uart_sunsu_port *up)
 	/*
 	 * Reset the UART.
 	 */
-#ifdef CONFIG_SERIAL_8250_RSA
-	if (up->port.type == PORT_RSA)
-		serial_outp(up, UART_RSA_FRR, 0);
-#endif
 	serial_outp(up, UART_MCR, save_mcr);
 	serial_outp(up, UART_FCR, (UART_FCR_ENABLE_FIFO |
 				     UART_FCR_CLEAR_RCVR |
diff --git a/include/linux/serial.h b/include/linux/serial.h
index ef91406..7bf30a6 100644
--- a/include/linux/serial.h
+++ b/include/linux/serial.h
@@ -76,7 +76,7 @@ struct serial_struct {
 #define PORT_16C950	10	/* Oxford Semiconductor */
 #define PORT_16654	11
 #define PORT_16850	12
-#define PORT_RSA	13	/* RSA-DV II/S card */
+#define PORT_RSA	13	/* obsolete RSA-DV II/S card */
 #define PORT_MAX	13
 
 #define SERIAL_IO_PORT	0
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 598992f..f84c560 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -38,7 +38,7 @@
 #define PORT_16C950	10
 #define PORT_16654	11
 #define PORT_16850	12
-#define PORT_RSA	13
+/* #define PORT_RSA	13 */	/* was: RSA-DV II/S */
 #define PORT_NS16550A	14
 #define PORT_XSCALE	15
 /* #define PORT_RM9000	16 */	/* was: PMC-Sierra RM9xxx internal UART */
diff --git a/include/linux/serial_reg.h b/include/linux/serial_reg.h
index c75bda3..4fa3d59 100644
--- a/include/linux/serial_reg.h
+++ b/include/linux/serial_reg.h
@@ -284,57 +284,6 @@
 #define UART_ACR_ICRRD	0x40	/* ICR Read enable */
 #define UART_ACR_ASREN	0x80	/* Additional status enable */
 
-
-
-/*
- * These definitions are for the RSA-DV II/S card, from
- *
- * Kiyokazu SUTO <suto@ks-and-ks.ne.jp>
- */
-
-#define UART_RSA_BASE (-8)
-
-#define UART_RSA_MSR ((UART_RSA_BASE) + 0) /* I/O: Mode Select Register */
-
-#define UART_RSA_MSR_SWAP (1 << 0) /* Swap low/high 8 bytes in I/O port addr */
-#define UART_RSA_MSR_FIFO (1 << 2) /* Enable the external FIFO */
-#define UART_RSA_MSR_FLOW (1 << 3) /* Enable the auto RTS/CTS flow control */
-#define UART_RSA_MSR_ITYP (1 << 4) /* Level (1) / Edge triger (0) */
-
-#define UART_RSA_IER ((UART_RSA_BASE) + 1) /* I/O: Interrupt Enable Register */
-
-#define UART_RSA_IER_Rx_FIFO_H (1 << 0) /* Enable Rx FIFO half full int. */
-#define UART_RSA_IER_Tx_FIFO_H (1 << 1) /* Enable Tx FIFO half full int. */
-#define UART_RSA_IER_Tx_FIFO_E (1 << 2) /* Enable Tx FIFO empty int. */
-#define UART_RSA_IER_Rx_TOUT (1 << 3) /* Enable char receive timeout int */
-#define UART_RSA_IER_TIMER (1 << 4) /* Enable timer interrupt */
-
-#define UART_RSA_SRR ((UART_RSA_BASE) + 2) /* IN: Status Read Register */
-
-#define UART_RSA_SRR_Tx_FIFO_NEMP (1 << 0) /* Tx FIFO is not empty (1) */
-#define UART_RSA_SRR_Tx_FIFO_NHFL (1 << 1) /* Tx FIFO is not half full (1) */
-#define UART_RSA_SRR_Tx_FIFO_NFUL (1 << 2) /* Tx FIFO is not full (1) */
-#define UART_RSA_SRR_Rx_FIFO_NEMP (1 << 3) /* Rx FIFO is not empty (1) */
-#define UART_RSA_SRR_Rx_FIFO_NHFL (1 << 4) /* Rx FIFO is not half full (1) */
-#define UART_RSA_SRR_Rx_FIFO_NFUL (1 << 5) /* Rx FIFO is not full (1) */
-#define UART_RSA_SRR_Rx_TOUT (1 << 6) /* Character reception timeout occurred (1) */
-#define UART_RSA_SRR_TIMER (1 << 7) /* Timer interrupt occurred */
-
-#define UART_RSA_FRR ((UART_RSA_BASE) + 2) /* OUT: FIFO Reset Register */
-
-#define UART_RSA_TIVSR ((UART_RSA_BASE) + 3) /* I/O: Timer Interval Value Set Register */
-
-#define UART_RSA_TCR ((UART_RSA_BASE) + 4) /* OUT: Timer Control Register */
-
-#define UART_RSA_TCR_SWITCH (1 << 0) /* Timer on */
-
-/*
- * The RSA DSV/II board has two fixed clock frequencies.  One is the
- * standard rate, and the other is 8 times faster.
- */
-#define SERIAL_RSA_BAUD_BASE (921600)
-#define SERIAL_RSA_BAUD_BASE_LO (SERIAL_RSA_BAUD_BASE / 8)
-
 /*
  * Extra serial register definitions for the internal UARTs
  * in TI OMAP processors.
-- 
1.7.5.4

Re: [PATCH 5/7] serial/8250: remove obsolete and broken PORT_RSA support

[Alan Cox]

On Mon, 27 Jun 2011 23:45:18 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> For all I can tell, the RSA support in the 8250 driver
> is not functional and has not been for a long time.

Seems fair enough

Only one that needs dealing with properly is the PIO method ifdef stuff.

[PATCH 6/7] serial/8250: sanitize fourport handling

[Arnd Bergmann]

Support for AST fourport cards is always built into
the 8250 driver, even if CONFIG_SERIAL_8250_FOURPORT
is disabled. This introduces a set of macros for
accessing the special interrupt control register on
the fourport card, so that support is left out
when the option is disabled, without adding more #ifdef
lines to the driver itself.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: linux-serial@vger.kernel.org
---
 drivers/tty/serial/8250.c |   18 +++++++-----------
 drivers/tty/serial/8250.h |    8 ++++++++
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/serial/8250.c b/drivers/tty/serial/8250.c
index 0ddba34..2b06c27 100644
--- a/drivers/tty/serial/8250.c
+++ b/drivers/tty/serial/8250.c
@@ -995,15 +995,13 @@ static void autoconfig_irq(struct uart_8250_port *up)
 {
 	unsigned char save_mcr, save_ier;
 	unsigned char save_ICP = 0;
-	unsigned int ICP = 0;
 	unsigned long irqs;
 	int irq;
 
 	if (up->port.flags & UPF_FOURPORT) {
-		ICP = (up->port.iobase & 0xfe0) | 0x1f;
-		save_ICP = inb_p(ICP);
-		outb_p(0x80, ICP);
-		(void) inb_p(ICP);
+		save_ICP = fourport_icp_in(up);
+		fourport_icp_out(up, 0x80);
+		(void) fourport_icp_in(up);
 	}
 
 	/* forget possible initially masked and pending IRQ */
@@ -1035,7 +1033,7 @@ static void autoconfig_irq(struct uart_8250_port *up)
 	serial_outp(up, UART_IER, save_ier);
 
 	if (up->port.flags & UPF_FOURPORT)
-		outb_p(save_ICP, ICP);
+		fourport_icp_out(up, save_ICP);
 
 	up->port.irq = (irq > 0) ? irq : 0;
 }
@@ -1914,13 +1912,11 @@ dont_test_tx_en:
 	serial_outp(up, UART_IER, up->ier);
 
 	if (up->port.flags & UPF_FOURPORT) {
-		unsigned int icp;
 		/*
 		 * Enable interrupts on the AST Fourport board
 		 */
-		icp = (up->port.iobase & 0xfe0) | 0x01f;
-		outb_p(0x80, icp);
-		(void) inb_p(icp);
+		fourport_icp_out(up, 0x80);
+		(void) fourport_icp_in(up);
 	}
 
 	return 0;
@@ -1941,7 +1937,7 @@ static void serial8250_shutdown(struct uart_port *port)
 	spin_lock_irqsave(&up->port.lock, flags);
 	if (up->port.flags & UPF_FOURPORT) {
 		/* reset interrupts on the AST Fourport board */
-		inb((up->port.iobase & 0xfe0) | 0x1f);
+		(void) fourport_icp_in(up);
 		up->port.mctrl |= TIOCM_OUT1;
 	} else
 		up->port.mctrl &= ~TIOCM_OUT2;
diff --git a/drivers/tty/serial/8250.h b/drivers/tty/serial/8250.h
index 6edf4a6..1fc46fe 100644
--- a/drivers/tty/serial/8250.h
+++ b/drivers/tty/serial/8250.h
@@ -60,6 +60,14 @@ struct serial8250_config {
 #define SERIAL8250_SHARE_IRQS 0
 #endif
 
+#if defined(CONFIG_SERIAL_8250_FOURPORT) || defined(CONFIG_SERIAL_8250_FOURPORT_MODULE)
+# define fourport_icp_in(up) 	 inb_p((up->port.iobase & 0xfe0) | 0x01f)
+# define fourport_icp_out(up, v) outb_p(v, (up->port.iobase & 0xfe0) | 0x01f)
+#else
+# define fourport_icp_in(up)	 (0)
+# define fourport_icp_out(up, v) do { } while (0)
+#endif
+
 #if defined(__alpha__) && !defined(CONFIG_PCI)
 /*
  * Digital did something really horribly wrong with the OUT1 and OUT2
-- 
1.7.5.4

Re: [PATCH 6/7] serial/8250: sanitize fourport handling

[Alan Cox]

On Mon, 27 Jun 2011 23:45:19 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> Support for AST fourport cards is always built into
> the 8250 driver, even if CONFIG_SERIAL_8250_FOURPORT
> is disabled. This introduces a set of macros for
> accessing the special interrupt control register on
> the fourport card, so that support is left out
> when the option is disabled, without adding more #ifdef
> lines to the driver itself.

Why not just leave it in - it seems to take almost no code and your
changes mean people setting the fourport flag are going to get strange
behaviour when it isn't compiled in.

Re: [PATCH 6/7] serial/8250: sanitize fourport handling

[Arnd Bergmann]

On Tuesday 28 June 2011, Alan Cox wrote:
> On Mon, 27 Jun 2011 23:45:19 +0200
> Arnd Bergmann <arnd@arndb.de> wrote:
> 
> > Support for AST fourport cards is always built into
> > the 8250 driver, even if CONFIG_SERIAL_8250_FOURPORT
> > is disabled. This introduces a set of macros for
> > accessing the special interrupt control register on
> > the fourport card, so that support is left out
> > when the option is disabled, without adding more #ifdef
> > lines to the driver itself.
> 
> Why not just leave it in - it seems to take almost no code and your
> changes mean people setting the fourport flag are going to get strange
> behaviour when it isn't compiled in.

Mostly to avoid the inb/outb when fourport is disabled. The regular
I/O can be handled using the indirect serial_in/out functions, but
the fourport handling requires some address calculations.

Also, now that you mention users toggling the fourport flag, I can't
see where that would ever do something good. On anything but a real
fourport card, it will lead to writing to ports you shouldn't write
to (0x1f on mmio based uarts, 18 bytes after the requested region
for regular ports).

If you have an actual AST fourport card, you won't be able to use
it unless you load the 8250_fourport driver that registers the correct
addresses, and disabling the fourport flag will then cause the
uart to stop getting interrupts.

	Arnd

[PATCH 7/7] serial/8250: make PIO support optional

[Arnd Bergmann]

It is not currently possible to build support for the 8250 UART
on architectures that don't define PC-style I/O accessors like
inb/outb. Most embedded systems without PCI don't actually have
PC-style I/O, so they should not have to make up their own
accessors.

This makes the PIO support in the 8250 driver completely
conditional on CONFIG_HAS_IOPORT so we can remove the bogus
definitions from all the places that only need them for 8250.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: linux-serial@vger.kernel.org
---
 drivers/tty/serial/8250.c |   20 ++++++++++++++------
 1 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/8250.c b/drivers/tty/serial/8250.c
index 2b06c27..a2933d6 100644
--- a/drivers/tty/serial/8250.c
+++ b/drivers/tty/serial/8250.c
@@ -318,6 +318,7 @@ static unsigned int mem32_serial_in(struct uart_port *p, int offset)
 	return readl(p->membase + offset);
 }
 
+#ifdef CONFIG_HAS_IOPORT
 static unsigned int io_serial_in(struct uart_port *p, int offset)
 {
 	offset = map_8250_in_reg(p, offset) << p->regshift;
@@ -329,16 +330,19 @@ static void io_serial_out(struct uart_port *p, int offset, int value)
 	offset = map_8250_out_reg(p, offset) << p->regshift;
 	outb(value, p->iobase + offset);
 }
+#endif
 
 static void set_io_from_upio(struct uart_port *p)
 {
 	struct uart_8250_port *up =
 		container_of(p, struct uart_8250_port, port);
 	switch (p->iotype) {
-	case UPIO_MEM:
-		p->serial_in = mem_serial_in;
-		p->serial_out = mem_serial_out;
+#ifdef CONFIG_HAS_IOPORT
+	case UPIO_PORT:
+		p->serial_in = io_serial_in;
+		p->serial_out = io_serial_out;
 		break;
+#endif
 
 	case UPIO_MEM32:
 		p->serial_in = mem32_serial_in;
@@ -346,9 +350,10 @@ static void set_io_from_upio(struct uart_port *p)
 		break;
 
 	default:
-		p->serial_in = io_serial_in;
-		p->serial_out = io_serial_out;
+		p->serial_in = mem_serial_in;
+		p->serial_out = mem_serial_out;
 		break;
+
 	}
 	/* Remember loaded iotype */
 	up->cur_iotype = p->iotype;
@@ -2250,12 +2255,13 @@ static int serial8250_request_port(struct uart_port *port)
 			}
 		}
 		break;
-
+#ifdef CONFIG_HAS_IOPORT
 	case UPIO_HUB6:
 	case UPIO_PORT:
 		if (!request_region(up->port.iobase, size, "serial"))
 			ret = -EBUSY;
 		break;
+#endif
 	}
 	return ret;
 }
@@ -2282,10 +2288,12 @@ static void serial8250_release_port(struct uart_port *port)
 		release_mem_region(up->port.mapbase, size);
 		break;
 
+#ifdef CONFIG_HAS_IOPORT
 	case UPIO_HUB6:
 	case UPIO_PORT:
 		release_region(up->port.iobase, size);
 		break;
+#endif
 	}
 }
 
-- 
1.7.5.4

Re: [PATCH 7/7] serial/8250: make PIO support optional

[Alan Cox]

> This makes the PIO support in the 8250 driver completely
> conditional on CONFIG_HAS_IOPORT so we can remove the bogus
> definitions from all the places that only need them for 8250.

NAK this as last time.

We had a discussion about how to fix this properly. I even posted some
sketched out patches to show how it could be addressed

Shotgunning the code with ifdefs was not that.


Get the port methods out of the core code as was proposed.

Re: [PATCH 7/7] serial/8250: make PIO support optional

[Arnd Bergmann]

On Tuesday 28 June 2011, Alan Cox wrote:
> > This makes the PIO support in the 8250 driver completely
> > conditional on CONFIG_HAS_IOPORT so we can remove the bogus
> > definitions from all the places that only need them for 8250.
> 
> NAK this as last time.
> 
> We had a discussion about how to fix this properly. I even posted some
> sketched out patches to show how it could be addressed
> 
> Shotgunning the code with ifdefs was not that.
> 
> 
> Get the port methods out of the core code as was proposed.

I got this done for all the obscure ones in patches 1-6, leaving
only the common UPIO_PORT and UPIO_MEM ones. Changing those would
mean a lot more churn, and also solving a few other problems:

Back then, I wrote:
>On Friday 11 March 2011, Alan Cox wrote:
>
>> For 8250 the way to do that is to remove all the switches and port type
>> stuff and propogate to setting ->serial_in and ->serial_out rather than
>> splattering the code with ifdefs. At that point you'd have a "lib8250" or
>> similar and an 8250_io/pci driver.
>
>Right, this absolutely makes sense. It's a lot more work than the originally
>suggested patch, but the result is much cleaner.

One of the problems I encountered now is that most of the frontend drivers
(pci, of, platform, pcmcia) still require access to both MEM and PORT
methods on some platforms, but we want to use the same drivers (platform
and of) on systems that don't have port methods, so it's back to #ifdef
anyway.

Another problem is that the platform devices that set ->iotype are
usually defined in builtin code. Changing the device definitions to
set serial_in/serial_out directly means we also have to link those
functions into the kernel, even if the actual 8250 driver is a module.

A possible option might be to use ioread/iowrite to replace the
choice between MEM and PORT I/O with a common function. I thought
about that, but wasn't sure if we can alwasy call ioport_map early
enough for the console driver.

On Tuesday 28 June 2011, Alan Cox wrote:
> > -             p->serial_in = io_serial_in;
> > -             p->serial_out = io_serial_out;
> > +             p->serial_in = mem_serial_in;
> > +             p->serial_out = mem_serial_out;
> >               break;
>
>And this kind of stuff changing defaults is asking for trouble.

I went back and forth between a few versions on this one, and only
later thought of one that would have been better. I did make sure
that the "default" case is only there for UPIO_PORT before and only
for UPIO_MEM after the patch. I can fix this in a better way.

The easiest approach would be to conditionalize just the inb/outb,
as below. As I explained the last time, the 8250 driver is really
the only one that has this problem because it doesn't use
ioread/iowrite and is common on both PC-style hardware and embedded
into SOCs that don't have PIO.

I would much prefer getting a build error on inb/outb for the latter
kind than having to provide bogus definitions in a lot of architectures.
For request_region, it's probably better to stub out the macro than
the users.

	Arnd

--- a/drivers/tty/serial/8250.c
+++ b/drivers/tty/serial/8250.c
@@ -499,14 +499,18

 static unsigned int io_serial_in(struct uart_port *p, int offset)
 {
-       offset = map_8250_in_reg(p, offset) << p->regshift;
+#ifdef CONFIG_HAS_IOPORT
        return inb(p->iobase + offset);
+#else
+       return 0xff;
+#endif
 }

 static void io_serial_out(struct uart_port *p, int offset, int value)
 {
-       offset = map_8250_out_reg(p, offset) << p->regshift;
+#ifdef CONFIG_HAS_IOPORT
        outb(value, p->iobase + offset);
+#endif
 }

 static void set_io_from_upio(struct uart_port *p)
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -145,8 +145,13 @@ static inline unsigned long resource_type(const struct resource *res)
 }

 /* Convenience shorthand with allocation */
+#ifdef CONFIG_HAS_IOPORT
 #define request_region(start,n,name)           __request_region(&ioport_resource, (start), (n), (name), 0)
 #define request_muxed_region(start,n,name)     __request_region(&ioport_resource, (start), (n), (name), IORESOURCE_MUXED)
+#else
+#define request_region(start,n,name)           NULL
+#define request_muxed_region(start,n,name)     NULL
+#endif
 #define __request_mem_region(start,n,name, excl) __request_region(&iomem_resource, (start), (n), (name), excl)
 #define request_mem_region(start,n,name) __request_region(&iomem_resource, (start), (n), (name), 0)
 #define request_mem_region_exclusive(start,n,name) \

Re: [PATCH 7/7] serial/8250: make PIO support optional

[Alan Cox]

> I got this done for all the obscure ones in patches 1-6, leaving
> only the common UPIO_PORT and UPIO_MEM ones. Changing those would
> mean a lot more churn, and also solving a few other problems:

Yes I like 1-5 ...

> I would much prefer getting a build error on inb/outb for the latter
> kind than having to provide bogus definitions in a lot of architectures.
> For request_region, it's probably better to stub out the macro than
> the users.

Agreed but that can be done by io_serial_in/io_serial_out out of the
8250.c file. Really we need p->ops or a function in another file which
provides the ioport 8250 interface and then all you'd have in the core
code would be

	uart8250_set_ioport_ops(p);

which would live in 8250_ioport.c or be a "return -EINVAL' for anything
else. 

That leaves request/release_std resources depending how such platforms
handle request_region attempts, and the other fourport bits which want
pushing into the 8250f_fourport driver perhaps as extra p-> ops.

	if (p->startup)
		p->startup(p);

etc

(Or indeed probably a p->ops-> for the lot is even saner)



Thoughts ?

Re: [PATCH 7/7] serial/8250: make PIO support optional

[Arnd Bergmann]

On Tuesday 28 June 2011, Alan Cox wrote:
> Agreed but that can be done by io_serial_in/io_serial_out out of the
> 8250.c file. Really we need p->ops or a function in another file which
> provides the ioport 8250 interface and then all you'd have in the core
> code would be
> 
>         uart8250_set_ioport_ops(p);
> 
> which would live in 8250_ioport.c or be a "return -EINVAL' for anything
> else. 
> 
> That leaves request/release_std resources depending how such platforms
> handle request_region attempts, and the other fourport bits which want
> pushing into the 8250f_fourport driver perhaps as extra p-> ops.
> 
>         if (p->startup)
>                 p->startup(p);
> 
> etc
> 
> (Or indeed probably a p->ops-> for the lot is even saner)
> 
> Thoughts ?

I've tried a few of variations of that now, and I felt that I'm
getting drawn into some of the darkest corners of Linux driver
history ;-)

I first started out by separating the classing ISA device probing
and the platform driver into separate files. I think these
are useful to split out, because e.g. the PCI driver has no
dependency on either. Having the option to build without the
ISA stuff simplifies a lot of things.

I also made a patch for the mn10300 use of SERIAL_PORT_DFNS,
which is the only architecture that defines non-ISA ports
that way.

Where I failed so far is the dynamic configuration of ports using
setserial. This intentionally allows changing the io_type setting
as well as the actual resources (ioport, mapbase, irq, ...).
Changing the io_type is not supported by /bin/setserial, but
other tools might be doing it.
My question is whether we should still care about those. If
we can remove the reconfiguration of existing ports or move
it to one of the more obscure parts of the driver, it's possible
to confine the dependencies on the ioport_ops to the front-end
drivers, while the core 8250 library driver would not need it
any more.

Today, most ports don't set the UPF_FIXED flag, even though
the ports definitely have fixed resources, e.g. all of the
8250-platform drivers in arch/ or the 8250_pnp and 8250_cs
front-ends. Do you think it would be reasonable to mark all
8250 ports except the ISA ones as UPF_FIXED, and move the
reconfiguration logic into the 8250_isa driver along with
old_serial_port, serial8250_isa_devs,  and
serial8250_isa_init_ports?

	Arnd

Re: [PATCH 7/7] serial/8250: make PIO support optional

[Alan Cox]

> Where I failed so far is the dynamic configuration of ports using
> setserial. This intentionally allows changing the io_type setting
> as well as the actual resources (ioport, mapbase, irq, ...).
> Changing the io_type is not supported by /bin/setserial, but
> other tools might be doing it.
> My question is whether we should still care about those. If
> we can remove the reconfiguration of existing ports or move
> it to one of the more obscure parts of the driver, it's possible
> to confine the dependencies on the ioport_ops to the front-end
> drivers, while the core 8250 library driver would not need it
> any more.

otherwise you need a table that drivers register their port types
in and to take module references on the table entry to pin the relevant
driver code ?
 
> Today, most ports don't set the UPF_FIXED flag, even though
> the ports definitely have fixed resources, e.g. all of the
> 8250-platform drivers in arch/ or the 8250_pnp and 8250_cs
> front-ends. Do you think it would be reasonable to mark all
> 8250 ports except the ISA ones as UPF_FIXED, and move the
> reconfiguration logic into the 8250_isa driver along with
> old_serial_port, serial8250_isa_devs,  and
> serial8250_isa_init_ports?

I suspect one or two people will scream about some peculiar configuration
that should be handled automatically anyway and those are best fixed
properly if so rather than by allowing setserial incantations to work
around stuff like unknown PCI idents

Re: [PATCH 7/7] serial/8250: make PIO support optional

[Arnd Bergmann]

On Monday 04 July 2011 19:02:16 Alan Cox wrote:

> > My question is whether we should still care about those. If
> > we can remove the reconfiguration of existing ports or move
> > it to one of the more obscure parts of the driver, it's possible
> > to confine the dependencies on the ioport_ops to the front-end
> > drivers, while the core 8250 library driver would not need it
> > any more.
> 
> otherwise you need a table that drivers register their port types
> in and to take module references on the table entry to pin the relevant
> driver code ?

I hadn't even thought of that, but yes. I would consider that
worse than leaving both UPIO variants defined in the 8250 core
file after some cleanups, and adding the #ifdef that you object
to.

Yet another option would be to separate out the ioport_ops into
a file for each one, but make them conditionally compiled, with
a direct reference from 8250.o that gets compiled out if no
other front-end ever needs it.

> > Today, most ports don't set the UPF_FIXED flag, even though
> > the ports definitely have fixed resources, e.g. all of the
> > 8250-platform drivers in arch/ or the 8250_pnp and 8250_cs
> > front-ends. Do you think it would be reasonable to mark all
> > 8250 ports except the ISA ones as UPF_FIXED, and move the
> > reconfiguration logic into the 8250_isa driver along with
> > old_serial_port, serial8250_isa_devs,  and
> > serial8250_isa_init_ports?
> 
> I suspect one or two people will scream about some peculiar configuration
> that should be handled automatically anyway and those are best fixed
> properly if so rather than by allowing setserial incantations to work
> around stuff like unknown PCI idents.

Ok, makes sense. If I split this out to the ISA driver, I would
probably also disallow changing io_type, as ISA 8250 ports are
all based on PIO mode.

The only use cases that I can think of for run-time configuration
of MMIO based 8250 ports are the misdetected PCI cards you mentioned
and weird embedded systems that fail to register the port correctly.

Both can be addressed by fixing them at the source.

	Arnd

Re: [PATCH 7/7] serial/8250: make PIO support optional

[Alan Cox]

> Ok, makes sense. If I split this out to the ISA driver, I would
> probably also disallow changing io_type, as ISA 8250 ports are
> all based on PIO mode.

Alas not, they also come in MMIO form now and then, not often but just
enough to be annoyingly unignorable. However the ISA driver can handle
that 'in-house' and I've never seen anything but PIO or simple MMIO.

Alan

Re: [PATCH 7/7] serial/8250: make PIO support optional

[Arnd Bergmann]

On Monday 04 July 2011 21:53:36 Alan Cox wrote:
> > Ok, makes sense. If I split this out to the ISA driver, I would
> > probably also disallow changing io_type, as ISA 8250 ports are
> > all based on PIO mode.
> 
> Alas not, they also come in MMIO form now and then, not often but just
> enough to be annoyingly unignorable. However the ISA driver can handle
> that 'in-house' and I've never seen anything but PIO or simple MMIO.

Ok.

How are people using those ISA cards today? The version of setserial
that I found is not able to set MMIO mode on those. I suppose you
are not talking about isapnp cards, right? Those should not need to
be handled through the ISA driver at all.

	Arnd

Re: [PATCH 7/7] serial/8250: make PIO support optional

[Alan Cox]

> How are people using those ISA cards today? The version of setserial
> that I found is not able to set MMIO mode on those. I suppose you

Various ugly hacks, if its tricky we can drop support and see who screams

> are not talking about isapnp cards, right? Those should not need to
> be handled through the ISA driver at all.

ISAPnP will do it automatically so the ISAPnP case isn't a problem. I've
never sen an ISAPnP port that was anything but standard.

Re: [PATCH 7/7] serial/8250: make PIO support optional

[Arnd Bergmann]

On Monday 04 July 2011 23:55:20 Alan Cox wrote:
> > How are people using those ISA cards today? The version of setserial
> > that I found is not able to set MMIO mode on those. I suppose you
> 
> Various ugly hacks, if its tricky we can drop support and see who screams

I don't think it's much of a problem, I just didn't expect these to exit.

	Arnd

Re: [PATCH 7/7] serial/8250: make PIO support optional

[Alan Cox]

> -		p->serial_in = io_serial_in;
> -		p->serial_out = io_serial_out;
> +		p->serial_in = mem_serial_in;
> +		p->serial_out = mem_serial_out;
>  		break;


And this kind of stuff changing defaults is asking for trouble.

Re: [PATCH 0/7] serial/8250: I/O accessor cleanups

[David Daney]

On 06/27/2011 02:45 PM, Arnd Bergmann wrote:
> Hi Greg,
>
> This series of patches cleans up the part of the 8250 device
> driver that is responsible for accessing the hardware registers.
> The driver defines all sorts of methods to do that right now,
> when it really should only support memory mapped and programmed
> I/O by default, and the latter only on PC-compatible platforms
> including those that have ISA/PCMCIA/PCI buses.
>
> The series shrinks the 8250 driver by about 10% in both binary
> and source code size, hopefully with no loss of functionality,
> and it allows platforms to no longer define bogus inb/outb
> functions when they don't provide CONFIG_HAS_IOPORT. I've
> build-tested for x86 and ARM with and without HAS_IOPORT.
>
> Hopefully Ralf can provide some feedback about the three
> MIPS platforms that have code changed by this.
>
> 	Arnd
>
> Arnd Bergmann (7):
>    serial/8250: remove obsolete RM9000 port type
>    serial/8250: move alchemy I/O handler to platform code
>    serial/8250: move UPIO_TSI to powerpc
>    serial/8250: move DWAP support to arch/mips
>    serial/8250: remove obsolete and broken PORT_RSA support
>    serial/8250: sanitize fourport handling
>    serial/8250: make PIO support optional
>
>   arch/mips/Kconfig                         |    7 -
>   arch/mips/alchemy/common/platform.c       |   50 +++
>   arch/mips/pmc-sierra/msp71xx/msp_serial.c |   32 ++-
>   arch/powerpc/kernel/legacy_serial.c       |   24 ++
>   drivers/tty/serial/8250.c                 |  472 +++--------------------------
>   drivers/tty/serial/8250.h                 |    8 +
>   drivers/tty/serial/8250_hub6.c            |   17 +
>   drivers/tty/serial/Kconfig                |   15 -
>   drivers/tty/serial/serial_core.c          |    4 -
>   drivers/tty/serial/sunsu.c                |   93 ------
>   include/linux/serial.h                    |    2 +-
>   include/linux/serial_core.h               |   10 +-
>   include/linux/serial_reg.h                |   51 ---
>   13 files changed, 171 insertions(+), 614 deletions(-)
>
> Cc: Ralf Baechle<ralf@linux-mips.org>
> Cc: linux-serial@vger.kernel.org
> Cc: Benjamin Herrenschmidt<benh@kernel.crashing.org>
>


FWIW, this was basically the intention when I added the I/O accessor 
functions.

If you like you can add:

Acked-by: David Daney <david.daney@cavium.com>