[PATCH 2/2] amd64-agp: Probe unknown AGP devices the right way

[PATCH 2/2] amd64-agp: Probe unknown AGP devices the right way

[Ben Hutchings]

The current initialisation code probes 'unsupported' AGP devices
simply by calling its own probe function.  It does not lock these
devices or even check whether another driver is already bound to
them.

We must use the device core to manage this.  So if the specific
device id table didn't match anything and agp_try_unsupported=1,
switch the device id table and call driver_attach() again.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
This is untested as I have no suitable hardware.

Greg, please confirm that the use of driver_attach() is sane.

Ben.

 drivers/char/agp/amd64-agp.c |   27 +++++++++++++++------------
 1 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/char/agp/amd64-agp.c b/drivers/char/agp/amd64-agp.c
index fd50ead..93f56d6 100644
--- a/drivers/char/agp/amd64-agp.c
+++ b/drivers/char/agp/amd64-agp.c
@@ -499,6 +499,10 @@ static int __devinit agp_amd64_probe(struct pci_dev *pdev,
 	u8 cap_ptr;
 	int err;
 
+	/* The Highlander principle */
+	if (agp_bridges_found)
+		return -ENODEV;
+
 	cap_ptr = pci_find_capability(pdev, PCI_CAP_ID_AGP);
 	if (!cap_ptr)
 		return -ENODEV;
@@ -562,6 +566,8 @@ static void __devexit agp_amd64_remove(struct pci_dev *pdev)
 			   amd64_aperture_sizes[bridge->aperture_size_idx].size);
 	agp_remove_bridge(bridge);
 	agp_put_bridge(bridge);
+
+	agp_bridges_found--;
 }
 
 #ifdef CONFIG_PM
@@ -709,6 +715,11 @@ static struct pci_device_id agp_amd64_pci_table[] = {
 
 MODULE_DEVICE_TABLE(pci, agp_amd64_pci_table);
 
+static DEFINE_PCI_DEVICE_TABLE(agp_amd64_pci_promisc_table) = {
+	{ PCI_DEVICE_CLASS(0, 0) },
+	{ }
+};
+
 static struct pci_driver agp_amd64_pci_driver = {
 	.name		= "agpgart-amd64",
 	.id_table	= agp_amd64_pci_table,
@@ -734,7 +745,6 @@ int __init agp_amd64_init(void)
 		return err;
 
 	if (agp_bridges_found == 0) {
-		struct pci_dev *dev;
 		if (!agp_try_unsupported && !agp_try_unsupported_boot) {
 			printk(KERN_INFO PFX "No supported AGP bridge found.\n");
 #ifdef MODULE
@@ -750,17 +760,10 @@ int __init agp_amd64_init(void)
 			return -ENODEV;
 
 		/* Look for any AGP bridge */
-		dev = NULL;
-		err = -ENODEV;
-		for_each_pci_dev(dev) {
-			if (!pci_find_capability(dev, PCI_CAP_ID_AGP))
-				continue;
-			/* Only one bridge supported right now */
-			if (agp_amd64_probe(dev, NULL) == 0) {
-				err = 0;
-				break;
-			}
-		}
+		agp_amd64_pci_driver.id_table = agp_amd64_pci_promisc_table;
+		err = driver_attach(&agp_amd64_pci_driver.driver);
+		if (err == 0 && agp_bridges_found == 0)
+			err = -ENODEV;
 	}
 	return err;
 }
-- 
1.7.0

Re: [PATCH 2/2] amd64-agp: Probe unknown AGP devices the right way

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 3001 bytes --]

On Wed, 2010-03-24 at 03:36 +0000, Ben Hutchings wrote:
> The current initialisation code probes 'unsupported' AGP devices
> simply by calling its own probe function.  It does not lock these
> devices or even check whether another driver is already bound to
> them.
> 
> We must use the device core to manage this.  So if the specific
> device id table didn't match anything and agp_try_unsupported=1,
> switch the device id table and call driver_attach() again.

Can you please apply or nak this?

Ben.

> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> This is untested as I have no suitable hardware.
> 
> Greg, please confirm that the use of driver_attach() is sane.
> 
> Ben.
> 
>  drivers/char/agp/amd64-agp.c |   27 +++++++++++++++------------
>  1 files changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/char/agp/amd64-agp.c b/drivers/char/agp/amd64-agp.c
> index fd50ead..93f56d6 100644
> --- a/drivers/char/agp/amd64-agp.c
> +++ b/drivers/char/agp/amd64-agp.c
> @@ -499,6 +499,10 @@ static int __devinit agp_amd64_probe(struct pci_dev *pdev,
>  	u8 cap_ptr;
>  	int err;
>  
> +	/* The Highlander principle */
> +	if (agp_bridges_found)
> +		return -ENODEV;
> +
>  	cap_ptr = pci_find_capability(pdev, PCI_CAP_ID_AGP);
>  	if (!cap_ptr)
>  		return -ENODEV;
> @@ -562,6 +566,8 @@ static void __devexit agp_amd64_remove(struct pci_dev *pdev)
>  			   amd64_aperture_sizes[bridge->aperture_size_idx].size);
>  	agp_remove_bridge(bridge);
>  	agp_put_bridge(bridge);
> +
> +	agp_bridges_found--;
>  }
>  
>  #ifdef CONFIG_PM
> @@ -709,6 +715,11 @@ static struct pci_device_id agp_amd64_pci_table[] = {
>  
>  MODULE_DEVICE_TABLE(pci, agp_amd64_pci_table);
>  
> +static DEFINE_PCI_DEVICE_TABLE(agp_amd64_pci_promisc_table) = {
> +	{ PCI_DEVICE_CLASS(0, 0) },
> +	{ }
> +};
> +
>  static struct pci_driver agp_amd64_pci_driver = {
>  	.name		= "agpgart-amd64",
>  	.id_table	= agp_amd64_pci_table,
> @@ -734,7 +745,6 @@ int __init agp_amd64_init(void)
>  		return err;
>  
>  	if (agp_bridges_found == 0) {
> -		struct pci_dev *dev;
>  		if (!agp_try_unsupported && !agp_try_unsupported_boot) {
>  			printk(KERN_INFO PFX "No supported AGP bridge found.\n");
>  #ifdef MODULE
> @@ -750,17 +760,10 @@ int __init agp_amd64_init(void)
>  			return -ENODEV;
>  
>  		/* Look for any AGP bridge */
> -		dev = NULL;
> -		err = -ENODEV;
> -		for_each_pci_dev(dev) {
> -			if (!pci_find_capability(dev, PCI_CAP_ID_AGP))
> -				continue;
> -			/* Only one bridge supported right now */
> -			if (agp_amd64_probe(dev, NULL) == 0) {
> -				err = 0;
> -				break;
> -			}
> -		}
> +		agp_amd64_pci_driver.id_table = agp_amd64_pci_promisc_table;
> +		err = driver_attach(&agp_amd64_pci_driver.driver);
> +		if (err == 0 && agp_bridges_found == 0)
> +			err = -ENODEV;
>  	}
>  	return err;
>  }

-- 
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]