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
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 <redacted>
---
arch/arm/mach-imx/system.c | 36 +++++++++++++++++-------------------
1 file changed, 17 insertions(+), 19 deletions(-)
@@ -106,26 +106,24 @@ void __init imx_init_l2cache(void)gotoout;}-if(readl_relaxed(l2x0_base+L2X0_CTRL)&L2X0_CTRL_EN)-gotoskip_if_enabled;--/* Configure the L2 PREFETCH and POWER registers */-val=readl_relaxed(l2x0_base+L310_PREFETCH_CTRL);-val|=0x70800000;-/*-*TheL2cachecontroller(PL310)versiononthei.MX6D/Qisr3p1-50rel0-*TheL2cachecontroller(PL310)versiononthei.MX6DL/SOLO/SLisr3p2-*ButaccordingtoARMPL310errata:752271-*ID:752271:Doublelinefillfeaturecancausedatacorruption-*FaultStatus:Presentin:r3p0,r3p1,r3p1-50rel0.Fixedinr3p2-*Workaround:Theonlyworkaroundtothiserratumistodisablethe-*doublelinefillfeature.Thisisthedefaultbehavior.-*/-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;+/*+*TheL2cachecontroller(PL310)versiononthei.MX6D/Qisr3p1-50rel0+*TheL2cachecontroller(PL310)versiononthei.MX6DL/SOLO/SLisr3p2+*ButaccordingtoARMPL310errata:752271+*ID:752271:Doublelinefillfeaturecancausedatacorruption+*FaultStatus:Presentin:r3p0,r3p1,r3p1-50rel0.Fixedinr3p2+*Workaround:Theonlyworkaroundtothiserratumistodisablethe+*doublelinefillfeature.Thisisthedefaultbehavior.+*/+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);
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 <redacted>
---
arch/arm/mach-imx/system.c | 11 -----------
1 file changed, 11 deletions(-)
@@ -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;-/*-*TheL2cachecontroller(PL310)versiononthei.MX6D/Qisr3p1-50rel0-*TheL2cachecontroller(PL310)versiononthei.MX6DL/SOLO/SLisr3p2-*ButaccordingtoARMPL310errata:752271-*ID:752271:Doublelinefillfeaturecancausedatacorruption-*FaultStatus:Presentin:r3p0,r3p1,r3p1-50rel0.Fixedinr3p2-*Workaround:Theonlyworkaroundtothiserratumistodisablethe-*doublelinefillfeature.Thisisthedefaultbehavior.-*/-if(cpu_is_imx6q())-val&=~(1<<30|1<<23);writel_relaxed(val,l2x0_base+L310_PREFETCH_CTRL);}
@@ -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);}
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 <redacted>
---
arch/arm/mm/cache-l2x0.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
@@ -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){u32val=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";}
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 <redacted>
Signed-off-by: Andrey Smirnov <redacted>
---
arch/arm/mach-imx/system.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
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 <redacted>
---
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(-)
@@ -98,13 +98,11 @@ void __init imx_init_l2cache(void)np=of_find_compatible_node(NULL,NULL,"arm,pl310-cache");if(!np)-gotoout;+return;l2x0_base=of_iomap(np,0);-if(!l2x0_base){-of_node_put(np);-gotoout;-}+if(!l2x0_base)+gotoput_node;if(!(readl_relaxed(l2x0_base+L2X0_CTRL)&L2X0_CTRL_EN)){/* Configure the L2 PREFETCH and POWER registers */
@@ -716,8 +716,10 @@ static void __init l2c310_fixup(void __iomem *base, u32 cache_id,revision<L310_CACHE_ID_RTL_R3P2){u32val=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";}
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
[off-list ref] 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
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>
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
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
On Sunday, June 19, 2016 10:53:13 AM CEST Andrey Smirnov wrote:
One more question, less about your patches than the existing code:
quoted
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.