Re: [Xen-devel] [PATCH V1 08/29] xen/dts: Don't add a fake property "name" in the device tree

[Julien Grall <julien.grall@xxxxxxxxxx>]

On 09/06/2013 05:28 PM, Ian Campbell wrote:
> On Wed, 2013-08-28 at 15:47 +0100, Julien Grall wrote:
> > On new Flat Device Tree version, the property "name" may not exist.
> > The property is never used in Xen code except to set the field "name" of
> > dt_device_node.
> >
> > For convenience, remove the fake property. It will save space during the
> > creation of the dom0 FDT.
> Is it worth diverging from the Linux code this is based on over this
> though?
If we don't diverge, I need to add some logic in device tree creation to 
know if the "name" property was generated by Xen.
The others solutions was:
   1) Add the logic during device tree creation
   2) Add a boolean to know if the property is a fake

Currently, the "name" property value is replicated in the name field of 
dt_device_node. Except staying close to Linux code, I don't see a good 
reason to create this fake property.
> > Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> > ---
> >   xen/common/device_tree.c |   21 +++++++++------------
> >   1 file changed, 9 insertions(+), 12 deletions(-)
> >
> > diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> > index be592d2..07a19ac 100644
> > --- a/xen/common/device_tree.c
> > +++ b/xen/common/device_tree.c
> > @@ -1543,6 +1543,7 @@ static unsigned long __init unflatten_dt_node(const void 
> > *fdt,
> >       if ( !has_name )
> >       {
> >           char *p1 = pathp, *ps = pathp, *pa = NULL;
> > +        char *tmp = NULL;
> >           int sz;
> >
> >           while ( *p1 )
> > @@ -1556,25 +1557,21 @@ static unsigned long __init unflatten_dt_node(const 
> > void *fdt,
> >           if ( pa < ps )
> >               pa = p1;
> >           sz = (pa - ps) + 1;
> > -        pp = unflatten_dt_alloc(&mem, sizeof(struct dt_property) + sz,
> > -                                __alignof__(struct dt_property));
> > +
> > +        tmp = unflatten_dt_alloc(&mem, sz, 1);
> >           if ( allnextpp )
> >           {
> > -            pp->name = "name";
> > -            pp->length = sz;
> > -            pp->value = pp + 1;
> > -            *prev_pp = pp;
> > -            prev_pp = &pp->next;
> > -            memcpy(pp->value, ps, sz - 1);
> > -            ((char *)pp->value)[sz - 1] = 0;
> > -            dt_dprintk("fixed up name for %s -> %s\n", pathp,
> > -                       (char *)pp->value);
> > +            memcpy(tmp, ps, sz - 1);
> > +            np->name = tmp;
> > +            tmp[sz - 1] = 0;
> > +            dt_dprintk("fixed up name for %s -> %s\n", pathp, np->name);
> >           }
> >       }
> > +
> >       if ( allnextpp )
> >       {
> >           *prev_pp = NULL;
> > -        np->name = dt_get_property(np, "name", NULL);
> > +        np->name = (np->name) ? : dt_get_property(np, "name", NULL);
> >           np->type = dt_get_property(np, "device_type", NULL);
> >
> >           if ( !np->name )

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel