[PATCH v4] ARM: l2c: add options to overwrite prefetching behavior

[PATCH v4] ARM: l2c: add options to overwrite prefetching behavior

[Hauke Mehrtens]

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(+)

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;
 }
 
-- 
2.1.4

[PATCH v4] ARM: l2c: add options to overwrite prefetching behavior

[Thomas Petazzoni]

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH v4] ARM: l2c: add options to overwrite prefetching behavior

[Hauke Mehrtens]

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

[PATCH v4] ARM: l2c: add options to overwrite prefetching behavior

[Thomas Petazzoni]

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH v4] ARM: l2c: add options to overwrite prefetching behavior

[Thomas Petazzoni]

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH v4] ARM: l2c: add options to overwrite prefetching behavior

[Russell King - ARM Linux]

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.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH v4] ARM: l2c: add options to overwrite prefetching behavior

[Russell King - ARM Linux]

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.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH v4] ARM: l2c: add options to overwrite prefetching behavior

[Thomas Petazzoni]

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH v4] ARM: l2c: add options to overwrite prefetching behavior

[Florian Fainelli]

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


-- 
Florian