| Message ID | 1386613669-30847-1-git-send-email-ezequiel.garcia@free-electrons.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Commit | b8c01d259a08d75c5049b2bd5f579648262c30a4 |
| Delegated to: | Dan Williams |
| Headers | show |
On Mon, Dec 09, 2013 at 03:27:49PM -0300, Ezequiel Garcia wrote: > This commit adds proper error checking for various DMA API calls, > as reported by DMA_API_DEBUG=y. > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > --- > drivers/dma/mv_xor.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 52 insertions(+), 4 deletions(-) > > diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c > index 7962088..670ee61 100644 > --- a/drivers/dma/mv_xor.c > +++ b/drivers/dma/mv_xor.c > @@ -784,7 +784,7 @@ static void mv_xor_issue_pending(struct dma_chan *chan) > > static int mv_xor_memcpy_self_test(struct mv_xor_chan *mv_chan) > { > - int i; > + int i, ret; > void *src, *dest; > dma_addr_t src_dma, dest_dma; > struct dma_chan *dma_chan; > @@ -821,19 +821,44 @@ static int mv_xor_memcpy_self_test(struct mv_xor_chan *mv_chan) > > src_dma = dma_map_page(dma_chan->device->dev, virt_to_page(src), 0, > PAGE_SIZE, DMA_TO_DEVICE); > - unmap->to_cnt = 1; > unmap->addr[0] = src_dma; > > + ret = dma_mapping_error(dma_chan->device->dev, src_dma); > + if (ret) { > + err = -ENOMEM; why override and not return 'ret'? > + goto free_resources; -- ~Vinod -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 10, 2013 at 09:49:07AM -0300, Ezequiel Garcia wrote: > Hi Vinod, > > On Tue, Dec 10, 2013 at 05:03:19PM +0530, Vinod Koul wrote: > [..] > > > > > > + ret = dma_mapping_error(dma_chan->device->dev, src_dma); > > > + if (ret) { > > > + err = -ENOMEM; > > why override and not return 'ret'? > > Because, as far as I could understand, dma_mapping_error() doesn't return an > error code, but a boolean. > > Maybe I overlooked something? Yes looks like this is the case indeed!
Hi Vinod, On Tue, Dec 10, 2013 at 05:03:19PM +0530, Vinod Koul wrote: [..] > > > > + ret = dma_mapping_error(dma_chan->device->dev, src_dma); > > + if (ret) { > > + err = -ENOMEM; > why override and not return 'ret'? Because, as far as I could understand, dma_mapping_error() doesn't return an error code, but a boolean. Maybe I overlooked something?
On Tue, Dec 10, 2013 at 06:06:24PM +0530, Vinod Koul wrote: > On Tue, Dec 10, 2013 at 09:49:07AM -0300, Ezequiel Garcia wrote: > > Hi Vinod, > > > > On Tue, Dec 10, 2013 at 05:03:19PM +0530, Vinod Koul wrote: > > [..] > > > > > > > > + ret = dma_mapping_error(dma_chan->device->dev, src_dma); > > > > + if (ret) { > > > > + err = -ENOMEM; > > > why override and not return 'ret'? > > > > Because, as far as I could understand, dma_mapping_error() doesn't return an > > error code, but a boolean. > > > > Maybe I overlooked something? > Yes looks like this is the case indeed! > I guess the fact I'm reusing the 'ret' variable and changing its meaning, doesn't help to avoid the confusion... With all the error checking in-place, the self-test code now looks somewhat ugly. I'll see about cleaning that up if I can find some time.
diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c index 7962088..670ee61 100644 --- a/drivers/dma/mv_xor.c +++ b/drivers/dma/mv_xor.c @@ -784,7 +784,7 @@ static void mv_xor_issue_pending(struct dma_chan *chan) static int mv_xor_memcpy_self_test(struct mv_xor_chan *mv_chan) { - int i; + int i, ret; void *src, *dest; dma_addr_t src_dma, dest_dma; struct dma_chan *dma_chan; @@ -821,19 +821,44 @@ static int mv_xor_memcpy_self_test(struct mv_xor_chan *mv_chan) src_dma = dma_map_page(dma_chan->device->dev, virt_to_page(src), 0, PAGE_SIZE, DMA_TO_DEVICE); - unmap->to_cnt = 1; unmap->addr[0] = src_dma; + ret = dma_mapping_error(dma_chan->device->dev, src_dma); + if (ret) { + err = -ENOMEM; + goto free_resources; + } + unmap->to_cnt = 1; + dest_dma = dma_map_page(dma_chan->device->dev, virt_to_page(dest), 0, PAGE_SIZE, DMA_FROM_DEVICE); - unmap->from_cnt = 1; unmap->addr[1] = dest_dma; + ret = dma_mapping_error(dma_chan->device->dev, dest_dma); + if (ret) { + err = -ENOMEM; + goto free_resources; + } + unmap->from_cnt = 1; unmap->len = PAGE_SIZE; tx = mv_xor_prep_dma_memcpy(dma_chan, dest_dma, src_dma, PAGE_SIZE, 0); + if (!tx) { + dev_err(dma_chan->device->dev, + "Self-test cannot prepare operation, disabling\n"); + err = -ENODEV; + goto free_resources; + } + cookie = mv_xor_tx_submit(tx); + if (dma_submit_error(cookie)) { + dev_err(dma_chan->device->dev, + "Self-test submit error, disabling\n"); + err = -ENODEV; + goto free_resources; + } + mv_xor_issue_pending(dma_chan); async_tx_ack(tx); msleep(1); @@ -868,7 +893,7 @@ out: static int mv_xor_xor_self_test(struct mv_xor_chan *mv_chan) { - int i, src_idx; + int i, src_idx, ret; struct page *dest; struct page *xor_srcs[MV_XOR_NUM_SRC_TEST]; dma_addr_t dma_srcs[MV_XOR_NUM_SRC_TEST]; @@ -931,19 +956,42 @@ mv_xor_xor_self_test(struct mv_xor_chan *mv_chan) unmap->addr[i] = dma_map_page(dma_chan->device->dev, xor_srcs[i], 0, PAGE_SIZE, DMA_TO_DEVICE); dma_srcs[i] = unmap->addr[i]; + ret = dma_mapping_error(dma_chan->device->dev, unmap->addr[i]); + if (ret) { + err = -ENOMEM; + goto free_resources; + } unmap->to_cnt++; } unmap->addr[src_count] = dma_map_page(dma_chan->device->dev, dest, 0, PAGE_SIZE, DMA_FROM_DEVICE); dest_dma = unmap->addr[src_count]; + ret = dma_mapping_error(dma_chan->device->dev, unmap->addr[src_count]); + if (ret) { + err = -ENOMEM; + goto free_resources; + } unmap->from_cnt = 1; unmap->len = PAGE_SIZE; tx = mv_xor_prep_dma_xor(dma_chan, dest_dma, dma_srcs, src_count, PAGE_SIZE, 0); + if (!tx) { + dev_err(dma_chan->device->dev, + "Self-test cannot prepare operation, disabling\n"); + err = -ENODEV; + goto free_resources; + } cookie = mv_xor_tx_submit(tx); + if (dma_submit_error(cookie)) { + dev_err(dma_chan->device->dev, + "Self-test submit error, disabling\n"); + err = -ENODEV; + goto free_resources; + } + mv_xor_issue_pending(dma_chan); async_tx_ack(tx); msleep(8);
This commit adds proper error checking for various DMA API calls, as reported by DMA_API_DEBUG=y. Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> --- drivers/dma/mv_xor.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 52 insertions(+), 4 deletions(-)