GNU bug report logs - #31599
[PATCH] system: Add u-boot-puma-rk3399.

Previous Next

Full log

Message #14 received at 31599 <at> debbugs.gnu.org (full text, mbox):

From: Danny Milosavljevic <dannym <at> scratchpost.org>
To: Vagrant Cascadian <vagrant <at> debian.org>
Cc: 31599 <at> debbugs.gnu.org
Subject: Re: [bug#31599] [PATCH] system: Add u-boot-puma-rk3399.
Date: Sat, 26 May 2018 19:12:02 +0200

[Message part 1 (text/plain, inline)]

> A couple hours after I submitted this a patch was submitted upstream
> that fixes the issue:
> 
>   https://patchwork.ozlabs.org/patch/920785/

Good.

> >>+                 ;; The u-boot.itb is not built by default  
> >
> > ??? How can that be?  Isn't it required for booting?  
> 
> There may be other implementations of boot firmware that consume the
> various parts of u-boot and don't use u-boot.itb.
> 
> The Debian u-boot package, for example, does not yet have
> arm-trusted-frimware or the cortex-m0 firmware available, so it needs to
> build without it and let the user build the other components and
> manually construct the u-boot.itb.

I see.  OK, I'm fine with building u-boot.itb manually for now.  I just asked
because when we aren't careful we get into whack-a-mole quickly.

(There's a huge amount of ARM boards and we shouldn't do the u-boot maintainer's
job for them.  It's more work *and* eventually we'd diverge from each other)

> An earlier patch I did added the tools directory to PATH and used the
> in-tree mkimage, but I opted for using the mkimage from u-boot-tools
> when I submitted. Wasn't sure which was better.

Right now, they are the same because u-boot-tools derives from u-boot in Guix.

That said, for the sake of modularization I'd use the package-local mkimage
if there is one.  One reason is that there are weird u-boot forks for some
boards (mainline u-boot doesn't work for them).  These might have local
quirks in mkimage and it's better to track these quirks when building that
u-boot.

(As long as we know the above in the back of our heads as a potential source
of problems it doesn't matter much which mkimage we use in practice)

u-boot-tools is meant for the end user.  I didn't intend it to be used
by the u-boots themselves (not that it's bad--but it's redundant right now).

> >>(add-after 'unpack 'set-environment
> >>+               (lambda* (#:key inputs #:allow-other-keys)
> >>+                 ;; Need to copy the firmware into u-boot build
> >>+                 ;; directory.
> >>+                 (copy-file (string-append (assoc-ref inputs "firmware")
> >>+                                           "/bl31.bin") "bl31-rk3399.bin")
> >>+                 (copy-file (string-append (assoc-ref inputs "firmware-m0")
> >>+                                           "/rk3399m0.bin") "rk3399m0.bin")))  
> >
> > Please end this phase with "#t".  
> Please forgive my ignorance, but I'm not sure where it belongs or even
> why... just a rank beginner with this. :)

Earlier guile didn't support exceptions--so phases have to return a boolean
(whether the phase succeeded or not).

In a lambda, the return value is the value of the last expression.

There's an effort in core-updates to get it all up-to-speed so that these
all actually return a boolean--and long term we'd get rid of this requirement.

Some procedures are written especially for phases ("invoke" is, for example)
and do what you'd expect (return #t if OK, otherwise raise exception).

Other procedures you have to be careful what they actually return.

In this case, copy-file's documentation[1] states "The return value is
unspecified" (that implies that error handling is via exceptions).

The unspecified return value is clearly not good.

[1] https://www.gnu.org/software/guile/manual/html_node/File-System.html

[Message part 2 (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.