[RFC/PARTIAL PATCH 1/3] dma: create linux/dma-direction.h

[RFC/PARTIAL PATCH 1/3] dma: create linux/dma-direction.h

[akepner]

Place the definition of enum dma_data_direction in its own 
include file, and add the definition of enum dma_data_attr 
and some simple routines for manipulating the direction and 
attributes.

Signed-off-by: Arthur Kepner <akepner@sgi.com>

-- 

 dma-direction.h |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 dma-mapping.h   |   17 +----------------
 2 files changed, 54 insertions(+), 16 deletions(-)

diff --git a/include/linux/dma-direction.h b/include/linux/dma-direction.h
index e69de29..4c67446 100644
--- a/include/linux/dma-direction.h
+++ b/include/linux/dma-direction.h
@@ -0,0 +1,53 @@
+#ifndef _LINUX_DMA_DIRECTION_H
+#define _LINUX_DMA_DIRECTION_H
+
+/* These definitions mirror those in pci.h, so they can be used
+ * interchangeably with their PCI_ counterparts */
+enum dma_data_direction {
+	DMA_BIDIRECTIONAL = 0,
+	DMA_TO_DEVICE = 1,
+	DMA_FROM_DEVICE = 2,
+	DMA_NONE = 3,
+};
+
+enum dma_data_attr {
+	DMA_ATTR_BARRIER = (1 << 0),
+	DMA_ATTR_FOO = (1 << 1),
+	DMA_ATTR_GOO = (1 << 2),
+	DMA_ATTR_MAX = (1 << 3),
+};
+
+#define DMA_FLAGS_ATTR_SHIFT	8
+#define DMA_FLAGS_DIR_MASK	((1 << DMA_FLAGS_ATTR_SHIFT) - 1)
+#define DMA_FLAGS_ATTR_MASK	~DMA_FLAGS_DIR_MASK
+
+static inline enum dma_data_direction dma_flags_get_dir(u32 fin)
+{
+	return (fin & DMA_FLAGS_DIR_MASK);
+}
+
+/* return a u32 (not a enum dma_data_attr) since more than one 
+ * attribute may be set */
+static inline u32 dma_flags_get_attr(u32 fin)
+{
+	return ((fin & DMA_FLAGS_ATTR_MASK) >> DMA_FLAGS_ATTR_SHIFT);
+}
+
+static inline u32 dma_flags_set_dir_attr(enum dma_data_direction dir, u32 attr)
+{
+	return (attr << DMA_FLAGS_ATTR_SHIFT) | (dir & DMA_FLAGS_DIR_MASK);
+}
+
+static inline int valid_dma_direction(enum dma_data_direction dir)
+{
+	return ((dir == DMA_BIDIRECTIONAL) ||
+		(dir == DMA_TO_DEVICE) ||
+		(dir == DMA_FROM_DEVICE));
+}
+
+static inline int valid_dma_attr(u32 attr)
+{
+	return(attr < DMA_ATTR_MAX);
+}
+
+#endif  /*_LINUX_DMA_DIRECTION_H */
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 101a2d4..be5825e 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -3,15 +3,7 @@
 
 #include <linux/device.h>
 #include <linux/err.h>
-
-/* These definitions mirror those in pci.h, so they can be used
- * interchangeably with their PCI_ counterparts */
-enum dma_data_direction {
-	DMA_BIDIRECTIONAL = 0,
-	DMA_TO_DEVICE = 1,
-	DMA_FROM_DEVICE = 2,
-	DMA_NONE = 3,
-};
+#include <linux/dma-direction.h>
 
 #define DMA_BIT_MASK(n)	(((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
 
@@ -36,13 +28,6 @@ enum dma_data_direction {
 
 #define DMA_MASK_NONE	0x0ULL
 
-static inline int valid_dma_direction(int dma_direction)
-{
-	return ((dma_direction == DMA_BIDIRECTIONAL) ||
-		(dma_direction == DMA_TO_DEVICE) ||
-		(dma_direction == DMA_FROM_DEVICE));
-}
-
 static inline int is_device_dma_capable(struct device *dev)
 {
 	return dev->dma_mask != NULL && *dev->dma_mask != DMA_MASK_NONE;



-- 
Arthur

Re: [RFC/PARTIAL PATCH 1/3] dma: create linux/dma-direction.h

[Ingo Molnar]

* akepner@sgi.com <akepner@sgi.com> wrote:

> +enum dma_data_attr {
> +	DMA_ATTR_BARRIER = (1 << 0),
> +	DMA_ATTR_FOO = (1 << 1),
> +	DMA_ATTR_GOO = (1 << 2),
> +	DMA_ATTR_MAX = (1 << 3),
> +};

FOO/GOO we dont need i guess ...

> +#define DMA_FLAGS_ATTR_SHIFT	8
> +#define DMA_FLAGS_DIR_MASK	((1 << DMA_FLAGS_ATTR_SHIFT) - 1)
> +#define DMA_FLAGS_ATTR_MASK	~DMA_FLAGS_DIR_MASK
> +
> +static inline enum dma_data_direction dma_flags_get_dir(u32 fin)
> +{
> +	return (fin & DMA_FLAGS_DIR_MASK);
> +}

the u32 looks a bit weird. Why not unsigned int ?

also, are the new dma_map_*() API compatible with the old one? I.e. does 
dma_map_*(...,0) and dma_map_*(...,1) map to the right thing? If yes 
then perhaps dont change 'int direction' to 'u32 flags' at all but just 
rename 'direction' to 'flags' and be done with it.

also, this conversion:

+       enum dma_data_direction direction = dma_flags_get_dir(flags);

would be unnecessary if callers passed in the bitmap already, instead of 
'flags'. 0 and 1 would still map to the right thing i think.

	Ingo

Re: [RFC/PARTIAL PATCH 1/3] dma: create linux/dma-direction.h

[akepner]

On Tue, Jan 08, 2008 at 09:58:31AM +0100, Ingo Molnar wrote:
> 
> * akepner@sgi.com <akepner@sgi.com> wrote:
> 
> > +enum dma_data_attr {
> > +	DMA_ATTR_BARRIER = (1 << 0),
> > +	DMA_ATTR_FOO = (1 << 1),
> > +	DMA_ATTR_GOO = (1 << 2),
> > +	DMA_ATTR_MAX = (1 << 3),
> > +};
> 
> FOO/GOO we dont need i guess ...

Right. That's example GOO ;-)

> 
> > +#define DMA_FLAGS_ATTR_SHIFT	8
> > +#define DMA_FLAGS_DIR_MASK	((1 << DMA_FLAGS_ATTR_SHIFT) - 1)
> > +#define DMA_FLAGS_ATTR_MASK	~DMA_FLAGS_DIR_MASK
> > +
> > +static inline enum dma_data_direction dma_flags_get_dir(u32 fin)
> > +{
> > +	return (fin & DMA_FLAGS_DIR_MASK);
> > +}
> 
> the u32 looks a bit weird. Why not unsigned int ?
> 

unsigned int would be fine with me.

> also, are the new dma_map_*() API compatible with the old one? I.e. does 
> dma_map_*(...,0) and dma_map_*(...,1) map to the right thing? If yes 
> then perhaps dont change 'int direction' to 'u32 flags' at all but just 
> rename 'direction' to 'flags' and be done with it.
>

Yes, the callers don't necessarily need to be modified. Callers 
of dma_map_* would only need to be changed if they want to pass 
some additional attribute(s).
 
> also, this conversion:
> 
> +       enum dma_data_direction direction = dma_flags_get_dir(flags);
> 
> would be unnecessary if callers passed in the bitmap already, instead of 
> 'flags'. 0 and 1 would still map to the right thing i think.
> 

Right, but if the caller *had* passed some optional attribute, we 
probably want to strip it off (and either use it or ignore it, as 
appropriate.)

-- 
Arthur