On 14.11.2013, at 08:30, Guillaume Gardet <[email protected]> wrote:

> 
> Le 14/11/2013 14:10, Alexander Graf a écrit :
>> 
>> Am 14.11.2013 um 05:03 schrieb Guillaume Gardet <[email protected]>:
>> 
>>> Le 12/11/2013 13:49, Alexander Graf a écrit :
>>>> 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 :).
>>> I wanted to default to zImage as most images are zImage now. Did I drop 
>>> uImage support with this modification?
>> Well, I don't see sny target setting this to uImage. But maybe I just missed 
>> it :).
>> 
>> I think it makes sense to go with zImage brw, but I'm not 100% sure all out 
>> u-boot versions are capable of it.
> 
> I reset it to uImage ATM. We should not break more our images now. We could 
> work on that on Factory later. ;)
> 
>> 
>>>>>   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.
>>> My fault. We could use loadfdt as upstream does.
>> Works for me :)
> 
> SR #206890

Thanks :). I accepted it.


Alex

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

Reply via email to