diff mbox

[5/6] gnu: make-u-boot-package: Add files-to-install argument.

Message ID 20160926103447.31830-5-david@craven.ch
State New
Headers show

Commit Message

David Craven Sept. 26, 2016, 10:34 a.m. UTC
* gnu/packages/u-boot.scm (make-u-boot-package): Add files-to-install.
  (u-boot-vexpress, u-boot-malta, u-boot-beagle-bone-black): Use
  files-to-install.
---
 gnu/packages/u-boot.scm | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

Comments

Danny Milosavljevic Sept. 26, 2016, 12:39 p.m. UTC | #1
Hi David,

On Mon, 26 Sep 2016 12:34:46 +0200
David Craven <david@craven.ch> wrote:

>  (define-public u-boot-vexpress
> -  (make-u-boot-package "vexpress_ca9x4" "arm-linux-gnueabihf"))
> +  (make-u-boot-package "vexpress_ca9x4" "arm-linux-gnueabihf" '("u-boot.bin")))
>  
>  (define-public u-boot-malta
> -  (make-u-boot-package "malta" "mips64el-linux-gnuabi64"))
> +  (make-u-boot-package "malta" "mips64el-linux-gnuabi64" '("u-boot.bin")))
>  
>  (define-public u-boot-beagle-bone-black
> -  (make-u-boot-package "am335x_boneblack" "arm-linux-gnueabihf"))
> +  (make-u-boot-package "am335x_boneblack" "arm-linux-gnueabihf" '("MLO" "u-boot.img")))
                                                                   ^^^^
Ughh... are we sure we want to do that? I don't think a regular user would know what to put there. If the U-Boot build is from a clean slate it's not like there are stray files lying around or anything - we can just copy them all if we want.

Also, I don't think that we will have these define-public things for different systems in the long run. I posted a generated patch adding *all* of the supported U-Boot systems and Ludo vetoed it (and I agree by now - it's not necessary).

My preferred approach would be to allow the user to put the following into /etc/config.scm :

(bootloader (u-boot-configuration
              (device "/dev/mmcblk0")
              (package (make-u-boot-package "malta")))

(Nothing was left off)

What do you think?
David Craven Sept. 26, 2016, 1:38 p.m. UTC | #2
> What do you think?

I think there is a big difference here between supported boards and
unsupported boards. It doesn't make sense to add shortcuts for all
possible boards. Boards that are tested and supported should have
shortcuts.

I'd like cuirass to build native disk images for our supported boards
so that we can just write it to an sd card.

It would also be nice if we had a cross-compiled image for qemu. This
also would solve the guix system vm problem, since we can get a
cross-compiled substitute that will allow building a native image
which can then be started in qemu.

I'm planning on adding support for bbb, zybo and qemu, because that's
the hardware I've got.
David Craven Sept. 26, 2016, 1:41 p.m. UTC | #3
FYI: I got guixsd to boot, need a tty that works over serial so that I
can actually login...
Danny Milosavljevic Sept. 26, 2016, 2:04 p.m. UTC | #4
Hi David,

On Mon, 26 Sep 2016 15:41:12 +0200
David Craven <david@craven.ch> wrote:

> FYI: I got guixsd to boot, need a tty that works over serial so that I
> can actually login...

\o/ very nice :)

Do you mean client software? Then GNU screen can do that.

$ screen /dev/ttyUSB0 115200

Or do you mean terminal client hardware?

Then FTDI USB serial adapters can do that.

Or do you mean server software?

mingetty
David Craven Sept. 26, 2016, 2:07 p.m. UTC | #5
> Or do you mean server software?
>
> mingetty

mingetty doesn't support serial or does it? the man page doesn't
document a baud rate flag, and setting the tty to ttyS1 causes a crash
- not a tty. agetty or the getty from toybox have a flag to set the
baud rate.
Danny Milosavljevic Sept. 26, 2016, 10:28 p.m. UTC | #6
On Mon, 26 Sep 2016 16:07:16 +0200
David Craven <david@craven.ch> wrote:

> mingetty doesn't support serial or does it? the man page doesn't
> document a baud rate flag, and setting the tty to ttyS1 causes a crash
> - not a tty. agetty or the getty from toybox have a flag to set the
> baud rate.

Right.

Debian on LIME2 (Allwinner A20) does:

root       883     1  0 Aug24 ttyS0    00:00:00 /sbin/agetty --keep-baud 115200 38400 9600 ttyS0 vt102
David Craven Oct. 7, 2016, 8:19 a.m. UTC | #7
> Ughh... are we sure we want to do that? I don't think a
> regular user would know what to put there. If the U-Boot
> build is from a clean slate it's not like there are stray files
> lying around or anything - we can just copy them all if we
> want.

I thought about this patch and I think we should add it. The regex
solution results in a lot of garbage inside the output directory.
Regular users don't compile u-boot, and anyone with the skills to port
guixsd to a new board will know what to put. It also serves as
documentation, from reading the package definition I can tell what the
relevant files are without examining the output directory (which is
full of garbage - so I have to google it anyway).
Danny Milosavljevic Oct. 7, 2016, 9:26 a.m. UTC | #8
Hi David,

On Fri, 7 Oct 2016 10:19:42 +0200
David Craven <david@craven.ch> wrote:
> > Ughh... are we sure we want to do that? I don't think a
> > regular user would know what to put there. If the U-Boot
> > build is from a clean slate it's not like there are stray files
> > lying around or anything - we can just copy them all if we
> > want.  
> 
> I thought about this patch and I think we should add it. The regex
> solution results in a lot of garbage inside the output directory.
> Regular users don't compile u-boot, and anyone with the skills to port
> guixsd to a new board will know what to put. It also serves as
> documentation, from reading the package definition I can tell what the
> relevant files are without examining the output directory (which is
> full of garbage - so I have to google it anyway).

Yeah, if there are a lot of garbage files I also think that it's okay to do it like you propose. 

(It's just something that we have been struggling with in the ARM world - people think their board is the only one and then you have some custom configuration which only works for their board for no good reason. In this case the U-Boot binaries (contents) are rightfully board-specific. But there's no good reason that the names and splitting-up-into-files are (or the installation process - it's 2016, a computer should automate the installation). I'll mention it upstream)

I wonder how U-Boot itself finds out the target names - but that's for a future patch. (I think one could examine ALL-y to find out)

So I agree. LTGM!
diff mbox

Patch

diff --git a/gnu/packages/u-boot.scm b/gnu/packages/u-boot.scm
index cdd52d8..60fd48f 100644
--- a/gnu/packages/u-boot.scm
+++ b/gnu/packages/u-boot.scm
@@ -82,7 +82,7 @@  tree binary files. These are board description files used by Linux and BSD.")
 also initializes the boards (RAM etc).")
     (license license:gpl2+)))
 
-(define (make-u-boot-package board triplet)
+(define (make-u-boot-package board triplet files-to-install)
   "Returns a u-boot package for BOARD cross-compiled for TRIPLET."
   (package
     (inherit u-boot)
@@ -114,23 +114,20 @@  also initializes the boards (RAM etc).")
                        (closedir dir))
                      #f)))))
          (replace 'install
-           (lambda* (#:key outputs make-flags #:allow-other-keys)
-             (let* ((out (assoc-ref outputs "out"))
-                    (libexec (string-append out "/libexec"))
-                    (uboot-files (find-files "." ".*\\.(bin|efi|spl)$")))
-               (mkdir-p libexec)
-               (for-each
-                (lambda (file)
-                  (let ((target-file (string-append libexec "/" file)))
-                    (mkdir-p (dirname target-file))
-                    (copy-file file target-file)))
-                uboot-files)))))))))
+           (lambda* (#:key outputs #:allow-other-keys)
+             (for-each (lambda (file)
+                         (let* ((out (assoc-ref outputs "out"))
+                                (source-file (string-append (getcwd) "/" file))
+                                (target-file (string-append out "/" file)))
+                           (mkdir-p (dirname target-file))
+                           (copy-file source-file target-file)))
+                       '(,@files-to-install)))))))))
 
 (define-public u-boot-vexpress
-  (make-u-boot-package "vexpress_ca9x4" "arm-linux-gnueabihf"))
+  (make-u-boot-package "vexpress_ca9x4" "arm-linux-gnueabihf" '("u-boot.bin")))
 
 (define-public u-boot-malta
-  (make-u-boot-package "malta" "mips64el-linux-gnuabi64"))
+  (make-u-boot-package "malta" "mips64el-linux-gnuabi64" '("u-boot.bin")))
 
 (define-public u-boot-beagle-bone-black
-  (make-u-boot-package "am335x_boneblack" "arm-linux-gnueabihf"))
+  (make-u-boot-package "am335x_boneblack" "arm-linux-gnueabihf" '("MLO" "u-boot.img")))