Messages in this thread | | | Subject | Re: [RFC/PATCH] dma: dma_{un}map_{single|sg}_attrs() interface | From | Roland Dreier <> | Date | Tue, 22 Jan 2008 20:59:50 -0800 |
| |
> --- a/include/linux/dma-attrs.h > +++ b/include/linux/dma-attrs.h > @@ -0,0 +1,33 @@ > +#ifndef _DMA_ATTR_H > +#define _DMA_ATTR_H > + > +#include <linux/bug.h> > + > +enum { > + DMA_ATTR_INVALID, > + DMA_ATTR_BARRIER, > + DMA_ATTR_FOO, > + DMA_ATTR_GOO, > + DMA_ATTR_MAX, > +}; > + > +struct dma_attrs { > + unsigned flags; > +}; > + > +static inline int dma_set_attr(struct dma_attrs *attrs, unsigned attr) {
maybe this would be cleaner if you named the DMA_ATTR enum and used that instead of unsigned here (and below)?
> + BUG_ON(attrs == NULL);
does this BUG_ON() buy us much? It seems the only thing we would fail to oops on is if someone did dma_set_attr(NULL, INVALID) and I'm not sure it's worth it to BUG here.
> + if (attr > DMA_ATTR_INVALID && attr < DMA_ATTR_MAX) { > + attrs->flags = (1 << attr); > + return 0; > + } > + return 1;
returning -EINVAL here instead of 1 would probably be more "kernelish".
> +} > + > +static inline int dma_get_attr(struct dma_attrs *attrs, unsigned attr) { > + if (attrs) > + return attrs->flags & (1 << attr);
so it's OK to pass attrs == NULL into dma_get_attr() but not into dma_set_attr()? seems kind of odd.
> + return 0; > +}
It seems you're missing a way to initialize a struct dma_attrs. How do I clear the flags field to start with?
A macro like DEFINE_DMA_ATTRS() that initializes things for you (like LIST_HEAD or DEFINE_SPIN_LOCK) would probably be a good thing to have as well.
Also I guess you could test ARCH_USES_DMA_ATTRS in this file and stub everything out and define an empty structure if it's not defined. save a few bytes of stack etc.
> + > +#endif /* _DMA_ATTR_H */
| |