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

[akepner@sgi.com]

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