[PATCH] powerpc: Add sync_*_for_* to dma_ops

Becky Bruce becky.bruce at freescale.com
Wed Nov 19 06:29:32 EST 2008


On Nov 18, 2008, at 10:57 AM, Segher Boessenkool wrote:

>> @@ -286,42 +306,75 @@ static inline void  
>> dma_sync_single_for_cpu(struct device *dev,
>> 		dma_addr_t dma_handle, size_t size,
>> 		enum dma_data_direction direction)
>> {
>> -	BUG_ON(direction == DMA_NONE);
>
> Did you intend to remove this here?  It would be nice to test for it
> even on platforms where the op is a nop; if there is an equivalent
> test in every implementation of the ops, remove it there instead
> (that's more source + binary code to remove, always a good thing ;-) )


You're right that we have lost this test in platforms which do not  
implement the sync ops, and if the test is useful, I can certainly  
move it.  However, it will actually result in more code on the current  
ppc kernel, as there are 3x more dma_sync_* instances than there are  
actual dma_direct_* instances (and nobody else implements  
dma_sync_* :)  Not that we're talking about a lot of code here, but it  
definitely won't make the kernel smaller since we just don't need a  
sync unless we're noncoherent which, thank <insert deity here>, is rare.

With the swiotlb case, the check needs to be in the swiotlb code  
because there are other architectures that use the swiotlb code that  
do not use a structure to get to their dma_ops like we do.  IA64, for  
example, sets up its dma_ops using config options and #defines the  
names of the various dma_ functions.  They don't go through a dma_ops  
dereference, so the (possibly redundant for us) check in the swiotlb  
code would need to remain.

So, the reason for moving it would be that we think it's still a  
useful check to have.  I'm happy to move it in that case.

Thanks,
-Becky




More information about the Linuxppc-dev mailing list