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

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

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

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.

Diffed against tty-next.

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

Sean Young (2):
  8250_pnp: merge into 8250 module so pnp probe can done first
  8250: blacklist IR ports

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

-- 
1.7.11.4

[PATCH 1/2] 8250_pnp: merge into 8250 module so pnp probe can done first

[Sean Young]

We first poke around and guess where the serial ports are and then check
pnp. If there is a non-standard configuration then this might not work;
if there is an IR device in pnp then we should not attempt to probe it.

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

[PATCH 2/2] 8250: blacklist IR ports

[Sean Young]

These ports should not be probed, but we need to keep track of them so we
can prevent the legacy (non-PNP) probe of serial ports from touching them
too.

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

diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
index bc25874..ea9166b 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_IR] = {
+		.name		= "IR 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_IR)
+		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_IR)
+		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_IR)
+		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_IR) {
 		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..6ab2ce1 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 IR_PORT	0x0800
 
 static const struct pnp_device_id pnp_dev_table[] = {
 	/* Archtek America Corp. */
@@ -362,6 +362,12 @@ static const struct pnp_device_id pnp_dev_table[] = {
 	{	"PNPCXXX",		UNKNOWN_DEV	},
 	/* More unknown PnP modems */
 	{	"PNPDXXX",		UNKNOWN_DEV	},
+	/* IR ports -- should not be probed. We should keep track of them
+	   to prevent the legacy serial driver from probing */
+	{	"WEC1022",		IR_PORT	},
+	{	"NSC6001",		IR_PORT },
+	{	"HWPC224",		IR_PORT },
+	{	"IBM0071",		IR_PORT },
 	{	"",			0	}
 };
 
@@ -409,7 +415,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 +434,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 +442,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 & IR_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 +460,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 & IR_PORT) {
+		uart.port.flags |= UPF_FIXED_PORT | UPF_FIXED_TYPE;
+		uart.port.type = PORT_IR;
+	}
 
 	uart.port.flags |= UPF_SKIP_TEST | UPF_BOOT_AUTOCONF;
 	if (pnp_irq_flags(dev, 0) & IORESOURCE_IRQ_SHAREABLE)
@@ -459,7 +472,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 & IR_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..c7e9edf 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_IR		23	/* 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.11.4

Re: [PATCH 2/2] 8250: blacklist IR ports

[Alan Cox]

On Thu,  6 Sep 2012 23:20:32 +0100
Sean Young <sean@mess.org> wrote:

> These ports should not be probed, but we need to keep track of them so we
> can prevent the legacy (non-PNP) probe of serial ports from touching them
> too.

I want to have a think about getting the ordering right so we don't
shuffle the ports on existing boxes for the first half. For the second
part

> 
> Signed-off-by: Sean Young <sean@mess.org>
> ---
>  drivers/tty/serial/8250/8250.c     | 16 ++++++++++++++--
>  drivers/tty/serial/8250/8250_pnp.c | 23 ++++++++++++++++++-----
>  include/linux/serial_core.h        |  3 ++-
>  3 files changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
> index bc25874..ea9166b 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_IR] = {
> +		.name		= "IR port"

Please use "CIR port". We have SIR and FIR serial ports (IrDA) too that we
do drive via tty. PORT_8250_CIR for the type might be best too - it means
the tools know what they are looking at and can report it sensibly.

Alan

Re: [PATCH 2/2] 8250: blacklist IR ports

[Sean Young]

On Fri, Sep 07, 2012 at 01:01:35AM +0100, Alan Cox wrote:
> On Thu,  6 Sep 2012 23:20:32 +0100
> Sean Young <sean@mess.org> wrote:
> 
> > These ports should not be probed, but we need to keep track of them so we
> > can prevent the legacy (non-PNP) probe of serial ports from touching them
> > too.
> 
> I want to have a think about getting the ordering right so we don't
> shuffle the ports on existing boxes for the first half. For the second
> part

If pnp lists serial devices in a different order than the legacy devices 
or a singular one which is not the same as ttyS0, things probably will look 
different. Not sure what can be done or anything needs to be done.

> > Signed-off-by: Sean Young <sean@mess.org>
> > ---
> >  drivers/tty/serial/8250/8250.c     | 16 ++++++++++++++--
> >  drivers/tty/serial/8250/8250_pnp.c | 23 ++++++++++++++++++-----
> >  include/linux/serial_core.h        |  3 ++-
> >  3 files changed, 34 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
> > index bc25874..ea9166b 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_IR] = {
> > +		.name		= "IR port"
> 
> Please use "CIR port". We have SIR and FIR serial ports (IrDA) too that we
> do drive via tty. PORT_8250_CIR for the type might be best too - it means
> the tools know what they are looking at and can report it sensibly.

At least for the nsc-ircc driver, user space must manually remove the
the uart driver and then load the nsc-ircc driver. This patch also 
fixes that for nsc-ircc devices. It should work although I am unable to 
find the hardware to test it on yet, I used to have one of these. Testers 
would be appreciated. See:

> diff --git a/drivers/tty/serial/8250/8250_pnp.c b/drivers/tty/serial/8250/8250_pnp.c
> index 28bf830..6ab2ce1 100644
> --- a/drivers/tty/serial/8250/8250_pnp.c
> +++ b/drivers/tty/serial/8250/8250_pnp.c
> @@ -362,6 +362,12 @@ static const struct pnp_device_id pnp_dev_table[] = {
>  	{	"PNPCXXX",		UNKNOWN_DEV	},
>  	/* More unknown PnP modems */
>  	{	"PNPDXXX",		UNKNOWN_DEV	},
> +	/* IR ports -- should not be probed. We should keep track of them
> +	   to prevent the legacy serial driver from probing */
> +	{	"WEC1022",		IR_PORT	},

Winbond CIR

> +	{	"NSC6001",		IR_PORT },
> +	{	"HWPC224",		IR_PORT },
> +	{	"IBM0071",		IR_PORT },

nsc-ircc IRDA

>  	{	"",			0	}
>  };

I should have been more explicit about this, possibly in comments. For now 
I am happy to remove those and rename the port PORT_8250_CIR if you prefer.


Sean

Re: [PATCH 2/2] 8250: blacklist IR ports

[Alan Cox]

> At least for the nsc-ircc driver, user space must manually remove the
> the uart driver and then load the nsc-ircc driver. This patch also 

The nsc can be driven in both SIR and FIR mode can it not. The serial
driver should drive it in SIR mode if so which means we need to be a spot
more careful.

> fixes that for nsc-ircc devices. It should work although I am unable to 
> find the hardware to test it on yet, I used to have one of these. Testers 
> would be appreciated. See:

Until it can be tested can we keep the nsc-ircc case separate. At most we
might need two identifiers (PORT_8250_CIR PORT_8250_FIR etc)

Alan

Re: [PATCH 2/2] 8250: blacklist IR ports

[Sean Young]

On Fri, Sep 07, 2012 at 11:25:10AM +0100, Alan Cox wrote:
> > At least for the nsc-ircc driver, user space must manually remove the
> > the uart driver and then load the nsc-ircc driver. This patch also 
> 
> The nsc can be driven in both SIR and FIR mode can it not. The serial
> driver should drive it in SIR mode if so which means we need to be a spot
> more careful.

Oops.

> > fixes that for nsc-ircc devices. It should work although I am unable to 
> > find the hardware to test it on yet, I used to have one of these. Testers 
> > would be appreciated. See:
> 
> Until it can be tested can we keep the nsc-ircc case separate. At most we
> might need two identifiers (PORT_8250_CIR PORT_8250_FIR etc)

Ok. I'll send out new patches over the weekend, after testing.

Thanks,
Sean