| Message ID | CAN1kZoqXPY7+C7f7-Fz5+puk836k+NBxTfYXxFiQ2qAYfvwSOw@mail.gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
On Thu, Sep 15, 2016 at 04:08:33PM +0200, Mario Six wrote: > Hi Stefan, > > I also see the same error on our Armada board. It stems from the fact that > boot_get_ramdisk in common/image.c treats a ENOLINK different from all other > errors (which the patch changed into a ENOENT). The following patch fixes the > problem on our board: > > diff --git a/common/image.c b/common/image.c > index 7ad04ca..c8d9bc8 100644 > --- a/common/image.c > +++ b/common/image.c > @@ -1078,7 +1078,7 @@ int boot_get_ramdisk(int argc, char * const > argv[], bootm_headers_t *images, > rd_addr = map_to_sysmem(images->fit_hdr_os); > rd_noffset = fit_get_node_from_config(images, > FIT_RAMDISK_PROP, rd_addr); > - if (rd_noffset == -ENOLINK) > + if (rd_noffset == -ENOENT) > return 0; > else if (rd_noffset < 0) > return 1; > > So, yes, 2016.09 breaks Armada, apparently. The callers of the proposed/rejected kwboot.c errno diff were all checked but the test of the result in the caller of this one seems to have been missed indeed. Sorry for missing this and Reviewed-by: Jonathan Gray <jsg@jsg.id.au> > > Best regards, > > Mario > > On Thu, Sep 15, 2016 at 2:49 PM, Stefan Roese <sr@denx.de> wrote: > > Hi Tom, > > Hi Jonathan, > > > > On 03.09.2016 00:30, Jonathan Gray wrote: > >> ENOLINK is not required by POSIX and does not exist on OpenBSD > >> and likely other systems. > >> > >> Signed-off-by: Jonathan Gray <jsg@jsg.id.au> > >> --- > >> common/image-fit.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/common/image-fit.c b/common/image-fit.c > >> index d8d4e95..79c0375 100644 > >> --- a/common/image-fit.c > >> +++ b/common/image-fit.c > >> @@ -1566,7 +1566,7 @@ int fit_get_node_from_config(bootm_headers_t *images, const char *prop_name, > >> noffset = fit_conf_get_prop_node(fit_hdr, cfg_noffset, prop_name); > >> if (noffset < 0) { > >> debug("* %s: no '%s' in config\n", prop_name, prop_name); > >> - return -ENOLINK; > >> + return -ENOENT; > >> } > >> > >> return noffset; > >> > > > > This patch breaks Linux booting via FIT-image on Marvell Armada > > XP for me. Here a short log with this patch applied: > > > > ## Loading kernel from FIT Image at 02000000 ... > > Using 'conf@1' configuration > > Trying 'kernel@1' kernel subimage > > Description: Linux kernel > > Type: Kernel Image > > Compression: uncompressed > > Data Start: 0x020000f4 > > Data Size: 5065728 Bytes = 4.8 MiB > > Architecture: ARM > > OS: Linux > > Load Address: 0x00008000 > > Entry Point: 0x00008000 > > Hash algo: sha1 > > Hash value: 11ddefa0b68cbc5db9d84b0fd74ec67da155fada > > Verifying Hash Integrity ... sha1+ OK > > Ramdisk image is corrupt or invalid > > > > > > And this is how is should look like: > > > > ## Loading kernel from FIT Image at 02000000 ... > > Using 'conf@1' configuration > > Trying 'kernel@1' kernel subimage > > Description: Linux kernel > > Type: Kernel Image > > Compression: uncompressed > > Data Start: 0x020000f4 > > Data Size: 5065728 Bytes = 4.8 MiB > > Architecture: ARM > > OS: Linux > > Load Address: 0x00008000 > > Entry Point: 0x00008000 > > Hash algo: sha1 > > Hash value: 11ddefa0b68cbc5db9d84b0fd74ec67da155fada > > Verifying Hash Integrity ... sha1+ OK > > ## Loading fdt from FIT Image at 02000000 ... > > Using 'conf@1' configuration > > Trying 'fdt@1' fdt subimage > > Description: Flattened Device Tree blob > > Type: Flat Device Tree > > Compression: uncompressed > > Data Start: 0x024d4de8 > > Data Size: 16971 Bytes = 16.6 KiB > > Architecture: ARM > > Hash algo: sha1 > > Hash value: 672f2964b334406749265f4508e2231fb54ccbf4 > > Verifying Hash Integrity ... sha1+ OK > > Booting using the fdt blob at 0x24d4de8 > > Loading Kernel Image ... OK > > Loading Device Tree to 0fff8000, end 0ffff24a ... OK > > > > Starting kernel ... > > > > > > Unfortunately v2016.09 is useless with this patch - at least for me. > > I really think that we should revert it and release v2016.09.01. > > > > What do you think? > > > > Thanks, > > Stefan > > _______________________________________________ > > U-Boot mailing list > > U-Boot@lists.denx.de > > http://lists.denx.de/mailman/listinfo/u-boot
Hi Jonathan On Thu, Sep 15, 2016 at 11:27 PM, Jonathan Gray <jsg@jsg.id.au> wrote: > > The callers of the proposed/rejected kwboot.c errno diff were all > checked but the test of the result in the caller of this one > seems to have been missed indeed. > > Sorry for missing this and > > Reviewed-by: Jonathan Gray <jsg@jsg.id.au> > The whole FIT image loading complex still is quite messy in regards to return values and the like (I still have two patches I want to send that fix masked and incorrect return values), so it's basically still a work-in-progress :-) I'll send a formal patch in a few. Best regards, Mario
diff --git a/common/image.c b/common/image.c index 7ad04ca..c8d9bc8 100644 --- a/common/image.c +++ b/common/image.c @@ -1078,7 +1078,7 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images, rd_addr = map_to_sysmem(images->fit_hdr_os); rd_noffset = fit_get_node_from_config(images, FIT_RAMDISK_PROP, rd_addr); - if (rd_noffset == -ENOLINK) + if (rd_noffset == -ENOENT) return 0; else if (rd_noffset < 0)
Hi Stefan, I also see the same error on our Armada board. It stems from the fact that boot_get_ramdisk in common/image.c treats a ENOLINK different from all other errors (which the patch changed into a ENOENT). The following patch fixes the problem on our board: return 1; So, yes, 2016.09 breaks Armada, apparently. Best regards, Mario On Thu, Sep 15, 2016 at 2:49 PM, Stefan Roese <sr@denx.de> wrote: > Hi Tom, > Hi Jonathan, > > On 03.09.2016 00:30, Jonathan Gray wrote: >> ENOLINK is not required by POSIX and does not exist on OpenBSD >> and likely other systems. >> >> Signed-off-by: Jonathan Gray <jsg@jsg.id.au> >> --- >> common/image-fit.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/common/image-fit.c b/common/image-fit.c >> index d8d4e95..79c0375 100644 >> --- a/common/image-fit.c >> +++ b/common/image-fit.c >> @@ -1566,7 +1566,7 @@ int fit_get_node_from_config(bootm_headers_t *images, const char *prop_name, >> noffset = fit_conf_get_prop_node(fit_hdr, cfg_noffset, prop_name); >> if (noffset < 0) { >> debug("* %s: no '%s' in config\n", prop_name, prop_name); >> - return -ENOLINK; >> + return -ENOENT; >> } >> >> return noffset; >> > > This patch breaks Linux booting via FIT-image on Marvell Armada > XP for me. Here a short log with this patch applied: > > ## Loading kernel from FIT Image at 02000000 ... > Using 'conf@1' configuration > Trying 'kernel@1' kernel subimage > Description: Linux kernel > Type: Kernel Image > Compression: uncompressed > Data Start: 0x020000f4 > Data Size: 5065728 Bytes = 4.8 MiB > Architecture: ARM > OS: Linux > Load Address: 0x00008000 > Entry Point: 0x00008000 > Hash algo: sha1 > Hash value: 11ddefa0b68cbc5db9d84b0fd74ec67da155fada > Verifying Hash Integrity ... sha1+ OK > Ramdisk image is corrupt or invalid > > > And this is how is should look like: > > ## Loading kernel from FIT Image at 02000000 ... > Using 'conf@1' configuration > Trying 'kernel@1' kernel subimage > Description: Linux kernel > Type: Kernel Image > Compression: uncompressed > Data Start: 0x020000f4 > Data Size: 5065728 Bytes = 4.8 MiB > Architecture: ARM > OS: Linux > Load Address: 0x00008000 > Entry Point: 0x00008000 > Hash algo: sha1 > Hash value: 11ddefa0b68cbc5db9d84b0fd74ec67da155fada > Verifying Hash Integrity ... sha1+ OK > ## Loading fdt from FIT Image at 02000000 ... > Using 'conf@1' configuration > Trying 'fdt@1' fdt subimage > Description: Flattened Device Tree blob > Type: Flat Device Tree > Compression: uncompressed > Data Start: 0x024d4de8 > Data Size: 16971 Bytes = 16.6 KiB > Architecture: ARM > Hash algo: sha1 > Hash value: 672f2964b334406749265f4508e2231fb54ccbf4 > Verifying Hash Integrity ... sha1+ OK > Booting using the fdt blob at 0x24d4de8 > Loading Kernel Image ... OK > Loading Device Tree to 0fff8000, end 0ffff24a ... OK > > Starting kernel ... > > > Unfortunately v2016.09 is useless with this patch - at least for me. > I really think that we should revert it and release v2016.09.01. > > What do you think? > > Thanks, > Stefan > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot