[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 at 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 at 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
>