[patch 03/11] arch/i386/pci/i386.c: Use new for_each_pci_dev macro

[patch 03/11] arch/i386/pci/i386.c: Use new for_each_pci_dev macro

[domen]

As requested by Christoph Hellwig I've created a new macro called
for_each_pci_dev. It is a wrapper for this common use of pci_get/find_device:

(while ((dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL))

This macro will return the pci_dev *for all pci devices.  Here is the first patch I 
used to test this macro with. Compiled and booted on my T23. There will be
53 more patches using this new macro.

Hanna Linder
IBM Linux Technology Center

Signed-off-by: Hanna Linder <hannal@us.ibm.com>
Signed-off-by: Maximilian Attems <janitor@sternwelten.at>

---

Signed-off-by: Domen Puncer <domen@coderock.org>
---


 kj-domen/arch/i386/pci/i386.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff -puN arch/i386/pci/i386.c~for-each-pci-dev-arch_i386_pci_i386 arch/i386/pci/i386.c
--- kj/arch/i386/pci/i386.c~for-each-pci-dev-arch_i386_pci_i386	2005-01-10 17:59:57.000000000 +0100
+++ kj-domen/arch/i386/pci/i386.c	2005-01-10 17:59:57.000000000 +0100
@@ -124,7 +124,7 @@ static void __init pcibios_allocate_reso
 	u16 command;
 	struct resource *r, *pr;
 
-	while ((dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
+	for_each_pci_dev(dev) {
 		pci_read_config_word(dev, PCI_COMMAND, &command);
 		for(idx = 0; idx < 6; idx++) {
 			r = &dev->resource[idx];
@@ -168,7 +168,7 @@ static int __init pcibios_assign_resourc
 	int idx;
 	struct resource *r;
 
-	while ((dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
+	for_each_pci_dev(dev) {
 		int class = dev->class >> 8;
 
 		/* Don't touch classless devices and host bridges */
_

Re: [patch 03/11] arch/i386/pci/i386.c: Use new for_each_pci_dev macro

[Dave Jones]

On Wed, Jan 12, 2005 at 12:34:58AM +0100, domen@coderock.org wrote:

 > As requested by Christoph Hellwig I've created a new macro called
 > for_each_pci_dev. It is a wrapper for this common use of pci_get/find_device:
 > 
 > (while ((dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL))
 > 
 > This macro will return the pci_dev *for all pci devices.  Here is the first patch I 
 > used to test this macro with. Compiled and booted on my T23. There will be
 > 53 more patches using this new macro.

Which looks just like the pci_for_each_dev we used to have.
That function got removed due some shortcoming or other that I never
fully understood, but ISTR it had something to do with locking.
(why it couldnt be hidden inside for_each_pci_dev is a mystery to me)

We've had lots of code in the kernel go from this..

pci_for_each_dev(loop_dev) {

to the disgustingly unreadable..

while ((loop_dev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, loop_dev)) != NULL) {

and now its going to ..

for_each_pci_dev(loop_dev) {

So,.. what has all this churn bought us, and where does it end ?
With four words in the function name, we've enough possibilities
for quite a few more iterations yet.

		Dave

Re: [patch 03/11] arch/i386/pci/i386.c: Use new for_each_pci_dev macro

[Greg KH]

On Tue, Jan 11, 2005 at 07:46:19PM -0500, Dave Jones wrote:
> On Wed, Jan 12, 2005 at 12:34:58AM +0100, domen@coderock.org wrote:
> 
>  > As requested by Christoph Hellwig I've created a new macro called
>  > for_each_pci_dev. It is a wrapper for this common use of pci_get/find_device:
>  > 
>  > (while ((dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL))
>  > 
>  > This macro will return the pci_dev *for all pci devices.  Here is the first patch I 
>  > used to test this macro with. Compiled and booted on my T23. There will be
>  > 53 more patches using this new macro.
> 
> Which looks just like the pci_for_each_dev we used to have.
> That function got removed due some shortcoming or other that I never
> fully understood, but ISTR it had something to do with locking.
> (why it couldnt be hidden inside for_each_pci_dev is a mystery to me)
> 
> We've had lots of code in the kernel go from this..
> 
> pci_for_each_dev(loop_dev) {

Which did not have any locks at all, and was broken for hotplug systems.

> to the disgustingly unreadable..
> 
> while ((loop_dev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, loop_dev)) != NULL) {

Yeah, blame me for that, but it was race proof for hotplug boxes.

> and now its going to ..
> 
> for_each_pci_dev(loop_dev) {

Which is because I didn't think of that in the first place, sorry.

> So,.. what has all this churn bought us, and where does it end ?
> With four words in the function name, we've enough possibilities
> for quite a few more iterations yet.

We now have a simple macro to iterate over all pci devices, in a
reference count and lock safe manner.

The next change will be to just delete the thing alltogether, as most
all drivers shouldn't be walking the list of pci devices in the first
place.

Yeah yeah yeah, I know I can't really do that, as there are lots of
places where it is valid to do so, but I can dream...

thanks,

greg k-h