Page MenuHomeFreeBSD

Start u-boot framework
AbandonedPublic

Authored by imp on Oct 16 2016, 10:30 PM.

Details

Reviewers
brd
gonzo
manu
jmcneill
db
ganbold
bapt
Group Reviewers
portmgr
Summary

This starts the u-boot framework.

Changes are:
o grab u-boot from my github tree that has branches port-vYYYY.MM for each branch we support. All patches move to here so we can track things better.
o slim down the makefile and move most of the infrastructure goo into sysutils/u-boot-master
o upgrade to 2016.05 -- beaglebone done as PoC. All other omaps will be done next: pandaboard, duovero.

Test Plan

Test boot this then push it in :)
Expand in future commits to include iMX6, allwinner and broadcom (rpi*) boards.
Expand in future to cope with UEFI provided by u-boot.
Consolidate the huge fugly hack we do to the config files somehow.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

imp updated this revision to Diff 21433.Oct 16 2016, 10:30 PM
imp retitled this revision from to Start u-boot framework.
imp updated this object.
imp edited the test plan for this revision. (Show Details)
imp added reviewers: bapt, brd, db, jmcneill, ganbold, gonzo, manu.
imp set the repository for this revision to rP FreeBSD ports repository.
bapt added inline comments.Oct 17 2016, 6:18 AM
Mk/Uses/u-boot.mk
65 ↗(On Diff #21433)

From there it should be in the post section (see mate.mk for a simple example)

67 ↗(On Diff #21433)

${SETENV} ${CONFIGURE_ENV} before ${MAKE_CMD} ?

jmcneill added inline comments.Oct 17 2016, 10:14 AM
Mk/Uses/u-boot.mk
7 ↗(On Diff #21433)

incldue -> include

23 ↗(On Diff #21433)

Where is this used?

manu edited edge metadata.Oct 17 2016, 11:08 AM

I don't see the BUILD_DEPENDS on arm-none-eabi-gcc

mat added a comment.Oct 17 2016, 12:59 PM

I guess this is so that there can be many u-boot-* ports that all do the same thing ?
In that case, I really thing the way to go is not a USES but a master port, with slaves only changing the MODEL/BOARD_CONFIG/...

Mk/Uses/u-boot.mk
11–19 ↗(On Diff #21433)

I feel those should be prefixed, like UBOOT_MODEL, UBOOT_BOARD_CONFIG...

36–38 ↗(On Diff #21433)

It feels really strange having a USES that defines PORTNAME, PORTVERSION and CATEGORIES.

43 ↗(On Diff #21433)

THIS SHOULD NOT BE SET.

44 ↗(On Diff #21433)

This is also very wrong.

db edited edge metadata.Oct 17 2016, 1:19 PM
In D8263#171941, @mat wrote:

I guess this is so that there can be many u-boot-* ports that all do the same thing ?
In that case, I really thing the way to go is not a USES but a master port, with slaves only changing the MODEL/BOARD_CONFIG/...

I think the original plan was a master port which makes sense to me.

imp added a comment.Oct 17 2016, 1:44 PM
In D8263#171941, @mat wrote:

I guess this is so that there can be many u-boot-* ports that all do the same thing ?
In that case, I really thing the way to go is not a USES but a master port, with slaves only changing the MODEL/BOARD_CONFIG/...

I tried that, and it simply didn't work. Perhaps you could share some reasons for your thinking here? This isn't really a master port in the traditional sense: there's no defaults, there's lots of special cases, etc. It's not something that fits that mold as well as you think it might.

Mk/Uses/u-boot.mk
7 ↗(On Diff #21433)

OK.

11–19 ↗(On Diff #21433)

Why? What benefit is there from the longer names.

23 ↗(On Diff #21433)

I'ts a typo for UBOOT_OMAP_VERSION. It works because the default version is the same and it picks that up.

43 ↗(On Diff #21433)

Why not?

44 ↗(On Diff #21433)

Why's that?

65 ↗(On Diff #21433)

OK. Will do.

67 ↗(On Diff #21433)

I don't think it's needed, but sure, if that's convention.

imp added a comment.Oct 17 2016, 1:45 PM
In D8263#171949, @db wrote:
In D8263#171941, @mat wrote:

I guess this is so that there can be many u-boot-* ports that all do the same thing ?
In that case, I really thing the way to go is not a USES but a master port, with slaves only changing the MODEL/BOARD_CONFIG/...

I think the original plan was a master port which makes sense to me.

It did, but it didn't really work or scale well.

mat added a comment.Oct 17 2016, 2:03 PM
In D8263#171950, @imp wrote:
In D8263#171941, @mat wrote:

I guess this is so that there can be many u-boot-* ports that all do the same thing ?
In that case, I really thing the way to go is not a USES but a master port, with slaves only changing the MODEL/BOARD_CONFIG/...

I tried that, and it simply didn't work. Perhaps you could share some reasons for your thinking here? This isn't really a master port in the traditional sense: there's no defaults, there's lots of special cases, etc. It's not something that fits that mold as well as you think it might.

It really is more of a semantic thing. I don't think a USES should be doing all those things like setting PORTNAME or PORTVERSION. It looks more like what the net/intel-* ports are doing, or what the actual mess of sysutils/u-boot-* ports are doing.

All that is in the u-boot.mk could be in sysutils/u-boot-beaglebone/Makefile, and the other sysutils/u-boot-* ports could use it and only redefine what is actually needed.

Mk/Uses/u-boot.mk
11–19 ↗(On Diff #21433)

namespace polution, all USES that use external variables have them.

43 ↗(On Diff #21433)

All variables that start with an _ are for internal, framework only, use, they should not be used outside.

44 ↗(On Diff #21433)

Well, mostly because it is a noop, it is defined as:

DISTVERSIONFULL= ${DISTVERSIONPREFIX}${DISTVERSION:C/:(.)/\1/g}${DISTVERSIONSUFFIX}

So setting it to something else will not do anything.

imp updated this revision to Diff 21455.Oct 17 2016, 4:41 PM
imp edited edge metadata.
imp removed rP FreeBSD ports repository as the repository for this revision.

Updated based on the comments here, also did duovero and pandaboard.

mat added inline comments.Oct 17 2016, 4:46 PM
Mk/Uses/u-boot.mk
47 ↗(On Diff #21455)

This still will not do anything.

mat added inline comments.Oct 17 2016, 4:55 PM
Mk/Uses/u-boot.mk
47 ↗(On Diff #21455)

Ok, ok, I get what the problem is.

You can't set PORTVERSION in a USES, it is too late.
I'm wondering what other breakage is lurking around as PORTNAME is set too late too.

This *really* should be a master/slave scheme.

imp added a comment.Oct 17 2016, 6:23 PM

comments from both before and after my update...

Mk/Uses/u-boot.mk
11–19 ↗(On Diff #21433)

I'll consider this.

36–38 ↗(On Diff #21433)

Yes, it is a bit strange, but master port route simply didn't work.

43 ↗(On Diff #21433)

It's broken without setting it. If I don't set it, the tar that's grabbed has no version it in:

Attempting to fetch https://codeload.github.com/bsdimp/u-boot/tar.gz/ports-v2016.05?dummy=/u-boot-_GH0.tar.gz

While setting it and DISTVERSIONFULL gives me:

Attempting to fetch https://codeload.github.com/bsdimp/u-boot/tar.gz/ports-v2016.05?dummy=/bsdimp-u-boot-beaglebone-ports-v2016.05_GH0.tar.gz

44 ↗(On Diff #21433)

And yet setting gives me the proper name.

Setting it gives me bsdimp-u-boot-<boardname>-ports-v2016.05_GH0.tar.gz and not setting it gives me two dashes in a row. Hardly a nop.

So
Attempting to fetch https://codeload.github.com/bsdimp/u-boot/tar.gz/ports-v2016.05?dummy=/bsdimp-u-boot-beaglebone-ports-v2016.05_GH0.tar.gz

vs

Attempting to fetch https://codeload.github.com/bsdimp/u-boot/tar.gz/ports-v2016.05?dummy=/bsdimp-u-boot--ports-v2016.05_GH0.tar.gz

Though to be honest, I suspect that I may need to grab a tag, not a branch here. But that's a minor issue.

67 ↗(On Diff #21433)

Ah, it's needed if we do the split.

47 ↗(On Diff #21455)

Master / slave doesn't work for this. It's more like when we used to build different parts of X11 that was in one big distribution. We didn't do master / slave for that. I couldn't get master / slave working because different revisions needed diferent patches (though maybe github solution would fix that). Other items just were a pain in the backside are easy here. This isn't quite like other things where we have 'variant' ports. Each port is its own thing and only kinda related to other ports of a similar name. That's not the master / slave data pattern.

And the line still actually does something, verified by commenting it out and adding it back.

Why can't I set PORTVERSION in a USES? Please help me understand.

imp updated this revision to Diff 21465.Oct 18 2016, 4:10 AM
imp updated this object.

Move to using master port.
The USES wasn't working, btw, because of the multiple layers of expansion which affect .ifs and at least one : assignment, which is why it was only kinda working for me...

Open issue: distfile.$VERSION or have all the checksums in one distfile. Since there's only one version in this so far, it doesn't matter, but it will when I add ALLWINNER support.

mat added a comment.Oct 18 2016, 2:27 PM

You should remove the distinfo files that are in the slave ports, they are not used.

The file is DISTINFO_FILE, and it defaults to MASTERDIR/distinfo

sysutils/u-boot-master/Makefile
9–17

You could remove the UBOOT_ prefix now.

41–56

Please, move that at the top of the file, also move the COMMENT paragraph between PKGNAMESUFFIX and USES, and add a MAINTAINER line just above the comment :-)

52

This should go just below the PORTVERSION line.

65

This block should go after the maintainer/comment block

73–81

If those can be overridden in slave ports, I think they should be wrapped in .ifs

mat added inline comments.Oct 18 2016, 2:35 PM
sysutils/u-boot-master/Makefile
60

This feels wrong, it's going to mess up some calculations, you should remove ${STAGEDIR}/ from it.

imp added inline comments.Oct 18 2016, 7:36 PM
sysutils/u-boot-master/Makefile
60

Mostly I'm looking for a shorthand for where to install stuff, but there's only one or two places so being explicit about STAGEDIR is fine there.

73–81

The goal is for them not to be overridden by the slave ports, so I'm starting out w/o the ifs.

imp updated this revision to Diff 21481.Oct 18 2016, 7:42 PM

Fixes to reflect latest round of critiques.

mat added inline comments.Oct 18 2016, 9:04 PM
sysutils/u-boot-master/Makefile
7–40

All this should go below, say, the MAKE_ARGS line.

60

I have no problem with shorthands, just don't call it DATADIR, it is a special variable name that the ports framework uses for "things" :-)

imp added inline comments.Oct 18 2016, 10:35 PM
sysutils/u-boot-master/Makefile
7–40

OK, but we use these variables in the next few lines... Is that kosher?

imp updated this revision to Diff 21489.Oct 19 2016, 5:35 AM

the suggested fixes seem to work, and that makes sense due to make's late binding of variables and none of the exceptions that cause issues are in play within this file.

mat added inline comments.Oct 19 2016, 7:32 AM
sysutils/u-boot-master/Makefile
7–40

make(1) does lazy evaluation, that is, if you do FOO=${BAR} it won't care about BAR until FOO needs to be evaluated.

imp abandoned this revision.Feb 26 2017, 6:35 AM

This has been committed, but in different form.