From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Fri, 15 Nov 2013 00:30:12 +0100
Subject: [U-Boot] [PATCH V2] imx: Define common routines to set cpu and
	board environment variables
In-Reply-To: <528542BE.8010703@boundarydevices.com>
References: <1384391809-9339-1-git-send-email-eric.nelson@boundarydevices.com>
	<201311142222.09785.marex@denx.de>
	<528542BE.8010703@boundarydevices.com>
Message-ID: <201311150030.12235.marex@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Eric,

> Thanks Marek,
> 
> On 11/14/2013 02:22 PM, Marek Vasut wrote:
> > Dear Eric Nelson,
> > 
> > +Albert, Tom
> > 
> >> These can be used in bootcmd to produce DTB file names.
> >> 
> >> set_board_env() allows over-ride for use when a developing custom
> >> DTB files.
> >> 
> >> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
> >> ---
> >> V2 adds void to the function declarations and definitions and adds
> >> a blank to keep checkpatch clean.
> >> 
> >> I'm feeling like I missed something here. Routines in imx-common/cpu.c
> >> are shared between various i.MX processors, but there doesn't appear
> >> to be a common header file.
> >> 
> >> It seems that arch/arm/include/asm/imx-common.h should be present
> >> but isn't. Am I missing something?
> >> 
> >> I also think there should be a way we could pull this into multiple
> >> boards without adding a full-up board_late_init() function into
> >> each board file, but tracing the init sequence, I'm not seeing an
> >> architecture-specific spot after env_init.
> >> 
> >>   arch/arm/imx-common/cpu.c                 | 15 +++++++++++++--
> >>   arch/arm/include/asm/arch-mx6/sys_proto.h |  9 +++++++++
> >>   2 files changed, 22 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/arch/arm/imx-common/cpu.c b/arch/arm/imx-common/cpu.c
> >> index 0cd2538..4214946 100644
> >> --- a/arch/arm/imx-common/cpu.c
> >> +++ b/arch/arm/imx-common/cpu.c
> >> @@ -99,8 +99,6 @@ unsigned imx_ddr_size(void)
> >> 
> >>   }
> >>   #endif
> >> 
> >> -#if defined(CONFIG_DISPLAY_CPUINFO)
> >> -
> >> 
> >>   const char *get_imx_type(u32 imxtype)
> >>   {
> >>   
> >>   	switch (imxtype) {
> >> 
> >> @@ -121,6 +119,19 @@ const char *get_imx_type(u32 imxtype)
> >> 
> >>   	}
> >>   
> >>   }
> >> 
> >> +void __weak set_cpu_env(void)
> >> +{
> >> +	setenv("cpu", get_imx_type(cpu_type(get_cpu_rev())));
> >> +}
> > 
> > I'd say we should have a U-Boot wide thing here + CPU-specific overrides
> > where applicable.
> 
> I'll follow your lead on this.

I'd wait for Tom's/Albert's opinion on this one too btw.

> I'm just hoping to get a trivial amount of machinery so a boot
> script can figure out which of a set of DTBs to load:
> 
> 	imx6q-nitrogen6x.dtb,
> 	imx6dl-nitrogen6x.dtb, or
> 	imx6s-nitrogen6x.dtb
> 
> Fabio's recent patch for SABRE SD showed the same need.

Full agreement here.

> This could be even easier on i.MX6 if we had more formalized (and
> lower-case) strings returned by get_imx_type(), but I wanted this
> to be very small.
> 
> I'm not sure how consistent the DTB naming is for other machines
> or if there's always the equivalent of get_imx_type().

In the worst-case scenario, you might end up with some mapping function full of 
strcmp()s ;-)