Re: [U-Boot] [PATCH 1/2] libfdt: overlay: Fix missing symbols condition

[Maxime Ripard]

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

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 1/2] libfdt: overlay: Fix missing symbols condition

[Stefan Agner]

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 
>>
>> 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 
>> ---
>>
>>  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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 1/2] libfdt: overlay: Fix missing symbols condition

[Maxime Ripard]

On Thu, Dec 15, 2016 at 03:03:26PM -0800, Stefan Agner wrote:
> From: Stefan Agner 
> 
> 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 
> ---
> 
>  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

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 1/2] libfdt: overlay: Fix missing symbols condition

[Stefan Agner]

From: Stefan Agner 

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

 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;
 
fdt_for_each_property_offset(property, fdto, fixups_off) {
-- 
2.10.2

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot