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

[Murali Karicheri]

On 07/17/2015 01:25 PM, Nishanth Menon wrote:
> On 07/17/2015 12:11 PM, Murali Karicheri wrote:
>> 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?
>
> include/config_distro* - both will eventually get integrated into
> armv7_common.h to benefit all TI SoC platforms.

Ok I see. But that doesn't mean we can accept duplicate env variables.
Do you see my point?

>
>>
>>> 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.
>>
>
> they are no variables, they are defines. they will eventually be fixed.

The defines finally create env variables on NAND or any other medium :)
So my comment stays.

> I am leery to make a huge jump on a single series. lets do consolidation
> in small baby steps please.
>
> as I have already shown in my reply - the status quo is being maintained
> and we are one step closer to an common framework.

Ok. If this happens soon (within a month) I am fine with this. 
Otherwise, this could go under the radar and cause maintenance issue.

>
>>>
>>>>>
>>>>> 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.
>
> This has been beaten to death in the past as well (I think some 3/4
> years ago.. i think it should be in the archives, just too lazy to dig
> through multi-year old discussions)..
>
> I will let Tom comment more here. My understanding is that the
> convention followed by all other TI SoCs will imply not having custom
> sys_prompt..
>
>
> That said, custom sys_prompt is NOT a need from automation perspective -
> we have all our boards in automation farm for years now with and without
> custom boot prompt. board identification can be done in other ways.
 From a users perspective as well, it is good to know which board I am 
working with when I work with multiple boards. Not sure what is the 
argument not to have a board specific prompt at u-boot level. Definitely 
like to hear from Tom.

Murali

>
>


-- 
Murali Karicheri
Linux Kernel, Keystone