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

[Tom Rini]

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 <ste...@agner.ch> wrote:
> >> From: Stefan Agner <stefan.ag...@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.ag...@toradex.com>
> >> Reviewed-by: Simon Glass <s...@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
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?

-- 
Tom

signature.asc
Description: Digital signature

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot