[bug#64149] [PATCH v2] bug#64149: WIP update u-boot to 2023.07-rc6

[Vagrant Cascadian <vagrant@debian.org>]

[-- Attachment #1: Type: text/plain, Size: 5003 bytes --]

On 2023-07-08, Vagrant Cascadian wrote:
> On 2023-07-08, Maxim Cournoyer wrote:
>> Vagrant Cascadian <vagrant@debian.org> writes:
>> [...]
>>> The one thing I would probably prefer is to split this into one package
>>> per line, but I tried to aim for a smaller diff:
>>>
>>> -       (prepend python-coverage python-pycryptodomex python-pytest sdl2)))
>>> +       (prepend python-coverage python-filelock python-pycryptodomex
>>> +python-pytest python-pytest-xdist sdl2)))
>>
>> Odd indentation; please use something like:
>>
>>           (prepend package1
>>                    package2
>>                    ...)
>
> Yup, that is the aproach I would propose for the next and/or final
> patch(es)!

Included in the upcoming patch series.

>>> * gnu/packages/patches/u-boot-infodocs-target.patch: Remove file.
>>> * gnu/packages/patches/u-boot-patman-guix-integration.patch: Remove
>>> file.
>>
>> Nitpick: I'd use "Delete" here instead of "Remove".
>
> Not my style, but not strongly opinionated either. :)
>
>>> * gnu/local.mk: Remove patches.
>>
>> Nitpick: I'd use "De-register" instead of remove.
>
> Even less my style, but also not strongly opinionated. :)

And also included.

>>> @@ -778,6 +778,9 @@ (define-public u-boot-tools
>>>                             ;; details.
>>>                             (("CONFIG_FIT_SIGNATURE=y")
>>>                              "CONFIG_FIT_SIGNATURE=n\nCONFIG_UT_LIB_ASN1=n\nCONFIG_TOOLS_LIBCRYPTO=n")
>>> +                           ;; Catch instances of implied CONFIG_FIG_SIGNATURE with VPL targets
>>> +                           (("CONFIG_SANDBOX_VPL=y")
>>> +                            "CONFIG_SANDBOX_VPL=y\nCONFIG_FIT_SIGNATURE=n\nCONFIG_VPL_FIT_SIGNATURE=n\nCONFIG_TOOLS_LIBCRYPTO=n")
>>
>> I know it's already busted on the line above, but we can format this in
>> a more readable way by using something like:
>>                               "\
>> CONFIG1=y
>> CONFIG2=n
>> ...\n"

I did something somewhat like that, in a way that did not require using
\n at all, for both the CONFIG_FIT_SIGNATURE and CONFIG_SANDBOX_VPL
lines.


>>> @@ -1009,6 +1012,8 @@ (define*-public (make-u-boot-sunxi64-package board triplet
>>>            #~(modify-phases #$phases
>>>                (add-after 'unpack 'set-environment
>>>                  (lambda* (#:key native-inputs inputs #:allow-other-keys)
>>> +                  ;; Avoid dependency on crust-firmware https://issues.guix.gnu.org/48371
>>
>> Trick to avoid busting the 80 characters per line limit: for links, you
>> can do (see: $link), which typically gets split like:
>>
>>                      ;; blablabla (see:
>>                      ;; https://...)
>>
>>> +                  (setenv "SCP" "/dev/null")
>
> Yeah, that sounds good...

Oops, forgot that... because...

> Although, now that "Add crust firmware for sunxi devices"
> (https://issues.guix.gnu.org/48371) finally got merged, we will have to
> fix this properly. :)

I made this work. :)


>>> @@ -1230,7 +1248,8 @@ (define-public u-boot-rockpro64-rk3399
>>>                                                 "CONFIG_SATA_SIL=y"
>>>                                                 "CONFIG_SCSI=y"
>>>                                                 "CONFIG_SCSI_AHCI=y"
>>> -                                               "CONFIG_DM_SCSI=y"))))
>>> +                                               "CONFIG_DM_SCSI=y"
>>> +                                               "# CONFIG_SPL_FIT_SIGNATURE is not set"))))
>>>      (package
>>>        (inherit base)
>>>        (arguments
>>> @@ -1240,6 +1259,13 @@ (define-public u-boot-rockpro64-rk3399
>>>                (add-after 'unpack 'set-environment
>>>                  (lambda* (#:key inputs #:allow-other-keys)
>>>                    (setenv "BL31" (search-input-file inputs "/bl31.elf"))))
>>> +              ;; Disable SPL FIT signatures, due to GPLv2 and Openssl license
>>> +              ;; incompatibilities
>>> +              (add-after 'unpack 'disable-spl-fit-signature
>>> +                (lambda _
>>> +                  (substitute* "configs/rockpro64-rk3399_defconfig"
>>> +                    (("CONFIG_SPL_FIT_SIGNATURE=y")
>>> +                     "# CONFIG_SPL_FIT_SIGNATURE is not set"))))
>>
>> Is this really needed, given we use "# CONFIG_SPL_FIT_SIGNATURE is not
>> set" in #:configs above?
>
> Only having it in #:configs resulted in a build failure (e.g. there were
> conflicting entries or something). Having it in both places seems better
> as it ensures it does not accidentally get enabled somehow. But we
> probably could drop the part in #:configs if we wanted ... or re-write
> how #:configs works, though that would be more than I want to get into
> right now! :)

I stuck with the above rationale and left it as it was...

I've included the patch updating arm-trusted-firmware to 2.9 from
https://issues.guix.gnu.org/64142 and the patches submitted regarding
patman, although as mentioned patman still needs some updates.


live well,
  vagrant

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]