[U-Boot] [PATCH 1/2] imx-common: Enable ACTLR.SMP bit for i.MX cortex-a7 platforms
According to the Cortex-A7 TRM, for ACTLR.SMP bit "You must ensure this bit is set to 1 before the caches and MMU are enabled, or any cache and TLB maintenance operations are performed".
ROM sets this bit in normal boot flow, but when in serial download mode, it is not set. Here we add it in u-boot as a common flow for all i.MX cortex-a7 platforms, including mx7d, mx6ul/ull and mx7ulp.
When cache not enabled in U-Boot, we also need enable the SMP bit. because kernel does not set this bit, if this bit not enabled, kernel works as cache disabled.
Signed-off-by: Peng Fan peng.fan@nxp.com Signed-off-by: Ye Li ye.li@nxp.com Cc: Stefano Babic sbabic@denx.de --- arch/arm/cpu/armv7/mx7/soc.c | 7 ------- arch/arm/imx-common/cache.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-)
diff --git a/arch/arm/cpu/armv7/mx7/soc.c b/arch/arm/cpu/armv7/mx7/soc.c index 8422f24..b847cf8 100644 --- a/arch/arm/cpu/armv7/mx7/soc.c +++ b/arch/arm/cpu/armv7/mx7/soc.c @@ -446,13 +446,6 @@ int mmc_get_env_dev(void)
void s_init(void) { -#if !defined CONFIG_SPL_BUILD - /* Enable SMP mode for CPU0, by setting bit 6 of Auxiliary Ctl reg */ - asm volatile( - "mrc p15, 0, r0, c1, c0, 1\n" - "orr r0, r0, #1 << 6\n" - "mcr p15, 0, r0, c1, c0, 1\n"); -#endif /* clock configuration. */ clock_init();
diff --git a/arch/arm/imx-common/cache.c b/arch/arm/imx-common/cache.c index 1c4a9a2..ed17f9e 100644 --- a/arch/arm/imx-common/cache.c +++ b/arch/arm/imx-common/cache.c @@ -10,6 +10,34 @@ #include <asm/io.h> #include <asm/imx-common/sys_proto.h>
+static void enable_ca7_smp(void) +{ + uint32_t val; + + /* Read MIDR */ + asm volatile ("mrc p15, 0, %0, c0, c0, 0\r\n" : "=r"(val)); + val = (val >> 4); + val &= 0xfff; + + /* Only set the SMP for Cortex A7 */ + if (val == 0xC07) { + /* Read auxiliary control register */ + asm volatile ("mrc p15, 0, %0, c1, c0, 1\r\n" : "=r"(val)); + + if (val & (1 << 6)) + return; + + /* Enable SMP */ + val |= (1 << 6); + + /* Write auxiliary control register */ + asm volatile ("mcr p15, 0, %0, c1, c0, 1\r\n" : : "r"(val)); + + DSB; + ISB; + } +} + #ifndef CONFIG_SYS_DCACHE_OFF void enable_caches(void) { @@ -18,6 +46,9 @@ void enable_caches(void) #else enum dcache_option option = DCACHE_WRITEBACK; #endif + /* Set ACTLR.SMP bit for Cortex-A7 */ + enable_ca7_smp(); + /* Avoid random hang when download by usb */ invalidate_dcache_all();
@@ -32,6 +63,17 @@ void enable_caches(void) IRAM_SIZE, option); } +#else +void enable_caches(void) +{ + /* + * Set ACTLR.SMP bit for Cortex-A7, even the + * caches not enabled. + */ + enable_ca7_smp(); + + puts("WARNING: Caches not enabled\n"); +} #endif
#ifndef CONFIG_SYS_L2CACHE_OFF
From: Ye Li ye.li@nxp.com
The num/denom is a float value, but in the calculation it is convert to integer 0, and wrong result.
Signed-off-by: Ye Li ye.li@nxp.com Signed-off-by: Peng Fan peng.fan@nxp.com Cc: Stefano Babic sbabic@denx.de --- arch/arm/cpu/armv7/mx7ulp/scg.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/arm/cpu/armv7/mx7ulp/scg.c b/arch/arm/cpu/armv7/mx7ulp/scg.c index ca8252d..c117af0 100644 --- a/arch/arm/cpu/armv7/mx7ulp/scg.c +++ b/arch/arm/cpu/armv7/mx7ulp/scg.c @@ -504,7 +504,9 @@ u32 decode_pll(enum pll_clocks pll) num = readl(&scg1_regs->spllnum); denom = readl(&scg1_regs->splldenom);
- return (infreq / pre_div) * (mult + num / denom); + infreq = infreq / pre_div; + + return infreq * mult + infreq * num / denom;
case PLL_A7_APLL: reg = readl(&scg1_regs->apllcsr); @@ -531,7 +533,9 @@ u32 decode_pll(enum pll_clocks pll) num = readl(&scg1_regs->apllnum); denom = readl(&scg1_regs->aplldenom);
- return (infreq / pre_div) * (mult + num / denom); + infreq = infreq / pre_div; + + return infreq * mult + infreq * num / denom;
case PLL_USB: reg = readl(&scg1_regs->upllcsr);
On 05/04/2017 04:36, Peng Fan wrote:
From: Ye Li ye.li@nxp.com
The num/denom is a float value, but in the calculation it is convert to integer 0, and wrong result.
Signed-off-by: Ye Li ye.li@nxp.com Signed-off-by: Peng Fan peng.fan@nxp.com Cc: Stefano Babic sbabic@denx.de
arch/arm/cpu/armv7/mx7ulp/scg.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/arm/cpu/armv7/mx7ulp/scg.c b/arch/arm/cpu/armv7/mx7ulp/scg.c index ca8252d..c117af0 100644 --- a/arch/arm/cpu/armv7/mx7ulp/scg.c +++ b/arch/arm/cpu/armv7/mx7ulp/scg.c @@ -504,7 +504,9 @@ u32 decode_pll(enum pll_clocks pll) num = readl(&scg1_regs->spllnum); denom = readl(&scg1_regs->splldenom);
return (infreq / pre_div) * (mult + num / denom);
infreq = infreq / pre_div;return infreq * mult + infreq * num / denom;case PLL_A7_APLL: reg = readl(&scg1_regs->apllcsr);
@@ -531,7 +533,9 @@ u32 decode_pll(enum pll_clocks pll) num = readl(&scg1_regs->apllnum); denom = readl(&scg1_regs->aplldenom);
return (infreq / pre_div) * (mult + num / denom);
infreq = infreq / pre_div;return infreq * mult + infreq * num / denom;case PLL_USB: reg = readl(&scg1_regs->upllcsr);
Applied to u-boot-imx, thanks!
Best regards, Stefano Babic
Hi Peng,
On 05/04/2017 04:36, Peng Fan wrote:
According to the Cortex-A7 TRM, for ACTLR.SMP bit "You must ensure this bit is set to 1 before the caches and MMU are enabled, or any cache and TLB maintenance operations are performed".
True, understood, thanks.
ROM sets this bit in normal boot flow, but when in serial download mode, it is not set. Here we add it in u-boot as a common flow for all i.MX cortex-a7 platforms, including mx7d, mx6ul/ull and mx7ulp.
When cache not enabled in U-Boot, we also need enable the SMP bit. because kernel does not set this bit, if this bit not enabled, kernel works as cache disabled.
Anyway, it looks to me wrong to fix this in i.MX code, as this must be fixed for all Cortex-A7. It should be fixed in general ARM code.
And Andre has already sent patches for this (at least for sunxi), but with the opposite rule (default is set if not specific requested).
Regards, Stefano
Signed-off-by: Peng Fan peng.fan@nxp.com Signed-off-by: Ye Li ye.li@nxp.com Cc: Stefano Babic sbabic@denx.de
arch/arm/cpu/armv7/mx7/soc.c | 7 ------- arch/arm/imx-common/cache.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-)
diff --git a/arch/arm/cpu/armv7/mx7/soc.c b/arch/arm/cpu/armv7/mx7/soc.c index 8422f24..b847cf8 100644 --- a/arch/arm/cpu/armv7/mx7/soc.c +++ b/arch/arm/cpu/armv7/mx7/soc.c @@ -446,13 +446,6 @@ int mmc_get_env_dev(void)
void s_init(void) { -#if !defined CONFIG_SPL_BUILD
- /* Enable SMP mode for CPU0, by setting bit 6 of Auxiliary Ctl reg */
- asm volatile(
"mrc p15, 0, r0, c1, c0, 1\n""orr r0, r0, #1 << 6\n""mcr p15, 0, r0, c1, c0, 1\n");-#endif /* clock configuration. */ clock_init();
diff --git a/arch/arm/imx-common/cache.c b/arch/arm/imx-common/cache.c index 1c4a9a2..ed17f9e 100644 --- a/arch/arm/imx-common/cache.c +++ b/arch/arm/imx-common/cache.c @@ -10,6 +10,34 @@ #include <asm/io.h> #include <asm/imx-common/sys_proto.h>
+static void enable_ca7_smp(void) +{
- uint32_t val;
- /* Read MIDR */
- asm volatile ("mrc p15, 0, %0, c0, c0, 0\r\n" : "=r"(val));
- val = (val >> 4);
- val &= 0xfff;
- /* Only set the SMP for Cortex A7 */
- if (val == 0xC07) {
/* Read auxiliary control register */asm volatile ("mrc p15, 0, %0, c1, c0, 1\r\n" : "=r"(val));if (val & (1 << 6))return;/* Enable SMP */val |= (1 << 6);/* Write auxiliary control register */asm volatile ("mcr p15, 0, %0, c1, c0, 1\r\n" : : "r"(val));DSB;ISB;- }
+}
#ifndef CONFIG_SYS_DCACHE_OFF void enable_caches(void) { @@ -18,6 +46,9 @@ void enable_caches(void) #else enum dcache_option option = DCACHE_WRITEBACK; #endif
- /* Set ACTLR.SMP bit for Cortex-A7 */
- enable_ca7_smp();
- /* Avoid random hang when download by usb */ invalidate_dcache_all();
@@ -32,6 +63,17 @@ void enable_caches(void) IRAM_SIZE, option); } +#else +void enable_caches(void) +{
- /*
* Set ACTLR.SMP bit for Cortex-A7, even the* caches not enabled.*/- enable_ca7_smp();
- puts("WARNING: Caches not enabled\n");
+} #endif
#ifndef CONFIG_SYS_L2CACHE_OFF
Hi Stefano,
On Wed, Apr 12, 2017 at 1:44 PM, Stefano Babic sbabic@denx.de wrote:
Anyway, it looks to me wrong to fix this in i.MX code, as this must be fixed for all Cortex-A7. It should be fixed in general ARM code.
I agree 100%. This was the exact same feedback I gave when I saw this patch being sent to the internal NXP U-Boot tree.
Thanks
Hi Stefano,
-----Original Message----- From: Stefano Babic [mailto:sbabic@denx.de] Sent: Thursday, April 13, 2017 12:45 AM To: Peng Fan peng.fan@nxp.com; sbabic@denx.de Cc: van.freenix@gmail.com; u-boot@lists.denx.de; andre.przywara@arm.com Subject: Re: [PATCH 1/2] imx-common: Enable ACTLR.SMP bit for i.MX cortex- a7 platforms
Hi Peng,
On 05/04/2017 04:36, Peng Fan wrote:
According to the Cortex-A7 TRM, for ACTLR.SMP bit "You must ensure this bit is set to 1 before the caches and MMU are enabled, or any cache and TLB maintenance operations are performed".
True, understood, thanks.
ROM sets this bit in normal boot flow, but when in serial download mode, it is not set. Here we add it in u-boot as a common flow for all i.MX cortex-a7 platforms, including mx7d, mx6ul/ull and mx7ulp.
When cache not enabled in U-Boot, we also need enable the SMP bit. because kernel does not set this bit, if this bit not enabled, kernel works as cache disabled.
Anyway, it looks to me wrong to fix this in i.MX code, as this must be fixed for all Cortex-A7. It should be fixed in general ARM code.
And Andre has already sent patches for this (at least for sunxi), but with the opposite rule (default is set if not specific requested).
Thanks for correcting me. I see Andre's patch https://lists.denx.de/pipermail/u-boot/2017-February/279918.html
It only handles sunxi platform. If we need fix i.MX A7 cores, do you prefer I set the SMP bit in s_init for i.MX6UL/7? Or you prefer this should be handled by common ARM code? I do not see code the detect A7/A9 in U-Boot now.
Thanks, Peng.
Regards, Stefano
Signed-off-by: Peng Fan peng.fan@nxp.com Signed-off-by: Ye Li ye.li@nxp.com Cc: Stefano Babic sbabic@denx.de
arch/arm/cpu/armv7/mx7/soc.c | 7 ------- arch/arm/imx-common/cache.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-)
diff --git a/arch/arm/cpu/armv7/mx7/soc.c b/arch/arm/cpu/armv7/mx7/soc.c index 8422f24..b847cf8 100644 --- a/arch/arm/cpu/armv7/mx7/soc.c +++ b/arch/arm/cpu/armv7/mx7/soc.c @@ -446,13 +446,6 @@ int mmc_get_env_dev(void)
void s_init(void) { -#if !defined CONFIG_SPL_BUILD
- /* Enable SMP mode for CPU0, by setting bit 6 of Auxiliary Ctl reg */
- asm volatile(
"mrc p15, 0, r0, c1, c0, 1\n""orr r0, r0, #1 << 6\n""mcr p15, 0, r0, c1, c0, 1\n");-#endif /* clock configuration. */ clock_init();
diff --git a/arch/arm/imx-common/cache.c b/arch/arm/imx-common/cache.c index 1c4a9a2..ed17f9e 100644 --- a/arch/arm/imx-common/cache.c +++ b/arch/arm/imx-common/cache.c @@ -10,6 +10,34 @@ #include <asm/io.h> #include <asm/imx-common/sys_proto.h>
+static void enable_ca7_smp(void) +{
- uint32_t val;
- /* Read MIDR */
- asm volatile ("mrc p15, 0, %0, c0, c0, 0\r\n" : "=r"(val));
- val = (val >> 4);
- val &= 0xfff;
- /* Only set the SMP for Cortex A7 */
- if (val == 0xC07) {
/* Read auxiliary control register */asm volatile ("mrc p15, 0, %0, c1, c0, 1\r\n" : "=r"(val));if (val & (1 << 6))return;/* Enable SMP */val |= (1 << 6);/* Write auxiliary control register */asm volatile ("mcr p15, 0, %0, c1, c0, 1\r\n" : : "r"(val));DSB;ISB;- }
+}
#ifndef CONFIG_SYS_DCACHE_OFF void enable_caches(void) { @@ -18,6 +46,9 @@ void enable_caches(void) #else enum dcache_option option = DCACHE_WRITEBACK; #endif
- /* Set ACTLR.SMP bit for Cortex-A7 */
- enable_ca7_smp();
- /* Avoid random hang when download by usb */ invalidate_dcache_all();
@@ -32,6 +63,17 @@ void enable_caches(void) IRAM_SIZE, option); } +#else +void enable_caches(void) +{
- /*
* Set ACTLR.SMP bit for Cortex-A7, even the* caches not enabled.*/- enable_ca7_smp();
- puts("WARNING: Caches not enabled\n"); }
#endif
#ifndef CONFIG_SYS_L2CACHE_OFF
--
========= DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de ============================================================ =========
Hi Peng,
On 13/04/2017 05:05, Peng Fan wrote:
Anyway, it looks to me wrong to fix this in i.MX code, as this must be fixed for all Cortex-A7. It should be fixed in general ARM code.
And Andre has already sent patches for this (at least for sunxi), but with the opposite rule (default is set if not specific requested).
Thanks for correcting me. I see Andre's patch https://lists.denx.de/pipermail/u-boot/2017-February/279918.html
It only handles sunxi platform. If we need fix i.MX A7 cores, do you prefer I set the SMP bit in s_init for i.MX6UL/7? Or you prefer this should be handled by common ARM code?
As far as I understand, this issue is common to all A7 cores ==> fix should be in common ARM code.
Best regards, Stefano
I do not see code the detect A7/A9 in U-Boot now.
Thanks, Peng.
Regards, Stefano
Signed-off-by: Peng Fan peng.fan@nxp.com Signed-off-by: Ye Li ye.li@nxp.com Cc: Stefano Babic sbabic@denx.de
arch/arm/cpu/armv7/mx7/soc.c | 7 ------- arch/arm/imx-common/cache.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-)
diff --git a/arch/arm/cpu/armv7/mx7/soc.c b/arch/arm/cpu/armv7/mx7/soc.c index 8422f24..b847cf8 100644 --- a/arch/arm/cpu/armv7/mx7/soc.c +++ b/arch/arm/cpu/armv7/mx7/soc.c @@ -446,13 +446,6 @@ int mmc_get_env_dev(void)
void s_init(void) { -#if !defined CONFIG_SPL_BUILD
- /* Enable SMP mode for CPU0, by setting bit 6 of Auxiliary Ctl reg */
- asm volatile(
"mrc p15, 0, r0, c1, c0, 1\n""orr r0, r0, #1 << 6\n""mcr p15, 0, r0, c1, c0, 1\n");-#endif /* clock configuration. */ clock_init();
diff --git a/arch/arm/imx-common/cache.c b/arch/arm/imx-common/cache.c index 1c4a9a2..ed17f9e 100644 --- a/arch/arm/imx-common/cache.c +++ b/arch/arm/imx-common/cache.c @@ -10,6 +10,34 @@ #include <asm/io.h> #include <asm/imx-common/sys_proto.h>
+static void enable_ca7_smp(void) +{
- uint32_t val;
- /* Read MIDR */
- asm volatile ("mrc p15, 0, %0, c0, c0, 0\r\n" : "=r"(val));
- val = (val >> 4);
- val &= 0xfff;
- /* Only set the SMP for Cortex A7 */
- if (val == 0xC07) {
/* Read auxiliary control register */asm volatile ("mrc p15, 0, %0, c1, c0, 1\r\n" : "=r"(val));if (val & (1 << 6))return;/* Enable SMP */val |= (1 << 6);/* Write auxiliary control register */asm volatile ("mcr p15, 0, %0, c1, c0, 1\r\n" : : "r"(val));DSB;ISB;- }
+}
#ifndef CONFIG_SYS_DCACHE_OFF void enable_caches(void) { @@ -18,6 +46,9 @@ void enable_caches(void) #else enum dcache_option option = DCACHE_WRITEBACK; #endif
- /* Set ACTLR.SMP bit for Cortex-A7 */
- enable_ca7_smp();
- /* Avoid random hang when download by usb */ invalidate_dcache_all();
@@ -32,6 +63,17 @@ void enable_caches(void) IRAM_SIZE, option); } +#else +void enable_caches(void) +{
- /*
* Set ACTLR.SMP bit for Cortex-A7, even the* caches not enabled.*/- enable_ca7_smp();
- puts("WARNING: Caches not enabled\n"); }
#endif
#ifndef CONFIG_SYS_L2CACHE_OFF
--
========= DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de ============================================================ =========
Hi Stefano,
-----Original Message----- From: Stefano Babic [mailto:sbabic@denx.de] Sent: Thursday, April 13, 2017 2:24 PM To: Peng Fan peng.fan@nxp.com; Stefano Babic sbabic@denx.de Cc: van.freenix@gmail.com; u-boot@lists.denx.de; andre.przywara@arm.com; Albert ARIBAUD albert.u.boot@aribaud.net Subject: Re: [PATCH 1/2] imx-common: Enable ACTLR.SMP bit for i.MX cortex- a7 platforms
Hi Peng,
On 13/04/2017 05:05, Peng Fan wrote:
Anyway, it looks to me wrong to fix this in i.MX code, as this must be fixed for all Cortex-A7. It should be fixed in general ARM code.
And Andre has already sent patches for this (at least for sunxi), but with the opposite rule (default is set if not specific requested).
Thanks for correcting me. I see Andre's patch https://lists.denx.de/pipermail/u-boot/2017-February/279918.html
It only handles sunxi platform. If we need fix i.MX A7 cores, do you prefer I
set the SMP bit in s_init for i.MX6UL/7?
Or you prefer this should be handled by common ARM code?
As far as I understand, this issue is common to all A7 cores ==> fix should be in common ARM code.
Ok. Since there is no Cortex-A[x] revision check in U-Boot, need to first add the code. Then set ACTLR.SMP for A7 SMP.
Regards, Peng.
On 13/04/17 07:27, Peng Fan wrote:
Hi,
-----Original Message----- From: Stefano Babic [mailto:sbabic@denx.de] Sent: Thursday, April 13, 2017 2:24 PM To: Peng Fan peng.fan@nxp.com; Stefano Babic sbabic@denx.de Cc: van.freenix@gmail.com; u-boot@lists.denx.de; andre.przywara@arm.com; Albert ARIBAUD albert.u.boot@aribaud.net Subject: Re: [PATCH 1/2] imx-common: Enable ACTLR.SMP bit for i.MX cortex- a7 platforms
Hi Peng,
On 13/04/2017 05:05, Peng Fan wrote:
Anyway, it looks to me wrong to fix this in i.MX code, as this must be fixed for all Cortex-A7. It should be fixed in general ARM code.
And Andre has already sent patches for this (at least for sunxi), but with the opposite rule (default is set if not specific requested).
Thanks for correcting me. I see Andre's patch https://lists.denx.de/pipermail/u-boot/2017-February/279918.html
It only handles sunxi platform. If we need fix i.MX A7 cores, do you prefer I
set the SMP bit in s_init for i.MX6UL/7?
Or you prefer this should be handled by common ARM code?
As far as I understand, this issue is common to all A7 cores ==> fix should be in common ARM code.
Well, it's not only common to all A7 cores, actually all ARMv7 SMP Cortex cores (A5, A7, A9, A15, A17) need this. Cortex-A8 is UP only, so does not have this bit. Sometimes this bit is set by other boot code before U-Boot (hence not every board might need this). ARMv8 Cortex cores have this bit as well, just in the CPUECTLR register. So looking at arm64, I find (in arch/arm/cpu/armv8/Kconfig): ============ config ARMV8_SET_SMPEN bool "Enable data coherency with other cores in cluster" help Say Y here if there is not any trust firmware to set CPUECTLR_EL1.SMPEN bit before U-Boot. .... ============
So I'd suggest to piggy back on this, either by moving this Kconfig option to some shared ARM code path, or by replicating this option to some ARMv7 Kconfig file.
Ok. Since there is no Cortex-A[x] revision check in U-Boot, need to first add the code.
Mmh, do you mean auto-detection during runtime? As in: if (MIDR == CortexA7) set_smp_bit();
The problem with this is that U-Boot might not be able to access the register (access to ACTLR from non-secure world can be forbidden by secure world).
So I'd go with a Kconfig option. Boards or platforms that need it could just select it. We just need to find a nice place to put the code, maybe in arch/arm/cpu/armv7/start.S. And another problem is that is needs to be set on every core in the system (like arch/arm/cpu/armv7/psci.S does already in psci_enable_smp). So it cannot really work for systems without PSCI or those not bringing all cores into U-Boot.
Cheers, Andre.
Then set ACTLR.SMP for A7 SMP.
Regards, Peng.
participants (4)
-
André Przywara -
Fabio Estevam -
Peng Fan -
Stefano Babic