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

[PATCH v2] 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.

We have to set these values in the aux and the prefetch register,
because these two bits in the aux registers are mapped to the prefetch
register. If only the prefetch register is changed there is an
inconsistence in the state in this driver.

Signed-off-by: Hauke Mehrtens <hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>
---
 Documentation/devicetree/bindings/arm/l2cc.txt |  4 ++++
 arch/arm/mm/cache-l2x0.c                       | 30 ++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt
index 0dbabe9..528821a 100644
--- a/Documentation/devicetree/bindings/arm/l2cc.txt
+++ b/Documentation/devicetree/bindings/arm/l2cc.txt
@@ -67,6 +67,10 @@ Optional properties:
   disable if zero.
 - arm,prefetch-offset : Override prefetch offset value. Valid values are
   0-7, 15, 23, and 31.
+- arm,prefetch-data : Enable data prefetch. Enabling prefetching
+  can improve performance.
+- arm,prefetch-instr : Enable instruction prefetch. Enabling prefetching
+  can improve performance.
 
 Example:
 
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index e309c8f..088b5ad 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -1199,6 +1199,36 @@ 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, "arm,prefetch-data", &val);
+	if (ret == 0) {
+		if (val) {
+			prefetch |= L310_PREFETCH_CTRL_DATA_PREFETCH;
+			*aux_val |= L310_AUX_CTRL_DATA_PREFETCH;
+			*aux_mask &= ~L310_AUX_CTRL_DATA_PREFETCH;
+		} else {
+			prefetch &= ~L310_PREFETCH_CTRL_DATA_PREFETCH;
+			*aux_val &= ~L310_AUX_CTRL_DATA_PREFETCH;
+			*aux_mask |= L310_AUX_CTRL_DATA_PREFETCH;
+		}
+	} else if (ret != -EINVAL) {
+		pr_err("L2C-310 OF arm,prefetch-data property value is missing\n");
+	}
+
+	ret = of_property_read_u32(np, "arm,prefetch-instr", &val);
+	if (ret == 0) {
+		if (val) {
+			prefetch |= L310_PREFETCH_CTRL_INSTR_PREFETCH;
+			*aux_val |= L310_AUX_CTRL_INSTR_PREFETCH;
+			*aux_mask &= ~L310_AUX_CTRL_INSTR_PREFETCH;
+		} else {
+			prefetch &= ~L310_PREFETCH_CTRL_INSTR_PREFETCH;
+			*aux_val &= ~L310_AUX_CTRL_INSTR_PREFETCH;
+			*aux_mask |= L310_AUX_CTRL_INSTR_PREFETCH;
+		}
+	} else if (ret != -EINVAL) {
+		pr_err("L2C-310 OF arm,prefetch-instr property value is missing\n");
+	}
+
 	l2x0_saved_regs.prefetch_ctrl = prefetch;
 }
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] 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.

We have to set these values in the aux and the prefetch register,
because these two bits in the aux registers are mapped to the prefetch
register. If only the prefetch register is changed there is an
inconsistence in the state in this driver.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 Documentation/devicetree/bindings/arm/l2cc.txt |  4 ++++
 arch/arm/mm/cache-l2x0.c                       | 30 ++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt
index 0dbabe9..528821a 100644
--- a/Documentation/devicetree/bindings/arm/l2cc.txt
+++ b/Documentation/devicetree/bindings/arm/l2cc.txt
@@ -67,6 +67,10 @@ Optional properties:
   disable if zero.
 - arm,prefetch-offset : Override prefetch offset value. Valid values are
   0-7, 15, 23, and 31.
+- arm,prefetch-data : Enable data prefetch. Enabling prefetching
+  can improve performance.
+- arm,prefetch-instr : Enable instruction prefetch. Enabling prefetching
+  can improve performance.
 
 Example:
 
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index e309c8f..088b5ad 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -1199,6 +1199,36 @@ 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, "arm,prefetch-data", &val);
+	if (ret == 0) {
+		if (val) {
+			prefetch |= L310_PREFETCH_CTRL_DATA_PREFETCH;
+			*aux_val |= L310_AUX_CTRL_DATA_PREFETCH;
+			*aux_mask &= ~L310_AUX_CTRL_DATA_PREFETCH;
+		} else {
+			prefetch &= ~L310_PREFETCH_CTRL_DATA_PREFETCH;
+			*aux_val &= ~L310_AUX_CTRL_DATA_PREFETCH;
+			*aux_mask |= L310_AUX_CTRL_DATA_PREFETCH;
+		}
+	} else if (ret != -EINVAL) {
+		pr_err("L2C-310 OF arm,prefetch-data property value is missing\n");
+	}
+
+	ret = of_property_read_u32(np, "arm,prefetch-instr", &val);
+	if (ret == 0) {
+		if (val) {
+			prefetch |= L310_PREFETCH_CTRL_INSTR_PREFETCH;
+			*aux_val |= L310_AUX_CTRL_INSTR_PREFETCH;
+			*aux_mask &= ~L310_AUX_CTRL_INSTR_PREFETCH;
+		} else {
+			prefetch &= ~L310_PREFETCH_CTRL_INSTR_PREFETCH;
+			*aux_val &= ~L310_AUX_CTRL_INSTR_PREFETCH;
+			*aux_mask |= L310_AUX_CTRL_INSTR_PREFETCH;
+		}
+	} else if (ret != -EINVAL) {
+		pr_err("L2C-310 OF arm,prefetch-instr property value is missing\n");
+	}
+
 	l2x0_saved_regs.prefetch_ctrl = prefetch;
 }
 
-- 
2.1.4

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

[Russell King - ARM Linux]

On Thu, May 14, 2015 at 06:13:55PM +0200, Hauke Mehrtens wrote:
> These options make it possible to overwrites the data and instruction
> prefetching behavior of the arm pl310 cache controller.
> 
> We have to set these values in the aux and the prefetch register,
> because these two bits in the aux registers are mapped to the prefetch
> register. If only the prefetch register is changed there is an
> inconsistence in the state in this driver.

No there isn't.  Just set the bits in the prefetch register.

Writing to the prefetch register changes the state of the bits in the
auxiliary control register at the same time.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Russell King - ARM Linux]

On Thu, May 14, 2015 at 06:13:55PM +0200, Hauke Mehrtens wrote:
> These options make it possible to overwrites the data and instruction
> prefetching behavior of the arm pl310 cache controller.
> 
> We have to set these values in the aux and the prefetch register,
> because these two bits in the aux registers are mapped to the prefetch
> register. If only the prefetch register is changed there is an
> inconsistence in the state in this driver.

No there isn't.  Just set the bits in the prefetch register.

Writing to the prefetch register changes the state of the bits in the
auxiliary control register at the same time.

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

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

[Russell King - ARM Linux]

On Thu, May 14, 2015 at 05:15:26PM +0100, Russell King - ARM Linux wrote:
> On Thu, May 14, 2015 at 06:13:55PM +0200, Hauke Mehrtens wrote:
> > These options make it possible to overwrites the data and instruction
> > prefetching behavior of the arm pl310 cache controller.
> > 
> > We have to set these values in the aux and the prefetch register,
> > because these two bits in the aux registers are mapped to the prefetch
> > register. If only the prefetch register is changed there is an
> > inconsistence in the state in this driver.
> 
> No there isn't.  Just set the bits in the prefetch register.
> 
> Writing to the prefetch register changes the state of the bits in the
> auxiliary control register at the same time.

I see what you're getting at now.  I think we ought to fix that in the
driver, so that the auxiliary control register is always written first,
before the prefetch control register.  This also makes l2c_configure()
reflect the structure of the rest of the driver.

 arch/arm/mm/cache-l2x0.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index e309c8f35af5..46427d1a5946 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -110,14 +110,6 @@ static inline void l2c_unlock(void __iomem *base, unsigned num)
 
 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);
 }
 
@@ -134,7 +126,11 @@ static void l2c_enable(void __iomem *base, u32 aux, unsigned num_lock)
 		return;
 
 	l2x0_saved_regs.aux_ctrl = aux;
-	l2c_configure(base);
+
+	if (outer_cache.configure)
+		outer_cache.configure(&l2x0_saved_regs);
+	else
+		l2x0_data->configure(base);
 
 	l2c_unlock(base, num_lock);
 
@@ -252,6 +248,7 @@ static const struct l2c_init_data l2c210_data __initconst = {
 	.num_lock = 1,
 	.enable = l2c_enable,
 	.save = l2c_save,
+	.configure = l2c_configure,
 	.outer_cache = {
 		.inv_range = l2c210_inv_range,
 		.clean_range = l2c210_clean_range,
@@ -409,6 +406,7 @@ static const struct l2c_init_data l2c220_data = {
 	.num_lock = 1,
 	.enable = l2c220_enable,
 	.save = l2c_save,
+	.configure = l2c_configure,
 	.outer_cache = {
 		.inv_range = l2c220_inv_range,
 		.clean_range = l2c220_clean_range,
@@ -569,6 +567,8 @@ static void l2c310_configure(void __iomem *base)
 {
 	unsigned revision;
 
+	l2c_configure(base);
+
 	/* restore pl310 setup */
 	l2c_write_sec(l2x0_saved_regs.tag_latency, base,
 		      L310_TAG_LATENCY_CTRL);
@@ -1066,6 +1066,7 @@ static const struct l2c_init_data of_l2c210_data __initconst = {
 	.of_parse = l2x0_of_parse,
 	.enable = l2c_enable,
 	.save = l2c_save,
+	.configure = l2c_configure,
 	.outer_cache = {
 		.inv_range   = l2c210_inv_range,
 		.clean_range = l2c210_clean_range,
@@ -1084,6 +1085,7 @@ static const struct l2c_init_data of_l2c220_data __initconst = {
 	.of_parse = l2x0_of_parse,
 	.enable = l2c220_enable,
 	.save = l2c_save,
+	.configure = l2c_configure,
 	.outer_cache = {
 		.inv_range   = l2c220_inv_range,
 		.clean_range = l2c220_clean_range,
@@ -1416,6 +1418,7 @@ static const struct l2c_init_data of_aurora_with_outer_data __initconst = {
 	.enable = l2c_enable,
 	.fixup = aurora_fixup,
 	.save  = aurora_save,
+	.configure = l2c_configure,
 	.outer_cache = {
 		.inv_range   = aurora_inv_range,
 		.clean_range = aurora_clean_range,
@@ -1435,6 +1438,7 @@ static const struct l2c_init_data of_aurora_no_outer_data __initconst = {
 	.enable = aurora_enable_no_outer,
 	.fixup = aurora_fixup,
 	.save  = aurora_save,
+	.configure = l2c_configure,
 	.outer_cache = {
 		.resume      = l2c_resume,
 	},
@@ -1608,6 +1612,7 @@ static void __init tauros3_save(void __iomem *base)
 
 static void tauros3_configure(void __iomem *base)
 {
+	l2c_configure(base);
 	writel_relaxed(l2x0_saved_regs.aux2_ctrl,
 		       base + TAUROS3_AUX2_CTRL);
 	writel_relaxed(l2x0_saved_regs.prefetch_ctrl,

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Russell King - ARM Linux]

On Thu, May 14, 2015 at 05:15:26PM +0100, Russell King - ARM Linux wrote:
> On Thu, May 14, 2015 at 06:13:55PM +0200, Hauke Mehrtens wrote:
> > These options make it possible to overwrites the data and instruction
> > prefetching behavior of the arm pl310 cache controller.
> > 
> > We have to set these values in the aux and the prefetch register,
> > because these two bits in the aux registers are mapped to the prefetch
> > register. If only the prefetch register is changed there is an
> > inconsistence in the state in this driver.
> 
> No there isn't.  Just set the bits in the prefetch register.
> 
> Writing to the prefetch register changes the state of the bits in the
> auxiliary control register at the same time.

I see what you're getting at now.  I think we ought to fix that in the
driver, so that the auxiliary control register is always written first,
before the prefetch control register.  This also makes l2c_configure()
reflect the structure of the rest of the driver.

 arch/arm/mm/cache-l2x0.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index e309c8f35af5..46427d1a5946 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -110,14 +110,6 @@ static inline void l2c_unlock(void __iomem *base, unsigned num)
 
 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);
 }
 
@@ -134,7 +126,11 @@ static void l2c_enable(void __iomem *base, u32 aux, unsigned num_lock)
 		return;
 
 	l2x0_saved_regs.aux_ctrl = aux;
-	l2c_configure(base);
+
+	if (outer_cache.configure)
+		outer_cache.configure(&l2x0_saved_regs);
+	else
+		l2x0_data->configure(base);
 
 	l2c_unlock(base, num_lock);
 
@@ -252,6 +248,7 @@ static const struct l2c_init_data l2c210_data __initconst = {
 	.num_lock = 1,
 	.enable = l2c_enable,
 	.save = l2c_save,
+	.configure = l2c_configure,
 	.outer_cache = {
 		.inv_range = l2c210_inv_range,
 		.clean_range = l2c210_clean_range,
@@ -409,6 +406,7 @@ static const struct l2c_init_data l2c220_data = {
 	.num_lock = 1,
 	.enable = l2c220_enable,
 	.save = l2c_save,
+	.configure = l2c_configure,
 	.outer_cache = {
 		.inv_range = l2c220_inv_range,
 		.clean_range = l2c220_clean_range,
@@ -569,6 +567,8 @@ static void l2c310_configure(void __iomem *base)
 {
 	unsigned revision;
 
+	l2c_configure(base);
+
 	/* restore pl310 setup */
 	l2c_write_sec(l2x0_saved_regs.tag_latency, base,
 		      L310_TAG_LATENCY_CTRL);
@@ -1066,6 +1066,7 @@ static const struct l2c_init_data of_l2c210_data __initconst = {
 	.of_parse = l2x0_of_parse,
 	.enable = l2c_enable,
 	.save = l2c_save,
+	.configure = l2c_configure,
 	.outer_cache = {
 		.inv_range   = l2c210_inv_range,
 		.clean_range = l2c210_clean_range,
@@ -1084,6 +1085,7 @@ static const struct l2c_init_data of_l2c220_data __initconst = {
 	.of_parse = l2x0_of_parse,
 	.enable = l2c220_enable,
 	.save = l2c_save,
+	.configure = l2c_configure,
 	.outer_cache = {
 		.inv_range   = l2c220_inv_range,
 		.clean_range = l2c220_clean_range,
@@ -1416,6 +1418,7 @@ static const struct l2c_init_data of_aurora_with_outer_data __initconst = {
 	.enable = l2c_enable,
 	.fixup = aurora_fixup,
 	.save  = aurora_save,
+	.configure = l2c_configure,
 	.outer_cache = {
 		.inv_range   = aurora_inv_range,
 		.clean_range = aurora_clean_range,
@@ -1435,6 +1438,7 @@ static const struct l2c_init_data of_aurora_no_outer_data __initconst = {
 	.enable = aurora_enable_no_outer,
 	.fixup = aurora_fixup,
 	.save  = aurora_save,
+	.configure = l2c_configure,
 	.outer_cache = {
 		.resume      = l2c_resume,
 	},
@@ -1608,6 +1612,7 @@ static void __init tauros3_save(void __iomem *base)
 
 static void tauros3_configure(void __iomem *base)
 {
+	l2c_configure(base);
 	writel_relaxed(l2x0_saved_regs.aux2_ctrl,
 		       base + TAUROS3_AUX2_CTRL);
 	writel_relaxed(l2x0_saved_regs.prefetch_ctrl,

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

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

[Hauke Mehrtens]

On 05/14/2015 06:30 PM, Russell King - ARM Linux wrote:
> On Thu, May 14, 2015 at 05:15:26PM +0100, Russell King - ARM Linux wrote:
>> On Thu, May 14, 2015 at 06:13:55PM +0200, Hauke Mehrtens wrote:
>>> These options make it possible to overwrites the data and instruction
>>> prefetching behavior of the arm pl310 cache controller.
>>>
>>> We have to set these values in the aux and the prefetch register,
>>> because these two bits in the aux registers are mapped to the prefetch
>>> register. If only the prefetch register is changed there is an
>>> inconsistence in the state in this driver.
>>
>> No there isn't.  Just set the bits in the prefetch register.
>>
>> Writing to the prefetch register changes the state of the bits in the
>> auxiliary control register at the same time.
> 
> I see what you're getting at now.  I think we ought to fix that in the
> driver, so that the auxiliary control register is always written first,
> before the prefetch control register.  This also makes l2c_configure()
> reflect the structure of the rest of the driver.
> 
>  arch/arm/mm/cache-l2x0.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)

Tested-by: Hauke Mehrtens <hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>

Yes this fixes my problem. I did not meant that the state of the
hardware registers would be inconsistent, but the state of the driver.
I saw the problem you fixed, but was not closely following the control
flow in the driver to see the problem before.

Is it save enough to relay on the aux register not being written after
the prefetch register is written?
To make it more secure you could call l2x0_data->save(base) directly
after l2x0_data->configure(base)

Hauke
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Hauke Mehrtens]

On 05/14/2015 06:30 PM, Russell King - ARM Linux wrote:
> On Thu, May 14, 2015 at 05:15:26PM +0100, Russell King - ARM Linux wrote:
>> On Thu, May 14, 2015 at 06:13:55PM +0200, Hauke Mehrtens wrote:
>>> These options make it possible to overwrites the data and instruction
>>> prefetching behavior of the arm pl310 cache controller.
>>>
>>> We have to set these values in the aux and the prefetch register,
>>> because these two bits in the aux registers are mapped to the prefetch
>>> register. If only the prefetch register is changed there is an
>>> inconsistence in the state in this driver.
>>
>> No there isn't.  Just set the bits in the prefetch register.
>>
>> Writing to the prefetch register changes the state of the bits in the
>> auxiliary control register at the same time.
> 
> I see what you're getting at now.  I think we ought to fix that in the
> driver, so that the auxiliary control register is always written first,
> before the prefetch control register.  This also makes l2c_configure()
> reflect the structure of the rest of the driver.
> 
>  arch/arm/mm/cache-l2x0.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)

Tested-by: Hauke Mehrtens <hauke@hauke-m.de>

Yes this fixes my problem. I did not meant that the state of the
hardware registers would be inconsistent, but the state of the driver.
I saw the problem you fixed, but was not closely following the control
flow in the driver to see the problem before.

Is it save enough to relay on the aux register not being written after
the prefetch register is written?
To make it more secure you could call l2x0_data->save(base) directly
after l2x0_data->configure(base)

Hauke