update_kernel: first cut at arm support

update_kernel: first cut at arm support

Change-Id: I5dc22223559d3cdad357530af66c14115da63c89

BUG=n0ne
TEST=update_kernel to seaboard (and mario for regression test)

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=f53fa0d

[Kenneth Waters, 2010-12-23 18:14:06]

Seems reasonable to me.  I'd like msb to give it a look as he's the original
author.

http://codereview.chromium.org/6031005/diff/1/update_kernel.sh
File update_kernel.sh (right):

http://codereview.chromium.org/6031005/diff/1/update_kernel.sh#newcode31
update_kernel.sh:31: info "Target reports root device is ${FLAGS_device}"
This is a bit misleading.  It doesn't learn the device name per-say (i.e. it
reports mmc0 and mmc0p).  I think a comment to that effect at the least is a
good idea.

http://codereview.chromium.org/6031005/diff/1/update_kernel.sh#newcode55
update_kernel.sh:55: --vmlinuz=/build/${FLAGS_board}/boot/vmlinux.uimg --to
new_kern.bin"
The quoting it burns.  Maybe this one instead?  It's ugly too so it's up to you

cmd="./build_kernel_image.sh --arch=arm \
     --root='"'/dev/${devname}${rootpart}'"' \
     --vmlinuz=/build/${FLAGS_board}/boot/vmlinux.uimg --to new_kern.bin"

http://codereview.chromium.org/6031005/diff/1/update_kernel.sh#newcode58
update_kernel.sh:58: cmd="vbutil_kernel --pack new_kern.bin \
I was somewhat upset that this didn't use build_kernel_image.sh in _all_ cases,
when it was originally written.  Does it make sense to change it so that's the
case now?

http://codereview.chromium.org/6031005/diff/1/update_kernel.sh#newcode135
update_kernel.sh:135: main $@
I know this isn't your mistake but this needs to be

main "$@"

[Olof Johansson, 2010-12-23 18:48:32]

http://codereview.chromium.org/6031005/diff/1/update_kernel.sh
File update_kernel.sh (right):

http://codereview.chromium.org/6031005/diff/1/update_kernel.sh#newcode31
update_kernel.sh:31: info "Target reports root device is ${FLAGS_device}"
Sure, will do

http://codereview.chromium.org/6031005/diff/1/update_kernel.sh#newcode55
update_kernel.sh:55: --vmlinuz=/build/${FLAGS_board}/boot/vmlinux.uimg --to
new_kern.bin"
Yeah, it's bad. Yours is a little less nasty though, I'll switch to that.

http://codereview.chromium.org/6031005/diff/1/update_kernel.sh#newcode58
update_kernel.sh:58: cmd="vbutil_kernel --pack new_kern.bin \
msb, care to comment? I didn't go down that route since I wasn't sure if it had
already been contemplated and rejected.

http://codereview.chromium.org/6031005/diff/1/update_kernel.sh#newcode135
update_kernel.sh:135: main $@
Will do.

[Mandeep Singh Baines, 2010-12-23 21:34:19]

LGTM once kwaters concerns are addressed.

http://codereview.chromium.org/6031005/diff/1/update_kernel.sh
File update_kernel.sh (right):

http://codereview.chromium.org/6031005/diff/1/update_kernel.sh#newcode58
update_kernel.sh:58: cmd="vbutil_kernel --pack new_kern.bin \
On 2010/12/23 18:48:33, Olof Johansson wrote:
> msb, care to comment? I didn't go down that route since I wasn't sure if it
had
> already been contemplated and rejected.

Never considered it. If it works, let's do it. Would be nice to get rid of the
if.

[Olof Johansson, 2011-01-21 00:14:06]

I addressed Ken's comments from the first round, and updated to current base.
Since Mandeep did changes to let it run inside chroot, some of the burning
quoting went away.

I didn't move over the x86 piece yet -- it'll probably take some changes to
vbutil to take and process a config.txt, not just produce it.

[Mandeep Singh Baines, 2011-01-21 21:59:09]

Heads up. Anush is in the process of moving this file.

http://codereview.chromium.org/6031005/diff/7001/update_kernel.sh
File update_kernel.sh (right):

http://codereview.chromium.org/6031005/diff/7001/update_kernel.sh#newcode107
update_kernel.sh:107: sudo tar -C /build/"${FLAGS_board}"/lib/modules -cjf
/tmp/new_modules.tar .
Maybe scp the tar file to the target to avoid running sudo locally?

http://codereview.chromium.org/6031005/diff/7001/update_kernel.sh#newcode117
update_kernel.sh:117: sudo tar -C /build/"${FLAGS_board}"/lib/firmware -cjf
/tmp/new_firmware.tar .
Maybe scp the tar file to the target to avoid running sudo locally?

[Olof Johansson, 2011-01-26 00:57:05]

Removed sudos, writing the tarballs to /tmp means they're not needed.

PTAL.

[Mandeep Singh Baines, 2011-01-26 20:33:53]

LGTM