[PATCH] Fix Winbond CIR driver initialisation

[PATCH] Fix Winbond CIR driver initialisation

[Sean Young]

Booting a vanilla kernel results in:

[    4.771256] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
[    4.771365] serial8250: ttyS0 at I/O 0x3f8 (irq = 4) is a 16550A
[    4.771452] serial8250: ttyS1 at I/O 0x2f8 (irq = 3) is a NS16550A
[    4.771674] 00:03: ttyS0 at I/O 0x3f8 (irq = 4) is a 16550A

Which prevents the winbond-cir driver from initialising since ttyS1 is 
a bastardised serial port, which can only be used for IR using the 
winbond-cir driver.

[   13.947470] Winbond CIR 00:03: Region 0x2f8-0x2ff already in use!
[   13.947741] Winbond CIR 00:03: disabled
[   13.947746] Winbond CIR: probe of 00:03 failed with error -16

A workaround is to boot with argument 8250.nr_uarts=1. This is not really 
desirable, so this patch makes ttyS1 go away.

This is tested on an Intel DG45ID motherboard with the real serial port 
enabled and disabled in the BIOS.

Signed-off-by: Sean Young <sean@mess.org>
---
diff --git a/drivers/input/misc/winbond-cir.c b/drivers/input/misc/winbond-cir.c
index 64f1de7..3da8b49 100644
--- a/drivers/input/misc/winbond-cir.c
+++ b/drivers/input/misc/winbond-cir.c
@@ -57,6 +57,7 @@
 #include <linux/bitrev.h>
 #include <linux/bitops.h>
 #include <linux/slab.h>
+#include <linux/serial_8250.h>
 
 #define DRVNAME "winbond-cir"
 
@@ -1392,10 +1393,22 @@ wbcir_probe(struct pnp_dev *device, const struct pnp_device_id *dev_id)
 	}
 
 	if (!request_region(data->sbase, SP_IOMEM_LEN, DRVNAME)) {
-		dev_err(dev, "Region 0x%lx-0x%lx already in use!\n",
-			data->sbase, data->sbase + SP_IOMEM_LEN - 1);
-		err = -EBUSY;
-		goto exit_release_ebase;
+		bool ok = false;
+#ifdef CONFIG_SERIAL_8250
+		/*
+		 * This device is a hacked serial port which will be
+		 * detected by the serial 8250 driver.
+		 */
+		dev_info(device, "Unregistering phony serial port ttyS1\n");
+		serial8250_unregister_port(1);
+		ok = request_region(data->sbase, SP_IOMEM_LEN, DRVNAME);
+#endif
+		if (!ok) {
+			dev_err(dev, "Region 0x%lx-0x%lx already in use!\n",
+				data->sbase, data->sbase + SP_IOMEM_LEN - 1);
+			err = -EBUSY;
+			goto exit_release_ebase;
+		}
 	}
 
 	err = request_irq(data->irq, wbcir_irq_handler,

Re: [PATCH] Fix Winbond CIR driver initialisation

[Alan Cox]

> A workaround is to boot with argument 8250.nr_uarts=1. This is not really 
> desirable, so this patch makes ttyS1 go away.

setserial can also be used for this surely ?

> +		dev_info(device, "Unregistering phony serial port ttyS1\n");
> +		serial8250_unregister_port(1);
> +		ok = request_region(data->sbase, SP_IOMEM_LEN, DRVNAME);

That's a hack that is only going to work on specific systems where it is
mapped the way you expect and doing stuff behind the back of the serial
driver.

I'm not averse to a better solution but it needs to be general and
maintainable. Is there a way to identify the presence of the windbond CIR
device as opposed to an 8250 ?

Alan

Re: [PATCH] Fix Winbond CIR driver initialisation

[David Härdeman]

On Sun, Nov 07, 2010 at 03:46:48PM +0000, Alan Cox wrote:
> > A workaround is to boot with argument 8250.nr_uarts=1. This is not really 
> > desirable, so this patch makes ttyS1 go away.
> 
> setserial can also be used for this surely ?

That's about as user-unfriendly as the boot option.

> > +		dev_info(device, "Unregistering phony serial port ttyS1\n");
> > +		serial8250_unregister_port(1);
> > +		ok = request_region(data->sbase, SP_IOMEM_LEN, DRVNAME);
> 
> That's a hack that is only going to work on specific systems where it is
> mapped the way you expect and doing stuff behind the back of the serial
> driver.
> 
> I'm not averse to a better solution but it needs to be general and
> maintainable. Is there a way to identify the presence of the windbond CIR
> device as opposed to an 8250 ?

Not sure which kind of identification you're looking for but would a PNP
id be sufficient?

-- 
David Härdeman

Re: [PATCH] Fix Winbond CIR driver initialisation

[Sean Young]

On Sun, Nov 07, 2010 at 03:46:48PM +0000, Alan Cox wrote:
> > A workaround is to boot with argument 8250.nr_uarts=1. This is not really 
> > desirable, so this patch makes ttyS1 go away.
> 
> setserial can also be used for this surely ?
> 
> > +		dev_info(device, "Unregistering phony serial port ttyS1\n");
> > +		serial8250_unregister_port(1);
> > +		ok = request_region(data->sbase, SP_IOMEM_LEN, DRVNAME);
> 
> That's a hack that is only going to work on specific systems where it is
> mapped the way you expect and doing stuff behind the back of the serial
> driver.
> 
> I'm not averse to a better solution but it needs to be general and
> maintainable. Is there a way to identify the presence of the windbond CIR
> device as opposed to an 8250 ?

The past couple of days I've been trying to find a way of identifying the 
winbond cir device correctly in the 8250 driver by its registers, but so 
far I've had no luck. It is a device which can operate in CEIR, UART and 
one the irda modes, but all I can find for identifying what is physically
connected is querying the Super I/O model or the PNP identifier for CIR.

The Super I/O model might not be enough in cases where different things
can be connected to it physically.

The hardware can be flipped between various IR (SIR/CIR/etc) and normal
uart modes. We could have an new call which represents this a bit
more closely.

To ensure it is mapped as expected, the call should include the I/O 
port range and irq.

  bool serial8250_port_non_uart(int ioport, int ioportlen, int irq);

The 8250 serial driver would unregister the port which matches this, and
unregister the corresponding port, returning true if such a port can be
found.

This problem here is also one which is encountered by some of the irda 
drivers, so it would be nice to solve as well for those too.

Would that make more sense?

Regards,
Sean