From: akepner@sgi.com
Date: Tue, 8 Jan 2008 10:10:29 -0800
To: Ingo Molnar <mingo@elte.hu>
Cc: Tony Luck <tony.luck@intel.com>,
       Grant Grundler <grundler@parisc-linux.org>,
       Jesse Barnes <jbarnes@virtuousgeek.org>, Jes Sorensen <jes@sgi.com>,
       Randy Dunlap <randy.dunlap@oracle.com>,
       Roland Dreier <rdreier@cisco.com>,
       James Bottomley <James.Bottomley@steeleye.com>,
       David Miller <davem@davemloft.net>, Muli Ben-Yehuda <muli@il.ibm.com>,
       linux-kernel@vger.kernel.org
Subject: Re: [RFC/PARTIAL PATCH 1/3] dma: create linux/dma-direction.h
Message-ID: <20080108181029.GY23661@sgi.com>
References: <20080108023559.GQ23661@sgi.com> <20080108085831.GA27671@elte.hu>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20080108085831.GA27671@elte.hu>
User-Agent: Mutt/1.4.1i
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1822
Lines: 60

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/