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

[Becky Bruce]

On Nov 18, 2008, at 6:22 AM, Benjamin Herrenschmidt wrote:

>
>> 				dma_addr_t dma_address, size_t size,
>> 				enum dma_data_direction direction,
>> 				struct dma_attrs *attrs);
>> +	void            (*sync_single_for_cpu)(struct device *hwdev,
>> +				dma_addr_t dma_handle, size_t size,
>> +				enum dma_data_direction direction);
>> +	void            (*sync_single_for_device)(struct device *hwdev,
>> +				dma_addr_t dma_handle, size_t size,
>> +				enum dma_data_direction direction);
>> +	void            (*sync_single_range_for_cpu)(struct device *hwdev,
>> +				dma_addr_t dma_handle, unsigned long offset,
>> +				size_t size,
>> +				enum dma_data_direction direction);
>> +	void            (*sync_single_range_for_device)(struct device  
>> *hwdev,
>> +				dma_addr_t dma_handle, unsigned long offset,
>> +				size_t size,
>> +				enum dma_data_direction direction);
>
> Can't we implement the first 2 using the next 2 and passing a 0  
> offset ?
> I mean, we're going to go through a function pointer, it's not like
> handling the offset was going to cost us something :-)

Looking at the swiotlb code; I think that's a reasonable thing to do.   
Will respin.

>
>
>> +	void            (*sync_sg_for_cpu)(struct device *hwdev,
>> +				struct scatterlist *sg, int nelems,
>> +				enum dma_data_direction direction);
>> +	void            (*sync_sg_for_device)(struct device *hwdev,
>> +				struct scatterlist *sg, int nelems,
>> +				enum dma_data_direction direction);
>> };
>>
>> /*
>> @@ -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);
>> -	__dma_sync(bus_to_virt(dma_handle), size, direction);
>> +	struct dma_mapping_ops *dma_ops = get_dma_ops(dev);
>> +
>> +	BUG_ON(!dma_ops);
>> +	if (dma_ops->sync_single_for_cpu != NULL)
>> +		dma_ops->sync_single_for_cpu(dev, dma_handle, size,
>> +					     direction);
>> }
>
> .../...
>
> The only objection that comes to mind here is that generally, you know
> that your platform is going to be coherent or not at compile time, and
> you don't want the above at all when it is (or won't need swiotlb).
>
> So maybe we could have a Kconfig trickery so that
> CONFIG_PPC_DMA_NEED_SYNC_OPS is set when either non coherent DMA is
> select'ed or swiotlb support is select'ed by one of the enabled
> platform. So if I enable only powermac, I get neither and don't get  
> the
> bloody inlines :-)

I thought about doing something like this, but didn't know if it was  
worth the ifdeffing/CONFIG foo that would ensue; I guess your response  
answers that question :)

Thanks!
B