Code review comment for lp:~mabac/linaro-image-tools/snowball-support

Revision history for this message
Mattias Backman (mabac) wrote :

> I am OK with this as "good enough". For future enhancement I'd like to note:

Great, thank you.

>
> 1) The bootloader environment in 'bootargs' and 'bootcmd' are never used
> currently, as we have a separate u-boot-env.bin file with these. I am unsure
> of where LMC puts its hard-coded data and I don't see the advantage of having
> it here. It does however no harm other than confuse. OK?

I think we can handle that in the hwpack v2 work which is about taking away the hard coded data and let hwpacks handle it all. I'm going to start expanding this spec next week to take into account the discussions we had at LDS.
    https://wiki.linaro.org/Platform/Specs/11.11/HardwarePacksV2

>
> 2) For the final version we need to have two separate packages containing
> startup files as the licenses are different (u-boot is GPL and the others are
> proprietary). as I understand this implementation there is only room for one
> SNOWBALL_STARTUP_FILES_CONFIG and this will then have to describe the contents
> of both its own plus an other package which is not pretty. A general solution
> would e.g loop over all packages and find one cfg file for each package. The
> current solution is however legally OK and works for now.

Then we'll go with this if it works for you and let's change it as soon as that new packaging scheme is fixed.

« Back to merge proposal