arm: imx: common rework cache settings for imx6

Hi Adrian,

Since this is only for mx6, why move the code to imx-common? "

arch/arm/imx-common/Makefile obj-y += misc.o · line 22

Quoted 3 lines

ifeq ($(SOC),$(filter $(SOC),mx6))

arch/arm/imx-common/Makefile +1 -1

-obj-y         += cache.o
+obj-y         += cache.o init.o

"

Also many pieces of code are only for imx6, imx7 do not need them for now, such as the ldo ramp part.

I think basic imx7 support do not need such a big change, if we do need to consolidate code for common usage, we can do this when we need to add more stuff.

Quoted 12 lines · Peng Fan

Hi Adrian,

Since this is only for mx6, why move the code to imx-common? "

arch/arm/imx-common/Makefile obj-y += misc.o · line 22

Quoted 3 lines

ifeq ($(SOC),$(filter $(SOC),mx6))

arch/arm/imx-common/Makefile +1 -1

-obj-y         += cache.o
+obj-y         += cache.o init.o

"

Also many pieces of code are only for imx6, imx7 do not need them for now, such as the ldo ramp part.

Quoted 3 lines

I think basic imx7 support do not need such a big change, if we do need to consolidate code for common usage, we can do this when we need to add more stuff.

Personally I think it's better off being done from the outset else it never gets done and we end up in the situation we were for imx6.

Peter

-----Original Message-----
From: Peng Fan [mailto:b51431 at freescale.com]
Sent: Thursday, August 27, 2015 3:16 AM
To: Peter Robinson <[email protected]>
Cc: Alonso Lazcano Adrian-B38018 <[email protected]>; Estevam Fabio- R49496 <[email protected]>; u-boot at lists.denx.de; otavio at ossystems.com.br
Subject: Re: [U-Boot] [PATCH] arm: imx: imx-common: init: move arch init common setup

Quoted 13 lines · Peter Robinson

Quoted 12 lines · Peng Fan

Hi Adrian,

Since this is only for mx6, why move the code to imx-common? "

arch/arm/imx-common/Makefile obj-y += misc.o · line 22

Quoted 3 lines

ifeq ($(SOC),$(filter $(SOC),mx6))

arch/arm/imx-common/Makefile +1 -1

-obj-y         += cache.o
+obj-y         += cache.o init.o

"

Also many pieces of code are only for imx6, imx7 do not need them for now, such as the ldo ramp part.

Quoted 6 lines

Quoted 3 lines

I think basic imx7 support do not need such a big change, if we do need to consolidate code for common usage, we can do this when we need to add more stuff.

Personally I think it's better off being done from the outset else it never gets done and we end up in the situation we were for imx6.

I agree that we'd better remove duplicated code and consolidate code for common usage to make it cleaner and easy to maintian.

But from the current patch,
clear_ldo_ramp
set_ldo_voltage
set_ahb_rate
clear_mmdc_ch_mask
init_bandgap
set_preclk_from_osc
are all imx6 specific, imx7 do not have the implementation for these functions for now.

The goal of arch_cpu_init is for different SoC usage from my understanding, I think it should be there for different SoCs.

The following functions can be consolidated for common usage. boot_mode_apply
init_src
init_aips

The Makefile part shows only for imx6 "ifeq ($(SOC),$(filter $(SOC),mx6))", from this patch. Then why move most code mainly imx6 specific to imx- common?

If we do need such a patch for now, init_aips and boot_mode_apply and init_src may can be consolidated for common usage for imx6/7. But anyway I prefer a small cleanup patch to do this after basic imx7 upstreamed.

Regards,
Peng.

Quoted 2 lines

Peter

--

Quoted 4 lines

If we do need such a patch for now, init_aips and boot_mode_apply and init_src may can be consolidated for common usage for imx6/7. But anyway I prefer a small cleanup patch to do this after basic imx7 upstreamed.

[Adrian] iMX7 upstream is been kept on hold due to feedback to try to reuse as much of Common code as possible, some drivers (thermal, usb) had already reworked to Support both imx6 and imx7; I think we need to push for:
"arm: imx: common rework cache settings for imx6" and this patch (version 2 will be sent ASAP) So basic imx7 support ca be accepted.

Rework cache settings for imx6, move cache configuration to imx-common/cache.c so it can be reused for newer SoC

Signed-off-by Adrian Alonso <[email protected]>

---

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

arch/arm/cpu/armv7/mx6/soc.c +0 -13

-{
-#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();
-

arch/arm/cpu/armv7/mx6/soc.c +0 -67

-
-#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 */

arch/arm/imx-common/Makefile +0 -0

index b9f1ca4..3b034e1 100644
@@ -19,6 +19,7 @@ obj-y	+= misc.o

obj-$(CONFIG_SPL_BUILD) += spl.o
endif
ifeq ($(SOC),$(filter $(SOC),mx6))
+obj-y += cache.o

-----Original Message-----
From: Peng Fan [mailto:b51431 at freescale.com]
Sent: Wednesday, August 26, 2015 8:34 PM
To: Alonso Lazcano Adrian-B38018 <[email protected]>
Cc: u-boot at lists.denx.de; sbabic at denx.de; Estevam Fabio-R49496 <[email protected]>; otavio at ossystems.com.br
Subject: Re: [U-Boot] [PATCH] arm: imx: common rework cache settings for imx6

Hi Adrian,

Quoted 2 lines · Adrian Alonso

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.

Quoted 10 lines

Signed-off-by Adrian Alonso <[email protected]>

---

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

[...]

arch/arm/cpu/armv7/mx6/soc.c int board_postclk_init(void) · line 444

Quoted 3 lines

arch/arm/cpu/armv7/mx6/soc.c +0 -2

-#ifndef CONFIG_SYS_DCACHE_OFF
-void enable_caches(void)

Different imx SoCs have different implementation for this.

Quoted 19 lines

arch/arm/cpu/armv7/mx6/soc.c +0 -18

-{
-#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,

arch/arm/cpu/armv7/mx6/soc.c int board_postclk_init(void) · line 467

Quoted 5 lines

arch/arm/cpu/armv7/mx6/soc.c +0 -5

-					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.

Quoted 5 lines

arch/arm/cpu/armv7/mx6/soc.c +0 -3

-
-#ifndef CONFIG_SYS_L2CACHE_OFF
-#define IOMUXC_GPR11_L2CACHE_AS_OCRAM 0x00000002 -void

v7_outer_cache_enable(void) -{

arch/arm/cpu/armv7/mx6/soc.c void imx_setup_hdmi(void)

Quoted 10 lines

arch/arm/cpu/armv7/mx6/soc.c +0 -10

-	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);

Quoted 14 lines

arch/arm/cpu/armv7/mx6/soc.c +0 -14

-
-#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);
-

arch/arm/cpu/armv7/mx6/soc.c void imx_setup_hdmi(void)

Quoted 10 lines

arch/arm/cpu/armv7/mx6/soc.c +0 -10

-	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

Review

Quoted 1 line

(quoted diff) +0 -1

-	 * The L2 cache controller(PL310) version on the i.MX6DL/SOLO/SL is

r3p2

Quoted 32 lines

arch/arm/cpu/armv7/mx6/soc.c +0 -27

-	 * 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 */

(quoted diff) +0 -0

b/arch/arm/imx-common/Makefile index b9f1ca4..3b034e1 100644

arch/arm/imx-common/Makefile +0 -0

@@ -19,6 +19,7 @@ obj-y	+= misc.o

Quoted 4 lines

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.

[...]

Regards,
Peng.
--