[U-Boot] [PATCH] Revert "image-fit: switch ENOLINK to ENOENT"

[U-Boot] [PATCH] Revert "image-fit: switch ENOLINK to ENOENT"

[Paul Burton]

Commit bac17b78dace ("image-fit: switch ENOLINK to ENOENT") changed
fit_get_node_from_config to return -ENOENT when a property doesn't
exist, but didn't change any of its callers which check return values.
Notably it didn't change boot_get_ramdisk, which leads to U-Boot failing
to boot FIT images which don't include ramdisks with the following
message:

  Ramdisk image is corrupt or invalid

The offending commit seems to dislike ENOLINK due to it not existing on
OpenBSD, but I'm not sure why that matters as we define it in
include/asm-generic/errno.h anyway so simply revert the commit to fix
FIT image handling.

This reverts commit bac17b78dace ("image-fit: switch ENOLINK to
ENOENT").

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: 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 9ce68f1..f833fe3 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 -ENOENT;
+		return -ENOLINK;
 	}
 
 	return noffset;
-- 
2.9.3

[U-Boot] [PATCH] Revert "image-fit: switch ENOLINK to ENOENT"

[Jonathan Gray]

On Fri, Sep 16, 2016 at 10:21:43PM +0100, Paul Burton wrote:
> Commit bac17b78dace ("image-fit: switch ENOLINK to ENOENT") changed
> fit_get_node_from_config to return -ENOENT when a property doesn't
> exist, but didn't change any of its callers which check return values.
> Notably it didn't change boot_get_ramdisk, which leads to U-Boot failing
> to boot FIT images which don't include ramdisks with the following
> message:
> 
>   Ramdisk image is corrupt or invalid
> 
> The offending commit seems to dislike ENOLINK due to it not existing on
> OpenBSD, but I'm not sure why that matters as we define it in
> include/asm-generic/errno.h anyway so simply revert the commit to fix
> FIT image handling.

That header is not used when building native tools.
So reverting it will break the build of u-boot on OpenBSD.

  WRAP    tools/common/image-fit.c
  HOSTCC  tools/common/image-fit.o
In file included from tools/common/image-fit.c:1:
/usr/users/jsg/src/u-boot/tools/../common/image-fit.c: In function 'fit_get_node_from_config':
/usr/users/jsg/src/u-boot/tools/../common/image-fit.c:1569: error: 'ENOLINK' undeclared (first use in this function)
/usr/users/jsg/src/u-boot/tools/../common/image-fit.c:1569: error: (Each undeclared identifier is reported only once
/usr/users/jsg/src/u-boot/tools/../common/image-fit.c:1569: error: for each function it appears in.)

> 
> This reverts commit bac17b78dace ("image-fit: switch ENOLINK to
> ENOENT").
> 
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> Cc: 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 9ce68f1..f833fe3 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 -ENOENT;
> +		return -ENOLINK;
>  	}
>  
>  	return noffset;
> -- 
> 2.9.3
> 

[U-Boot] [PATCH] Revert "image-fit: switch ENOLINK to ENOENT"

[Marek Vasut]

On 09/17/2016 05:42 AM, Jonathan Gray wrote:
> On Fri, Sep 16, 2016 at 10:21:43PM +0100, Paul Burton wrote:
>> Commit bac17b78dace ("image-fit: switch ENOLINK to ENOENT") changed
>> fit_get_node_from_config to return -ENOENT when a property doesn't
>> exist, but didn't change any of its callers which check return values.
>> Notably it didn't change boot_get_ramdisk, which leads to U-Boot failing
>> to boot FIT images which don't include ramdisks with the following
>> message:
>>
>>   Ramdisk image is corrupt or invalid
>>
>> The offending commit seems to dislike ENOLINK due to it not existing on
>> OpenBSD, but I'm not sure why that matters as we define it in
>> include/asm-generic/errno.h anyway so simply revert the commit to fix
>> FIT image handling.
> 
> That header is not used when building native tools.
> So reverting it will break the build of u-boot on OpenBSD.
> 
>   WRAP    tools/common/image-fit.c
>   HOSTCC  tools/common/image-fit.o
> In file included from tools/common/image-fit.c:1:
> /usr/users/jsg/src/u-boot/tools/../common/image-fit.c: In function 'fit_get_node_from_config':
> /usr/users/jsg/src/u-boot/tools/../common/image-fit.c:1569: error: 'ENOLINK' undeclared (first use in this function)
> /usr/users/jsg/src/u-boot/tools/../common/image-fit.c:1569: error: (Each undeclared identifier is reported only once
> /usr/users/jsg/src/u-boot/tools/../common/image-fit.c:1569: error: for each function it appears in.)
> 

I seriously do not care if it's broken on OpenBSD if there is about one
user of such system. The problem is the original patch broke booting of
fitImage-wrapped kernels and this is serious breakage which makes
2016.09 release effectively useless.

I do support this revert and we will likely need v2016.09.1 unfortunately.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH] Revert "image-fit: switch ENOLINK to ENOENT"

[Jonathan Gray]

On Sun, Sep 18, 2016 at 03:14:30PM +0200, Marek Vasut wrote:
> On 09/17/2016 05:42 AM, Jonathan Gray wrote:
> > On Fri, Sep 16, 2016 at 10:21:43PM +0100, Paul Burton wrote:
> >> Commit bac17b78dace ("image-fit: switch ENOLINK to ENOENT") changed
> >> fit_get_node_from_config to return -ENOENT when a property doesn't
> >> exist, but didn't change any of its callers which check return values.
> >> Notably it didn't change boot_get_ramdisk, which leads to U-Boot failing
> >> to boot FIT images which don't include ramdisks with the following
> >> message:
> >>
> >>   Ramdisk image is corrupt or invalid
> >>
> >> The offending commit seems to dislike ENOLINK due to it not existing on
> >> OpenBSD, but I'm not sure why that matters as we define it in
> >> include/asm-generic/errno.h anyway so simply revert the commit to fix
> >> FIT image handling.
> > 
> > That header is not used when building native tools.
> > So reverting it will break the build of u-boot on OpenBSD.
> > 
> >   WRAP    tools/common/image-fit.c
> >   HOSTCC  tools/common/image-fit.o
> > In file included from tools/common/image-fit.c:1:
> > /usr/users/jsg/src/u-boot/tools/../common/image-fit.c: In function 'fit_get_node_from_config':
> > /usr/users/jsg/src/u-boot/tools/../common/image-fit.c:1569: error: 'ENOLINK' undeclared (first use in this function)
> > /usr/users/jsg/src/u-boot/tools/../common/image-fit.c:1569: error: (Each undeclared identifier is reported only once
> > /usr/users/jsg/src/u-boot/tools/../common/image-fit.c:1569: error: for each function it appears in.)
> > 
> 
> I seriously do not care if it's broken on OpenBSD if there is about one
> user of such system. The problem is the original patch broke booting of
> fitImage-wrapped kernels and this is serious breakage which makes
> 2016.09 release effectively useless.

So in your view uboot should remove support for hardware you don't have
and booting methods you don't use and not strive to remove the use of
non-portable interfaces?

Not having to deal with wrapped kernels with the efi interface present
in uboot >= 2016.05 and being able to have entry points for basic things
like character io and reading blocks off disks is what made uboot actually
useable for many people that previously had to deal with the largely
unmaintained api interface.

> 
> I do support this revert and we will likely need v2016.09.1 unfortunately.

[U-Boot] [PATCH] Revert "image-fit: switch ENOLINK to ENOENT"

[Tom Rini]

On Sun, Sep 18, 2016 at 03:14:30PM +0200, Marek Vasut wrote:
> On 09/17/2016 05:42 AM, Jonathan Gray wrote:
> > On Fri, Sep 16, 2016 at 10:21:43PM +0100, Paul Burton wrote:
> >> Commit bac17b78dace ("image-fit: switch ENOLINK to ENOENT") changed
> >> fit_get_node_from_config to return -ENOENT when a property doesn't
> >> exist, but didn't change any of its callers which check return values.
> >> Notably it didn't change boot_get_ramdisk, which leads to U-Boot failing
> >> to boot FIT images which don't include ramdisks with the following
> >> message:
> >>
> >>   Ramdisk image is corrupt or invalid
> >>
> >> The offending commit seems to dislike ENOLINK due to it not existing on
> >> OpenBSD, but I'm not sure why that matters as we define it in
> >> include/asm-generic/errno.h anyway so simply revert the commit to fix
> >> FIT image handling.
> > 
> > That header is not used when building native tools.
> > So reverting it will break the build of u-boot on OpenBSD.
> > 
> >   WRAP    tools/common/image-fit.c
> >   HOSTCC  tools/common/image-fit.o
> > In file included from tools/common/image-fit.c:1:
> > /usr/users/jsg/src/u-boot/tools/../common/image-fit.c: In function 'fit_get_node_from_config':
> > /usr/users/jsg/src/u-boot/tools/../common/image-fit.c:1569: error: 'ENOLINK' undeclared (first use in this function)
> > /usr/users/jsg/src/u-boot/tools/../common/image-fit.c:1569: error: (Each undeclared identifier is reported only once
> > /usr/users/jsg/src/u-boot/tools/../common/image-fit.c:1569: error: for each function it appears in.)
> > 
> 
> I seriously do not care if it's broken on OpenBSD if there is about one
> user of such system. The problem is the original patch broke booting of
> fitImage-wrapped kernels and this is serious breakage which makes
> 2016.09 release effectively useless.
> 
> I do support this revert and we will likely need v2016.09.1 unfortunately.

Lets take a step back.  It's my fault that things broke in the release
as I assumed that the tests I was automatically running to exercise FIT
stuff included a successful boot check.  They didn't, so this slipped
in.  I've posted an update to make sure that the FIT tests will cover
this kind of thing in the future.

And non-Linux support, both host and target, is something I see as a
valuable and important use case for U-Boot.  This includes more and less
public and visible and open and commercial things.

> 
> -- 
> Best regards,
> Marek Vasut

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160918/16b51fbe/attachment.sig>