From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43081) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bjXkd-0007pO-8I for qemu-devel@nongnu.org; Mon, 12 Sep 2016 16:22:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bjXkY-00035w-9z for qemu-devel@nongnu.org; Mon, 12 Sep 2016 16:22:47 -0400 Received: from mail-wm0-f54.google.com ([74.125.82.54]:38407) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bjXkW-00035G-Ss for qemu-devel@nongnu.org; Mon, 12 Sep 2016 16:22:42 -0400 Received: by mail-wm0-f54.google.com with SMTP id 1so165360134wmz.1 for ; Mon, 12 Sep 2016 13:22:39 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <20160822161740.4252-1-prem.mallappa@broadcom.com> <20160822161740.4252-3-prem.mallappa@broadcom.com> From: Prem Mallappa Date: Tue, 13 Sep 2016 01:51:38 +0530 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH v2 2/9] devicetree: Added new APIs to make use of more fdt functions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Auger Eric Cc: Peter Maydell , "Edgar E . Iglesias" , QEMU Developers On Fri, Sep 9, 2016 at 9:32 PM, Auger Eric wrote: > Hi Prem, > > > SMMUv3 needs device tree entry like below > To me the commit message should be more explicit and mention appendprop > functionality > > > > interrupt-names = "gerror", "priq", "eventq", "cmdq-sync"; > > > > This patch introduces helper function to add entries like above > > > > Signed-off-by: Prem Mallappa > > --- > > device_tree.c | 35 +++++++++++++++++++++++++++++++++++ > > include/sysemu/device_tree.h | 18 ++++++++++++++++++ > > 2 files changed, 53 insertions(+) > > > > diff --git a/device_tree.c b/device_tree.c > > index 6e06320..5d5966e 100644 > > --- a/device_tree.c > > +++ b/device_tree.c > > @@ -297,6 +297,24 @@ int qemu_fdt_setprop(void *fdt, const char > *node_path, > > return r; > > } > > > > +int qemu_fdt_appendprop(void *fdt, const char *node_path, > > + const char *property, const void *val, int size) > > +{ > > + int r; > > + > > + r = fdt_appendprop(fdt, findnode_nofail(fdt, node_path), property, > > + val, size); > > + if (r < 0) { > > + error_report("%s: Couldn't set %s/%s: %s", __func__, node_path, > > + property, fdt_strerror(r)); > > + exit(1); > > + } > > + > > + return r; > > +} > > + > > + > spare void lines > > + > > int qemu_fdt_setprop_cell(void *fdt, const char *node_path, > > const char *property, uint32_t val) > > { > > @@ -319,6 +337,23 @@ int qemu_fdt_setprop_u64(void *fdt, const char > *node_path, > > return qemu_fdt_setprop(fdt, node_path, property, &val, > sizeof(val)); > > } > > > > +int qemu_fdt_appendprop_string(void *fdt, const char *node_path, > > + const char *property, const char *string) > > +{ > > + int r; > > + > > + r = fdt_appendprop_string(fdt, findnode_nofail(fdt, node_path), > > + property, string); > > + if (r < 0) { > > + error_report("%s: Couldn't set %s/%s = %s: %s", __func__, > > + node_path, property, string, fdt_strerror(r)); > > + exit(1); > > + } > > + > > + return r; > > +} > > + > same > > + > > int qemu_fdt_setprop_string(void *fdt, const char *node_path, > > const char *property, const char *string) > > { > > diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h > > index 705650a..5a0a297 100644 > > --- a/include/sysemu/device_tree.h > > +++ b/include/sysemu/device_tree.h > > @@ -45,12 +45,16 @@ char **qemu_fdt_node_path(void *fdt, const char > *name, char *compat, > > > > int qemu_fdt_setprop(void *fdt, const char *node_path, > > const char *property, const void *val, int size); > > +int qemu_fdt_appendprop(void *fdt, const char *node_path, > > + const char *property, const void *val, int size); > > int qemu_fdt_setprop_cell(void *fdt, const char *node_path, > > const char *property, uint32_t val); > > int qemu_fdt_setprop_u64(void *fdt, const char *node_path, > > const char *property, uint64_t val); > > int qemu_fdt_setprop_string(void *fdt, const char *node_path, > > const char *property, const char *string); > > +int qemu_fdt_appendprop_string(void *fdt, const char *node_path, > > + const char *property, const char > *string); > > int qemu_fdt_setprop_phandle(void *fdt, const char *node_path, > > const char *property, > > const char *target_node_path); > > @@ -98,6 +102,20 @@ int qemu_fdt_add_subnode(void *fdt, const char > *name); > > sizeof(qdt_tmp)); > \ > > } while (0) > > > > + > same > > +#define qemu_fdt_appendprop_cells(fdt, node_path, property, ...) > \ > > + do { > \ > > + uint32_t qdt_tmp[] = { __VA_ARGS__ }; > \ > > + int i; > \ > > + > \ > > + for (i = 0; i < ARRAY_SIZE(qdt_tmp); i++) { > \ > > + qdt_tmp[i] = cpu_to_be32(qdt_tmp[i]); > \ > > + } > \ > > + qemu_fdt_appendprop(fdt, node_path, property, qdt_tmp, > \ > > + sizeof(qdt_tmp)); > \ > > + } while (0) > > + > > + > same here. > > Will take care of the extra blank lines. > While I understand the benefits I think we could manage without this new > API by using qemu_fdt_setprop and populating the uint32_t array separately. > > I'll give this a try and see how it goes. > I understood the QEMU API for manipulating flattened trees was not > supposed to grow too much but I don't have a strong option here. You > should CC David Gibson I think. > > Will sure do in the next drop. -- Cheers, /Prem