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

[Marek Vasut]

On Monday, March 31, 2014 at 11:09:14 PM, Troy Kisky wrote:
> 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?

Looks reasonable.

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

This should be introduced and work for _all_ CPUs and must not be imx specific. 
Otherwise, for i.MX, you can pick whatever is suitable, IMX%s is OK.

> 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.

Again, should work across all boards.

> in mx6sabresd.h
> setenv board=sabresd
> 
> and in some common file
> setenv dtbpIMX6Q setenv dtbprefix imx6q

This must be part of a per-board environment if and only if the board can 
contain multiple CPU configurations.
[...]