On Wed, Sep 29, 2010, Shawn Guo wrote:
I made a l-m-c patch to add imx51 support. I need your help to review and merge the code.
Thanks!
You probably want to use the board name, mx51evk, not the SoC name, imx51; this also avoids mixing mx51 (kernel flavor) and imx51 (DEVNAME) in the script.
install_hwpack() {
- ensure_command qemu-arm-static qemu-arm-static
- # Check host architecture
- arch_is_arm=false
- arch_str=`uname -m`
- if [[ "$arch_str" == arm* ]]; then
- arch_is_arm=true
- fi
- if [ ! $arch_is_arm ]; then
These [ ! false ] or [ ! true ] are incorrect; [ ! anything ] is always false; "!" tests for non-empty.
I suggest you move the ensure_command to the place where qemu is copied and change it into something less bash-specific for readability: local arch_is_arm=no case `uname -m` in arm*) arch_is_arm=yes ensure_command qemu-arm-static qemu-arm-static sudo cp /usr/bin/qemu-arm-static ${chroot}/usr/bin ;; esac [...] if [ "arch_is_arm" = no ]; then sudo rm -f ${chroot}/usr/bin/qemu-arm-static fi
@@ -371,10 +385,17 @@
# Create a VFAT or FAT16 partition of 9 cylinders which is about 64M # and a linux partition of the rest
- if [ "$DEVIMAGE" = imx51 ]; then
- sudo sfdisk -D -H $HEADS -S $SECTORS $CYLINDER_ARG $partdev << THEEND
+1,9,$PARTITION_TYPE,* +10,,,- +THEEND
- else sudo sfdisk -D -H $HEADS -S $SECTORS $CYLINDER_ARG $partdev << THEEND
,9,$PARTITION_TYPE,* ,,,- THEEND
- fi
Could we just use the same partition layout for all images (the imx51 one), avoiding the if entirely?
Not your doing, but the sfdisk man page recommends avoiding the c,h,s specs especially since -H/-S are passed already
If you're using the early data on the image to store stuff, I recommend you protect it with a dumb partition (e.g. "non-FS data" type, 0xda).
- imx51)
sudo dd if=binary/usr/lib/u-boot/mx51evk/u-boot.imxof=/dev/mmcblk0 bs=512 seek=2
bs=1024 seek=1?
I definitely think you want a non-FS data partition protecting this zone
sync
Do you really need a sync in there? There are a bunch of syncs in the script already
sudo mkimage -A arm -O linux -T kernel -C none -a 0x90008000 -e0x90008000 \
-n Linux -d "${DIR}/binary/${parts_dir}"/vmlinuz*-mx51"${BOOT_DISK}/uImage"
sudo mkimage -A arm -O linux -T ramdisk -C none -a 0 \-e 0 -n initramfs \-d "${DIR}/binary/${parts_dir}"/initrd.img-*-linaro-mx51 \"${BOOT_DISK}/uInitrd"
Could you use -linaro-mx51 for the vmlinuz pattern as well? Ideally, please wrap this at 80 chars like the rest of the code now.
cat > ${TMP_DIR}/boot.cmd << BOOTCMD+setenv bootcmd 'fatload mmc 0:1 0x90000000 uImage; fatload mmc 0:1 0x90800000 uInitrd; bootm 0x90000000 0x90800000'
no mmc init?
+setenv bootargs '${serial_opts} ${splash_opts} ${lowmem_opt} ${boot_snippet} rootwait ro' +boot +BOOTCMD
would be great if you could merge this with the create_boot_cmd() logic
sudo mkimage -A arm -O linux -T script -C none -a 0 \-e 0 -n "$CODENAME 10.05" -d "${TMP_DIR}/boot.cmd" \"${BOOT_DISK}/boot.scr";;
the -n stuff needs to be updated
Otherwise looks good, thanks again!