diff mbox series

[bug#70892,4/6] gnu: Add u-boot-starfive-visionfive2.

Message ID 7c3b72a45a1c5d6adfb6ccd586ab68d587cfc420.1715508730.git.zhengjunjie@iscas.ac.cn
State New
Headers show
Series Add visionfive2 support. | expand

Commit Message

Zheng Junjie May 12, 2024, 11:09 a.m. UTC
* gnu/packages/bootloaders.scm (u-boot-starfive-visionfive2): New variable.

Change-Id: I2a7d2275b1a0f4253ffc3152c892687aae11a471
---
 gnu/packages/bootloaders.scm | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

Comments

Vagrant Cascadian May 15, 2024, 5:59 p.m. UTC | #1
On 2024-05-12, Zheng Junjie wrote:
> diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
> index cfe8046731..1d52e961fd 100644
> --- a/gnu/packages/bootloaders.scm
> +++ b/gnu/packages/bootloaders.scm
...
> @@ -1343,6 +1344,36 @@ (define-public u-boot-sifive-unmatched
>         (modify-inputs (package-inputs base)
>           (append opensbi-generic))))))
>  
> +(define-public u-boot-starfive-visionfive2
> +  (let ((opensbi (package
> +                   (inherit opensbi-generic)
> +                   (arguments
> +                    (substitute-keyword-arguments
> +                        (package-arguments opensbi-generic)
> +                      ((#:make-flags flags)
> +                       `(cons* "FW_TEXT_START=0x40000000"
> +                               "FW_OPTIONS=0"
> +                               ,flags))))))
> +        (base (make-u-boot-package "starfive_visionfive2" "riscv64-linux-gnu")))

I would not want to block this patch on this, but...

Curious about the advantages and disadvantages of making this an
on-the-fly opensbi package variant... as so far I think most u-boot
packages just pull in inputs of other packages
(e.g. arm-trusted-firmware-*) rather than modifying them as part of the
u-boot-* package.

If this seems to be a good approach overall, maybe we should switch more
packages to use this approach ... or if there are significant downsides,
perhaps this patch series should just create another opensbi variant and
add it to inputs or whatever?

Thoughts?


That aside, looks good to me. :)

Thanks!

live well,
  vagrant
Zheng Junjie May 18, 2024, 5:58 p.m. UTC | #2
Vagrant Cascadian <vagrant@debian.org> writes:

> On 2024-05-12, Zheng Junjie wrote:
>> diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
>> index cfe8046731..1d52e961fd 100644
>> --- a/gnu/packages/bootloaders.scm
>> +++ b/gnu/packages/bootloaders.scm
> ...
>> @@ -1343,6 +1344,36 @@ (define-public u-boot-sifive-unmatched
>>         (modify-inputs (package-inputs base)
>>           (append opensbi-generic))))))
>>  
>> +(define-public u-boot-starfive-visionfive2
>> +  (let ((opensbi (package
>> +                   (inherit opensbi-generic)
>> +                   (arguments
>> +                    (substitute-keyword-arguments
>> +                        (package-arguments opensbi-generic)
>> +                      ((#:make-flags flags)
>> +                       `(cons* "FW_TEXT_START=0x40000000"
>> +                               "FW_OPTIONS=0"
>> +                               ,flags))))))
>> +        (base (make-u-boot-package "starfive_visionfive2" "riscv64-linux-gnu")))
>
> I would not want to block this patch on this, but...
>
> Curious about the advantages and disadvantages of making this an
> on-the-fly opensbi package variant... as so far I think most u-boot
> packages just pull in inputs of other packages
> (e.g. arm-trusted-firmware-*) rather than modifying them as part of the
> u-boot-* package.

As far as I know, the opensbi support in visionfive2 was added later, so
the opensbi configuration needs to be modified, and later other boards
should be able to use the generic version of opensbi.

>
> If this seems to be a good approach overall, maybe we should switch more
> packages to use this approach ... or if there are significant downsides,
> perhaps this patch series should just create another opensbi variant and
> add it to inputs or whatever?


The disadvantage of this is that cuirass can't see on-the-fly opensbi,
and if the hidden input takes a long time to compile, it will affect the
compilation time of the package. So I split opensbi-for-visionfive2 in
the v2 patchset.

>
> Thoughts?
>
>
> That aside, looks good to me. :)
>
> Thanks!
>
> live well,
>   vagrant
diff mbox series

Patch

diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
index cfe8046731..1d52e961fd 100644
--- a/gnu/packages/bootloaders.scm
+++ b/gnu/packages/bootloaders.scm
@@ -19,6 +19,7 @@ 
 ;;; Copyright © 2021 Stefan <stefan-guix@vodafonemail.de>
 ;;; Copyright © 2022, 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;; Copyright © 2023 Herman Rimm <herman@rimm.ee>
+;;; Copyright © 2024 Zheng Junjie <873216071@qq.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1343,6 +1344,36 @@  (define-public u-boot-sifive-unmatched
        (modify-inputs (package-inputs base)
          (append opensbi-generic))))))
 
+(define-public u-boot-starfive-visionfive2
+  (let ((opensbi (package
+                   (inherit opensbi-generic)
+                   (arguments
+                    (substitute-keyword-arguments
+                        (package-arguments opensbi-generic)
+                      ((#:make-flags flags)
+                       `(cons* "FW_TEXT_START=0x40000000"
+                               "FW_OPTIONS=0"
+                               ,flags))))))
+        (base (make-u-boot-package "starfive_visionfive2" "riscv64-linux-gnu")))
+    (package
+      (inherit base)
+      (arguments
+       (substitute-keyword-arguments (package-arguments base)
+         ((#:phases phases)
+          #~(modify-phases #$phases
+              (add-after 'unpack 'set-environment
+                (lambda* (#:key inputs #:allow-other-keys)
+                  (setenv "OPENSBI" (search-input-file inputs
+                                                       "fw_dynamic.bin"))))
+              (add-after 'install 'install-u-boot-spl.bin.normal.out
+                (lambda _
+                  (install-file "spl/u-boot-spl.bin.normal.out"
+                                (string-append #$output
+                                               "/libexec/spl"))))))))
+      (inputs
+       (modify-inputs (package-inputs base)
+         (append opensbi))))))
+
 (define-public u-boot-rock64-rk3328
   (let ((base (make-u-boot-package "rock64-rk3328" "aarch64-linux-gnu")))
     (package