Re: [PATCH 1/12] drivers/isdn/hisax/avm_pci.c: replace pci_find_devicewith pci_get_device

From: Jeff Garzik
Date: Sun Jul 15 2007 - 00:59:44 EST


surya.prabhakar@xxxxxxxxx wrote:


S.Çaglar Onur wrote:
> @@ -858,5 +858,10 @@ ready:
> cs->irq_func = &avm_pcipnp_interrupt;
> cs->writeisac(cs, ISAC_MASK, 0xFF);
> ISACVersion(cs, (cs->subtyp == AVM_FRITZ_PCI) ? "AVM PCI:" : "AVM PnP:");
> + pci_dev_put(dev_avm);
> return (1);
> +
> +dev_avm_cleanup:
> + pci_dev_put(dev_avm);
> + return (0);
> }


>NAK -- every single one of these patches is wrong. All you did was make
>the warning go away, while INTRODUCING new lifetime problems.

>The ISDN PCI driver obviously continues execution after the setup
>function ends, yet you pci_dev_put() the device at the end of setup. As
>a result, no reference is held even though we continue using the device.

This series of patches were initially released by me and were also accepted by kkeil.
It was ack'ed by keil and is now in -mm tree list of andrew morton.
http://marc.info/?l=linux-kernel&m=118436681515269&w=2 <http://marc.info/?l=linux-kernel&m=118436681515269&w=2>

There are many other drivers with similar implementation. Please have a look at
linux2.6/drivers/char/sonypi.c , ide/pci/cs5530.c etc..

Moreover my earlier patch for sound/oss/trident.c was ack'ed by Alan cox and is now in the
current kernel with similar implementation.

Simple question: where is a reference -held-, when the PCI device is in use?

If you cannot show this, your patch is provably wrong. And I did not see a reference kept in any of your patches.

If you can show me where a reference is stored, I rescind my NAK. Otherwise, the patches are demonstrably, provably incorrect.

ACKs cannot get around the -facts-.

Jeff



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/