[U-Boot] [PATCH 1/2] libfdt: fdt_address_cells() and fdt_size_cells()
From: Matthias Brugger mbrugger@suse.com
Add internal fdt_cells() to avoid copy and paste. Fix typo in fdt_size_cells() documentation comment.
This is based in upstream commit: c12b2b0 ("libfdt: fdt_address_cells() and fdt_size_cells()") but misses the test cases, as we don't implement them in u-boot.
Signed-off-by: Matthias Brugger mbrugger@suse.com --- scripts/dtc/libfdt/fdt_addresses.c | 35 +++++++++++------------------- scripts/dtc/libfdt/libfdt.h | 2 +- 2 files changed, 14 insertions(+), 23 deletions(-)
diff --git a/scripts/dtc/libfdt/fdt_addresses.c b/scripts/dtc/libfdt/fdt_addresses.c index eff4dbcc72..49537b578d 100644 --- a/scripts/dtc/libfdt/fdt_addresses.c +++ b/scripts/dtc/libfdt/fdt_addresses.c @@ -1,6 +1,7 @@ /* * libfdt - Flat Device Tree manipulation * Copyright (C) 2014 David Gibson david@gibson.dropbear.id.au + * Copyright (C) 2018 embedded brains GmbH * * libfdt is dual licensed: you can use it either under the terms of * the GPL, or the BSD license, at your option. @@ -55,42 +56,32 @@
#include "libfdt_internal.h"
-int fdt_address_cells(const void *fdt, int nodeoffset) +static int fdt_cells(const void *fdt, int nodeoffset, const char *name) { - const fdt32_t *ac; + const fdt32_t *c; int val; int len;
- ac = fdt_getprop(fdt, nodeoffset, "#address-cells", &len); - if (!ac) + c = fdt_getprop(fdt, nodeoffset, name, &len); + if (!c) return 2;
- if (len != sizeof(*ac)) + if (len != sizeof(*c)) return -FDT_ERR_BADNCELLS;
- val = fdt32_to_cpu(*ac); + val = fdt32_to_cpu(*c); if ((val <= 0) || (val > FDT_MAX_NCELLS)) return -FDT_ERR_BADNCELLS;
return val; }
-int fdt_size_cells(const void *fdt, int nodeoffset) +int fdt_address_cells(const void *fdt, int nodeoffset) { - const fdt32_t *sc; - int val; - int len; - - sc = fdt_getprop(fdt, nodeoffset, "#size-cells", &len); - if (!sc) - return 2; - - if (len != sizeof(*sc)) - return -FDT_ERR_BADNCELLS; - - val = fdt32_to_cpu(*sc); - if ((val < 0) || (val > FDT_MAX_NCELLS)) - return -FDT_ERR_BADNCELLS; + return fdt_cells(fdt, nodeoffset, "#address-cells"); +}
- return val; +int fdt_size_cells(const void *fdt, int nodeoffset) +{ + return fdt_cells(fdt, nodeoffset, "#size-cells"); } diff --git a/scripts/dtc/libfdt/libfdt.h b/scripts/dtc/libfdt/libfdt.h index cf86ddba88..66f01fec53 100644 --- a/scripts/dtc/libfdt/libfdt.h +++ b/scripts/dtc/libfdt/libfdt.h @@ -1109,7 +1109,7 @@ int fdt_address_cells(const void *fdt, int nodeoffset); * * returns: * 0 <= n < FDT_MAX_NCELLS, on success - * 2, if the node has no #address-cells property + * 2, if the node has no #size-cells property * -FDT_ERR_BADNCELLS, if the node has a badly formatted or invalid * #size-cells property * -FDT_ERR_BADMAGIC,
From: Matthias Brugger mbrugger@suse.com
According to the device tree specification, the default value for was not present.
This patch also makes fdt_address_cells() and fdt_size_cells() conform to the behaviour documented in libfdt.h. The defaults are only returned if fdt_getprop() returns -FDT_ERR_NOTFOUND, otherwise the actual error is returned.
This is based on upstream commit: aa7254d ("libfdt: return correct value if #size-cells property is not present") but misses the test case part, as we don't implement them in u-boot.
Signed-off-by: Matthias Brugger mbrugger@suse.com --- scripts/dtc/libfdt/fdt_addresses.c | 16 +++++++++++++--- scripts/dtc/libfdt/libfdt.h | 2 +- 2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/scripts/dtc/libfdt/fdt_addresses.c b/scripts/dtc/libfdt/fdt_addresses.c index 49537b578d..f13a87dfa0 100644 --- a/scripts/dtc/libfdt/fdt_addresses.c +++ b/scripts/dtc/libfdt/fdt_addresses.c @@ -64,7 +64,7 @@ static int fdt_cells(const void *fdt, int nodeoffset, const char *name)
c = fdt_getprop(fdt, nodeoffset, name, &len); if (!c) - return 2; + return len;
if (len != sizeof(*c)) return -FDT_ERR_BADNCELLS; @@ -78,10 +78,20 @@ static int fdt_cells(const void *fdt, int nodeoffset, const char *name)
int fdt_address_cells(const void *fdt, int nodeoffset) { - return fdt_cells(fdt, nodeoffset, "#address-cells"); + int val; + + val = fdt_cells(fdt, nodeoffset, "#address-cells"); + if (val == -FDT_ERR_NOTFOUND) + return 2; + return val; }
int fdt_size_cells(const void *fdt, int nodeoffset) { - return fdt_cells(fdt, nodeoffset, "#size-cells"); + int val; + + val = fdt_cells(fdt, nodeoffset, "#size-cells"); + if (val == -FDT_ERR_NOTFOUND) + return 1; + return val; } diff --git a/scripts/dtc/libfdt/libfdt.h b/scripts/dtc/libfdt/libfdt.h index 66f01fec53..5c778b115b 100644 --- a/scripts/dtc/libfdt/libfdt.h +++ b/scripts/dtc/libfdt/libfdt.h @@ -1109,7 +1109,7 @@ int fdt_address_cells(const void *fdt, int nodeoffset); * * returns: * 0 <= n < FDT_MAX_NCELLS, on success - * 2, if the node has no #size-cells property + * 1, if the node has no #size-cells property * -FDT_ERR_BADNCELLS, if the node has a badly formatted or invalid * #size-cells property * -FDT_ERR_BADMAGIC,
Hi all,
On 26/07/2019 11:13, matthias.bgg@kernel.org wrote:
From: Matthias Brugger mbrugger@suse.com
According to the device tree specification, the default value for was not present.
This patch also makes fdt_address_cells() and fdt_size_cells() conform to the behaviour documented in libfdt.h. The defaults are only returned if fdt_getprop() returns -FDT_ERR_NOTFOUND, otherwise the actual error is returned.
This is based on upstream commit: aa7254d ("libfdt: return correct value if #size-cells property is not present") but misses the test case part, as we don't implement them in u-boot.
Signed-off-by: Matthias Brugger mbrugger@suse.com
After running these two patches through the CI [1] I realized that three test are failing: test/py sandbox test/py sandbox with clang test/py sandbox_flattree
All three fail dm_test_fdt_translation() in the case "No translation for busses with #size-cells == 0" [2].
Can anybody with more insight in the test infrastructure and the sandbox architecture help me to identify if this is a) a bug in the sandbox b) a bug in our test c) a bug in my patch
I write this because I'm pretty sure that it is not option c), as we just stick to the specs here.
Regards, Matthias
[1] https://travis-ci.org/mbgg/u-boot/builds/565955218 [2] https://github.com/u-boot/u-boot/blob/master/test/dm/test-fdt.c#L511
scripts/dtc/libfdt/fdt_addresses.c | 16 +++++++++++++--- scripts/dtc/libfdt/libfdt.h | 2 +- 2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/scripts/dtc/libfdt/fdt_addresses.c b/scripts/dtc/libfdt/fdt_addresses.c index 49537b578d..f13a87dfa0 100644 --- a/scripts/dtc/libfdt/fdt_addresses.c +++ b/scripts/dtc/libfdt/fdt_addresses.c @@ -64,7 +64,7 @@ static int fdt_cells(const void *fdt, int nodeoffset, const char *name)
c = fdt_getprop(fdt, nodeoffset, name, &len); if (!c)
return 2;
return len;if (len != sizeof(*c)) return -FDT_ERR_BADNCELLS;
@@ -78,10 +78,20 @@ static int fdt_cells(const void *fdt, int nodeoffset, const char *name)
int fdt_address_cells(const void *fdt, int nodeoffset) {
- return fdt_cells(fdt, nodeoffset, "#address-cells");
- int val;
- val = fdt_cells(fdt, nodeoffset, "#address-cells");
- if (val == -FDT_ERR_NOTFOUND)
return 2;- return val;
}
int fdt_size_cells(const void *fdt, int nodeoffset) {
- return fdt_cells(fdt, nodeoffset, "#size-cells");
- int val;
- val = fdt_cells(fdt, nodeoffset, "#size-cells");
- if (val == -FDT_ERR_NOTFOUND)
return 1;- return val;
} diff --git a/scripts/dtc/libfdt/libfdt.h b/scripts/dtc/libfdt/libfdt.h index 66f01fec53..5c778b115b 100644 --- a/scripts/dtc/libfdt/libfdt.h +++ b/scripts/dtc/libfdt/libfdt.h @@ -1109,7 +1109,7 @@ int fdt_address_cells(const void *fdt, int nodeoffset);
- returns:
- 0 <= n < FDT_MAX_NCELLS, on success
2, if the node has no #size-cells property
1, if the node has no #size-cells property-FDT_ERR_BADNCELLS, if the node has a badly formatted or invalid#size-cells property- -FDT_ERR_BADMAGIC,
+Stephen Warren
Hi Matthias,
On Thu, 1 Aug 2019 at 05:42, Matthias Brugger matthias.bgg@gmail.com wrote:
Hi all,
On 26/07/2019 11:13, matthias.bgg@kernel.org wrote:
From: Matthias Brugger mbrugger@suse.com
According to the device tree specification, the default value for was not present.
This patch also makes fdt_address_cells() and fdt_size_cells() conform to the behaviour documented in libfdt.h. The defaults are only returned if fdt_getprop() returns -FDT_ERR_NOTFOUND, otherwise the actual error is returned.
This is based on upstream commit: aa7254d ("libfdt: return correct value if #size-cells property is not present") but misses the test case part, as we don't implement them in u-boot.
Signed-off-by: Matthias Brugger mbrugger@suse.com
After running these two patches through the CI [1] I realized that three test are failing: test/py sandbox test/py sandbox with clang test/py sandbox_flattree
All three fail dm_test_fdt_translation() in the case "No translation for busses with #size-cells == 0" [2].
Can anybody with more insight in the test infrastructure and the sandbox architecture help me to identify if this is a) a bug in the sandbox b) a bug in our test c) a bug in my patch
I write this because I'm pretty sure that it is not option c), as we just stick to the specs here.
Can you check the test and see? It might well be that the test is wrong.
I hope we don't have tet code relying on this.
Regards, SImon
Hi all,
On 13/08/2019 11:34, Simon Glass wrote:
+Stephen Warren
Hi Matthias,
On Thu, 1 Aug 2019 at 05:42, Matthias Brugger matthias.bgg@gmail.com wrote:
Hi all,
On 26/07/2019 11:13, matthias.bgg@kernel.org wrote:
From: Matthias Brugger mbrugger@suse.com
According to the device tree specification, the default value for was not present.
This patch also makes fdt_address_cells() and fdt_size_cells() conform to the behaviour documented in libfdt.h. The defaults are only returned if fdt_getprop() returns -FDT_ERR_NOTFOUND, otherwise the actual error is returned.
This is based on upstream commit: aa7254d ("libfdt: return correct value if #size-cells property is not present") but misses the test case part, as we don't implement them in u-boot.
Signed-off-by: Matthias Brugger mbrugger@suse.com
After running these two patches through the CI [1] I realized that three test are failing: test/py sandbox test/py sandbox with clang test/py sandbox_flattree
All three fail dm_test_fdt_translation() in the case "No translation for busses with #size-cells == 0" [2].
Can anybody with more insight in the test infrastructure and the sandbox architecture help me to identify if this is a) a bug in the sandbox b) a bug in our test c) a bug in my patch
I write this because I'm pretty sure that it is not option c), as we just stick to the specs here.
Can you check the test and see? It might well be that the test is wrong.
I hope we don't have tet code relying on this.
I think I found the error. I missed a commit in libftd which fixes the issue. I'll send a v2 soon.
Thanks, Matthias
On Fri, 26 Jul 2019 at 03:13, matthias.bgg@kernel.org wrote:
From: Matthias Brugger mbrugger@suse.com
Add internal fdt_cells() to avoid copy and paste. Fix typo in fdt_size_cells() documentation comment.
This is based in upstream commit: c12b2b0 ("libfdt: fdt_address_cells() and fdt_size_cells()") but misses the test cases, as we don't implement them in u-boot.
U-Boot
I suppose we could implement them but the idea is to keep the code synced with upstream so it is unnecessary. I've sent a code-size series to help with that.
Signed-off-by: Matthias Brugger mbrugger@suse.com
scripts/dtc/libfdt/fdt_addresses.c | 35 +++++++++++------------------- scripts/dtc/libfdt/libfdt.h | 2 +- 2 files changed, 14 insertions(+), 23 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Regards, Simon
participants (4)
-
Matthias Brugger -
Matthias Brugger -
matthias.bgg＠kernel.org -
Simon Glass