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

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

Hello?  Did this patch fix all the reported problems?

On Wed, Feb 15, 2006 at 11:00:34AM +0000, Russell King wrote:
> 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
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

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