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