Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752936Ab1CLJLD (ORCPT ); Sat, 12 Mar 2011 04:11:03 -0500 Received: from mail-px0-f179.google.com ([209.85.212.179]:60438 "EHLO mail-px0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752352Ab1CLJK7 (ORCPT ); Sat, 12 Mar 2011 04:10:59 -0500 Date: Sat, 12 Mar 2011 02:10:56 -0700 From: Grant Likely To: Andres Salomon Cc: devicetree-discuss@lists.ozlabs.org, Daniel Drake , linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/3] of/flattree: use of_attach_node to build tree, and associated cleanups Message-ID: <20110312091056.GG9347@angua.secretlab.ca> References: <1299716167-9656-1-git-send-email-dilinger@queued.net> <1299716167-9656-4-git-send-email-dilinger@queued.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1299716167-9656-4-git-send-email-dilinger@queued.net> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8151 Lines: 208 On Wed, Mar 09, 2011 at 04:16:07PM -0800, Andres Salomon wrote: > Use a common function (of_attach_node) to build the device tree. This > simplifies the flat device tree creation a bit, and as an added bonus allows > us to drop a (now unused) field from the device_node struct. > > There are a few minor cleanups snuck in here as well (comment additions, etc). > > Signed-off-by: Andres Salomon In addition to my comment about changing the tree order on the last patch, unfortunately this patch will also break the newly added of_fdt_unflatten_tree(). of_fdt_unflatten_tree() allows a driver to unflatten a private dtb for its own use without it being attached to the global tree or the global list of all nodes. I had also forgotten about this. Shoot. The solution would be a variant of of_attach_node which accepts a private allnodes pointer. That would also help with the ordering issues because the order of the list could be explicitly reversed before assigning the value to the real allnodes pointer. Another possible option would be an optional 'tail' pointer argument to of_attach_node() which if present it would use as the insertion point for adding the node, which would preserve the current sort order. g. > --- > drivers/of/fdt.c | 42 ++++++++++++++++-------------------------- > include/linux/of.h | 1 - > 2 files changed, 16 insertions(+), 27 deletions(-) > > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c > index af824e7..e70cee8 100644 > --- a/drivers/of/fdt.c > +++ b/drivers/of/fdt.c > @@ -139,16 +139,17 @@ static void *unflatten_dt_alloc(unsigned long *mem, unsigned long size, > /** > * 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 > * @p: pointer to node in flat tree > * @dad: Parent struct device_node > - * @allnextpp: pointer to ->allnext from last allocated device_node > + * @size_scan: are we figuring out amount of memory to allocate? > * @fpsize: Size of the node path up at the current depth. > */ > unsigned long unflatten_dt_node(struct boot_param_header *blob, > unsigned long mem, > unsigned long *p, > struct device_node *dad, > - struct device_node ***allnextpp, > + bool size_scan, > unsigned long fpsize) > { > struct device_node *np; > @@ -195,7 +196,7 @@ unsigned long unflatten_dt_node(struct boot_param_header *blob, > > np = unflatten_dt_alloc(&mem, sizeof(struct device_node) + allocl, > __alignof__(struct device_node)); > - if (allnextpp) { > + if (!size_scan) { > memset(np, 0, sizeof(*np)); > np->full_name = ((char *)np) + sizeof(struct device_node); > if (new_format) { > @@ -217,19 +218,10 @@ unsigned long unflatten_dt_node(struct boot_param_header *blob, > } else > memcpy(np->full_name, pathp, l); > prev_pp = &np->properties; > - **allnextpp = np; > - *allnextpp = &np->allnext; > - if (dad != NULL) { > - np->parent = dad; > - /* we temporarily use the next field as `last_child'*/ > - if (dad->next == NULL) > - dad->child = np; > - else > - dad->next->sibling = np; > - dad->next = np; > - } > + np->parent = dad; > kref_init(&np->kref); > } > + /* process properties */ > while (1) { > u32 sz, noff; > char *pname; > @@ -258,7 +250,7 @@ unsigned long unflatten_dt_node(struct boot_param_header *blob, > l = strlen(pname) + 1; > pp = unflatten_dt_alloc(&mem, sizeof(struct property), > __alignof__(struct property)); > - if (allnextpp) { > + if (!size_scan) { > /* We accept flattened tree phandles either in > * ePAPR-style "phandle" properties, or the > * legacy "linux,phandle" properties. If both > @@ -301,7 +293,7 @@ unsigned long unflatten_dt_node(struct boot_param_header *blob, > sz = (pa - ps) + 1; > pp = unflatten_dt_alloc(&mem, sizeof(struct property) + sz, > __alignof__(struct property)); > - if (allnextpp) { > + if (!size_scan) { > pp->name = "name"; > pp->length = sz; > pp->value = pp + 1; > @@ -313,7 +305,7 @@ unsigned long unflatten_dt_node(struct boot_param_header *blob, > (char *)pp->value); > } > } > - if (allnextpp) { > + if (!size_scan) { > *prev_pp = NULL; > np->name = of_get_property(np, "name", NULL); > np->type = of_get_property(np, "device_type", NULL); > @@ -322,12 +314,14 @@ unsigned long unflatten_dt_node(struct boot_param_header *blob, > np->name = ""; > if (!np->type) > np->type = ""; > + > + of_attach_node(np); > } > while (tag == OF_DT_BEGIN_NODE || tag == OF_DT_NOP) { > if (tag == OF_DT_NOP) > *p += 4; > else > - mem = unflatten_dt_node(blob, mem, p, np, allnextpp, > + mem = unflatten_dt_node(blob, mem, p, np, size_scan, > fpsize); > tag = be32_to_cpup((__be32 *)(*p)); > } > @@ -347,16 +341,13 @@ unsigned long unflatten_dt_node(struct boot_param_header *blob, > * pointers of the nodes so the normal device-tree walking functions > * can be used. > * @blob: The blob to expand > - * @mynodes: The device_node tree created by the call > * @dt_alloc: An allocator that provides a virtual address to memory > * for the resulting tree > */ > void __unflatten_device_tree(struct boot_param_header *blob, > - struct device_node **mynodes, > void * (*dt_alloc)(u64 size, u64 align)) > { > unsigned long start, mem, size; > - struct device_node **allnextp = mynodes; > > pr_debug(" -> unflatten_device_tree()\n"); > > @@ -378,7 +369,7 @@ void __unflatten_device_tree(struct boot_param_header *blob, > /* First pass, scan for size */ > start = ((unsigned long)blob) + > be32_to_cpu(blob->off_dt_struct); > - size = unflatten_dt_node(blob, 0, &start, NULL, NULL, 0); > + size = unflatten_dt_node(blob, 0, &start, NULL, true, 0); > size = (size | 3) + 1; > > pr_debug(" size is %lx, allocating...\n", size); > @@ -394,13 +385,12 @@ void __unflatten_device_tree(struct boot_param_header *blob, > /* Second pass, do actual unflattening */ > start = ((unsigned long)blob) + > be32_to_cpu(blob->off_dt_struct); > - unflatten_dt_node(blob, mem, &start, NULL, &allnextp, 0); > + unflatten_dt_node(blob, mem, &start, NULL, false, 0); > if (be32_to_cpup((__be32 *)start) != OF_DT_END) > pr_warning("Weird tag at end of tree: %08x\n", *((u32 *)start)); > if (be32_to_cpu(((__be32 *)mem)[size / 4]) != 0xdeadbeef) > pr_warning("End of tree marker overwritten: %08x\n", > be32_to_cpu(((__be32 *)mem)[size / 4])); > - *allnextp = NULL; > > pr_debug(" <- unflatten_device_tree()\n"); > } > @@ -423,7 +413,7 @@ void of_fdt_unflatten_tree(unsigned long *blob, > { > struct boot_param_header *device_tree = > (struct boot_param_header *)blob; > - __unflatten_device_tree(device_tree, mynodes, &kernel_tree_alloc); > + __unflatten_device_tree(device_tree, &kernel_tree_alloc); > } > EXPORT_SYMBOL_GPL(of_fdt_unflatten_tree); > > @@ -702,7 +692,7 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname, > */ > void __init unflatten_device_tree(void) > { > - __unflatten_device_tree(initial_boot_params, &allnodes, > + __unflatten_device_tree(initial_boot_params, > early_init_dt_alloc_memory_arch); > > /* Get pointer to OF "/chosen" node for use everywhere */ > diff --git a/include/linux/of.h b/include/linux/of.h > index bb36473..95754ca 100644 > --- a/include/linux/of.h > +++ b/include/linux/of.h > @@ -50,7 +50,6 @@ struct device_node { > struct device_node *parent; > struct device_node *child; > struct device_node *sibling; > - struct device_node *next; /* next device of same type */ > struct device_node *allnext; /* next in list of all nodes */ > struct proc_dir_entry *pde; /* this node's proc directory */ > struct kref kref; > -- > 1.7.2.3 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/