fdt: Fix handling of paths with options in them

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 <[email protected]>

---
  arch/arm/dts/sun7i-a20-pcduino3.dts |  2 +-
  lib/libfdt/fdt_ro.c                 | 25 ++++++++++++++++++++++---
  2 files changed, 23 insertions(+), 4 deletions(-)

arch/arm/dts/sun7i-a20-pcduino3.dts +0 -0

index cd05267..624abf2 100644

lib/libfdt/fdt_ro.c +22 -3

index 03733e5..44fc0aa 100644
@@ -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;
 

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 <[email protected]>

---
 arch/arm/dts/sun7i-a20-pcduino3.dts |  2 +-
 lib/libfdt/fdt_ro.c                 | 25 ++++++++++++++++++++++---

 2 files changed, 23 insertions(+), 4 deletions(-)

arch/arm/dts/sun7i-a20-pcduino3.dts +1 -1

index cd05267..624abf2 100644
@@ -64,7 +64,7 @@
        };
 
        chosen {
-               stdout-path = "serial0:115200n8";
+               stdout-path = "/soc at 01c00000/serial at 01c28000:115200";
        };
 
        leds {

lib/libfdt/fdt_ro.c +22 -4

index 03733e5..44fc0aa 100644
@@ -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

Hi Hans,

Quoted 9 lines · 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.

Quoted 6 lines

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

Signed-off-by Hans de Goede <[email protected]>

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

Quoted 68 lines

  2 files changed, 23 insertions(+), 4 deletions(-)

arch/arm/dts/sun7i-a20-pcduino3.dts +1 -1

index cd05267..624abf2 100644
@@ -64,7 +64,7 @@
         };
 
         chosen {
-               stdout-path = "serial0:115200n8";
+               stdout-path = "/soc at 01c00000/serial at 01c28000:115200";
         };
 
         leds {

lib/libfdt/fdt_ro.c +22 -4

index 03733e5..44fc0aa 100644
@@ -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;
 
--

Quoted 2 lines

2.3.5

Regards,
Simon

Hi,

Quoted 23 lines · Simon Glass

Hi Hans,

Quoted 17 lines · 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 <[email protected]>

---
  arch/arm/dts/sun7i-a20-pcduino3.dts |  2 +-
  lib/libfdt/fdt_ro.c                 | 25 ++++++++++++++++++++++---

Quoted 2 lines

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

(quoted diff) +3 -0

+       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

Quoted 81 lines

Quoted 75 lines

  2 files changed, 23 insertions(+), 4 deletions(-)

arch/arm/dts/sun7i-a20-pcduino3.dts +1 -1

index cd05267..624abf2 100644
@@ -64,7 +64,7 @@
         };
 
         chosen {
-               stdout-path = "serial0:115200n8";
+               stdout-path = "/soc at 01c00000/serial at 01c28000:115200";
         };
 
         leds {

lib/libfdt/fdt_ro.c +2 -0

index 03733e5..44fc0aa 100644
@@ -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.
+ */

lib/libfdt/fdt_ro.c +14 -1

+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;

lib/libfdt/fdt_ro.c +2 -3

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

Quoted 2 lines

2.3.5

Regards,
Simon

Hi Hans,

Quoted 30 lines · Hans de Goede

Hi,

Quoted 25 lines · Simon Glass

Hi Hans,

Quoted 11 lines · 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.

Quoted 6 lines

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

Signed-off-by Hans de Goede <[email protected]>

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

Quoted 3 lines

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.

Quoted 49 lines

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

(quoted diff) +3 -0

+       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;
}

Quoted 94 lines

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

Quoted 81 lines

Quoted 75 lines

   2 files changed, 23 insertions(+), 4 deletions(-)

arch/arm/dts/sun7i-a20-pcduino3.dts +1 -1

index cd05267..624abf2 100644
@@ -64,7 +64,7 @@
          };
 
          chosen {
-               stdout-path = "serial0:115200n8";
+               stdout-path = "/soc at 01c00000/serial at 01c28000:115200";
          };
 
          leds {

lib/libfdt/fdt_ro.c +2 -0

index 03733e5..44fc0aa 100644
@@ -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.
+ */

lib/libfdt/fdt_ro.c +14 -1

+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;

lib/libfdt/fdt_ro.c +2 -3

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

Quoted 2 lines

2.3.5

Regards,
Simon

Regards,
Simon

Hi,

Quoted 47 lines · Simon Glass

Hi Hans,

Quoted 37 lines · Hans de Goede

Hi,

Quoted 28 lines · Simon Glass

Hi Hans,

Quoted 14 lines · 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.

Quoted 6 lines

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

Signed-off-by Hans de Goede <[email protected]>

---
   arch/arm/dts/sun7i-a20-pcduino3.dts |  2 +-
   lib/libfdt/fdt_ro.c                 | 25 ++++++++++++++++++++++---

Quoted 2 lines

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

Quoted 4 lines

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:

Quoted 4 lines

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

Quoted 154 lines

Quoted 59 lines

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

(quoted diff) +3 -0

+       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.

Quoted 91 lines

Regards,

Hans

Quoted 83 lines

Quoted 77 lines

   2 files changed, 23 insertions(+), 4 deletions(-)

arch/arm/dts/sun7i-a20-pcduino3.dts +1 -1

index cd05267..624abf2 100644
@@ -64,7 +64,7 @@
          };
 
          chosen {
-               stdout-path = "serial0:115200n8";
+               stdout-path = "/soc at 01c00000/serial at 01c28000:115200";
          };
 
          leds {

lib/libfdt/fdt_ro.c +2 -0

index 03733e5..44fc0aa 100644
@@ -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.
+ */

lib/libfdt/fdt_ro.c +14 -1

+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;

lib/libfdt/fdt_ro.c +2 -3

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

Quoted 2 lines

2.3.5

Regards,
Simon

Regards,
Simon

Hi Hans,

Quoted 67 lines · Hans de Goede

Hi,

Quoted 47 lines · Simon Glass

Hi Hans,

Quoted 37 lines · Hans de Goede

Hi,

Quoted 28 lines · Simon Glass

Hi Hans,

Quoted 14 lines · 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.

Quoted 6 lines

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

Signed-off-by Hans de Goede <[email protected]>

---
   arch/arm/dts/sun7i-a20-pcduino3.dts |  2 +-
   lib/libfdt/fdt_ro.c                 | 25 ++++++++++++++++++++++---

Quoted 2 lines

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

Quoted 4 lines

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:

Quoted 4 lines

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.

Quoted 3 lines

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 <[email protected]>

Hi Hans,

Quoted 74 lines · Simon Glass

Hi Hans,

Quoted 70 lines · Hans de Goede

Hi,

Quoted 47 lines · Simon Glass

Hi Hans,

Quoted 37 lines · Hans de Goede

Hi,

Quoted 28 lines · Simon Glass

Hi Hans,

Quoted 14 lines · 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.

Quoted 6 lines

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

Signed-off-by Hans de Goede <[email protected]>

---
    arch/arm/dts/sun7i-a20-pcduino3.dts |  2 +-
    lib/libfdt/fdt_ro.c                 | 25 ++++++++++++++++++++++---

Quoted 2 lines

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

Quoted 4 lines

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:

Quoted 4 lines

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 ?

Quoted 5 lines

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 <[email protected]>

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.

Hi,

Quoted 99 lines · Simon Glass

Hi Hans,

Quoted 95 lines · Simon Glass

Hi Hans,

Quoted 82 lines · Hans de Goede

Hi,

Quoted 57 lines · Simon Glass

Hi Hans,

Quoted 44 lines · Hans de Goede

Hi,

Quoted 33 lines · Simon Glass

Hi Hans,

Quoted 16 lines · 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.

Quoted 7 lines

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

Signed-off-by Hans de Goede <[email protected]>

---
    arch/arm/dts/sun7i-a20-pcduino3.dts |  2 +-
    lib/libfdt/fdt_ro.c                 | 25

++++++++++++++++++++++---

Quoted 2 lines

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

Quoted 5 lines

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:

Quoted 4 lines

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.

Quoted 4 lines

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.

Quoted 5 lines

Quoted 2 lines

Acked-by Simon Glass <[email protected]>

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.

Quoted 106 lines · Hans de Goede

Hi,

Quoted 102 lines · Simon Glass

Hi Hans,

Quoted 97 lines · Simon Glass

Hi Hans,

Quoted 82 lines · Hans de Goede

Hi,

Quoted 57 lines · Simon Glass

Hi Hans,

Quoted 44 lines · Hans de Goede

Hi,

Quoted 33 lines · Simon Glass

Hi Hans,

Quoted 16 lines · 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.

Quoted 7 lines

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

Signed-off-by Hans de Goede <[email protected]>

---
    arch/arm/dts/sun7i-a20-pcduino3.dts |  2 +-
    lib/libfdt/fdt_ro.c                 | 25

++++++++++++++++++++++---

Quoted 2 lines

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

Quoted 5 lines

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:

Quoted 4 lines

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.

Quoted 4 lines

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 <[email protected]>

Quoted 5 lines

Quoted 2 lines

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!