All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/2] libfdt: fdt_address_cells() and fdt_size_cells()
@ 2019-07-26  9:13 matthias.bgg at kernel.org
  2019-07-26  9:13 ` [U-Boot] [PATCH 2/2] libfdt: return correct value if #size-cells property is not present matthias.bgg at kernel.org
  2019-08-13  9:34 ` [U-Boot] [PATCH 1/2] libfdt: fdt_address_cells() and fdt_size_cells() Simon Glass
  0 siblings, 2 replies; 6+ messages in thread
From: matthias.bgg at kernel.org @ 2019-07-26  9:13 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/2] libfdt: return correct value if #size-cells property is not present
  2019-07-26  9:13 [U-Boot] [PATCH 1/2] libfdt: fdt_address_cells() and fdt_size_cells() matthias.bgg at kernel.org
@ 2019-07-26  9:13 ` matthias.bgg at kernel.org
  2019-08-01 11:42   ` Matthias Brugger
  2019-08-13  9:34 ` [U-Boot] [PATCH 1/2] libfdt: fdt_address_cells() and fdt_size_cells() Simon Glass
  1 sibling, 1 reply; 6+ messages in thread
From: matthias.bgg at kernel.org @ 2019-07-26  9:13 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/2] libfdt: return correct value if #size-cells property is not present
  2019-07-26  9:13 ` [U-Boot] [PATCH 2/2] libfdt: return correct value if #size-cells property is not present matthias.bgg at kernel.org
@ 2019-08-01 11:42   ` Matthias Brugger
  2019-08-13  9:34     ` Simon Glass
  0 siblings, 1 reply; 6+ messages in thread
From: Matthias Brugger @ 2019-08-01 11:42 UTC (permalink / raw)
  To: u-boot

Hi all,

On 26/07/2019 11:13, matthias.bgg at 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,
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 1/2] libfdt: fdt_address_cells() and fdt_size_cells()
  2019-07-26  9:13 [U-Boot] [PATCH 1/2] libfdt: fdt_address_cells() and fdt_size_cells() matthias.bgg at kernel.org
  2019-07-26  9:13 ` [U-Boot] [PATCH 2/2] libfdt: return correct value if #size-cells property is not present matthias.bgg at kernel.org
@ 2019-08-13  9:34 ` Simon Glass
  1 sibling, 0 replies; 6+ messages in thread
From: Simon Glass @ 2019-08-13  9:34 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/2] libfdt: return correct value if #size-cells property is not present
  2019-08-01 11:42   ` Matthias Brugger
@ 2019-08-13  9:34     ` Simon Glass
  2019-09-04 16:29       ` Matthias Brugger
  0 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2019-08-13  9:34 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/2] libfdt: return correct value if #size-cells property is not present
  2019-08-13  9:34     ` Simon Glass
@ 2019-09-04 16:29       ` Matthias Brugger
  0 siblings, 0 replies; 6+ messages in thread
From: Matthias Brugger @ 2019-09-04 16:29 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-09-04 16:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-26  9:13 [U-Boot] [PATCH 1/2] libfdt: fdt_address_cells() and fdt_size_cells() matthias.bgg at kernel.org
2019-07-26  9:13 ` [U-Boot] [PATCH 2/2] libfdt: return correct value if #size-cells property is not present matthias.bgg at kernel.org
2019-08-01 11:42   ` Matthias Brugger
2019-08-13  9:34     ` Simon Glass
2019-09-04 16:29       ` Matthias Brugger
2019-08-13  9:34 ` [U-Boot] [PATCH 1/2] libfdt: fdt_address_cells() and fdt_size_cells() 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.