From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Nelson Date: Sat, 16 Nov 2013 09:14:19 -0700 Subject: [U-Boot] [PATCH V2] imx: Define common routines to set cpu and board environment variables In-Reply-To: <20131115191525.GH420@bill-the-cat> References: <1384391809-9339-1-git-send-email-eric.nelson@boundarydevices.com> <201311142222.09785.marex@denx.de> <528542BE.8010703@boundarydevices.com> <201311150030.12235.marex@denx.de> <20131115191525.GH420@bill-the-cat> Message-ID: <528799DB.20201@boundarydevices.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Thanks Tom, On 11/15/2013 12:15 PM, Tom Rini wrote: > On Fri, Nov 15, 2013 at 12:30:12AM +0100, Marek Vasut wrote: >> 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 >>>>> --- >>>>> 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. > > So, CONFIG_ENV_VARS_UBOOT_CONFIG says that these variables are > _build_time_. If you want to whack their values, update the README on > CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG to say so. Having the i.MX code > that updates this at run-time is fine to live in the i.MX code area. > I wasn't aware of these two options, and they seem useful. They also suggest that using "cpu" and "board" are probably wrong, since CONFIG_SYS_CPU and environment "cpu" are already set to "armv7" and "board" is configured with the build-time board name. Overriding "board_name" seems to be in-line with the README if we can just figure out the right spot for initialization. Some other variable ("cpu_name" or "imx_type" perhaps?) should probably be used for the CPU variant. Chasing down how "board_name" is initialized in get_board_revision() leads me to arch_misc_init() is the right place for this on i.MX, since it's called after the environment is initialized. > [snip] >>> 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 ;-) > > Note that in TI-land we do this with 'findfdt' in the environment and > yes, some string compares. But we set board_name in C so that we can do > what we need to, to determine the board. Maybe something like that > would help here? > Thanks. That does help. Regards, Eric