[U-Boot] [PATCH v2 5/9] mx7: set soc environment according to exact SoC type

[Stefan Agner]

On 2016-07-18 17:36, Tom Rini wrote:
> On Mon, Jul 18, 2016 at 03:21:39PM -0700, Stefan Agner wrote:
>> On 2016-07-18 15:19, Fabio Estevam wrote:
>> > On Wed, Jul 13, 2016 at 4:25 AM, Stefan Agner <stefan at agner.ch> wrote:
>> >> From: Stefan Agner <stefan.agner at toradex.com>
>> >>
>> >> This can be useful if the same U-Boot binary is used for boards
>> >> available with a i.MX 7Solo and i.MX 7Dual.
>> >>
>> >> Signed-off-by: Stefan Agner <stefan.agner at toradex.com>
>> >> Reviewed-by: Simon Glass <sjg at chromium.org>
>> >> ---
>> >>
>> >>  arch/arm/cpu/armv7/mx7/soc.c | 14 ++++++++++++++
>> >>  include/configs/mx7_common.h |  2 ++
>> >>  2 files changed, 16 insertions(+)
>> >>
>> >> diff --git a/arch/arm/cpu/armv7/mx7/soc.c b/arch/arm/cpu/armv7/mx7/soc.c
>> >> index ef46c92..dead1d3 100644
>> >> --- a/arch/arm/cpu/armv7/mx7/soc.c
>> >> +++ b/arch/arm/cpu/armv7/mx7/soc.c
>> >> @@ -248,6 +248,20 @@ int arch_cpu_init(void)
>> >>         return 0;
>> >>  }
>> >>
>> >> +#ifdef CONFIG_ARCH_MISC_INIT
>> >> +int arch_misc_init(void)
>> >> +{
>> >> +#ifdef CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG
>> >> +       if (is_mx7d())
>> >> +               setenv("soc", "imx7d");
>> >> +       else
>> >> +               setenv("soc", "imx7s");
>> >> +#endif
>> >> +
>> >> +       return 0;
>> >> +}
>> >> +#endif
>> >
>> > What about adding the following code in your board file?
>> >
>> > int board_late_init(void)
>> > {
>> > #ifdef CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG
>> >     if (is_mx7d())
>> >         setenv("board_rev", "MX7D");
>> >     else
>> >         setenv("board_rev", "MX7S");
>> > #endif
>> >
>> >     return 0;
>> > }
>> >
>> > This was the suggestion I got from Tom when I was doing runtime SoC
>> > detection for mx6cubox.
>>
>> I followed the runtime detection for Vybrid in:
>> arch/arm/cpu/armv7/vf610/generic.c
>>
>> But sure, doing it at board level would be possible too.
>>
>> Due to the similarity of i.MX 7Solo and 7Dual there will probably be
>> many board designs available in this two variants, hence I feel it would
>> be worthwhile to have it on arch level...
> 
> So, yeah, I may have said the wrong thing for cubox, maybe not.  I think
> that for the actual SoC (and 'soc') we should set that as high as we
> easily and _correctly_ can.  We set 'board_rev' today in the board

Can I take this as an Ack from your side Tom?

Also note that the current default for i.MX 7 is "mx7" which is not
useful to build the typically used (Linux) device tree file name. It
also seems that no i.MX 6/7 board uses ${soc} in its default
environment...

IMHO ${soc} should be the first part of the device tree file, which is
in this case imx7d- (and imx7s-...)

> files, and this makes sense to me, especially when used as the "board"
> part, eg:
> board/freescale/mx6ul_14x14_evk/mx6ul_14x14_evk.c: setenv("board_rev", "9X9");
> board/freescale/mx6ul_14x14_evk/mx6ul_14x14_evk.c: setenv("board_rev", "14X14")
> 
> We set 'board_rev' today in a number of board files and a quick review
> just now tells me that yes, we should have done that higher up.
> Probably.  Using the mx6cubox example, I see:
> static bool is_mx6q(void)
> {
>         if (is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D))
> 	                return true;
> 
> Can we move this to global code, check for each of the MX6 variants, MX7
> variants and always be returning what we return today?

What is board_rev commonly used for?

It sounds more board specific, so we certainly would have to have a way
to overwrite it on board level (e.g. if somebody has two MX6Q SKUs?)

--
Stefan