[U-Boot] [PATCH] arm: imx: common rework cache settings for imx6

[Alonso Adrian]

Hi Peng,

See my comments below.

> -----Original Message-----
> From: Peng Fan [mailto:b51431 at freescale.com]
> Sent: Wednesday, August 26, 2015 8:34 PM
> To: Alonso Lazcano Adrian-B38018 <aalonso at freescale.com>
> Cc: u-boot at lists.denx.de; sbabic at denx.de; Estevam Fabio-R49496
> <Fabio.Estevam at freescale.com>; otavio at ossystems.com.br
> Subject: Re: [U-Boot] [PATCH] arm: imx: common rework cache settings for
> imx6
> 
> Hi Adrian,
> 
> On Wed, Aug 26, 2015 at 06:27:42PM -0500, Adrian Alonso wrote:
> >Rework cache settings for imx6, move cache configuration to
> >imx-common/cache.c so it can be reused for newer SoC
> 
> Better not do this now.
> See below comments.
> 
> >
> >Signed-off-by: Adrian Alonso <aalonso at freescale.com>
> >---
> > arch/arm/cpu/armv7/mx6/soc.c |  95 ---------------------------------------
> > arch/arm/imx-common/Makefile |   1 +
> > arch/arm/imx-common/cache.c  | 103
> >+++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 104 insertions(+), 95 deletions(-)  create mode
> >100644 arch/arm/imx-common/cache.c
> >
> 
> [...]
> 
> >
> >-#ifndef CONFIG_SYS_DCACHE_OFF
> >-void enable_caches(void)
> 
> Different imx SoCs have different implementation for this.
[Adrian] True, but cache settings are the same for imx6 and imx7 and would like to reuse for both SoC family platforms.
> 
> >-{
> >-#if defined(CONFIG_SYS_ARM_CACHE_WRITETHROUGH)
> >-	enum dcache_option option = DCACHE_WRITETHROUGH;
> >-#else
> >-	enum dcache_option option = DCACHE_WRITEBACK;
> >-#endif
> >-
> >-	/* Avoid random hang when download by usb */
> >-	invalidate_dcache_all();
> >-
> >-	/* Enable D-cache. I-cache is already enabled in start.S */
> >-	dcache_enable();
> >-
> >-	/* Enable caching on OCRAM and ROM */
> >-	mmu_set_region_dcache_behaviour(ROMCP_ARB_BASE_ADDR,
> >-					ROMCP_ARB_END_ADDR,
> >-					option);
> >-	mmu_set_region_dcache_behaviour(IRAM_BASE_ADDR,
> >-					IRAM_SIZE,
> >-					option);
> >-}
> >-#endif
> >-
> 
> From my understanding, only i.MX6 have this pl310 support.
> There is no need to move the code to imx-common.
 [Adrian] Yes that's the case, that's why I added the macro check for CONFIG_SYS_L2_PL310
To just enable v7_outer_chache_xx functions for iMX6 who defines PL310 macro.
> 
> >-
> >-#ifndef CONFIG_SYS_L2CACHE_OFF
> >-#define IOMUXC_GPR11_L2CACHE_AS_OCRAM 0x00000002 -void
> >v7_outer_cache_enable(void) -{
> >-	struct pl310_regs *const pl310 = (struct pl310_regs *)L2_PL310_BASE;
> >-	unsigned int val;
> >-
> >-
> >-	/*
> >-	 * Set bit 22 in the auxiliary control register. If this bit
> >-	 * is cleared, PL310 treats Normal Shared Non-cacheable
> >-	 * accesses as Cacheable no-allocate.
> >-	 */
> >-	setbits_le32(&pl310->pl310_aux_ctrl,
> L310_SHARED_ATT_OVERRIDE_ENABLE);
> >-
> >-#if defined CONFIG_MX6SL
> >-	struct iomuxc *iomux = (struct iomuxc *)IOMUXC_BASE_ADDR;
> >-	val = readl(&iomux->gpr[11]);
> >-	if (val & IOMUXC_GPR11_L2CACHE_AS_OCRAM) {
> >-		/* L2 cache configured as OCRAM, reset it */
> >-		val &= ~IOMUXC_GPR11_L2CACHE_AS_OCRAM;
> >-		writel(val, &iomux->gpr[11]);
> >-	}
> >-#endif
> >-
> >-	/* Must disable the L2 before changing the latency parameters */
> >-	clrbits_le32(&pl310->pl310_ctrl, L2X0_CTRL_EN);
> >-
> >-	writel(0x132, &pl310->pl310_tag_latency_ctrl);
> >-	writel(0x132, &pl310->pl310_data_latency_ctrl);
> >-
> >-	val = readl(&pl310->pl310_prefetch_ctrl);
> >-
> >-	/* Turn on the L2 I/D prefetch */
> >-	val |= 0x30000000;
> >-
> >-	/*
> >-	 * The L2 cache controller(PL310) version on the i.MX6D/Q is r3p1-
> 50rel0
> >-	 * The L2 cache controller(PL310) version on the i.MX6DL/SOLO/SL is
> r3p2
> >-	 * But according to ARM PL310 errata: 752271
> >-	 * ID: 752271: Double linefill feature can cause data corruption
> >-	 * Fault Status: Present in: r3p0, r3p1, r3p1-50rel0. Fixed in r3p2
> >-	 * Workaround: The only workaround to this erratum is to disable the
> >-	 * double linefill feature. This is the default behavior.
> >-	 */
> >-
> >-#ifndef CONFIG_MX6Q
> >-	val |= 0x40800000;
> >-#endif
> >-	writel(val, &pl310->pl310_prefetch_ctrl);
> >-
> >-	val = readl(&pl310->pl310_power_ctrl);
> >-	val |= L2X0_DYNAMIC_CLK_GATING_EN;
> >-	val |= L2X0_STNDBY_MODE_EN;
> >-	writel(val, &pl310->pl310_power_ctrl);
> >-
> >-	setbits_le32(&pl310->pl310_ctrl, L2X0_CTRL_EN);
> >-}
> >-
> >-void v7_outer_cache_disable(void)
> >-{
> >-	struct pl310_regs *const pl310 = (struct pl310_regs *)L2_PL310_BASE;
> >-
> >-	clrbits_le32(&pl310->pl310_ctrl, L2X0_CTRL_EN);
> >-}
> >-#endif /* !CONFIG_SYS_L2CACHE_OFF */
> >diff --git a/arch/arm/imx-common/Makefile
> >b/arch/arm/imx-common/Makefile index b9f1ca4..3b034e1 100644
> >--- a/arch/arm/imx-common/Makefile
> >+++ b/arch/arm/imx-common/Makefile
> >@@ -19,6 +19,7 @@ obj-y	+= misc.o
> > obj-$(CONFIG_SPL_BUILD)	+= spl.o
> > endif
> > ifeq ($(SOC),$(filter $(SOC),mx6))
> >+obj-y 	+= cache.o
> 
> Only mx6. Then there is no need to move the code imx-common.
> From the long run, we may move most code to saying arch/arm/mach-imx in
> future. But now I think we let the code be there.
[Adrian] I was thinking on just adding in a new patch the filter for iMX7 soc to also use the cache common function.
ifeq ($(SOC),$(filter $(SOC),mx6 mx7))
obj-y 	+= cache.o
endif
> 
> [...]
> 
> Regards,
> Peng.
> --