Message ID | 20161215230327.28896-1-stefan@agner.ch |
---|---|
State | Rejected |
Delegated to: | Tom Rini |
Headers | show |
On Thu, Dec 15, 2016 at 03:03:26PM -0800, Stefan Agner wrote: > From: Stefan Agner <stefan.agner@toradex.com> > > When there is no symbols section in the device tree, > overlay_fixup_phandles should return FDT_ERR_NOTFOUND instead of > FDT_ERR_BADOFFSET. > > Signed-off-by: Stefan Agner <stefan.agner@toradex.com> > --- > > lib/libfdt/fdt_overlay.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/lib/libfdt/fdt_overlay.c b/lib/libfdt/fdt_overlay.c > index bb41404..4a9ba40 100644 > --- a/lib/libfdt/fdt_overlay.c > +++ b/lib/libfdt/fdt_overlay.c > @@ -495,9 +495,9 @@ static int overlay_fixup_phandles(void *fdt, void *fdto) > if ((fixups_off < 0 && (fixups_off != -FDT_ERR_NOTFOUND))) > return fixups_off; > > - /* And base DTs without symbols */ > + /* But if we need to fixup phandles, symbols are required */ > symbols_off = fdt_path_offset(fdt, "/__symbols__"); > - if ((symbols_off < 0 && (symbols_off != -FDT_ERR_NOTFOUND))) > + if (symbols_off < 0) > return symbols_off; A base device tree doesn't need to have a symbols node. In the (uncommon, true, but still real) case where you wouldn't have any label in the base DT, __symbols__ wouldn't be there. Now, that code would return an error only if there's such a reference expressed in __fixups__, but no __symbols__ node that also has that reference. This is definitely an error but not really a NOTFOUND. You had an offset already, but this offset was bad. Hence the error. Do you have a base DT and an overlay showing up the error you're trying to fix? Either way, that should be discussed with upstream's libfdt ml and maintainer, not (only) on U-Boot. Maxime
On 2016-12-16 01:32, Maxime Ripard wrote: > On Thu, Dec 15, 2016 at 03:03:26PM -0800, Stefan Agner wrote: >> From: Stefan Agner <stefan.agner@toradex.com> >> >> When there is no symbols section in the device tree, >> overlay_fixup_phandles should return FDT_ERR_NOTFOUND instead of >> FDT_ERR_BADOFFSET. >> >> Signed-off-by: Stefan Agner <stefan.agner@toradex.com> >> --- >> >> lib/libfdt/fdt_overlay.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/lib/libfdt/fdt_overlay.c b/lib/libfdt/fdt_overlay.c >> index bb41404..4a9ba40 100644 >> --- a/lib/libfdt/fdt_overlay.c >> +++ b/lib/libfdt/fdt_overlay.c >> @@ -495,9 +495,9 @@ static int overlay_fixup_phandles(void *fdt, void *fdto) >> if ((fixups_off < 0 && (fixups_off != -FDT_ERR_NOTFOUND))) >> return fixups_off; >> >> - /* And base DTs without symbols */ >> + /* But if we need to fixup phandles, symbols are required */ >> symbols_off = fdt_path_offset(fdt, "/__symbols__"); >> - if ((symbols_off < 0 && (symbols_off != -FDT_ERR_NOTFOUND))) >> + if (symbols_off < 0) >> return symbols_off; > > A base device tree doesn't need to have a symbols node. In the > (uncommon, true, but still real) case where you wouldn't have any > label in the base DT, __symbols__ wouldn't be there. Yeah I know, but in case there are fixup offsets found (just above), symbols get essentially mandatory. > > Now, that code would return an error only if there's such a reference > expressed in __fixups__, but no __symbols__ node that also has that > reference. This is definitely an error but not really a NOTFOUND. You > had an offset already, but this offset was bad. Hence the error. It probably would a new error code? > > Do you have a base DT and an overlay showing up the error you're > trying to fix? Either way, that should be discussed with upstream's > libfdt ml and maintainer, not (only) on U-Boot. Yeah my problem was that my base DT had no symbols and used labels. Wasn't aware of the whole symbols problematic... I actually just realized that this got fixed upstream: https://git.kernel.org/cgit/utils/dtc/dtc.git/commit/libfdt/fdt_overlay.c?id=7d8ef6e1db9794f72805a0855f4f7f12fadd03d3 I guess we could backport that fix? -- Stefan
On Sat, Dec 17, 2016 at 04:56:49PM -0800, Stefan Agner wrote: > > Do you have a base DT and an overlay showing up the error you're > > trying to fix? Either way, that should be discussed with upstream's > > libfdt ml and maintainer, not (only) on U-Boot. > > Yeah my problem was that my base DT had no symbols and used labels. > Wasn't aware of the whole symbols problematic... > > I actually just realized that this got fixed upstream: > https://git.kernel.org/cgit/utils/dtc/dtc.git/commit/libfdt/fdt_overlay.c?id=7d8ef6e1db9794f72805a0855f4f7f12fadd03d3 > > I guess we could backport that fix? Yep. Thanks! Maxime
diff --git a/lib/libfdt/fdt_overlay.c b/lib/libfdt/fdt_overlay.c index bb41404..4a9ba40 100644 --- a/lib/libfdt/fdt_overlay.c +++ b/lib/libfdt/fdt_overlay.c @@ -495,9 +495,9 @@ static int overlay_fixup_phandles(void *fdt, void *fdto) if ((fixups_off < 0 && (fixups_off != -FDT_ERR_NOTFOUND))) return fixups_off; - /* And base DTs without symbols */ + /* But if we need to fixup phandles, symbols are required */ symbols_off = fdt_path_offset(fdt, "/__symbols__"); - if ((symbols_off < 0 && (symbols_off != -FDT_ERR_NOTFOUND))) + if (symbols_off < 0) return symbols_off; fdt_for_each_property_offset(property, fdto, fixups_off) {