From: Jeff Garzik <jeff@garzik.org>
To: surya.prabhakar@wipro.com
Cc: caglar@pardus.org.tr, linux-kernel@vger.kernel.org,
kkeil@suse.de, kai.germaschewski@gmx.de,
isdn4linux@listserv.isdn4linux.de, akpm@linux-foundation.org,
alan@lxorguk.ukuu.org.uk
Subject: Re: [PATCH 1/12] drivers/isdn/hisax/avm_pci.c: replace pci_find_device with pci_get_device
Date: Sun, 15 Jul 2007 00:59:25 -0400 [thread overview]
Message-ID: <4699A9AD.2050206@garzik.org> (raw)
In-Reply-To: <A94AD757879CE142B7CEBC3E6FF5D3EC015DAC49@BLR-EC-MBX02.wipro.com>
surya.prabhakar@wipro.com 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
next prev parent reply other threads:[~2007-07-15 4:59 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-07-13 22:44 [PATCH 1/12] drivers/isdn/hisax/avm_pci.c: replace pci_find_device with pci_get_device S.Çağlar Onur
2007-07-15 0:40 ` Jeff Garzik
[not found] ` <A94AD757879CE142B7CEBC3E6FF5D3EC015DAC49@BLR-EC-MBX02.wipro.com>
2007-07-15 4:59 ` Jeff Garzik [this message]
2007-07-15 7:40 ` Jeff Garzik
2007-07-15 10:11 ` Surya Prabhakar N
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4699A9AD.2050206@garzik.org \
--to=jeff@garzik.org \
--cc=akpm@linux-foundation.org \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=caglar@pardus.org.tr \
--cc=isdn4linux@listserv.isdn4linux.de \
--cc=kai.germaschewski@gmx.de \
--cc=kkeil@suse.de \
--cc=linux-kernel@vger.kernel.org \
--cc=surya.prabhakar@wipro.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox