On Monday 27 June 2011, Marek Szyprowski wrote:
On Friday, June 24, 2011 5:37 PM Arnd Bergmann wrote:
On Monday 20 June 2011, Marek Szyprowski wrote:
This patch modifies dma-mapping implementation on ARM architecture to use common dma_map_ops structure and asm-generic/dma-mapping-common.h helpers.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
This is a good idea in general, but I have a few concerns about details:
First of all, should we only allow using dma_map_ops on ARM, or do we also want to support a case where these are all inlined as before?
I really wonder if it is possible to have a clean implementation of dma_map_ops based and linear inlined dma-mapping framework together. Theoretically it should be possible, but it will end with a lot of #ifdef hackery which is really hard to follow and understand for anyone but the authors.
Right. It's probably not worth unless there is a significant overhead in terms of code size or performance in the coherent linear case.
I suppose for the majority of the cases, the overhead of the indirect function call is near-zero, compared to the overhead of the cache management operation, so it would only make a difference for coherent systems without an IOMMU. Do we care about micro-optimizing those?
Even in coherent case, the overhead caused by additional function call should have really negligible impact on drivers performance.
What about object code size? I guess since ixp23xx is the only platform that announces itself as coherent, we probably don't need to worry about it too much either. Lennert?
On everything else, we only replace a direct functin call with an indirect one.
diff --git a/arch/arm/include/asm/dma-mapping.h
b/arch/arm/include/asm/dma-mapping.h
index 799669d..f4e4968 100644 --- a/arch/arm/include/asm/dma-mapping.h +++ b/arch/arm/include/asm/dma-mapping.h @@ -10,6 +10,27 @@ #include <asm-generic/dma-coherent.h> #include <asm/memory.h>
+extern struct dma_map_ops dma_ops;
+static inline struct dma_map_ops *get_dma_ops(struct device *dev) +{
- if (dev->archdata.dma_ops)
return dev->archdata.dma_ops;- return &dma_ops;
+}
I would not name the global structure just 'dma_ops', the identifier could too easily conflict with a local variable in some driver. How about arm_dma_ops or linear_dma_ops instead?
I'm fine with both of them. Even arm_linear_dma_ops make some sense.
Ok, just pick one then if nobody has a strong opinion either way.
You should not need to make these symbols visible in the header file any more unless they are used outside of the main file later.
They are used by the dma bounce code once converted to dma_map_ops framework.
Ok, I see.
Arnd