| Message ID | 20200805133407.101338-1-marex@denx.de |
|---|---|
| State | Accepted |
| Commit | c72372d38cb9d625f32f32e95e8fe821e5e2734b |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | [1/4] ARM: imx: Add bmode support for iMX7 | expand |
Hi Marek, On 05.08.20 15:34, Marek Vasut wrote: > Add the basic differentiation between i.MX6 and i.MX7 into the bmode > command, the mechanism really works almost the same on both platforms. > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Fabio Estevam <festevam@gmail.com> > Cc: NXP i.MX U-Boot Team <uboot-imx@nxp.com> > Cc: Peng Fan <peng.fan@nxp.com> > Cc: Stefano Babic <sbabic@denx.de> > --- > arch/arm/include/asm/mach-imx/sys_proto.h | 6 +++++- > arch/arm/mach-imx/Kconfig | 2 +- > arch/arm/mach-imx/init.c | 12 +++++++++--- > arch/arm/mach-imx/mx7/soc.c | 8 ++++++++ > 4 files changed, 23 insertions(+), 5 deletions(-) > > diff --git a/arch/arm/include/asm/mach-imx/sys_proto.h b/arch/arm/include/asm/mach-imx/sys_proto.h > index ab94024c9b..2d18b1f56b 100644 > --- a/arch/arm/include/asm/mach-imx/sys_proto.h > +++ b/arch/arm/include/asm/mach-imx/sys_proto.h > @@ -78,7 +78,7 @@ struct bd_info; > #define is_imx8qxp() (is_cpu_type(MXC_CPU_IMX8QXP)) > > #ifdef CONFIG_MX6 > -#define IMX6_SRC_GPR10_BMODE BIT(28) > +#define IMX6_SRC_GPR10_BMODE BIT(28) > > #define IMX6_BMODE_MASK GENMASK(7, 0) > #define IMX6_BMODE_SHIFT 4 > @@ -126,6 +126,10 @@ void gpr_init(void); > > #endif /* CONFIG_MX6 */ > > +#ifdef CONFIG_MX7 > +#define IMX7_SRC_GPR10_BMODE BIT(28) > +#endif > + It is questionable why we need two different defines, that also have exactly the same definition. Do we really need to differentiate and to use #ifdef ? > /* address translation table */ > struct rproc_att { > u32 da; /* device address (From Cortex M4 view) */ > diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig > index 1531d09f3b..8f64e23195 100644 > --- a/arch/arm/mach-imx/Kconfig > +++ b/arch/arm/mach-imx/Kconfig > @@ -62,7 +62,7 @@ config CSF_SIZE > config CMD_BMODE > bool "Support the 'bmode' command" > default y > - depends on ARCH_MX6 || ARCH_MX5 > + depends on ARCH_MX7 || ARCH_MX6 || ARCH_MX5 > help > This enables the 'bmode' (bootmode) command for forcing > a boot from specific media. > diff --git a/arch/arm/mach-imx/init.c b/arch/arm/mach-imx/init.c > index 693b724429..e30d63b896 100644 > --- a/arch/arm/mach-imx/init.c > +++ b/arch/arm/mach-imx/init.c > @@ -103,14 +103,20 @@ void init_src(void) > #ifdef CONFIG_CMD_BMODE > void boot_mode_apply(unsigned cfg_val) > { > - unsigned reg; > +#ifdef CONFIG_MX6 > + const u32 bmode = IMX6_SRC_GPR10_BMODE; > +#elif CONFIG_MX7 > + const u32 bmode = IMX7_SRC_GPR10_BMODE; > +#endif Ditto. > struct src *psrc = (struct src *)SRC_BASE_ADDR; > + unsigned reg; > + > writel(cfg_val, &psrc->gpr9); > reg = readl(&psrc->gpr10); > if (cfg_val) > - reg |= IMX6_SRC_GPR10_BMODE; > + reg |= bmode; > else > - reg &= ~IMX6_SRC_GPR10_BMODE; > + reg &= ~bmode; > writel(reg, &psrc->gpr10); > } > #endif > diff --git a/arch/arm/mach-imx/mx7/soc.c b/arch/arm/mach-imx/mx7/soc.c > index 8db672fb05..2698ae623e 100644 > --- a/arch/arm/mach-imx/mx7/soc.c > +++ b/arch/arm/mach-imx/mx7/soc.c > @@ -13,6 +13,7 @@ > #include <asm/mach-imx/hab.h> > #include <asm/mach-imx/rdc-sema.h> > #include <asm/arch/imx-rdc.h> > +#include <asm/mach-imx/boot_mode.h> > #include <asm/arch/crm_regs.h> > #include <dm.h> > #include <env.h> > @@ -411,6 +412,13 @@ void s_init(void) > return; > } > > +#ifndef CONFIG_SPL_BUILD > +const struct boot_mode soc_boot_modes[] = { > + {"normal", MAKE_CFGVAL(0x00, 0x00, 0x00, 0x00)}, > + {NULL, 0}, > +}; > +#endif > + > void reset_misc(void) > { > #ifndef CONFIG_SPL_BUILD > Regards, Stefano
On 8/5/20 3:59 PM, Stefano Babic wrote: > Hi Marek, Hi, > On 05.08.20 15:34, Marek Vasut wrote: >> Add the basic differentiation between i.MX6 and i.MX7 into the bmode >> command, the mechanism really works almost the same on both platforms. >> >> Signed-off-by: Marek Vasut <marex@denx.de> >> Cc: Fabio Estevam <festevam@gmail.com> >> Cc: NXP i.MX U-Boot Team <uboot-imx@nxp.com> >> Cc: Peng Fan <peng.fan@nxp.com> >> Cc: Stefano Babic <sbabic@denx.de> >> --- >> arch/arm/include/asm/mach-imx/sys_proto.h | 6 +++++- >> arch/arm/mach-imx/Kconfig | 2 +- >> arch/arm/mach-imx/init.c | 12 +++++++++--- >> arch/arm/mach-imx/mx7/soc.c | 8 ++++++++ >> 4 files changed, 23 insertions(+), 5 deletions(-) >> >> diff --git a/arch/arm/include/asm/mach-imx/sys_proto.h b/arch/arm/include/asm/mach-imx/sys_proto.h >> index ab94024c9b..2d18b1f56b 100644 >> --- a/arch/arm/include/asm/mach-imx/sys_proto.h >> +++ b/arch/arm/include/asm/mach-imx/sys_proto.h >> @@ -78,7 +78,7 @@ struct bd_info; >> #define is_imx8qxp() (is_cpu_type(MXC_CPU_IMX8QXP)) >> >> #ifdef CONFIG_MX6 >> -#define IMX6_SRC_GPR10_BMODE BIT(28) >> +#define IMX6_SRC_GPR10_BMODE BIT(28) >> >> #define IMX6_BMODE_MASK GENMASK(7, 0) >> #define IMX6_BMODE_SHIFT 4 >> @@ -126,6 +126,10 @@ void gpr_init(void); >> >> #endif /* CONFIG_MX6 */ >> >> +#ifdef CONFIG_MX7 >> +#define IMX7_SRC_GPR10_BMODE BIT(28) >> +#endif >> + > > It is questionable why we need two different defines, that also have > exactly the same definition. Do we really need to differentiate and to > use #ifdef ? Yes, because this file is also used by iMXes which are not 6/7 .
On 05.08.20 16:40, Marek Vasut wrote: > On 8/5/20 3:59 PM, Stefano Babic wrote: >> Hi Marek, > > Hi, > >> On 05.08.20 15:34, Marek Vasut wrote: >>> Add the basic differentiation between i.MX6 and i.MX7 into the bmode >>> command, the mechanism really works almost the same on both platforms. >>> >>> Signed-off-by: Marek Vasut <marex@denx.de> >>> Cc: Fabio Estevam <festevam@gmail.com> >>> Cc: NXP i.MX U-Boot Team <uboot-imx@nxp.com> >>> Cc: Peng Fan <peng.fan@nxp.com> >>> Cc: Stefano Babic <sbabic@denx.de> >>> --- >>> arch/arm/include/asm/mach-imx/sys_proto.h | 6 +++++- >>> arch/arm/mach-imx/Kconfig | 2 +- >>> arch/arm/mach-imx/init.c | 12 +++++++++--- >>> arch/arm/mach-imx/mx7/soc.c | 8 ++++++++ >>> 4 files changed, 23 insertions(+), 5 deletions(-) >>> >>> diff --git a/arch/arm/include/asm/mach-imx/sys_proto.h b/arch/arm/include/asm/mach-imx/sys_proto.h >>> index ab94024c9b..2d18b1f56b 100644 >>> --- a/arch/arm/include/asm/mach-imx/sys_proto.h >>> +++ b/arch/arm/include/asm/mach-imx/sys_proto.h >>> @@ -78,7 +78,7 @@ struct bd_info; >>> #define is_imx8qxp() (is_cpu_type(MXC_CPU_IMX8QXP)) >>> >>> #ifdef CONFIG_MX6 >>> -#define IMX6_SRC_GPR10_BMODE BIT(28) >>> +#define IMX6_SRC_GPR10_BMODE BIT(28) >>> >>> #define IMX6_BMODE_MASK GENMASK(7, 0) >>> #define IMX6_BMODE_SHIFT 4 >>> @@ -126,6 +126,10 @@ void gpr_init(void); >>> >>> #endif /* CONFIG_MX6 */ >>> >>> +#ifdef CONFIG_MX7 >>> +#define IMX7_SRC_GPR10_BMODE BIT(28) >>> +#endif >>> + >> >> It is questionable why we need two different defines, that also have >> exactly the same definition. Do we really need to differentiate and to >> use #ifdef ? > > Yes, because this file is also used by iMXes which are not 6/7 . Yes, but does it disturb ? There should be a define SRC_GPR10_BMODE that is not used at all if we build for mx3/mx5. I just prefer to reduce the number of #ifdef, when they are not strictly required. Regards, Stefano
On 8/5/20 4:54 PM, Stefano Babic wrote: [...] >>>> #define is_imx8qxp() (is_cpu_type(MXC_CPU_IMX8QXP)) >>>> >>>> #ifdef CONFIG_MX6 >>>> -#define IMX6_SRC_GPR10_BMODE BIT(28) >>>> +#define IMX6_SRC_GPR10_BMODE BIT(28) >>>> >>>> #define IMX6_BMODE_MASK GENMASK(7, 0) >>>> #define IMX6_BMODE_SHIFT 4 >>>> @@ -126,6 +126,10 @@ void gpr_init(void); >>>> >>>> #endif /* CONFIG_MX6 */ >>>> >>>> +#ifdef CONFIG_MX7 >>>> +#define IMX7_SRC_GPR10_BMODE BIT(28) >>>> +#endif >>>> + >>> >>> It is questionable why we need two different defines, that also have >>> exactly the same definition. Do we really need to differentiate and to >>> use #ifdef ? >> >> Yes, because this file is also used by iMXes which are not 6/7 . > > Yes, but does it disturb ? There should be a define SRC_GPR10_BMODE that > is not used at all if we build for mx3/mx5. I just prefer to reduce the > number of #ifdef, when they are not strictly required. Its putting macros into the preprocessor which don't need to be there. Otherwise, it is inline with the style in the rest of the file.
> Add the basic differentiation between i.MX6 and i.MX7 into the bmode > command, the mechanism really works almost the same on both platforms. > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Fabio Estevam <festevam@gmail.com> > Cc: NXP i.MX U-Boot Team <uboot-imx@nxp.com> > Cc: Peng Fan <peng.fan@nxp.com> > Cc: Stefano Babic <sbabic@denx.de> Applied to u-boot-imx, master, thanks ! Best regards, Stefano Babic
diff --git a/arch/arm/include/asm/mach-imx/sys_proto.h b/arch/arm/include/asm/mach-imx/sys_proto.h index ab94024c9b..2d18b1f56b 100644 --- a/arch/arm/include/asm/mach-imx/sys_proto.h +++ b/arch/arm/include/asm/mach-imx/sys_proto.h @@ -78,7 +78,7 @@ struct bd_info; #define is_imx8qxp() (is_cpu_type(MXC_CPU_IMX8QXP)) #ifdef CONFIG_MX6 -#define IMX6_SRC_GPR10_BMODE BIT(28) +#define IMX6_SRC_GPR10_BMODE BIT(28) #define IMX6_BMODE_MASK GENMASK(7, 0) #define IMX6_BMODE_SHIFT 4 @@ -126,6 +126,10 @@ void gpr_init(void); #endif /* CONFIG_MX6 */ +#ifdef CONFIG_MX7 +#define IMX7_SRC_GPR10_BMODE BIT(28) +#endif + /* address translation table */ struct rproc_att { u32 da; /* device address (From Cortex M4 view) */ diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig index 1531d09f3b..8f64e23195 100644 --- a/arch/arm/mach-imx/Kconfig +++ b/arch/arm/mach-imx/Kconfig @@ -62,7 +62,7 @@ config CSF_SIZE config CMD_BMODE bool "Support the 'bmode' command" default y - depends on ARCH_MX6 || ARCH_MX5 + depends on ARCH_MX7 || ARCH_MX6 || ARCH_MX5 help This enables the 'bmode' (bootmode) command for forcing a boot from specific media. diff --git a/arch/arm/mach-imx/init.c b/arch/arm/mach-imx/init.c index 693b724429..e30d63b896 100644 --- a/arch/arm/mach-imx/init.c +++ b/arch/arm/mach-imx/init.c @@ -103,14 +103,20 @@ void init_src(void) #ifdef CONFIG_CMD_BMODE void boot_mode_apply(unsigned cfg_val) { - unsigned reg; +#ifdef CONFIG_MX6 + const u32 bmode = IMX6_SRC_GPR10_BMODE; +#elif CONFIG_MX7 + const u32 bmode = IMX7_SRC_GPR10_BMODE; +#endif struct src *psrc = (struct src *)SRC_BASE_ADDR; + unsigned reg; + writel(cfg_val, &psrc->gpr9); reg = readl(&psrc->gpr10); if (cfg_val) - reg |= IMX6_SRC_GPR10_BMODE; + reg |= bmode; else - reg &= ~IMX6_SRC_GPR10_BMODE; + reg &= ~bmode; writel(reg, &psrc->gpr10); } #endif diff --git a/arch/arm/mach-imx/mx7/soc.c b/arch/arm/mach-imx/mx7/soc.c index 8db672fb05..2698ae623e 100644 --- a/arch/arm/mach-imx/mx7/soc.c +++ b/arch/arm/mach-imx/mx7/soc.c @@ -13,6 +13,7 @@ #include <asm/mach-imx/hab.h> #include <asm/mach-imx/rdc-sema.h> #include <asm/arch/imx-rdc.h> +#include <asm/mach-imx/boot_mode.h> #include <asm/arch/crm_regs.h> #include <dm.h> #include <env.h> @@ -411,6 +412,13 @@ void s_init(void) return; } +#ifndef CONFIG_SPL_BUILD +const struct boot_mode soc_boot_modes[] = { + {"normal", MAKE_CFGVAL(0x00, 0x00, 0x00, 0x00)}, + {NULL, 0}, +}; +#endif + void reset_misc(void) { #ifndef CONFIG_SPL_BUILD
Add the basic differentiation between i.MX6 and i.MX7 into the bmode command, the mechanism really works almost the same on both platforms. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Fabio Estevam <festevam@gmail.com> Cc: NXP i.MX U-Boot Team <uboot-imx@nxp.com> Cc: Peng Fan <peng.fan@nxp.com> Cc: Stefano Babic <sbabic@denx.de> --- arch/arm/include/asm/mach-imx/sys_proto.h | 6 +++++- arch/arm/mach-imx/Kconfig | 2 +- arch/arm/mach-imx/init.c | 12 +++++++++--- arch/arm/mach-imx/mx7/soc.c | 8 ++++++++ 4 files changed, 23 insertions(+), 5 deletions(-)