Re: [PATCH] Problem with detecting the serial ports on a NetMos 9845

[Russell King <rmk+lkml@arm.linux.org.uk>]

On Fri, Feb 10, 2006 at 06:48:36PM +0100, Steinar H. Gunderson wrote:
> We have a NetMos 9845 controller with four serial ports but no parallel port, 
> and tried to use parport_serial driver with it. However, all it would say was
> that it had detected a parallel port at 9710:9735, and then exit.
> 
> We tracked it down, and found two problems:
> 
>   - For some reason, it detects the 9845 as a 9735 -- it appears this is
>     simply related to the ordering in parport_serial_pci_tbl[]. If we move
>     the 9845 up above the 9735, it prints out 9710:9845, but no change in
>     behaviour. (We didn't find out why this was the case; we left it alone
>     since it didn't affect our problem.)

The driver debugging code is buggy.  Let's look:

enum parport_pc_pci_cards {
        titan_110l = 0,
        titan_210l,
        netmos_9xx5_combo,
        netmos_9855,
...

so, netmos_9xx5_combo has the value '2'.
static struct pci_device_id parport_serial_pci_tbl[] = {
        /* PCI cards */
        { PCI_VENDOR_ID_TITAN, PCI_DEVICE_ID_TITAN_110L,
          PCI_ANY_ID, PCI_ANY_ID, 0, 0, titan_110l },
        { PCI_VENDOR_ID_TITAN, PCI_DEVICE_ID_TITAN_210L,
          PCI_ANY_ID, PCI_ANY_ID, 0, 0, titan_210l },
        { PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9735,
          PCI_ANY_ID, PCI_ANY_ID, 0, 0, netmos_9xx5_combo },

This is the second entry in this table - make a note of that.

        { PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9745,
          PCI_ANY_ID, PCI_ANY_ID, 0, 0, netmos_9xx5_combo },
        { PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9835,
          PCI_ANY_ID, PCI_ANY_ID, 0, 0, netmos_9xx5_combo },
        { PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9835,
          PCI_ANY_ID, PCI_ANY_ID, 0, 0, netmos_9xx5_combo },
        { PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9845,
          PCI_ANY_ID, PCI_ANY_ID, 0, 0, netmos_9xx5_combo },

and this is the entry which your card matches, and uses netmos_9xx5_combo.
...

static int __devinit parport_register (struct pci_dev *dev,
                                       const struct pci_device_id *id)
{
        int i = id->driver_data, n;

id->driver_data is the 7th value in the pci table above.  As we
noted, this is netmos_9xx5_combo which has value '2', so i=2.
...
                printk (KERN_DEBUG "PCI parallel port detected: %04x:%04x, "
                        "I/O at %#lx(%#lx)\n",
                        parport_serial_pci_tbl[i].vendor,
                        parport_serial_pci_tbl[i].device, io_lo, io_hi);

and so we index the pci device id table with something which is an
index to a different table.  parport_serial_pci_tbl[2] happens to
be the Netmos 9735 entry.

> --- linux-source-2.6.12-2.6.12/drivers/parport/parport_serial.c	2005-06-17 21:48:29.000000000 +0200
> @@ -418,10 +419,13 @@
>  		return err;
>  	}
>  
> -	if (parport_register (dev, id)) {
> +	err = parport_register (dev, id);
> +	if (err < 0) {
>  		pci_set_drvdata (dev, NULL);
>  		kfree (priv);
>  		return -ENODEV;
> +	} else if (err) {
> +		priv->num_par = 0;

num_par will be zero here anyway, so this else clause isn't gaining
us anything.  Here's an alternative patch which should also fix your
other issue:

diff --git a/drivers/parport/parport_serial.c b/drivers/parport/parport_serial.c
--- a/drivers/parport/parport_serial.c
+++ b/drivers/parport/parport_serial.c
@@ -312,8 +312,7 @@ static int __devinit parport_register (s
 {
 	struct parport_pc_pci *card;
 	struct parport_serial_private *priv = pci_get_drvdata (dev);
-	int i = id->driver_data, n;
-	int success = 0;
+	int n, success = 0;
 
 	priv->par = cards[id->driver_data];
 	card = &priv->par;
@@ -344,10 +343,8 @@ static int __devinit parport_register (s
                                         "hi" as an offset (see SYBA
                                         def.) */
 		/* TODO: test if sharing interrupts works */
-		printk (KERN_DEBUG "PCI parallel port detected: %04x:%04x, "
-			"I/O at %#lx(%#lx)\n",
-			parport_serial_pci_tbl[i].vendor,
-			parport_serial_pci_tbl[i].device, io_lo, io_hi);
+		dev_dbg(&dev->dev, "PCI parallel port detected: I/O at "
+			"%#lx(%#lx)\n", io_lo, io_hi);
 		port = parport_pc_probe_port (io_lo, io_hi, PARPORT_IRQ_NONE,
 					      PARPORT_DMA_NONE, dev);
 		if (port) {
@@ -359,7 +356,7 @@ static int __devinit parport_register (s
 	if (card->postinit_hook)
 		card->postinit_hook (dev, card, !success);
 
-	return success ? 0 : 1;
+	return 0;
 }
 
 static int __devinit parport_serial_pci_probe (struct pci_dev *dev,


-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core