[PATCH] gnu: Add dtc.

[PATCH] gnu: Add dtc.

[David Craven]

From: Danny Milosavljevic <dannym@scratchpost.org>

* gnu/packages/u-boot.scm (dtc): New variable.

Co-authored-by: David Craven <david@craven.ch>
---
 gnu/packages/u-boot.scm | 60 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 gnu/packages/u-boot.scm

diff --git a/gnu/packages/u-boot.scm b/gnu/packages/u-boot.scm
new file mode 100644
index 0000000..e9376a4
--- /dev/null
+++ b/gnu/packages/u-boot.scm
@@ -0,0 +1,60 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2016 Danny Milosavljevic <dannym@scratchpost.org>
+;;; Copyright © 2016 David Craven <david@craven.ch>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu packages u-boot)
+  #:use-module (guix build-system gnu)
+  #:use-module (guix download)
+  #:use-module (guix packages)
+  #:use-module ((guix licenses) #:prefix license:)
+  #:use-module (gnu packages bison)
+  #:use-module (gnu packages flex))
+
+(define-public dtc
+  (package
+    (name "dtc")
+    (version "1.4.1")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append
+                    "https://www.kernel.org/pub/software/utils/dtc/"
+                    "dtc-" version ".tar.xz"))
+              (sha256
+               (base32
+                "155v52palf5fwfcnq696s41whjk0a5dqx98b7maqzdn7xbc2m6bp"))))
+    (build-system gnu-build-system)
+    (native-inputs
+     `(("bison" ,bison)
+       ("flex" ,flex)))
+    (arguments
+     `(#:make-flags
+       (list "CC=gcc" (string-append "PREFIX=" (assoc-ref %outputs "out")))
+       #:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'patch-paths
+           (lambda _
+             (substitute* "Makefile"
+               (("/usr/bin/install") "install"))
+             (substitute* "Makefile"
+               (("PREFIX = \\$\\(HOME\\)") ""))))
+         (delete 'configure))))
+    (home-page "https://www.devicetree.org")
+    (synopsis "Compiles Device Tree Source Files (.dts)")
+    (description "dtc compiles Device Tree Source Files to Device Tree Binary Files.
+These are hardware (board) description files (used by Linux and BSD).")
+    (license license:gpl2+)))
-- 
2.9.0

Re: [PATCH] gnu: Add dtc.

[David Craven]

I made some stylistic changes, updated the license, updated to 1.4.1,
removed the patches (checked the arch package and the nixos package,
both don't patch dtc)

I forgot to add u-boot.scm to local.mk...

Danny, WDYT? Is it an improvement?

Re: [PATCH] gnu: Add dtc.

[Danny Milosavljevic]

On Mon, 29 Aug 2016 16:53:10 +0200
David Craven <david@craven.ch> wrote:

> I made some stylistic changes, updated the license, updated to 1.4.1,
> removed the patches (checked the arch package and the nixos package,
> both don't patch dtc)
> 
> I forgot to add u-boot.scm to local.mk...
> 
> Danny, WDYT? Is it an improvement?

Back then, I cut it down to two patches only: one to build the documentation (just nice to have) and one to add "fdt_first_subnode" and "fdt_next_subnode" to the exports so you can actually traverse device trees yourself. That's necessary because they use a version script that defaults everything to local (exports nothing but their listed symbols).

While I almost always look up documentation online, the latter patch is kinda important. Debian patches it still.

So I'd include these two patches only. I'm not sure why they don't make a new release with this fix.

The name "device-tree-compiler" was also from Debian - they renamed it from "dtc" due to naming conflict. There's also a dtc bundled with the Linux kernel. So I'm fine with either name.

[PATCH] gnu: Add u-boot.

[David Craven]

From: Danny Milosavljevic <dannym@scratchpost.org>

* gnu/packages/u-boot.scm (u-boot, make-u-boot-package,
  u-boot-vexpress_ca9x4, u-boot-malta armhf-linux-uboot,
  mips64el-linux-uboot): New variables.

Co-authored-by: David Craven <david@craven.ch>
---
 gnu/packages/u-boot.scm | 83 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 82 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/u-boot.scm b/gnu/packages/u-boot.scm
index e9376a4..fd69bc1 100644
--- a/gnu/packages/u-boot.scm
+++ b/gnu/packages/u-boot.scm
@@ -22,8 +22,12 @@
   #:use-module (guix download)
   #:use-module (guix packages)
   #:use-module ((guix licenses) #:prefix license:)
+  #:use-module (gnu packages)
+  #:use-module ((gnu packages algebra) #:select (bc))
   #:use-module (gnu packages bison)
-  #:use-module (gnu packages flex))
+  #:use-module (gnu packages cross-base)
+  #:use-module (gnu packages flex)
+  #:use-module (gnu packages python))
 
 (define-public dtc
   (package
@@ -58,3 +62,80 @@
     (description "dtc compiles Device Tree Source Files to Device Tree Binary Files.
 These are hardware (board) description files (used by Linux and BSD).")
     (license license:gpl2+)))
+
+(define u-boot
+  (package
+    (name "u-boot")
+    (version "2016.07")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append
+                    "ftp://ftp.denx.de/pub/u-boot/"
+                    "u-boot-" version ".tar.bz2"))
+              (sha256
+               (base32
+                "0lqj4ckmfqiap8mc6z2d5albs3g2h5mzccbn60hsgxhabhibfkwp"))))
+    (native-inputs
+     `(("bc" ,bc)
+       ("dtc" ,dtc)
+       ("python-2" ,python-2)))
+    (build-system  gnu-build-system)
+    (home-page "http://www.denx.de/wiki/U-Boot/")
+    (synopsis "ARM Universal Bootloader")
+    (description "U-Boot is an universal bootloader mostly used for ARM boards.
+It also initializes the boards (RAM etc).")
+    (license license:gpl2+)))
+
+(define (make-u-boot-package board triplet xgcc)
+  (package
+    (inherit u-boot)
+    (name (string-append "u-boot-" (string-downcase board)))
+    (native-inputs
+     `(("cross-gcc" ,xgcc)
+       ("cross-binutils" ,(cross-binutils triplet))
+       ,@(package-native-inputs u-boot)))
+    (arguments
+     `(#:test-target "test"
+       #:make-flags
+       (list "HOSTCC=gcc" (string-append "CROSS_COMPILE=" ,triplet "-"))
+       #:phases
+       (modify-phases %standard-phases
+         (replace 'configure
+           (lambda* (#:key outputs make-flags #:allow-other-keys)
+             (let ((configname (string-append ,board "_defconfig")))
+               (if (file-exists? (string-append "configs/" configname))
+                   (zero? (apply system* "make" `(,@make-flags ,configname)))
+                   (begin
+                     (display "Invalid boardname. Valid boardnames would have been:")
+                     (newline)
+                     (system* "ls" "-1" "configs")
+                     #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)$")))
+               (mkdir-p libexec)
+               (for-each
+                (lambda (file-path)
+                  (let ((target-file-path (string-append libexec "/" file-path)))
+                    (mkdir-p (dirname target-file-path))
+                    (copy-file file-path target-file-path)))
+                uboot-files)))))))))
+
+(define-public u-boot-vexpress_ca9x4
+  (make-u-boot-package "vexpress_ca9x4" "arm-linux-gnueabihf" xgcc-armhf))
+
+(define-public u-boot-malta
+  (make-u-boot-package "malta" "mips64el-linux-gnuabi64" xgcc-mips64el))
+
+(define-public armhf-linux-uboot
+  u-boot-vexpress_ca9x4)
+
+(define-public mips64el-linux-uboot
+  u-boot-malta)
+
+;; Something should:
+;; - create extlinux.conf and put it on the first bootable partition
+;;   (the one with the Active flag)
+;; - install the u-boot bootloader
-- 
2.9.0

Re: [PATCH] gnu: Add u-boot.

[Danny Milosavljevic]

> +(define (make-u-boot-package board triplet xgcc)
...
> +                   (begin
> +                     (display "Invalid boardname. Valid boardnames would have been:")
> +                     (newline)
> +                     (system* "ls" "-1" "configs")
> +                     #f)))))

I've improved the error message if the board name is unknown:

       #:phases
        (modify-phases %standard-phases
         (replace
          'configure
           (lambda* (#:key outputs make-flags #:allow-other-keys)
             (let ((configname (string-append ,board "_defconfig")))
               (if (zero? (system* "ls" (string-append "configs/" configname)))
                  (zero? (apply system* "make" `(,@make-flags ,configname)))
                  (begin
                    (use-modules (ice-9 match) (ice-9 format))
                    (display "Invalid boardname. Valid boardnames would have been:")
                    (newline)
                    (let ((dir (opendir "configs")))
                      (do ((filename (readdir dir) (readdir dir)))
                          ((eof-object? filename))
                        (if (string-suffix? "_defconfig" filename)
                          (format #t
                                  "- ~A\n"
                                  (string-drop-right filename
                                                     (string-length "_defconfig")))))
                      (closedir dir))
                    #f)))))
         (replace 'install
           (lambda* (#:key outputs make-flags #:allow-other-keys)
             (let ((out (string-append (assoc-ref outputs "out") "/libexec")))
               (mkdir-p out)
               (for-each (lambda (name)
                           (let ((outname (string-append out "/" name)))
                             (mkdir-p (dirname outname))
                             (copy-file name outname)))
                         (find-files "." ".*\\.(bin|efi)$"))))))))))

Re: [PATCH] gnu: Add u-boot.

[Danny Milosavljevic]

On Mon, 29 Aug 2016 20:27:52 +0200
Danny Milosavljevic <dannym@scratchpost.org> wrote:

>                          (find-files "." ".*\\.(bin|efi)$"))))))))))

Also in order to support Novena it should be 

                          (find-files "." ".*\\.(bin|efi|spl)$"))))))))))

Re: [PATCH] gnu: Add u-boot.

[Ludovic Courtès]

David Craven <david@craven.ch> skribis:

> From: Danny Milosavljevic <dannym@scratchpost.org>
>
> * gnu/packages/u-boot.scm (u-boot, make-u-boot-package,
>   u-boot-vexpress_ca9x4, u-boot-malta armhf-linux-uboot,
>   mips64el-linux-uboot): New variables.
>
> Co-authored-by: David Craven <david@craven.ch>

Cool, thanks for picking it up!

Overall this LGTM.  I guess Danny’s comments should be taken into
account.  Other than that, I only have cosmetic suggestions:

> +    (synopsis "ARM Universal Bootloader")

“ARM bootloader” (or “ARM universal bootloader” if you insist).

> +    (description "U-Boot is an universal bootloader mostly used for ARM boards.
> +It also initializes the boards (RAM etc).")

I also think “universal” can be removed.

> +(define (make-u-boot-package board triplet xgcc)

A docstring would be nice here.

I think you could remove the ‘xgcc’ parameter and simply do:

  (let ((xgcc (cross-gcc triplet)))
    (package
       …))

> +           (lambda* (#:key outputs make-flags #:allow-other-keys)
> +             (let ((configname (string-append ,board "_defconfig")))

Should be ‘config-name’ per our conventions, but ‘config’ is probably
enough.

> +               (if (file-exists? (string-append "configs/" configname))
> +                   (zero? (apply system* "make" `(,@make-flags ,configname)))
> +                   (begin
> +                     (display "Invalid boardname. Valid boardnames would have been:")
> +                     (newline)
> +                     (system* "ls" "-1" "configs")
> +                     #f)))))

“board name” (two words).

> +         (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)$")))
> +               (mkdir-p libexec)
> +               (for-each
> +                (lambda (file-path)
> +                  (let ((target-file-path (string-append libexec "/" file-path)))
> +                    (mkdir-p (dirname target-file-path))
> +                    (copy-file file-path target-file-path)))
> +                uboot-files)))))))))

s/-path//                

> +
> +(define-public u-boot-vexpress_ca9x4
> +  (make-u-boot-package "vexpress_ca9x4" "arm-linux-gnueabihf" xgcc-armhf))
> +
> +(define-public u-boot-malta
> +  (make-u-boot-package "malta" "mips64el-linux-gnuabi64" xgcc-mips64el))
> +
> +(define-public armhf-linux-uboot
> +  u-boot-vexpress_ca9x4)
> +
> +(define-public mips64el-linux-uboot
> +  u-boot-malta)

I think these two aliases can be removed, no?

Thanks!

Ludo’.

Re: [PATCH] gnu: Add u-boot.

[Danny Milosavljevic]

Hi Ludo,

On Wed, 31 Aug 2016 22:40:57 +0200
ludo@gnu.org (Ludovic Courtès) wrote:

> > +           (lambda* (#:key outputs make-flags #:allow-other-keys)
> > +             (let ((configname (string-append ,board "_defconfig")))  
> 
> Should be ‘config-name’ per our conventions, but ‘config’ is probably
> enough.
> 
> > +               (if (file-exists? (string-append "configs/" configname))
> > +                   (zero? (apply system* "make" `(,@make-flags ,configname)))
> > +                   (begin
> > +                     (display "Invalid boardname. Valid boardnames would have been:")
>
> “board name” (two words).

> > +                    (copy-file file-path target-file-path)))
> > +                uboot-files)))))))))  
> 
> s/-path//                

For the record, a filename (or file path) is something completely different from a file. It makes no sense to call a filename "file". Likewise, a boardname is the name of a board. It's not the board. A configname is the name of a config [file]. "config" would be the configuration itself.

To develop this habit has taken a long time for me and it has paid off well.

I'm not totally against naming filenames "file" - or "f" for that matter - but I distinguish for good reason.

Also when adapting gnu/system.scm etc for U-Boot I spent lots of time finding out what are confignames and what are configurations because it likewise does not distinguish.

A rose by any other name would smell as sweet, you know :)

Re: [PATCH] gnu: Add u-boot.

[David Craven]

> I'm not totally against naming filenames "file" - or "f" for that matter - but I distinguish for good reason.

I believe I renamed your variables... xD

(replace 'install
+           (lambda* (#:key outputs make-flags #:allow-other-keys)
+             (let ((out (string-append (assoc-ref outputs "out") "/libexec")))
+               (mkdir-p out)
+               (for-each (lambda (name)
+                           (let ((outname (string-append out "/" name)))
+                             (mkdir-p (dirname outname))
+                             (copy-file name outname)))
+                         (find-files "." ".*\\.(bin|efi)$"))))))))))

Re: [PATCH] gnu: Add u-boot.

[Ludovic Courtès]

Hi Danny!

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> On Wed, 31 Aug 2016 22:40:57 +0200
> ludo@gnu.org (Ludovic Courtès) wrote:
>
>> > +           (lambda* (#:key outputs make-flags #:allow-other-keys)
>> > +             (let ((configname (string-append ,board "_defconfig")))  
>> 
>> Should be ‘config-name’ per our conventions, but ‘config’ is probably
>> enough.
>> 
>> > +               (if (file-exists? (string-append "configs/" configname))
>> > +                   (zero? (apply system* "make" `(,@make-flags ,configname)))
>> > +                   (begin
>> > +                     (display "Invalid boardname. Valid boardnames would have been:")
>>
>> “board name” (two words).
>
>> > +                    (copy-file file-path target-file-path)))
>> > +                uboot-files)))))))))  
>> 
>> s/-path//                
>
> For the record, a filename (or file path) is something completely different from a file. It makes no sense to call a filename "file". Likewise, a boardname is the name of a board. It's not the board. A configname is the name of a config [file]. "config" would be the configuration itself.
>
> To develop this habit has taken a long time for me and it has paid off well.

I guess it’s just a matter of convention, so no argument here.  :-)

GNU’s convention is to call “file name” the name of a file (not
“filepath”, not “path”, not “filename”); in Scheme, that would give
“file-name” as the variable name, which I often shorten to “file”
because there are no files in Scheme, only I/O ports.

Buy yeah, I’m nitpicking, or maybe even “bikeshedding”.  :-)

Ludo’.

Re: [PATCH] gnu: Add dtc.

[Ludovic Courtès]

David Craven <david@craven.ch> skribis:

> From: Danny Milosavljevic <dannym@scratchpost.org>
>
> * gnu/packages/u-boot.scm (dtc): New variable.
>
> Co-authored-by: David Craven <david@craven.ch>

[...]

> +    (synopsis "Compiles Device Tree Source Files (.dts)")

Maybe: “Compile device tree source files”.

> +    (description "dtc compiles Device Tree Source Files to Device Tree Binary Files.

And: “@command{dtc} compiles device tree source (DTS) files to device
tree binary (DTB) files. …”

“dtc” appears to be the upstream name (on kernel.org), so we should
stick to it.

I’ll let Danny and you figure out whether anything else needs to be
done.  :-)

Thank you!

Ludo’.