[PATCH] ARM: mm: show prefetch state when either data or instr prefect is enabled

[PATCH] ARM: mm: show prefetch state when either data or instr prefect is enabled

[Thomas Petazzoni]

The l2c310_enable() function shows the state of prefetch configuration
only when both data *and* instruction prefetching is enabled, but the
message itself is able to cope with having either data or instruction
prefetch enabled, showing "ID prefetch enabled" when both are enabled,
"D prefetch enabled" when only data prefetch is enabled, and "I
prefetch enabled" when only instruction prefetch is enabled.

Therefore, this commit adjusts the code to display the prefetch
configuration when either data *or* instruction prefetching is
enabled. On my system with data prefetching enabled only, it shows:

  [    0.000000] L2C-310 D prefetch enabled, offset 1 lines

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/mm/cache-l2x0.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index d6e43d8..6bdf3a5 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -656,7 +656,8 @@ static void __init l2c310_enable(void __iomem *base, u32 aux, unsigned num_lock)
 	/* Read back resulting AUX_CTRL value as it could have been altered. */
 	aux = readl_relaxed(base + L2X0_AUX_CTRL);
 
-	if (aux & (L310_AUX_CTRL_DATA_PREFETCH | L310_AUX_CTRL_INSTR_PREFETCH)) {
+	if ((aux & L310_AUX_CTRL_DATA_PREFETCH) ||
+	    (aux & L310_AUX_CTRL_INSTR_PREFETCH)) {
 		u32 prefetch = readl_relaxed(base + L310_PREFETCH_CTRL);
 
 		pr_info("L2C-310 %s%s prefetch enabled, offset %u lines\n",
-- 
2.1.0

[PATCH] ARM: mm: show prefetch state when either data or instr prefect is enabled

[Russell King - ARM Linux]

On Thu, Jun 11, 2015 at 01:52:11PM +0200, Thomas Petazzoni wrote:
> The l2c310_enable() function shows the state of prefetch configuration
> only when both data *and* instruction prefetching is enabled, but the

Completely wrong, sorry.

> diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
> index d6e43d8..6bdf3a5 100644
> --- a/arch/arm/mm/cache-l2x0.c
> +++ b/arch/arm/mm/cache-l2x0.c
> @@ -656,7 +656,8 @@ static void __init l2c310_enable(void __iomem *base, u32 aux, unsigned num_lock)
>  	/* Read back resulting AUX_CTRL value as it could have been altered. */
>  	aux = readl_relaxed(base + L2X0_AUX_CTRL);
>  
> -	if (aux & (L310_AUX_CTRL_DATA_PREFETCH | L310_AUX_CTRL_INSTR_PREFETCH)) {

What this says is...
	first, bitwise _OR_ L310_AUX_CTRL_DATA_PREFETCH with
	L310_AUX_CTRL_INSTR_PREFETCH.  This produces a number which has
	_two_ bits set.

	then, bitwise _AND_ that number with the auxiliary control register.

	If the result is non-zero, then execute the code within the if() { }
	block.

So, I'm not going to take this patch.  The code is correct as it stands.

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

[PATCH] ARM: mm: show prefetch state when either data or instr prefect is enabled

[Thomas Petazzoni]

Russell,

On Thu, 11 Jun 2015 13:10:52 +0100, Russell King - ARM Linux wrote:

> > diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
> > index d6e43d8..6bdf3a5 100644
> > --- a/arch/arm/mm/cache-l2x0.c
> > +++ b/arch/arm/mm/cache-l2x0.c
> > @@ -656,7 +656,8 @@ static void __init l2c310_enable(void __iomem *base, u32 aux, unsigned num_lock)
> >  	/* Read back resulting AUX_CTRL value as it could have been altered. */
> >  	aux = readl_relaxed(base + L2X0_AUX_CTRL);
> >  
> > -	if (aux & (L310_AUX_CTRL_DATA_PREFETCH | L310_AUX_CTRL_INSTR_PREFETCH)) {
> 
> What this says is...
> 	first, bitwise _OR_ L310_AUX_CTRL_DATA_PREFETCH with
> 	L310_AUX_CTRL_INSTR_PREFETCH.  This produces a number which has
> 	_two_ bits set.
> 
> 	then, bitwise _AND_ that number with the auxiliary control register.
> 
> 	If the result is non-zero, then execute the code within the if() { }
> 	block.
> 
> So, I'm not going to take this patch.  The code is correct as it stands.

You are indeed absolutely correct. I got confused because I wasn't
seeing the message, but it was caused by the other problem (writing
AUX_CTRL overwriting the changes made to PREFETCH_CTRL). But that
obviously had nothing to do with this part of the code.

Sorry for the noise, and thanks for the quick feedback,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com