All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] cmd/fdt: support single value replacement within an array
Date: Sat, 05 Aug 2017 00:23:19 +0300	[thread overview]
Message-ID: <2232013.ItvgJkA94s@avalon> (raw)

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@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

             reply	other threads:[~2017-08-04 21:23 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-04 21:23 Laurent Pinchart [this message]
2017-08-16  7:05 ` [U-Boot] cmd/fdt: support single value replacement within an array Hannes Schmelzer
2017-08-18  8:07 ` Hannes Schmelzer
2017-08-18  8:14   ` Laurent Pinchart
2017-08-18 12:49     ` [U-Boot] Antwort: " Hannes Schmelzer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2232013.ItvgJkA94s@avalon \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.