[U-Boot] [PATCH V2] imx: Define common routines to set cpu and board environment variables

Tom Rini
2013-11-15T19:15:25Z

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

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.

[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?

--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20131115/8d6b431e/attachment.pgp>