[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Providing (armhf) u-boot images together with d-i images?



On Tue, Dec 30, 2014 at 03:28:31PM +0000, Ian Campbell wrote:
> On Sat, 2014-12-27 at 00:00 +0100, Karsten Merker wrote:
> > attached is V2 of the patchset. Changes since V1:
>
> I should start by saying that I'm not personally particularly interested
> in this functionality, but I don't want to be a blocker for people who
> are so since I've been asked I'll give my 2pence...

Thanks for the review.

> Please could you post the diff in the lists of files generated by the
> build, including the sizes of the new stuff.

Please see below.
 
> FWIW review would be a lot easier if the patches were patchbombed to the
> list in the normal git send-email way rather than as multiple
> attachments on a single mail (people can kill-thread if they aren't
> interested) [...]

OK, I will do that for the V3.
 
> 0001-Add-boot-arm-u-boot-image-config.patch
> 
>         Seems fine.
> 
> 0002-Provide-u-boot-binaries-for-armhf-systems-without-u-.patch

>         I think this isn't actually publishing the u-boot binaries, but
>         rather sd-card images with the u-boot inlined, is that right? Is
>         there any use in shipping the raw binaries too?

It provides both

- a naked (set of) u-boot binary(s) for people who manually want
  to install our u-boot version onto a device while keeping an
  existing partition table and the related filesystems.  That
  requires manually dd-ing the raw SPL/TPL files to the
  appropriate offsets for the target platform, which is not
  particularly end-user friendly, but is the only way to keep an
  existing partition table.

- a ready-to-copy minimal SD card image with the u-boot SPL/TPL
  already put at the appropriate places, which is a lot easier
  for the end-user and also works from Windows systems where no
  dd is available to write images to specified offsets, but it
  overwrites any existing partition table, which might not always
  be wanted.

> 0003-Add-utils-gen-hd-image.patch
> 
>         I didn't review in detail, but perhaps the functionality of
>         patch 0002 could be folded in? TBH I'm not sure what the
>         distinction is between what 0002 does and what this script would
>         produce (except I can see the script clearly has more
>         functionality)

It depends :-).

gen-hd-image can provide different types of images - full images
(SD card image including partition table, u-boot, kernel, initrd
and dtbs) and concatenateable images, which is effectively a
full image split up into a device-specific part (containing the
partition table and u-boot) and a device-independent part
(containing kernel, initrd and dtbs).

If one decides to build concatenateable images, the
device-specific part is mostly the same as the ready-to-copy SD
card image generated by patch 2, with the difference that it
contains a filled partition table with a predefined geometry and
medium size.

My original idea was to build u-boot-only images from patch 2 for
people who want to install by TFTP or from USB-Stick using the
existing hd-media tarball.  This was the first thing I
implemented.  Vagrant proposed also building full images (u-boot,
kernel, initrd and dtbs) similar to the images we build for
i386/amd64, so I implemented that.  As full images need a bit of
space, the idea of providing concatenateable images instead of
full images came up, but doing that is dependent on solving the
"one boot script for all platforms" issue, for which we do not
yet have a proper solution, therefore I implemented that as an
option for testing this approach.

> 0004-Add-SD-card-image-build-support-for-hd-media-builds-.patch
> 
>         I don't think we need all of (.gz|.bz2|.xz) (like the README
>         suggests, although it's not clear these are all actually
>         present). Just one would do IMHO.

Only the gzipped variant is actually built, the README just
covers the possible options (when calling gen-hd-image with
-z/-j/-J).
 
>         It seems to be creating a copy of dtbs again, please lets try
>         and keep it to one set of these files in the top level
>         component.

The dtbs are only inside the generated images, they are not
copied elsewhere as files.  Having all of them inside the image
is a necessity if we want to have the option of using
concatenateable builds as in this case they are in the
device-independent part. The size of the dtbs inside the
image is less than 900kB, so I do not think that is much of
an issue size-wise.

>         The README should describe how to actually do the concatenation.

Ok, I will do that.
 
> 0005-Add-SD-card-image-and-tftpboot-tarball-build-support.patch
> 
>         What is this providing? Is it a mini.iso like netboot sd image?
>         I'm not sure how this is different from what the previous
>         patches introduced.

The previous patch produces hd-media builds for use with CD/DVD
iso images (equivalent to the i386/amd64 hd-media
boot.img.gz). This patch produces a netboot SD card (equivalent to
the i386/amd64 netboot mini.iso), plus a netboot.tar.gz which
contains everything needed to be put onto a tftp server for
tftp-booting the installer (equivalent to the i386/amd64
PXE netboot.tar.gz).

>         For the tftpboot part, u-boot supports standard pxelinux.cfg
>         syntax configuration files -- would it be better to just
>         generate a suitable one of those?

I have not in detail checked the pxelinux.cfg support in u-boot
yet, but as we unfortunately have to build special-casing into the
boot script for several platform (such as the i.MX6 console
baudrate issue), I do not see how to implement that with the
pxelinux.cfg syntax. Pointers welcome :).

> 0006-Add-additional-dependencies-needed-for-building-boot.patch
> 
>         Seems ok. But by having this last at least half the previous
>         patches won't work in isolation. Better IMHO to add each
>         dependency as the dependency is introduced.

Ok, I'll do that for V3.

> I suppose my main overall concern is that this seems to be providing the
> same thing 3 or 4 times and I'm not sure what the difference between
> each of those things is (or perhaps as likely I've misunderstood what is
> going on somewhere along the line).

I never intended to actually build all options the patchset
provides (and tried to state that in my previous mails, but
perhaps I should have been more explicit). The patchset offers
various possibilities for testing out of which we should choose
a suitable set. My intention for the daily builds was to enable
exactly two options:

- u-boot-only for people using tftpboot or the existing hd-media
  tarball.

- full images (in the hd-media and netboot variants) for people
  who prefer just copying a single image to an SD card and have a
  working installer as proposed by Vagrant.  Those could possibly
  later be replaced by the concatenateable variant, if the full
  images should use too much space/bandwidth and we have solved
  the "one-bootscript-for-all" problem.

> Perhaps a summary of the new toplevel output directories (i.e. things
> added alongside netboot, hd-media, network-console etc) describing what
> each one is would help?

The patchset creates only one additional top-level directory,
which is the "u-boot-only" one:

$ ls -R u-boot/
u-boot/:
A10-OLinuXino-Lime  BeagleBoneBlack  Cubietruck   PandaBoard
A20-OLinuXino-Lime  Cubieboard       MX53LOCO     Wandboard_Quad
BananaPi            Cubieboard2      MX6_Cubox-i

u-boot/A10-OLinuXino-Lime:
A10-OLinuXino-Lime.sdcard.img.gz  u-boot-sunxi-with-spl.bin.gz

u-boot/A20-OLinuXino-Lime:
A20-OLinuXino-Lime.sdcard.img.gz  u-boot-sunxi-with-spl.bin.gz

u-boot/BananaPi:
BananaPi.sdcard.img.gz  u-boot-sunxi-with-spl.bin.gz

u-boot/BeagleBoneBlack:
BeagleBoneBlack.sdcard.img.gz  MLO.gz  u-boot.img.gz

u-boot/Cubieboard:
Cubieboard.sdcard.img.gz  u-boot-sunxi-with-spl.bin.gz

u-boot/Cubieboard2:
Cubieboard2.sdcard.img.gz  u-boot-sunxi-with-spl.bin.gz

u-boot/Cubietruck:
Cubietruck.sdcard.img.gz  u-boot-sunxi-with-spl.bin.gz

u-boot/MX53LOCO:
MX53LOCO.sdcard.img.gz  u-boot.imx.gz

u-boot/MX6_Cubox-i:
MX6_Cubox-i.sdcard.img.gz  SPL.gz  u-boot.img.gz

u-boot/PandaBoard:
MLO.gz  PandaBoard.sdcard.img.gz  u-boot.bin.gz

u-boot/Wandboard_Quad:
u-boot.imx.gz  Wandboard_Quad.sdcard.img.gz

$ du -s u-boot/*
324	u-boot/A10-OLinuXino-Lime
332	u-boot/A20-OLinuXino-Lime
332	u-boot/BananaPi
476	u-boot/BeagleBoneBlack
324	u-boot/Cubieboard
332	u-boot/Cubieboard2
332	u-boot/Cubietruck
324	u-boot/MX53LOCO
324	u-boot/MX6_Cubox-i
308	u-boot/PandaBoard
292	u-boot/Wandboard_Quad

Inside the existing hd-media and netboot directories, a
subdirectory "SD-card-images" gets created, which then contains
the created images. These have the following sizes:

option "netboot full image":
18568	A10-OLinuXino-Lime.img.gz
18572	A20-OLinuXino-Lime.img.gz
18572	BananaPi.img.gz
18640	BeagleBoneBlack.img.gz
18572	Cubieboard2.img.gz
18568	Cubieboard.img.gz
18572	Cubietruck.img.gz
18564	MX53LOCO.img.gz
18568	MX6_Cubox-i.img.gz
18560	PandaBoard.img.gz
18552	Wandboard_Quad.img.gz

option "hd-media full image":
17708	A10-OLinuXino-Lime.img.gz
17712	A20-OLinuXino-Lime.img.gz
17712	BananaPi.img.gz
17784	BeagleBoneBlack.img.gz
17712	Cubieboard2.img.gz
17708	Cubieboard.img.gz
17712	Cubietruck.img.gz
17712	MX53LOCO.img.gz
17712	MX6_Cubox-i.img.gz
17700	PandaBoard.img.gz
17692	Wandboard_Quad.img.gz

The "concatenateable" variants of these would only use roughly
18MB once for the filesystem image plus around 200kB per target
platform.

Regards,
Karsten
-- 
Gem. Par. 28 Abs. 4 Bundesdatenschutzgesetz widerspreche ich der Nutzung
sowie der Weitergabe meiner personenbezogenen Daten für Zwecke der
Werbung sowie der Markt- oder Meinungsforschung.


Reply to: