[U-Boot] [PATCH 1/1] imx-common: cpu: add fdt_file environment variable

[Troy Kisky]

On 3/31/2014 12:47 PM, Marek Vasut wrote:
> On Monday, March 31, 2014 at 09:38:02 PM, Otavio Salvador wrote:
>> On Mon, Mar 31, 2014 at 4:22 PM, Marek Vasut <marex at denx.de> wrote:
>>> On Monday, March 31, 2014 at 08:36:55 PM, Troy Kisky wrote:
>>>> On 3/29/2014 4:11 PM, Eric Nelson wrote:
>>>>> Hi Troy,
>>>>>
>>>>> On 03/29/2014 03:34 PM, Troy Kisky wrote:
>>>>>> This removes one block in the move toward 1 u-boot
>>>>>> for both a mx6q (quad) and mx6dl (duallite) processor.
>>>>>>
>>>>>> Now fdt_file hardcoded value can be removed.
>>>>>>
>>>>>> Signed-off-by: Troy Kisky <troy.kisky at boundarydevices.com>
>>>>>> ---
>>>>>>
>>>>>>   arch/arm/imx-common/cpu.c | 44
>>>>>>   ++++++++++++++++++++++++++++++++++++++++++++ arch/arm/lib/board.c
>>>>>>   
>>>>>>    |  7 +++++++
>>>>>>   
>>>>>>   2 files changed, 51 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm/imx-common/cpu.c b/arch/arm/imx-common/cpu.c
>>>>>> index a77c4de..5d48011 100644
>>>>>> --- a/arch/arm/imx-common/cpu.c
>>>>>> +++ b/arch/arm/imx-common/cpu.c
>>>>>> @@ -180,3 +180,47 @@ void arch_preboot_os(void)
>>>>>>
>>>>>>       ipuv3_fb_shutdown();
>>>>>>   
>>>>>>   }
>>>>>>   #endif
>>>>>>
>>>>>> +
>>>>>> +const char *get_dtb_prefix(u32 imxtype)
>>>>>> +{
>>>>>> +    switch (imxtype) {
>>>>>> +    case MXC_CPU_MX6Q:
>>>>>> +    case MXC_CPU_MX6D:
>>>>>> +        return "imx6q";        /* Quad/Dual-core version of the mx6
>>>>>> */ +    case MXC_CPU_MX6DL:
>>>>>> +    case MXC_CPU_MX6SOLO:
>>>>>> +        return "imx6dl";    /* Dual Lite/Solo version of the mx6 */
>>>>>> +    case MXC_CPU_MX6SL:
>>>>>> +        return "imx6sl";    /* Solo-Lite version of the mx6 */
>>>>>> +    case MXC_CPU_MX51:
>>>>>> +        return "imx51";
>>>>>> +    case MXC_CPU_MX53:
>>>>>> +        return "imx53";
>>>>>> +    }
>>>>>> +    return "??";
>>>>>> +}
>>>>>> +
>>>>>
>>>>> I really dislike this implementation of naming policy in code.
>>>>
>>>> It is not truly a policy. It is a convenience which can be ignored
>>>> if so desired. Though I do agree that cpu and board environment
>>>> variables would also be useful. Still, a cpu variable would still
>>>> require some scripting to combine the quad/dual, duallite/solo. So,
>>>> your way is not as convenient for dtb file names.
>>>
>>> You can make the CPU code set the CPU type into some variable.
>>> Afterwards, some scripts in the HUSH can assemble the DTB name based on
>>> those variables. This would be much more generic and re-usable than this
>>> ...
>>
>> The problem is this script will be mostly the same for most boards.
> 
> This does by no means justify poluting code with non-reusable convoluted stuff. 
> A script which is part of the environment can be tweaked on a running system as 
> seen fit at any later date, updating bootloader on a running system is often not 
> an option.
> 
> Furthermore, having partial information which can be used for decisionmaking is 
> much better than having a lengthy string which needs to be parsed first. 
> Especially with the limited parsing options we have in some configurations of U-
> Boot.
> 

I can agree to that, if I understand you correctly. So are you fine with having a board and
cpu variable? If so, what should the cpu variable contain?



I propose cpu defaults to "IMX%s", get_imx_type(imxtype)

so cpu=IMX6Q, IMX6D, IMX6DL, IMX6SOLO, IMX6SL, IMX51, IMX53

and board defaults to plain CONFIG_SYS_BOARD. So, mx6sabresd
would need to set board=sabresd.

in mx6sabresd.h
setenv board=sabresd

and in some common file
setenv dtbpIMX6Q setenv dtbprefix imx6q
setenv dtbpIMX6D setenv dtbprefix imx6d
setenv dtbpIMX6DL setenv dtbprefix imx6dl
setenv dtbpIMX6SOLO setenv dtbprefix imx6dl
setenv dtbpIMX6SL setenv dtbprefix imx6sl
setenv find_dtb_file 'run dtbp${cpu} ; setenv fdt_file $dtbprefix-$board'

run find_dtb_file
echo $fdt_file


--------
Or if you know a better way please speak up.

Thanks
Troy