Netmos parallel/serial/combo support

Here's another iteration of my patch for better Netmos support. This is against 2.6.12-rc1-mm1. Kerry and Darac have sucessfully tested a similar patch with 9835 cards. I think we're ready for wider testing, such as -mm.

There's a bugzilla entry for this here:

    http://bugzilla.kernel.org/show_bug.cgi?id=4334

I'd like to hear about any problems or issues (or even better, about successes, of course :-)). If you encounter a problem, please include the dmesg log and the output of "lspci -vvn".

This should fix all the problems I know about with Netmos combo cards:
- 9735, 9835, and 9855 are not supported
- combo cards with parallel are erroneously claimed by serial driver
- serial and parport_serial blindly probe for ports

parport_pc:
Sort Netmos device IDs, no functional change.
parport_serial:
Previously supported 9735 and 9835. Add 9745, 9845, 9855, and add init hooks to discover how many serial/parallel ports are actually present (the boards are available in various configs). Add protection for overflow of static tables.
quirks:
Detect Netmos combo (parallel + serial) cards and change class from SERIAL to OTHER to prevent serial driver from claiming them.
8250:
Add init hook to discover the number of serial ports present. This prevents us from poking at unused BARs.
pci_ids:

    Add Netmos 9745, 9845, and sort.

diff -u -r 2.6.12-rc1-mm1/drivers/parport/parport_pc.c 2.6.12-rc1-mm1-netmos/drivers/parport/parport_pc.c
@@ -2733,11 +2733,11 @@
 	aks_0100,
 	mobility_pp,
 	netmos_9705,
+	netmos_9715,
+	netmos_9755,
 	netmos_9805,
 	netmos_9815,
 	netmos_9855,
-	netmos_9755,
-	netmos_9715
 };
 
 

@@ -2808,11 +2808,11 @@
 	/* aks_0100 */                  { 1, { { 0, -1 }, } },
 	/* mobility_pp */		{ 1, { { 0, 1 }, } },
 	/* netmos_9705 */               { 1, { { 0, -1 }, } }, /* untested */
+        /* netmos_9715 */               { 2, { { 0, 1 }, { 2, 3 },} }, /* untested */
+        /* netmos_9755 */               { 2, { { 0, 1 }, { 2, 3 },} }, /* untested */
 	/* netmos_9805 */               { 1, { { 0, -1 }, } }, /* untested */
 	/* netmos_9815 */               { 2, { { 0, -1 }, { 2, -1 }, } }, /* untested */
 	/* netmos_9855 */               { 2, { { 0, -1 }, { 2, -1 }, } }, /* untested */
-        /* netmos_9755 */               { 2, { { 0, 1 }, { 2, 3 },} }, /* untested */
-        /* netmos_9715 */               { 2, { { 0, 1 }, { 2, 3 },} }, /* untested */
 };
 
 static struct pci_device_id parport_pc_pci_tbl[] = {

@@ -2885,16 +2885,16 @@
 	/* NetMos communication controllers */
 	{ PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9705,
 	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, netmos_9705 },
+	{ PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9715,
+	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, netmos_9715 },
+	{ PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9755,
+	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, netmos_9755 },
 	{ PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9805,
 	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, netmos_9805 },
 	{ PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9815,
 	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, netmos_9815 },
 	{ PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9855,
 	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, netmos_9855 },
-	{ PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9755,
-	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, netmos_9755 },
-	{ PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9715,
-	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, netmos_9715 },
 	{ 0, } /* terminate list */
 };
 MODULE_DEVICE_TABLE(pci,parport_pc_pci_tbl);

diff -u -r 2.6.12-rc1-mm1/drivers/pci/pci.ids 2.6.12-rc1-mm1-netmos/drivers/pci/pci.ids
@@ -9903,14 +9903,25 @@
 	6565  6565
 9710  NetMos Technology
 	7780  USB IRDA-port
-	9815  PCI 9815 Multi-I/O Controller
+	9705  PCI 9705 Parallel Port
+	9715  PCI 9715 Dual Parallel Port
+	9735  PCI 9735 Multi-I/O Controller
+		1000 0002  0P2S (2 serial)
+		1000 0012  1P2S (1 parallel + 2 serial)
+	9745  PCI 9745 Multi-I/O Controller
+		1000 0002  0P2S (2 serial)
+		1000 0012  1P2S (1 parallel + 2 serial)
+	9755  PCI 9755 Parallel Port and ISA Bridge
+	9805  PCI 9805 Parallel Port
+	9815  PCI 9815 Dual Parallel Port
 		1000 0020  2P0S (2 port parallel adaptor)
 	9835  PCI 9835 Multi-I/O Controller
-		1000 0002  2S (16C550 UART)
+		1000 0002  0P2S (16C550 UART)
 		1000 0012  1P2S
 	9845  PCI 9845 Multi-I/O Controller
 		1000 0004  0P4S (4 port 16550A serial card)
-		1000 0006  0P6S (6 port 16550a serial card)
+		1000 0006  0P6S (6 port 16550A serial card)
+		1000 0014  1P4S (4 port 16550A serial card + parallel)
 	9855  PCI 9855 Multi-I/O Controller
 		1000 0014  1P4S
 9902  Stargen Inc.

diff -u -r 2.6.12-rc1-mm1/drivers/pci/quirks.c 2.6.12-rc1-mm1-netmos/drivers/pci/quirks.c
@@ -1259,6 +1259,40 @@
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7320_MCH,	quirk_pcie_mch );
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7525_MCH,	quirk_pcie_mch );
 
+static void __devinit quirk_netmos(struct pci_dev *dev)
+{
+	unsigned int num_parallel = (dev->subsystem_device & 0xf0) >> 4;
+	unsigned int num_serial = dev->subsystem_device & 0xf;
+
+	/*
+	 * These Netmos parts are multiport serial devices with optional
+	 * parallel ports.  Even when parallel ports are present, they
+	 * are identified as class SERIAL, which means the serial driver
+	 * will claim them.  To prevent this, mark them as class OTHER.
+	 * These combo devices should be claimed by parport_serial.
+	 *
+	 * The subdevice ID is of the form 0x00PS, where <P> is the number
+	 * of parallel ports and <S> is the number of serial ports.
+	 */
+	switch (dev->device) {
+	case PCI_DEVICE_ID_NETMOS_9735:
+	case PCI_DEVICE_ID_NETMOS_9745:
+	case PCI_DEVICE_ID_NETMOS_9835:
+	case PCI_DEVICE_ID_NETMOS_9845:
+	case PCI_DEVICE_ID_NETMOS_9855:
+		if ((dev->class >> 8) == PCI_CLASS_COMMUNICATION_SERIAL &&
+		    num_parallel) {
+			printk(KERN_INFO "PCI: Netmos %04x (%u parallel, "
+				"%u serial); changing class SERIAL to OTHER "
+				"(use parport_serial)\n",
+				dev->device, num_parallel, num_serial);
+			dev->class = (PCI_CLASS_COMMUNICATION_OTHER << 8) |
+			    (dev->class & 0xff);
+		}
+	}
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_NETMOS, PCI_ANY_ID, quirk_netmos);
+
 static void pci_do_fixups(struct pci_dev *dev, struct pci_fixup *f, struct pci_fixup *end)
 {
 	while (f < end) {

diff -u -r 2.6.12-rc1-mm1/drivers/serial/8250_pci.c 2.6.12-rc1-mm1-netmos/drivers/serial/8250_pci.c
@@ -577,6 +577,16 @@
 	return 0;
 }
 
+static int __devinit pci_netmos_init(struct pci_dev *dev)
+{
+	/* subdevice 0x00PS means <P> parallel, <S> serial */
+	unsigned int num_serial = dev->subsystem_device & 0xf;
+
+	if (num_serial == 0)
+		return -ENODEV;
+	return num_serial;
+}
+
 static int
 pci_default_setup(struct pci_dev *dev, struct pci_board *board,
 		  struct uart_port *port, int idx)

@@ -934,6 +944,17 @@
 		.setup		= pci_default_setup,
 	},
 	/*
+	 * Netmos cards
+	 */
+	{
+		.vendor		= PCI_VENDOR_ID_NETMOS,
+		.device		= PCI_ANY_ID,
+		.subvendor	= PCI_ANY_ID,
+		.subdevice	= PCI_ANY_ID,
+		.init		= pci_netmos_init,
+		.setup		= pci_default_setup,
+	},
+	/*
 	 * Default "match everything" terminator entry
 	 */
 	{

diff -u -r 2.6.12-rc1-mm1/include/linux/pci_ids.h 2.6.12-rc1-mm1-netmos/include/linux/pci_ids.h
@@ -2533,13 +2533,15 @@
 
 #define PCI_VENDOR_ID_NETMOS		0x9710
 #define PCI_DEVICE_ID_NETMOS_9705	0x9705
+#define PCI_DEVICE_ID_NETMOS_9715	0x9715
 #define PCI_DEVICE_ID_NETMOS_9735	0x9735
+#define PCI_DEVICE_ID_NETMOS_9745	0x9745
+#define PCI_DEVICE_ID_NETMOS_9755	0x9755
 #define PCI_DEVICE_ID_NETMOS_9805	0x9805
 #define PCI_DEVICE_ID_NETMOS_9815	0x9815
 #define PCI_DEVICE_ID_NETMOS_9835	0x9835
+#define PCI_DEVICE_ID_NETMOS_9845	0x9845
 #define PCI_DEVICE_ID_NETMOS_9855	0x9855
-#define PCI_DEVICE_ID_NETMOS_9755	0x9755
-#define PCI_DEVICE_ID_NETMOS_9715	0x9715
 
 #define PCI_SUBVENDOR_ID_EXSYS		0xd84d
 #define PCI_SUBDEVICE_ID_EXSYS_4014	0x4014

[Hmm... Too large Cc list. Dunno which addresses should be keept...]

So, do you expect 9[78]35 cards to work? ;)

With this patch applied, my 9835 card now works when loading 8250_pci module. But things does not completely work still.

I've a 9835 card with two serial and no parallel ports:

0000:01:00.0 0700: 9710:9835 (rev 01) (prog-if 02)

        Subsystem: 1000:0002
        Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B-
        Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
        Interrupt: pin A routed to IRQ 193
        Region 0: I/O ports at a400 [size=8]
        Region 1: I/O ports at a000 [size=8]
        Region 2: I/O ports at 9800 [size=8]
        Region 3: I/O ports at 9400 [size=8]
        Region 4: I/O ports at 9000 [size=8]
        Region 5: I/O ports at 8800 [size=16]

When I first load 8250_pci, it correctly detects one onboard serial port (ttyS0) and two ports on the card (ttyS4 and ttyS5):

Serial: 8250/16550 driver $Revision: 1.90 $ 8 ports, IRQ sharing enabled ttyS0 at I/O 0x3f8 (irq = 4) is a 16550A
ACPI: PCI interrupt 0000:01:00.0[A] -> GSI 18 (level, low) -> IRQ 193 ttyS4 at I/O 0xa400 (irq = 193) is a 16550A
ttyS5 at I/O 0xa000 (irq = 193) is a 16550A

When I load parport_pc after loading 8250_pci, it correctly detects onboard parallel port and nothing more:

parport: PnPBIOS parport detected.
parport0: PC-style at 0x378, irq 7 [PCSPP]

But after reloading parport_pc, it does not see the built-in port anymore; more, after unloading 8250_pci and 8250, parport_pc finds one parallel port -- on this netmos card only (there's no parallel port on this card):

PCI parallel port detected: 9710:9835, I/O at 0x9800(0x9400)
parport0: PC-style at 0x9800 (0x9400) [PCSPP,TRISTATE]

When parport_pc loaded, 8250[_pci] can't detect the two serial ports it detected previously:

Serial: 8250/16550 driver $Revision: 1.90 $ 8 ports, IRQ sharing enabled ttyS0 at I/O 0x3f8 (irq = 4) is a 16550A

So, when loading 8250_pci first, and parport_pc second (and not unloading anything), things works as expected. When loading parport_pc first (not parport_serial), and 8250_pci after, there will be 2 parallel and only one serial ports:

parport: PnPBIOS parport detected.
parport0: PC-style at 0x378, irq 7 [PCSPP]
ACPI: PCI interrupt 0000:01:00.0[A] -> GSI 18 (level, low) -> IRQ 193 PCI parallel port detected: 9710:9835, I/O at 0x9800(0x9400)
parport1: PC-style at 0x9800 (0x9400) [PCSPP,TRISTATE]
Serial: 8250/16550 driver $Revision: 1.90 $ 8 ports, IRQ sharing enabled ttyS0 at I/O 0x3f8 (irq = 4) is a 16550A

When loading parport_serial only (which loads all the rest), things works as expected (well... it's due to dependencies resolved in a "correct" order - 8250 stuff first and parport_pc second).

The issue with built-in port not being detected after re-loading parport_pc may be unrelated - looks like it is this way without the card inserted.

/mjt

Yes, I expect them all to work. Thanks very much for testing yours!

Everything looks good so far.

Hmmm... Do you have an init script or something that pokes 9835 into /sys/bus/pci/drivers/parport_pc/new_id? If not, I don't see how parport_pc could claim your 9835, since it's not compiled into parport_pc_pci_tbl. It looks like you should be able to turn off the new_id functionality by disabling CONFIG_HOTPLUG.

That's because there's only one PCI device, and parport_pc already claimed it.

Can you add some printks to figure out how parport_pc claims your board? For example, the patch below might be a start:

@@ -2930,10 +2930,13 @@
 		return -ENODEV;
 	}
 
+	printk("parport_pc: dev %s i %d numports %d\n", pci_name(dev), i,
+		cards[i].numports);
 	for (n = 0; n < cards[i].numports; n++) {
 		int lo = cards[i].addr[n].lo;
 		int hi = cards[i].addr[n].hi;
 		unsigned long io_lo, io_hi;
+		printk("parport_pc:   port %d lo %d hi %d\n", n, lo, hi);
 		io_lo = pci_resource_start (dev, lo);
 		io_hi = 0;
 		if ((hi >= 0) && (hi <= 6))

@@ -358,6 +358,8 @@
 	    card->preinit_hook (dev, card, PARPORT_IRQ_NONE, PARPORT_DMA_NONE))
 		return -ENODEV;
 
+	printk("parport_serial: dev %s i %d numports %d\n", pci_name(dev), i,
+		card->numports);
 	for (n = 0; n < card->numports; n++) {
 		struct parport *port;
 		int lo = card->addr[n].lo;

@@ -372,6 +374,7 @@
 			break;
 		}
 
+		printk("parport_serial:   port %d lo %d hi %d\n", n, lo, hi);
 		io_lo = pci_resource_start (dev, lo);
 		io_hi = 0;
 		if ((hi >= 0) && (hi <= 6))

Bjorn Helgaas wrote:
[]

[]

Well, I do see how parport_pc is claiming the device. This is the chunk from your patch I applied manually, or, rather, didn't apply at all because the change was about re-ordering stuff only.

It did not apply because the two IDs - 9735 and 9835 - are still present in 2.6.11 kernel. Your mention about parport_pc_pci_tbl[] made me realize that I applied the patch incorrectly. So I finally found the two devices aren't present in Linus rc1 tree, and removed them here too (so that parport_pc.c reordering of your patch now applies too).

With that change, parport_pc does not claim the device anymore, which is the right behaviour, -- ie, for 9835 card, looks like everything works just fine now. So please treat this my report as a success story about netmos cards support... and thank you for your work!..

The problem with built-in port and parport_pc reloading still exists (or seems to be), even after applying the rest of parport_pc changes which are in Linus rc1. But.. after modifying all those files several times, I can't be sure it wasn't my fault again. I will try to repatch and recompile it all tomorrow. Here's how it looks like now:

# modprobe parport_pc
parport: PnPBIOS parport detected.
parport0: PC-style at 0x378, irq 7 [PCSPP]
# rmmod parport_pc
pnp: Device 00:07 disabled.
# modprobe parport_pc
pnp: Device 00:07 activated.
parport: PnPBIOS parport detected.
pnp: Device 00:07 disabled.
... and so on - no parport0 anymore.

/mjt