[patch] R4k cache code synchronization

[patch] R4k cache code synchronization

[Maciej W. Rozycki]

Ralf,

 Here are some bits to synchronize c-r4k.c across ports.  There are no
functional changes, at least no intended ones.  OK to apply?

 I can't see any need for flush_cache_l1() and flush_cache_l2().  I'd like
to remove them.  A single flush_cache_all() seems sufficient for our
needs.  Any objections? 

  Maciej

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

patch-mips-2.4.20-pre6-20030107-mips-c-r4k-sync-0
diff -up --recursive --new-file linux-mips-2.4.20-pre6-20030107.macro/arch/mips/mm/c-r4k.c linux-mips-2.4.20-pre6-20030107/arch/mips/mm/c-r4k.c
--- linux-mips-2.4.20-pre6-20030107.macro/arch/mips/mm/c-r4k.c	2002-12-20 03:56:50.000000000 +0000
+++ linux-mips-2.4.20-pre6-20030107/arch/mips/mm/c-r4k.c	2003-01-09 22:23:33.000000000 +0000
@@ -948,12 +948,13 @@ static void r4k_flush_icache_page_p(stru
 static void r4k_dma_cache_wback_inv_pc(unsigned long addr, unsigned long size)
 {
 	unsigned long end, a;
-	unsigned int flags;
 
 	if (size >= dcache_size) {
 		flush_cache_all();
 	} else {
 #ifdef R4600_V2_HIT_CACHEOP_WAR
+		unsigned long flags;
+
 		/* Workaround for R4600 bug.  See comment in <asm/war>. */
 		local_irq_save(flags);
 		*(volatile unsigned long *)KSEG1;
@@ -962,7 +963,7 @@ static void r4k_dma_cache_wback_inv_pc(u
 		a = addr & ~(dc_lsize - 1);
 		end = (addr + size - 1) & ~(dc_lsize - 1);
 		while (1) {
-			flush_dcache_line(a); /* Hit_Writeback_Inv_D */
+			flush_dcache_line(a);	/* Hit_Writeback_Inv_D */
 			if (a == end) break;
 			a += dc_lsize;
 		}
@@ -970,6 +971,7 @@ static void r4k_dma_cache_wback_inv_pc(u
 		local_irq_restore(flags);
 #endif
 	}
+
 	bc_wback_inv(addr, size);
 }
 
@@ -994,12 +996,13 @@ static void r4k_dma_cache_wback_inv_sc(u
 static void r4k_dma_cache_inv_pc(unsigned long addr, unsigned long size)
 {
 	unsigned long end, a;
-	unsigned int flags;
 
 	if (size >= dcache_size) {
 		flush_cache_all();
 	} else {
 #ifdef R4600_V2_HIT_CACHEOP_WAR
+		unsigned long flags;
+
 		/* Workaround for R4600 bug.  See comment in <asm/war>. */
 		local_irq_save(flags);
 		*(volatile unsigned long *)KSEG1;
@@ -1008,7 +1011,7 @@ static void r4k_dma_cache_inv_pc(unsigne
 		a = addr & ~(dc_lsize - 1);
 		end = (addr + size - 1) & ~(dc_lsize - 1);
 		while (1) {
-			flush_dcache_line(a); /* Hit_Writeback_Inv_D */
+			flush_dcache_line(a);	/* Hit_Writeback_Inv_D */
 			if (a == end) break;
 			a += dc_lsize;
 		}
@@ -1016,6 +1019,7 @@ static void r4k_dma_cache_inv_pc(unsigne
 		local_irq_restore(flags);
 #endif
 	}
+
 	bc_inv(addr, size);
 }
 
@@ -1031,7 +1035,7 @@ static void r4k_dma_cache_inv_sc(unsigne
 	a = addr & ~(sc_lsize - 1);
 	end = (addr + size - 1) & ~(sc_lsize - 1);
 	while (1) {
-		flush_scache_line(a); /* Hit_Writeback_Inv_SD */
+		flush_scache_line(a);	/* Hit_Writeback_Inv_SD */
 		if (a == end) break;
 		a += sc_lsize;
 	}
@@ -1232,10 +1236,10 @@ static void __init setup_noscache_funcs(
 		_flush_cache_page = r4k_flush_cache_page_d32i32;
 		break;
 	}
-	___flush_cache_all = _flush_cache_all;
-
 	_flush_icache_page = r4k_flush_icache_page_p;
 
+	___flush_cache_all = _flush_cache_all;
+
 	_dma_cache_wback_inv = r4k_dma_cache_wback_inv_pc;
 	_dma_cache_wback = r4k_dma_cache_wback_inv_pc;
 	_dma_cache_inv = r4k_dma_cache_inv_pc;
@@ -1317,8 +1321,10 @@ static void __init setup_scache_funcs(vo
 		_copy_page = r4k_copy_page_s128;
 		break;
 	}
-	___flush_cache_all = _flush_cache_all;
 	_flush_icache_page = r4k_flush_icache_page_s;
+
+	___flush_cache_all = _flush_cache_all;
+
 	_dma_cache_wback_inv = r4k_dma_cache_wback_inv_sc;
 	_dma_cache_wback = r4k_dma_cache_wback_inv_sc;
 	_dma_cache_inv = r4k_dma_cache_inv_sc;
@@ -1373,10 +1379,10 @@ void __init ld_mmu_r4xx0(void)
 	}
 
 	_flush_cache_sigtramp = r4k_flush_cache_sigtramp;
-	_flush_icache_range = r4k_flush_icache_range;	/* Ouch */
 	if ((read_c0_prid() & 0xfff0) == 0x2020) {
 		_flush_cache_sigtramp = r4600v20k_flush_cache_sigtramp;
 	}
+	_flush_icache_range = r4k_flush_icache_range;	/* Ouch */
 
 	__flush_cache_all();
 }
diff -up --recursive --new-file linux-mips-2.4.20-pre6-20030107.macro/arch/mips64/mm/c-r4k.c linux-mips-2.4.20-pre6-20030107/arch/mips64/mm/c-r4k.c
--- linux-mips-2.4.20-pre6-20030107.macro/arch/mips64/mm/c-r4k.c	2002-12-20 03:56:52.000000000 +0000
+++ linux-mips-2.4.20-pre6-20030107/arch/mips64/mm/c-r4k.c	2003-01-09 23:21:39.000000000 +0000
@@ -950,7 +950,7 @@ static void r4k_dma_cache_wback_inv_pc(u
 	unsigned long end, a;
 
 	if (size >= dcache_size) {
-		flush_cache_l1();
+		flush_cache_all();
 	} else {
 #ifdef R4600_V2_HIT_CACHEOP_WAR
 		unsigned long flags;
@@ -963,7 +963,7 @@ static void r4k_dma_cache_wback_inv_pc(u
 		a = addr & ~(dc_lsize - 1);
 		end = (addr + size - 1) & ~(dc_lsize - 1);
 		while (1) {
-			flush_dcache_line(a); /* Hit_Writeback_Inv_D */
+			flush_dcache_line(a);	/* Hit_Writeback_Inv_D */
 			if (a == end) break;
 			a += dc_lsize;
 		}
@@ -971,6 +971,7 @@ static void r4k_dma_cache_wback_inv_pc(u
 		local_irq_restore(flags);
 #endif
 	}
+
 	bc_wback_inv(addr, size);
 }
 
@@ -979,7 +980,7 @@ static void r4k_dma_cache_wback_inv_sc(u
 	unsigned long end, a;
 
 	if (size >= scache_size) {
-		flush_cache_l1();
+		flush_cache_all();
 		return;
 	}
 
@@ -997,7 +998,7 @@ static void r4k_dma_cache_inv_pc(unsigne
 	unsigned long end, a;
 
 	if (size >= dcache_size) {
-		flush_cache_l1();
+		flush_cache_all();
 	} else {
 #ifdef R4600_V2_HIT_CACHEOP_WAR
 		unsigned long flags;
@@ -1010,7 +1011,7 @@ static void r4k_dma_cache_inv_pc(unsigne
 		a = addr & ~(dc_lsize - 1);
 		end = (addr + size - 1) & ~(dc_lsize - 1);
 		while (1) {
-			flush_dcache_line(a); /* Hit_Writeback_Inv_D */
+			flush_dcache_line(a);	/* Hit_Writeback_Inv_D */
 			if (a == end) break;
 			a += dc_lsize;
 		}
@@ -1027,14 +1028,14 @@ static void r4k_dma_cache_inv_sc(unsigne
 	unsigned long end, a;
 
 	if (size >= scache_size) {
-		flush_cache_l1();
+		flush_cache_all();
 		return;
 	}
 
 	a = addr & ~(sc_lsize - 1);
 	end = (addr + size - 1) & ~(sc_lsize - 1);
 	while (1) {
-		flush_scache_line(a); /* Hit_Writeback_Inv_SD */
+		flush_scache_line(a);	/* Hit_Writeback_Inv_SD */
 		if (a == end) break;
 		a += sc_lsize;
 	}
@@ -1241,9 +1242,10 @@ static void __init setup_noscache_funcs(
 		_flush_cache_page = r4k_flush_cache_page_d32i32;
 		break;
 	}
-
 	_flush_icache_page = r4k_flush_icache_page_p;
 
+	___flush_cache_all = _flush_cache_all;
+
 	_dma_cache_wback_inv = r4k_dma_cache_wback_inv_pc;
 	_dma_cache_wback = r4k_dma_cache_wback_inv_pc;
 	_dma_cache_inv = r4k_dma_cache_inv_pc;
@@ -1333,6 +1335,9 @@ static void __init setup_scache_funcs(vo
 		break;
 	}
 	_flush_icache_page = r4k_flush_icache_page_s;
+
+	___flush_cache_all = _flush_cache_all;
+
 	_dma_cache_wback_inv = r4k_dma_cache_wback_inv_sc;
 	_dma_cache_wback = r4k_dma_cache_wback_inv_sc;
 	_dma_cache_inv = r4k_dma_cache_inv_sc;
@@ -1394,5 +1399,5 @@ void __init ld_mmu_r4xx0(void)
 
 	_flush_cache_l2 = r4k_flush_cache_l2;
 
-	flush_cache_l1();
+	__flush_cache_all();
 }

Re: [patch] R4k cache code synchronization

[Ralf Baechle]

On Fri, Jan 10, 2003 at 01:37:12PM +0100, Maciej W. Rozycki wrote:

>  I can't see any need for flush_cache_l1() and flush_cache_l2().  I'd like
> to remove them.  A single flush_cache_all() seems sufficient for our
> needs.  Any objections? 

The reason for the existance of flush_cache_l1 and flush_cache_l2 is the
Origin.  An empty flush_cache_all() is sufficient on the Origin because
it's R10000 processor doesn't suffer from cache aliases.  During bootup
we have to flush all caches or the cache coherence logic will send crazy
exceptions at us.  For all other occasions just a flush of the primary
caches is sufficient which is why there is flush_cache_l1.

So I think we want to wrap things a bit nicer but basically we have to
keep those cacheops for the sake of the Origin.

  Ralf

Re: [patch] R4k cache code synchronization

[Maciej W. Rozycki]

On Fri, 10 Jan 2003, Ralf Baechle wrote:

> The reason for the existance of flush_cache_l1 and flush_cache_l2 is the
> Origin.  An empty flush_cache_all() is sufficient on the Origin because
> it's R10000 processor doesn't suffer from cache aliases.  During bootup
> we have to flush all caches or the cache coherence logic will send crazy
> exceptions at us.  For all other occasions just a flush of the primary
> caches is sufficient which is why there is flush_cache_l1.

 So flush_cache_l1() as currently defined is sufficient for DMA coherency
on the R10000, isn't it?

> So I think we want to wrap things a bit nicer but basically we have to
> keep those cacheops for the sake of the Origin.

 The naming is grossly incorrect.  If the R10000 has such a cache
semantics, then it should use flush_cache_all() (targetting L1) for
coherency and __flush_cache_all() (targetting L2; I assume L2 operations
imply L1 ones, otherwise the function should explicitly operate on L1,
too) for a maintenance flush.  Just like it's done for the 32-bit port. 

 There was a patch attached -- what about it? 

  Maciej

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: [patch] R4k cache code synchronization

[Maciej W. Rozycki]

On 10 Jan 2003, Juan Quintela wrote:

> The only thing that could be controversial is the _l1() thing, and as
> current thing is broken, I vote for insclusion.
> 
> maciej> diff -up --recursive --new-file linux-mips-2.4.20-pre6-20030107.macro/arch/mips64/mm/c-r4k.c linux-mips-2.4.20-pre6-20030107/arch/mips64/mm/c-r4k.c
> maciej> --- linux-mips-2.4.20-pre6-20030107.macro/arch/mips64/mm/c-r4k.c	2002-12-20 03:56:52.000000000 +0000
> maciej> +++ linux-mips-2.4.20-pre6-20030107/arch/mips64/mm/c-r4k.c	2003-01-09 23:21:39.000000000 +0000
> @@ -979,7 +980,7 @@ static void r4k_dma_cache_wback_inv_sc(u
>  	unsigned long end, a;
>  
>  	if (size >= scache_size) {
> -		flush_cache_l1();
> +		flush_cache_all();
>  		return;
>  	}
> 
> This one is fixing a bug, we are talking about a chip with Secondary
> cache and don't touch the secondary cache at all :(

 That bug is inactive -- both function pointers are defined to the same
value as surprisinly enough "l1" means "both caches" for the R4k.  Anyway,
I for removing flush_cache_l1() altogether in the next step. 

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: [patch] R4k cache code synchronization

[Juan Quintela]

I agree with the cleanup.

The only thing that could be controversial is the _l1() thing, and as
current thing is broken, I vote for insclusion.

maciej> diff -up --recursive --new-file linux-mips-2.4.20-pre6-20030107.macro/arch/mips64/mm/c-r4k.c linux-mips-2.4.20-pre6-20030107/arch/mips64/mm/c-r4k.c
maciej> --- linux-mips-2.4.20-pre6-20030107.macro/arch/mips64/mm/c-r4k.c	2002-12-20 03:56:52.000000000 +0000
maciej> +++ linux-mips-2.4.20-pre6-20030107/arch/mips64/mm/c-r4k.c	2003-01-09 23:21:39.000000000 +0000
@@ -979,7 +980,7 @@ static void r4k_dma_cache_wback_inv_sc(u
 	unsigned long end, a;
 
 	if (size >= scache_size) {
-		flush_cache_l1();
+		flush_cache_all();
 		return;
 	}

This one is fixing a bug, we are talking about a chip with Secondary
cache and don't touch the secondary cache at all :(

Later, Juan. 

-- 
In theory, practice and theory are the same, but in practice they 
are different -- Larry McVoy

Re: [patch] R4k cache code synchronization

[Juan Quintela]

>>>>> "ralf" == Ralf Baechle <ralf@linux-mips.org> writes:

ralf> On Fri, Jan 10, 2003 at 01:37:12PM +0100, Maciej W. Rozycki wrote:
>> I can't see any need for flush_cache_l1() and flush_cache_l2().  I'd like
>> to remove them.  A single flush_cache_all() seems sufficient for our
>> needs.  Any objections? 

ralf> The reason for the existance of flush_cache_l1 and flush_cache_l2 is the
ralf> Origin.  An empty flush_cache_all() is sufficient on the Origin because
ralf> it's R10000 processor doesn't suffer from cache aliases.  During bootup
ralf> we have to flush all caches or the cache coherence logic will send crazy
ralf> exceptions at us.  For all other occasions just a flush of the primary
ralf> caches is sufficient which is why there is flush_cache_l1.

ralf> So I think we want to wrap things a bit nicer but basically we have to
ralf> keep those cacheops for the sake of the Origin.

Humm, I thought that there wasn't origins with R4400 processors :)

Anyways, I think that the change is the good direction, at least:
- unsigned int flags -> unsigned long flags
  && under proper #ifdefs

and the mips64 port just now should work only in R4400MC & R4400PC (to me
knowlegge the most unusual ones), and broke in the R4400SC :(


Once talking about caches, why do we _allways_ do the writeback,
indeed if they asked us to do a invalidate?

Something like that?

Later, Juan.

Index: arch/mips64/mm/c-r4k.c
===================================================================
RCS file: /home/cvs/linux/arch/mips64/mm/c-r4k.c,v
retrieving revision 1.1.2.11
diff -u -r1.1.2.11 c-r4k.c
--- arch/mips64/mm/c-r4k.c	8 Jan 2003 14:16:31 -0000	1.1.2.11
+++ arch/mips64/mm/c-r4k.c	9 Jan 2003 23:16:41 -0000
@@ -979,7 +979,7 @@
 	unsigned long end, a;
 
 	if (size >= scache_size) {
-		flush_cache_l1();
+		flush_cache_all();
 		return;
 	}
 
@@ -1010,7 +1010,7 @@
 		a = addr & ~(dc_lsize - 1);
 		end = (addr + size - 1) & ~(dc_lsize - 1);
 		while (1) {
-			flush_dcache_line(a); /* Hit_Writeback_Inv_D */
+			invalidate_dcache_line(a); /* Hit_Invalidate_D */
 			if (a == end) break;
 			a += dc_lsize;
 		}
@@ -1027,14 +1027,14 @@
 	unsigned long end, a;
 
 	if (size >= scache_size) {
-		flush_cache_l1();
+		flush_cache_all();
 		return;
 	}
 
 	a = addr & ~(sc_lsize - 1);
 	end = (addr + size - 1) & ~(sc_lsize - 1);
 	while (1) {
-		flush_scache_line(a); /* Hit_Writeback_Inv_SD */
+		invalidate_scache_line(a); /* Hit_Invalidate_SD */
 		if (a == end) break;
 		a += sc_lsize;
 	}
 




-- 
In theory, practice and theory are the same, but in practice they 
are different -- Larry McVoy

Re: [patch] R4k cache code synchronization

[Juan Quintela]

>>>>> "maciej" == Maciej W Rozycki <macro@ds2.pg.gda.pl> writes:

maciej> On 10 Jan 2003, Juan Quintela wrote:
>> The only thing that could be controversial is the _l1() thing, and as
>> current thing is broken, I vote for insclusion.
>> 
maciej> diff -up --recursive --new-file linux-mips-2.4.20-pre6-20030107.macro/arch/mips64/mm/c-r4k.c linux-mips-2.4.20-pre6-20030107/arch/mips64/mm/c-r4k.c
maciej> --- linux-mips-2.4.20-pre6-20030107.macro/arch/mips64/mm/c-r4k.c	2002-12-20 03:56:52.000000000 +0000
maciej> +++ linux-mips-2.4.20-pre6-20030107/arch/mips64/mm/c-r4k.c	2003-01-09 23:21:39.000000000 +0000
>> @@ -979,7 +980,7 @@ static void r4k_dma_cache_wback_inv_sc(u
>> unsigned long end, a;
>> 
>> if (size >= scache_size) {
>> -		flush_cache_l1();
>> +		flush_cache_all();
>> return;
>> }
>> 
>> This one is fixing a bug, we are talking about a chip with Secondary
>> cache and don't touch the secondary cache at all :(

maciej> That bug is inactive -- both function pointers are defined to the same
maciej> value as surprisinly enough "l1" means "both caches" for the R4k.  Anyway,
maciej> I for removing flush_cache_l1() altogether in the next step. 

Yep, you are right, only 2 weeks since I looked at that file and
already forgot it.

Ralf, current code (as Maciej tolds), just have _l1 & _l2 variants,
but at least in that file, they are defined to be the same :(

I also vote to unify the mips & mips64 versions of that file, they are
the same :(

Maciej, in the other hand, you didn't coment in the other part, that
we writeback & invalidate when we are asked only to invalidate?

Later, Juan.

-- 
In theory, practice and theory are the same, but in practice they 
are different -- Larry McVoy

Re: [patch] R4k cache code synchronization

[Maciej W. Rozycki]

On 10 Jan 2003, Juan Quintela wrote:

> Maciej, in the other hand, you didn't coment in the other part, that
> we writeback & invalidate when we are asked only to invalidate?

 I did some investigation now and I think dma_cache_wback_inv(),
dma_cache_wback() and dma_cache_inv() all should do what the name implies. 
I'll make a fix.

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +