Messages in this thread | | | Date | Thu, 13 Mar 2008 08:27:54 -0700 | From | Randy Dunlap <> | Subject | Re: [PATCH 0/4 v4] dma: dma_{un}map_{single|sg}_attrs() interface |
| |
On Thu, 13 Mar 2008 01:41:06 -0700 Andrew Morton wrote:
> > +#ifdef ARCH_USES_DMA_ATTRS > > There is no precedent for ARCH_USES_*. > > There is a little bit of precedent for ARCH_HAVE_* > > There is lots of precendence for ARCH_HAS_*. > > We don't like ARCH_HAS_* anyway ;) What can we do to get rid of this? > Ideally, make it available on all architectures at zero cost to those which > don't need it. If that is impractical (why?) then it is preferable to do > this in Kconfig.
Sam has been pushing HAVE_* and even added that to Documentation/kbuild/kconfig-language.txt.
> > +/* > > + * dma_set_attr - set a specific attribute > > + * may be called with a null attrs > > + */
Use kernel-doc notation?
> > +static inline int dma_set_attr(struct dma_attrs *attrs, enum dma_attr attr) > > +{ > > + if (!attrs) > > + return 0; > > + if (attr < DMA_ATTR_MAX) { > > + attrs->flags |= (1 << attr); > > + return 0; > > + } > > + return -EINVAL; > > +} > > Is there any non-buggy reason why code would pass an out-of-range attribute > into this function? If not, BUG_ON() would be appropriate treatment. > > This function might already be too large to inline, and a BUG_ON() might > make it larger. > > > +/* > > + * dma_get_attr - check for a specific attribute > > + * may be called with a null attrs > > + */
Ditto.
> > +static inline int dma_get_attr(struct dma_attrs *attrs, enum dma_attr attr) > > +{ > > + if (!attrs) > > + return 0; > > + if (attr < DMA_ATTR_MAX) { > > + int ret = attrs->flags & (1 << attr); > > + return !!ret; > > + } > > + return -EINVAL; > > +}
--- ~Randy
| |