Messages in this thread | | | Date | Sat, 17 May 2014 15:19:21 +0300 | From | Eli Billauer <> | Subject | Re: [PATCH 2/5] dma-mapping: Add devm_ interface for dma_map_single() |
| |
Hello Tejun,
On 17/05/14 00:08, Tejun Heo wrote: > > Don't we wanna map the underlying operation - dma_map_single_attrs() - > instead? >
I'll resubmit this patch promptly, with a follow-up patch for the diff to implement dmam_map_single_attrs() instead. Plus a define-statement for dmam_map_single(). I can't test the case of a non-NULL value for @attrs however.
> >> + if (dma_mapping_error(dev, dma_handle)) { >> + devres_free(dr); >> + return 0; >> > Can't we just keep returning dma_handle? Even if that means invoking > ->mapping_error() twice? It's yucky to have subtly different error > return especially because in most cases it won't fail. > Yucky it is indeed. There are however two problems with keeping the existing API:
* What to do if devres_alloc() fails. How do I signal back an error? The only way I can think of is returning zero. But if the caller should know that zero means failure, I've already broken the API. I might as well return zero for any kind of failure. * It seems like a lot of dma_mapping_error() implementations always return no-error, since the DMA mapping can't fail on specific architectures. If callers use dma_mapping_error(), the possible devres_alloc() failure will be missed.
By the way, where I've seen dma_mapping_error() doing something, it checks for dma_handle == 0.
Submitting updated patches for the DMA mapping part soon.
Regards, Eli
| |