[I'm not subscribed to linux-kernel, so please Cc me on any replies -- I
tried posting this to linux-serial in December, but never got any reply, so
I guess nobody's reading the list.]
Hi,
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 card has no parallel port (at least no physical ones), yet it reports
(via its subsystem ID of 0x0014) one parallel port and four serial ports.
The probe for the parallel port fails, and the driver just aborts. Thus,
it doesn't find the serial ports.
We tested this under both 2.6.12 and 2.6.15, and both had the same problem.
The attached patch (against 2.6.12) changes the logic for the parallel port
detection: If there are detected parallel ports but the probe fails, the
number of parallel ports is simply set to zero, and the probe continues as
usual. This makes it work correctly in our case, and should not present a
problem for other systems.
/* Steinar */
--
Homepage: http://www.sesse.net/
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
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 [email protected]
> 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