[PATCH v3 0/5] i.MX L2-cache code cleanups and performance tweaks

[PATCH v3 0/5] i.MX L2-cache code cleanups and performance tweaks

[Andrey Smirnov]

Hi everyone,

Here's the second version of i.MX L2-cache related patches. Here's
what's new from v2:

       - Generic ARM patches were split inot a separte series

       - Typo and whitespace fixes (as per Uwe's suggestions)

       - Re-ordered l2c_aux_val, and l2c_aux_mask are initializers are placed
         in the order of their declaration

Let me know if any more changes to the series are needed.

Andrey Smirnov

Andrey Smirnov (5):
  i.MX: system.c: Convert goto to if statement
  i.MX: system.c: Remove redundant errata 752271 code
  i.MX: system.c: Replace magic numbers
  i.MX: system.c: Tweak prefetch settings for performance
  i.MX: Do not explicitly call l2x0_of_init()

 arch/arm/mach-imx/imx35-dt.c    | 10 +++-------
 arch/arm/mach-imx/mach-imx6q.c  |  2 ++
 arch/arm/mach-imx/mach-imx6sl.c |  2 ++
 arch/arm/mach-imx/mach-imx6sx.c |  2 ++
 arch/arm/mach-imx/system.c      | 42 ++++++++++++++++-------------------------
 5 files changed, 25 insertions(+), 33 deletions(-)

-- 
2.5.5

[PATCH 1/2] ARM: cache-l2x0.c: Replace magic numbers

[Andrey Smirnov]

Replace magic numbers used for L310 Prefetch Control Register

Acked-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/mm/cache-l2x0.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 9f9d542..30e2012 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -716,8 +716,10 @@ static void __init l2c310_fixup(void __iomem *base, u32 cache_id,
 	    revision < L310_CACHE_ID_RTL_R3P2) {
 		u32 val = l2x0_saved_regs.prefetch_ctrl;
 		/* I don't think bit23 is required here... but iMX6 does so */
-		if (val & (BIT(30) | BIT(23))) {
-			val &= ~(BIT(30) | BIT(23));
+		if (val & (L310_PREFETCH_CTRL_DBL_LINEFILL |
+			   L310_PREFETCH_CTRL_DBL_LINEFILL_INCR)) {
+			val &= ~(L310_PREFETCH_CTRL_DBL_LINEFILL |
+				 L310_PREFETCH_CTRL_DBL_LINEFILL_INCR);
 			l2x0_saved_regs.prefetch_ctrl = val;
 			errata[n++] = "752271";
 		}
-- 
2.5.5

[PATCH v3 1/5] i.MX: system.c: Convert goto to if statement

[Andrey Smirnov]

Using goto here doesn't bring any advantages and only makes the code
flow less clear. No functional changes.

Acked-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/mach-imx/system.c | 36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/arch/arm/mach-imx/system.c b/arch/arm/mach-imx/system.c
index 105d1ce..d9f8b0e 100644
--- a/arch/arm/mach-imx/system.c
+++ b/arch/arm/mach-imx/system.c
@@ -106,26 +106,24 @@ void __init imx_init_l2cache(void)
 		goto out;
 	}
 
-	if (readl_relaxed(l2x0_base + L2X0_CTRL) & L2X0_CTRL_EN)
-		goto skip_if_enabled;
-
-	/* Configure the L2 PREFETCH and POWER registers */
-	val = readl_relaxed(l2x0_base + L310_PREFETCH_CTRL);
-	val |= 0x70800000;
-	/*
-	 * 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.
-	 */
-	if (cpu_is_imx6q())
-		val &= ~(1 << 30 | 1 << 23);
-	writel_relaxed(val, l2x0_base + L310_PREFETCH_CTRL);
+	if (!(readl_relaxed(l2x0_base + L2X0_CTRL) & L2X0_CTRL_EN)) {
+		/* Configure the L2 PREFETCH and POWER registers */
+		val = readl_relaxed(l2x0_base + L310_PREFETCH_CTRL);
+		val |= 0x70800000;
+		/*
+		 * 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.
+		 */
+		if (cpu_is_imx6q())
+			val &= ~(1 << 30 | 1 << 23);
+		writel_relaxed(val, l2x0_base + L310_PREFETCH_CTRL);
+	}
 
-skip_if_enabled:
 	iounmap(l2x0_base);
 	of_node_put(np);
 
-- 
2.5.5

[PATCH 2/2] ARM: cache-l2x0.c: Do not clear bit 23 in prefetch control register

[Andrey Smirnov]

As per L2C-310 TRM[1]:

"... You can control this feature using bits 30,27 and 23 of the
Prefetch Control Register. Bit 23 and 27 are only used if you set bit 30
HIGH..."

which means there is no need to clear bit 23 if bit 30 is being cleared.

[1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0246e/CJAJACBJ.html

Acked-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/mm/cache-l2x0.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 30e2012..12c1ba7 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -715,11 +715,8 @@ static void __init l2c310_fixup(void __iomem *base, u32 cache_id,
 	if (revision >= L310_CACHE_ID_RTL_R3P0 &&
 	    revision < L310_CACHE_ID_RTL_R3P2) {
 		u32 val = l2x0_saved_regs.prefetch_ctrl;
-		/* I don't think bit23 is required here... but iMX6 does so */
-		if (val & (L310_PREFETCH_CTRL_DBL_LINEFILL |
-			   L310_PREFETCH_CTRL_DBL_LINEFILL_INCR)) {
-			val &= ~(L310_PREFETCH_CTRL_DBL_LINEFILL |
-				 L310_PREFETCH_CTRL_DBL_LINEFILL_INCR);
+		if (val & L310_PREFETCH_CTRL_DBL_LINEFILL) {
+			val &= ~L310_PREFETCH_CTRL_DBL_LINEFILL;
 			l2x0_saved_regs.prefetch_ctrl = val;
 			errata[n++] = "752271";
 		}
-- 
2.5.5

[PATCH v3 2/5] i.MX: system.c: Remove redundant errata 752271 code

[Andrey Smirnov]

Applying a fix for ARM errata 752271 would already be taken care by a
call to a 'fixup' hook as a part of l2x0_of_init() -> __l2c_init() call
chain. Moreso the code in 'fixup' function would do that based on the
PL310's revsion information, whereas removed code does so based on SoC
version which does not work very well on i.MX6Q+ which identifies itself
as i.MX6Q as well but is not affected by 752271.

Acked-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/mach-imx/system.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/arch/arm/mach-imx/system.c b/arch/arm/mach-imx/system.c
index d9f8b0e..b153376 100644
--- a/arch/arm/mach-imx/system.c
+++ b/arch/arm/mach-imx/system.c
@@ -110,17 +110,6 @@ void __init imx_init_l2cache(void)
 		/* Configure the L2 PREFETCH and POWER registers */
 		val = readl_relaxed(l2x0_base + L310_PREFETCH_CTRL);
 		val |= 0x70800000;
-		/*
-		 * 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.
-		 */
-		if (cpu_is_imx6q())
-			val &= ~(1 << 30 | 1 << 23);
 		writel_relaxed(val, l2x0_base + L310_PREFETCH_CTRL);
 	}
 
-- 
2.5.5

[PATCH v3 3/5] i.MX: system.c: Replace magic numbers

[Andrey Smirnov]

Replace magic numbers used to form L310 Prefetch Control Register value.

Acked-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/mach-imx/system.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/system.c b/arch/arm/mach-imx/system.c
index b153376..7ba9dc8 100644
--- a/arch/arm/mach-imx/system.c
+++ b/arch/arm/mach-imx/system.c
@@ -109,7 +109,10 @@ void __init imx_init_l2cache(void)
 	if (!(readl_relaxed(l2x0_base + L2X0_CTRL) & L2X0_CTRL_EN)) {
 		/* Configure the L2 PREFETCH and POWER registers */
 		val = readl_relaxed(l2x0_base + L310_PREFETCH_CTRL);
-		val |= 0x70800000;
+		val |= L310_PREFETCH_CTRL_DBL_
+			L310_PREFETCH_CTRL_INSTR_PREFETCH |
+			L310_PREFETCH_CTRL_DATA_PREFETCH |
+			L310_PREFETCH_CTRL_DBL_LINEFILL_INCR;
 		writel_relaxed(val, l2x0_base + L310_PREFETCH_CTRL);
 	}
 
-- 
2.5.5

[PATCH v3 4/5] i.MX: system.c: Tweak prefetch settings for performance

[Andrey Smirnov]

Update Prefetch Control Register settings to match that of Freescale's
Linux tree. As the commit e3addf1b773964eac7f797e8538c69481be4279c
states (author Nitin Garg):

"... set Prefetch offset to 15, since it improves memcpy performance by
35%. Don't enable Incr double Linefill enable since it adversely affects
memcpy performance by about 32MB/s and reads by 90MB/s. Tested with 4K
to 16MB sized src and dst aligned buffer..."

Those results are also corroborated by our own testing.

Acked-by: Arnd Bergmann <arnd@arndb.de>
Tested-by: Chris Healy <cphealy@gmail.com>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/mach-imx/system.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/system.c b/arch/arm/mach-imx/system.c
index 7ba9dc8..d1300b9 100644
--- a/arch/arm/mach-imx/system.c
+++ b/arch/arm/mach-imx/system.c
@@ -111,8 +111,12 @@ void __init imx_init_l2cache(void)
 		val = readl_relaxed(l2x0_base + L310_PREFETCH_CTRL);
 		val |= L310_PREFETCH_CTRL_DBL_
 			L310_PREFETCH_CTRL_INSTR_PREFETCH |
-			L310_PREFETCH_CTRL_DATA_PREFETCH |
-			L310_PREFETCH_CTRL_DBL_LINEFILL_INCR;
+			L310_PREFETCH_CTRL_DATA_PREFETCH;
+
+		/* Set perfetch offset to improve performance */
+		val &= ~L310_PREFETCH_CTRL_OFFSET_MASK;
+		val |= 15;
+
 		writel_relaxed(val, l2x0_base + L310_PREFETCH_CTRL);
 	}
 
-- 
2.5.5

[PATCH v3 5/5] i.MX: Do not explicitly call l2x0_of_init()

[Andrey Smirnov]

There's no need to explicitly call l2x0_of_init() since it will be
called as a part of init_IRQ() (see arch/arm/kernel/irq.c for
details). This way we can simplify imx_init_l2cache() and ditch the call
to it on i.MX35 (which does not claim compatibility with
"arm,pl310-cache") alltogether.

Acked-by: Arnd Bergmann <arnd@arndb.de>
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/mach-imx/imx35-dt.c    | 10 +++-------
 arch/arm/mach-imx/mach-imx6q.c  |  2 ++
 arch/arm/mach-imx/mach-imx6sl.c |  2 ++
 arch/arm/mach-imx/mach-imx6sx.c |  2 ++
 arch/arm/mach-imx/system.c      | 12 ++++--------
 5 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-imx/imx35-dt.c b/arch/arm/mach-imx/imx35-dt.c
index e939603..99bb63d 100644
--- a/arch/arm/mach-imx/imx35-dt.c
+++ b/arch/arm/mach-imx/imx35-dt.c
@@ -20,20 +20,16 @@
 #include "common.h"
 #include "mx35.h"
 
-static void __init imx35_irq_init(void)
-{
-	imx_init_l2cache();
-	mx35_init_irq();
-}
-
 static const char * const imx35_dt_board_compat[] __initconst = {
 	"fsl,imx35",
 	NULL
 };
 
 DT_MACHINE_START(IMX35_DT, "Freescale i.MX35 (Device Tree Support)")
+	.l2c_aux_val 	= 0,
+	.l2c_aux_mask	= ~0,
 	.map_io		= mx35_map_io,
 	.init_early	= imx35_init_early,
-	.init_irq	= imx35_irq_init,
+	.init_irq	= mx35_init_irq,
 	.dt_compat	= imx35_dt_board_compat,
 MACHINE_END
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index cb27d56..b31890f 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -407,6 +407,8 @@ static const char * const imx6q_dt_compat[] __initconst = {
 };
 
 DT_MACHINE_START(IMX6Q, "Freescale i.MX6 Quad/DualLite (Device Tree)")
+	.l2c_aux_val 	= 0,
+	.l2c_aux_mask	= ~0,
 	.smp		= smp_ops(imx_smp_ops),
 	.map_io		= imx6q_map_io,
 	.init_irq	= imx6q_init_irq,
diff --git a/arch/arm/mach-imx/mach-imx6sl.c b/arch/arm/mach-imx/mach-imx6sl.c
index 3003263..f9a9a36 100644
--- a/arch/arm/mach-imx/mach-imx6sl.c
+++ b/arch/arm/mach-imx/mach-imx6sl.c
@@ -75,6 +75,8 @@ static const char * const imx6sl_dt_compat[] __initconst = {
 };
 
 DT_MACHINE_START(IMX6SL, "Freescale i.MX6 SoloLite (Device Tree)")
+	.l2c_aux_val 	= 0,
+	.l2c_aux_mask	= ~0,
 	.init_irq	= imx6sl_init_irq,
 	.init_machine	= imx6sl_init_machine,
 	.init_late      = imx6sl_init_late,
diff --git a/arch/arm/mach-imx/mach-imx6sx.c b/arch/arm/mach-imx/mach-imx6sx.c
index 6a0b061..07a3a34 100644
--- a/arch/arm/mach-imx/mach-imx6sx.c
+++ b/arch/arm/mach-imx/mach-imx6sx.c
@@ -103,6 +103,8 @@ static const char * const imx6sx_dt_compat[] __initconst = {
 };
 
 DT_MACHINE_START(IMX6SX, "Freescale i.MX6 SoloX (Device Tree)")
+	.l2c_aux_val 	= 0,
+	.l2c_aux_mask	= ~0,
 	.init_irq	= imx6sx_init_irq,
 	.init_machine	= imx6sx_init_machine,
 	.dt_compat	= imx6sx_dt_compat,
diff --git a/arch/arm/mach-imx/system.c b/arch/arm/mach-imx/system.c
index d1300b9..a6212f0 100644
--- a/arch/arm/mach-imx/system.c
+++ b/arch/arm/mach-imx/system.c
@@ -98,13 +98,11 @@ void __init imx_init_l2cache(void)
 
 	np = of_find_compatible_node(NULL, NULL, "arm,pl310-cache");
 	if (!np)
-		goto out;
+		return;
 
 	l2x0_base = of_iomap(np, 0);
-	if (!l2x0_base) {
-		of_node_put(np);
-		goto out;
-	}
+	if (!l2x0_base)
+		goto put_node;
 
 	if (!(readl_relaxed(l2x0_base + L2X0_CTRL) & L2X0_CTRL_EN)) {
 		/* Configure the L2 PREFETCH and POWER registers */
@@ -121,9 +119,7 @@ void __init imx_init_l2cache(void)
 	}
 
 	iounmap(l2x0_base);
+put_node:
 	of_node_put(np);
-
-out:
-	l2x0_of_init(0, ~0);
 }
 #endif
-- 
2.5.5

[PATCH v3 0/5] i.MX L2-cache code cleanups and performance tweaks

[Andrey Smirnov]

Please ignore this version of the patch set, I just realized that I
screwed up patch 3/5. I'll be sending fixed v4 shortly.

Sorry for the noise,
Andrey Smirnov

On Sat, Jun 18, 2016 at 5:31 PM, Andrey Smirnov
<andrew.smirnov@gmail.com> wrote:
> Hi everyone,
>
> Here's the second version of i.MX L2-cache related patches. Here's
> what's new from v2:
>
>        - Generic ARM patches were split inot a separte series
>
>        - Typo and whitespace fixes (as per Uwe's suggestions)
>
>        - Re-ordered l2c_aux_val, and l2c_aux_mask are initializers are placed
>          in the order of their declaration
>
> Let me know if any more changes to the series are needed.
>
> Andrey Smirnov
>
> Andrey Smirnov (5):
>   i.MX: system.c: Convert goto to if statement
>   i.MX: system.c: Remove redundant errata 752271 code
>   i.MX: system.c: Replace magic numbers
>   i.MX: system.c: Tweak prefetch settings for performance
>   i.MX: Do not explicitly call l2x0_of_init()
>
>  arch/arm/mach-imx/imx35-dt.c    | 10 +++-------
>  arch/arm/mach-imx/mach-imx6q.c  |  2 ++
>  arch/arm/mach-imx/mach-imx6sl.c |  2 ++
>  arch/arm/mach-imx/mach-imx6sx.c |  2 ++
>  arch/arm/mach-imx/system.c      | 42 ++++++++++++++++-------------------------
>  5 files changed, 25 insertions(+), 33 deletions(-)
>
> --
> 2.5.5
>

[PATCH v3 3/5] i.MX: system.c: Replace magic numbers

[kbuild test robot]

Hi,

[auto build test ERROR on shawnguo/for-next]
[also build test ERROR on v4.7-rc3 next-20160617]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Andrey-Smirnov/i-MX-L2-cache-code-cleanups-and-performance-tweaks/20160619-083620
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git for-next
config: arm-multi_v7_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 5.3.1-8) 5.3.1 20160205
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   arch/arm/mach-imx/system.c: In function 'imx_init_l2cache':
>> arch/arm/mach-imx/system.c:112:10: error: implicit declaration of function 'L310_PREFETCH_CTRL_DBL_' [-Werror=implicit-function-declaration]
      val |= L310_PREFETCH_CTRL_DBL_
             ^
   cc1: some warnings being treated as errors

vim +/L310_PREFETCH_CTRL_DBL_ +112 arch/arm/mach-imx/system.c

   106			goto out;
   107		}
   108	
   109		if (!(readl_relaxed(l2x0_base + L2X0_CTRL) & L2X0_CTRL_EN)) {
   110			/* Configure the L2 PREFETCH and POWER registers */
   111			val = readl_relaxed(l2x0_base + L310_PREFETCH_CTRL);
 > 112			val |= L310_PREFETCH_CTRL_DBL_
   113				L310_PREFETCH_CTRL_INSTR_PREFETCH |
   114				L310_PREFETCH_CTRL_DATA_PREFETCH |
   115				L310_PREFETCH_CTRL_DBL_LINEFILL_INCR;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 37700 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160619/fa9cf85e/attachment-0001.obj>

[PATCH v3 0/5] i.MX L2-cache code cleanups and performance tweaks

[Arnd Bergmann]

On Saturday, June 18, 2016 5:31:01 PM CEST Andrey Smirnov wrote:
> Hi everyone,
> 
> Here's the second version of i.MX L2-cache related patches. Here's
> what's new from v2:
> 
>        - Generic ARM patches were split inot a separte series
> 
>        - Typo and whitespace fixes (as per Uwe's suggestions)
> 
>        - Re-ordered l2c_aux_val, and l2c_aux_mask are initializers are placed
>          in the order of their declaration
> 
> Let me know if any more changes to the series are needed.

One more question, less about your patches than the existing code:

After going through the current users of l2x0_init for board files,
I noticed that imx35 enables l2x0 for both dts and all the board files
and has an arm,l210-cache node in its dts, but imx31 only enables
it for the board file and in the dt case doesn't init the cache nor
does it have the dts node.

I'm guessing this is a bug on i.mx31 dt support, right?

	Arnd

[PATCH v3 0/5] i.MX L2-cache code cleanups and performance tweaks

[Andrey Smirnov]

 One more question, less about your patches than the existing code:
>
> After going through the current users of l2x0_init for board files,
> I noticed that imx35 enables l2x0 for both dts and all the board files
> and has an arm,l210-cache node in its dts, but imx31 only enables
> it for the board file and in the dt case doesn't init the cache nor
> does it have the dts node.
>
> I'm guessing this is a bug on i.mx31 dt support, right?

I would agree that it is. OTOH, I can't find any i.MX31 boards that
use device tree. The only file that references imx31.dtsi is
imx31-bug.dts, but at the same time that board has a dedicated board
file (mach-bug.c) so I wonder if it was ever used.

It looks like Sacha was the author of i.MX31, Sascha do you have any
comment on this?

There's also another small cleanup opportunity in collapsing
imx31_dt_timer_init() and mx31_clocks_init_dt() into a single function
given how the latter always returns 0 and can be converted to void.

I am more than happy to make both changes and include them in the set,
but I only have i.MX6 HW, and would only be able to do a compile-test,
so I am not sure if I should.

Thanks,
Andrey Smirnov

[PATCH v3 0/5] i.MX L2-cache code cleanups and performance tweaks

[Arnd Bergmann]

On Sunday, June 19, 2016 10:53:13 AM CEST Andrey Smirnov wrote:
>  One more question, less about your patches than the existing code:
> >
> > After going through the current users of l2x0_init for board files,
> > I noticed that imx35 enables l2x0 for both dts and all the board files
> > and has an arm,l210-cache node in its dts, but imx31 only enables
> > it for the board file and in the dt case doesn't init the cache nor
> > does it have the dts node.
> >
> > I'm guessing this is a bug on i.mx31 dt support, right?
> 
> I would agree that it is. OTOH, I can't find any i.MX31 boards that
> use device tree. The only file that references imx31.dtsi is
> imx31-bug.dts, but at the same time that board has a dedicated board
> file (mach-bug.c) so I wonder if it was ever used.

My understanding is that many of the i.mx dts files were introduced
a few years ago as conversions of the board files, and we are now
in the (slow) process of removing the board files, assuming that
everyone has had time for migration if they still run that hardware
on new kernels, or that it doesn't hurt if nobody uses it.

This one was added with the comment

    arm/dts: Add support for i.MX31 bug 1.x board from buglabs.
    
    Only the main UART and the memory node information are added.

and it's unclear if that has ever been tested. A few device nodes
were added to the imx31.dtsi file later, indicating that it probably
had /some/ testing, but no other board file was ever added as you
say. The only difference I see between mach-bug.c and imx31-bug.dts
is the configuration of the uart pins. I don't know if that is
required in the dts, because I'd assume that the boot loader would
leave the pinmux in a state in which the console uart works.

> It looks like Sacha was the author of i.MX31, Sascha do you have any
> comment on this?
> 
> There's also another small cleanup opportunity in collapsing
> imx31_dt_timer_init() and mx31_clocks_init_dt() into a single function
> given how the latter always returns 0 and can be converted to void.

Or we could go one step further and use CLK_OF_DECLARE() to remove
that init function entirely. Same thing for mx31_init_irq and
IRQCHIP_DECLARE().

> I am more than happy to make both changes and include them in the set,
> but I only have i.MX6 HW, and would only be able to do a compile-test,
> so I am not sure if I should.

Let's wait for Sascha to reply.

	Arnd