[RFC] powerpc: Merge 32/64 cacheflush code

[Milton Miller]

On Dec 19, 2005, at 7:06 PM, David Gibson wrote:

> On Mon, Dec 19, 2005 at 08:52:47AM -0600, Milton Miller wrote:
>> On Mon Dec 19 16:44:10 EST 2005, David Gibson wrote:
>>
>>> +extern void wback_dcache_range(unsigned long start, unsigned long
>>> stop);
>>> +extern void wback_inval_dcache_range(unsigned long start, unsigned
>>> long stop);
>>
>> I think that while we are here we should change the arguments to be
>> pointers (void *).   The assembly doesn't care, and almost all of the
>> users are casting from pointer to usigned long at the call site, with
>> dart being the exception.  The instruction cache flush should also
>> change.
>
> True.  And while we're at that, the dcache flushing functions are
> almost invariable called as *_dcache_range(start, start+length), so
> how about changing them to take start and length instead of start and
> end.  However, flush_icache_range() is called from generic code, so I
> don't want to change it's interface.
>

The information from the above paragraph should be in the change log.

And if we are not going to change icache, then I would propose we not 
use range on the dcache side, it is confusing.   How about rename to th 
start, length ones to _region ?


> Revised patch below:
>
> powerpc: Merge 32/64 cacheflush code
>
> This patch merges the cache flushing code for 32 and 64 bit powerpc
> machines.  This means the ppc64_caches mechanism for determining
> correct cache sizes at runtime is ported to 32-bit, and is thus
> renamed as 'powerpc_caches'.  The merged cache flushing functions go
> in new file arch/powerpc/kernel/cache.S.
>
> Previously, the ppc32 version of flush_dcache_range() did a writeback
> and invalidate of the given cache lines (dcbf) whereas the ppc64
> version did just a writeback (dcbst).  In general, there's no
> consistent meaning of "flush" as one or the other, so this patch also
> renames the dcache flushing functions less ambiguously.  The new names
> are:
>
> 	wback_dcache_range() - previously flush_dcache_range() on
> ppc64 and clean_dcache_range() on ppc32
>
> 	wback_inval_dcache_range() - previously
> flush_inval_dcache_range() on ppc64 and flush_dcache_range on ppc32
>
> 	invalidate_dcache_range() - didn't previously exist on ppc64,
> unchanged on ppc32
>
> Finally we also cleanup the initialization of the powerpc_caches
> structure from the old ppc64 specific version.  We remove a pointless
> loop, and remove a dependence on _machine.

...

> +/*
> + * Flush a particular page from the data cache to RAM.
> + * Note: this is necessary because the instruction cache does *not*
> + * snoop from the data cache.
> + *
> + *	void __flush_dcache_icache(void *page)

When I see *page i think struct page *page .... even though this is 
void, how about page_address ?

...

> +/*
> + * Like above, but only do the D-cache.
> + *
> + * wback_dcache_range(void *start, unsigned long len)
> + *
> + *    writeback all bytes from start to stop-1 inclusive
> + */
> +_GLOBAL(wback_dcache_range)
> + 	LOAD_REG_ADDR(r10, powerpc_caches)
> +	lwz	r7,DCACHEL1LINESIZE(r10)	/* Get dcache line size */
> +	addi	r5,r7,-1
> +	andc	r6,r3,r5		/* round low to line bdy */
> +	and	r8,r3,r5		/* get cacheline offset of start */

Does the above term ever make a difference since we round up?

> +	add	r8,r8,r4		/* add length */
> +	add	r8,r8,r5		/* ensure we get enough */
> +	lwz	r9,DCACHEL1LOGLINESIZE(r10)	/* Get dcache line shift */

Moving this load up might help some processors

> +	srw.	r8,r8,r9		/* compute line count */

So its really unsigned int len (or did you want a PPC_ macro here?)

> +	beqlr				/* nothing to do? */
> +	mtctr	r8
> +0:	dcbst	0,r6
> +	add	r6,r6,r7
> +	bdnz	0b
> +	sync
> +	blr
> +
>

milton