dma: add dma_flags_set/get_*() interfaces
PATCH 1/3 dma: add dma_flags_set/get_*() interfaces
Introduce the dma_flags_set/get_*() interfaces and give them default implementations.
Architectures which allow DMA to be reordered between a device and host memory (within a NUMA interconnect) can redefine these to allow a driver to explicitly synchronize DMA from the device when necessary.
---
Andrew, this is the first in a series of three patches:
[1/3] dma: add dma_flags_set/get_*() interfaces [2/3] dma: redefine dma_flags_set/get_*() for sn-ia64 [3/3] dma: document dma_flags_set/get_*()
Variants of these patches have been discussed on several occasions, most recently in a thread beginning:
http://marc.info/?l=linux-kernel&m=119137949604365&w=2
Please consider this for 2.6.24.
Jes, Tony, please note that I added an explicit test for CONFIG_IA64_SGI_SN2 in asm-ia64/sn/io.h (Yuck). This is needed for IA64_GENERIC to build, since there's at least one driver (qla1280) that includes asm-ia64/sn/io.h for CONFIG_IA64_GENERIC.
dma-mapping.h | 18 ++++++++++++++++++ 1 files changed, 18 insertions(+)
include/linux/dma-mapping.h +18 -0
@@ -106,4 +106,22 @@ static inline void dmam_release_declared_memory(struct device *dev) }#endif /* ARCH_HAS_DMA_DECLARE_COHERENT_MEMORY */ +#define DMA_BARRIER_ATTR 0x1 +#ifndef ARCH_USES_DMA_ATTRS +static inline int dma_flags_set_attr (u32 attr,enum dma_data_direction dir) +{ +return dir; +} + +static inline int dma_flags_get_dir (int flags) +{ +return flags; +} + +static inline int dma_flags_get_attr (int flags) +{ +return 0 ; +} +#endif /* ARCH_USES_DMA_ATTRS */ +#endif
include/linux/dma-mapping.h static inline void dmam_release_declared_memory(struct device *dev) · line 114
Quoted 6 lines · On Tue, 16 Oct 2007 18:41:28 -0700 [email protected] wrote:
include/linux/dma-mapping.h +6
-0+#define DMA_BARRIER_ATTR 0x1 +#ifndef ARCH_USES_DMA_ATTRS +static inline int dma_flags_set_attr (u32 attr,enum dma_data_direction dir) +{ +return dir; +}
This function takes an `enum dma_data_direction' as its second arg, but your ia64 implementation takes an 'int'.
Quoted 9 lines
include/linux/dma-mapping.h +9
-0+static inline int dma_flags_get_dir (int flags) +{ +return flags; +} + +static inline int dma_flags_get_attr (int flags) +{ +return 0 ; +}
[reply to the series of three mails below]
Quoted 12 lines · Andrew Morton
include/linux/dma-mapping.h static inline void dmam_release_declared_memory(struct device *dev) · line 114
Quoted 6 lines · On Tue, 16 Oct 2007 18:41:28 -0700 [email protected] wrote:
include/linux/dma-mapping.h +6
-0+#define DMA_BARRIER_ATTR 0x1 +#ifndef ARCH_USES_DMA_ATTRS +static inline int dma_flags_set_attr (u32 attr,enum dma_data_direction dir) +{ +return dir; +}This function takes an `enum
dma_data_direction' as its second arg, but your ia64 implementation takes an 'int'.
This is because the dma_data_direction enum type isn't available at the point the ia64 implementation is defined.
Quoted 11 lines
Review
Quoted 10 lines
.....
dma_addr_tsn_dma_map_single(struct device *dev, void*cpu_addr,size_tsize,(quoted diff) +2
-1- int direction) + int flags) { dma_addr_t dma_addr; unsigned long phys_addr; struct pci_dev *pdev = to_pci_dev(dev); struct sn_pcibus_provider *provider = SN_PCIDEV_BUSPROVIDER(pdev); + int dmabarrier = dma_flags_get_attr(flags) & DMA_BARRIER_ATTR;
Quoted 4 lines
So we take an `enum
data_direction' and then wedge it into a word alongside some extra flags?Can we do something nicer than that?
Changing the type of the last argument to dma_map_* functions to be a bitmask? Or adding an additional argument? (Both of which you mention below.)
Quoted 12 lines
Review
Quoted 10 lines
.....
(quoted diff) +8
-0+DMA_BARRIER_ATTR would be set when the memory region is mapped for DMA, +e.g.: + + int count; + int flags = dma_flags_set_attr(DMA_BARRIER_ATTR, DMA_BIDIRECTIONAL); + .... + count = dma_map_sg(dev, sglist, nents, flags); +Isn't this rather a kludge?
I prefer the term "hack".
Quoted 5 lines
What would be the cost of doing this cleanly and either redefining
dma_data_directionto be a field-of-bits or just leavedma_data_directionalone (it is quite unrelated to this work, isn't it?) and adding new fields/arguments to manage this new functionality?
It'd be significantly more work to do change or add arguments to the dma_map_* functions. But if that's what I need to do to get this accepted, I'll back up and have another go at it.
Signature · 1 line
Arthur
Quoted 9 lines · On Wed, 17 Oct 2007 09:03:27 -0700 [email protected] wrote:
Quoted 5 lines
What would be the cost of doing this cleanly and either redefining
dma_data_directionto be a field-of-bits or just leavedma_data_directionalone (it is quite unrelated to this work, isn't it?) and adding new fields/arguments to manage this new functionality?It'd be significantly more work to do change or add arguments to the dma_map_* functions. But if that's what I need to do to get this accepted, I'll back up and have another go at it.
I don't have any particularly strong opinions on which would be the best way to clean this up. Hopefully someone who is more involved with the DMA mapping interfaces can help out.
It wouldn't be efficient for you to implement something new, only to have it criticized again. I'd suggest that you come up with a concrete design, describe to us what you propose to do and let's take it from there.