[PATCH 0/4] libfdt: A few overlay fixes

[PATCH 0/4] libfdt: A few overlay fixes

[Maxime Ripard]

Hi David,

Here are a few patches to fix the things you requested and a few bugs
encountered while doing so.

Let me know what you think,
Maxime

Maxime Ripard (4):
  libfdt: Add BADPHANDLE error string
  libfdt: overlay: Report a bad overlay for mismatching local fixups
  libfdt: overlay: Fix symbols and fixups nodes condition
  tests: overlay: Move back the bad fixup tests

 libfdt/fdt_overlay.c  |  9 +++++----
 libfdt/fdt_strerror.c |  1 +
 tests/run_tests.sh    | 14 +++++++-------
 3 files changed, 13 insertions(+), 11 deletions(-)

-- 
2.9.3

[PATCH 1/4] libfdt: Add BADPHANDLE error string

[Maxime Ripard]

The BADPHANDLE error was missing a string, leading to an <unknown error>
string being returned if you were to call fdt_strerror.

Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 libfdt/fdt_strerror.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libfdt/fdt_strerror.c b/libfdt/fdt_strerror.c
index 3cb357f60454..e00fcb19df60 100644
--- a/libfdt/fdt_strerror.c
+++ b/libfdt/fdt_strerror.c
@@ -69,6 +69,7 @@ static struct fdt_errtabent fdt_errtable[] = {
 
 	FDT_ERRTABENT(FDT_ERR_BADOFFSET),
 	FDT_ERRTABENT(FDT_ERR_BADPATH),
+	FDT_ERRTABENT(FDT_ERR_BADPHANDLE),
 	FDT_ERRTABENT(FDT_ERR_BADSTATE),
 
 	FDT_ERRTABENT(FDT_ERR_TRUNCATED),
-- 
2.9.3

[PATCH 2/4] libfdt: overlay: Report a bad overlay for mismatching local fixups

[Maxime Ripard]

The __local_fixups__ node as a structure that mimics the structure of the
main overlay part.

This means that if we have a child node somewhere in the local fixups
sub-tree and if that node is not present in the main tree, the overlay is
poorly formatted, and we should report it as such.

Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 libfdt/fdt_overlay.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c
index 322e7adb41b8..2f306e4717ec 100644
--- a/libfdt/fdt_overlay.c
+++ b/libfdt/fdt_overlay.c
@@ -272,6 +272,8 @@ static int overlay_update_local_node_references(void *fdto,
 
 		tree_child = fdt_subnode_offset(fdto, tree_node,
 						fixup_child_name);
+		if (ret == -FDT_ERR_NOTFOUND)
+			return -FDT_ERR_BADOVERLAY;
 		if (tree_child < 0)
 			return tree_child;
 
-- 
2.9.3

[PATCH 3/4] libfdt: overlay: Fix symbols and fixups nodes condition

[Maxime Ripard]

Some base device tree might not have any __symbols__ nodes, since they
might not have any phandle at all.

Similarly, if an overlay doesn't use any base device tree phandles, its
__fixups__ node will be empty.

In such cases, we don't want to stop the phandle parsing, but rather just
ignore the error reported about the missing node.

If it's actually an issue for the overlay we're trying to apply on a given
base device tree, it will be caught later on, but we cannot make the
assumption that early in the application process.

Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 libfdt/fdt_overlay.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c
index 2f306e4717ec..bb4140412971 100644
--- a/libfdt/fdt_overlay.c
+++ b/libfdt/fdt_overlay.c
@@ -492,13 +492,12 @@ static int overlay_fixup_phandles(void *fdt, void *fdto)
 
 	/* We can have overlays without any fixups */
 	fixups_off = fdt_path_offset(fdto, "/__fixups__");
-	if (fixups_off == -FDT_ERR_NOTFOUND)
-		return 0;
-	if (fixups_off < 0)
+	if ((fixups_off < 0 && (fixups_off != -FDT_ERR_NOTFOUND)))
 		return fixups_off;
 
+	/* And base DTs without symbols */
 	symbols_off = fdt_path_offset(fdt, "/__symbols__");
-	if (symbols_off < 0)
+	if ((symbols_off < 0 && (symbols_off != -FDT_ERR_NOTFOUND)))
 		return symbols_off;
 
 	fdt_for_each_property_offset(property, fdto, fixups_off) {
-- 
2.9.3

[PATCH 4/4] tests: overlay: Move back the bad fixup tests

[Maxime Ripard]

The bad fixups tests were meant to be usable even for a non-overlay-enabled
dtc.

Move them out of that check.

Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 tests/run_tests.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 10b41bca85cb..dc19f80b2fb6 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -181,14 +181,14 @@ overlay_tests () {
         run_dtc_test -@ -I dts -O dtb -o overlay_base.dtb overlay_base.dts
         run_dtc_test -@ -I dts -O dtb -o overlay_overlay.dtb overlay_overlay_dtc.dts
         run_test overlay overlay_base.dtb overlay_overlay.dtb
-
-	# Bad fixup tests
-	for test in $BAD_FIXUP_TREES; do
-	    tree="overlay_bad_fixup_$test"
-	    run_dtc_test -I dts -O dtb -o $tree.dtb $tree.dts
-	    run_test overlay_bad_fixup overlay_base.dtb $tree.dtb
-	done
     fi
+
+    # Bad fixup tests
+    for test in $BAD_FIXUP_TREES; do
+	tree="overlay_bad_fixup_$test"
+	run_dtc_test -I dts -O dtb -o $tree.dtb $tree.dts
+	run_test overlay_bad_fixup overlay_base.dtb $tree.dtb
+    done
 }
 
 tree1_tests () {
-- 
2.9.3

Re: [PATCH 0/4] libfdt: A few overlay fixes

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1590 bytes --]

On Thu, Oct 06, 2016 at 01:39:55PM +0200, Maxime Ripard wrote:
> Hi David,
> 
> Here are a few patches to fix the things you requested and a few bugs
> encountered while doing so.
> 
> Let me know what you think,

All applied, thanks.

Couple of other little errors to correct if you have time:

  - BADOVERLAY is listed twice in the error string table (a hangover
    from when there was both BADFIXUP and BADOVERLAY, I guess)

  - The testcases re-use the same file name for the intermediate
    dtbs.  That's not a terrible problem, but it can make sorting out
    what's going on when there are multiple errors in a test run
    harder

  - Generated dtb/dts files during the test should generally have a
   .test.dtb / .test.dts extension (makes gitignores and clean targets
   easier)

  - In order for the apply_overlay function to be usable from the
    libfdt shared library, you'll need to add it to version.lds

> Maxime
> 
> Maxime Ripard (4):
>   libfdt: Add BADPHANDLE error string
>   libfdt: overlay: Report a bad overlay for mismatching local fixups
>   libfdt: overlay: Fix symbols and fixups nodes condition
>   tests: overlay: Move back the bad fixup tests
> 
>  libfdt/fdt_overlay.c  |  9 +++++----
>  libfdt/fdt_strerror.c |  1 +
>  tests/run_tests.sh    | 14 +++++++-------
>  3 files changed, 13 insertions(+), 11 deletions(-)
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 0/4] libfdt: A few overlay fixes

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1288 bytes --]

Hi David,

On Fri, Oct 07, 2016 at 03:08:35PM +1100, David Gibson wrote:
> On Thu, Oct 06, 2016 at 01:39:55PM +0200, Maxime Ripard wrote:
> > Hi David,
> > 
> > Here are a few patches to fix the things you requested and a few bugs
> > encountered while doing so.
> > 
> > Let me know what you think,
> 
> All applied, thanks.

It doesn't look like it's on the repo, did you push it?

> Couple of other little errors to correct if you have time:
> 
>   - BADOVERLAY is listed twice in the error string table (a hangover
>     from when there was both BADFIXUP and BADOVERLAY, I guess)
> 
>   - The testcases re-use the same file name for the intermediate
>     dtbs.  That's not a terrible problem, but it can make sorting out
>     what's going on when there are multiple errors in a test run
>     harder
> 
>   - Generated dtb/dts files during the test should generally have a
>    .test.dtb / .test.dts extension (makes gitignores and clean targets
>    easier)
> 
>   - In order for the apply_overlay function to be usable from the
>     libfdt shared library, you'll need to add it to version.lds

I'll deal with those this week. Thanks!
Maxime

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 0/4] libfdt: A few overlay fixes

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1550 bytes --]

On Mon, Oct 10, 2016 at 12:08:19PM +0200, Maxime Ripard wrote:
> Hi David,
> 
> On Fri, Oct 07, 2016 at 03:08:35PM +1100, David Gibson wrote:
> > On Thu, Oct 06, 2016 at 01:39:55PM +0200, Maxime Ripard wrote:
> > > Hi David,
> > > 
> > > Here are a few patches to fix the things you requested and a few bugs
> > > encountered while doing so.
> > > 
> > > Let me know what you think,
> > 
> > All applied, thanks.
> 
> It doesn't look like it's on the repo, did you push it?

Oops, I did not.

Try now.

> > Couple of other little errors to correct if you have time:
> > 
> >   - BADOVERLAY is listed twice in the error string table (a hangover
> >     from when there was both BADFIXUP and BADOVERLAY, I guess)
> > 
> >   - The testcases re-use the same file name for the intermediate
> >     dtbs.  That's not a terrible problem, but it can make sorting out
> >     what's going on when there are multiple errors in a test run
> >     harder
> > 
> >   - Generated dtb/dts files during the test should generally have a
> >    .test.dtb / .test.dts extension (makes gitignores and clean targets
> >    easier)
> > 
> >   - In order for the apply_overlay function to be usable from the
> >     libfdt shared library, you'll need to add it to version.lds
> 
> I'll deal with those this week. Thanks!
> Maxime
> 



-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 0/4] libfdt: A few overlay fixes

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 766 bytes --]

On Tue, Oct 11, 2016 at 12:40:35AM +1100, David Gibson wrote:
> On Mon, Oct 10, 2016 at 12:08:19PM +0200, Maxime Ripard wrote:
> > Hi David,
> > 
> > On Fri, Oct 07, 2016 at 03:08:35PM +1100, David Gibson wrote:
> > > On Thu, Oct 06, 2016 at 01:39:55PM +0200, Maxime Ripard wrote:
> > > > Hi David,
> > > > 
> > > > Here are a few patches to fix the things you requested and a few bugs
> > > > encountered while doing so.
> > > > 
> > > > Let me know what you think,
> > > 
> > > All applied, thanks.
> > 
> > It doesn't look like it's on the repo, did you push it?
> 
> Oops, I did not.
> 
> Try now.

Yep, it's there now, thanks!
Maxime

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 0/4] libfdt: A few overlay fixes

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 577 bytes --]

Hi,

On Fri, Oct 07, 2016 at 03:08:35PM +1100, David Gibson wrote:
>   - The testcases re-use the same file name for the intermediate
>     dtbs.  That's not a terrible problem, but it can make sorting out
>     what's going on when there are multiple errors in a test run
>     harder

I'm not sure how exactly you want me to fix that. All the overlays
have a different name already. Or do you want me to rename the base
device tree as well?

Thanks,
Maxime

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 0/4] libfdt: A few overlay fixes

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 914 bytes --]

On Tue, Oct 11, 2016 at 10:16:06AM +0200, Maxime Ripard wrote:
> Hi,
> 
> On Fri, Oct 07, 2016 at 03:08:35PM +1100, David Gibson wrote:
> >   - The testcases re-use the same file name for the intermediate
> >     dtbs.  That's not a terrible problem, but it can make sorting out
> >     what's going on when there are multiple errors in a test run
> >     harder
> 
> I'm not sure how exactly you want me to fix that. All the overlays
> have a different name already. Or do you want me to rename the base
> device tree as well?

Uh.. looking again the problem is less extensive than I thought.

It's just that the variants of the base tree compiled with and without
-@ are given the same output name.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]