Add Efika MX Smartbook/Smarttop support

what is the reason for the rm -f after using mv ? do you not trust mv to move the files ?

oops, forgot to remove that line

what do you think about patch overall?

overall it looks fine to me if the prerequisites are fulfilled (i.e. /boot/boot.script needs to exist)
do you actually want to have uInitrd/uImage in /boot given that /boot can not be a vfat i would
suggest using the same solution we use on the preinstalled images and have a hidden or non-data
partition for carrying the actual bootloader files/data where u-boot needs them.

Oliver, it is our intent that /boot always be an ext2 partition.

We are phasing out support for booting from vfat partitions as "recommended practise" in so far as it's only in U-Boot to keep old systems working before migrating partitions to ext2. Once we release U-Boot 2.0.6 for both boards, everyone can use ext2 as a /boot partition and everything will be nice and safe.

The only sticking point right now is that it is impossible to automagically patch U-Boot from older Smarttop systems (Smartbook is fine though) without going into Linux first, and we have an SPI bug somewhere which is causing MTD operation to fail miserably. We are working on a solution.

flash-kernel should NOT be hacked to support VFAT boot partitions in the meantime.

BTW commenting our check_running_subarch is also only there because Smartbook still uses the "efikasb" subarch, and will move to the "efikamx" subarch shortly.

Adding Ubiquity as being affected as it seems to include flash-kernel.

for u-boot it would be desirable to sync up with upstream, note that we nowadays use the same source package for all u-boot's and it would be nice to have the efika support in the same source tree instead of getting another source package into the archive, please talk to linaro (specifically jcrigby) to get your patches against the recent upstream tree included.

if your setup is always ext2 (dunno how you got that to behave stable, we have seen issues with that in the past (mainly boot speed but also stability issues)) then marcins patch is fine and we can include it in natty.

vfat u-boot support in flash-kernel is already there for plenty of other boards though, it would not have to be hacked (though we might move away from using a filesystem at all for the boot stuff and just go with a raw partition, since that allows to use the std u-boot environment settings without boot.scr (i.e. saveenv would work there))

also note that we sync flash-kernel from debian, i assume they would like to have support for the efika too so it might be desirable to just get the patch in their tree, ubuntu will just get it automagically then.

Oliver, I have to completely disagree with you that U-Boot source code is even required for the board - let alone something based on mainline. This is not some crazy little embedded board like Beagle where the updates come fast and there are 10 forks, and to be honest we would be doing a great disservice to customers by making it as such. Ubuntu has absolutely no benefit and no business updating the core boot firmware on a board.

We are porting to U-Boot 2010.12-rc3 right now on the sole basis that it is less maintenance for us, and source *is* available, we are simply not in the frame of mind to publish this for everyone to mess around with. GPL compliance is one thing (ask and ye shall receive :) but *packaging* it is egregious.

flash-kernel support for vfat is, in that sense, an egregious hack we know works on omap3 boards, but wanted to do away with. vfat support for booting on the latest unpublished U-Boot (2009.01-2.0.6) is a capitulation to the fact that we would like to run kernels and boot scripts from clean unboxed SD cards; just drop them in a reader, copy the files. No formatting, partitioning or anything. This works just because we use the same concept in our bootcmd as flash-kernel's Dove boot.scr, just built in to the firmware (it is a nested loop over devices, units and filesystems and it's not really fair to exclude vfat booting off PATA when systems shipped for a year already).

What we would like is to get this U-Boot flashing problem fixed: this either means getting SPI NOR working correctly under 2.6.31, or 2.6.35 (ongoing) or mainline (ongoing) in order to get the older U-Boot revisions (2009.01-1.x.x) to automagically do it from their current boot scheme (look for devices that have "uImage" on them and boot it direct). Then every system on the planet can be upgraded and nobody will have to use VFAT as a /boot ever again, once booted, they may simply convert their filesystem with 3 or 4 lines in a terminal or run a script we may provide.

I do not believe we should be hacking flash-kernel to support the seperate "U-Boot partition" just so we can patch it out again. We have to work on unifying the behavior of the systems. What we can do, is provide you guys with U-Boot source code for the update, which will probably happen in the next few days, if not an automatic installer SD which will refresh Maverick..

As it stands the patch supports the board fine as long as you are using the required ext2 partition for /boot - this includes every Smartbook given out at UDS. If you are unlucky enough to use vfat, then unfortunately you will need to migrate here for it to work ideally. We are willing to help people migrate. By the time Natty rolls around this thing will be settled. In the meantime send me a mail and I'll make sure you guys can get U-Boot and give instructions on how to manually update it on a Smarttop.

i dont know why you are raving so much now, i already said the patch looks fine to me and i'll include it in our flash-kernel if it doesnt go to debian before :)

flash-kernel is supporting separate partitions *by design*, there is nothing wrong about it, its whole purpose it to adjust to specific bootloader setups for specific boards if that is on NAND, a vfat partition, a raw or ext2 partition or whatever ...

i didnt mean to attack you (since your comment above indicates that you feel attacked) nor to tell you how to set up your boot stuff i was just commenting on whats there already...

what i meant with my comment about u-boot is that if you ever want to build ubuntu images "officially" (which i dont know if you will ever do it as i.e. a community build or some such) you will need a bootloader binary package in the archive and this binary should better come from the same source package as all others to not have additional maintenance overhead, john rigby would be the right person to talk to to get u-boot patches included in the source tree we use.

flash-kernel doesn't support seperate partitions by any design except to put this in board-specific code. It's not global, it's duplicated every time it's needed (and in this case it seems only on boards that can only boot from vfat). We don't feel we need to support it. In any case if the patch is accepted, great, let's run with that.

Why does official support for Ubuntu require that we package a bootloader binary? This makes absolutely no sense at all. Does Dell ship you BIOS binaries for their laptops so you can make it official? Does HP? I have a real problem of having source code for source code's sake. We do not support whatsoever building U-Boot and flashing it yourself with a custom build as it complicates board support, and I really don't think Canonical wants to take on this user support process on our behalf. It is simply unneeded.

We will support u-boot updates simply by shipping a binary and a boot.scr which can be copied to an SD card. Source code will be in a git repo or hosted at PowerDeveloper. I will definitely feel attacked if Ubuntu's policy is to allow users to generate spurious support requests targeted at Genesi from people building U-Boot with various compiler versions, various distribution versions, various user-supplied patches and then finding something breaks for them.

dell systems dont have to ship the BIOS on disk ... think more like grub here ;)

our image build system puts the u-boot.bin binary in place for arches that dont boot from NAND, to have this binary available we need a package ... if your platform doesnt need u-boot.bin on the SD thats indeed a moot point.

we dont have source code for source codes sake but to be
a) GPL compliant and
b) enable developers to easily build changed binaries if they want to enhance it or help with bug fixing

users wont build u-boot with "varoius compiler versions", what we make sure is that u-boot is built with the supported compiler for that one release. feel free to do as you like but note that the ubuntu image build system does not have any internet access during build time, there is only a local package archive mirror so having a binary package of the bootloader, be it grub, syslinux, isolinux or u-boot is a requirement for any officially built image, it is how it is, there is nothing to discuss about it :)

oh, and on a side note there is work going on to generalize flash-kernel more (see the subarch detection spec) so it will have more generic ways to access partitions (hopefully soon) at some point

you are right that this duplication is a mess and i think flash-kernel upstream (debian) as well as anyone else working with it is aware that this is a shortcoming in its design.

OK, so what's next?

nothing on your side, i will add the patch as soon as i'm done merging the package (if not someone sends that patch to debian before)

as i said above :)

Please remember ubiquity contains flash-kernel too and there is another detection script (flash-kernel-installer?) which needs the machine and subarch added to it.

Those both shipped on the smartbook too.. I can send the patches if needed but they are plainly obvious additions.

Btw why does ubiquity need an embedded clone if flash-kernel anyway?

Oliver if you have time I have some simple suggestions for improving it for cmdline argument additions, and flash-kernel also should ship with kernel postinst and postrm hooks too or it never does anything on a default installed system after oem-config.

ubiquity doesnt contain a clone of flash-kernel, flash-kernel-installer is what is used by the debian-installer scripts to set up a bootloader, this is what ubiquity uses, please read the code closer ;)

dont worry about the proper inclusion of the patch, i will add all bits needed for HW detection etc.

kernel poistinst and postrm bits were used in the past and are dropped upstream (and in ubuntu as well) with the vanishing of support for /etc/kernel-img.conf, flash-kernel is called by update-initramfs nowadays please have a look at its code too...

for enhancements of flash-kernel cmdline opts i'd suggest a wishlist bug against flash-kernel.

For d-i, a special type of stripped down .debs is used (udebs) and these are installed in special packages indices in the archive; these are not supposed to be installed on an end-user system, only when creating d-i images. However images using ubiquity are essentially like a installed system, so ubiquity has a script to pull a copy of all the latest udeb into its package, and embeds all udebs it calls. It's a copy, but in a way this avoids code duplication between d-i and ubiquity images.

This bug was fixed in the package flash-kernel - 2.28ubuntu14

---------------
flash-kernel (2.28ubuntu14) natty; urgency=low

  [ Marcin Juszkiewicz ]
  * Add Efika MX Smartbook/Smarttop support based on work done by Genesi.
    (LP: #671027)
 -- Oliver Grawert <email address hidden> Thu, 17 Feb 2011 16:01:42 +0100

noticed a couple of things:
* "#check_subarch" would ideally be enabled and consistent with other platforms, or removed entirely.
* tmpfile is not removed
* $tmpfile.someextension is an insecure construct (used in other places of the script)

Loic:

Feel free to re-enable check_subarch

Also remove any check for "efikasb" as a subarch, since it's all efikamx now. That's the only reason check_subarch was disabled, because we had two, and now we have one. For Natty it will stay -efikamx.

As for the extensions and tmpfile stuff, this was copied from the Dove code..

Okay I have some problems with the support committed to flash-kernel, namely

* it doesn't differentiate between kernel versions, so uImage and uInitrd will overwrite any older uImage and uInitrd version. This is unfriendly and it is impossible to tell at a glance (without using mkimage to list the details) what version of the kernel it is
* generating boot.scr from boot.script is fine enough for the above, and allows users to edit options, but.. see above
* if uInitrd doesn't already exist, it will not be created as the entire creation part is wrapped in an if [ -f /boot/uInitrd ]. What should happen is

if [ -f /boot/uInitrd ]; then
   cp /boot/uInitrd /boot/uInitrd.bak
fi

# create uInitrd here regardless

Actually, apart from the last one I'm willing to live with what happens here, as it makes cleanup of kernels much easier. But, it does beg a question, if I dpkg --purge a kernel image, update-initramfs will remove the initrd.img-${kver} - why can't flash-kernel remove kernel files?

Also why can't we ship a boot.script which you can use sed to insert the kernel version (cp boot.script /tmp/boot.script; sed -i -e's/KVERSION/${kver}/' /tmp/boot.script; mkimage blah -d /tmp/boot.script /boot/boot.scr; rm -f /tmp/boot.script), and create versioned uInitrd and uImage files?

(BTW regarding last comment, if you guys approve I will submit a patch...)

Additional comment: Loic, PLEASE can we put back "efikamx" subarch support? It would be a legacy hack but it will mean our current kernels work. I don't want to have to build a vendor-specific package and track it for the sake of 2 lines. Neither Maverick nor Natty has or will have an "mx51" subarch kernel so the fix has basically caused a nightmare AND a breakage, for the sake of "doing it right" not "keeping what works, working".

Can we revert the change please...

        Hey

On Thu, Mar 03, 2011, Matt Sealey wrote:
> Okay I have some problems with the support committed to flash-kernel,
> namely
> * it doesn't differentiate between kernel versions, so uImage and
> uInitrd will overwrite any older uImage and uInitrd version. This is
> unfriendly and it is impossible to tell at a glance (without using
> mkimage to list the details) what version of the kernel it is

 This is a limitation of flash-kernel; it would probably be best to move
 discussions on evolving the design of flash-kernel elsewhere; I took
 some notes of possible flash-kernel improvements at the Emdebian sprint
 last week; see http://wiki.debian.org/FlashKernelRework

 You can call flash-kernel with a kernel version, or none in which case
 /vmlinuz or /boot/vmlinuz gets used (and corresponding initrd.img); in
 any case, the last call wins.

> * if uInitrd doesn't already exist, it will not be created as the
> entire creation part is wrapped in an if [ -f /boot/uInitrd ]. What
> should happen is
>
> if [ -f /boot/uInitrd ]; then
> cp /boot/uInitrd /boot/uInitrd.bak
> fi
>
> # create uInitrd here regardless

 Makes sense; the rest of the code uses mv, but I agree with the comment

 Would you or Marcin be tempted to offer a patch for this?

> Actually, apart from the last one I'm willing to live with what happens
> here, as it makes cleanup of kernels much easier. But, it does beg a
> question, if I dpkg --purge a kernel image, update-initramfs will remove
> the initrd.img-${kver} - why can't flash-kernel remove kernel files?

 There should only be one uImage on your system, and I guess it makes
 sense to keep it even if you remove the last kernel installed on your
 system. Perhaps you meant the vmlinux-* files? These are removed by
 dpkg when removing the linux-image-* .debs which ship them.

> Also why can't we ship a boot.script which you can use sed to insert the
> kernel version (cp boot.script /tmp/boot.script; sed -i
> -e's/KVERSION/${kver}/' /tmp/boot.script; mkimage blah -d
> /tmp/boot.script /boot/boot.scr; rm -f /tmp/boot.script), and create
> versioned uInitrd and uImage files?

 That could be a flash-kernel improvement; currently it:
 * doesn't allow having more than one kernel ready to boot
 * doesn't cleanup files it generates in boot

 This feature would be specific to systems booting from SD, which are
 increasingly common. I would prefer seeing this feature developed in
 upstream (Debian's) flash-kernel before we add it in Ubuntu though.
 The delta is large enough as it is :-)

On Thu, Mar 03, 2011, Matt Sealey wrote:
> Additional comment: Loic, PLEASE can we put back "efikamx" subarch
> support? It would be a legacy hack but it will mean our current kernels
> work. I don't want to have to build a vendor-specific package and track
> it for the sake of 2 lines. Neither Maverick nor Natty has or will have
> an "mx51" subarch kernel so the fix has basically caused a nightmare AND
> a breakage, for the sake of "doing it right" not "keeping what works,
> working".

 mx51 is the computed subarch of the Linaro mx51 kernel; back then I
 thought it supported efikamx smartbook too, even if minimally.

 As I wrote to you in ...

I was under the impression that flash-kernel, as a subordinate of update-initramfs or at the very least kernel postinst.d hook, is ALWAYS called with a version (in fact, standards state that it passes the kernel version as $1 AND $2 for some funky reason). There's very very little reason to call it with no version, and even if it was, shouldn't it pull "uname -r"? Otherwise it would have absolutely no idea which kernel to work on, and so far as I have seen after about a gazillion kernel installs, removes and replacements, the "vmlinux" and "initrd.img" symlinks either in /boot or / are some magic which doesn't always happen.

However, vmlinux-$kvers and initrd.img-$kvers are always going to be there. Imagine for a second that flash-kernel is a bootloader helper tool like grub-install and it needs to know which kernels to pick out and handle. It's not useful to leave a kernel file in there "unversioned".

Case in point: it never broke before, and the flash-kernel code we adapted for Efika MX we "stole" from the Dove support in the existing script. It supported appending the version to the file. We merged in a few cute tweaks from OMAP too.. it was a mix of what was already there, done a little bit better.

~

As for the kernel "mx51" subarch, mx5 is a possibility too (although an upstream seems to have kicked out MX51 and MX53 both working at the same time in the same kernel) - but so are many other combinationsm possibly Cortex-A9 support for the MX6 on top. As always the subarch is specific to the distribution and if it's not upstream, most certainly vendor-specific. Ours is "efikamx" and we used to have "efikasb" to differentiate between the two different compiles (there were horrible hacks which made them not build together, which are now gone). As it stands we can stick to "efikamx" right now. Probably the best subarch to go for would be "imx", which is almost too generic.. but it would work considering the restrictions of the distribution (armv7-a, Thumb2..). The suffix and therefore subarch is a distribution thing not a kernel mainline thing.

~

Mainline doesn't support Smarttop nearly well enough (unless you enjoy having no display, good luck!) and Smartbook is even worse. We have customer support to engage with 2.6.35 BSP kernels, and then we will make the jump to mainline. Far, far too much needs changing in mainline right now to make it an effort we can engage in, and still get adequate platform support for the board given it's feature set (proper EDID parsing for HDMI sinks (omap team committed some horrible code, and it's crashy!), proper coordination between transmitter and ASoC for HDMI audio, rt2800usb has been broken for about 2 years, we need extra features for power_supply class to support the battery in a board-agnostic manner, and many noticeable shortfalls in the machine files, not to mention actual multimedia support and the Flash 10.2 plugin). We are skipping ahead as soon as our customers are happy that 2.6.35 will work for them for the time being. Until then, and even then, it'll stay "efikamx" from Genesi, and even stay that way when we get an MX53 board.

Parsing kernel configs doesn't work to identify the running ...

For reference this is my untested hook for the Efika..

efikamx_flash_kernel() {
 tmp=$(tempfile)
 printf "Generating kernel u-boot image... " >&2
 mkimage -A arm -O linux -T kernel -C none -a 0x90008000 \
  -e 0x90008000 -d $kfile "$tmp.uImage" >&2 1>/dev/null
 echo "done." >&2

 if [ -e /boot/uImage-${kvers} ]; then
  mv /boot/uImage-${kvers} /boot/uImage-${kvers}.bak
 fi
 mv $tmp.uImage /boot/uImage-${kvers}
 rm -f "$tmp.uImage"

 if [ -e ${ifile} ]; then
  printf "Generating initrd u-boot image... " >&2
  mkimage -A arm -O linux -T ramdisk -C none -a 0x0 \
   -e 0x0 -n "$idesc" -d "$ifile" "$tmp.uInitrd" >&2 1>/dev/null
  echo "done." >&2

  if [ -e /boot/uInitrd-${kvers} ]; then
   mv /boot/uInitrd-${kvers} /boot/uInitrd-${kvers}.bak
  fi
  mv "$tmp.uInitrd" /boot/uInitrd-${kvers}
  rm -f "$tmp.uInitrd"
 fi

 if [ -e /boot/boot.script ];then
  cp /boot/boot.scr /boot/boot.scr.bak

  printf "Generating u-boot configuration from /boot/boot.script... " >&2
  cp /boot/boot.script "$tmp.boot.script"
  # not sure if sed or bash is going to freak at the dollar or curlybraces.. TEST TEST!! - NEKO
  sed -i 'backup' -e's/KVERSION/${kvers}/' "$tmp.boot.script"
  mkimage -A arm -T script -C none -n "Ubuntu boot script" \
   -d "$tmp.boot.script" /boot/boot.scr >&2 1>/dev/null
  echo "done." >&2
  rm -f "$tmp.boot.script"
 fi

 rm -f "$tmp"
}

 Just a note: I think this is getting off topic for this bug; we should
 only exchange on resolving this particular bug in a relatively simple
 way; flash-kernel design discussions should be held elsewhere IMO.

On Thu, Mar 03, 2011, Matt Sealey wrote:
> I was under the impression that flash-kernel, as a subordinate of
> update-initramfs or at the very least kernel postinst.d hook, is ALWAYS
> called with a version (in fact, standards state that it passes the
> kernel version as $1 AND $2 for some funky reason). There's very very
> little reason to call it with no version, and even if it was, shouldn't
> it pull "uname -r"? Otherwise it would have absolutely no idea which
> kernel to work on, and so far as I have seen after about a gazillion
> kernel installs, removes and replacements, the "vmlinux" and
> "initrd.img" symlinks either in /boot or / are some magic which doesn't
> always happen.
>
> However, vmlinux-$kvers and initrd.img-$kvers are always going to be
> there. Imagine for a second that flash-kernel is a bootloader helper
> tool like grub-install and it needs to know which kernels to pick out
> and handle. It's not useful to leave a kernel file in there
> "unversioned".

 It isn't called with a version from d-i for instance; also, some
 documentations tell people to just run "flash-kernel" unversioned. I'm
 not arguing whether it's a good or bad thing, it's just a currently
 supported interface.

> As for the kernel "mx51" subarch, mx5 is a possibility too (although an
> upstream seems to have kicked out MX51 and MX53 both working at the same
> time in the same kernel) - but so are many other combinationsm possibly
> Cortex-A9 support for the MX6 on top. As always the subarch is specific
> to the distribution and if it's not upstream, most certainly vendor-
> specific. Ours is "efikamx" and we used to have "efikasb" to
> differentiate between the two different compiles (there were horrible
> hacks which made them not build together, which are now gone). As it
> stands we can stick to "efikamx" right now. Probably the best subarch to
> go for would be "imx", which is almost too generic.. but it would work
> considering the restrictions of the distribution (armv7-a, Thumb2..).
> The suffix and therefore subarch is a distribution thing not a kernel
> mainline thing.

 You're making two points above:
 * people might be running any kernel name (which is the point I was
   making myself against subarch in general, and the reason I propose
   testing the contents of the config file instead)
 * that the current subarch concept is distribution specific; I agree
   with this point as well, but your -efikamx kernel isn't in Ubuntu, so
   by that logic, the -efikamx string should be tested in a flash-kernel
   version which lives in the same repository as your -efikamx kernel; I
   would personally prefer if you hadn't your own flash-kernel fork,
   which is why I'm proposing to test the config instead because that
   seems to be always correct, whatever the kernel extension

> Parsing kernel configs doesn't work to identify the running board. After
> all, a config file can have many boards in it. How do you parse the
> config file and THEN...

On Thu, Mar 03, 2011, Matt Sealey wrote:
> For reference this is my untested hook for the Efika..

 In general, I find patches easier to review; will you test this new
 version?

--
Loïc Minier

On Thu, Mar 03, 2011, Loïc Minier wrote:
> Debian #601789 describes related issue around running flash-kernel at
> the right time. I think we want to dpkg-trigger flash-kernel from
> update-initramfs and perhaps from linux' postinsts.

 Sorry, that's Debian #550584

--
Loïc Minier

Attaching a patch which I have tested extensively which restores the -efikamx subarch and also makes the script work to our basic specifications. I did capitulate to the new "boot.script" method but we can properly handle kernel versions. It is amazing what a little bit of 'sed' can do.

Also fixes
 * No uInitrd would be generated if one didn't already exist. The check has been changed to check for an existing initrd.img of the correct version, and then if a uInitrd of that version already exists, back that one up
 * Restores the kernel description 'comment' for uImage generation
 * Lots of anal retentive curly brackets
 * Picks a rootfs properly but this is a bit of a hack as I don't understand the initramfs hook or the difference between the default and override (I guess, my script will explode if run from a udeb since /dev/root is the livecd and not the target)
 * Stop re-compressing the uInitrd. It is already gzip compressed, so compressing it again in from mkimage is redundant
 * Needs a /boot/boot.script which is NOT shipped in this patch

If you could apply this to the Maverick flash-kernel I would much appreciate it. For Natty we need to think bigger, or we can fix up this patch some more.

One thing I am really concerned about is where the proper place to ship the boot.script source for generating the boot.scr - is this meant to be in some hwpack metapackage or something? How do we assure it's installed?

boot.script to go with the previous patch. Drop it in /boot.

+ ukfile=/boot/uImage-${kvers}
+ uifile=/boot/uInitrd-${kvers}

while this sounds like a good idea, I think this is too wide-ranging: nothing is going to cleanup the accumulated uImage/uInitrd files, and ABI changes are fairly frequent. I personally don't think we should add fancy features to Ubuntu's flash-kernel before we resync with Debian.

The rest looks fine, albeit I'm not sure whether it's a good idea to only check for "efikamx" kernels.

actually, I hadn't read the rest carefully enough:
+ eval "sed -i'.bak' -e's,%KERNELVERSION%,$kvers,g' $tmp.boot.script"
+ eval "sed -i'.bak' -e's,%ROOTPARTITION%,$rootfs,g' $tmp.boot.script"

it's not clear to me which piece of code will write the boot.script, nor why eval is needed.

When will this resync occur? I keep hearing it being thrown around a s a reason to not implement features but nothing seems to be being done to make it happen. The other reason we are proposing this change here is because linaro supports like 3 boards and Debian supports ~40. We can make the changes to Debians stuff but with no way to test them on all those different boards.

Sent from my iPhone

On Mar 6, 2011, at 6:39 PM, Loïc Minier <email address hidden> wrote:

> + ukfile=/boot/uImage-${kvers}
> + uifile=/boot/uInitrd-${kvers}
>
> while this sounds like a good idea, I think this is too wide-ranging:
> nothing is going to cleanup the accumulated uImage/uInitrd files, and
> ABI changes are fairly frequent. I personally don't think we should add
> fancy features to Ubuntu's flash-kernel before we resync with Debian.
>
> The rest looks fine, albeit I'm not sure whether it's a good idea to
> only check for "efikamx" kernels.
>
> --
> You received this bug notification because you are a direct subscriber
> of the bug.
> https://bugs.launchpad.net/bugs/671027
>
> Title:
> Add Efika MX Smartbook/Smarttop support
>
> To unsubscribe from this bug, go to:
> https://bugs.launchpad.net/efikamx/+bug/671027/+subscribe

Not being able to remove kernels generated is a flaw in the fundamental way flash-kernel is written. We discussed this. It needs to "act" like update-initramfs but it doesn't. Adding more and more hacks to work around it is just that - more and more hacks.

There is one reason and one reason only here that we do it this way and that is because having several kernel versions around is far more useful than it is not. The only reliable and fast way to boot a system and check that the images are correct, and correctly work around the absense of a ramdisk (which is entirely possible in development) is that 400 byte script (let's imagine this is 20 lines of hand-typed commands on a U-Boot shell to reproduce, which is cruel and unusual to inflict on developers).

To fix it by "not allowing more than one bootable kernel" is a hack to work around a missing, sorely needed feature implementation.

The reason I want this in there is because right now, we run Ubuntu. We do not run Debian. We need Ubuntu to work. We do not want to wait around while Canonical, Linaro or whatever gets their act together and decides to perform an ineffectual, lackadaisical rewrite which solves absolutely no problems whatsoever. We have had no problems in the 2 years of sales of people running out of disk space in /boot because of "leftover kernels". Right now, Ubuntu does not even SHIP an Efika kernel, and the code path can never be run on any board you really care about, so I don't see what the argument is.

We want this in because it means maintaining Ubuntu support for our board gets much easier, and we can support customers better, and then we can get on with the needful things which is what you are asking us to wait for. This is a workload issue.

To answer your sed question, it is easily answered if you understand what sed does. -i means "in place" and takes an argument to rename the file as a backup before it edits the file in place. I would think it is obvious what it is trying to do, what happens is basically sed takes the $tmp.boot.script file and replaces all instances of %KERNELVERSION% in it, in place. The original file, before this modification, is backed up to $tmp.boot.script.bak by sed. The second call replaces all instances of %ROOTPARTITION%, in place on the same file. It will overwrite the previous backup, however on failure you only need to see the last backup made to see if it did not make any of the pertinent variable changes when debugging.

The next lines take $tmp.boot.script (which has, by now, had two modifications made to it, and one backup is left) and make the boot.scr. These are the important ones.

It then cleans up the files including that leftover backup (you might want to remove that line if you want to debug it).

eval is required because without that (i.e. just writing it out as sed -i..) the shell will not perform variable substitution inside single quotes, which are the usual way of specifying sed arguments. sed will be running what is written out verbatim. We obviously want to replace the %VARIABLES% with $variables, hence the eval.

Without eval it would run

sed -i'.bak' -e's,%KERNELVERSION%,$kvers,g' $tmp.boot.script

With eval it ...

What I'm asking is for you guys to give us a break so we can move on to other things.. :)

Flash-Kernel NG is almost done, I just have to "port" a bunch of plugins and find the Debian maintainers so we can drive it upstream. Some of the things in there I am not sure are even used, and a lot of the boards are really weird.. I wonder if they are even used anymore...

On Mon, Mar 07, 2011, Matt Sealey wrote:
> The reason I want this in there is because right now, we run Ubuntu.

 Or rather you base on Ubuntu; let's call that some kind of mini Ubuntu
 derivative.

> We do not want to wait around
> while Canonical, Linaro or whatever gets their act together and decides
> to perform an ineffectual, lackadaisical rewrite which solves absolutely
> no problems whatsoever.

 I'm taking good note of your thoughts.

> Right now, Ubuntu does not even SHIP an Efika kernel, and the
> code path can never be run on any board you really care about, so I
> don't see what the argument is.

 You're incorrect, Ubuntu ships a Linaro kernel wich supports Efika
 smarttops right now, and I would hope will support smartbooks in the
 future.

> We want this in because it means maintaining Ubuntu support for our
> board gets much easier, and we can support customers better, and then we
> can get on with the needful things which is what you are asking us to
> wait for. This is a workload issue.

 By pushing your delta into Ubuntu, you are making it Ubuntu's problem;
 Ubuntu tries to stay close to Debian, by pushing more Ubuntu-specific
 delta, you're making it even harder for Ubuntu developers to resync
 with Debian.

> To answer your sed question, it is easily answered if you understand
> what sed does. -i means "in place" and takes an argument to rename the
> file as a backup before it edits the file in place. I would think it is
> obvious what it is trying to do, what happens is basically sed takes the
> $tmp.boot.script file and replaces all instances of %KERNELVERSION% in
> it, in place. The original file, before this modification, is backed up
> to $tmp.boot.script.bak by sed. The second call replaces all instances
> of %ROOTPARTITION%, in place on the same file. It will overwrite the
> previous backup, however on failure you only need to see the last backup
> made to see if it did not make any of the pertinent variable changes
> when debugging.

 I was asking about eval specifically; I don't think eval is needed,
 it's generally dangerous and I prefer avoiding it.

> eval is required because without that (i.e. just writing it out as sed
> -i..) the shell will not perform variable substitution inside single
> quotes, which are the usual way of specifying sed arguments. sed will be
> running what is written out verbatim. We obviously want to replace the
> %VARIABLES% with $variables, hence the eval.

 Using eval means that your single quotes gets stripped, and you're not
 in the "usual way of specifying sed arguments" anymore; try:
    eval sed 's/ //'

 You should use double-quotes instead, which is the far more common way
 to do variable substitutions with sed:
    sed "s/foo/$bar/"
 obviously, you have to take care of properly quoting within
 double-quotes, but it's less cumbersome than dealing with the
 two-levels of quoting required for an eval.

> Without eval it would run
> sed -i'.bak' -e's,%KERNELVERSION%,$kvers,g' $tmp.boot.script

 sed -i'.bak' "s,%KERNELVERSION%,$kvers,g" "$tmp.boot.script"

> I am rewriting it (flash-kernel as a concept) now, so that won't be too...

I removed the task on the upstream project as it doesn't track bugs in Launchpad (but in Debian) and uploaded the original proposal of not checking subarch (initial version of the code).

Yes, we run what you might consider a fork of Maverick with about 5 packages different, and about 10 on top.. but the fact is we use the vast majority of the Ubuntu archive for the convenience of our customers. The less we change the better the board gets supported. If Ubuntu wants to support it or Linaro wants to support it, fine, and we will eventually push to upstream, but the real concept to grasp here is supporting users, and doing the needful things before the "ethically correct" things. Patch now, upstream later. This is not a security bug that needs full disclosure and full cooperation, it's a patch you picked that was broken, which I've fixed and aligned with the support we coded in the first place (which is the *vendor's intent of kernel behavior*)

I don't think the eval is dangerous at all, we know the content of the variable, we know that one of them is going to have slashes in it (hence the comma seperators) and we don't want bash to evaluate too much. I worked this out, this is the best way. If you can get it to work another way, please test it and go ahead, but every test I ran... didn't work. Bash either didn't replace the variable or sed complained. I am well aware of the problems of passing unsanitized, unchecked user arguments to eval, but it can also be said that $(readlink /dev/root) is pretty hard to fake, and not a very common vector to break. In reality we should be using flash_kernel_set_root hook, but in this case, the data is pretty well checked for us too. There is no problem with eval here. I considered the implications, and only used it as a last resort when every other idea fell flat.

The behavior for multiple kernel versions I think should be standard, and it's also the *current* behavior of the script we replace in our custom flash-kernel and ubiquity builds to support our board. Neither way is "more correct", but I prefer that we continue with the seperate versions. Since neither Ubuntu nor Genesi is going to update kernels or change ABIs with that regularity (through lack of support and simple lack of new kernels) it's not going to affect anything.

Understand your concern about "making it Ubuntu's problem", but either way a package that is in the Ubuntu archive marked as being built by Genesi, still becomes Ubuntu's problem for users. Not everyone has a great knowledge of packaging policy, versioning or who to really talk to - we encourage our users to report bugs to the efikamx project first, and I personally assign them to bugs upstream where needed. This is not a bug in user support, it's just a fact of life.

I should also mention, let me reiterate the first comment in this bug, and our experience with the Linaro "kernel support", that the supplied boot scripts were totally incorrect behavior for our board. Nobody consulted us on this. Consider this consulting work for Linaro - we fixed the boot script to do the correct behavior, and this is how we want our kernels on disk. We will figure out a better way to delete kernels later, in the meantime, we ship to users with a 128MB ext3 /boot and running out has never been a problem.