[U-Boot] [PATCH V2 4/4] configs: ti_armv7_keystone2: start using armv7_common

[Murali Karicheri]

On 07/17/2015 12:52 PM, Nishanth Menon wrote:
> On 07/17/2015 11:04 AM, Murali Karicheri wrote:
>> On 07/16/2015 03:08 PM, Nishanth Menon wrote:
>>> Try to maintain as much commonality by conditionally including stuff
>>> in armv7_common as necessary and removing the common defines from
>>> keystone2 header.
>>>
>> Including the common ti_armv7_common.h for keystone also add duplication
>> of the various addresses
>>
>> #define DEFAULT_LINUX_BOOT_ENV \
>>      "loadaddr=0x82000000\0" \
>>      "kernel_addr_r=0x82000000\0" \
>>      "fdtaddr=0x88000000\0" \
>>      "fdt_addr_r=0x88000000\0" \
>>      "rdaddr=0x88080000\0" \
>>      "ramdisk_addr_r=0x88080000\0" \
>>      "bootm_size=0x10000000\0"
>>
>> Some of these are also defined in keystone common file. The env scripts
>> for keystone to be reworked to use the common variable above.
>>
>> Rework the CONFIG_EXTRA_ENV_KS2_BOARD_SETTINGS to include common as well.
>
> we need to cleanup all the variables  once we get the distro config

What do you mean by distro config? Could you explain?

> included in anyways... I had decided not to rock the apple cart too much
> with this patch -> just the basic consolidation with as minimal changes
> as necessary. inclusion of DEFAULT_LINUX_BOOT_ENV into keystone2.h can
> be done as a follow on patch.

Probably not this one. User would see both these variables and will 
cause confusion and should be fixed.

>
>> Did you do a test with env default -f -a with this to check if it
>> continues to work for Keystone Linux boot.
>>
>
> after the series: http://pastebin.ubuntu.com/11893531/
> before the series: http://pastebin.ubuntu.com/11893576/
> deltas:
> bootdelay is 1 now
> additional definitions:
> arch=arm
> soc=keystone    	
> vendor=ti
> cpu=armv7
> board=ks2_evm
> board_name=ks2_evm
>
> will be great to get a tested by on that. Nothing else seems to have
> changed.
>
>>>
>>> diff --git a/include/configs/k2e_evm.h b/include/configs/k2e_evm.h
>>> index ac50a01b2980..f1e650141ae1 100644
>>> --- a/include/configs/k2e_evm.h
>>> +++ b/include/configs/k2e_evm.h
>>> @@ -15,8 +15,6 @@
>>>    #define CONFIG_K2E_EVM
>>>
>>>    /* U-Boot general configuration */
>>> -#define CONFIG_SYS_PROMPT               "K2E EVM # "
>>
>> Why remove this?
>
> arm_v7_common defines just "u-boot#" for all SoC and boards. So, we dont
> need this.

Sorry, this may be needed from the automation perspective. Also for 
backward compatibility for users. Would like to keep for K2.

>
>>>    #define CONFIG_EXTRA_ENV_KS2_BOARD_SETTINGS                \
>>>        "addr_mon=0x0c140000\0"                        \
>>>        "args_ubi=setenv bootargs ${bootargs} rootfstype=ubifs "    \
>>> diff --git a/include/configs/ti_armv7_common.h
>>> b/include/configs/ti_armv7_common.h
>>> index 63244dbc83ff..814da3409c46 100644
>>> --- a/include/configs/ti_armv7_common.h
>>> +++ b/include/configs/ti_armv7_common.h
>>> @@ -73,9 +73,13 @@
>>>    #ifndef CONFIG_NR_DRAM_BANKS
>>>    #define CONFIG_NR_DRAM_BANKS        1
>>>    #endif
>>> +
>> Why these extra spaces?
>
> Thanks.. will drop.
>
>> I assume the one you have removed is already part of
>> include/configs/ti_armv7_keystone2.h.
>
> for i in `git grep "^#define" include/configs/ti_armv7_keystone2.h|sed
> -e "s/\s\s*/ /g"|cut -d ' ' -f2|sort|uniq`; do k=`git grep $i
> include/configs/ti_armv7_common.h`; if [ -n "$k" ]; then echo $i; fi; done
>
> Then started cleaning them up.
>
>
>


-- 
Murali Karicheri
Linux Kernel, Keystone