add install to arm platform

add install to arm platform

now we could install to mmcblk0.
for vogue, we even change the uboot environment.
on Samsung, you had to use 'chromeos-install --skip_src_removable' because mmcblk1 is not removable somehow.

[jiesun, 2010-04-10 00:03:28]

adapt chromeos-install for arm platform.
thanks piman's device name manipulation functions.

[piman, 2010-04-10 00:50:35]

Adding Bill because he may have some comments.

http://codereview.chromium.org/1618013/diff/1/3
File src/platform/installer/chromeos-install (right):

http://codereview.chromium.org/1618013/diff/1/3#newcode49
src/platform/installer/chromeos-install:49: echo ${SRC}
remove debugging output

http://codereview.chromium.org/1618013/diff/1/3#newcode54
src/platform/installer/chromeos-install:54: #exit 1
why commenting this ?

http://codereview.chromium.org/1618013/diff/1/3#newcode203
src/platform/installer/chromeos-install:203: install_gpt $FLAGS_dst $ROOTFS_IMG
$KERNEL_IMG $STATEFUL_IMG /dev/zero $ESP_IMG
80 chars

http://codereview.chromium.org/1618013/diff/1/3#newcode221
src/platform/installer/chromeos-install:221: if [ -f "$MKIMAGE".gz ]; then
you don't need this block

http://codereview.chromium.org/1618013/diff/1/3#newcode230
src/platform/installer/chromeos-install:230: if="$MBR_SCRIPT_UIMG" of="$MBR_IMG"
conv=notrunc
you can actually do all this after the next install_gpt, by passing there an
empty MBR, and then dd the computed one at the beginning of ${SRC}

http://codereview.chromium.org/1618013/diff/1/3#newcode231
src/platform/installer/chromeos-install:231: hexdump -v -C "$MBR_IMG"
you can skip the hexdump

[Bill Richardson, 2010-04-10 01:46:30]

Yes, I have a few comments. Thanks.

http://codereview.chromium.org/1618013/diff/1/3
File src/platform/installer/chromeos-install (right):

http://codereview.chromium.org/1618013/diff/1/3#newcode236
src/platform/installer/chromeos-install:236: PMBRCODE=${MBR_IMG}
All of the preceding code to create the PMBR (or something much like it) is in
src/scripts/build_gpt.sh. It should be cut out of there (and here) and put into
chromeos-common, so that we can generate the correct U-Boot MBR script from both
the installer and when we build the image the first time.

http://codereview.chromium.org/1618013/diff/1/3#newcode320
src/platform/installer/chromeos-install:320: uboot-env.py -f /dev/mtdblock0 -o
0x7c000 -s 0x4000 \
python is not available on the target device. What is this 'uboot-env.py'
script?

[piman, 2010-04-10 01:53:56]

On Fri, Apr 9, 2010 at 6:46 PM, <wfrichar@chromium.org> wrote:

> Yes, I have a few comments. Thanks.
>
>
>
>
> http://codereview.chromium.org/1618013/diff/1/3
> File src/platform/installer/chromeos-install (right):
>
> http://codereview.chromium.org/1618013/diff/1/3#newcode236
> src/platform/installer/chromeos-install:236: PMBRCODE=${MBR_IMG}
> All of the preceding code to create the PMBR (or something much like it)
> is in src/scripts/build_gpt.sh. It should be cut out of there (and here)
> and put into chromeos-common, so that we can generate the correct U-Boot
> MBR script from both the installer and when we build the image the first
> time.
>
> http://codereview.chromium.org/1618013/diff/1/3#newcode320
> src/platform/installer/chromeos-install:320: uboot-env.py -f
> /dev/mtdblock0 -o 0x7c000 -s 0x4000 \
> python is not available on the target device. What is this
> 'uboot-env.py' script?


This is a temporary hack for Samsung only - and the script depends on
python, so we make sure it is installed.
Once we have a working u-boot on Samsung, both that script and the need for
it will disappear


>
>
> http://codereview.chromium.org/1618013/show
>

[jiesun, 2010-04-12 18:34:38]

please have another look

http://codereview.chromium.org/1618013/diff/1/3
File src/platform/installer/chromeos-install (right):

http://codereview.chromium.org/1618013/diff/1/3#newcode49
src/platform/installer/chromeos-install:49: echo ${SRC}
On 2010/04/10 00:50:36, piman wrote:
> remove debugging output

Done.

http://codereview.chromium.org/1618013/diff/1/3#newcode54
src/platform/installer/chromeos-install:54: #exit 1
On 2010/04/10 00:50:36, piman wrote:
> why commenting this ?
because /sys/block/mmcblk1/removable = 0
otherwise you had to use 'chromeos_install --skip_src_removable'

http://codereview.chromium.org/1618013/diff/1/3#newcode203
src/platform/installer/chromeos-install:203: install_gpt $FLAGS_dst $ROOTFS_IMG
$KERNEL_IMG $STATEFUL_IMG /dev/zero $ESP_IMG
On 2010/04/10 00:50:36, piman wrote:
> 80 chars

Done.

http://codereview.chromium.org/1618013/diff/1/3#newcode221
src/platform/installer/chromeos-install:221: if [ -f "$MKIMAGE".gz ]; then
On 2010/04/10 00:50:36, piman wrote:
> you don't need this block

Done.

http://codereview.chromium.org/1618013/diff/1/3#newcode230
src/platform/installer/chromeos-install:230: if="$MBR_SCRIPT_UIMG" of="$MBR_IMG"
conv=notrunc
On 2010/04/10 00:50:36, piman wrote:
> you can actually do all this after the next install_gpt, by passing there an
> empty MBR, and then dd the computed one at the beginning of ${SRC}
you mean DST, right?

http://codereview.chromium.org/1618013/diff/1/3#newcode231
src/platform/installer/chromeos-install:231: hexdump -v -C "$MBR_IMG"
On 2010/04/10 00:50:36, piman wrote:
> you can skip the hexdump

Done.

http://codereview.chromium.org/1618013/diff/1/3#newcode236
src/platform/installer/chromeos-install:236: PMBRCODE=${MBR_IMG}
On 2010/04/10 01:46:30, Bill Richardson wrote:
> All of the preceding code to create the PMBR (or something much like it) is in
> src/scripts/build_gpt.sh. It should be cut out of there (and here) and put
into
> chromeos-common, so that we can generate the correct U-Boot MBR script from
both
> the installer and when we build the image the first time.

I had moved bulk of code to chromeos_common.sh, but do you mind I change
build_gpt path later, because it involve more time-consuming testing. Also I am
not sure about the file as output image path.

http://codereview.chromium.org/1618013/diff/1/3#newcode320
src/platform/installer/chromeos-install:320: uboot-env.py -f /dev/mtdblock0 -o
0x7c000 -s 0x4000 \
On 2010/04/10 01:46:30, Bill Richardson wrote:
> python is not available on the target device. What is this 'uboot-env.py'
> script?
> 
it write the NOR flash to update the Uboot environments. I do not think it is
mandetory if you are able to access debug  serial. but it is convenient for now.

[Bill Richardson, 2010-04-12 20:26:20]

I'd really prefer that you NOT split up common code. I'd also prefer that this
change wait another couple of days, because I need to get some more GPT changes
in before the new release tomorrow, and that will affect the same set of files.

[adlr, 2010-04-12 20:31:52]

if you are waiting on bill, you might want to wait on this comment b/c it will
make merging in other changes harder.

http://codereview.chromium.org/1618013/diff/10001/11002
File src/platform/installer/chromeos-install (right):

http://codereview.chromium.org/1618013/diff/10001/11002#newcode233
src/platform/installer/chromeos-install:233: # We can't guarantee that the
kernel will see the new partition table, so we
indent this

[jiesun, 2010-04-12 21:51:45]

Hello, All, thanks for the review.
Bill: I had unfork the make mbr code and tested on vogue platform. During our
conversation, it seems that you had not begin to implement the new features. I
really would like to check this in if possible. because further merge require
more time-consuming testing. If you had not test your new feature, it maybe make
sense to test them together. I assumed merge code is fast than doing the test.
How do you think?

[piman, 2010-04-12 21:59:01]

LGTM with couple of nits.

http://codereview.chromium.org/1618013/diff/4002/15002
File src/platform/installer/chromeos-install (right):

http://codereview.chromium.org/1618013/diff/4002/15002#newcode236
src/platform/installer/chromeos-install:236: # left over from the last install,
and we don't want that either. So any access
80 chars

http://codereview.chromium.org/1618013/diff/4002/15002#newcode237
src/platform/installer/chromeos-install:237: # that we need to do to the
destination partitions will have to go through loop
80 chars

[Bill Richardson, 2010-04-12 23:21:43]

Thanks for making that change.

LGTM!