Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751137AbWBOLAp (ORCPT ); Wed, 15 Feb 2006 06:00:45 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751154AbWBOLAp (ORCPT ); Wed, 15 Feb 2006 06:00:45 -0500 Received: from caramon.arm.linux.org.uk ([212.18.232.186]:45576 "EHLO caramon.arm.linux.org.uk") by vger.kernel.org with ESMTP id S1751137AbWBOLAo (ORCPT ); Wed, 15 Feb 2006 06:00:44 -0500 Date: Wed, 15 Feb 2006 11:00:34 +0000 From: Russell King To: "Steinar H. Gunderson" Cc: linux-kernel@vger.kernel.org, magne@samfundet.no Subject: Re: [PATCH] Problem with detecting the serial ports on a NetMos 9845 Message-ID: <20060215110034.GF21003@flint.arm.linux.org.uk> Mail-Followup-To: "Steinar H. Gunderson" , linux-kernel@vger.kernel.org, magne@samfundet.no References: <20060210174836.GA16968@uio.no> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20060210174836.GA16968@uio.no> User-Agent: Mutt/1.4.1i Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4879 Lines: 127 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/