[U-Boot] cmd/fdt: support single value replacement within an array

[Hannes Schmelzer]

Hi Laurent,

thanks for reporting this.
I just came back from holiday. I will check / fix this as soon as 
possible.

cheers,
Hannes


Laurent Pinchart <laurent.pinchart at ideasonboard.com> schrieb am 04.08.2017 
23:23:19:

> Von: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> An: u-boot at lists.denx.de
> Kopie: Hannes Schmelzer <hannes.schmelzer at br-automation.com>, Simon 
Glass 
> <sjg at chromium.org>
> Datum: 04.08.2017 23:23
> Betreff: Re: cmd/fdt: support single value replacement within an array
> 
> Hi Hannes,
> 
> (I'm not subscribed to the list, please keep me CC'ed on replies)
> 
> On Tuesday 30 May 2017 13:05:00 Hannes Schmelzer wrote:
> > With this commit we can modify single values within an array of a dts
> > property.
> 
> But with this commit U-Boot crashes if you try to create a new property 
with 
> the fdt set command :-/
> 
> I've tested v2017.07 with the commit reverted, and fdt set works again 
for me. 
> The issue is that your fdt_getprop() call fails and return NULL with len 
set 
> to -1. You can easily imagine what the memcpy() following it will do.
> 
> > This is useful if we have for example a pwm-backlight where we want to
> > modifiy the pwm frequency per u-boot script.
> > 
> > The pwm is described in dts like this:
> > 
> > backlight {
> >    pwms = <0x0000002b 0x00000000 0x004c4b40>;
> > };
> > 
> > For changing the frequency, here the 3rd parameter, we simply type:
> > 
> > fdt set /backlight pwms <? ? 0x1E8480>;
> > 
> > For doing all this we:
> > - backup the property content into our 'SCRATCHPAD'
> > - only modify the array-cell if the new content doesn't start with '?'
> > 
> > Signed-off-by: Hannes Schmelzer <hannes.schmelzer at br-automation.com>
> > 
> > ---
> > 
> >  cmd/fdt.c | 29 +++++++++++++++++++++--------
> >  1 file changed, 21 insertions(+), 8 deletions(-)
> > 
> > diff --git a/cmd/fdt.c b/cmd/fdt.c
> > index a21415d..e55102a 100644
> > --- a/cmd/fdt.c
> > +++ b/cmd/fdt.c
> > @@ -257,6 +257,7 @@  static int do_fdt(cmd_tbl_t *cmdtp, int flag, int 
argc,
> > char * const argv[])> 
> >        char *prop;      /* property */
> >        int  nodeoffset;   /* node offset from libfdt */
> >        static char data[SCRATCHPAD];   /* storage for the property */
> > +      const void *ptmp;
> >        int  len;      /* new length of the property */
> >        int  ret;      /* return value */
> > 
> > @@ -268,13 +269,6 @@  static int do_fdt(cmd_tbl_t *cmdtp, int flag, 
int
> > argc, char * const argv[])> 
> >        pathp  = argv[2];
> >        prop   = argv[3];
> > 
> > -      if (argc == 4) {
> > -         len = 0;
> > -      } else {
> > -         ret = fdt_parse_prop(&argv[4], argc - 4, data, &len);
> > -         if (ret != 0)
> > -            return ret;
> > -      }
> > 
> >        nodeoffset = fdt_path_offset (working_fdt, pathp);
> >        if (nodeoffset < 0) {
> > @@ -286,6 +280,21 @@  static int do_fdt(cmd_tbl_t *cmdtp, int flag, 
int
> > argc, char * const argv[])
> >           return 1;
> >        }
> > 
> > +      if (argc == 4) {
> > +         len = 0;
> > +      } else {
> > +         ptmp = fdt_getprop(working_fdt, nodeoffset, prop,
> > &len);
> > +         if (len > SCRATCHPAD) {
> > +            printf("prop (%d) doesn't fit in scratchpad!
> > \n",
> > +                   len);
> > +            return 1;
> > +         }
> > +         memcpy(data, ptmp, len);
> > +         ret = fdt_parse_prop(&argv[4], argc - 4, data, &len);
> > +         if (ret != 0)
> > +            return ret;
> > +      }
> > +
> >        ret = fdt_setprop(working_fdt, nodeoffset, prop, data, len);
> >        if (ret < 0) {
> >           printf ("libfdt fdt_setprop(): %s\n",
> > fdt_strerror(ret));
> > @@ -766,7 +775,11 @@  static int fdt_parse_prop(char * const *newval, 
int
> > count, char *data, int *len)
> >           cp = newp;
> >           tmp = simple_strtoul(cp, &newp, 0);
> > 
> > -         *(fdt32_t *)data = cpu_to_fdt32(tmp);
> > +         if (*cp != '?')
> > +            *(fdt32_t *)data = cpu_to_fdt32(tmp);
> > +         else
> > +            newp++;
> > +
> > 
> >           data  += 4;
> >           *len += 4;
> -- 
> Regards,
> 
> Laurent Pinchart
>