[PATCH v3 0/2] winbond-cir: Fix initialization

[PATCH v3 0/2] winbond-cir: Fix initialization

[Sean Young]

The serial driver will detect the winbond cir device as a serial port,
since it looks exactly like a serial port unless you know what it is
from the PNP ID.

v2:
Alan Cox pointed out[1] that the pnp serial probe should be done before
the legacy probe and we should keep track of any IR ports during the
pnp scan. This is an implementation of that idea.

[1] http://www.spinics.net/lists/linux-serial/msg06757.html

v3:
 - Removed IrDA entries
 - Renamed PORT_IR to PORT_8250_CIR
 - commit message rewording

Diffed against tty-next.

Sean Young (2):
  8250_pnp: do pnp probe before legacy probe
  8250: blacklist Winbond CIR port

 drivers/tty/serial/8250/8250.c     |   47 ++++++++++++++++++++++++-----------
 drivers/tty/serial/8250/8250.h     |    9 +++++++
 drivers/tty/serial/8250/8250_pnp.c |   31 +++++++++++++----------
 drivers/tty/serial/8250/Kconfig    |   16 ++++++------
 drivers/tty/serial/8250/Makefile   |    5 ++-
 include/linux/serial_core.h        |    3 +-
 6 files changed, 72 insertions(+), 39 deletions(-)

-- 
1.7.2.5

[PATCH v3 1/2] 8250_pnp: do pnp probe before legacy probe

[Sean Young]

We first probe the legacy serial ports and then check pnp. If there
is a non-standard configuration then this might not work, also this
change is needed so we can blacklist Winbond CIR based on PNP ID.

For this to work the 8250_pnp driver must be merged into the 8250
module.

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/tty/serial/8250/8250.c     |   31 ++++++++++++++++++-------------
 drivers/tty/serial/8250/8250.h     |    9 +++++++++
 drivers/tty/serial/8250/8250_pnp.c |   11 +++--------
 drivers/tty/serial/8250/Kconfig    |   16 ++++++++--------
 drivers/tty/serial/8250/Makefile   |    5 +++--
 5 files changed, 41 insertions(+), 31 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
index 44f52c6..bc25874 100644
--- a/drivers/tty/serial/8250/8250.c
+++ b/drivers/tty/serial/8250/8250.c
@@ -2672,6 +2672,9 @@ static void __init serial8250_isa_init_ports(void)
 		return;
 	first = 0;
 
+	if (nr_uarts > UART_NR)
+		nr_uarts = UART_NR;
+
 	for (i = 0; i < nr_uarts; i++) {
 		struct uart_8250_port *up = &serial8250_ports[i];
 		struct uart_port *port = &up->port;
@@ -2681,6 +2684,7 @@ static void __init serial8250_isa_init_ports(void)
 
 		init_timer(&up->timer);
 		up->timer.function = serial8250_timeout;
+		up->cur_iotype = 0xFF;
 
 		/*
 		 * ALPHA_KLUDGE_MCR needs to be killed.
@@ -2732,13 +2736,9 @@ serial8250_register_ports(struct uart_driver *drv, struct device *dev)
 
 	for (i = 0; i < nr_uarts; i++) {
 		struct uart_8250_port *up = &serial8250_ports[i];
-		up->cur_iotype = 0xFF;
-	}
 
-	serial8250_isa_init_ports();
-
-	for (i = 0; i < nr_uarts; i++) {
-		struct uart_8250_port *up = &serial8250_ports[i];
+		if (up->port.dev)
+			continue;
 
 		up->port.dev = dev;
 
@@ -2863,9 +2863,6 @@ static struct console serial8250_console = {
 
 static int __init serial8250_console_init(void)
 {
-	if (nr_uarts > UART_NR)
-		nr_uarts = UART_NR;
-
 	serial8250_isa_init_ports();
 	register_console(&serial8250_console);
 	return 0;
@@ -3148,7 +3145,8 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
 
 	uart = serial8250_find_match_or_unused(&up->port);
 	if (uart) {
-		uart_remove_one_port(&serial8250_reg, &uart->port);
+		if (uart->port.dev)
+			uart_remove_one_port(&serial8250_reg, &uart->port);
 
 		uart->port.iobase       = up->port.iobase;
 		uart->port.membase      = up->port.membase;
@@ -3232,8 +3230,7 @@ static int __init serial8250_init(void)
 {
 	int ret;
 
-	if (nr_uarts > UART_NR)
-		nr_uarts = UART_NR;
+	serial8250_isa_init_ports();
 
 	printk(KERN_INFO "Serial: 8250/16550 driver, "
 		"%d ports, IRQ sharing %sabled\n", nr_uarts,
@@ -3248,11 +3245,15 @@ static int __init serial8250_init(void)
 	if (ret)
 		goto out;
 
+	ret = serial8250_pnp_init();
+	if (ret)
+		goto unreg_uart_drv;
+
 	serial8250_isa_devs = platform_device_alloc("serial8250",
 						    PLAT8250_DEV_LEGACY);
 	if (!serial8250_isa_devs) {
 		ret = -ENOMEM;
-		goto unreg_uart_drv;
+		goto unreg_pnp;
 	}
 
 	ret = platform_device_add(serial8250_isa_devs);
@@ -3268,6 +3269,8 @@ static int __init serial8250_init(void)
 	platform_device_del(serial8250_isa_devs);
 put_dev:
 	platform_device_put(serial8250_isa_devs);
+unreg_pnp:
+	serial8250_pnp_exit();
 unreg_uart_drv:
 #ifdef CONFIG_SPARC
 	sunserial_unregister_minors(&serial8250_reg, UART_NR);
@@ -3292,6 +3295,8 @@ static void __exit serial8250_exit(void)
 	platform_driver_unregister(&serial8250_isa_driver);
 	platform_device_unregister(isa_dev);
 
+	serial8250_pnp_exit();
+
 #ifdef CONFIG_SPARC
 	sunserial_unregister_minors(&serial8250_reg, UART_NR);
 #else
diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 0c5e908..5a76f9c 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -97,3 +97,12 @@ static inline void serial_dl_write(struct uart_8250_port *up, int value)
 #else
 #define ALPHA_KLUDGE_MCR 0
 #endif
+
+#ifdef CONFIG_SERIAL_8250_PNP
+int serial8250_pnp_init(void);
+void serial8250_pnp_exit(void);
+#else
+static inline int serial8250_pnp_init(void) { return 0; }
+static inline void serial8250_pnp_exit(void) { }
+#endif
+
diff --git a/drivers/tty/serial/8250/8250_pnp.c b/drivers/tty/serial/8250/8250_pnp.c
index fde5aa6..28bf830 100644
--- a/drivers/tty/serial/8250/8250_pnp.c
+++ b/drivers/tty/serial/8250/8250_pnp.c
@@ -1,5 +1,5 @@
 /*
- *  Probe module for 8250/16550-type ISAPNP serial ports.
+ *  Probe for 8250/16550-type ISAPNP serial ports.
  *
  *  Based on drivers/char/serial.c, by Linus Torvalds, Theodore Ts'o.
  *
@@ -507,18 +507,13 @@ static struct pnp_driver serial_pnp_driver = {
 	.id_table	= pnp_dev_table,
 };
 
-static int __init serial8250_pnp_init(void)
+int serial8250_pnp_init(void)
 {
 	return pnp_register_driver(&serial_pnp_driver);
 }
 
-static void __exit serial8250_pnp_exit(void)
+void serial8250_pnp_exit(void)
 {
 	pnp_unregister_driver(&serial_pnp_driver);
 }
 
-module_init(serial8250_pnp_init);
-module_exit(serial8250_pnp_exit);
-
-MODULE_LICENSE("GPL");
-MODULE_DESCRIPTION("Generic 8250/16x50 PnP serial driver");
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index a27dd05..f3d283f 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -33,6 +33,14 @@ config SERIAL_8250
 	  Most people will say Y or M here, so that they can use serial mice,
 	  modems and similar devices connecting to the standard serial ports.
 
+config SERIAL_8250_PNP
+	bool "8250/16550 PNP device support" if EXPERT
+	depends on SERIAL_8250 && PNP
+	default y
+	---help---
+	  This builds standard PNP serial support. You may be able to
+	  disable this feature if you only need legacy serial support.
+
 config SERIAL_8250_CONSOLE
 	bool "Console on 8250/16550 and compatible serial port"
 	depends on SERIAL_8250=y
@@ -85,14 +93,6 @@ config SERIAL_8250_PCI
 	  disable this feature if you only need legacy serial support.
 	  Saves about 9K.
 
-config SERIAL_8250_PNP
-	tristate "8250/16550 PNP device support" if EXPERT
-	depends on SERIAL_8250 && PNP
-	default SERIAL_8250
-	help
-	  This builds standard PNP serial support. You may be able to
-	  disable this feature if you only need legacy serial support.
-
 config SERIAL_8250_HP300
 	tristate
 	depends on SERIAL_8250 && HP300
diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
index d7533c7..108fe7f 100644
--- a/drivers/tty/serial/8250/Makefile
+++ b/drivers/tty/serial/8250/Makefile
@@ -2,8 +2,9 @@
 # Makefile for the 8250 serial device drivers.
 #
 
-obj-$(CONFIG_SERIAL_8250)		+= 8250.o
-obj-$(CONFIG_SERIAL_8250_PNP)		+= 8250_pnp.o
+obj-$(CONFIG_SERIAL_8250)		+= 8250_core.o
+8250_core-y				:= 8250.o
+8250_core-$(CONFIG_SERIAL_8250_PNP)	+= 8250_pnp.o
 obj-$(CONFIG_SERIAL_8250_GSC)		+= 8250_gsc.o
 obj-$(CONFIG_SERIAL_8250_PCI)		+= 8250_pci.o
 obj-$(CONFIG_SERIAL_8250_HP300)		+= 8250_hp300.o
-- 
1.7.2.5

[PATCH v3 2/2] 8250: blacklist Winbond CIR port

[Sean Young]

The legacy serial driver will detect the Winbond CIR device as a serial
port, since it looks exactly like a serial port unless you know what
it is from the PNP ID.

Here we track this port as a special PORT_8250_CIR type, preventing the
legacy serial driver from probing it.

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/tty/serial/8250/8250.c     |   16 ++++++++++++++--
 drivers/tty/serial/8250/8250_pnp.c |   20 +++++++++++++++-----
 include/linux/serial_core.h        |    3 ++-
 3 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
index bc25874..5eaa6ba 100644
--- a/drivers/tty/serial/8250/8250.c
+++ b/drivers/tty/serial/8250/8250.c
@@ -290,6 +290,9 @@ static const struct serial8250_config uart_config[] = {
 				  UART_FCR_R_TRIG_00 | UART_FCR_T_TRIG_00,
 		.flags		= UART_CAP_FIFO,
 	},
+	[PORT_8250_CIR] = {
+		.name		= "CIR port"
+	}
 };
 
 /* Uart divisor latch read */
@@ -1897,6 +1900,9 @@ static int serial8250_startup(struct uart_port *port)
 	unsigned char lsr, iir;
 	int retval;
 
+	if (port->type == PORT_8250_CIR)
+		return -ENODEV;
+
 	port->fifosize = uart_config[up->port.type].fifo_size;
 	up->tx_loadsz = uart_config[up->port.type].tx_loadsz;
 	up->capabilities = uart_config[up->port.type].flags;
@@ -2554,7 +2560,10 @@ 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;
+	int ret;
+
+	if (port->type == PORT_8250_CIR)
+		return -ENODEV;
 
 	ret = serial8250_request_std_resource(up);
 	if (ret == 0 && port->type == PORT_RSA) {
@@ -2573,6 +2582,9 @@ static void serial8250_config_port(struct uart_port *port, int flags)
 	int probeflags = PROBE_ANY;
 	int ret;
 
+	if (port->type == PORT_8250_CIR)
+		return;
+
 	/*
 	 * Find the region that we can probe for.  This in turn
 	 * tells us whether we can probe for the type of port.
@@ -3144,7 +3156,7 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
 	mutex_lock(&serial_mutex);
 
 	uart = serial8250_find_match_or_unused(&up->port);
-	if (uart) {
+	if (uart && uart->port.type != PORT_8250_CIR) {
 		if (uart->port.dev)
 			uart_remove_one_port(&serial8250_reg, &uart->port);
 
diff --git a/drivers/tty/serial/8250/8250_pnp.c b/drivers/tty/serial/8250/8250_pnp.c
index 28bf830..f8ee250 100644
--- a/drivers/tty/serial/8250/8250_pnp.c
+++ b/drivers/tty/serial/8250/8250_pnp.c
@@ -25,7 +25,7 @@
 #include "8250.h"
 
 #define UNKNOWN_DEV 0x3000
-
+#define CIR_PORT	0x0800
 
 static const struct pnp_device_id pnp_dev_table[] = {
 	/* Archtek America Corp. */
@@ -362,6 +362,9 @@ static const struct pnp_device_id pnp_dev_table[] = {
 	{	"PNPCXXX",		UNKNOWN_DEV	},
 	/* More unknown PnP modems */
 	{	"PNPDXXX",		UNKNOWN_DEV	},
+	/* Winbond CIR port, should not be probed. We should keep track
+	   of it to prevent the legacy serial driver from probing it */
+	{	"WEC1022",		CIR_PORT	},
 	{	"",			0	}
 };
 
@@ -409,7 +412,7 @@ static int __devinit check_resources(struct pnp_dev *dev)
  * PnP modems, alternatively we must hardcode all modems in pnp_devices[]
  * table.
  */
-static int __devinit serial_pnp_guess_board(struct pnp_dev *dev, int *flags)
+static int __devinit serial_pnp_guess_board(struct pnp_dev *dev)
 {
 	if (!(check_name(pnp_dev_name(dev)) ||
 		(dev->card && check_name(dev->card->name))))
@@ -428,7 +431,7 @@ serial_pnp_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
 	int ret, line, flags = dev_id->driver_data;
 
 	if (flags & UNKNOWN_DEV) {
-		ret = serial_pnp_guess_board(dev, &flags);
+		ret = serial_pnp_guess_board(dev);
 		if (ret < 0)
 			return ret;
 	}
@@ -436,7 +439,10 @@ serial_pnp_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
 	memset(&uart, 0, sizeof(uart));
 	if (pnp_irq_valid(dev, 0))
 		uart.port.irq = pnp_irq(dev, 0);
-	if (pnp_port_valid(dev, 0)) {
+	if ((flags & CIR_PORT) && pnp_port_valid(dev, 2)) {
+		uart.port.iobase = pnp_port_start(dev, 2);
+		uart.port.iotype = UPIO_PORT;
+	} else if (pnp_port_valid(dev, 0)) {
 		uart.port.iobase = pnp_port_start(dev, 0);
 		uart.port.iotype = UPIO_PORT;
 	} else if (pnp_mem_valid(dev, 0)) {
@@ -451,6 +457,10 @@ serial_pnp_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
 		"Setup PNP port: port %x, mem 0x%lx, irq %d, type %d\n",
 		       uart.port.iobase, uart.port.mapbase, uart.port.irq, uart.port.iotype);
 #endif
+	if (flags & CIR_PORT) {
+		uart.port.flags |= UPF_FIXED_PORT | UPF_FIXED_TYPE;
+		uart.port.type = PORT_8250_CIR;
+	}
 
 	uart.port.flags |= UPF_SKIP_TEST | UPF_BOOT_AUTOCONF;
 	if (pnp_irq_flags(dev, 0) & IORESOURCE_IRQ_SHAREABLE)
@@ -459,7 +469,7 @@ serial_pnp_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
 	uart.port.dev = &dev->dev;
 
 	line = serial8250_register_8250_port(&uart);
-	if (line < 0)
+	if (line < 0 || (flags & CIR_PORT))
 		return -ENODEV;
 
 	pnp_set_drvdata(dev, (void *)((long)line + 1));
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 7cf0b68..bb01003 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -48,7 +48,8 @@
 #define PORT_TEGRA	20	/* NVIDIA Tegra internal UART */
 #define PORT_XR17D15X	21	/* Exar XR17D15x UART */
 #define PORT_LPC3220	22	/* NXP LPC32xx SoC "Standard" UART */
-#define PORT_MAX_8250	22	/* max port ID */
+#define PORT_8250_CIR	23	/* CIR infrared port, has its own driver */
+#define PORT_MAX_8250	23	/* max port ID */
 
 /*
  * ARM specific type numbers.  These are not currently guaranteed
-- 
1.7.2.5

Re: [PATCH v3 0/2] winbond-cir: Fix initialization

[Sean Young]

Hi Alan,

On Fri, Sep 07, 2012 at 07:06:22PM +0100, Sean Young wrote:
> The serial driver will detect the winbond cir device as a serial port,
> since it looks exactly like a serial port unless you know what it is
> from the PNP ID.
> 
> v2:
> Alan Cox pointed out[1] that the pnp serial probe should be done before
> the legacy probe and we should keep track of any IR ports during the
> pnp scan. This is an implementation of that idea.
> 
> [1] http://www.spinics.net/lists/linux-serial/msg06757.html
> 
> v3:
>  - Removed IrDA entries
>  - Renamed PORT_IR to PORT_8250_CIR
>  - commit message rewording

It's been more than two weeks. Do you have any review comments? It
would be great to have this in 3.7.

Thanks
Sean

> Diffed against tty-next.
> 
> Sean Young (2):
>   8250_pnp: do pnp probe before legacy probe
>   8250: blacklist Winbond CIR port
> 
>  drivers/tty/serial/8250/8250.c     |   47 ++++++++++++++++++++++++-----------
>  drivers/tty/serial/8250/8250.h     |    9 +++++++
>  drivers/tty/serial/8250/8250_pnp.c |   31 +++++++++++++----------
>  drivers/tty/serial/8250/Kconfig    |   16 ++++++------
>  drivers/tty/serial/8250/Makefile   |    5 ++-
>  include/linux/serial_core.h        |    3 +-
>  6 files changed, 72 insertions(+), 39 deletions(-)
> 
> -- 
> 1.7.2.5
> 
> --
> 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 v3 0/2] winbond-cir: Fix initialization

[Alan Cox]

> > 
> > [1] http://www.spinics.net/lists/linux-serial/msg06757.html
> > 
> > v3:
> >  - Removed IrDA entries
> >  - Renamed PORT_IR to PORT_8250_CIR
> >  - commit message rewording
> 
> It's been more than two weeks. Do you have any review comments? It
> would be great to have this in 3.7.

Agreed - I'd been trying to see if there was a nicer way to split the
PnP and non PnP stuff cleanly but no inspiration has appeared

Acked-by: Alan Cox <alan@linux.intel.com>