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

[Laurent Pinchart]

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