[PATCH 0/8] powerpc/pseries: fix/cleanup broken mobility device-tree update code

[PATCH 0/8] powerpc/pseries: fix/cleanup broken mobility device-tree update code

[Tyrel Datwyler]

Currently we still perform updating of the device tree after a suspend or
migration from drmgr in userspace through the ugly /proc/ppc64/ofdt interface.
Code exists to do this update from within the kernel, but testing revealed it
to be severely broken to the point that the system hardlocks up stuck in an
infinite loop with irqs disabled.

The first 4 patches in the series fix several issues encountered tyring to
perform node property updates as dictated by the ibm,update-properties rtas
call. The next 2 patches refactor the logic of dlpar_configure_connector to
enusre that nodes created in a device nodes subtree are initialized
correctly. The remainder of the patchset fixes a couple problems with node
removal.

Tyrel Datwyler (8):
  powerpc/pseries: fix creation of loop in device node property list
  powerpc/pseries: fix over writing of rtas return code in
    update_dt_node
  powerpc/pseries: pack update_props_workarea to map correctly to rtas
    buffer header
  powerpc/pseries: fix parsing of initial node path in update_dt_node
  powerpc/pseries: do all node initialization in dlpar_parse_cc_node
  powerpc/pseries: make dlpar_configure_connector parent node aware
  powerpc/pseries: add mising of_node_put in delete_dt_node
  powerpc/pseries: child nodes are not detached by dlpar_detach_node

 arch/powerpc/platforms/pseries/dlpar.c    | 67 ++++++++++++++++++-------------
 arch/powerpc/platforms/pseries/mobility.c | 45 ++++++++++-----------
 arch/powerpc/platforms/pseries/pseries.h  |  2 +-
 3 files changed, 62 insertions(+), 52 deletions(-)

-- 
1.7.12.4

[PATCH 1/8] powerpc/pseries: fix creation of loop in device node property list

[Tyrel Datwyler]

The update_dt_prop helper function fails to set the IN/OUT parameter prop to
NULL after a complete property has been parsed from the work area returned by
the ibm,update-properties rtas function. This results in the property list of
the device node being updated is corrupted and becomes a loop since the same
property structure is used repeatedly.

Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/mobility.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index 3d01eee..f28abee 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -119,7 +119,7 @@ static int update_dt_property(struct device_node *dn, struct property **prop,
 
 	if (!more) {
 		of_update_property(dn, new_prop);
-		new_prop = NULL;
+		*prop = NULL;
 	}
 
 	return 0;
-- 
1.7.12.4

Re: [PATCH 1/8] powerpc/pseries: fix creation of loop in device node property list

[Nathan Fontenot]

On 08/15/2013 12:23 AM, Tyrel Datwyler wrote:
> The update_dt_prop helper function fails to set the IN/OUT parameter prop to
> NULL after a complete property has been parsed from the work area returned by
> the ibm,update-properties rtas function. This results in the property list of
> the device node being updated is corrupted and becomes a loop since the same
> property structure is used repeatedly.
> 
> Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>

Acked-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>

> ---
>  arch/powerpc/platforms/pseries/mobility.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
> index 3d01eee..f28abee 100644
> --- a/arch/powerpc/platforms/pseries/mobility.c
> +++ b/arch/powerpc/platforms/pseries/mobility.c
> @@ -119,7 +119,7 @@ static int update_dt_property(struct device_node *dn, struct property **prop,
>  
>  	if (!more) {
>  		of_update_property(dn, new_prop);
> -		new_prop = NULL;
> +		*prop = NULL;
>  	}
>  
>  	return 0;
> 

[PATCH 2/8] powerpc/pseries: fix over writing of rtas return code in update_dt_node

[Tyrel Datwyler]

The rc variable is initially used to store the return code from the
ibm,update-properties rtas call which returns 0 or 1 on success. A return
code of 1 indicates that ibm,update-properties must be called again for the
node. However, the rc variable is overwritten by a call to update_dt_prop
which returns 0 on success. This results in ibm,update-properties not being
called again for the given node when the rtas call rc was previously 1.

Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/mobility.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index f28abee..aaae85d 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -130,7 +130,7 @@ static int update_dt_node(u32 phandle, s32 scope)
 	struct update_props_workarea *upwa;
 	struct device_node *dn;
 	struct property *prop = NULL;
-	int i, rc;
+	int i, rc, rtas_rc;
 	char *prop_data;
 	char *rtas_buf;
 	int update_properties_token;
@@ -154,9 +154,9 @@ static int update_dt_node(u32 phandle, s32 scope)
 	upwa->phandle = phandle;
 
 	do {
-		rc = mobility_rtas_call(update_properties_token, rtas_buf,
+		rtas_rc = mobility_rtas_call(update_properties_token, rtas_buf,
 					scope);
-		if (rc < 0)
+		if (rtas_rc < 0)
 			break;
 
 		prop_data = rtas_buf + sizeof(*upwa);
@@ -202,7 +202,7 @@ static int update_dt_node(u32 phandle, s32 scope)
 				prop_data += vd;
 			}
 		}
-	} while (rc == 1);
+	} while (rtas_rc == 1);
 
 	of_node_put(dn);
 	kfree(rtas_buf);
-- 
1.7.12.4

Re: [PATCH 2/8] powerpc/pseries: fix over writing of rtas return code in update_dt_node

[Nathan Fontenot]

On 08/15/2013 12:23 AM, Tyrel Datwyler wrote:
> The rc variable is initially used to store the return code from the
> ibm,update-properties rtas call which returns 0 or 1 on success. A return
> code of 1 indicates that ibm,update-properties must be called again for the
> node. However, the rc variable is overwritten by a call to update_dt_prop
> which returns 0 on success. This results in ibm,update-properties not being
> called again for the given node when the rtas call rc was previously 1.
> 
> Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>

Acked-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>

> ---
>  arch/powerpc/platforms/pseries/mobility.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
> index f28abee..aaae85d 100644
> --- a/arch/powerpc/platforms/pseries/mobility.c
> +++ b/arch/powerpc/platforms/pseries/mobility.c
> @@ -130,7 +130,7 @@ static int update_dt_node(u32 phandle, s32 scope)
>  	struct update_props_workarea *upwa;
>  	struct device_node *dn;
>  	struct property *prop = NULL;
> -	int i, rc;
> +	int i, rc, rtas_rc;
>  	char *prop_data;
>  	char *rtas_buf;
>  	int update_properties_token;
> @@ -154,9 +154,9 @@ static int update_dt_node(u32 phandle, s32 scope)
>  	upwa->phandle = phandle;
>  
>  	do {
> -		rc = mobility_rtas_call(update_properties_token, rtas_buf,
> +		rtas_rc = mobility_rtas_call(update_properties_token, rtas_buf,
>  					scope);
> -		if (rc < 0)
> +		if (rtas_rc < 0)
>  			break;
>  
>  		prop_data = rtas_buf + sizeof(*upwa);
> @@ -202,7 +202,7 @@ static int update_dt_node(u32 phandle, s32 scope)
>  				prop_data += vd;
>  			}
>  		}
> -	} while (rc == 1);
> +	} while (rtas_rc == 1);
>  
>  	of_node_put(dn);
>  	kfree(rtas_buf);
> 

[PATCH 3/8] powerpc/pseries: pack update_props_workarea to map correctly to rtas buffer header

[Tyrel Datwyler]

The work area buffer returned by the ibm,update-properties rtas call contains
20 bytes of header information prior to the property value descriptor data.
Currently update_dt_node tries to advance over this header using sizeof(upwa).
The update_props_workarea struct contains 20 bytes worth of fields, that map
to the relevant header data, but the sizeof the structure is 24 bytes due to
4 bytes of padding at the end of the structure. Packing the structure ensures
that we don't advance too far over the rtas buffer.

Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/mobility.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index aaae85d..023e354 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -28,7 +28,7 @@ struct update_props_workarea {
 	u32 state;
 	u64 reserved;
 	u32 nprops;
-};
+} __packed;
 
 #define NODE_ACTION_MASK	0xff000000
 #define NODE_COUNT_MASK		0x00ffffff
-- 
1.7.12.4

Re: [PATCH 3/8] powerpc/pseries: pack update_props_workarea to map correctly to rtas buffer header

[Nathan Fontenot]

On 08/15/2013 12:23 AM, Tyrel Datwyler wrote:
> The work area buffer returned by the ibm,update-properties rtas call contains
> 20 bytes of header information prior to the property value descriptor data.
> Currently update_dt_node tries to advance over this header using sizeof(upwa).
> The update_props_workarea struct contains 20 bytes worth of fields, that map
> to the relevant header data, but the sizeof the structure is 24 bytes due to
> 4 bytes of padding at the end of the structure. Packing the structure ensures
> that we don't advance too far over the rtas buffer.
> 
> Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>

Acked-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>

> ---
>  arch/powerpc/platforms/pseries/mobility.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
> index aaae85d..023e354 100644
> --- a/arch/powerpc/platforms/pseries/mobility.c
> +++ b/arch/powerpc/platforms/pseries/mobility.c
> @@ -28,7 +28,7 @@ struct update_props_workarea {
>  	u32 state;
>  	u64 reserved;
>  	u32 nprops;
> -};
> +} __packed;
>  
>  #define NODE_ACTION_MASK	0xff000000
>  #define NODE_COUNT_MASK		0x00ffffff
> 

[PATCH 4/8] powerpc/pseries: fix parsing of initial node path in update_dt_node

[Tyrel Datwyler]

On the first call to ibm,update-properties for a node the first property
returned is the full node path. Currently this is not parsed correctly by the
update_dt_node function. Commit 2e9b7b0 attempted to fix this, but was
incorrect as it made a wrong assumption about the layout of the first
property in the work area. Further, if ibm,update-properties must be called
multiple times for the same node this special property should only be skipped
after the initial call. The first property descriptor returned consists of
the property name, property value length, and property value. The property
name is an empty string, property length is encoded in 4 byte integer, and
the property value is the node path.

Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/mobility.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index 023e354..ed5426f 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -161,18 +161,19 @@ static int update_dt_node(u32 phandle, s32 scope)
 
 		prop_data = rtas_buf + sizeof(*upwa);
 
-		/* The first element of the buffer is the path of the node
-		 * being updated in the form of a 8 byte string length
-		 * followed by the string. Skip past this to get to the
-		 * properties being updated.
+		/* On the first call to ibm,update-properties for a node the
+		 * the first property value descriptor contains an empty
+		 * property name, the property value length encoded as u32,
+		 * and the property value is the node path being updated.
 		 */
-		vd = *prop_data++;
-		prop_data += vd;
+		if (*prop_data == 0) {
+			prop_data++;
+			vd = *(u32 *)prop_data;
+			prop_data += vd + sizeof(vd);
+			upwa->nprops--;
+		}
 
-		/* The path we skipped over is counted as one of the elements
-		 * returned so start counting at one.
-		 */
-		for (i = 1; i < upwa->nprops; i++) {
+		for (i = 0; i < upwa->nprops; i++) {
 			char *prop_name;
 
 			prop_name = prop_data;
-- 
1.7.12.4

Re: [PATCH 4/8] powerpc/pseries: fix parsing of initial node path in update_dt_node

[Nathan Fontenot]

On 08/15/2013 12:23 AM, Tyrel Datwyler wrote:
> On the first call to ibm,update-properties for a node the first property
> returned is the full node path. Currently this is not parsed correctly by the
> update_dt_node function. Commit 2e9b7b0 attempted to fix this, but was
> incorrect as it made a wrong assumption about the layout of the first
> property in the work area. Further, if ibm,update-properties must be called
> multiple times for the same node this special property should only be skipped
> after the initial call. The first property descriptor returned consists of
> the property name, property value length, and property value. The property
> name is an empty string, property length is encoded in 4 byte integer, and
> the property value is the node path.
> 
> Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>

Acked-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>

> ---
>  arch/powerpc/platforms/pseries/mobility.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
> index 023e354..ed5426f 100644
> --- a/arch/powerpc/platforms/pseries/mobility.c
> +++ b/arch/powerpc/platforms/pseries/mobility.c
> @@ -161,18 +161,19 @@ static int update_dt_node(u32 phandle, s32 scope)
>  
>  		prop_data = rtas_buf + sizeof(*upwa);
>  
> -		/* The first element of the buffer is the path of the node
> -		 * being updated in the form of a 8 byte string length
> -		 * followed by the string. Skip past this to get to the
> -		 * properties being updated.
> +		/* On the first call to ibm,update-properties for a node the
> +		 * the first property value descriptor contains an empty
> +		 * property name, the property value length encoded as u32,
> +		 * and the property value is the node path being updated.
>  		 */
> -		vd = *prop_data++;
> -		prop_data += vd;
> +		if (*prop_data == 0) {
> +			prop_data++;
> +			vd = *(u32 *)prop_data;
> +			prop_data += vd + sizeof(vd);
> +			upwa->nprops--;
> +		}
>  
> -		/* The path we skipped over is counted as one of the elements
> -		 * returned so start counting at one.
> -		 */
> -		for (i = 1; i < upwa->nprops; i++) {
> +		for (i = 0; i < upwa->nprops; i++) {
>  			char *prop_name;
>  
>  			prop_name = prop_data;
> 

[PATCH 5/8] powerpc/pseries: do all node initialization in dlpar_parse_cc_node

[Tyrel Datwyler]

Currently the OF_DYNAMIC and kref initialization for a node happens in
dlpar_attach_node. However, a node passed to dlpar_attach_node may be a tree
containing child nodes, and no initialization traversal is done on the
tree. Since the children never get their kref initialized or the OF_DYNAMIC
flag set these nodes are prevented from ever being released from memory
should they become detached. This initialization step is better done at the
time each node is allocated in dlpar_parse_cc_node.

Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/dlpar.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index a1a7b9a..c855233 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -83,6 +83,9 @@ static struct device_node *dlpar_parse_cc_node(struct cc_workarea *ccwa)
 		return NULL;
 	}
 
+	of_node_set_flag(dn, OF_DYNAMIC);
+	kref_init(&dn->kref);
+
 	return dn;
 }
 
@@ -256,8 +259,6 @@ int dlpar_attach_node(struct device_node *dn)
 {
 	int rc;
 
-	of_node_set_flag(dn, OF_DYNAMIC);
-	kref_init(&dn->kref);
 	dn->parent = derive_parent(dn->full_name);
 	if (!dn->parent)
 		return -ENOMEM;
-- 
1.7.12.4

Re: [PATCH 5/8] powerpc/pseries: do all node initialization in dlpar_parse_cc_node

[Nathan Fontenot]

On 08/15/2013 12:23 AM, Tyrel Datwyler wrote:
> Currently the OF_DYNAMIC and kref initialization for a node happens in
> dlpar_attach_node. However, a node passed to dlpar_attach_node may be a tree
> containing child nodes, and no initialization traversal is done on the
> tree. Since the children never get their kref initialized or the OF_DYNAMIC
> flag set these nodes are prevented from ever being released from memory
> should they become detached. This initialization step is better done at the
> time each node is allocated in dlpar_parse_cc_node.
> 
> Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>

Acked-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>

> ---
>  arch/powerpc/platforms/pseries/dlpar.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
> index a1a7b9a..c855233 100644
> --- a/arch/powerpc/platforms/pseries/dlpar.c
> +++ b/arch/powerpc/platforms/pseries/dlpar.c
> @@ -83,6 +83,9 @@ static struct device_node *dlpar_parse_cc_node(struct cc_workarea *ccwa)
>  		return NULL;
>  	}
>  
> +	of_node_set_flag(dn, OF_DYNAMIC);
> +	kref_init(&dn->kref);
> +
>  	return dn;
>  }
>  
> @@ -256,8 +259,6 @@ int dlpar_attach_node(struct device_node *dn)
>  {
>  	int rc;
>  
> -	of_node_set_flag(dn, OF_DYNAMIC);
> -	kref_init(&dn->kref);
>  	dn->parent = derive_parent(dn->full_name);
>  	if (!dn->parent)
>  		return -ENOMEM;
> 

[PATCH 6/8] powerpc/pseries: make dlpar_configure_connector parent node aware

[Tyrel Datwyler]

Currently the device nodes created in the device subtree returned by a call to
dlpar_configure_connector are all named in the root node. This is because the
the node name in the work area returned by ibm,configure-connector rtas call
only contains the node name and not the entire node path. Passing the parent
node where the new subtree will be created to dlpar_configure_connector allows
the correct node path to be prefixed in the full_name field.

Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/dlpar.c    | 55 ++++++++++++++++---------------
 arch/powerpc/platforms/pseries/mobility.c | 11 +++----
 arch/powerpc/platforms/pseries/pseries.h  |  2 +-
 3 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index c855233..4ea667d 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -63,21 +63,24 @@ static struct property *dlpar_parse_cc_property(struct cc_workarea *ccwa)
 	return prop;
 }
 
-static struct device_node *dlpar_parse_cc_node(struct cc_workarea *ccwa)
+static struct device_node *dlpar_parse_cc_node(struct cc_workarea *ccwa,
+					       const char *path)
 {
 	struct device_node *dn;
 	char *name;
 
+	/* If parent node path is "/" advance path to NULL terminator to
+	 * prevent double leading slashs in full_name.
+	 */
+	if (!path[1])
+		path++;
+
 	dn = kzalloc(sizeof(*dn), GFP_KERNEL);
 	if (!dn)
 		return NULL;
 
-	/* The configure connector reported name does not contain a
-	 * preceding '/', so we allocate a buffer large enough to
-	 * prepend this to the full_name.
-	 */
 	name = (char *)ccwa + ccwa->name_offset;
-	dn->full_name = kasprintf(GFP_KERNEL, "/%s", name);
+	dn->full_name = kasprintf(GFP_KERNEL, "%s/%s", path, name);
 	if (!dn->full_name) {
 		kfree(dn);
 		return NULL;
@@ -123,7 +126,8 @@ void dlpar_free_cc_nodes(struct device_node *dn)
 #define CALL_AGAIN	-2
 #define ERR_CFG_USE     -9003
 
-struct device_node *dlpar_configure_connector(u32 drc_index)
+struct device_node *dlpar_configure_connector(u32 drc_index,
+					      struct device_node *parent)
 {
 	struct device_node *dn;
 	struct device_node *first_dn = NULL;
@@ -132,6 +136,7 @@ struct device_node *dlpar_configure_connector(u32 drc_index)
 	struct property *last_property = NULL;
 	struct cc_workarea *ccwa;
 	char *data_buf;
+	const char *parent_path = parent->full_name;
 	int cc_token;
 	int rc = -1;
 
@@ -165,7 +170,7 @@ struct device_node *dlpar_configure_connector(u32 drc_index)
 			break;
 
 		case NEXT_SIBLING:
-			dn = dlpar_parse_cc_node(ccwa);
+			dn = dlpar_parse_cc_node(ccwa, parent_path);
 			if (!dn)
 				goto cc_error;
 
@@ -175,13 +180,17 @@ struct device_node *dlpar_configure_connector(u32 drc_index)
 			break;
 
 		case NEXT_CHILD:
-			dn = dlpar_parse_cc_node(ccwa);
+			if (first_dn)
+				parent_path = last_dn->full_name;
+
+			dn = dlpar_parse_cc_node(ccwa, parent_path);
 			if (!dn)
 				goto cc_error;
 
-			if (!first_dn)
+			if (!first_dn) {
+				dn->parent = parent;
 				first_dn = dn;
-			else {
+			} else {
 				dn->parent = last_dn;
 				if (last_dn)
 					last_dn->child = dn;
@@ -205,6 +214,7 @@ struct device_node *dlpar_configure_connector(u32 drc_index)
 
 		case PREV_PARENT:
 			last_dn = last_dn->parent;
+			parent_path = last_dn->parent->full_name;
 			break;
 
 		case CALL_AGAIN:
@@ -383,9 +393,8 @@ out:
 
 static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
 {
-	struct device_node *dn;
+	struct device_node *dn, *parent;
 	unsigned long drc_index;
-	char *cpu_name;
 	int rc;
 
 	cpu_hotplug_driver_lock();
@@ -395,25 +404,19 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
 		goto out;
 	}
 
-	dn = dlpar_configure_connector(drc_index);
-	if (!dn) {
-		rc = -EINVAL;
+	parent = of_find_node_by_path("/cpus");
+	if (!parent) {
+		rc = -ENODEV;
 		goto out;
 	}
 
-	/* configure-connector reports cpus as living in the base
-	 * directory of the device tree.  CPUs actually live in the
-	 * cpus directory so we need to fixup the full_name.
-	 */
-	cpu_name = kasprintf(GFP_KERNEL, "/cpus%s", dn->full_name);
-	if (!cpu_name) {
-		dlpar_free_cc_nodes(dn);
-		rc = -ENOMEM;
+	dn = dlpar_configure_connector(drc_index, parent);
+	if (!dn) {
+		rc = -EINVAL;
 		goto out;
 	}
 
-	kfree(dn->full_name);
-	dn->full_name = cpu_name;
+	of_node_put(parent);
 
 	rc = dlpar_acquire_drc(drc_index);
 	if (rc) {
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index ed5426f..ff102e2 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -216,17 +216,14 @@ static int add_dt_node(u32 parent_phandle, u32 drc_index)
 	struct device_node *parent_dn;
 	int rc;
 
-	dn = dlpar_configure_connector(drc_index);
-	if (!dn)
+	parent_dn = of_find_node_by_phandle(parent_phandle);
+	if (!parent_dn)
 		return -ENOENT;
 
-	parent_dn = of_find_node_by_phandle(parent_phandle);
-	if (!parent_dn) {
-		dlpar_free_cc_nodes(dn);
+	dn = dlpar_configure_connector(drc_index, parent_dn);
+	if (!dn)
 		return -ENOENT;
-	}
 
-	dn->parent = parent_dn;
 	rc = dlpar_attach_node(dn);
 	if (rc)
 		dlpar_free_cc_nodes(dn);
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index c2a3a25..defb3c9 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -56,7 +56,7 @@ extern void hvc_vio_init_early(void);
 /* Dynamic logical Partitioning/Mobility */
 extern void dlpar_free_cc_nodes(struct device_node *);
 extern void dlpar_free_cc_property(struct property *);
-extern struct device_node *dlpar_configure_connector(u32);
+extern struct device_node *dlpar_configure_connector(u32, struct device_node *);
 extern int dlpar_attach_node(struct device_node *);
 extern int dlpar_detach_node(struct device_node *);
 
-- 
1.7.12.4

Re: [PATCH 6/8] powerpc/pseries: make dlpar_configure_connector parent node aware

[Nathan Fontenot]

On 08/15/2013 12:23 AM, Tyrel Datwyler wrote:
> Currently the device nodes created in the device subtree returned by a call to
> dlpar_configure_connector are all named in the root node. This is because the
> the node name in the work area returned by ibm,configure-connector rtas call
> only contains the node name and not the entire node path. Passing the parent
> node where the new subtree will be created to dlpar_configure_connector allows
> the correct node path to be prefixed in the full_name field.
> 
> Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>

Acked-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>

> ---
>  arch/powerpc/platforms/pseries/dlpar.c    | 55 ++++++++++++++++---------------
>  arch/powerpc/platforms/pseries/mobility.c | 11 +++----
>  arch/powerpc/platforms/pseries/pseries.h  |  2 +-
>  3 files changed, 34 insertions(+), 34 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
> index c855233..4ea667d 100644
> --- a/arch/powerpc/platforms/pseries/dlpar.c
> +++ b/arch/powerpc/platforms/pseries/dlpar.c
> @@ -63,21 +63,24 @@ static struct property *dlpar_parse_cc_property(struct cc_workarea *ccwa)
>  	return prop;
>  }
>  
> -static struct device_node *dlpar_parse_cc_node(struct cc_workarea *ccwa)
> +static struct device_node *dlpar_parse_cc_node(struct cc_workarea *ccwa,
> +					       const char *path)
>  {
>  	struct device_node *dn;
>  	char *name;
>  
> +	/* If parent node path is "/" advance path to NULL terminator to
> +	 * prevent double leading slashs in full_name.
> +	 */
> +	if (!path[1])
> +		path++;
> +
>  	dn = kzalloc(sizeof(*dn), GFP_KERNEL);
>  	if (!dn)
>  		return NULL;
>  
> -	/* The configure connector reported name does not contain a
> -	 * preceding '/', so we allocate a buffer large enough to
> -	 * prepend this to the full_name.
> -	 */
>  	name = (char *)ccwa + ccwa->name_offset;
> -	dn->full_name = kasprintf(GFP_KERNEL, "/%s", name);
> +	dn->full_name = kasprintf(GFP_KERNEL, "%s/%s", path, name);
>  	if (!dn->full_name) {
>  		kfree(dn);
>  		return NULL;
> @@ -123,7 +126,8 @@ void dlpar_free_cc_nodes(struct device_node *dn)
>  #define CALL_AGAIN	-2
>  #define ERR_CFG_USE     -9003
>  
> -struct device_node *dlpar_configure_connector(u32 drc_index)
> +struct device_node *dlpar_configure_connector(u32 drc_index,
> +					      struct device_node *parent)
>  {
>  	struct device_node *dn;
>  	struct device_node *first_dn = NULL;
> @@ -132,6 +136,7 @@ struct device_node *dlpar_configure_connector(u32 drc_index)
>  	struct property *last_property = NULL;
>  	struct cc_workarea *ccwa;
>  	char *data_buf;
> +	const char *parent_path = parent->full_name;
>  	int cc_token;
>  	int rc = -1;
>  
> @@ -165,7 +170,7 @@ struct device_node *dlpar_configure_connector(u32 drc_index)
>  			break;
>  
>  		case NEXT_SIBLING:
> -			dn = dlpar_parse_cc_node(ccwa);
> +			dn = dlpar_parse_cc_node(ccwa, parent_path);
>  			if (!dn)
>  				goto cc_error;
>  
> @@ -175,13 +180,17 @@ struct device_node *dlpar_configure_connector(u32 drc_index)
>  			break;
>  
>  		case NEXT_CHILD:
> -			dn = dlpar_parse_cc_node(ccwa);
> +			if (first_dn)
> +				parent_path = last_dn->full_name;
> +
> +			dn = dlpar_parse_cc_node(ccwa, parent_path);
>  			if (!dn)
>  				goto cc_error;
>  
> -			if (!first_dn)
> +			if (!first_dn) {
> +				dn->parent = parent;
>  				first_dn = dn;
> -			else {
> +			} else {
>  				dn->parent = last_dn;
>  				if (last_dn)
>  					last_dn->child = dn;
> @@ -205,6 +214,7 @@ struct device_node *dlpar_configure_connector(u32 drc_index)
>  
>  		case PREV_PARENT:
>  			last_dn = last_dn->parent;
> +			parent_path = last_dn->parent->full_name;
>  			break;
>  
>  		case CALL_AGAIN:
> @@ -383,9 +393,8 @@ out:
>  
>  static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
>  {
> -	struct device_node *dn;
> +	struct device_node *dn, *parent;
>  	unsigned long drc_index;
> -	char *cpu_name;
>  	int rc;
>  
>  	cpu_hotplug_driver_lock();
> @@ -395,25 +404,19 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
>  		goto out;
>  	}
>  
> -	dn = dlpar_configure_connector(drc_index);
> -	if (!dn) {
> -		rc = -EINVAL;
> +	parent = of_find_node_by_path("/cpus");
> +	if (!parent) {
> +		rc = -ENODEV;
>  		goto out;
>  	}
>  
> -	/* configure-connector reports cpus as living in the base
> -	 * directory of the device tree.  CPUs actually live in the
> -	 * cpus directory so we need to fixup the full_name.
> -	 */
> -	cpu_name = kasprintf(GFP_KERNEL, "/cpus%s", dn->full_name);
> -	if (!cpu_name) {
> -		dlpar_free_cc_nodes(dn);
> -		rc = -ENOMEM;
> +	dn = dlpar_configure_connector(drc_index, parent);
> +	if (!dn) {
> +		rc = -EINVAL;
>  		goto out;
>  	}
>  
> -	kfree(dn->full_name);
> -	dn->full_name = cpu_name;
> +	of_node_put(parent);
>  
>  	rc = dlpar_acquire_drc(drc_index);
>  	if (rc) {
> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
> index ed5426f..ff102e2 100644
> --- a/arch/powerpc/platforms/pseries/mobility.c
> +++ b/arch/powerpc/platforms/pseries/mobility.c
> @@ -216,17 +216,14 @@ static int add_dt_node(u32 parent_phandle, u32 drc_index)
>  	struct device_node *parent_dn;
>  	int rc;
>  
> -	dn = dlpar_configure_connector(drc_index);
> -	if (!dn)
> +	parent_dn = of_find_node_by_phandle(parent_phandle);
> +	if (!parent_dn)
>  		return -ENOENT;
>  
> -	parent_dn = of_find_node_by_phandle(parent_phandle);
> -	if (!parent_dn) {
> -		dlpar_free_cc_nodes(dn);
> +	dn = dlpar_configure_connector(drc_index, parent_dn);
> +	if (!dn)
>  		return -ENOENT;
> -	}
>  
> -	dn->parent = parent_dn;
>  	rc = dlpar_attach_node(dn);
>  	if (rc)
>  		dlpar_free_cc_nodes(dn);
> diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
> index c2a3a25..defb3c9 100644
> --- a/arch/powerpc/platforms/pseries/pseries.h
> +++ b/arch/powerpc/platforms/pseries/pseries.h
> @@ -56,7 +56,7 @@ extern void hvc_vio_init_early(void);
>  /* Dynamic logical Partitioning/Mobility */
>  extern void dlpar_free_cc_nodes(struct device_node *);
>  extern void dlpar_free_cc_property(struct property *);
> -extern struct device_node *dlpar_configure_connector(u32);
> +extern struct device_node *dlpar_configure_connector(u32, struct device_node *);
>  extern int dlpar_attach_node(struct device_node *);
>  extern int dlpar_detach_node(struct device_node *);
>  
> 

[PATCH 7/8] powerpc/pseries: add mising of_node_put in delete_dt_node

[Tyrel Datwyler]

The node to be detached is retrieved via its phandle by a call to
of_find_node_by_phandle which increments the ref count. We need a matching
call to of_node_put to decrement the ref count and ensure the node is
actually freed.

Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/mobility.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index ff102e2..cde4e0a 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -62,6 +62,7 @@ static int delete_dt_node(u32 phandle)
 		return -ENOENT;
 
 	dlpar_detach_node(dn);
+	of_node_put(dn);
 	return 0;
 }
 
-- 
1.7.12.4

Re: [PATCH 7/8] powerpc/pseries: add mising of_node_put in delete_dt_node

[Nathan Fontenot]

On 08/15/2013 12:23 AM, Tyrel Datwyler wrote:
> The node to be detached is retrieved via its phandle by a call to
> of_find_node_by_phandle which increments the ref count. We need a matching
> call to of_node_put to decrement the ref count and ensure the node is
> actually freed.
> 
> Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>

Acked-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>

> ---
>  arch/powerpc/platforms/pseries/mobility.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
> index ff102e2..cde4e0a 100644
> --- a/arch/powerpc/platforms/pseries/mobility.c
> +++ b/arch/powerpc/platforms/pseries/mobility.c
> @@ -62,6 +62,7 @@ static int delete_dt_node(u32 phandle)
>  		return -ENOENT;
>  
>  	dlpar_detach_node(dn);
> +	of_node_put(dn);
>  	return 0;
>  }
>  
> 

[PATCH 8/8] powerpc/pseries: child nodes are not detached by dlpar_detach_node

[Tyrel Datwyler]

Calls to dlpar_detach_node do not iterate over child nodes detaching them as
well. By iterating and detaching the child nodes we ensure that they have the
OF_DETACHED flag set and that their reference counts are decremented such that
the node will be freed from memory by of_node_release.

Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/dlpar.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index 4ea667d..7cfdaae 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -286,8 +286,15 @@ int dlpar_attach_node(struct device_node *dn)
 
 int dlpar_detach_node(struct device_node *dn)
 {
+	struct device_node *child;
 	int rc;
 
+	child = of_get_next_child(dn, NULL);
+	while (child) {
+		dlpar_detach_node(child);
+		child = of_get_next_child(dn, child);
+	}
+
 	rc = of_detach_node(dn);
 	if (rc)
 		return rc;
-- 
1.7.12.4

Re: [PATCH 8/8] powerpc/pseries: child nodes are not detached by dlpar_detach_node

[Nathan Fontenot]

On 08/15/2013 12:23 AM, Tyrel Datwyler wrote:
> Calls to dlpar_detach_node do not iterate over child nodes detaching them as
> well. By iterating and detaching the child nodes we ensure that they have the
> OF_DETACHED flag set and that their reference counts are decremented such that
> the node will be freed from memory by of_node_release.
> 
> Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>

Acked-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>

> ---
>  arch/powerpc/platforms/pseries/dlpar.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
> index 4ea667d..7cfdaae 100644
> --- a/arch/powerpc/platforms/pseries/dlpar.c
> +++ b/arch/powerpc/platforms/pseries/dlpar.c
> @@ -286,8 +286,15 @@ int dlpar_attach_node(struct device_node *dn)
>  
>  int dlpar_detach_node(struct device_node *dn)
>  {
> +	struct device_node *child;
>  	int rc;
>  
> +	child = of_get_next_child(dn, NULL);
> +	while (child) {
> +		dlpar_detach_node(child);
> +		child = of_get_next_child(dn, child);
> +	}
> +
>  	rc = of_detach_node(dn);
>  	if (rc)
>  		return rc;
>