Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1420456 > unrolled thread

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

Started byAndrey Smirnov <andrew.smirnov@gmail.com>
First post2016-06-13 08:00 +0200
Last post2016-06-13 11:30 +0200
Articles 7 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/3] i.MX: system.c: Convert goto to if statement Andrey Smirnov <andrew.smirnov@gmail.com> - 2016-06-13 08:00 +0200
    [PATCH 3/3] i.MX: system.c: Replace magic numbers Andrey Smirnov <andrew.smirnov@gmail.com> - 2016-06-13 08:00 +0200
      Re: [PATCH 3/3] i.MX: system.c: Replace magic numbers Arnd Bergmann <arnd@arndb.de> - 2016-06-13 11:30 +0200
        Re: [PATCH 3/3] i.MX: system.c: Replace magic numbers Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-06-13 11:50 +0200
      Re: [PATCH 3/3] i.MX: system.c: Replace magic numbers Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-06-13 11:30 +0200
    [RESEND PATCH 2/3] i.MX: system.c: Remove redundant errata 752271 code Andrey Smirnov <andrew.smirnov@gmail.com> - 2016-06-13 08:00 +0200
    Re: [PATCH 1/3] i.MX: system.c: Convert goto to if statement Arnd Bergmann <arnd@arndb.de> - 2016-06-13 11:30 +0200

#1420456 — [PATCH 1/3] i.MX: system.c: Convert goto to if statement

FromAndrey Smirnov <andrew.smirnov@gmail.com>
Date2016-06-13 08:00 +0200
Subject[PATCH 1/3] i.MX: system.c: Convert goto to if statement
Message-ID<rJt4J-23X-3@gated-at.bofh.it>
Using goto here doesn't bring any advantages and only makes the code
flow less clear. No functional changes.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---

 RESEND, now with Russell King on CC list.

 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

[toc] | [next] | [standalone]

#1420458 — [PATCH 3/3] i.MX: system.c: Replace magic numbers

FromAndrey Smirnov <andrew.smirnov@gmail.com>
Date2016-06-13 08:00 +0200
Subject[PATCH 3/3] i.MX: system.c: Replace magic numbers
Message-ID<rJt4J-23X-7@gated-at.bofh.it>
In reply to#1420456
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---

RESEND, now with Russell King on CC list.

 arch/arm/include/asm/hardware/cache-l2x0.h | 7 +++++++
 arch/arm/mach-imx/system.c                 | 5 ++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/hardware/cache-l2x0.h b/arch/arm/include/asm/hardware/cache-l2x0.h
index 3a5ec1c..5e27162 100644
--- a/arch/arm/include/asm/hardware/cache-l2x0.h
+++ b/arch/arm/include/asm/hardware/cache-l2x0.h
@@ -61,6 +61,13 @@
 #define L2X0_LINE_TAG			0xF30
 #define L2X0_DEBUG_CTRL			0xF40
 #define L310_PREFETCH_CTRL		0xF60
+#define   L310_DOUBLE_LINEFILL_EN			BIT(30)
+#define   L310_INSTRUCTION_PREFETCH_EN			BIT(29)
+#define   L310_DATA_PREFETCH_EN				BIT(28)
+#define   L310_DOUBLE_LINEFILL_ON_WRAP_READ_DIS		BIT(27)
+#define   L310_PREFETCH_DROP_EN				BIT(24)
+#define   L310_INCR_DOUBLE_LINEFILL_EN			BIT(23)
+#define   L310_ESCLUSIVE_SEQUENCE_EN			BIT(21)
 #define L310_POWER_CTRL			0xF80
 #define   L310_DYNAMIC_CLK_GATING_EN	(1 << 1)
 #define   L310_STNDBY_MODE_EN		(1 << 0)
diff --git a/arch/arm/mach-imx/system.c b/arch/arm/mach-imx/system.c
index b153376..bd9a96b 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_DOUBLE_LINEFILL_EN |
+		        L310_INSTRUCTION_PREFETCH_EN |
+			L310_DATA_PREFETCH_EN |
+			L310_INCR_DOUBLE_LINEFILL_EN;
 		writel_relaxed(val, l2x0_base + L310_PREFETCH_CTRL);
 	}
 
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1420646 — Re: [PATCH 3/3] i.MX: system.c: Replace magic numbers

FromArnd Bergmann <arnd@arndb.de>
Date2016-06-13 11:30 +0200
SubjectRe: [PATCH 3/3] i.MX: system.c: Replace magic numbers
Message-ID<rJwlY-4ho-3@gated-at.bofh.it>
In reply to#1420458
On Sunday, June 12, 2016 10:55:29 PM CEST Andrey Smirnov wrote:
> @@ -61,6 +61,13 @@
>  #define L2X0_LINE_TAG                  0xF30
>  #define L2X0_DEBUG_CTRL                        0xF40
>  #define L310_PREFETCH_CTRL             0xF60
> +#define   L310_DOUBLE_LINEFILL_EN                      BIT(30)
> +#define   L310_INSTRUCTION_PREFETCH_EN                 BIT(29)
> +#define   L310_DATA_PREFETCH_EN                                BIT(28)
> +#define   L310_DOUBLE_LINEFILL_ON_WRAP_READ_DIS                BIT(27)
> +#define   L310_PREFETCH_DROP_EN                                BIT(24)
> +#define   L310_INCR_DOUBLE_LINEFILL_EN                 BIT(23)
> +#define   L310_ESCLUSIVE_SEQUENCE_EN                   BIT(21)
>  #define L310_POWER_CTRL                        0xF80
>  #define   L310_DYNAMIC_CLK_GATING_EN   (1 << 1)
>  #define   L310_STNDBY_MODE_EN          (1 << 0)
> diff --git a/arch/arm/mach-imx/system.c b/arch/arm/mach-imx/system.c
> 


Most of these macros seem to all exist further down in the file already
under slightly differnet names, e.g.

#define L310_PREFETCH_CTRL_DBL_LINEFILL_INCR   BIT(23)

	Arnd

[toc] | [prev] | [next] | [standalone]

#1420656 — Re: [PATCH 3/3] i.MX: system.c: Replace magic numbers

FromRussell King - ARM Linux <linux@armlinux.org.uk>
Date2016-06-13 11:50 +0200
SubjectRe: [PATCH 3/3] i.MX: system.c: Replace magic numbers
Message-ID<rJwFk-4on-5@gated-at.bofh.it>
In reply to#1420646
On Mon, Jun 13, 2016 at 11:24:02AM +0200, Arnd Bergmann wrote:
> On Sunday, June 12, 2016 10:55:29 PM CEST Andrey Smirnov wrote:
> > @@ -61,6 +61,13 @@
> >  #define L2X0_LINE_TAG                  0xF30
> >  #define L2X0_DEBUG_CTRL                        0xF40
> >  #define L310_PREFETCH_CTRL             0xF60
> > +#define   L310_DOUBLE_LINEFILL_EN                      BIT(30)
> > +#define   L310_INSTRUCTION_PREFETCH_EN                 BIT(29)
> > +#define   L310_DATA_PREFETCH_EN                                BIT(28)
> > +#define   L310_DOUBLE_LINEFILL_ON_WRAP_READ_DIS                BIT(27)
> > +#define   L310_PREFETCH_DROP_EN                                BIT(24)
> > +#define   L310_INCR_DOUBLE_LINEFILL_EN                 BIT(23)
> > +#define   L310_ESCLUSIVE_SEQUENCE_EN                   BIT(21)
> >  #define L310_POWER_CTRL                        0xF80
> >  #define   L310_DYNAMIC_CLK_GATING_EN   (1 << 1)
> >  #define   L310_STNDBY_MODE_EN          (1 << 0)
> > diff --git a/arch/arm/mach-imx/system.c b/arch/arm/mach-imx/system.c
> > 
> 
> 
> Most of these macros seem to all exist further down in the file already
> under slightly differnet names, e.g.
> 
> #define L310_PREFETCH_CTRL_DBL_LINEFILL_INCR   BIT(23)

Indeed they do.  Andrey, please use the existing definitions.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1420647 — Re: [PATCH 3/3] i.MX: system.c: Replace magic numbers

FromRussell King - ARM Linux <linux@armlinux.org.uk>
Date2016-06-13 11:30 +0200
SubjectRe: [PATCH 3/3] i.MX: system.c: Replace magic numbers
Message-ID<rJwlY-4ho-9@gated-at.bofh.it>
In reply to#1420458
On Sun, Jun 12, 2016 at 10:55:29PM -0700, Andrey Smirnov wrote:
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
> 
> RESEND, now with Russell King on CC list.
> 
>  arch/arm/include/asm/hardware/cache-l2x0.h | 7 +++++++
>  arch/arm/mach-imx/system.c                 | 5 ++++-
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/hardware/cache-l2x0.h b/arch/arm/include/asm/hardware/cache-l2x0.h
> index 3a5ec1c..5e27162 100644
> --- a/arch/arm/include/asm/hardware/cache-l2x0.h
> +++ b/arch/arm/include/asm/hardware/cache-l2x0.h
> @@ -61,6 +61,13 @@
>  #define L2X0_LINE_TAG			0xF30
>  #define L2X0_DEBUG_CTRL			0xF40
>  #define L310_PREFETCH_CTRL		0xF60
> +#define   L310_DOUBLE_LINEFILL_EN			BIT(30)
> +#define   L310_INSTRUCTION_PREFETCH_EN			BIT(29)
> +#define   L310_DATA_PREFETCH_EN				BIT(28)
> +#define   L310_DOUBLE_LINEFILL_ON_WRAP_READ_DIS		BIT(27)
> +#define   L310_PREFETCH_DROP_EN				BIT(24)
> +#define   L310_INCR_DOUBLE_LINEFILL_EN			BIT(23)
> +#define   L310_ESCLUSIVE_SEQUENCE_EN			BIT(21)

Looks like a spelling error... surely this should be EXCLUSIVE ?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1420459 — [RESEND PATCH 2/3] i.MX: system.c: Remove redundant errata 752271 code

FromAndrey Smirnov <andrew.smirnov@gmail.com>
Date2016-06-13 08:00 +0200
Subject[RESEND PATCH 2/3] i.MX: system.c: Remove redundant errata 752271 code
Message-ID<rJt4J-23X-9@gated-at.bofh.it>
In reply to#1420456
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, but is not affected by 752271.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---

RESEND, now with Russell King on CC list.

 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

[toc] | [prev] | [next] | [standalone]

#1420651

FromArnd Bergmann <arnd@arndb.de>
Date2016-06-13 11:30 +0200
Message-ID<rJwlY-4ho-19@gated-at.bofh.it>
In reply to#1420456
On Sunday, June 12, 2016 10:55:27 PM CEST Andrey Smirnov wrote:
> Using goto here doesn't bring any advantages and only makes the code
> flow less clear. No functional changes.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

All three patches looks reasonable to me, but now that I looked at
the function, I wonder if we could do another cleanup on top and
not call this function for imx35 at all but instead set the
.l2c_aux_val/.l2c_aux_mask fields in its machine descriptor.

The call to l2x0_of_init(0, ~0) seems to be the only thing we
do on imx35 (where arm,pl310-cache is not present), and the kernel
can do it automatically.

	Arnd

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web