| Message ID | 1432941541-18588-1-git-send-email-hauke@hauke-m.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Dear Hauke Mehrtens, On Sat, 30 May 2015 01:19:01 +0200, Hauke Mehrtens wrote: > These options make it possible to overwrites the data and instruction > prefetching behavior of the arm pl310 cache controller. > > Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> This patch would be useful to me. I'll test it tomorrow. Did you submit it into the Russell patch tracking system, as Russell suggested to do in his reply on your v3 ? Russell said: """ I don't see anything wrong with it, but as ever, it needs to end up in the patch system if it's not going to get buried beneath a huge pile of email. """ Thanks, Thomas
On 29/05/15 16:19, Hauke Mehrtens wrote: > These options make it possible to overwrites the data and instruction > prefetching behavior of the arm pl310 cache controller. > > Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> Acked-by: Florian Fainelli <f.fainelli@gmail.com> > --- > changes since > v3: > * remove arm prefix > * improve documentation > v2: only set prefetch > v1: set prefetch and aux > Documentation/devicetree/bindings/arm/l2cc.txt | 5 +++++ > arch/arm/mm/cache-l2x0.c | 20 ++++++++++++++++++++ > 2 files changed, 25 insertions(+) > > diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt > index 0dbabe9..2251dcc 100644 > --- a/Documentation/devicetree/bindings/arm/l2cc.txt > +++ b/Documentation/devicetree/bindings/arm/l2cc.txt > @@ -67,6 +67,11 @@ Optional properties: > disable if zero. > - arm,prefetch-offset : Override prefetch offset value. Valid values are > 0-7, 15, 23, and 31. > +- prefetch-data : Data prefetch. Value: <0> (forcibly disable), <1> > + (forcibly enable), property absent (retain settings set by firmware) > +- prefetch-instr : Instruction prefetch. Value: <0> (forcibly disable), > + <1> (forcibly enable), property absent (retain settings set by > + firmware) > > Example: > > diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c > index e309c8f..2570f83 100644 > --- a/arch/arm/mm/cache-l2x0.c > +++ b/arch/arm/mm/cache-l2x0.c > @@ -1199,6 +1199,26 @@ static void __init l2c310_of_parse(const struct device_node *np, > pr_err("L2C-310 OF arm,prefetch-offset property value is missing\n"); > } > > + ret = of_property_read_u32(np, "prefetch-data", &val); > + if (ret == 0) { > + if (val) > + prefetch |= L310_PREFETCH_CTRL_DATA_PREFETCH; > + else > + prefetch &= ~L310_PREFETCH_CTRL_DATA_PREFETCH; > + } else if (ret != -EINVAL) { > + pr_err("L2C-310 OF prefetch-data property value is missing\n"); > + } > + > + ret = of_property_read_u32(np, "prefetch-instr", &val); > + if (ret == 0) { > + if (val) > + prefetch |= L310_PREFETCH_CTRL_INSTR_PREFETCH; > + else > + prefetch &= ~L310_PREFETCH_CTRL_INSTR_PREFETCH; > + } else if (ret != -EINVAL) { > + pr_err("L2C-310 OF prefetch-instr property value is missing\n"); > + } > + > l2x0_saved_regs.prefetch_ctrl = prefetch; > } > >
On 06/10/2015 05:21 PM, Thomas Petazzoni wrote: > Dear Hauke Mehrtens, > > On Sat, 30 May 2015 01:19:01 +0200, Hauke Mehrtens wrote: >> These options make it possible to overwrites the data and instruction >> prefetching behavior of the arm pl310 cache controller. >> >> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> > > This patch would be useful to me. I'll test it tomorrow. Did you submit > it into the Russell patch tracking system, as Russell suggested to do > in his reply on your v3 ? > > Russell said: > > """ > I don't see anything wrong with it, but as ever, it needs to end up in > the patch system if it's not going to get buried beneath a huge pile of > email. > """ Thanks for asking, I did not know Russell's patch process, but Florian guided me and now it is submitted, see http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8391/1 Hauke
Hello, On Wed, 10 Jun 2015 21:26:35 +0200, Hauke Mehrtens wrote: > > This patch would be useful to me. I'll test it tomorrow. Did you submit > > it into the Russell patch tracking system, as Russell suggested to do > > in his reply on your v3 ? > > > > Russell said: > > > > """ > > I don't see anything wrong with it, but as ever, it needs to end up in > > the patch system if it's not going to get buried beneath a huge pile of > > email. > > """ > Thanks for asking, I did not know Russell's patch process, but Florian > guided me and now it is submitted, see > http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8391/1 Great, thanks! Thomas
Dear Hauke Mehrtens, On Wed, 10 Jun 2015 21:26:35 +0200, Hauke Mehrtens wrote: > Thanks for asking, I did not know Russell's patch process, but Florian > guided me and now it is submitted, see > http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8391/1 I tested your patch, and I'm not sure how it can work for you, because the changes made to the PREFETCH_CTRL register on the data/instruction prefetch bits are later overridden by changes made to the AUX_CTRL register. Here is what I've found so far: static void l2c_configure(void __iomem *base) { if (outer_cache.configure) { outer_cache.configure(&l2x0_saved_regs); return; } if (l2x0_data->configure) l2x0_data->configure(base); l2c_write_sec(l2x0_saved_regs.aux_ctrl, base, L2X0_AUX_CTRL); } l2x0_data->configure() is what writes the PREFETCH_CTRL register with the value determined in l2c310_of_parse() : if (revision >= L310_CACHE_ID_RTL_R2P0) { l2c_write_sec(l2x0_saved_regs.prefetch_ctrl, base, L310_PREFETCH_CTRL); } The value written in the L310_PREFETCH_CTRL register is correct: it properly has bits 28/29 sets depending on prefetch-data/prefetch-instr. However, when l2c_configure() does: l2c_write_sec(l2x0_saved_regs.aux_ctrl, base, L2X0_AUX_CTRL); It writes l2x0_saved_regs.aux_ctrl to the AUX_CTRL register, which has a "clone" of the prefetch data and prefetch instruction bits. And it resets them to zero. I've added debug messages before/after this line, and here is what I see: [ 0.000000] ==> (1) prefetch is now 0x58800000 [ 0.000000] ==> (2) prefetch is now 0x48800000 I had enabled only the prefetch-data, so in step (1) (before aux_ctrl is written to AUX_CTRL), bit 28 is correctly set to 1. However, after AUX_CTRL is written, it's restored to 0. How does your patch handles the fact that the prefetch data and prefetch instr are cloned between PREFETCH_CTRL and AUX_CTRL ? Thanks, Thomas
On Thu, Jun 11, 2015 at 11:24:59AM +0200, Thomas Petazzoni wrote: > Dear Hauke Mehrtens, > > On Wed, 10 Jun 2015 21:26:35 +0200, Hauke Mehrtens wrote: > > > Thanks for asking, I did not know Russell's patch process, but Florian > > guided me and now it is submitted, see > > http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8391/1 > > I tested your patch, and I'm not sure how it can work for you, because > the changes made to the PREFETCH_CTRL register on the data/instruction > prefetch bits are later overridden by changes made to the AUX_CTRL > register. There's also a patch I sent on May 14th which needs to be applied for this to work, and fixes the above. > How does your patch handles the fact that the prefetch data and > prefetch instr are cloned between PREFETCH_CTRL and AUX_CTRL ? It needs my patch.
On Thu, Jun 11, 2015 at 10:34:17AM +0100, Russell King - ARM Linux wrote: > On Thu, Jun 11, 2015 at 11:24:59AM +0200, Thomas Petazzoni wrote: > > Dear Hauke Mehrtens, > > > > On Wed, 10 Jun 2015 21:26:35 +0200, Hauke Mehrtens wrote: > > > > > Thanks for asking, I did not know Russell's patch process, but Florian > > > guided me and now it is submitted, see > > > http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8391/1 > > > > I tested your patch, and I'm not sure how it can work for you, because > > the changes made to the PREFETCH_CTRL register on the data/instruction > > prefetch bits are later overridden by changes made to the AUX_CTRL > > register. > > There's also a patch I sent on May 14th which needs to be applied for this > to work, and fixes the above. > > > How does your patch handles the fact that the prefetch data and > > prefetch instr are cloned between PREFETCH_CTRL and AUX_CTRL ? > > It needs my patch. ... in fact, please see all the other L2C patches queued in my tree. Hauke's patch is designed to be applied on top of these patches, not to 4.1-rc directly.
Russell, On Thu, 11 Jun 2015 10:39:07 +0100, Russell King - ARM Linux wrote: > > > How does your patch handles the fact that the prefetch data and > > > prefetch instr are cloned between PREFETCH_CTRL and AUX_CTRL ? > > > > It needs my patch. > > ... in fact, please see all the other L2C patches queued in my tree. > Hauke's patch is designed to be applied on top of these patches, not > to 4.1-rc directly. Yes, right. In the mean time, I discovered the discussion you had with Hauke on the v2 of his patch series, and started looking at your for-next branch. I wanted to give a test before reporting back, and it's now done: with "ARM: l2c: write auxiliary control register first" + Hauke's patch, it works fine for me. Thanks! Thomas
diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt index 0dbabe9..2251dcc 100644 --- a/Documentation/devicetree/bindings/arm/l2cc.txt +++ b/Documentation/devicetree/bindings/arm/l2cc.txt @@ -67,6 +67,11 @@ Optional properties: disable if zero. - arm,prefetch-offset : Override prefetch offset value. Valid values are 0-7, 15, 23, and 31. +- prefetch-data : Data prefetch. Value: <0> (forcibly disable), <1> + (forcibly enable), property absent (retain settings set by firmware) +- prefetch-instr : Instruction prefetch. Value: <0> (forcibly disable), + <1> (forcibly enable), property absent (retain settings set by + firmware) Example: diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c index e309c8f..2570f83 100644 --- a/arch/arm/mm/cache-l2x0.c +++ b/arch/arm/mm/cache-l2x0.c @@ -1199,6 +1199,26 @@ static void __init l2c310_of_parse(const struct device_node *np, pr_err("L2C-310 OF arm,prefetch-offset property value is missing\n"); } + ret = of_property_read_u32(np, "prefetch-data", &val); + if (ret == 0) { + if (val) + prefetch |= L310_PREFETCH_CTRL_DATA_PREFETCH; + else + prefetch &= ~L310_PREFETCH_CTRL_DATA_PREFETCH; + } else if (ret != -EINVAL) { + pr_err("L2C-310 OF prefetch-data property value is missing\n"); + } + + ret = of_property_read_u32(np, "prefetch-instr", &val); + if (ret == 0) { + if (val) + prefetch |= L310_PREFETCH_CTRL_INSTR_PREFETCH; + else + prefetch &= ~L310_PREFETCH_CTRL_INSTR_PREFETCH; + } else if (ret != -EINVAL) { + pr_err("L2C-310 OF prefetch-instr property value is missing\n"); + } + l2x0_saved_regs.prefetch_ctrl = prefetch; }
These options make it possible to overwrites the data and instruction prefetching behavior of the arm pl310 cache controller. Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> --- changes since v3: * remove arm prefix * improve documentation v2: only set prefetch v1: set prefetch and aux Documentation/devicetree/bindings/arm/l2cc.txt | 5 +++++ arch/arm/mm/cache-l2x0.c | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+)