diff mbox series

[U-Boot,v2,2/4] libfdt: return correct value if #size-cells property is not present

Message ID 20190905084849.20596-3-matthias.bgg@kernel.org
State Accepted
Commit 0ba41ce1b7816c229cc19e0621148b98f990cb68
Delegated to: Simon Glass
Headers show
Series Fix default values for address and size cells | expand

Commit Message

Matthias Brugger Sept. 5, 2019, 8:48 a.m. UTC
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(-)

Comments

Simon Glass Sept. 17, 2019, 5:48 a.m. UTC | #1
Hi,

On Thu, 5 Sep 2019 at 02:49, <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>

This is v2 but I don't see a change log?

> ---
>
>  scripts/dtc/libfdt/fdt_addresses.c | 16 +++++++++++++---
>  scripts/dtc/libfdt/libfdt.h        |  2 +-
>  2 files changed, 14 insertions(+), 4 deletions(-)

Regards,
Simon
Matthias Brugger Sept. 17, 2019, 7:28 a.m. UTC | #2
Hi Simon,

On 17/09/2019 07:48, Simon Glass wrote:
> Hi,
> 
> On Thu, 5 Sep 2019 at 02:49, <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>
> 
> This is v2 but I don't see a change log?
> 

I put the changelog into the cover letter:
https://patchwork.ozlabs.org/cover/1158304/

From your email I understand that you prefer a patch by patch changelog, correct?

Regards,
Matthias

>> ---
>>
>>  scripts/dtc/libfdt/fdt_addresses.c | 16 +++++++++++++---
>>  scripts/dtc/libfdt/libfdt.h        |  2 +-
>>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> Regards,
> Simon
>
Simon Glass Sept. 17, 2019, 4:52 p.m. UTC | #3
Hi Matthias,

On Tue, 17 Sep 2019 at 00:29, Matthias Brugger <mbrugger@suse.com> wrote:
>
> Hi Simon,
>
> On 17/09/2019 07:48, Simon Glass wrote:
> > Hi,
> >
> > On Thu, 5 Sep 2019 at 02:49, <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>
> >
> > This is v2 but I don't see a change log?
> >
>
> I put the changelog into the cover letter:
> https://patchwork.ozlabs.org/cover/1158304/
>
> From your email I understand that you prefer a patch by patch changelog, correct?

Both is best. If you use patman it does this for you.

Regards,
Simon
Simon Glass Sept. 27, 2019, 1:48 a.m. UTC | #4
On Tue, 17 Sep 2019 at 09:52, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Matthias,
>
> On Tue, 17 Sep 2019 at 00:29, Matthias Brugger <mbrugger@suse.com> wrote:
> >
> > Hi Simon,
> >
> > On 17/09/2019 07:48, Simon Glass wrote:
> > > Hi,
> > >
> > > On Thu, 5 Sep 2019 at 02:49, <matthias.bgg@kernel.org> wrote:
> > >>
> > >> From: Matthias Brugger <mbrugger@suse.com>
> > >>
> > >> According to the device tree specification, the default value for

There seems to be something missing here/...

> > >> 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>
> > >
> > > This is v2 but I don't see a change log?
> > >
> >
> > I put the changelog into the cover letter:
> > https://patchwork.ozlabs.org/cover/1158304/
> >
> > From your email I understand that you prefer a patch by patch changelog, correct?
>
> Both is best. If you use patman it does this for you.

Reviewed-by: Simon Glass <sjg@chromium.org>

Regards,
Simon
Matthias Brugger Sept. 27, 2019, 11:37 a.m. UTC | #5
On 27/09/2019 03:48, Simon Glass wrote:
> On Tue, 17 Sep 2019 at 09:52, Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Matthias,
>>
>> On Tue, 17 Sep 2019 at 00:29, Matthias Brugger <mbrugger@suse.com> wrote:
>>>
>>> Hi Simon,
>>>
>>> On 17/09/2019 07:48, Simon Glass wrote:
>>>> Hi,
>>>>
>>>> On Thu, 5 Sep 2019 at 02:49, <matthias.bgg@kernel.org> wrote:
>>>>>
>>>>> From: Matthias Brugger <mbrugger@suse.com>
>>>>>
>>>>> According to the device tree specification, the default value for
> 
> There seems to be something missing here/...
> 

Argh, you are right, I accidentally deleted one line from the commit message.
The correct paragraph should read like this:

<snip>
According to the device tree specification, the default value for
#size-cells is 1, but fdt_size_cells() was returning 2 if this property
was not present.
</snip>

BTW this series is a fix and I'd like to see it in v2019.10-rc5. I'm not sure
who is to take it, you or Tom.

Regards,
Matthias

>>>>> 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>
>>>>
>>>> This is v2 but I don't see a change log?
>>>>
>>>
>>> I put the changelog into the cover letter:
>>> https://patchwork.ozlabs.org/cover/1158304/
>>>
>>> From your email I understand that you prefer a patch by patch changelog, correct?
>>
>> Both is best. If you use patman it does this for you.
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> Regards,
> Simon
>
Simon Glass Sept. 27, 2019, 11:28 p.m. UTC | #6
On 27/09/2019 03:48, Simon Glass wrote:
> On Tue, 17 Sep 2019 at 09:52, Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Matthias,
>>
>> On Tue, 17 Sep 2019 at 00:29, Matthias Brugger <mbrugger@suse.com> wrote:
>>>
>>> Hi Simon,
>>>
>>> On 17/09/2019 07:48, Simon Glass wrote:
>>>> Hi,
>>>>
>>>> On Thu, 5 Sep 2019 at 02:49, <matthias.bgg@kernel.org> wrote:
>>>>>
>>>>> From: Matthias Brugger <mbrugger@suse.com>
>>>>>
>>>>> According to the device tree specification, the default value for
>
> There seems to be something missing here/...
>

Argh, you are right, I accidentally deleted one line from the commit message.
The correct paragraph should read like this:

<snip>
According to the device tree specification, the default value for
#size-cells is 1, but fdt_size_cells() was returning 2 if this property
was not present.
</snip>

BTW this series is a fix and I'd like to see it in v2019.10-rc5. I'm not sure
who is to take it, you or Tom.

Regards,
Matthias

>>>>> 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>
>>>>
>>>> This is v2 but I don't see a change log?
>>>>
>>>
>>> I put the changelog into the cover letter:
>>> https://patchwork.ozlabs.org/cover/1158304/
>>>
>>> From your email I understand that you prefer a patch by patch changelog, correct?
>>
>> Both is best. If you use patman it does this for you.
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> Regards,
> Simon
>

Applied to u-boot-dm/next, thanks!
diff mbox series

Patch

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,