[U-Boot] [PATCH 0/1] fdt: Fix handling of paths with options in them

[U-Boot] [PATCH 0/1] fdt: Fix handling of paths with options in them

[Hans de Goede]

Hi all,

Here is a small but important fdt parsing fix. See the patch / commit
message for details.

I'm wondering how to best take this upstream. This is a dependency for the
sunxi dm work I'm doing so I will happily take this upstream (once reviewed)
through the sunxi tree.

Regards,

Hans

[U-Boot] [PATCH] fdt: Fix handling of paths with options in them

[Hans de Goede]

After syncing the sunxi dts files with the upstream kernel dm/fdt sunxi
builds would no longer boot.

The problem is that stdout-path is now set like this in the upstream dts
files: stdout-path = "serial0:115200n8". The use of options in of-paths,
either after an alias name, or after a full path, e.g. stdout-path =
"/soc at 01c00000/serial at 01c28000:115200", is standard of usage, but something
which the u-boot dts code so far did not handle.

This commit fixes this, adding support for both path formats.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 arch/arm/dts/sun7i-a20-pcduino3.dts |  2 +-
 lib/libfdt/fdt_ro.c                 | 25 ++++++++++++++++++++++---
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/arch/arm/dts/sun7i-a20-pcduino3.dts b/arch/arm/dts/sun7i-a20-pcduino3.dts
index cd05267..624abf2 100644
--- a/arch/arm/dts/sun7i-a20-pcduino3.dts
+++ b/arch/arm/dts/sun7i-a20-pcduino3.dts
@@ -64,7 +64,7 @@
 	};
 
 	chosen {
-		stdout-path = "serial0:115200n8";
+		stdout-path = "/soc at 01c00000/serial at 01c28000:115200";
 	};
 
 	leds {
diff --git a/lib/libfdt/fdt_ro.c b/lib/libfdt/fdt_ro.c
index 03733e5..44fc0aa 100644
--- a/lib/libfdt/fdt_ro.c
+++ b/lib/libfdt/fdt_ro.c
@@ -113,6 +113,25 @@ int fdt_subnode_offset(const void *fdt, int parentoffset,
 	return fdt_subnode_offset_namelen(fdt, parentoffset, name, strlen(name));
 }
 
+/*
+ * Find the next of path seperator, note we need to search for both '/' and ':'
+ * and then take the first one so that we do the rigth thing for e.g.
+ * "foo/bar:option" and "bar:option/otheroption", both of which happen, so
+ * first searching for either ':' or '/' does not work.
+ */
+static const char *fdt_path_next_seperator(const char *path)
+{
+	const char *sep1 = strchr(path, '/');
+	const char *sep2 = strchr(path, ':');
+
+	if (sep1 && sep2)
+		return (sep1 < sep2) ? sep1 : sep2;
+	else if (sep1)
+		return sep1;
+	else
+		return sep2;
+}
+
 int fdt_path_offset(const void *fdt, const char *path)
 {
 	const char *end = path + strlen(path);
@@ -123,7 +142,7 @@ int fdt_path_offset(const void *fdt, const char *path)
 
 	/* see if we have an alias */
 	if (*path != '/') {
-		const char *q = strchr(path, '/');
+		const char *q = fdt_path_next_seperator(path);
 
 		if (!q)
 			q = end;
@@ -141,9 +160,9 @@ int fdt_path_offset(const void *fdt, const char *path)
 
 		while (*p == '/')
 			p++;
-		if (! *p)
+		if (*p == '\0' || *p == ':')
 			return offset;
-		q = strchr(p, '/');
+		q = fdt_path_next_seperator(p);
 		if (! q)
 			q = end;
 
-- 
2.3.5

[U-Boot] [PATCH] fdt: Fix handling of paths with options in them

[Hans de Goede]

Hi,

On 20-04-15 11:13, Hans de Goede wrote:
> After syncing the sunxi dts files with the upstream kernel dm/fdt sunxi
> builds would no longer boot.
>
> The problem is that stdout-path is now set like this in the upstream dts
> files: stdout-path = "serial0:115200n8". The use of options in of-paths,
> either after an alias name, or after a full path, e.g. stdout-path =
> "/soc at 01c00000/serial at 01c28000:115200", is standard of usage, but something
> which the u-boot dts code so far did not handle.
>
> This commit fixes this, adding support for both path formats.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>   arch/arm/dts/sun7i-a20-pcduino3.dts |  2 +-
>   lib/libfdt/fdt_ro.c                 | 25 ++++++++++++++++++++++---
>   2 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/dts/sun7i-a20-pcduino3.dts b/arch/arm/dts/sun7i-a20-pcduino3.dts
> index cd05267..624abf2 100644
> --- a/arch/arm/dts/sun7i-a20-pcduino3.dts
> +++ b/arch/arm/dts/sun7i-a20-pcduino3.dts
> @@ -64,7 +64,7 @@
>   	};
>
>   	chosen {
> -		stdout-path = "serial0:115200n8";
> +		stdout-path = "/soc at 01c00000/serial at 01c28000:115200";
>   	};
>
>   	leds {

Ugh, this does not belong in here, this was just here to test the second code path
of fdt_path_offset handles paths with options correctly.

Will drop from my personal tree.

Regards,

Hans


> diff --git a/lib/libfdt/fdt_ro.c b/lib/libfdt/fdt_ro.c
> index 03733e5..44fc0aa 100644
> --- a/lib/libfdt/fdt_ro.c
> +++ b/lib/libfdt/fdt_ro.c
> @@ -113,6 +113,25 @@ int fdt_subnode_offset(const void *fdt, int parentoffset,
>   	return fdt_subnode_offset_namelen(fdt, parentoffset, name, strlen(name));
>   }
>
> +/*
> + * Find the next of path seperator, note we need to search for both '/' and ':'
> + * and then take the first one so that we do the rigth thing for e.g.
> + * "foo/bar:option" and "bar:option/otheroption", both of which happen, so
> + * first searching for either ':' or '/' does not work.
> + */
> +static const char *fdt_path_next_seperator(const char *path)
> +{
> +	const char *sep1 = strchr(path, '/');
> +	const char *sep2 = strchr(path, ':');
> +
> +	if (sep1 && sep2)
> +		return (sep1 < sep2) ? sep1 : sep2;
> +	else if (sep1)
> +		return sep1;
> +	else
> +		return sep2;
> +}
> +
>   int fdt_path_offset(const void *fdt, const char *path)
>   {
>   	const char *end = path + strlen(path);
> @@ -123,7 +142,7 @@ int fdt_path_offset(const void *fdt, const char *path)
>
>   	/* see if we have an alias */
>   	if (*path != '/') {
> -		const char *q = strchr(path, '/');
> +		const char *q = fdt_path_next_seperator(path);
>
>   		if (!q)
>   			q = end;
> @@ -141,9 +160,9 @@ int fdt_path_offset(const void *fdt, const char *path)
>
>   		while (*p == '/')
>   			p++;
> -		if (! *p)
> +		if (*p == '\0' || *p == ':')
>   			return offset;
> -		q = strchr(p, '/');
> +		q = fdt_path_next_seperator(p);
>   		if (! q)
>   			q = end;
>
>

[U-Boot] [PATCH] fdt: Fix handling of paths with options in them

[Simon Glass]

Hi Hans,

On 20 April 2015 at 03:13, Hans de Goede <hdegoede@redhat.com> wrote:
> After syncing the sunxi dts files with the upstream kernel dm/fdt sunxi
> builds would no longer boot.
>
> The problem is that stdout-path is now set like this in the upstream dts
> files: stdout-path = "serial0:115200n8". The use of options in of-paths,
> either after an alias name, or after a full path, e.g. stdout-path =
> "/soc at 01c00000/serial at 01c28000:115200", is standard of usage, but something
> which the u-boot dts code so far did not handle.
>
> This commit fixes this, adding support for both path formats.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  arch/arm/dts/sun7i-a20-pcduino3.dts |  2 +-
>  lib/libfdt/fdt_ro.c                 | 25 ++++++++++++++++++++++---

I haven't looked. but is this change in dtc upstream or just in the kernel?

>  2 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/dts/sun7i-a20-pcduino3.dts b/arch/arm/dts/sun7i-a20-pcduino3.dts
> index cd05267..624abf2 100644
> --- a/arch/arm/dts/sun7i-a20-pcduino3.dts
> +++ b/arch/arm/dts/sun7i-a20-pcduino3.dts
> @@ -64,7 +64,7 @@
>         };
>
>         chosen {
> -               stdout-path = "serial0:115200n8";
> +               stdout-path = "/soc at 01c00000/serial at 01c28000:115200";
>         };
>
>         leds {
> diff --git a/lib/libfdt/fdt_ro.c b/lib/libfdt/fdt_ro.c
> index 03733e5..44fc0aa 100644
> --- a/lib/libfdt/fdt_ro.c
> +++ b/lib/libfdt/fdt_ro.c
> @@ -113,6 +113,25 @@ int fdt_subnode_offset(const void *fdt, int parentoffset,
>         return fdt_subnode_offset_namelen(fdt, parentoffset, name, strlen(name));
>  }
>
> +/*
> + * Find the next of path seperator, note we need to search for both '/' and ':'
> + * and then take the first one so that we do the rigth thing for e.g.
> + * "foo/bar:option" and "bar:option/otheroption", both of which happen, so
> + * first searching for either ':' or '/' does not work.
> + */
> +static const char *fdt_path_next_seperator(const char *path)
> +{
> +       const char *sep1 = strchr(path, '/');
> +       const char *sep2 = strchr(path, ':');
> +
> +       if (sep1 && sep2)
> +               return (sep1 < sep2) ? sep1 : sep2;
> +       else if (sep1)
> +               return sep1;
> +       else
> +               return sep2;
> +}
> +
>  int fdt_path_offset(const void *fdt, const char *path)
>  {
>         const char *end = path + strlen(path);
> @@ -123,7 +142,7 @@ int fdt_path_offset(const void *fdt, const char *path)
>
>         /* see if we have an alias */
>         if (*path != '/') {
> -               const char *q = strchr(path, '/');
> +               const char *q = fdt_path_next_seperator(path);
>
>                 if (!q)
>                         q = end;
> @@ -141,9 +160,9 @@ int fdt_path_offset(const void *fdt, const char *path)
>
>                 while (*p == '/')
>                         p++;
> -               if (! *p)
> +               if (*p == '\0' || *p == ':')
>                         return offset;
> -               q = strchr(p, '/');
> +               q = fdt_path_next_seperator(p);
>                 if (! q)
>                         q = end;
>
> --
> 2.3.5
>

Regards,
Simon

[U-Boot] [PATCH] fdt: Fix handling of paths with options in them

[Hans de Goede]

Hi,

On 20-04-15 17:39, Simon Glass wrote:
> Hi Hans,
>
> On 20 April 2015 at 03:13, Hans de Goede <hdegoede@redhat.com> wrote:
>> After syncing the sunxi dts files with the upstream kernel dm/fdt sunxi
>> builds would no longer boot.
>>
>> The problem is that stdout-path is now set like this in the upstream dts
>> files: stdout-path = "serial0:115200n8". The use of options in of-paths,
>> either after an alias name, or after a full path, e.g. stdout-path =
>> "/soc at 01c00000/serial at 01c28000:115200", is standard of usage, but something
>> which the u-boot dts code so far did not handle.
>>
>> This commit fixes this, adding support for both path formats.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   arch/arm/dts/sun7i-a20-pcduino3.dts |  2 +-
>>   lib/libfdt/fdt_ro.c                 | 25 ++++++++++++++++++++++---
>
> I haven't looked. but is this change in dtc upstream or just in the kernel?

This is just a change in the dts files shipped with the kernel not in dtc,
the dts files for sunxi used to not set stdout-path, and you patched in
a stdout-path setting for u-boot:

http://git.denx.de/?p=u-boot.git;a=commitdiff;h=1a81cf8399675056beef5e76be8a9380d88c4ebf

+       chosen {
+               stdout-path = &uart0;
+       };

But now the upstream dts contains a stdout-path itself, but like this;

	alias {
		serial0 = &uart0;
	};

	chosen {
		stdout-path = "serial0:115200n8";
	};

And the current u-boot dts parsing does not grok this due to it not recognizing
the : in there.

Where as the kernel has:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/of/base.c#n713

static struct device_node *__of_find_node_by_path(struct device_node *parent,
						const char *path)
{
	struct device_node *child;
	int len;

	len = strcspn(path, "/:");
	if (!len)
		return NULL;

	__for_each_child_of_node(parent, child) {
		const char *name = strrchr(child->full_name, '/');
		if (WARN(!name, "malformed device_node %s\n", child->full_name))
			continue;
		name++;
		if (strncmp(path, name, len) == 0 && (strlen(name) == len))
			return child;
	}
	return NULL;
}

Where the strcspn surves the same purpose as the fdt_path_next_seperator my patch
introduces find the basename to match for stopping at the first occurence of
either a '/' or a ':' char.

Regards,

Hans



>
>>   2 files changed, 23 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/dts/sun7i-a20-pcduino3.dts b/arch/arm/dts/sun7i-a20-pcduino3.dts
>> index cd05267..624abf2 100644
>> --- a/arch/arm/dts/sun7i-a20-pcduino3.dts
>> +++ b/arch/arm/dts/sun7i-a20-pcduino3.dts
>> @@ -64,7 +64,7 @@
>>          };
>>
>>          chosen {
>> -               stdout-path = "serial0:115200n8";
>> +               stdout-path = "/soc at 01c00000/serial at 01c28000:115200";
>>          };
>>
>>          leds {
>> diff --git a/lib/libfdt/fdt_ro.c b/lib/libfdt/fdt_ro.c
>> index 03733e5..44fc0aa 100644
>> --- a/lib/libfdt/fdt_ro.c
>> +++ b/lib/libfdt/fdt_ro.c
>> @@ -113,6 +113,25 @@ int fdt_subnode_offset(const void *fdt, int parentoffset,
>>          return fdt_subnode_offset_namelen(fdt, parentoffset, name, strlen(name));
>>   }
>>
>> +/*
>> + * Find the next of path seperator, note we need to search for both '/' and ':'
>> + * and then take the first one so that we do the rigth thing for e.g.
>> + * "foo/bar:option" and "bar:option/otheroption", both of which happen, so
>> + * first searching for either ':' or '/' does not work.
>> + */
>> +static const char *fdt_path_next_seperator(const char *path)
>> +{
>> +       const char *sep1 = strchr(path, '/');
>> +       const char *sep2 = strchr(path, ':');
>> +
>> +       if (sep1 && sep2)
>> +               return (sep1 < sep2) ? sep1 : sep2;
>> +       else if (sep1)
>> +               return sep1;
>> +       else
>> +               return sep2;
>> +}
>> +
>>   int fdt_path_offset(const void *fdt, const char *path)
>>   {
>>          const char *end = path + strlen(path);
>> @@ -123,7 +142,7 @@ int fdt_path_offset(const void *fdt, const char *path)
>>
>>          /* see if we have an alias */
>>          if (*path != '/') {
>> -               const char *q = strchr(path, '/');
>> +               const char *q = fdt_path_next_seperator(path);
>>
>>                  if (!q)
>>                          q = end;
>> @@ -141,9 +160,9 @@ int fdt_path_offset(const void *fdt, const char *path)
>>
>>                  while (*p == '/')
>>                          p++;
>> -               if (! *p)
>> +               if (*p == '\0' || *p == ':')
>>                          return offset;
>> -               q = strchr(p, '/');
>> +               q = fdt_path_next_seperator(p);
>>                  if (! q)
>>                          q = end;
>>
>> --
>> 2.3.5
>>
>
> Regards,
> Simon
>

[U-Boot] [PATCH] fdt: Fix handling of paths with options in them

[Simon Glass]

Hi Hans,

On 20 April 2015 at 12:10, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 20-04-15 17:39, Simon Glass wrote:
>>
>> Hi Hans,
>>
>> On 20 April 2015 at 03:13, Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> After syncing the sunxi dts files with the upstream kernel dm/fdt sunxi
>>> builds would no longer boot.
>>>
>>> The problem is that stdout-path is now set like this in the upstream dts
>>> files: stdout-path = "serial0:115200n8". The use of options in of-paths,
>>> either after an alias name, or after a full path, e.g. stdout-path =
>>> "/soc at 01c00000/serial at 01c28000:115200", is standard of usage, but
>>> something
>>> which the u-boot dts code so far did not handle.
>>>
>>> This commit fixes this, adding support for both path formats.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>>   arch/arm/dts/sun7i-a20-pcduino3.dts |  2 +-
>>>   lib/libfdt/fdt_ro.c                 | 25 ++++++++++++++++++++++---
>>
>>
>> I haven't looked. but is this change in dtc upstream or just in the
>> kernel?
>
>
> This is just a change in the dts files shipped with the kernel not in dtc,
> the dts files for sunxi used to not set stdout-path, and you patched in
> a stdout-path setting for u-boot:

In that case, can we change this in the fdt support /fdtdec code,
instead of making a change to libfdt that will never go upstream?

If that doesn't work or is too painful, then we should take this patch.

>
> http://git.denx.de/?p=u-boot.git;a=commitdiff;h=1a81cf8399675056beef5e76be8a9380d88c4ebf
>
> +       chosen {
> +               stdout-path = &uart0;
> +       };
>
> But now the upstream dts contains a stdout-path itself, but like this;
>
>         alias {
>                 serial0 = &uart0;
>         };
>
>         chosen {
>                 stdout-path = "serial0:115200n8";
>         };
>
> And the current u-boot dts parsing does not grok this due to it not
> recognizing
> the : in there.
>
> Where as the kernel has:
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/of/base.c#n713
>
> static struct device_node *__of_find_node_by_path(struct device_node
> *parent,
>                                                 const char *path)
> {
>         struct device_node *child;
>         int len;
>
>         len = strcspn(path, "/:");
>         if (!len)
>                 return NULL;
>
>         __for_each_child_of_node(parent, child) {
>                 const char *name = strrchr(child->full_name, '/');
>                 if (WARN(!name, "malformed device_node %s\n",
> child->full_name))
>                         continue;
>                 name++;
>                 if (strncmp(path, name, len) == 0 && (strlen(name) == len))
>                         return child;
>         }
>         return NULL;
> }
>
> Where the strcspn surves the same purpose as the fdt_path_next_seperator my
> patch
> introduces find the basename to match for stopping at the first occurence of
> either a '/' or a ':' char.
>
> Regards,
>
> Hans
>
>
>
>
>>
>>>   2 files changed, 23 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/arm/dts/sun7i-a20-pcduino3.dts
>>> b/arch/arm/dts/sun7i-a20-pcduino3.dts
>>> index cd05267..624abf2 100644
>>> --- a/arch/arm/dts/sun7i-a20-pcduino3.dts
>>> +++ b/arch/arm/dts/sun7i-a20-pcduino3.dts
>>> @@ -64,7 +64,7 @@
>>>          };
>>>
>>>          chosen {
>>> -               stdout-path = "serial0:115200n8";
>>> +               stdout-path = "/soc at 01c00000/serial at 01c28000:115200";
>>>          };
>>>
>>>          leds {
>>> diff --git a/lib/libfdt/fdt_ro.c b/lib/libfdt/fdt_ro.c
>>> index 03733e5..44fc0aa 100644
>>> --- a/lib/libfdt/fdt_ro.c
>>> +++ b/lib/libfdt/fdt_ro.c
>>> @@ -113,6 +113,25 @@ int fdt_subnode_offset(const void *fdt, int
>>> parentoffset,
>>>          return fdt_subnode_offset_namelen(fdt, parentoffset, name,
>>> strlen(name));
>>>   }
>>>
>>> +/*
>>> + * Find the next of path seperator, note we need to search for both '/'
>>> and ':'
>>> + * and then take the first one so that we do the rigth thing for e.g.
>>> + * "foo/bar:option" and "bar:option/otheroption", both of which happen,
>>> so
>>> + * first searching for either ':' or '/' does not work.
>>> + */
>>> +static const char *fdt_path_next_seperator(const char *path)
>>> +{
>>> +       const char *sep1 = strchr(path, '/');
>>> +       const char *sep2 = strchr(path, ':');
>>> +
>>> +       if (sep1 && sep2)
>>> +               return (sep1 < sep2) ? sep1 : sep2;
>>> +       else if (sep1)
>>> +               return sep1;
>>> +       else
>>> +               return sep2;
>>> +}
>>> +
>>>   int fdt_path_offset(const void *fdt, const char *path)
>>>   {
>>>          const char *end = path + strlen(path);
>>> @@ -123,7 +142,7 @@ int fdt_path_offset(const void *fdt, const char
>>> *path)
>>>
>>>          /* see if we have an alias */
>>>          if (*path != '/') {
>>> -               const char *q = strchr(path, '/');
>>> +               const char *q = fdt_path_next_seperator(path);
>>>
>>>                  if (!q)
>>>                          q = end;
>>> @@ -141,9 +160,9 @@ int fdt_path_offset(const void *fdt, const char
>>> *path)
>>>
>>>                  while (*p == '/')
>>>                          p++;
>>> -               if (! *p)
>>> +               if (*p == '\0' || *p == ':')
>>>                          return offset;
>>> -               q = strchr(p, '/');
>>> +               q = fdt_path_next_seperator(p);
>>>                  if (! q)
>>>                          q = end;
>>>
>>> --
>>> 2.3.5
>>>
>>
>> Regards,
>> Simon
>>
>

Regards,
Simon

[U-Boot] [PATCH] fdt: Fix handling of paths with options in them

[Hans de Goede]

Hi,

On 22-04-15 19:20, Simon Glass wrote:
> Hi Hans,
>
> On 20 April 2015 at 12:10, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 20-04-15 17:39, Simon Glass wrote:
>>>
>>> Hi Hans,
>>>
>>> On 20 April 2015 at 03:13, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>
>>>> After syncing the sunxi dts files with the upstream kernel dm/fdt sunxi
>>>> builds would no longer boot.
>>>>
>>>> The problem is that stdout-path is now set like this in the upstream dts
>>>> files: stdout-path = "serial0:115200n8". The use of options in of-paths,
>>>> either after an alias name, or after a full path, e.g. stdout-path =
>>>> "/soc at 01c00000/serial at 01c28000:115200", is standard of usage, but
>>>> something
>>>> which the u-boot dts code so far did not handle.
>>>>
>>>> This commit fixes this, adding support for both path formats.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>> ---
>>>>    arch/arm/dts/sun7i-a20-pcduino3.dts |  2 +-
>>>>    lib/libfdt/fdt_ro.c                 | 25 ++++++++++++++++++++++---
>>>
>>>
>>> I haven't looked. but is this change in dtc upstream or just in the
>>> kernel?
>>
>>
>> This is just a change in the dts files shipped with the kernel not in dtc,
>> the dts files for sunxi used to not set stdout-path, and you patched in
>> a stdout-path setting for u-boot:
>
> In that case, can we change this in the fdt support /fdtdec code,
> instead of making a change to libfdt that will never go upstream?
>
> If that doesn't work or is too painful, then we should take this patch.

Actually I started with fixing this the fdtdev level, but then I noticed
that the kernel does this at the of_find_node_by_path level, so if we
fix this for stdout-path only (which a fdtdec patch would do) then we may
get bit by this again later.

Also fixing it at the fdtdec level means adding a strdup + error checking
since then we need to pass a truncated (options removed) copy of the path
to fdt_path_offset(), while with the current patch we can keep using
read only access to the fdt.

So I've a slight preference for going this way. Why would libfdt upstream not
take this patch ?

Regards,

Hans

>
>>
>> http://git.denx.de/?p=u-boot.git;a=commitdiff;h=1a81cf8399675056beef5e76be8a9380d88c4ebf
>>
>> +       chosen {
>> +               stdout-path = &uart0;
>> +       };
>>
>> But now the upstream dts contains a stdout-path itself, but like this;
>>
>>          alias {
>>                  serial0 = &uart0;
>>          };
>>
>>          chosen {
>>                  stdout-path = "serial0:115200n8";
>>          };
>>
>> And the current u-boot dts parsing does not grok this due to it not
>> recognizing
>> the : in there.
>>
>> Where as the kernel has:
>>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/of/base.c#n713
>>
>> static struct device_node *__of_find_node_by_path(struct device_node
>> *parent,
>>                                                  const char *path)
>> {
>>          struct device_node *child;
>>          int len;
>>
>>          len = strcspn(path, "/:");
>>          if (!len)
>>                  return NULL;
>>
>>          __for_each_child_of_node(parent, child) {
>>                  const char *name = strrchr(child->full_name, '/');
>>                  if (WARN(!name, "malformed device_node %s\n",
>> child->full_name))
>>                          continue;
>>                  name++;
>>                  if (strncmp(path, name, len) == 0 && (strlen(name) == len))
>>                          return child;
>>          }
>>          return NULL;
>> }
>>
>> Where the strcspn surves the same purpose as the fdt_path_next_seperator my
>> patch
>> introduces find the basename to match for stopping at the first occurence of
>> either a '/' or a ':' char.
>>
>> Regards,
>>
>> Hans
>>
>>
>>
>>
>>>
>>>>    2 files changed, 23 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/arch/arm/dts/sun7i-a20-pcduino3.dts
>>>> b/arch/arm/dts/sun7i-a20-pcduino3.dts
>>>> index cd05267..624abf2 100644
>>>> --- a/arch/arm/dts/sun7i-a20-pcduino3.dts
>>>> +++ b/arch/arm/dts/sun7i-a20-pcduino3.dts
>>>> @@ -64,7 +64,7 @@
>>>>           };
>>>>
>>>>           chosen {
>>>> -               stdout-path = "serial0:115200n8";
>>>> +               stdout-path = "/soc at 01c00000/serial at 01c28000:115200";
>>>>           };
>>>>
>>>>           leds {
>>>> diff --git a/lib/libfdt/fdt_ro.c b/lib/libfdt/fdt_ro.c
>>>> index 03733e5..44fc0aa 100644
>>>> --- a/lib/libfdt/fdt_ro.c
>>>> +++ b/lib/libfdt/fdt_ro.c
>>>> @@ -113,6 +113,25 @@ int fdt_subnode_offset(const void *fdt, int
>>>> parentoffset,
>>>>           return fdt_subnode_offset_namelen(fdt, parentoffset, name,
>>>> strlen(name));
>>>>    }
>>>>
>>>> +/*
>>>> + * Find the next of path seperator, note we need to search for both '/'
>>>> and ':'
>>>> + * and then take the first one so that we do the rigth thing for e.g.
>>>> + * "foo/bar:option" and "bar:option/otheroption", both of which happen,
>>>> so
>>>> + * first searching for either ':' or '/' does not work.
>>>> + */
>>>> +static const char *fdt_path_next_seperator(const char *path)
>>>> +{
>>>> +       const char *sep1 = strchr(path, '/');
>>>> +       const char *sep2 = strchr(path, ':');
>>>> +
>>>> +       if (sep1 && sep2)
>>>> +               return (sep1 < sep2) ? sep1 : sep2;
>>>> +       else if (sep1)
>>>> +               return sep1;
>>>> +       else
>>>> +               return sep2;
>>>> +}
>>>> +
>>>>    int fdt_path_offset(const void *fdt, const char *path)
>>>>    {
>>>>           const char *end = path + strlen(path);
>>>> @@ -123,7 +142,7 @@ int fdt_path_offset(const void *fdt, const char
>>>> *path)
>>>>
>>>>           /* see if we have an alias */
>>>>           if (*path != '/') {
>>>> -               const char *q = strchr(path, '/');
>>>> +               const char *q = fdt_path_next_seperator(path);
>>>>
>>>>                   if (!q)
>>>>                           q = end;
>>>> @@ -141,9 +160,9 @@ int fdt_path_offset(const void *fdt, const char
>>>> *path)
>>>>
>>>>                   while (*p == '/')
>>>>                           p++;
>>>> -               if (! *p)
>>>> +               if (*p == '\0' || *p == ':')
>>>>                           return offset;
>>>> -               q = strchr(p, '/');
>>>> +               q = fdt_path_next_seperator(p);
>>>>                   if (! q)
>>>>                           q = end;
>>>>
>>>> --
>>>> 2.3.5
>>>>
>>>
>>> Regards,
>>> Simon
>>>
>>
>
> Regards,
> Simon
>

[U-Boot] [PATCH] fdt: Fix handling of paths with options in them

[Simon Glass]

Hi Hans,

On 23 April 2015 at 00:55, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 22-04-15 19:20, Simon Glass wrote:
>>
>> Hi Hans,
>>
>> On 20 April 2015 at 12:10, Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> Hi,
>>>
>>> On 20-04-15 17:39, Simon Glass wrote:
>>>>
>>>>
>>>> Hi Hans,
>>>>
>>>> On 20 April 2015 at 03:13, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>
>>>>>
>>>>> After syncing the sunxi dts files with the upstream kernel dm/fdt sunxi
>>>>> builds would no longer boot.
>>>>>
>>>>> The problem is that stdout-path is now set like this in the upstream
>>>>> dts
>>>>> files: stdout-path = "serial0:115200n8". The use of options in
>>>>> of-paths,
>>>>> either after an alias name, or after a full path, e.g. stdout-path =
>>>>> "/soc at 01c00000/serial at 01c28000:115200", is standard of usage, but
>>>>> something
>>>>> which the u-boot dts code so far did not handle.
>>>>>
>>>>> This commit fixes this, adding support for both path formats.
>>>>>
>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>> ---
>>>>>    arch/arm/dts/sun7i-a20-pcduino3.dts |  2 +-
>>>>>    lib/libfdt/fdt_ro.c                 | 25 ++++++++++++++++++++++---
>>>>
>>>>
>>>>
>>>> I haven't looked. but is this change in dtc upstream or just in the
>>>> kernel?
>>>
>>>
>>>
>>> This is just a change in the dts files shipped with the kernel not in
>>> dtc,
>>> the dts files for sunxi used to not set stdout-path, and you patched in
>>> a stdout-path setting for u-boot:
>>
>>
>> In that case, can we change this in the fdt support /fdtdec code,
>> instead of making a change to libfdt that will never go upstream?
>>
>> If that doesn't work or is too painful, then we should take this patch.
>
>
> Actually I started with fixing this the fdtdev level, but then I noticed
> that the kernel does this at the of_find_node_by_path level, so if we
> fix this for stdout-path only (which a fdtdec patch would do) then we may
> get bit by this again later.
>
> Also fixing it at the fdtdec level means adding a strdup + error checking
> since then we need to pass a truncated (options removed) copy of the path
> to fdt_path_offset(), while with the current patch we can keep using
> read only access to the fdt.
>
> So I've a slight preference for going this way. Why would libfdt upstream
> not
> take this patch ?

I'm not sure, but please go ahead and send it upstream. Thanks for the
explanation, I'll pick this up.

Acked-by: Simon Glass <sjg@chromium.org>

>
> Regards,
>
> Hans
>
>
>>
>>>
>>>
>>> http://git.denx.de/?p=u-boot.git;a=commitdiff;h=1a81cf8399675056beef5e76be8a9380d88c4ebf
>>>
>>> +       chosen {
>>> +               stdout-path = &uart0;
>>> +       };
>>>
>>> But now the upstream dts contains a stdout-path itself, but like this;
>>>
>>>          alias {
>>>                  serial0 = &uart0;
>>>          };
>>>
>>>          chosen {
>>>                  stdout-path = "serial0:115200n8";
>>>          };
>>>
>>> And the current u-boot dts parsing does not grok this due to it not
>>> recognizing
>>> the : in there.
>>>
>>> Where as the kernel has:
>>>
>>>
>>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/of/base.c#n713
>>>
>>> static struct device_node *__of_find_node_by_path(struct device_node
>>> *parent,
>>>                                                  const char *path)
>>> {
>>>          struct device_node *child;
>>>          int len;
>>>
>>>          len = strcspn(path, "/:");
>>>          if (!len)
>>>                  return NULL;
>>>
>>>          __for_each_child_of_node(parent, child) {
>>>                  const char *name = strrchr(child->full_name, '/');
>>>                  if (WARN(!name, "malformed device_node %s\n",
>>> child->full_name))
>>>                          continue;
>>>                  name++;
>>>                  if (strncmp(path, name, len) == 0 && (strlen(name) ==
>>> len))
>>>                          return child;
>>>          }
>>>          return NULL;
>>> }
>>>
>>> Where the strcspn surves the same purpose as the fdt_path_next_seperator
>>> my
>>> patch
>>> introduces find the basename to match for stopping at the first occurence
>>> of
>>> either a '/' or a ':' char.
>>>
>>> Regards,
>>>
>>> Hans
>>>
>>>
>>>
>>>
>>>>
>>>>>    2 files changed, 23 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/dts/sun7i-a20-pcduino3.dts
>>>>> b/arch/arm/dts/sun7i-a20-pcduino3.dts
>>>>> index cd05267..624abf2 100644
>>>>> --- a/arch/arm/dts/sun7i-a20-pcduino3.dts
>>>>> +++ b/arch/arm/dts/sun7i-a20-pcduino3.dts
>>>>> @@ -64,7 +64,7 @@
>>>>>           };
>>>>>
>>>>>           chosen {
>>>>> -               stdout-path = "serial0:115200n8";
>>>>> +               stdout-path = "/soc at 01c00000/serial at 01c28000:115200";
>>>>>           };
>>>>>
>>>>>           leds {
>>>>> diff --git a/lib/libfdt/fdt_ro.c b/lib/libfdt/fdt_ro.c
>>>>> index 03733e5..44fc0aa 100644
>>>>> --- a/lib/libfdt/fdt_ro.c
>>>>> +++ b/lib/libfdt/fdt_ro.c
>>>>> @@ -113,6 +113,25 @@ int fdt_subnode_offset(const void *fdt, int
>>>>> parentoffset,
>>>>>           return fdt_subnode_offset_namelen(fdt, parentoffset, name,
>>>>> strlen(name));
>>>>>    }
>>>>>
>>>>> +/*
>>>>> + * Find the next of path seperator, note we need to search for both
>>>>> '/'
>>>>> and ':'
>>>>> + * and then take the first one so that we do the rigth thing for e.g.
>>>>> + * "foo/bar:option" and "bar:option/otheroption", both of which
>>>>> happen,
>>>>> so
>>>>> + * first searching for either ':' or '/' does not work.
>>>>> + */
>>>>> +static const char *fdt_path_next_seperator(const char *path)
>>>>> +{
>>>>> +       const char *sep1 = strchr(path, '/');
>>>>> +       const char *sep2 = strchr(path, ':');
>>>>> +
>>>>> +       if (sep1 && sep2)
>>>>> +               return (sep1 < sep2) ? sep1 : sep2;
>>>>> +       else if (sep1)
>>>>> +               return sep1;
>>>>> +       else
>>>>> +               return sep2;
>>>>> +}
>>>>> +
>>>>>    int fdt_path_offset(const void *fdt, const char *path)
>>>>>    {
>>>>>           const char *end = path + strlen(path);
>>>>> @@ -123,7 +142,7 @@ int fdt_path_offset(const void *fdt, const char
>>>>> *path)
>>>>>
>>>>>           /* see if we have an alias */
>>>>>           if (*path != '/') {
>>>>> -               const char *q = strchr(path, '/');
>>>>> +               const char *q = fdt_path_next_seperator(path);
>>>>>
>>>>>                   if (!q)
>>>>>                           q = end;
>>>>> @@ -141,9 +160,9 @@ int fdt_path_offset(const void *fdt, const char
>>>>> *path)
>>>>>
>>>>>                   while (*p == '/')
>>>>>                           p++;
>>>>> -               if (! *p)
>>>>> +               if (*p == '\0' || *p == ':')
>>>>>                           return offset;
>>>>> -               q = strchr(p, '/');
>>>>> +               q = fdt_path_next_seperator(p);
>>>>>                   if (! q)
>>>>>                           q = end;
>>>>>
>>>>> --
>>>>> 2.3.5
>>>>>
>>>>
>>>> Regards,
>>>> Simon
>>>>
>>>
>>
>> Regards,
>> Simon
>>
>

[U-Boot] [PATCH] fdt: Fix handling of paths with options in them

[Simon Glass]

Hi Hans,

On 23 April 2015 at 10:15, Simon Glass <sjg@chromium.org> wrote:
> Hi Hans,
>
> On 23 April 2015 at 00:55, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>>
>> On 22-04-15 19:20, Simon Glass wrote:
>>>
>>> Hi Hans,
>>>
>>> On 20 April 2015 at 12:10, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 20-04-15 17:39, Simon Glass wrote:
>>>>>
>>>>>
>>>>> Hi Hans,
>>>>>
>>>>> On 20 April 2015 at 03:13, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>
>>>>>>
>>>>>> After syncing the sunxi dts files with the upstream kernel dm/fdt sunxi
>>>>>> builds would no longer boot.
>>>>>>
>>>>>> The problem is that stdout-path is now set like this in the upstream
>>>>>> dts
>>>>>> files: stdout-path = "serial0:115200n8". The use of options in
>>>>>> of-paths,
>>>>>> either after an alias name, or after a full path, e.g. stdout-path =
>>>>>> "/soc at 01c00000/serial at 01c28000:115200", is standard of usage, but
>>>>>> something
>>>>>> which the u-boot dts code so far did not handle.
>>>>>>
>>>>>> This commit fixes this, adding support for both path formats.
>>>>>>
>>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>>> ---
>>>>>>    arch/arm/dts/sun7i-a20-pcduino3.dts |  2 +-
>>>>>>    lib/libfdt/fdt_ro.c                 | 25 ++++++++++++++++++++++---
>>>>>
>>>>>
>>>>>
>>>>> I haven't looked. but is this change in dtc upstream or just in the
>>>>> kernel?
>>>>
>>>>
>>>>
>>>> This is just a change in the dts files shipped with the kernel not in
>>>> dtc,
>>>> the dts files for sunxi used to not set stdout-path, and you patched in
>>>> a stdout-path setting for u-boot:
>>>
>>>
>>> In that case, can we change this in the fdt support /fdtdec code,
>>> instead of making a change to libfdt that will never go upstream?
>>>
>>> If that doesn't work or is too painful, then we should take this patch.
>>
>>
>> Actually I started with fixing this the fdtdev level, but then I noticed
>> that the kernel does this at the of_find_node_by_path level, so if we
>> fix this for stdout-path only (which a fdtdec patch would do) then we may
>> get bit by this again later.
>>
>> Also fixing it at the fdtdec level means adding a strdup + error checking
>> since then we need to pass a truncated (options removed) copy of the path
>> to fdt_path_offset(), while with the current patch we can keep using
>> read only access to the fdt.
>>
>> So I've a slight preference for going this way. Why would libfdt upstream
>> not
>> take this patch ?
>
> I'm not sure, but please go ahead and send it upstream. Thanks for the
> explanation, I'll pick this up.
>
> Acked-by: Simon Glass <sjg@chromium.org>
>

Can I just check if you are OK to send this upstream? We don't need to
wait for it to be accepted, just want to know it will be sent.

Regards,
Simon

[U-Boot] [PATCH] fdt: Fix handling of paths with options in them

[Hans de Goede]

Hi,

On 24-04-15 14:42, Simon Glass wrote:
> Hi Hans,
>
> On 23 April 2015 at 10:15, Simon Glass <sjg@chromium.org> wrote:
>> Hi Hans,
>>
>> On 23 April 2015 at 00:55, Hans de Goede <hdegoede@redhat.com> wrote:
>>> Hi,
>>>
>>>
>>> On 22-04-15 19:20, Simon Glass wrote:
>>>>
>>>> Hi Hans,
>>>>
>>>> On 20 April 2015 at 12:10, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> On 20-04-15 17:39, Simon Glass wrote:
>>>>>>
>>>>>>
>>>>>> Hi Hans,
>>>>>>
>>>>>> On 20 April 2015 at 03:13, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>> After syncing the sunxi dts files with the upstream kernel dm/fdt sunxi
>>>>>>> builds would no longer boot.
>>>>>>>
>>>>>>> The problem is that stdout-path is now set like this in the upstream
>>>>>>> dts
>>>>>>> files: stdout-path = "serial0:115200n8". The use of options in
>>>>>>> of-paths,
>>>>>>> either after an alias name, or after a full path, e.g. stdout-path =
>>>>>>> "/soc at 01c00000/serial at 01c28000:115200", is standard of usage, but
>>>>>>> something
>>>>>>> which the u-boot dts code so far did not handle.
>>>>>>>
>>>>>>> This commit fixes this, adding support for both path formats.
>>>>>>>
>>>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>>>> ---
>>>>>>>     arch/arm/dts/sun7i-a20-pcduino3.dts |  2 +-
>>>>>>>     lib/libfdt/fdt_ro.c                 | 25 ++++++++++++++++++++++---
>>>>>>
>>>>>>
>>>>>>
>>>>>> I haven't looked. but is this change in dtc upstream or just in the
>>>>>> kernel?
>>>>>
>>>>>
>>>>>
>>>>> This is just a change in the dts files shipped with the kernel not in
>>>>> dtc,
>>>>> the dts files for sunxi used to not set stdout-path, and you patched in
>>>>> a stdout-path setting for u-boot:
>>>>
>>>>
>>>> In that case, can we change this in the fdt support /fdtdec code,
>>>> instead of making a change to libfdt that will never go upstream?
>>>>
>>>> If that doesn't work or is too painful, then we should take this patch.
>>>
>>>
>>> Actually I started with fixing this the fdtdev level, but then I noticed
>>> that the kernel does this at the of_find_node_by_path level, so if we
>>> fix this for stdout-path only (which a fdtdec patch would do) then we may
>>> get bit by this again later.
>>>
>>> Also fixing it at the fdtdec level means adding a strdup + error checking
>>> since then we need to pass a truncated (options removed) copy of the path
>>> to fdt_path_offset(), while with the current patch we can keep using
>>> read only access to the fdt.
>>>
>>> So I've a slight preference for going this way. Why would libfdt upstream
>>> not
>>> take this patch ?
>>
>> I'm not sure, but please go ahead and send it upstream. Thanks for the
>> explanation, I'll pick this up.
>>
>> Acked-by: Simon Glass <sjg@chromium.org>
>>
>
> Can I just check if you are OK to send this upstream? We don't need to
> wait for it to be accepted, just want to know it will be sent.

Yes I will submit this upstream.

Regards,

Hans

[U-Boot] [PATCH] fdt: Fix handling of paths with options in them

[Simon Glass]

On 24 April 2015 at 07:34, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 24-04-15 14:42, Simon Glass wrote:
>>
>> Hi Hans,
>>
>> On 23 April 2015 at 10:15, Simon Glass <sjg@chromium.org> wrote:
>>>
>>> Hi Hans,
>>>
>>> On 23 April 2015 at 00:55, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>>
>>>> On 22-04-15 19:20, Simon Glass wrote:
>>>>>
>>>>>
>>>>> Hi Hans,
>>>>>
>>>>> On 20 April 2015 at 12:10, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On 20-04-15 17:39, Simon Glass wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Hi Hans,
>>>>>>>
>>>>>>> On 20 April 2015 at 03:13, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> After syncing the sunxi dts files with the upstream kernel dm/fdt
>>>>>>>> sunxi
>>>>>>>> builds would no longer boot.
>>>>>>>>
>>>>>>>> The problem is that stdout-path is now set like this in the upstream
>>>>>>>> dts
>>>>>>>> files: stdout-path = "serial0:115200n8". The use of options in
>>>>>>>> of-paths,
>>>>>>>> either after an alias name, or after a full path, e.g. stdout-path =
>>>>>>>> "/soc at 01c00000/serial at 01c28000:115200", is standard of usage, but
>>>>>>>> something
>>>>>>>> which the u-boot dts code so far did not handle.
>>>>>>>>
>>>>>>>> This commit fixes this, adding support for both path formats.
>>>>>>>>
>>>>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>>>>> ---
>>>>>>>>     arch/arm/dts/sun7i-a20-pcduino3.dts |  2 +-
>>>>>>>>     lib/libfdt/fdt_ro.c                 | 25
>>>>>>>> ++++++++++++++++++++++---
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> I haven't looked. but is this change in dtc upstream or just in the
>>>>>>> kernel?
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> This is just a change in the dts files shipped with the kernel not in
>>>>>> dtc,
>>>>>> the dts files for sunxi used to not set stdout-path, and you patched
>>>>>> in
>>>>>> a stdout-path setting for u-boot:
>>>>>
>>>>>
>>>>>
>>>>> In that case, can we change this in the fdt support /fdtdec code,
>>>>> instead of making a change to libfdt that will never go upstream?
>>>>>
>>>>> If that doesn't work or is too painful, then we should take this patch.
>>>>
>>>>
>>>>
>>>> Actually I started with fixing this the fdtdev level, but then I noticed
>>>> that the kernel does this at the of_find_node_by_path level, so if we
>>>> fix this for stdout-path only (which a fdtdec patch would do) then we
>>>> may
>>>> get bit by this again later.
>>>>
>>>> Also fixing it at the fdtdec level means adding a strdup + error
>>>> checking
>>>> since then we need to pass a truncated (options removed) copy of the
>>>> path
>>>> to fdt_path_offset(), while with the current patch we can keep using
>>>> read only access to the fdt.
>>>>
>>>> So I've a slight preference for going this way. Why would libfdt
>>>> upstream
>>>> not
>>>> take this patch ?
>>>
>>>
>>> I'm not sure, but please go ahead and send it upstream. Thanks for the
>>> explanation, I'll pick this up.
>>>
>>> Acked-by: Simon Glass <sjg@chromium.org>
>>>
>>
>> Can I just check if you are OK to send this upstream? We don't need to
>> wait for it to be accepted, just want to know it will be sent.
>
>
> Yes I will submit this upstream.

Applied to u-boot-fdt, thanks!

- Simon

[U-Boot] [PATCH] fdt: Fix handling of paths with options in them

[Simon Glass]

+Scott

Hi Hans,

On 28 April 2015 at 17:29, Simon Glass <sjg@chromium.org> wrote:
>
> On 24 April 2015 at 07:34, Hans de Goede <hdegoede@redhat.com> wrote:
> > Hi,
> >
> >
> > On 24-04-15 14:42, Simon Glass wrote:
> >>
> >> Hi Hans,
> >>
> >> On 23 April 2015 at 10:15, Simon Glass <sjg@chromium.org> wrote:
> >>>
> >>> Hi Hans,
> >>>
> >>> On 23 April 2015 at 00:55, Hans de Goede <hdegoede@redhat.com> wrote:
> >>>>
> >>>> Hi,
> >>>>
> >>>>
> >>>> On 22-04-15 19:20, Simon Glass wrote:
> >>>>>
> >>>>>
> >>>>> Hi Hans,
> >>>>>
> >>>>> On 20 April 2015 at 12:10, Hans de Goede <hdegoede@redhat.com> wrote:
> >>>>>>
> >>>>>>
> >>>>>> Hi,
> >>>>>>
> >>>>>> On 20-04-15 17:39, Simon Glass wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> Hi Hans,
> >>>>>>>
> >>>>>>> On 20 April 2015 at 03:13, Hans de Goede <hdegoede@redhat.com> wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> After syncing the sunxi dts files with the upstream kernel dm/fdt
> >>>>>>>> sunxi
> >>>>>>>> builds would no longer boot.
> >>>>>>>>
> >>>>>>>> The problem is that stdout-path is now set like this in the upstream
> >>>>>>>> dts
> >>>>>>>> files: stdout-path = "serial0:115200n8". The use of options in
> >>>>>>>> of-paths,
> >>>>>>>> either after an alias name, or after a full path, e.g. stdout-path =
> >>>>>>>> "/soc at 01c00000/serial at 01c28000:115200", is standard of usage, but
> >>>>>>>> something
> >>>>>>>> which the u-boot dts code so far did not handle.
> >>>>>>>>
> >>>>>>>> This commit fixes this, adding support for both path formats.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >>>>>>>> ---
> >>>>>>>>     arch/arm/dts/sun7i-a20-pcduino3.dts |  2 +-
> >>>>>>>>     lib/libfdt/fdt_ro.c                 | 25
> >>>>>>>> ++++++++++++++++++++++---
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> I haven't looked. but is this change in dtc upstream or just in the
> >>>>>>> kernel?
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> This is just a change in the dts files shipped with the kernel not in
> >>>>>> dtc,
> >>>>>> the dts files for sunxi used to not set stdout-path, and you patched
> >>>>>> in
> >>>>>> a stdout-path setting for u-boot:
> >>>>>
> >>>>>
> >>>>>
> >>>>> In that case, can we change this in the fdt support /fdtdec code,
> >>>>> instead of making a change to libfdt that will never go upstream?
> >>>>>
> >>>>> If that doesn't work or is too painful, then we should take this patch.
> >>>>
> >>>>
> >>>>
> >>>> Actually I started with fixing this the fdtdev level, but then I noticed
> >>>> that the kernel does this at the of_find_node_by_path level, so if we
> >>>> fix this for stdout-path only (which a fdtdec patch would do) then we
> >>>> may
> >>>> get bit by this again later.
> >>>>
> >>>> Also fixing it at the fdtdec level means adding a strdup + error
> >>>> checking
> >>>> since then we need to pass a truncated (options removed) copy of the
> >>>> path
> >>>> to fdt_path_offset(), while with the current patch we can keep using
> >>>> read only access to the fdt.
> >>>>
> >>>> So I've a slight preference for going this way. Why would libfdt
> >>>> upstream
> >>>> not
> >>>> take this patch ?
> >>>
> >>>
> >>> I'm not sure, but please go ahead and send it upstream. Thanks for the
> >>> explanation, I'll pick this up.
> >>>
> >>> Acked-by: Simon Glass <sjg@chromium.org>
> >>>
> >>
> >> Can I just check if you are OK to send this upstream? We don't need to
> >> wait for it to be accepted, just want to know it will be sent.
> >
> >
> > Yes I will submit this upstream.
>
> Applied to u-boot-fdt, thanks!

It looks like this was rejected upstream. If so can you please create
a patch for fdtdec.c (I assume) to move your code into there?

Regards,
Simon