Hi Peng,

See my comments below.

> -----Original Message-----
> From: Peng Fan [mailto:b51...@freescale.com]
> Sent: Wednesday, August 26, 2015 8:34 PM
> To: Alonso Lazcano Adrian-B38018 <aalo...@freescale.com>
> Cc: u-boot@lists.denx.de; sba...@denx.de; Estevam Fabio-R49496
> <fabio.este...@freescale.com>; ota...@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 <aalo...@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.
> --
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to