Re: [PATCH v8 40/45] drivers/of: Split unflatten_dt_node()

[Gavin Shan]

On Wed, Feb 17, 2016 at 08:30:42AM -0600, Rob Herring wrote:
>On Tue, Feb 16, 2016 at 9:44 PM, Gavin Shan  wrote:
>> The function unflatten_dt_node() is called recursively to unflatten
>> device nodes and properties in the FDT blob. It looks complicated
>> and hard to be understood.
>>
>> This splits the function into 3 functions: populate_properties(),
>> populate_node() and unflatten_dt_node(). populate_properties(),
>> which is called by populate_node(), creates properties for the
>> indicated device node. The later one creates the device nodes
>> from FDT blob. populate_node() gets the offset in FDT blob for
>> next device nodes and then calls populate_node(). No logical
>> changes introduced.
>>
>> Signed-off-by: Gavin Shan 
>> ---
>>  drivers/of/fdt.c | 249 
>> ---
>>  1 file changed, 147 insertions(+), 102 deletions(-)
>
>One nit, otherwise:
>
>Acked-by: Rob Herring 
>
>[...]
>
>> +   /* And we process the "ibm,phandle" property
>> +* used in pSeries dynamic device tree
>> +* stuff
>> +*/
>> +   if (!strcmp(pname, "ibm,phandle"))
>> +   np->phandle = be32_to_cpup(val);
>> +
>> +   pp->name   = (char *)pname;
>> +   pp->length = sz;
>> +   pp->value  = (__be32 *)val;
>
>This cast should not be needed.
>

It's needed. Otherwise, we will have warning. So I will keep it. I just
went through this one for next revision and sorry for late response.

drivers/of/fdt.c:225:14: warning: assignment discards ‘const’ qualifier from 
pointer target type
   pp->value  = val;
  ^

Thanks,
Gavin

>> +   *pprev = pp;
>> +   pprev  = >next;
>> +   }
>

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v8 40/45] drivers/of: Split unflatten_dt_node()

[Gavin Shan]

On Wed, Feb 17, 2016 at 08:30:42AM -0600, Rob Herring wrote:
>On Tue, Feb 16, 2016 at 9:44 PM, Gavin Shan  wrote:
>> The function unflatten_dt_node() is called recursively to unflatten
>> device nodes and properties in the FDT blob. It looks complicated
>> and hard to be understood.
>>
>> This splits the function into 3 functions: populate_properties(),
>> populate_node() and unflatten_dt_node(). populate_properties(),
>> which is called by populate_node(), creates properties for the
>> indicated device node. The later one creates the device nodes
>> from FDT blob. populate_node() gets the offset in FDT blob for
>> next device nodes and then calls populate_node(). No logical
>> changes introduced.
>>
>> Signed-off-by: Gavin Shan 
>> ---
>>  drivers/of/fdt.c | 249 
>> ---
>>  1 file changed, 147 insertions(+), 102 deletions(-)
>
>One nit, otherwise:
>
>Acked-by: Rob Herring 
>
>[...]
>
>> +   /* And we process the "ibm,phandle" property
>> +* used in pSeries dynamic device tree
>> +* stuff
>> +*/
>> +   if (!strcmp(pname, "ibm,phandle"))
>> +   np->phandle = be32_to_cpup(val);
>> +
>> +   pp->name   = (char *)pname;
>> +   pp->length = sz;
>> +   pp->value  = (__be32 *)val;
>
>This cast should not be needed.
>

Rob, very sorry to response so lately. I will fix it up in next revision.

>> +   *pprev = pp;
>> +   pprev  = >next;
>> +   }
>

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v8 40/45] drivers/of: Split unflatten_dt_node()

[Rob Herring]

On Tue, Feb 16, 2016 at 9:44 PM, Gavin Shan  wrote:
> The function unflatten_dt_node() is called recursively to unflatten
> device nodes and properties in the FDT blob. It looks complicated
> and hard to be understood.
>
> This splits the function into 3 functions: populate_properties(),
> populate_node() and unflatten_dt_node(). populate_properties(),
> which is called by populate_node(), creates properties for the
> indicated device node. The later one creates the device nodes
> from FDT blob. populate_node() gets the offset in FDT blob for
> next device nodes and then calls populate_node(). No logical
> changes introduced.
>
> Signed-off-by: Gavin Shan 
> ---
>  drivers/of/fdt.c | 249 
> ---
>  1 file changed, 147 insertions(+), 102 deletions(-)

One nit, otherwise:

Acked-by: Rob Herring 

[...]

> +   /* And we process the "ibm,phandle" property
> +* used in pSeries dynamic device tree
> +* stuff
> +*/
> +   if (!strcmp(pname, "ibm,phandle"))
> +   np->phandle = be32_to_cpup(val);
> +
> +   pp->name   = (char *)pname;
> +   pp->length = sz;
> +   pp->value  = (__be32 *)val;

This cast should not be needed.

> +   *pprev = pp;
> +   pprev  = >next;
> +   }
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v8 40/45] drivers/of: Split unflatten_dt_node()

[Gavin Shan]

The function unflatten_dt_node() is called recursively to unflatten
device nodes and properties in the FDT blob. It looks complicated
and hard to be understood.

This splits the function into 3 functions: populate_properties(),
populate_node() and unflatten_dt_node(). populate_properties(),
which is called by populate_node(), creates properties for the
indicated device node. The later one creates the device nodes
from FDT blob. populate_node() gets the offset in FDT blob for
next device nodes and then calls populate_node(). No logical
changes introduced.

Signed-off-by: Gavin Shan 
---
 drivers/of/fdt.c | 249 ---
 1 file changed, 147 insertions(+), 102 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 655f79d..3c69002 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -161,39 +161,127 @@ static void *unflatten_dt_alloc(void **mem, unsigned 
long size,
return res;
 }
 
-/**
- * unflatten_dt_node - Alloc and populate a device_node from the flat tree
- * @blob: The parent device tree blob
- * @mem: Memory chunk to use for allocating device nodes and properties
- * @poffset: pointer to node in flat tree
- * @dad: Parent struct device_node
- * @nodepp: The device_node tree created by the call
- * @fpsize: Size of the node path up at the current depth.
- * @dryrun: If true, do not allocate device nodes but still calculate needed
- * memory size
- */
-static void * unflatten_dt_node(const void *blob,
-   void *mem,
-   int *poffset,
-   struct device_node *dad,
-   struct device_node **nodepp,
-   unsigned long fpsize,
+static void populate_properties(const void *blob,
+   int offset,
+   void **mem,
+   struct device_node *np,
+   const char *nodename,
bool dryrun)
 {
-   const __be32 *p;
+   struct property *pp, **pprev = NULL;
+   int cur;
+   bool has_name = false;
+
+   pprev = >properties;
+   for (cur = fdt_first_property_offset(blob, offset);
+cur >= 0;
+cur = fdt_next_property_offset(blob, cur)) {
+   const __be32 *val;
+   const char *pname;
+   u32 sz;
+
+   val = fdt_getprop_by_offset(blob, cur, , );
+   if (!val) {
+   pr_warn("%s: Cannot locate property at 0x%x\n",
+   __func__, cur);
+   continue;
+   }
+
+   if (!pname) {
+   pr_warn("%s: Cannot find property name at 0x%x\n",
+   __func__, cur);
+   continue;
+   }
+
+   if (!strcmp(pname, "name"))
+   has_name = true;
+
+   pp = unflatten_dt_alloc(mem, sizeof(struct property),
+   __alignof__(struct property));
+   if (dryrun)
+   continue;
+
+   /* We accept flattened tree phandles either in
+* ePAPR-style "phandle" properties, or the
+* legacy "linux,phandle" properties.  If both
+* appear and have different values, things
+* will get weird. Don't do that.
+*/
+   if (!strcmp(pname, "phandle") ||
+   !strcmp(pname, "linux,phandle")) {
+   if (!np->phandle)
+   np->phandle = be32_to_cpup(val);
+   }
+
+   /* And we process the "ibm,phandle" property
+* used in pSeries dynamic device tree
+* stuff
+*/
+   if (!strcmp(pname, "ibm,phandle"))
+   np->phandle = be32_to_cpup(val);
+
+   pp->name   = (char *)pname;
+   pp->length = sz;
+   pp->value  = (__be32 *)val;
+   *pprev = pp;
+   pprev  = >next;
+   }
+
+   /* With version 0x10 we may not have the name property,
+* recreate it here from the unit name if absent
+*/
+   if (!has_name) {
+   const char *p = nodename, *ps = p, *pa = NULL;
+   int len;
+
+   while (*p) {
+   if ((*p) == '@')
+   pa = p;
+   else if ((*p) == '/')
+   ps = p + 1;
+   p++;
+   }
+
+   if (pa < ps)
+   pa = p;
+   len = (pa - ps) + 1;
+   pp = unflatten_dt_alloc(mem, sizeof(struct property) + len,
+   __alignof__(struct property));
+