fix improper use of pci_module_init() in drivers/char/agp/amd64-agp.c

Hi,

In going through the tree and auditing the usage of pci_module_init(), I noticed that the amd64-agp driver was assuming that the return value of this function could be greater than 0 (which is what could happen in 2.2 and 2.4 kernels.) As this is no longer true, I think the following patch is correct.

I can add this to my bk-pci tree if you wish, otherwise feel free to send it upwards.

Quoted 8 lines · Greg KH

Hi,

In going through the tree and auditing the usage of pci_module_init(), I noticed that the amd64-agp driver was assuming that the return value of this function could be greater than 0 (which is what could happen in 2.2 and 2.4 kernels.) As this is no longer true, I think the following patch is correct.

Quoted 2 lines

I can add this to my bk-pci tree if you wish, otherwise feel free to send it upwards.

There needs to be some replacement for it, you cannot just delete the code.

The idea is to run it as fallback when no devices are found.

How about this patch?

Quoted 3 lines

The idea is to run it as fallback when no devices are found.

How about this patch?

That does not work the way you are asking it to work. pci_module_init() is just a replacement for pci_register_driver these days. It will return either "0" if the driver is successfully registered, or a negative value if something bad happened. It will not return the number of devices that this driver bound to.

So, if no devices are in the system, it will return 0, and again, the code you are wanting to run, will not.

So, how about using the new pci_dev_present() call instead? That should be what you want, right?

Quoted 9 lines · Andi Kleen

Quoted 8 lines · Greg KH

Hi,

In going through the tree and auditing the usage of pci_module_init(), I noticed that the amd64-agp driver was assuming that the return value of this function could be greater than 0 (which is what could happen in 2.2 and 2.4 kernels.) As this is no longer true, I think the following patch is correct.

Quoted 5 lines

Quoted 2 lines

I can add this to my bk-pci tree if you wish, otherwise feel free to send it upwards.

There needs to be some replacement for it, you cannot just delete the code.

But that code has not ever run, since early 2.5 days. Don't tell me people are used to it :)

Quoted 4 lines

The idea is to run it as fallback when no devices are found.

How about this patch?

That does not work the way you are asking it to work. pci_module_init() is just a replacement for pci_register_driver these days. It will return either "0" if the driver is successfully registered, or a negative value if something bad happened. It will not return the number of devices that this driver bound to.

So, if no devices are in the system, it will return 0, and again, the code you are wanting to run, will not.

So, how about using the new pci_dev_present() call instead? That should be what you want, right?

Quoted 16 lines · Greg KH

Quoted 14 lines · Andi Kleen

Quoted 10 lines · Greg KH

Hi,

In going through the tree and auditing the usage of pci_module_init(), I noticed that the amd64-agp driver was assuming that the return value of this function could be greater than 0 (which is what could happen in 2.2 and 2.4 kernels.) As this is no longer true, I think the following patch is correct.

I can add this to my bk-pci tree if you wish, otherwise feel free to send it upwards.

There needs to be some replacement for it, you cannot just delete the code.

Quoted 2 lines

But that code has not ever run, since early 2.5 days. Don't tell me people are used to it :)

No, but it's needed for new chipsets that are not in the PCI tables. People probably didn't complain because we're covering the current generation of K8 AGP bridges, but there should be new ones soon

2.4 had similar code.

Quoted 10 lines

Quoted 3 lines

The idea is to run it as fallback when no devices are found.

How about this patch?

That does not work the way you are asking it to work. pci_module_init() is just a replacement for pci_register_driver these days. It will return either "0" if the driver is successfully registered, or a negative value if something bad happened. It will not return the number of devices that this driver bound to.

Quoted 2 lines

So, if no devices are in the system, it will return 0, and again, the code you are wanting to run, will not.

Oh, yes I forgot that hotplug makes everything simple complicated.

Quoted 2 lines

So, how about using the new pci_dev_present() call instead? That should be what you want, right?

% grep pci_dev_present include/linux/*
%

This patch will probably do and doesn't rely on any nonexisting calls.

----------------------------------------------------------------------

Fix fallback code in K8 AGP driver.

Problem pointed out by Greg KH

Signed-off-by Andi Kleen <[email protected]>

diff -u linux-2.6.9rc3-work/drivers/char/agp/amd64-agp.c-o linux-2.6.9rc3-work/drivers/char/agp/amd64-agp.c

drivers/char/agp/amd64-agp.c renamed from drivers/char/agp/amd64-agp.c-o +3 -0

@@ -55,6 +55,8 @@
 static int gart_iterator;
 #define for_each_nb() for(gart_iterator=0;gart_iterator<nr_garts;gart_iterator++)
 
+static int num_bridges;
+
 static void flush_amd64_tlb(struct pci_dev *dev)
 {
 	u32 tmp;

@@ -514,6 +516,7 @@
 	}
 
 	pci_set_drvdata(pdev, bridge);
+	num_bridges++;
 	return agp_add_bridge(bridge);
 }
 

Hi,

In going through the tree and auditing the usage of pci_module_init(), I noticed that the amd64-agp driver was assuming that the return value of this function could be greater than 0 (which is what could happen in 2.2 and 2.4 kernels.) As this is no longer true, I think the following patch is correct.

I can add this to my bk-pci tree if you wish, otherwise feel free to send it upwards.