PCI code cleanup

PATCH Superseded Louis Zhuang 3 participants 4 messages Feb 18, 2003 PCI

lore

Share & tools

lore

Louis Zhuang

author

2003-02-18 02:33 UTC #0 lore raw

PATCH 1/1 PCI code cleanup

Dear Greg,
The patch clean up some old-style usage of list_head. Pls apply if you like it. Thanks

Signature · 70 lines

Yours truly,
Louis Zhuang
--
Fault Injection Test Harness Project
BK tree: http://fault-injection.bkbits.net/linux-2.5
Home Page: http://sf.net/projects/fault-injection


===== drivers/pci/probe.c 1.26 vs edited =====
-- 1.26/drivers/pci/probe.c	Mon Jan 13 11:44:26 2003
+++ edited/drivers/pci/probe.c	Tue Feb 18 09:28:40 2003
@@ -533,7 +533,7 @@
 {
 	const struct list_head *l;
 
-	for(l=list->next; l != list; l = l->next) {
+	list_for_each(l, list) {
 		const struct pci_bus *b = pci_bus_b(l);
 		if (b->number == nr || pci_bus_exists(&b->children, nr))
 			return 1;
===== drivers/pci/setup-bus.c 1.12 vs edited =====
-- 1.12/drivers/pci/setup-bus.c	Sun Dec 22 07:46:25 2002
+++ edited/drivers/pci/setup-bus.c	Tue Feb 18 09:33:33 2003
@@ -45,7 +45,7 @@
 	int idx, found_vga = 0;
 
 	head.next = NULL;
-	for (ln=bus->devices.next; ln != &bus->devices; ln=ln->next) {
+	list_for_each(ln, &bus->devices) {
 		struct pci_dev *dev = pci_dev_b(ln);
 		u16 class = dev->class >> 8;
 
@@ -208,7 +208,7 @@
 	if (!(b_res->flags & IORESOURCE_IO))
 		return;
 
-	for (ln=bus->devices.next; ln != &bus->devices; ln=ln->next) {
+	list_for_each(ln, &bus->devices) {
 		struct pci_dev *dev = pci_dev_b(ln);
 		int i;
 		
@@ -261,7 +261,7 @@
 	max_order = 0;
 	size = 0;
 
-	for (ln=bus->devices.next; ln != &bus->devices; ln=ln->next) {
+	list_for_each(ln, &bus->devices) {
 		struct pci_dev *dev = pci_dev_b(ln);
 		int i;
 		
@@ -325,8 +325,9 @@
 	struct list_head *ln;
 	unsigned long mask, type;
 
-	for (ln=bus->children.next; ln != &bus->children; ln=ln->next)
+	list_for_each(ln, &bus->children) {
 		pci_bus_size_bridges(pci_bus_b(ln));
+	}
 
 	/* The root bus? */
 	if (!bus->self)
@@ -361,7 +362,7 @@
 			b->resource[0]->flags |= IORESOURCE_BUS_HAS_VGA;
 		}
 	}
-	for (ln=bus->children.next; ln != &bus->children; ln=ln->next) {
+	list_for_each(ln, &bus->children) {
 		struct pci_bus *b = pci_bus_b(ln);
 
 		pci_bus_assign_resources(b);

Louis Zhuang

author

2003-02-18 03:26 UTC #2 lore raw

PATCH RESEND 1/1 PCI code cleanup

Quoted 9 lines

Well, if you're changing that anyway, you could as well use
{
	const struct pci_bus *b;

	list_for_each_entry(b, list) {
		if (b->number ...

Quoted 3 lines

which looks even nicer ;-)
--Kai

How about following patch? ;-)

- Louis

===== drivers/pci/probe.c 1.26 vs edited =====

drivers/pci/probe.c +2 -3

@@ -531,10 +531,9 @@
 
 int __devinit pci_bus_exists(const struct list_head *list, int nr)
 {
-	const struct list_head *l;
+	const struct pci_bus *b;
 
-	for(l=list->next; l != list; l = l->next) {
-		const struct pci_bus *b = pci_bus_b(l);
+	list_for_each_entry(b, list, node) {
 		if (b->number == nr || pci_bus_exists(&b->children, nr))
 			return 1;
 	}

===== drivers/pci/setup-bus.c 1.12 vs edited =====

drivers/pci/setup-bus.c +12 ~~-18~~

@@ -39,14 +39,13 @@
 static int __devinit
 pbus_assign_resources_sorted(struct pci_bus *bus)
 {
-	struct list_head *ln;
+	struct pci_dev *dev;
 	struct resource *res;
 	struct resource_list head, *list, *tmp;
 	int idx, found_vga = 0;
 
 	head.next = NULL;
-	for (ln=bus->devices.next; ln != &bus->devices; ln=ln->next) {
-		struct pci_dev *dev = pci_dev_b(ln);
+	list_for_each_entry(dev, &bus->devices, bus_list) {
 		u16 class = dev->class >> 8;
 
 		if (class == PCI_CLASS_DISPLAY_VGA

@@ -201,15 +200,14 @@
 static void __devinit
 pbus_size_io(struct pci_bus *bus)
 {
-	struct list_head *ln;
+	struct pci_dev *dev;
 	struct resource *b_res = bus->resource[0];
 	unsigned long size = 0, size1 = 0;
 
 	if (!(b_res->flags & IORESOURCE_IO))
 		return;
 
-	for (ln=bus->devices.next; ln != &bus->devices; ln=ln->next) {
-		struct pci_dev *dev = pci_dev_b(ln);
+	list_for_each_entry(dev, &bus->devices, bus_list) {
 		int i;
 		
 		for (i = 0; i < PCI_NUM_RESOURCES; i++) {

@@ -250,7 +248,7 @@
 static void __devinit
 pbus_size_mem(struct pci_bus *bus, unsigned long mask, unsigned long type)
 {
-	struct list_head *ln;
+	struct pci_dev *dev;
 	unsigned long min_align, align, size;
 	unsigned long aligns[12];	/* Alignments from 1Mb to 2Gb */
 	int order, max_order;

@@ -261,8 +259,7 @@
 	max_order = 0;
 	size = 0;
 
-	for (ln=bus->devices.next; ln != &bus->devices; ln=ln->next) {
-		struct pci_dev *dev = pci_dev_b(ln);
+	list_for_each_entry(dev, &bus->devices, bus_list) {
 		int i;
 		
 		for (i = 0; i < PCI_NUM_RESOURCES; i++) {

@@ -322,11 +319,12 @@
 void __devinit
 pci_bus_size_bridges(struct pci_bus *bus)
 {
-	struct list_head *ln;
+	struct pci_bus *b;
 	unsigned long mask, type;
 
-	for (ln=bus->children.next; ln != &bus->children; ln=ln->next)
-		pci_bus_size_bridges(pci_bus_b(ln));
+	list_for_each_entry(b, &bus->children, node) {
+		pci_bus_size_bridges(b);
+	}
 
 	/* The root bus? */
 	if (!bus->self)

@@ -350,20 +348,16 @@
 void __devinit
 pci_bus_assign_resources(struct pci_bus *bus)
 {
-	struct list_head *ln;
+	struct pci_bus *b;
 	int found_vga = pbus_assign_resources_sorted(bus);
 
 	if (found_vga) {
-		struct pci_bus *b;
-
 		/* Propagate presence of the VGA to upstream bridges */
 		for (b = bus; b->parent; b = b->parent) {
 			b->resource[0]->flags |= IORESOURCE_BUS_HAS_VGA;
 		}
 	}
-	for (ln=bus->children.next; ln != &bus->children; ln=ln->next) {
-		struct pci_bus *b = pci_bus_b(ln);
-
+	list_for_each_entry(b, &bus->children, node) {
 		pci_bus_assign_resources(b);
 		pci_setup_bridge(b);
 	}

Jeff Garzik

2003-02-18 04:30 UTC #3 lore raw

Wow, looks pretty good to me.

I was too lazy to check, but you may want to be careful of:
list iteration direction. IIRC there were some list traversals that happened in reverse order, and you don't want to accidentally change that in the cleanup. I'm not saying you did... just that is something to check.