From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59569) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TBaYz-0002K9-IC for qemu-devel@nongnu.org; Tue, 11 Sep 2012 20:12:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TBaYx-00071v-VP for qemu-devel@nongnu.org; Tue, 11 Sep 2012 20:12:17 -0400 Received: from mail-ob0-f173.google.com ([209.85.214.173]:64084) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TBaYx-00071p-OP for qemu-devel@nongnu.org; Tue, 11 Sep 2012 20:12:15 -0400 Received: by obbta14 with SMTP id ta14so1591174obb.4 for ; Tue, 11 Sep 2012 17:12:15 -0700 (PDT) From: Peter Crosthwaite In-Reply-To: <1347236407-10465-6-git-send-email-crwulff@gmail.com> References: <1347236407-10465-1-git-send-email-crwulff@gmail.com> <1347236407-10465-6-git-send-email-crwulff@gmail.com> Content-Type: text/plain; charset="UTF-8" Date: Wed, 12 Sep 2012 10:12:03 +1000 Message-ID: <1347408723.30441.33.camel@PetaLogix-ws2> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 5/9] FDT: Add additional access methods for array types and walking children. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: crwulff@gmail.com Cc: peter.maydell@linaro.org, peter.crosthwaite@xilinx.com, qemu-devel@nongnu.org On Sun, 2012-09-09 at 20:20 -0400, crwulff@gmail.com wrote: > From: Chris Wulff > > Signed-off-by: Chris Wulff > --- > device_tree.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > device_tree.h | 18 ++++++++++++ > 2 files changed, 106 insertions(+) > > diff --git a/device_tree.c b/device_tree.c > index d7a9b6b..7fa7646 100644 > --- a/device_tree.c > +++ b/device_tree.c > @@ -27,6 +27,14 @@ > > #include > > +#define CHECK_HEADER(fdt) \ > + { \ > + int err = fdt_check_header(fdt); \ > + if (err != 0) { \ > + return err; \ > + } \ > + } > + Im thinking the best way (and I have recently done some patches along this line) is to use the qemu error reporting mechanism. Rather than return 1, populate an Error ** with an error code. Not a blocker as this is out of scope of this series, but would make this macro foo go away with a nice inline function. > #define FDT_MAX_SIZE 0x10000 > > void *create_device_tree(int *sizep) > @@ -304,3 +312,83 @@ int qemu_devtree_add_subnode(void *fdt, const char *name) > g_free(dupname); > return retval; > } > + > +int qemu_devtree_node_offset(void *fdt, const char *node_path) > +{ > + return fdt_path_offset(fdt, node_path); > +} device_tree so far has been able to insulate clients from dealing with offsets. Good to keep it that way and use node_paths instead. Offsets have the problem where they are globally invalidated as soon as the device tree is manipulated. I have an alternate API extension that does a lot of what you are trying to do but nodepaths are used as handles to nodes rather than offsets. Will send patch. > + > +int qemu_devtree_subnode_offset_namelen(void *fdt, int parentoffset, > + const char *name, int namelen) > +{ > + return fdt_subnode_offset_namelen(fdt, parentoffset, name, namelen); > +} No need for no-operation wrappers, just call into lib-fdt directly. Most device-tree api calls exit(1) on failure which is why we have these thin wrappers. > + > +int qemu_devtree_next_child_offset(void *fdt, int parentoffset, int childoffset) > +{ > + int level = 0; > + uint32_t tag; > + int offset, nextoffset; > + > + CHECK_HEADER(fdt); > + tag = fdt_next_tag(fdt, parentoffset, &nextoffset); > + if (tag != FDT_BEGIN_NODE) { > + return -FDT_ERR_BADOFFSET; > + } > + > + do { > + offset = nextoffset; > + tag = fdt_next_tag(fdt, offset, &nextoffset); > + > + switch (tag) { > + case FDT_END: > + return -FDT_ERR_TRUNCATED; > + > + case FDT_BEGIN_NODE: > + level++; > + if (level != 1) { > + continue; > + } > + if (offset > childoffset) { > + return offset; > + } > + break; > + > + case FDT_END_NODE: > + level--; > + break; > + > + case FDT_PROP: > + case FDT_NOP: > + break; > + > + default: > + return -FDT_ERR_BADSTRUCTURE; > + } > + } while (level >= 0); > + > + return -FDT_ERR_NOTFOUND; > +} > + > +const char *qemu_devtree_get_name(const void *fdt, int nodeoffset, int *lenp) > +{ > + return fdt_get_name(fdt, nodeoffset, lenp); > +} > + > +const void *qemu_devtree_getprop_offset(const void *fdt, int nodeoffset, > + const char *name, int *lenp) > +{ > + return fdt_getprop(fdt, nodeoffset, name, lenp); > +} > + > +uint32_t qemu_devtree_int_array_index(const void *propval, unsigned int index) > +{ > + return be32_to_cpu(((uint32_t *)propval)[index]); > +} > + > +int qemu_devtree_node_check_compatible(const void *fdt, int nodeoffset, > + const char *compatible) > +{ > + return fdt_node_check_compatible(fdt, nodeoffset, compatible); > +} > + > diff --git a/device_tree.h b/device_tree.h > index f7a3e6c..e01b460 100644 > --- a/device_tree.h > +++ b/device_tree.h > @@ -49,4 +49,22 @@ int qemu_devtree_add_subnode(void *fdt, const char *name); > sizeof(qdt_tmp)); \ > } while (0) > > +int qemu_devtree_node_offset(void *fdt, const char *node_path); > + > +int qemu_devtree_subnode_offset_namelen(void *fdt, int parentoffset, > + const char *name, int namelen); > + > +int qemu_devtree_next_child_offset(void *fdt, int parentoffset, > + int childoffset); > + > +const char *qemu_devtree_get_name(const void *fdt, int nodeoffset, int *lenp); > + > +const void *qemu_devtree_getprop_offset(const void *fdt, int nodeoffset, > + const char *name, int *lenp); > + > +uint32_t qemu_devtree_int_array_index(const void *propval, unsigned int index); This is get_prop_cell but with an index. We debated this on the list recently (cc PMM) that get_prop_cell should(nt) have an index field into it that lets you get the non-zeroth property. I have a patch in my tree that changes this that ill send you along with the rest. > + > +int qemu_devtree_node_check_compatible(const void *fdt, int nodeoffset, > + const char *compatible); > + > #endif /* __DEVICE_TREE_H__ */ Regards, Peter