Page MenuHomeFreeBSD

sysutils/u-boot-master, -rpi2, -rpi3: Update to v2018.07
ClosedPublic

Authored by manu on Jul 12 2018, 10:00 AM.

Details

Summary

This add:

  • Explicit Depency on bison
  • Remove the DTC= on MAKE_ARGS as we use the BINARY_ALIAS already
  • metadata variables
  • Add cache cmds and disable secure boot
  • All RPI uses some special fragments to enable OF_BOARD. This makes u-boot using the DTB loaded by the firmware instead of using it's own.
  • For patches accepted but not in mainline due to timing issue use patchwork directly instead of putting files in the port
  • All armv7 boards are EFI enabled and boot in secure mode

More on metadata file :

The goal is to have a u-boot-install package that will read those and auto
install u-boot either on a dir (method copy) or on a device (method raw)
As all the u-boots are either one file to be dd'ed or one/multiple files
to be copied this simple method is good enough.
Since the maximum variable is 4 (for the raw method), simply use echo instead
of a .in files (ok bapt@)

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

manu created this revision.Jul 12 2018, 10:00 AM
linimon retitled this revision from u-boot: Update to v2018.07 to sysutils/u-boot-master, -rpi2, -rpi3: Update to v2018.07.Jul 12 2018, 10:49 AM
linimon requested changes to this revision.Jul 12 2018, 11:21 AM
linimon added inline comments.
sysutils/u-boot-master/Makefile
24 ↗(On Diff #45198)

adding BISON to USES is the preferred style.

161 ↗(On Diff #45198)

Simplification: move this line down one and change it to CONFIG_FRAGMENT=${FILESDIR}/${FRAGMENT_NAME}

162 ↗(On Diff #45198)

To accomodate the u-boot-rpi-0-w port, doesn't this need to be FRAGMENT_NAME?=

192 ↗(On Diff #45198)

by adding the following at line 135, I think these 3 lines of change can be dropped:

UBOOT_PLIST+=metadata

This revision now requires changes to proceed.Jul 12 2018, 11:21 AM
manu added inline comments.Jul 12 2018, 12:33 PM
sysutils/u-boot-master/Makefile
24 ↗(On Diff #45198)

Instead of adding it to BUILD_DEPENDS you mean ?

161 ↗(On Diff #45198)

We don't use Fragment on aarch64

162 ↗(On Diff #45198)

No because of the !defined(CONFIG_FRAGMENT)

192 ↗(On Diff #45198)

We don't have metadata for all flavors.
But in the end it should be like that yes.

mat added a comment.Jul 12 2018, 12:43 PM

Could you use devel/arcanist, or at least generate a diff with full context like it does, with svn diff -x -U9999 or git diff -U9999.

sysutils/u-boot-master/Makefile
25 ↗(On Diff #45198)

I don't get why bison is here, there is no bison in the base system.

38 ↗(On Diff #45198)

As you are adding all the patch with PATCHFILES with :-p1, you can remove this.

linimon added inline comments.Jul 12 2018, 1:00 PM
sysutils/u-boot-master/Makefile
24 ↗(On Diff #45198)

Yes.

158 ↗(On Diff #45198)

this line can remain unchanged with the below patch.

161 ↗(On Diff #45198)

this line can go away with the below patch.

161 ↗(On Diff #45198)

You have already screened out the case of aarch64 by having .if ${UBOOT_ARCH} == arm, correct?

163 ↗(On Diff #45198)

this line can go away with the below patch.

165 ↗(On Diff #45198)

change this to .if defined(FRAGMENT_NAME) and ...

167 ↗(On Diff #45198)

... change this line to
+ ${CP} ${.CURDIR}/files/${FRAGMENT_NAME} ${WRKSRC}/configs/

Then all the uses of CONFIG_FRAGMENT can go away.

192 ↗(On Diff #45198)

But because of the conditional in line 132, you are already checking for whether the port has the metadata file.

manu added a comment.Jul 12 2018, 1:09 PM
In D16241#344503, @mat wrote:

Could you use devel/arcanist, or at least generate a diff with full context like it does, with svn diff -x -U9999 or git diff -U9999.

Yeah I forgot to add context, sorry.
arcanist sucks when having multiple commits on the same branch to create multiple review.

manu added inline comments.Jul 12 2018, 1:14 PM
sysutils/u-boot-master/Makefile
24 ↗(On Diff #45198)

Ok will change

25 ↗(On Diff #45198)

It's unneeded.

38 ↗(On Diff #45198)

Having a default doesn't hurt ?
This is the master port and the patch files are added in the slave one.

192 ↗(On Diff #45198)

Yeah ok sorry, I see what you mean.

mat added inline comments.Jul 12 2018, 1:23 PM
sysutils/u-boot-master/Makefile
38 ↗(On Diff #45198)

Well, it looks strange, you say -p1 here, and then you still add :-p1 on all the patches, it feels like a belt and suspenders situation, only one is needed, why have both?

It is confusing, when someone else will look at this, they will wonder why both are set, and search for some subtlety that does not exist.

manu added inline comments.Jul 12 2018, 1:41 PM
sysutils/u-boot-master/Makefile
38 ↗(On Diff #45198)

I'll remove p1 from the patches and use the default one.

manu added inline comments.Jul 12 2018, 1:50 PM
sysutils/u-boot-master/Makefile
161 ↗(On Diff #45198)

Yeah so that doesn't work as FreeBSD_Fragment is in the u-boot-master port and the slave ports can override the Fragment to use.

manu updated this revision to Diff 45211.Jul 12 2018, 2:35 PM
manu marked 23 inline comments as done.
imp accepted this revision.Jul 12 2018, 3:36 PM
linimon added inline comments.Jul 12 2018, 9:01 PM
sysutils/u-boot-master/Makefile
158 ↗(On Diff #45198)

I was wrong. If the line remains unchanged you can't build e.g. u-boot-orangepi-2e.

This revision was not accepted when it landed; it landed in state Needs Review.Jul 22 2018, 11:58 AM
Closed by commit rP475103: u-boot: Update to v2018.07 (authored by manu). · Explain Why
This revision was automatically updated to reflect the committed changes.

All of these fail the checksum in the fetch phase primarily due to file sizes for the mbox patches changing from those set in distinfo.