Cool, thanks a lot for thd patch! See a few comments inline.

Am 11.11.2013 um 05:18 schrieb <[email protected]>:

> 
> 
>   home:Guillaume_G:branches:openSUSE:13.1:Ports/JeOS -> 
> openSUSE:13.1:Ports/JeOS
> 
> 
>   https://build.opensuse.org/request/show/206130
> 
>   Description: * Various fixes:
> - rename loadfdt (already used differently upstream) in should_load_fdt 
> - rename usefdt in should_use_fdt
> - Undef $bootpart before using it in for loop otherwise var is not updated 
> (bug or feature?)
> 
> ++++++ uboot-image-arndale-setup
> --- uboot-image-arndale-setup
> +++ uboot-image-arndale-setup
> @@ -64,7 +64,7 @@
>     # mkinitrd to point to the kernel and initrd in order
>     # to support kernel updates
>     # ----
> -    kernel=uImage
> +    kernel=zImage

Don't we need uImages at all anymore? That change is not documented in the 
description btw :).

>     initrd=initrd
> 
>     # gather kernel command line
> @@ -100,7 +100,7 @@
> case $flavor in
>   highbank)
>     bootdevs=scsi
> -    usefdt=1
> +    should_use_fdt=1
>     setdev=1
>     # Highbank's u-boot already prepends 0x to the file size
>     sizeprefix=
> @@ -114,30 +114,30 @@
>     boottype=bootm
>     kerneladdr=0x48000000
>     ramdiskaddr=0x43100000
> -    loadfdt=1
> -    fdt=$flavor.bin
> +    should_load_fdt=1
> +    fdtfile=$flavor.bin
>     fdt_addr=0x43000000
>     ;;
> 
>   loco)
> -    usefdt=1
> -    loadfdt=1
> +    should_use_fdt=1
> +    should_load_fdt=1
>     units="0 1"
> -    fdt=imx53-qsb.dtb
> +    fdtfile=imx53-qsb.dtb
>     ;;
>   arndale)
>     boottype=bootm
> -    usefdt=1
> -    loadfdt=1
> -    fdt=exynos5250-arndale.dtb
> +    should_use_fdt=1
> +    should_load_fdt=1
> +    fdtfile=exynos5250-arndale.dtb
>     fdt_addr=0x44000000
>     ;;
>   chromebook)
>     bootdevs="mmc usb"
>     bootparts='${unit}:2'
> -    usefdt=1
> -    loadfdt=1
> -    fdt=exynos5250-snow.dtb
> +    should_use_fdt=1
> +    should_load_fdt=1
> +    fdtfile=exynos5250-snow.dtb
>     ;;
> esac
> 
> @@ -150,9 +150,9 @@
> fdt_high=0xffffffff
> 
> # copy bash variables into boot script
> -for variable in kernel initrd fdt flavor target bootargs bootdevs bootparts \
> -                initrd_high fdt_high boottype kerneladdr ramdiskaddr usefdt \
> -                loadfdt fdt_addr setdev units sizeprefix; do
> +for variable in kernel initrd fdtfile flavor target bootargs bootdevs 
> bootparts \
> +                initrd_high fdt_high boottype kerneladdr ramdiskaddr 
> should_use_fdt \
> +                should_load_fdt fdt_addr setdev units sizeprefix; do
>     value=$(eval "echo $(echo \$$variable)")
>     # only set variables that contain data, leave the others alone
>     if [ "$value" ]; then
> @@ -165,18 +165,18 @@
>    printenv ramdiskaddr|| setenv ramdiskaddr ${ramdisk_addr_r}
>    setenv load_kernel 'ext2load ${bootdev} ${bootpart} ${kerneladdr} 
> ${kernel}'
>    setenv load_initrd 'ext2load ${bootdev} ${bootpart} ${ramdiskaddr} 
> ${initrd}; setenv rd_filesize ${sizeprefix}${filesize}'
> -    if itest 1${loadfdt} == 11; then
> -      setenv load_fdt 'ext2load ${bootdev} ${bootpart} ${fdt_addr} ${fdt}'
> +    if itest 1${should_load_fdt} == 11; then

This makes sense

> +      setenv should_load_fdt 'ext2load ${bootdev} ${bootpart} ${fdt_addr} 
> ${fdtfile}'

Here the should is a misnomer, sorry. It should rather be 'do_load_fdt' ir 
something similar, as the contents of that variable are going to be executed 
later. We should give it a name that makes its function obvious. Sorry for 
misguiding you on it earlier.


Alex

--
To unsubscribe, e-mail: [email protected]
To contact the owner, e-mail: [email protected]

Reply via email to