GNU bug report logs -
#60224
[PATCH 0/9] Improvements to our u-boot tooling
Previous Next
Full log
View this message in rfc822 format
Hi Ricardo!
Ricardo Wurmus <rekado <at> elephly.net> writes:
> Hi Maxim,
>
> this looks reasonable to me. Some comments below.
Sorry for the late reply, it hadn't reached my INBOX (please keep me in
CC to ensure it does :-)).
> A minor comment about the first patch: you still bind “outputs” in the
> build phases, but since you’re using #$output anyway this value is never
> used.
Fixed!
> [PATCH 3/9] introduces a comment in the definition of “native-build?”,
> which references %current-target-system, yet only %current-system is
> used. Is this a mistake?
Fixed!
> [PATCH 4/9] — This one appends arm-trusted-firmware-rk3399 instead of
> prepending it. This differs from how it was done with the labeled
> inputs. Does this have any consequences? Is the “firmware” label used
> anywhere (such as downstream packages)? The same applies to patches
> 5/9, 7/9, and 8/9.
I don't think it matters; the base u-boot package which gets used
doesn't include any "firmware" input, and the file provided via
arm-trusted-firmware-rk3399 is searched via "search-input-file". I've
grepped for 'assoc-ref.*"firmware"' and there doesn't seem to be any
remnants except for u-boot-rockpro64-rk3399, which I've now fixed in the
last commit.
> [PATCH 6/9] — The change from .bin to .elf confuses me. Is this due to the
> fact that “target” is now actually set and the package build thus
> behaves differently?
I think so. I was puzzled by it too, especially since some packages
already were searching for a .elf file rather than a .bin file.
> [PATCH 8/9] removes a reference to “firware”; this answers my question
> to patch 4/9, but perhaps other such references remain?
Answered above.
Thanks for the review! v3 will appear shortly.
--
Thanks,
Maxim
This bug report was last modified 2 years and 182 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.