Re: [Qemu-devel] [PATCH v2 2/9] devicetree: Added new APIs to make use of more fdt functions

[Prem Mallappa]

On Fri, Sep 9, 2016 at 9:32 PM, Auger Eric <address@hidden> 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 <address@hidden>
> > ---
> >  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