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

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()))); +} + +void __weak set_board_env(void) +{ + char *old = getenv("board"); + if (!old) + setenv("board", CONFIG_SYS_BOARD); +} + +#if defined(CONFIG_DISPLAY_CPUINFO) int print_cpuinfo(void) { u32 cpurev; diff --git a/arch/arm/include/asm/arch-mx6/sys_proto.h b/arch/arm/include/asm/arch-mx6/sys_proto.h index 8c21364..f58ebc3 100644 --- a/arch/arm/include/asm/arch-mx6/sys_proto.h +++ b/arch/arm/include/asm/arch-mx6/sys_proto.h @@ -27,8 +27,17 @@ u32 get_cpu_rev(void); #define is_cpu_type(cpu) (cpu_type(get_cpu_rev()) == cpu)
const char *get_imx_type(u32 imxtype); + unsigned imx_ddr_size(void);
+/* Set standard board and cpu environment variables. + * for use in loading DTB files. + * + * Call these in board_late_init if needed + */ +void set_cpu_env(void); +void set_board_env(void); + void set_vddsoc(u32 mv);
/*

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.
+void __weak set_board_env(void) +{
- char *old = getenv("board");
- if (!old)
setenv("board", CONFIG_SYS_BOARD);
+}
This might go into arch/FOO/lib/board.c ? [...]

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'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.
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().
Regards,
Eric

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 ;-)

On 11/14/2013 04:30 PM, Marek Vasut wrote:
Hi Eric,
Thanks Marek, On 11/14/2013 02:22 PM, Marek Vasut wrote:
Dear Eric Nelson,
<snip>
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 ;-)
This is much easier for us to do, since we defer much of the scripting into a boot script.
It will be harder for Fabio to shoe-horn in until we get environment settings in an external file...

Dear Eric Nelson,
On 11/14/2013 04:30 PM, Marek Vasut wrote:
Hi Eric,
Thanks Marek,
On 11/14/2013 02:22 PM, Marek Vasut wrote:
Dear Eric Nelson,
<snip>
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 ;-)
This is much easier for us to do, since we defer much of the scripting into a boot script.
It will be harder for Fabio to shoe-horn in until we get environment settings in an external file...
Yeah, patching the env unto u-boot is the way to go here I guess. What's the progress on this anyway, Tom ?
Best regards, Marek Vasut

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?

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

Hi Eric,
On Thu, Nov 14, 2013 at 7:38 PM, Eric Nelson eric.nelson@boundarydevices.com wrote:
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
imx6dl-nitrogen6x.dtb should cover both dl and solo variants, so no need for a imx6s-nitrogen6x.dtb version.
Regards,
Fabio Estevam

On 11/16/2013 07:03 AM, Fabio Estevam wrote:
Hi Eric,
On Thu, Nov 14, 2013 at 7:38 PM, Eric Nelson eric.nelson@boundarydevices.com wrote:
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
imx6dl-nitrogen6x.dtb should cover both dl and solo variants, so no need for a imx6s-nitrogen6x.dtb version.
Thanks Fabio.
participants (4)
-
Eric Nelson
-
Fabio Estevam
-
Marek Vasut
-
Tom Rini