[PATCH 01/16] Add of_get_next_parent()

[PATCH 01/16] Add of_get_next_parent()

[Michael Ellerman]

Iterating through a device node's parents is simple enough, but dealing
with the refcounts properly is a little ugly, and replicating that logic
is asking for someone to get it wrong or forget it all together, eg:

while (dn != NULL) {
	/* loop body */
	tmp = of_get_parent(dn);
	of_node_put(dn);
	dn = tmp;
}

So add of_get_next_parent(), inspired by of_get_next_child(). The contract
is that it returns the parent and drops the reference on the current node,
this makes the loop look like:

while (dn != NULL) {
	/* loop body */
	dn = of_get_next_parent(dn);
}

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 drivers/of/base.c  |   25 +++++++++++++++++++++++++
 include/linux/of.h |    1 +
 2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 9377f3b..e0db2b5 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -138,6 +138,31 @@ struct device_node *of_get_parent(const struct device_node *node)
 EXPORT_SYMBOL(of_get_parent);
 
 /**
+ *	of_get_next_parent - Iterate to a node's parent
+ *	@node:	Node to get parent of
+ *
+ * 	This is like of_get_parent() except that it drops the
+ * 	refcount on the passed node, making it suitable for iterating
+ * 	through a node's parents.
+ *
+ *	Returns a node pointer with refcount incremented, use
+ *	of_node_put() on it when done.
+ */
+struct device_node *of_get_next_parent(struct device_node *node)
+{
+	struct device_node *parent;
+
+	if (!node)
+		return NULL;
+
+	read_lock(&devtree_lock);
+	parent = of_node_get(node->parent);
+	of_node_put(node);
+	read_unlock(&devtree_lock);
+	return parent;
+}
+
+/**
  *	of_get_next_child - Iterate a node childs
  *	@node:	parent node
  *	@prev:	previous child of the parent node, or NULL to get first
diff --git a/include/linux/of.h b/include/linux/of.h
index 5c39b92..3557d1e 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -44,6 +44,7 @@ extern struct device_node *of_find_compatible_node(struct device_node *from,
 extern struct device_node *of_find_node_by_path(const char *path);
 extern struct device_node *of_find_node_by_phandle(phandle handle);
 extern struct device_node *of_get_parent(const struct device_node *node);
+extern struct device_node *of_get_next_parent(struct device_node *node);
 extern struct device_node *of_get_next_child(const struct device_node *node,
 					     struct device_node *prev);
 extern struct property *of_find_property(const struct device_node *np,
-- 
1.5.2.rc1.1884.g59b20

[PATCH 02/16] Use of_get_next_parent() in pci_dma_bus_setup_pSeries()

[Michael Ellerman]

pci_dma_bus_setup_pSeries() should use of_get_next_parent() to safely
iterate through the parent nodes.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/pseries/iommu.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index be17d23..83c0e0f 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -314,7 +314,7 @@ static void pci_dma_bus_setup_pSeries(struct pci_bus *bus)
 {
 	struct device_node *dn;
 	struct iommu_table *tbl;
-	struct device_node *isa_dn, *isa_dn_orig;
+	struct device_node *isa_dn;
 	struct device_node *tmp;
 	struct pci_dn *pci;
 	int children;
@@ -334,13 +334,13 @@ static void pci_dma_bus_setup_pSeries(struct pci_bus *bus)
 	/* Check if the ISA bus on the system is under
 	 * this PHB.
 	 */
-	isa_dn = isa_dn_orig = of_find_node_by_type(NULL, "isa");
+	isa_dn = of_find_node_by_type(NULL, "isa");
 
 	while (isa_dn && isa_dn != dn)
-		isa_dn = isa_dn->parent;
+		isa_dn = of_get_next_parent(isa_dn);
 
-	if (isa_dn_orig)
-		of_node_put(isa_dn_orig);
+	/* Drop our reference, it's still safe to check the pointer below */
+	of_node_put(isa_dn);
 
 	/* Count number of direct PCI children of the PHB. */
 	for (children = 0, tmp = dn->child; tmp; tmp = tmp->sibling)
-- 
1.5.2.rc1.1884.g59b20

[PATCH 03/16] Use of_get_next_parent() in pci_dma_bus_setup_pSeriesLP()

[Michael Ellerman]

pci_dma_bus_setup_pSeriesLP() should use of_get_next_parent() to safely
iterate through the parent nodes.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/pseries/iommu.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 83c0e0f..e2b325d 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -403,7 +403,7 @@ static void pci_dma_bus_setup_pSeriesLP(struct pci_bus *bus)
 	DBG("pci_dma_bus_setup_pSeriesLP: setting up bus %s\n", dn->full_name);
 
 	/* Find nearest ibm,dma-window, walking up the device tree */
-	for (pdn = dn; pdn != NULL; pdn = pdn->parent) {
+	for (pdn = of_node_get(dn); pdn; pdn = of_get_next_parent(pdn)) {
 		dma_window = of_get_property(pdn, "ibm,dma-window", NULL);
 		if (dma_window != NULL)
 			break;
@@ -411,6 +411,7 @@ static void pci_dma_bus_setup_pSeriesLP(struct pci_bus *bus)
 
 	if (dma_window == NULL) {
 		DBG("  no ibm,dma-window property !\n");
+		of_node_put(pdn);
 		return;
 	}
 
@@ -437,6 +438,8 @@ static void pci_dma_bus_setup_pSeriesLP(struct pci_bus *bus)
 
 	if (pdn != dn)
 		PCI_DN(dn)->iommu_table = ppci->iommu_table;
+
+	of_node_put(pdn);
 }
 
 
-- 
1.5.2.rc1.1884.g59b20

[PATCH 04/16] Use of_get_next_parent() in pci_dma_dev_setup_pSeries()

[Michael Ellerman]

pci_dma_dev_setup_pSeries() should use of_get_next_parent() to safely
iterate through the parent nodes.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/pseries/iommu.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index e2b325d..5e9430e 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -472,14 +472,17 @@ static void pci_dma_dev_setup_pSeries(struct pci_dev *dev)
 	 * an already allocated iommu table is found and use that.
 	 */
 
+	dn = of_node_get(dn);
 	while (dn && PCI_DN(dn) && PCI_DN(dn)->iommu_table == NULL)
-		dn = dn->parent;
+		dn = of_get_next_parent(dn);
 
 	if (dn && PCI_DN(dn))
 		dev->dev.archdata.dma_data = PCI_DN(dn)->iommu_table;
 	else
 		printk(KERN_WARNING "iommu: Device %s has no iommu table\n",
 		       pci_name(dev));
+
+	of_node_put(dn);
 }
 
 static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
-- 
1.5.2.rc1.1884.g59b20

[PATCH 05/16] Use of_get_next_parent() in pci_dma_dev_setup_pSeriesLP()

[Michael Ellerman]

pci_dma_dev_setup_pSeriesLP() should use of_get_next_parent() to safely
iterate through the parent nodes.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/pseries/iommu.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 5e9430e..ef1aa8d 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -503,8 +503,9 @@ static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
 	dn = pci_device_to_OF_node(dev);
 	DBG("  node is %s\n", dn->full_name);
 
-	for (pdn = dn; pdn && PCI_DN(pdn) && !PCI_DN(pdn)->iommu_table;
-	     pdn = pdn->parent) {
+	for (pdn = of_node_get(dn);
+	     pdn && PCI_DN(pdn) && !PCI_DN(pdn)->iommu_table;
+	     pdn = of_get_next_parent(pdn)) {
 		dma_window = of_get_property(pdn, "ibm,dma-window", NULL);
 		if (dma_window)
 			break;
@@ -514,7 +515,7 @@ static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
 		printk(KERN_WARNING "pci_dma_dev_setup_pSeriesLP: "
 		       "no DMA window found for pci dev=%s dn=%s\n",
 				 pci_name(dev), dn? dn->full_name : "<null>");
-		return;
+		goto out_put;
 	}
 	DBG("  parent is %s\n", pdn->full_name);
 
@@ -524,7 +525,7 @@ static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
 	if (dma_window == NULL || pdn->parent == NULL) {
 		DBG("  no dma window for device, linking to parent\n");
 		dev->dev.archdata.dma_data = PCI_DN(pdn)->iommu_table;
-		return;
+		goto out_put;
 	}
 
 	pci = PCI_DN(pdn);
@@ -544,6 +545,9 @@ static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
 	}
 
 	dev->dev.archdata.dma_data = pci->iommu_table;
+
+out_put:
+	of_node_put(pdn);
 }
 #else  /* CONFIG_PCI */
 #define pci_dma_bus_setup_pSeries	NULL
-- 
1.5.2.rc1.1884.g59b20

[PATCH 06/16] Use of_get_next_child() in pci_dma_bus_setup_pSeries()

[Michael Ellerman]

pci_dma_bus_setup_pSeries() should use of_get_next_child() to safely
iterate through the nodes children.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/pseries/iommu.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index ef1aa8d..1a9e14f 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -343,7 +343,7 @@ static void pci_dma_bus_setup_pSeries(struct pci_bus *bus)
 	of_node_put(isa_dn);
 
 	/* Count number of direct PCI children of the PHB. */
-	for (children = 0, tmp = dn->child; tmp; tmp = tmp->sibling)
+	for (children = 0, tmp = NULL; (tmp = of_get_next_child(dn, tmp));)
 		children++;
 
 	DBG("Children: %d\n", children);
-- 
1.5.2.rc1.1884.g59b20

[PATCH 07/16] Use of_get_next_parent() in xics_setup_8259_cascade()

[Michael Ellerman]

Use of_get_next_parent() in xics_setup_8259_cascade() to simplify
the loop logic.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/pseries/xics.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/xics.c b/arch/powerpc/platforms/pseries/xics.c
index 66e7d68..58a3cc7 100644
--- a/arch/powerpc/platforms/pseries/xics.c
+++ b/arch/powerpc/platforms/pseries/xics.c
@@ -617,7 +617,7 @@ static void __init xics_init_one_node(struct device_node *np,
 
 static void __init xics_setup_8259_cascade(void)
 {
-	struct device_node *np, *old, *found = NULL;
+	struct device_node *np, *found = NULL;
 	int cascade, naddr;
 	const u32 *addrp;
 	unsigned long intack = 0;
@@ -638,11 +638,8 @@ static void __init xics_setup_8259_cascade(void)
 	}
 	pr_debug("xics: cascade mapped to irq %d\n", cascade);
 
-	for (old = of_node_get(found); old != NULL ; old = np) {
-		np = of_get_parent(old);
-		of_node_put(old);
-		if (np == NULL)
-			break;
+	np = of_node_get(found);
+	while ((np = of_get_next_parent(np))) {
 		if (strcmp(np->name, "pci") != 0)
 			continue;
 		addrp = of_get_property(np, "8259-interrupt-acknowledge", NULL);
-- 
1.5.2.rc1.1884.g59b20

[PATCH 08/16] Use of_get_next_parent() in pseries_mpic_init_IRQ()

[Michael Ellerman]

Use of_get_next_parent() in pseries_mpic_init_IRQ() to simplify
the loop logic.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/pseries/setup.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index fdb9b1c..f08dfaf 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -129,7 +129,7 @@ void pseries_8259_cascade(unsigned int irq, struct irq_desc *desc)
 
 static void __init pseries_mpic_init_IRQ(void)
 {
-	struct device_node *np, *old, *cascade = NULL;
+	struct device_node *np, *cascade = NULL;
         const unsigned int *addrp;
 	unsigned long intack = 0;
 	const unsigned int *opprop;
@@ -182,11 +182,8 @@ static void __init pseries_mpic_init_IRQ(void)
 	}
 
 	/* Check ACK type */
-	for (old = of_node_get(cascade); old != NULL ; old = np) {
-		np = of_get_parent(old);
-		of_node_put(old);
-		if (np == NULL)
-			break;
+	np = of_node_get(cascade);
+	while ((np = of_get_next_parent(np))) {
 		if (strcmp(np->name, "pci") != 0)
 			continue;
 		addrp = of_get_property(np, "8259-interrupt-acknowledge",
-- 
1.5.2.rc1.1884.g59b20

[PATCH 09/16] Use of_get_next_parent() in axon_msi.c

[Michael Ellerman]

We can use of_get_next_parent() in two places in axon_msi.c to
simplify the looping logic.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/cell/axon_msi.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/cell/axon_msi.c b/arch/powerpc/platforms/cell/axon_msi.c
index 095988f..76870fe 100644
--- a/arch/powerpc/platforms/cell/axon_msi.c
+++ b/arch/powerpc/platforms/cell/axon_msi.c
@@ -125,7 +125,7 @@ static struct axon_msic *find_msi_translator(struct pci_dev *dev)
 		return NULL;
 	}
 
-	for (; dn; tmp = of_get_parent(dn), of_node_put(dn), dn = tmp) {
+	for (; dn; dn = of_get_next_parent(dn)) {
 		ph = of_get_property(dn, "msi-translator", NULL);
 		if (ph)
 			break;
@@ -171,7 +171,7 @@ static int axon_msi_check_device(struct pci_dev *dev, int nvec, int type)
 
 static int setup_msi_msg_address(struct pci_dev *dev, struct msi_msg *msg)
 {
-	struct device_node *dn, *tmp;
+	struct device_node *dn;
 	struct msi_desc *entry;
 	int len;
 	const u32 *prop;
@@ -184,7 +184,7 @@ static int setup_msi_msg_address(struct pci_dev *dev, struct msi_msg *msg)
 
 	entry = list_first_entry(&dev->msi_list, struct msi_desc, list);
 
-	for (; dn; tmp = of_get_parent(dn), of_node_put(dn), dn = tmp) {
+	for (; dn; dn = of_get_next_parent(dn)) {
 		if (entry->msi_attrib.is_64) {
 			prop = of_get_property(dn, "msi-address-64", &len);
 			if (prop)
-- 
1.5.2.rc1.1884.g59b20

[PATCH 10/16] Use of_get_next_child() in EEH gather_pci_data()

[Michael Ellerman]

We should use of_get_next_child() in the EEH gather_pci_data()
routine to safely traverse the node's children.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/pseries/eeh.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c
index 22322b3..7309caa 100644
--- a/arch/powerpc/platforms/pseries/eeh.c
+++ b/arch/powerpc/platforms/pseries/eeh.c
@@ -238,12 +238,10 @@ static size_t gather_pci_data(struct pci_dn *pdn, char * buf, size_t len)
 
 	/* Gather status on devices under the bridge */
 	if (dev->class >> 16 == PCI_BASE_CLASS_BRIDGE) {
-		dn = pdn->node->child;
-		while (dn) {
+		for (dn = NULL; (dn = of_get_next_child(pdn->node, dn));) {
 			pdn = PCI_DN(dn);
 			if (pdn)
 				n += gather_pci_data(pdn, buf+n, len-n);
-			dn = dn->sibling;
 		}
 	}
 
-- 
1.5.2.rc1.1884.g59b20

[PATCH 11/16] Use of_get_next_child() in eeh_restore_bars()

[Michael Ellerman]

We should use of_get_next_child() in the eeh_restore_bars()
routine to safely traverse the node's children.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/pseries/eeh.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c
index 7309caa..abe3de1 100644
--- a/arch/powerpc/platforms/pseries/eeh.c
+++ b/arch/powerpc/platforms/pseries/eeh.c
@@ -841,11 +841,8 @@ void eeh_restore_bars(struct pci_dn *pdn)
 	if ((pdn->eeh_mode & EEH_MODE_SUPPORTED) && !IS_BRIDGE(pdn->class_code))
 		__restore_bars (pdn);
 
-	dn = pdn->node->child;
-	while (dn) {
+	for (dn = NULL; (dn = of_get_next_child(pdn->node, dn));)
 		eeh_restore_bars (PCI_DN(dn));
-		dn = dn->sibling;
-	}
 }
 
 /**
-- 
1.5.2.rc1.1884.g59b20

[PATCH 12/16] Use of_get_next_child() in eeh_add_device_tree_early()

[Michael Ellerman]

We should use of_get_next_child() in the eeh_add_device_tree_early()
routine to safely traverse the node's children.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/pseries/eeh.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c
index abe3de1..d1d6d55 100644
--- a/arch/powerpc/platforms/pseries/eeh.c
+++ b/arch/powerpc/platforms/pseries/eeh.c
@@ -1120,7 +1120,7 @@ static void eeh_add_device_early(struct device_node *dn)
 void eeh_add_device_tree_early(struct device_node *dn)
 {
 	struct device_node *sib;
-	for (sib = dn->child; sib; sib = sib->sibling)
+	for (sib = NULL; (sib = of_get_next_child(dn, sib));)
 		eeh_add_device_tree_early(sib);
 	eeh_add_device_early(dn);
 }
-- 
1.5.2.rc1.1884.g59b20

[PATCH 13/16] Use of_get_next_child() in __eeh_mark_slot()

[Michael Ellerman]

We should use of_get_next_child() in __eeh_mark_slot() to safely
traverse the node's children.

To achieve this we need to change __eeh_mark_slot() to take the parent
node, not the child. This is also safer, as passing anything other than
node->child to the existing routine will not traverse all peers, only
those deeper in the sibling list.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/pseries/eeh.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c
index d1d6d55..d06ab36 100644
--- a/arch/powerpc/platforms/pseries/eeh.c
+++ b/arch/powerpc/platforms/pseries/eeh.c
@@ -373,9 +373,11 @@ struct device_node * find_device_pe(struct device_node *dn)
  *  an interrupt context, which is bad.
  */
 
-static void __eeh_mark_slot (struct device_node *dn, int mode_flag)
+static void __eeh_mark_slot(struct device_node *parent, int mode_flag)
 {
-	while (dn) {
+	struct device_node *dn;
+
+	for (dn = NULL; (dn = of_get_next_child(parent, dn));) {
 		if (PCI_DN(dn)) {
 			/* Mark the pci device driver too */
 			struct pci_dev *dev = PCI_DN(dn)->pcidev;
@@ -386,9 +388,8 @@ static void __eeh_mark_slot (struct device_node *dn, int mode_flag)
 				dev->error_state = pci_channel_io_frozen;
 
 			if (dn->child)
-				__eeh_mark_slot (dn->child, mode_flag);
+				__eeh_mark_slot(dn, mode_flag);
 		}
-		dn = dn->sibling;
 	}
 }
 
@@ -408,7 +409,7 @@ void eeh_mark_slot (struct device_node *dn, int mode_flag)
 	if (dev)
 		dev->error_state = pci_channel_io_frozen;
 
-	__eeh_mark_slot (dn->child, mode_flag);
+	__eeh_mark_slot(dn, mode_flag);
 }
 
 static void __eeh_clear_slot (struct device_node *dn, int mode_flag)
-- 
1.5.2.rc1.1884.g59b20

[PATCH 14/16] Use of_get_next_child() in __eeh_clear_slot()

[Michael Ellerman]

We should use of_get_next_child() in __eeh_clear_slot() to safely
traverse the node's children.

To achieve this we need to change __eeh_clear_slot() to take the parent
node, not the child. This is also safer, as passing anything other than
node->child to the existing routine will not traverse all peers, only
those deeper in the sibling list.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/pseries/eeh.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c
index d06ab36..1537597 100644
--- a/arch/powerpc/platforms/pseries/eeh.c
+++ b/arch/powerpc/platforms/pseries/eeh.c
@@ -412,16 +412,17 @@ void eeh_mark_slot (struct device_node *dn, int mode_flag)
 	__eeh_mark_slot(dn, mode_flag);
 }
 
-static void __eeh_clear_slot (struct device_node *dn, int mode_flag)
+static void __eeh_clear_slot(struct device_node *parent, int mode_flag)
 {
-	while (dn) {
+	struct device_node *dn;
+
+	for (dn = NULL; (dn = of_get_next_child(parent, dn));) {
 		if (PCI_DN(dn)) {
 			PCI_DN(dn)->eeh_mode &= ~mode_flag;
 			PCI_DN(dn)->eeh_check_count = 0;
 			if (dn->child)
-				__eeh_clear_slot (dn->child, mode_flag);
+				__eeh_clear_slot(dn, mode_flag);
 		}
-		dn = dn->sibling;
 	}
 }
 
@@ -438,7 +439,7 @@ void eeh_clear_slot (struct device_node *dn, int mode_flag)
 
 	PCI_DN(dn)->eeh_mode &= ~mode_flag;
 	PCI_DN(dn)->eeh_check_count = 0;
-	__eeh_clear_slot (dn->child, mode_flag);
+	__eeh_clear_slot(dn, mode_flag);
 	spin_unlock_irqrestore(&confirm_error_lock, flags);
 }
 
-- 
1.5.2.rc1.1884.g59b20

[PATCH 15/16] Use of_get_next_child() in eeh_reset_device()

[Michael Ellerman]

We should use of_get_next_child() in eeh_reset_device() to safely
traverse the node's children.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

---

Linas, I don't grok the logic in here, can you check it's OK. The old code
would potentially not walk through all siblings if pe_dn->node was not
equal to pe_dn->node->parent->child, but now it will regardless.


 arch/powerpc/platforms/pseries/eeh_driver.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/eeh_driver.c b/arch/powerpc/platforms/pseries/eeh_driver.c
index 15e015e..abf1850 100644
--- a/arch/powerpc/platforms/pseries/eeh_driver.c
+++ b/arch/powerpc/platforms/pseries/eeh_driver.c
@@ -249,7 +249,7 @@ static void eeh_report_failure(struct pci_dev *dev, void *userdata)
 
 static int eeh_reset_device (struct pci_dn *pe_dn, struct pci_bus *bus)
 {
-	struct device_node *dn;
+	struct device_node *dn, *parent;
 	int cnt, rc;
 
 	/* pcibios will clear the counter; save the value */
@@ -270,15 +270,16 @@ static int eeh_reset_device (struct pci_dn *pe_dn, struct pci_bus *bus)
 	if (!pcibios_find_pci_bus(dn) && PCI_DN(dn->parent))
 		dn = dn->parent->child;
 
-	while (dn) {
+	parent = of_node_get(dn->parent);
+	for (dn = NULL; (dn = of_get_next_child(parent, dn));) {
 		struct pci_dn *ppe = PCI_DN(dn);
 		/* On Power4, always true because eeh_pe_config_addr=0 */
 		if (pe_dn->eeh_pe_config_addr == ppe->eeh_pe_config_addr) {
 			rtas_configure_bridge(ppe);
 			eeh_restore_bars(ppe);
  		}
-		dn = dn->sibling;
 	}
+	of_node_put(parent);
 
 	/* Give the system 5 seconds to finish running the user-space
 	 * hotplug shutdown scripts, e.g. ifdown for ethernet.  Yes, 
-- 
1.5.2.rc1.1884.g59b20

[PATCH 16/16] Use of_get_next_child() in EEH print_device_node_tree()

[Michael Ellerman]

We should use of_get_next_child() in print_device_node_tree() to safely
traverse the node's children.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/pseries/eeh_driver.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/eeh_driver.c b/arch/powerpc/platforms/pseries/eeh_driver.c
index abf1850..0a23bc4 100644
--- a/arch/powerpc/platforms/pseries/eeh_driver.c
+++ b/arch/powerpc/platforms/pseries/eeh_driver.c
@@ -44,6 +44,7 @@ static inline const char * pcid_name (struct pci_dev *pdev)
 #ifdef DEBUG
 static void print_device_node_tree (struct pci_dn *pdn, int dent)
 {
+	struct device_node *pc;
 	int i;
 	if (!pdn) return;
 	for (i=0;i<dent; i++)
@@ -52,11 +53,8 @@ static void print_device_node_tree (struct pci_dn *pdn, int dent)
 		pdn->node->name, pdn->eeh_mode, pdn->eeh_config_addr,
 		pdn->eeh_pe_config_addr, pdn->node->full_name);
 	dent += 3;
-	struct device_node *pc = pdn->node->child;
-	while (pc) {
+	for (pc = NULL; (pc = of_get_next_child(pdn->node, pc));)
 		print_device_node_tree(PCI_DN(pc), dent);
-		pc = pc->sibling;
-	}
 }
 #endif
 
-- 
1.5.2.rc1.1884.g59b20

Re: [PATCH 09/16] Use of_get_next_parent() in axon_msi.c

[Stephen Rothwell]

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

On Fri, 26 Oct 2007 16:54:41 +1000 (EST) Michael Ellerman <michael@ellerman.id.au> wrote:
>
> +++ b/arch/powerpc/platforms/cell/axon_msi.c
> @@ -125,7 +125,7 @@ static struct axon_msic *find_msi_translator(struct pci_dev *dev)
>  		return NULL;
>  	}
>  
> -	for (; dn; tmp = of_get_parent(dn), of_node_put(dn), dn = tmp) {
> +	for (; dn; dn = of_get_next_parent(dn)) {
>  		ph = of_get_property(dn, "msi-translator", NULL);
>  		if (ph)
>  			break;

You no longer assign anything to tmp, but just below here, you may jump
to out_error: which will do an of_node_put(tmp).  So you need to
initialise tmp or have another error goto label.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 11/16] Use of_get_next_child() in eeh_restore_bars()

[Stephen Rothwell]

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

On Fri, 26 Oct 2007 16:54:43 +1000 (EST) Michael Ellerman <michael@ellerman.id.au> wrote:
>
> +++ b/arch/powerpc/platforms/pseries/eeh.c
> @@ -841,11 +841,8 @@ void eeh_restore_bars(struct pci_dn *pdn)
>  	if ((pdn->eeh_mode & EEH_MODE_SUPPORTED) && !IS_BRIDGE(pdn->class_code))
>  		__restore_bars (pdn);
>  
> -	dn = pdn->node->child;
> -	while (dn) {
> +	for (dn = NULL; (dn = of_get_next_child(pdn->node, dn));)

Just wondering if we need

#define for_each_child_node(dn, parent) \
	for (dn = of_get_next_child(parent, NULL); dn; \
		dn = of_get_next_child(parent, dn))

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 01/16] Add of_get_next_parent()

[David Miller]

From: Michael Ellerman <michael@ellerman.id.au>
Date: Fri, 26 Oct 2007 16:54:31 +1000 (EST)

> Iterating through a device node's parents is simple enough, but dealing
> with the refcounts properly is a little ugly, and replicating that logic
> is asking for someone to get it wrong or forget it all together, eg:
> 
> while (dn != NULL) {
> 	/* loop body */
> 	tmp = of_get_parent(dn);
> 	of_node_put(dn);
> 	dn = tmp;
> }
> 
> So add of_get_next_parent(), inspired by of_get_next_child(). The contract
> is that it returns the parent and drops the reference on the current node,
> this makes the loop look like:
> 
> while (dn != NULL) {
> 	/* loop body */
> 	dn = of_get_next_parent(dn);
> }
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

Looks good to me:

Acked-by: David S. Miller <davem@davemloft.net>

Re: [PATCH 11/16] Use of_get_next_child() in eeh_restore_bars()

[Michael Ellerman]

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


On Fri, 2007-10-26 at 17:29 +1000, Stephen Rothwell wrote:
> On Fri, 26 Oct 2007 16:54:43 +1000 (EST) Michael Ellerman <michael@ellerman.id.au> wrote:
> >
> > +++ b/arch/powerpc/platforms/pseries/eeh.c
> > @@ -841,11 +841,8 @@ void eeh_restore_bars(struct pci_dn *pdn)
> >  	if ((pdn->eeh_mode & EEH_MODE_SUPPORTED) && !IS_BRIDGE(pdn->class_code))
> >  		__restore_bars (pdn);
> >  
> > -	dn = pdn->node->child;
> > -	while (dn) {
> > +	for (dn = NULL; (dn = of_get_next_child(pdn->node, dn));)
> 
> Just wondering if we need
> 
> #define for_each_child_node(dn, parent) \
> 	for (dn = of_get_next_child(parent, NULL); dn; \
> 		dn = of_get_next_child(parent, dn))

Hmm, perhaps. I count ~30 places we could use it under arch/powerpc, and
about another ~20 in drivers/.

If no one objects, I guess I'll get to it, sigh ... another Yak.

Should we perhaps make it for_each_child_device_node() ?
Otherwise it's slightly non obvious what kind of node it's talking
about, especially in driver code.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

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

Re: [PATCH 11/16] Use of_get_next_child() in eeh_restore_bars()

[Linas Vepstas]

On Mon, Oct 29, 2007 at 02:46:13PM +1100, Michael Ellerman wrote:
> 
> On Fri, 2007-10-26 at 17:29 +1000, Stephen Rothwell wrote:
> > On Fri, 26 Oct 2007 16:54:43 +1000 (EST) Michael Ellerman <michael@ellerman.id.au> wrote:
> > >
> > > -	dn = pdn->node->child;
> > > -	while (dn) {
> > > +	for (dn = NULL; (dn = of_get_next_child(pdn->node, dn));)
> > 
> > Just wondering if we need
> > 
> > #define for_each_child_node(dn, parent) \
> > 	for (dn = of_get_next_child(parent, NULL); dn; \
> > 		dn = of_get_next_child(parent, dn))

Yes, I like this much better too, if for no other reason than
the for-loop tructure is more orthodox.

> Should we perhaps make it for_each_child_device_node() ?

foreach_of_device_node_child() or

of_foreach_device_node_child()