Hi,

On 22-04-15 19:20, Simon Glass wrote:
Hi Hans,

On 20 April 2015 at 12:10, Hans de Goede <hdego...@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 <hdego...@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@01c00000/serial@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 <hdego...@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@01c00000/serial@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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to