[Patch] Add imx51 support into linaro-media-create

Loïc Minier loic.minier at linaro.org
Wed Sep 29 13:28:44 UTC 2010


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.imx
> of=/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 -e
> 0x90008000 \
> +           -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!

-- 
Loïc Minier



More information about the linaro-dev mailing list