* [U-Boot] [PATCH 0/1] fdt: Fix handling of paths with options in them @ 2015-04-20 9:13 Hans de Goede 2015-04-20 9:13 ` [U-Boot] [PATCH] " Hans de Goede 0 siblings, 1 reply; 12+ messages in thread From: Hans de Goede @ 2015-04-20 9:13 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] fdt: Fix handling of paths with options in them 2015-04-20 9:13 [U-Boot] [PATCH 0/1] fdt: Fix handling of paths with options in them Hans de Goede @ 2015-04-20 9:13 ` Hans de Goede 2015-04-20 14:58 ` Hans de Goede 2015-04-20 15:39 ` Simon Glass 0 siblings, 2 replies; 12+ messages in thread From: Hans de Goede @ 2015-04-20 9:13 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] fdt: Fix handling of paths with options in them 2015-04-20 9:13 ` [U-Boot] [PATCH] " Hans de Goede @ 2015-04-20 14:58 ` Hans de Goede 2015-04-20 15:39 ` Simon Glass 1 sibling, 0 replies; 12+ messages in thread From: Hans de Goede @ 2015-04-20 14:58 UTC (permalink / raw) To: u-boot 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; > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] fdt: Fix handling of paths with options in them 2015-04-20 9:13 ` [U-Boot] [PATCH] " Hans de Goede 2015-04-20 14:58 ` Hans de Goede @ 2015-04-20 15:39 ` Simon Glass 2015-04-20 18:10 ` Hans de Goede 1 sibling, 1 reply; 12+ messages in thread From: Simon Glass @ 2015-04-20 15:39 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] fdt: Fix handling of paths with options in them 2015-04-20 15:39 ` Simon Glass @ 2015-04-20 18:10 ` Hans de Goede 2015-04-22 17:20 ` Simon Glass 0 siblings, 1 reply; 12+ messages in thread From: Hans de Goede @ 2015-04-20 18:10 UTC (permalink / raw) To: u-boot 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 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] fdt: Fix handling of paths with options in them 2015-04-20 18:10 ` Hans de Goede @ 2015-04-22 17:20 ` Simon Glass 2015-04-23 6:55 ` Hans de Goede 0 siblings, 1 reply; 12+ messages in thread From: Simon Glass @ 2015-04-22 17:20 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] fdt: Fix handling of paths with options in them 2015-04-22 17:20 ` Simon Glass @ 2015-04-23 6:55 ` Hans de Goede 2015-04-23 16:15 ` Simon Glass 0 siblings, 1 reply; 12+ messages in thread From: Hans de Goede @ 2015-04-23 6:55 UTC (permalink / raw) To: u-boot 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 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] fdt: Fix handling of paths with options in them 2015-04-23 6:55 ` Hans de Goede @ 2015-04-23 16:15 ` Simon Glass 2015-04-24 12:42 ` Simon Glass 0 siblings, 1 reply; 12+ messages in thread From: Simon Glass @ 2015-04-23 16:15 UTC (permalink / raw) To: u-boot 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 >> > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] fdt: Fix handling of paths with options in them 2015-04-23 16:15 ` Simon Glass @ 2015-04-24 12:42 ` Simon Glass 2015-04-24 13:34 ` Hans de Goede 0 siblings, 1 reply; 12+ messages in thread From: Simon Glass @ 2015-04-24 12:42 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] fdt: Fix handling of paths with options in them 2015-04-24 12:42 ` Simon Glass @ 2015-04-24 13:34 ` Hans de Goede 2015-04-28 23:29 ` Simon Glass 0 siblings, 1 reply; 12+ messages in thread From: Hans de Goede @ 2015-04-24 13:34 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] fdt: Fix handling of paths with options in them 2015-04-24 13:34 ` Hans de Goede @ 2015-04-28 23:29 ` Simon Glass 2015-07-14 19:49 ` Simon Glass 0 siblings, 1 reply; 12+ messages in thread From: Simon Glass @ 2015-04-28 23:29 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] fdt: Fix handling of paths with options in them 2015-04-28 23:29 ` Simon Glass @ 2015-07-14 19:49 ` Simon Glass 0 siblings, 0 replies; 12+ messages in thread From: Simon Glass @ 2015-07-14 19:49 UTC (permalink / raw) To: u-boot +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 ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-07-14 19:49 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-04-20 9:13 [U-Boot] [PATCH 0/1] fdt: Fix handling of paths with options in them Hans de Goede 2015-04-20 9:13 ` [U-Boot] [PATCH] " Hans de Goede 2015-04-20 14:58 ` Hans de Goede 2015-04-20 15:39 ` Simon Glass 2015-04-20 18:10 ` Hans de Goede 2015-04-22 17:20 ` Simon Glass 2015-04-23 6:55 ` Hans de Goede 2015-04-23 16:15 ` Simon Glass 2015-04-24 12:42 ` Simon Glass 2015-04-24 13:34 ` Hans de Goede 2015-04-28 23:29 ` Simon Glass 2015-07-14 19:49 ` Simon Glass
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.