Date: Tue, 1 Feb 2005 18:00:48 +0100 (CET)
To: greg@kroah.com, aurelien@aurel32.net
Subject: Re: [PATCH 2.6] I2C: New chip driver: sis5595
Message-ID: <UG87F3dV.1107277247.9606340.khali@localhost>
In-Reply-To: <20050201165434.GD23118@kroah.com>
From: "Jean Delvare" <khali@linux-fr.org>
Bounce-To: "Jean Delvare" <khali@linux-fr.org>
CC: "Alexey Dobriyan" <adobriyan@mail.ru>,
       "LKML" <linux-kernel@vger.kernel.org>,
       "LM Sensors" <sensors@Stimpy.netroedge.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7BIT
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1357
Lines: 40


Hi Greg & all,

> > +/* Locate SiS bridge and correct base address for SIS5595 */
> > +static int sis5595_find_sis(int *address)
> > +{
> > +	u16 val;
> > +	int *i;
> > +
> > +	if (!(s_bridge =
> > +	    pci_get_device(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503, NULL)))
> > +		return -ENODEV;
>
> You never free the reference you grabbed on the pci_dev here.  Please
> read the docs on how pci_get_device() is to be used, it isn't just a
> drop in replacement for pci_find_device(), sorry.
>
> > +	/* Look for imposters */
> > +	for(i = blacklist; *i != 0; i++) {
> > +		if (pci_get_device(PCI_VENDOR_ID_SI, *i, NULL)) {
>
> Same here, you are leaking a reference count.
>
> Why are you not using the pci driver interface instead?

Not sure it exactly answers your question, but the sis5595 much
ressembles the via686a in that respect, i.e. it isn't driving a PCI
device, merely grabbing the ISA I/O address data from PCI configuration
space, then driving that ISA device. I'd guess that whatever was done
in via686a (including the recent updates) should work equally well for
the sis5595.

Thanks,
--
Jean Delvare
-
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/