lkml.org 
[lkml]   [2008]   [Jan]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC/PATCH] dma: dma_{un}map_{single|sg}_attrs() interface
From
Date
 > --- 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 */


\
 
 \ /
  Last update: 2008-01-23 06:03    [W:0.060 / U:0.408 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site