[PATCH] sworks-agp: Switch to PCI ref counting APIs

[PATCH] sworks-agp: Switch to PCI ref counting APIs

[Alan Cox]

Signed-off-by: Alan Cox <alan@redhat.com>

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.21-rc6-mm1/drivers/char/agp/sworks-agp.c linux-2.6.21-rc6-mm1/drivers/char/agp/sworks-agp.c
--- linux.vanilla-2.6.21-rc6-mm1/drivers/char/agp/sworks-agp.c	2007-04-12 14:15:03.000000000 +0100
+++ linux-2.6.21-rc6-mm1/drivers/char/agp/sworks-agp.c	2007-04-23 11:58:02.342524528 +0100
@@ -455,15 +455,6 @@
 	u32 temp, temp2;
 	u8 cap_ptr = 0;
 
-	/* Everything is on func 1 here so we are hardcoding function one */
-	bridge_dev = pci_find_slot((unsigned int)pdev->bus->number,
-			PCI_DEVFN(0, 1));
-	if (!bridge_dev) {
-		printk(KERN_INFO PFX "Detected a Serverworks chipset "
-		       "but could not find the secondary device.\n");
-		return -ENODEV;
-	}
-
 	cap_ptr = pci_find_capability(pdev, PCI_CAP_ID_AGP);
 
 	switch (pdev->device) {
@@ -483,6 +474,15 @@
 		return -ENODEV;
 	}
 
+	/* Everything is on func 1 here so we are hardcoding function one */
+	bridge_dev = pci_get_bus_and_slot((unsigned int)pdev->bus->number,
+			PCI_DEVFN(0, 1));
+	if (!bridge_dev) {
+		printk(KERN_INFO PFX "Detected a Serverworks chipset "
+		       "but could not find the secondary device.\n");
+		return -ENODEV;
+	}
+
 	serverworks_private.svrwrks_dev = bridge_dev;
 	serverworks_private.gart_addr_ofs = 0x10;
 
@@ -515,7 +515,7 @@
 
 	bridge->driver = &sworks_driver;
 	bridge->dev_private_data = &serverworks_private,
-	bridge->dev = pdev;
+	bridge->dev = pci_dev_get(pdev);
 
 	pci_set_drvdata(pdev, bridge);
 	return agp_add_bridge(bridge);
@@ -525,8 +525,11 @@
 {
 	struct agp_bridge_data *bridge = pci_get_drvdata(pdev);
 
+	pci_dev_put(bridge->dev);
 	agp_remove_bridge(bridge);
 	agp_put_bridge(bridge);
+	pci_dev_put(serverworks_private.svrwrks_dev)
+	serverworks_private.svrwrks_dev = NULL;
 }
 
 static struct pci_device_id agp_serverworks_pci_table[] = {

Re: [PATCH] sworks-agp: Switch to PCI ref counting APIs

[Andrew Morton]

On Mon, 23 Apr 2007 14:51:29 +0100 Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

>  {
>  	struct agp_bridge_data *bridge = pci_get_drvdata(pdev);
>  
> +	pci_dev_put(bridge->dev);
>  	agp_remove_bridge(bridge);
>  	agp_put_bridge(bridge);
> +	pci_dev_put(serverworks_private.svrwrks_dev)
> +	serverworks_private.svrwrks_dev = NULL;

err, guys?

Re: [PATCH] sworks-agp: Switch to PCI ref counting APIs

[Dave Jones]

On Wed, Apr 25, 2007 at 07:21:58PM -0700, Andrew Morton wrote:
 > On Mon, 23 Apr 2007 14:51:29 +0100 Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
 > 
 > >  {
 > >  	struct agp_bridge_data *bridge = pci_get_drvdata(pdev);
 > >  
 > > +	pci_dev_put(bridge->dev);
 > >  	agp_remove_bridge(bridge);
 > >  	agp_put_bridge(bridge);
 > > +	pci_dev_put(serverworks_private.svrwrks_dev)
 > > +	serverworks_private.svrwrks_dev = NULL;
 > 
 > err, guys?

? One put for the agp bridge, one for the host bridge.
What am I missing?

	Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH] sworks-agp: Switch to PCI ref counting APIs

[Andrew Morton]

On Thu, 26 Apr 2007 00:20:19 -0400 Dave Jones <davej@redhat.com> wrote:

> On Wed, Apr 25, 2007 at 07:21:58PM -0700, Andrew Morton wrote:
>  > On Mon, 23 Apr 2007 14:51:29 +0100 Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>  > 
>  > >  {
>  > >  	struct agp_bridge_data *bridge = pci_get_drvdata(pdev);
>  > >  
>  > > +	pci_dev_put(bridge->dev);
>  > >  	agp_remove_bridge(bridge);
>  > >  	agp_put_bridge(bridge);
>  > > +	pci_dev_put(serverworks_private.svrwrks_dev)
>  > > +	serverworks_private.svrwrks_dev = NULL;
>  > 
>  > err, guys?
> 
> ? One put for the agp bridge, one for the host bridge.
> What am I missing?
> 

a semicolon.

Re: [PATCH] sworks-agp: Switch to PCI ref counting APIs

[Dave Jones]

On Wed, Apr 25, 2007 at 09:23:39PM -0700, Andrew Morton wrote:
 > On Thu, 26 Apr 2007 00:20:19 -0400 Dave Jones <davej@redhat.com> wrote:
 > 
 > > On Wed, Apr 25, 2007 at 07:21:58PM -0700, Andrew Morton wrote:
 > >  > On Mon, 23 Apr 2007 14:51:29 +0100 Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
 > >  > 
 > >  > >  {
 > >  > >  	struct agp_bridge_data *bridge = pci_get_drvdata(pdev);
 > >  > >  
 > >  > > +	pci_dev_put(bridge->dev);
 > >  > >  	agp_remove_bridge(bridge);
 > >  > >  	agp_put_bridge(bridge);
 > >  > > +	pci_dev_put(serverworks_private.svrwrks_dev)
 > >  > > +	serverworks_private.svrwrks_dev = NULL;
 > >  > 
 > >  > err, guys?
 > > 
 > > ? One put for the agp bridge, one for the host bridge.
 > > What am I missing?
 > > 
 > 
 > a semicolon.

Yow. I thought I build tested that.
I'll regenerate the git tree tomorrow. Same goes for the cpufreq
tree with the acpi fixup.

Thanks.

	Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH] sworks-agp: Switch to PCI ref counting APIs

[Alan Cox]

On Wed, 25 Apr 2007 19:21:58 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Mon, 23 Apr 2007 14:51:29 +0100 Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> 
> >  {
> >  	struct agp_bridge_data *bridge = pci_get_drvdata(pdev);
> >  
> > +	pci_dev_put(bridge->dev);
> >  	agp_remove_bridge(bridge);
> >  	agp_put_bridge(bridge);
> > +	pci_dev_put(serverworks_private.svrwrks_dev)
> > +	serverworks_private.svrwrks_dev = NULL;
> 
> err, guys?

I merely fixed the code to refcount, the actual code itself is complete
bollocks and does exactly what that implies. It also explodes if you ever
use the hotplug testing code to simulate a plug or replug and goes
completely bonkers if you put two of them in a qemu simulator 8)

Alan

Re: [PATCH] sworks-agp: Switch to PCI ref counting APIs

[Alan Cox]

On Thu, 26 Apr 2007 00:20:19 -0400
Dave Jones <davej@redhat.com> wrote:

> On Wed, Apr 25, 2007 at 07:21:58PM -0700, Andrew Morton wrote:
>  > On Mon, 23 Apr 2007 14:51:29 +0100 Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>  > 
>  > >  {
>  > >  	struct agp_bridge_data *bridge = pci_get_drvdata(pdev);
>  > >  
>  > > +	pci_dev_put(bridge->dev);
>  > >  	agp_remove_bridge(bridge);
>  > >  	agp_put_bridge(bridge);
>  > > +	pci_dev_put(serverworks_private.svrwrks_dev)
>  > > +	serverworks_private.svrwrks_dev = NULL;
>  > 
>  > err, guys?
> 
> ? One put for the agp bridge, one for the host bridge.
> What am I missing?

Nothing - I assume Andrew was referring the bizarre way one of the
devices is global and one is per structure ?