On Mon, May 13, 2013 at 07:02:07PM -0500, H Hartley Sweeten wrote:
> On Monday, May 13, 2013 1:06 AM, Dan Carpenter wrote:
> > On Fri, May 10, 2013 at 11:37:38AM -0500, H Hartley Sweeten wrote:
> >> On Friday, May 10, 2013 6:07 AM, Ian Abbott wrote:
> >>> @@ -84,11 +86,15 @@ static void __comedi_buf_alloc(struct comedi_device
> >>> *dev,
> >>> for (i = 0; i < n_pages; i++) {
> >>> buf = &async->buf_page_list[i];
> >>> if (s->async_dma_dir != DMA_NONE)
> >>> +#ifdef CONFIG_HAS_DMA
> >>> buf->virt_addr = dma_alloc_coherent(dev->hw_dev,
> >>> PAGE_SIZE,
> >>> &buf->dma_addr,
> >>> GFP_KERNEL |
> >>> __GFP_COMP);
> >>> +#else
> >>> + break;
> >>> +#endif
> >>
> >> This break is unnecessary.
> >>
> >
> > You'd need something there or it would cause a parse error when
> > CONFIG_HAS_DMA is disabled. "break;" is probably as good as
> > "buf->virt_addr = NULL;" or whatever.
>
> async->buf_page_list is vzalloc'ed, so buf->virt_addr is already NULL.
>
Yes. I know. So what you're suggesting is this?:
if (s->async_dma_dir != DMA_NONE)
#ifdef CONFIG_HAS_DMA
buf->virt_addr = dma_alloc_coherent(dev->hw_dev,
PAGE_SIZE,
&buf->dma_addr,
GFP_KERNEL |
__GFP_COMP);
#else
;
#endif
That looks nasty to me. Actually this should probably be hidden
away in a .h file per kernel style guidelines.
#ifdef CONFIG_HAS_DMA
static inline void *
comedi_dma_alloc_coherent(struct device *dev, size_t size, dma_addr_t
*dma_handle,
gfp_t gfp)
{
return dma_alloc_coherent(dev, size, dma_handle, gfp);
}
#else
static inline void *
comedi_dma_alloc_coherent(struct device *dev, size_t size, dma_addr_t
*dma_handle,
gfp_t gfp)
{
return NULL;
}
#endif
regards,
dan carpenter
_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
