* [PATCH 0/8] Various dmaengine cleanups @ 2016-06-07 17:38 Peter Griffin 2016-06-07 17:38 ` [PATCH 1/8] dmaengine: fsl-edma: Fix clock handling error paths Peter Griffin ` (8 more replies) 0 siblings, 9 replies; 20+ messages in thread From: Peter Griffin @ 2016-06-07 17:38 UTC (permalink / raw) To: linux-arm-kernel Hi Vinod, This series is a bunch of cleanup updates to various dmaengine drivers, based on some of the review feeback to my fdma series. regards, Peter. Peter Griffin (8): dmaengine: fsl-edma: Fix clock handling error paths dmaengine: fsl-edma: print error code in error messages. dmaengine: coh901318: Only calculate residue if txstate exists. dmaengine: s3c24xx: Simplify code in s3c24xx_dma_tx_status() dmaengine: ste_dma40: Only calculate residue if txstate exists. dmaengine: sun6i-dma: Only calculate residue if state exists. dmaengine: tegra20-apb-dma: Only calculate residue if txstate exists. dmaengine: Remove site specific OOM error messages on kzalloc drivers/dma/amba-pl08x.c | 10 +--------- drivers/dma/bestcomm/bestcomm.c | 2 -- drivers/dma/coh901318.c | 2 +- drivers/dma/edma.c | 16 ++++------------ drivers/dma/fsl-edma.c | 25 +++++++++++++++++++------ drivers/dma/fsldma.c | 2 -- drivers/dma/k3dma.c | 10 ++++------ drivers/dma/mmp_tdma.c | 5 ++--- drivers/dma/moxart-dma.c | 4 +--- drivers/dma/nbpfaxi.c | 5 ++--- drivers/dma/pl330.c | 5 +---- drivers/dma/ppc4xx/adma.c | 2 -- drivers/dma/s3c24xx-dma.c | 11 ++--------- drivers/dma/sh/shdmac.c | 9 ++------- drivers/dma/sh/sudmac.c | 9 ++------- drivers/dma/sirf-dma.c | 5 ++--- drivers/dma/ste_dma40.c | 6 ++---- drivers/dma/sun6i-dma.c | 2 +- drivers/dma/tegra20-apb-dma.c | 13 ++++--------- drivers/dma/timb_dma.c | 8 ++------ 20 files changed, 52 insertions(+), 99 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/8] dmaengine: fsl-edma: Fix clock handling error paths 2016-06-07 17:38 [PATCH 0/8] Various dmaengine cleanups Peter Griffin @ 2016-06-07 17:38 ` Peter Griffin 2016-06-07 17:38 ` [PATCH 2/8] dmaengine: fsl-edma: print error code in error messages Peter Griffin ` (7 subsequent siblings) 8 siblings, 0 replies; 20+ messages in thread From: Peter Griffin @ 2016-06-07 17:38 UTC (permalink / raw) To: linux-arm-kernel Currently fsl-edma doesn't clk_disable_unprepare() its clocks on error conditions. This patch adds a fsl_disable_clocks helper for this, and also only disables clocks which were enabled if encountering an error whilst enabling clocks. Signed-off-by: Peter Griffin <peter.griffin@linaro.org> --- drivers/dma/fsl-edma.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/drivers/dma/fsl-edma.c b/drivers/dma/fsl-edma.c index be2e62b..7208fc9 100644 --- a/drivers/dma/fsl-edma.c +++ b/drivers/dma/fsl-edma.c @@ -852,6 +852,14 @@ fsl_edma_irq_init(struct platform_device *pdev, struct fsl_edma_engine *fsl_edma return 0; } +static void fsl_disable_clocks(struct fsl_edma_engine *fsl_edma) +{ + int i; + + for (i = 0; i < DMAMUX_NR; i++) + clk_disable_unprepare(fsl_edma->muxclk[i]); +} + static int fsl_edma_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node; @@ -897,6 +905,10 @@ static int fsl_edma_probe(struct platform_device *pdev) ret = clk_prepare_enable(fsl_edma->muxclk[i]); if (ret) { + /* disable only clks which were enabled on error */ + for (; i >= 0; i--) + clk_disable_unprepare(fsl_edma->muxclk[i]); + dev_err(&pdev->dev, "DMAMUX clk block failed.\n"); return ret; } @@ -952,6 +964,7 @@ static int fsl_edma_probe(struct platform_device *pdev) ret = dma_async_device_register(&fsl_edma->dma_dev); if (ret) { dev_err(&pdev->dev, "Can't register Freescale eDMA engine.\n"); + fsl_disable_clocks(fsl_edma); return ret; } @@ -959,6 +972,7 @@ static int fsl_edma_probe(struct platform_device *pdev) if (ret) { dev_err(&pdev->dev, "Can't register Freescale eDMA of_dma.\n"); dma_async_device_unregister(&fsl_edma->dma_dev); + fsl_disable_clocks(fsl_edma); return ret; } @@ -972,13 +986,10 @@ static int fsl_edma_remove(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node; struct fsl_edma_engine *fsl_edma = platform_get_drvdata(pdev); - int i; of_dma_controller_free(np); dma_async_device_unregister(&fsl_edma->dma_dev); - - for (i = 0; i < DMAMUX_NR; i++) - clk_disable_unprepare(fsl_edma->muxclk[i]); + fsl_disable_clocks(fsl_edma); return 0; } -- 1.9.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/8] dmaengine: fsl-edma: print error code in error messages. 2016-06-07 17:38 [PATCH 0/8] Various dmaengine cleanups Peter Griffin 2016-06-07 17:38 ` [PATCH 1/8] dmaengine: fsl-edma: Fix clock handling error paths Peter Griffin @ 2016-06-07 17:38 ` Peter Griffin 2016-06-07 17:38 ` [PATCH 3/8] dmaengine: coh901318: Only calculate residue if txstate exists Peter Griffin ` (6 subsequent siblings) 8 siblings, 0 replies; 20+ messages in thread From: Peter Griffin @ 2016-06-07 17:38 UTC (permalink / raw) To: linux-arm-kernel It is useful to print the error code as part of the error message. Signed-off-by: Peter Griffin <peter.griffin@linaro.org> --- drivers/dma/fsl-edma.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/dma/fsl-edma.c b/drivers/dma/fsl-edma.c index 7208fc9..cc06eea 100644 --- a/drivers/dma/fsl-edma.c +++ b/drivers/dma/fsl-edma.c @@ -963,14 +963,16 @@ static int fsl_edma_probe(struct platform_device *pdev) ret = dma_async_device_register(&fsl_edma->dma_dev); if (ret) { - dev_err(&pdev->dev, "Can't register Freescale eDMA engine.\n"); + dev_err(&pdev->dev, + "Can't register Freescale eDMA engine. (%d)\n", ret); fsl_disable_clocks(fsl_edma); return ret; } ret = of_dma_controller_register(np, fsl_edma_xlate, fsl_edma); if (ret) { - dev_err(&pdev->dev, "Can't register Freescale eDMA of_dma.\n"); + dev_err(&pdev->dev, + "Can't register Freescale eDMA of_dma. (%d)\n", ret); dma_async_device_unregister(&fsl_edma->dma_dev); fsl_disable_clocks(fsl_edma); return ret; -- 1.9.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/8] dmaengine: coh901318: Only calculate residue if txstate exists. 2016-06-07 17:38 [PATCH 0/8] Various dmaengine cleanups Peter Griffin 2016-06-07 17:38 ` [PATCH 1/8] dmaengine: fsl-edma: Fix clock handling error paths Peter Griffin 2016-06-07 17:38 ` [PATCH 2/8] dmaengine: fsl-edma: print error code in error messages Peter Griffin @ 2016-06-07 17:38 ` Peter Griffin 2016-06-08 12:15 ` Linus Walleij 2016-06-07 17:38 ` [PATCH 4/8] dmaengine: s3c24xx: Simplify code in s3c24xx_dma_tx_status() Peter Griffin ` (5 subsequent siblings) 8 siblings, 1 reply; 20+ messages in thread From: Peter Griffin @ 2016-06-07 17:38 UTC (permalink / raw) To: linux-arm-kernel There is no point in calculating the residue if there is no txstate to store the value. Signed-off-by: Peter Griffin <peter.griffin@linaro.org> --- drivers/dma/coh901318.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma/coh901318.c b/drivers/dma/coh901318.c index c340ca9..c100616 100644 --- a/drivers/dma/coh901318.c +++ b/drivers/dma/coh901318.c @@ -2422,7 +2422,7 @@ coh901318_tx_status(struct dma_chan *chan, dma_cookie_t cookie, enum dma_status ret; ret = dma_cookie_status(chan, cookie, txstate); - if (ret == DMA_COMPLETE) + if (ret == DMA_COMPLETE || !txstate) return ret; dma_set_residue(txstate, coh901318_get_bytes_left(chan)); -- 1.9.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/8] dmaengine: coh901318: Only calculate residue if txstate exists. 2016-06-07 17:38 ` [PATCH 3/8] dmaengine: coh901318: Only calculate residue if txstate exists Peter Griffin @ 2016-06-08 12:15 ` Linus Walleij 0 siblings, 0 replies; 20+ messages in thread From: Linus Walleij @ 2016-06-08 12:15 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jun 7, 2016 at 7:38 PM, Peter Griffin <peter.griffin@linaro.org> wrote: > There is no point in calculating the residue if there is no > txstate to store the value. > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> Acked-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 4/8] dmaengine: s3c24xx: Simplify code in s3c24xx_dma_tx_status() 2016-06-07 17:38 [PATCH 0/8] Various dmaengine cleanups Peter Griffin ` (2 preceding siblings ...) 2016-06-07 17:38 ` [PATCH 3/8] dmaengine: coh901318: Only calculate residue if txstate exists Peter Griffin @ 2016-06-07 17:38 ` Peter Griffin 2016-06-07 17:38 ` [PATCH 5/8] dmaengine: ste_dma40: Only calculate residue if txstate exists Peter Griffin ` (4 subsequent siblings) 8 siblings, 0 replies; 20+ messages in thread From: Peter Griffin @ 2016-06-07 17:38 UTC (permalink / raw) To: linux-arm-kernel Doing so saves a few lines of code in the driver. Signed-off-by: Peter Griffin <peter.griffin@linaro.org> --- drivers/dma/s3c24xx-dma.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/dma/s3c24xx-dma.c b/drivers/dma/s3c24xx-dma.c index 17ccdfd..f7d2c7a 100644 --- a/drivers/dma/s3c24xx-dma.c +++ b/drivers/dma/s3c24xx-dma.c @@ -768,16 +768,12 @@ static enum dma_status s3c24xx_dma_tx_status(struct dma_chan *chan, spin_lock_irqsave(&s3cchan->vc.lock, flags); ret = dma_cookie_status(chan, cookie, txstate); - if (ret == DMA_COMPLETE) { - spin_unlock_irqrestore(&s3cchan->vc.lock, flags); - return ret; - } /* * There's no point calculating the residue if there's * no txstate to store the value. */ - if (!txstate) { + if (ret == DMA_COMPLETE || !txstate) { spin_unlock_irqrestore(&s3cchan->vc.lock, flags); return ret; } -- 1.9.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 5/8] dmaengine: ste_dma40: Only calculate residue if txstate exists. 2016-06-07 17:38 [PATCH 0/8] Various dmaengine cleanups Peter Griffin ` (3 preceding siblings ...) 2016-06-07 17:38 ` [PATCH 4/8] dmaengine: s3c24xx: Simplify code in s3c24xx_dma_tx_status() Peter Griffin @ 2016-06-07 17:38 ` Peter Griffin 2016-06-08 12:15 ` Linus Walleij 2016-06-07 17:38 ` [PATCH 6/8] dmaengine: sun6i-dma: Only calculate residue if state exists Peter Griffin ` (3 subsequent siblings) 8 siblings, 1 reply; 20+ messages in thread From: Peter Griffin @ 2016-06-07 17:38 UTC (permalink / raw) To: linux-arm-kernel There is no point calculating the residue if there is no txstate to store the value. Signed-off-by: Peter Griffin <peter.griffin@linaro.org> --- drivers/dma/ste_dma40.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c index 6fb8307..378cc47 100644 --- a/drivers/dma/ste_dma40.c +++ b/drivers/dma/ste_dma40.c @@ -2588,7 +2588,7 @@ static enum dma_status d40_tx_status(struct dma_chan *chan, } ret = dma_cookie_status(chan, cookie, txstate); - if (ret != DMA_COMPLETE) + if (ret != DMA_COMPLETE && txstate) dma_set_residue(txstate, stedma40_residue(chan)); if (d40_is_paused(d40c)) -- 1.9.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 5/8] dmaengine: ste_dma40: Only calculate residue if txstate exists. 2016-06-07 17:38 ` [PATCH 5/8] dmaengine: ste_dma40: Only calculate residue if txstate exists Peter Griffin @ 2016-06-08 12:15 ` Linus Walleij 0 siblings, 0 replies; 20+ messages in thread From: Linus Walleij @ 2016-06-08 12:15 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jun 7, 2016 at 7:38 PM, Peter Griffin <peter.griffin@linaro.org> wrote: > There is no point calculating the residue if there is > no txstate to store the value. > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> Acked-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 6/8] dmaengine: sun6i-dma: Only calculate residue if state exists. 2016-06-07 17:38 [PATCH 0/8] Various dmaengine cleanups Peter Griffin ` (4 preceding siblings ...) 2016-06-07 17:38 ` [PATCH 5/8] dmaengine: ste_dma40: Only calculate residue if txstate exists Peter Griffin @ 2016-06-07 17:38 ` Peter Griffin 2016-06-07 17:38 ` [PATCH 7/8] dmaengine: tegra20-apb-dma: Only calculate residue if txstate exists Peter Griffin ` (2 subsequent siblings) 8 siblings, 0 replies; 20+ messages in thread From: Peter Griffin @ 2016-06-07 17:38 UTC (permalink / raw) To: linux-arm-kernel There is no point in calculating the residue if state does not exist to store the value. Signed-off-by: Peter Griffin <peter.griffin@linaro.org> --- drivers/dma/sun6i-dma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c index 5065ca4..3835fcd 100644 --- a/drivers/dma/sun6i-dma.c +++ b/drivers/dma/sun6i-dma.c @@ -865,7 +865,7 @@ static enum dma_status sun6i_dma_tx_status(struct dma_chan *chan, size_t bytes = 0; ret = dma_cookie_status(chan, cookie, state); - if (ret == DMA_COMPLETE) + if (ret == DMA_COMPLETE || !state) return ret; spin_lock_irqsave(&vchan->vc.lock, flags); -- 1.9.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 7/8] dmaengine: tegra20-apb-dma: Only calculate residue if txstate exists. 2016-06-07 17:38 [PATCH 0/8] Various dmaengine cleanups Peter Griffin ` (5 preceding siblings ...) 2016-06-07 17:38 ` [PATCH 6/8] dmaengine: sun6i-dma: Only calculate residue if state exists Peter Griffin @ 2016-06-07 17:38 ` Peter Griffin 2016-06-08 8:51 ` Jon Hunter 2016-06-07 17:38 ` [PATCH 8/8] dmaengine: Remove site specific OOM error messages on kzalloc Peter Griffin 2016-06-21 16:05 ` [PATCH 0/8] Various dmaengine cleanups Vinod Koul 8 siblings, 1 reply; 20+ messages in thread From: Peter Griffin @ 2016-06-07 17:38 UTC (permalink / raw) To: linux-arm-kernel There is no point calculating the residue if there is no txstate to store the value. Signed-off-by: Peter Griffin <peter.griffin@linaro.org> --- drivers/dma/tegra20-apb-dma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c index 01e316f..7f4af8c 100644 --- a/drivers/dma/tegra20-apb-dma.c +++ b/drivers/dma/tegra20-apb-dma.c @@ -814,7 +814,7 @@ static enum dma_status tegra_dma_tx_status(struct dma_chan *dc, unsigned int residual; ret = dma_cookie_status(dc, cookie, txstate); - if (ret == DMA_COMPLETE) + if (ret == DMA_COMPLETE || !txstate) return ret; spin_lock_irqsave(&tdc->lock, flags); -- 1.9.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 7/8] dmaengine: tegra20-apb-dma: Only calculate residue if txstate exists. 2016-06-07 17:38 ` [PATCH 7/8] dmaengine: tegra20-apb-dma: Only calculate residue if txstate exists Peter Griffin @ 2016-06-08 8:51 ` Jon Hunter 2016-06-21 16:01 ` Vinod Koul 0 siblings, 1 reply; 20+ messages in thread From: Jon Hunter @ 2016-06-08 8:51 UTC (permalink / raw) To: linux-arm-kernel Hi Peter, On 07/06/16 18:38, Peter Griffin wrote: > There is no point calculating the residue if there is > no txstate to store the value. > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > --- > drivers/dma/tegra20-apb-dma.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c > index 01e316f..7f4af8c 100644 > --- a/drivers/dma/tegra20-apb-dma.c > +++ b/drivers/dma/tegra20-apb-dma.c > @@ -814,7 +814,7 @@ static enum dma_status tegra_dma_tx_status(struct dma_chan *dc, > unsigned int residual; > > ret = dma_cookie_status(dc, cookie, txstate); > - if (ret == DMA_COMPLETE) > + if (ret == DMA_COMPLETE || !txstate) > return ret; Thanks for reporting this. I agree that we should not do this, however, looking at the code for Tegra, I am wondering if this could change the actual state that is returned. Looking at dma_cookie_status() it will call dma_async_is_complete() which will return either DMA_COMPLETE or DMA_IN_PROGRESS. It could be possible that the actual state for the DMA transfer in the tegra driver is DMA_ERROR, so I am wondering if we should do something like the following ... diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c index 01e316f73559..45edab7418d0 100644 --- a/drivers/dma/tegra20-apb-dma.c +++ b/drivers/dma/tegra20-apb-dma.c @@ -822,13 +822,8 @@ static enum dma_status tegra_dma_tx_status(struct dma_chan *dc, /* Check on wait_ack desc status */ list_for_each_entry(dma_desc, &tdc->free_dma_desc, node) { if (dma_desc->txd.cookie == cookie) { - residual = dma_desc->bytes_requested - - (dma_desc->bytes_transferred % - dma_desc->bytes_requested); - dma_set_residue(txstate, residual); ret = dma_desc->dma_status; - spin_unlock_irqrestore(&tdc->lock, flags); - return ret; + goto found; } } @@ -836,17 +831,23 @@ static enum dma_status tegra_dma_tx_status(struct dma_chan *dc, list_for_each_entry(sg_req, &tdc->pending_sg_req, node) { dma_desc = sg_req->dma_desc; if (dma_desc->txd.cookie == cookie) { - residual = dma_desc->bytes_requested - - (dma_desc->bytes_transferred % - dma_desc->bytes_requested); - dma_set_residue(txstate, residual); ret = dma_desc->dma_status; - spin_unlock_irqrestore(&tdc->lock, flags); - return ret; + goto found; } } - dev_dbg(tdc2dev(tdc), "cookie %d does not found\n", cookie); + dev_warn(tdc2dev(tdc), "cookie %d not found\n", cookie); + spin_unlock_irqrestore(&tdc->lock, flags); + return ret; + +found: + if (txstate) { + residual = dma_desc->bytes_requested - + (dma_desc->bytes_transferred % + dma_desc->bytes_requested); + dma_set_residue(txstate, residual); + } + spin_unlock_irqrestore(&tdc->lock, flags); return ret; } Cheers Jon -- nvpublic ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 7/8] dmaengine: tegra20-apb-dma: Only calculate residue if txstate exists. 2016-06-08 8:51 ` Jon Hunter @ 2016-06-21 16:01 ` Vinod Koul 2016-06-21 17:19 ` Jon Hunter 0 siblings, 1 reply; 20+ messages in thread From: Vinod Koul @ 2016-06-21 16:01 UTC (permalink / raw) To: linux-arm-kernel On Wed, Jun 08, 2016 at 09:51:57AM +0100, Jon Hunter wrote: > Hi Peter, > > On 07/06/16 18:38, Peter Griffin wrote: > > There is no point calculating the residue if there is > > no txstate to store the value. > > > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > > --- > > drivers/dma/tegra20-apb-dma.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c > > index 01e316f..7f4af8c 100644 > > --- a/drivers/dma/tegra20-apb-dma.c > > +++ b/drivers/dma/tegra20-apb-dma.c > > @@ -814,7 +814,7 @@ static enum dma_status tegra_dma_tx_status(struct dma_chan *dc, > > unsigned int residual; > > > > ret = dma_cookie_status(dc, cookie, txstate); > > - if (ret == DMA_COMPLETE) > > + if (ret == DMA_COMPLETE || !txstate) > > return ret; > > Thanks for reporting this. I agree that we should not do this, however, > looking at the code for Tegra, I am wondering if this could change the > actual state that is returned. Looking at dma_cookie_status() it will > call dma_async_is_complete() which will return either DMA_COMPLETE or > DMA_IN_PROGRESS. It could be possible that the actual state for the > DMA transfer in the tegra driver is DMA_ERROR, so I am wondering if we > should do something like the following ... This one is stopping code execution when residue is not valid. Do notice that it check for DMA_COMPLETE OR txstate. In other cases, wit will return 'that' state when txstate is NULL. I am going to apply this. > > diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c > index 01e316f73559..45edab7418d0 100644 > --- a/drivers/dma/tegra20-apb-dma.c > +++ b/drivers/dma/tegra20-apb-dma.c > @@ -822,13 +822,8 @@ static enum dma_status tegra_dma_tx_status(struct dma_chan *dc, > /* Check on wait_ack desc status */ > list_for_each_entry(dma_desc, &tdc->free_dma_desc, node) { > if (dma_desc->txd.cookie == cookie) { > - residual = dma_desc->bytes_requested - > - (dma_desc->bytes_transferred % > - dma_desc->bytes_requested); > - dma_set_residue(txstate, residual); > ret = dma_desc->dma_status; > - spin_unlock_irqrestore(&tdc->lock, flags); > - return ret; > + goto found; > } > } > > @@ -836,17 +831,23 @@ static enum dma_status tegra_dma_tx_status(struct dma_chan *dc, > list_for_each_entry(sg_req, &tdc->pending_sg_req, node) { > dma_desc = sg_req->dma_desc; > if (dma_desc->txd.cookie == cookie) { > - residual = dma_desc->bytes_requested - > - (dma_desc->bytes_transferred % > - dma_desc->bytes_requested); > - dma_set_residue(txstate, residual); > ret = dma_desc->dma_status; > - spin_unlock_irqrestore(&tdc->lock, flags); > - return ret; > + goto found; > } > } > > - dev_dbg(tdc2dev(tdc), "cookie %d does not found\n", cookie); > + dev_warn(tdc2dev(tdc), "cookie %d not found\n", cookie); > + spin_unlock_irqrestore(&tdc->lock, flags); > + return ret; > + > +found: > + if (txstate) { > + residual = dma_desc->bytes_requested - > + (dma_desc->bytes_transferred % > + dma_desc->bytes_requested); > + dma_set_residue(txstate, residual); > + } > + I feel this optimizes stuff, which seems okay. Feel free to send as proper patch. > spin_unlock_irqrestore(&tdc->lock, flags); > return ret; > } > > Cheers > Jon > > -- > nvpublic -- ~Vinod ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 7/8] dmaengine: tegra20-apb-dma: Only calculate residue if txstate exists. 2016-06-21 16:01 ` Vinod Koul @ 2016-06-21 17:19 ` Jon Hunter 2016-06-28 4:04 ` Vinod Koul 0 siblings, 1 reply; 20+ messages in thread From: Jon Hunter @ 2016-06-21 17:19 UTC (permalink / raw) To: linux-arm-kernel On 21/06/16 17:01, Vinod Koul wrote: > On Wed, Jun 08, 2016 at 09:51:57AM +0100, Jon Hunter wrote: >> Hi Peter, >> >> On 07/06/16 18:38, Peter Griffin wrote: >>> There is no point calculating the residue if there is >>> no txstate to store the value. >>> >>> Signed-off-by: Peter Griffin <peter.griffin@linaro.org> >>> --- >>> drivers/dma/tegra20-apb-dma.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c >>> index 01e316f..7f4af8c 100644 >>> --- a/drivers/dma/tegra20-apb-dma.c >>> +++ b/drivers/dma/tegra20-apb-dma.c >>> @@ -814,7 +814,7 @@ static enum dma_status tegra_dma_tx_status(struct dma_chan *dc, >>> unsigned int residual; >>> >>> ret = dma_cookie_status(dc, cookie, txstate); >>> - if (ret == DMA_COMPLETE) >>> + if (ret == DMA_COMPLETE || !txstate) >>> return ret; >> >> Thanks for reporting this. I agree that we should not do this, however, >> looking at the code for Tegra, I am wondering if this could change the >> actual state that is returned. Looking at dma_cookie_status() it will >> call dma_async_is_complete() which will return either DMA_COMPLETE or >> DMA_IN_PROGRESS. It could be possible that the actual state for the >> DMA transfer in the tegra driver is DMA_ERROR, so I am wondering if we >> should do something like the following ... > > This one is stopping code execution when residue is not valid. Do notice > that it check for DMA_COMPLETE OR txstate. In other cases, wit will return > 'that' state when txstate is NULL. Sorry what do you mean by "this one"? My point is that if the status is not DMA_COMPLETE, then it is possible that it could be DMA_ERROR (for tegra that is). However, dma_cookie_status will only return DMA_IN_PROGRESS or DMA_COMPLETE and so if 'txstate' is NULL we will not see the DMA_ERROR status anymore and just think it is in progress when it is actually an error. I do agree that the driver is broken as we are not checking for !txstate, but this also changes the behaviour a bit. Cheers Jon -- nvpublic ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 7/8] dmaengine: tegra20-apb-dma: Only calculate residue if txstate exists. 2016-06-21 17:19 ` Jon Hunter @ 2016-06-28 4:04 ` Vinod Koul 0 siblings, 0 replies; 20+ messages in thread From: Vinod Koul @ 2016-06-28 4:04 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jun 21, 2016 at 06:19:50PM +0100, Jon Hunter wrote: > > On 21/06/16 17:01, Vinod Koul wrote: > > On Wed, Jun 08, 2016 at 09:51:57AM +0100, Jon Hunter wrote: > >> Hi Peter, > >> > >> On 07/06/16 18:38, Peter Griffin wrote: > >>> There is no point calculating the residue if there is > >>> no txstate to store the value. > >>> > >>> Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > >>> --- > >>> drivers/dma/tegra20-apb-dma.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c > >>> index 01e316f..7f4af8c 100644 > >>> --- a/drivers/dma/tegra20-apb-dma.c > >>> +++ b/drivers/dma/tegra20-apb-dma.c > >>> @@ -814,7 +814,7 @@ static enum dma_status tegra_dma_tx_status(struct dma_chan *dc, > >>> unsigned int residual; > >>> > >>> ret = dma_cookie_status(dc, cookie, txstate); > >>> - if (ret == DMA_COMPLETE) > >>> + if (ret == DMA_COMPLETE || !txstate) > >>> return ret; > >> > >> Thanks for reporting this. I agree that we should not do this, however, > >> looking at the code for Tegra, I am wondering if this could change the > >> actual state that is returned. Looking at dma_cookie_status() it will > >> call dma_async_is_complete() which will return either DMA_COMPLETE or > >> DMA_IN_PROGRESS. It could be possible that the actual state for the > >> DMA transfer in the tegra driver is DMA_ERROR, so I am wondering if we > >> should do something like the following ... > > > > This one is stopping code execution when residue is not valid. Do notice > > that it check for DMA_COMPLETE OR txstate. In other cases, wit will return > > 'that' state when txstate is NULL. > > Sorry what do you mean by "this one"? the patch > > My point is that if the status is not DMA_COMPLETE, then it is possible > that it could be DMA_ERROR (for tegra that is). right it can be any state... and we check for only DMA_COMPLETE as residue has no meaning. > However, > dma_cookie_status will only return DMA_IN_PROGRESS or DMA_COMPLETE and > so if 'txstate' is NULL we will not see the DMA_ERROR status anymore and > just think it is in progress when it is actually an error. Yes that is an existing issue, if you are reporting DMA_ERROR then you should add a check for DMA_ERROR as well and not do residue calculation for that case. > > I do agree that the driver is broken as we are not checking for > !txstate, but this also changes the behaviour a bit. It changes for better and not worse. Btw pls feel free to send a patch handling DMA_ERROR :) -- ~Vinod ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 8/8] dmaengine: Remove site specific OOM error messages on kzalloc 2016-06-07 17:38 [PATCH 0/8] Various dmaengine cleanups Peter Griffin ` (6 preceding siblings ...) 2016-06-07 17:38 ` [PATCH 7/8] dmaengine: tegra20-apb-dma: Only calculate residue if txstate exists Peter Griffin @ 2016-06-07 17:38 ` Peter Griffin 2016-06-08 8:22 ` Jon Hunter 2016-06-08 12:12 ` Linus Walleij 2016-06-21 16:05 ` [PATCH 0/8] Various dmaengine cleanups Vinod Koul 8 siblings, 2 replies; 20+ messages in thread From: Peter Griffin @ 2016-06-07 17:38 UTC (permalink / raw) To: linux-arm-kernel If kzalloc() fails it will issue it's own error message including a dump_stack(). So remove the site specific error messages. Signed-off-by: Peter Griffin <peter.griffin@linaro.org> --- drivers/dma/amba-pl08x.c | 10 +--------- drivers/dma/bestcomm/bestcomm.c | 2 -- drivers/dma/edma.c | 16 ++++------------ drivers/dma/fsldma.c | 2 -- drivers/dma/k3dma.c | 10 ++++------ drivers/dma/mmp_tdma.c | 5 ++--- drivers/dma/moxart-dma.c | 4 +--- drivers/dma/nbpfaxi.c | 5 ++--- drivers/dma/pl330.c | 5 +---- drivers/dma/ppc4xx/adma.c | 2 -- drivers/dma/s3c24xx-dma.c | 5 +---- drivers/dma/sh/shdmac.c | 9 ++------- drivers/dma/sh/sudmac.c | 9 ++------- drivers/dma/sirf-dma.c | 5 ++--- drivers/dma/ste_dma40.c | 4 +--- drivers/dma/tegra20-apb-dma.c | 11 +++-------- drivers/dma/timb_dma.c | 8 ++------ 17 files changed, 28 insertions(+), 84 deletions(-) diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c index 81db1c4..939a7c3 100644 --- a/drivers/dma/amba-pl08x.c +++ b/drivers/dma/amba-pl08x.c @@ -1443,8 +1443,6 @@ static struct dma_async_tx_descriptor *pl08x_prep_dma_memcpy( dsg = kzalloc(sizeof(struct pl08x_sg), GFP_NOWAIT); if (!dsg) { pl08x_free_txd(pl08x, txd); - dev_err(&pl08x->adev->dev, "%s no memory for pl080 sg\n", - __func__); return NULL; } list_add_tail(&dsg->node, &txd->dsg_list); @@ -1901,11 +1899,8 @@ static int pl08x_dma_init_virtual_channels(struct pl08x_driver_data *pl08x, */ for (i = 0; i < channels; i++) { chan = kzalloc(sizeof(*chan), GFP_KERNEL); - if (!chan) { - dev_err(&pl08x->adev->dev, - "%s no memory for channel\n", __func__); + if (!chan) return -ENOMEM; - } chan->host = pl08x; chan->state = PL08X_CHAN_IDLE; @@ -2360,9 +2355,6 @@ static int pl08x_probe(struct amba_device *adev, const struct amba_id *id) pl08x->phy_chans = kzalloc((vd->channels * sizeof(*pl08x->phy_chans)), GFP_KERNEL); if (!pl08x->phy_chans) { - dev_err(&adev->dev, "%s failed to allocate " - "physical channel holders\n", - __func__); ret = -ENOMEM; goto out_no_phychans; } diff --git a/drivers/dma/bestcomm/bestcomm.c b/drivers/dma/bestcomm/bestcomm.c index 180fedb..7ce8437 100644 --- a/drivers/dma/bestcomm/bestcomm.c +++ b/drivers/dma/bestcomm/bestcomm.c @@ -397,8 +397,6 @@ static int mpc52xx_bcom_probe(struct platform_device *op) /* Get a clean struct */ bcom_eng = kzalloc(sizeof(struct bcom_engine), GFP_KERNEL); if (!bcom_eng) { - printk(KERN_ERR DRIVER_NAME ": " - "Can't allocate state structure\n"); rv = -ENOMEM; goto error_sramclean; } diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c index 8181ed1..3c84cd8 100644 --- a/drivers/dma/edma.c +++ b/drivers/dma/edma.c @@ -1069,10 +1069,8 @@ static struct dma_async_tx_descriptor *edma_prep_slave_sg( edesc = kzalloc(sizeof(*edesc) + sg_len * sizeof(edesc->pset[0]), GFP_ATOMIC); - if (!edesc) { - dev_err(dev, "%s: Failed to allocate a descriptor\n", __func__); + if (!edesc) return NULL; - } edesc->pset_nr = sg_len; edesc->residue = 0; @@ -1173,10 +1171,8 @@ static struct dma_async_tx_descriptor *edma_prep_dma_memcpy( edesc = kzalloc(sizeof(*edesc) + nslots * sizeof(edesc->pset[0]), GFP_ATOMIC); - if (!edesc) { - dev_dbg(dev, "Failed to allocate a descriptor\n"); + if (!edesc) return NULL; - } edesc->pset_nr = nslots; edesc->residue = edesc->residue_stat = len; @@ -1298,10 +1294,8 @@ static struct dma_async_tx_descriptor *edma_prep_dma_cyclic( edesc = kzalloc(sizeof(*edesc) + nslots * sizeof(edesc->pset[0]), GFP_ATOMIC); - if (!edesc) { - dev_err(dev, "%s: Failed to allocate a descriptor\n", __func__); + if (!edesc) return NULL; - } edesc->cyclic = 1; edesc->pset_nr = nslots; @@ -2207,10 +2201,8 @@ static int edma_probe(struct platform_device *pdev) return ret; ecc = devm_kzalloc(dev, sizeof(*ecc), GFP_KERNEL); - if (!ecc) { - dev_err(dev, "Can't allocate controller\n"); + if (!ecc) return -ENOMEM; - } ecc->dev = dev; ecc->id = pdev->id; diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c index a8828ed..911b717 100644 --- a/drivers/dma/fsldma.c +++ b/drivers/dma/fsldma.c @@ -1234,7 +1234,6 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev, /* alloc channel */ chan = kzalloc(sizeof(*chan), GFP_KERNEL); if (!chan) { - dev_err(fdev->dev, "no free memory for DMA channels!\n"); err = -ENOMEM; goto out_return; } @@ -1340,7 +1339,6 @@ static int fsldma_of_probe(struct platform_device *op) fdev = kzalloc(sizeof(*fdev), GFP_KERNEL); if (!fdev) { - dev_err(&op->dev, "No enough memory for 'priv'\n"); err = -ENOMEM; goto out_return; } diff --git a/drivers/dma/k3dma.c b/drivers/dma/k3dma.c index 1ba2fd7..35961af 100644 --- a/drivers/dma/k3dma.c +++ b/drivers/dma/k3dma.c @@ -425,10 +425,9 @@ static struct dma_async_tx_descriptor *k3_dma_prep_memcpy( num = DIV_ROUND_UP(len, DMA_MAX_SIZE); ds = kzalloc(sizeof(*ds) + num * sizeof(ds->desc_hw[0]), GFP_ATOMIC); - if (!ds) { - dev_dbg(chan->device->dev, "vchan %p: kzalloc fail\n", &c->vc); + if (!ds) return NULL; - } + ds->desc_hw_lli = __virt_to_phys((unsigned long)&ds->desc_hw[0]); ds->size = len; ds->desc_num = num; @@ -481,10 +480,9 @@ static struct dma_async_tx_descriptor *k3_dma_prep_slave_sg( } ds = kzalloc(sizeof(*ds) + num * sizeof(ds->desc_hw[0]), GFP_ATOMIC); - if (!ds) { - dev_dbg(chan->device->dev, "vchan %p: kzalloc fail\n", &c->vc); + if (!ds) return NULL; - } + ds->desc_hw_lli = __virt_to_phys((unsigned long)&ds->desc_hw[0]); ds->desc_num = num; num = 0; diff --git a/drivers/dma/mmp_tdma.c b/drivers/dma/mmp_tdma.c index 3df0422..ba7f412 100644 --- a/drivers/dma/mmp_tdma.c +++ b/drivers/dma/mmp_tdma.c @@ -551,10 +551,9 @@ static int mmp_tdma_chan_init(struct mmp_tdma_device *tdev, /* alloc channel */ tdmac = devm_kzalloc(tdev->dev, sizeof(*tdmac), GFP_KERNEL); - if (!tdmac) { - dev_err(tdev->dev, "no free memory for DMA channels!\n"); + if (!tdmac) return -ENOMEM; - } + if (irq) tdmac->irq = irq; tdmac->dev = tdev->dev; diff --git a/drivers/dma/moxart-dma.c b/drivers/dma/moxart-dma.c index 631c443..b3a1d9a 100644 --- a/drivers/dma/moxart-dma.c +++ b/drivers/dma/moxart-dma.c @@ -574,10 +574,8 @@ static int moxart_probe(struct platform_device *pdev) struct moxart_dmadev *mdc; mdc = devm_kzalloc(dev, sizeof(*mdc), GFP_KERNEL); - if (!mdc) { - dev_err(dev, "can't allocate DMA container\n"); + if (!mdc) return -ENOMEM; - } irq = irq_of_parse_and_map(node, 0); if (irq == NO_IRQ) { diff --git a/drivers/dma/nbpfaxi.c b/drivers/dma/nbpfaxi.c index 2b5a198..9f0e98b 100644 --- a/drivers/dma/nbpfaxi.c +++ b/drivers/dma/nbpfaxi.c @@ -1300,10 +1300,9 @@ static int nbpf_probe(struct platform_device *pdev) nbpf = devm_kzalloc(dev, sizeof(*nbpf) + num_channels * sizeof(nbpf->chan[0]), GFP_KERNEL); - if (!nbpf) { - dev_err(dev, "Memory allocation failed\n"); + if (!nbpf) return -ENOMEM; - } + dma_dev = &nbpf->dma_dev; dma_dev->dev = dev; diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index 372b435..c8767d3 100644 --- a/drivers/dma/pl330.c +++ b/drivers/dma/pl330.c @@ -2828,10 +2828,8 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) /* Allocate a new DMAC and its Channels */ pl330 = devm_kzalloc(&adev->dev, sizeof(*pl330), GFP_KERNEL); - if (!pl330) { - dev_err(&adev->dev, "unable to allocate mem\n"); + if (!pl330) return -ENOMEM; - } pd = &pl330->ddma; pd->dev = &adev->dev; @@ -2890,7 +2888,6 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) pl330->peripherals = kzalloc(num_chan * sizeof(*pch), GFP_KERNEL); if (!pl330->peripherals) { ret = -ENOMEM; - dev_err(&adev->dev, "unable to allocate pl330->peripherals\n"); goto probe_err2; } diff --git a/drivers/dma/ppc4xx/adma.c b/drivers/dma/ppc4xx/adma.c index 9217f89..da3688b 100644 --- a/drivers/dma/ppc4xx/adma.c +++ b/drivers/dma/ppc4xx/adma.c @@ -4084,7 +4084,6 @@ static int ppc440spe_adma_probe(struct platform_device *ofdev) /* create a device */ adev = kzalloc(sizeof(*adev), GFP_KERNEL); if (!adev) { - dev_err(&ofdev->dev, "failed to allocate device\n"); initcode = PPC_ADMA_INIT_ALLOC; ret = -ENOMEM; goto err_adev_alloc; @@ -4145,7 +4144,6 @@ static int ppc440spe_adma_probe(struct platform_device *ofdev) /* create a channel */ chan = kzalloc(sizeof(*chan), GFP_KERNEL); if (!chan) { - dev_err(&ofdev->dev, "can't allocate channel structure\n"); initcode = PPC_ADMA_INIT_CHANNEL; ret = -ENOMEM; goto err_chan_alloc; diff --git a/drivers/dma/s3c24xx-dma.c b/drivers/dma/s3c24xx-dma.c index f7d2c7a..0d2d187 100644 --- a/drivers/dma/s3c24xx-dma.c +++ b/drivers/dma/s3c24xx-dma.c @@ -1101,11 +1101,8 @@ static int s3c24xx_dma_init_virtual_channels(struct s3c24xx_dma_engine *s3cdma, */ for (i = 0; i < channels; i++) { chan = devm_kzalloc(dmadev->dev, sizeof(*chan), GFP_KERNEL); - if (!chan) { - dev_err(dmadev->dev, - "%s no memory for channel\n", __func__); + if (!chan) return -ENOMEM; - } chan->id = i; chan->host = s3cdma; diff --git a/drivers/dma/sh/shdmac.c b/drivers/dma/sh/shdmac.c index 80d8640..c94ffab 100644 --- a/drivers/dma/sh/shdmac.c +++ b/drivers/dma/sh/shdmac.c @@ -532,11 +532,8 @@ static int sh_dmae_chan_probe(struct sh_dmae_device *shdev, int id, sh_chan = devm_kzalloc(sdev->dma_dev.dev, sizeof(struct sh_dmae_chan), GFP_KERNEL); - if (!sh_chan) { - dev_err(sdev->dma_dev.dev, - "No free memory for allocating dma channels!\n"); + if (!sh_chan) return -ENOMEM; - } schan = &sh_chan->shdma_chan; schan->max_xfer_len = SH_DMA_TCR_MAX + 1; @@ -732,10 +729,8 @@ static int sh_dmae_probe(struct platform_device *pdev) shdev = devm_kzalloc(&pdev->dev, sizeof(struct sh_dmae_device), GFP_KERNEL); - if (!shdev) { - dev_err(&pdev->dev, "Not enough memory\n"); + if (!shdev) return -ENOMEM; - } dma_dev = &shdev->shdma_dev.dma_dev; diff --git a/drivers/dma/sh/sudmac.c b/drivers/dma/sh/sudmac.c index 6da2eaa..69b9564 100644 --- a/drivers/dma/sh/sudmac.c +++ b/drivers/dma/sh/sudmac.c @@ -245,11 +245,8 @@ static int sudmac_chan_probe(struct sudmac_device *su_dev, int id, int irq, int err; sc = devm_kzalloc(&pdev->dev, sizeof(struct sudmac_chan), GFP_KERNEL); - if (!sc) { - dev_err(sdev->dma_dev.dev, - "No free memory for allocating dma channels!\n"); + if (!sc) return -ENOMEM; - } schan = &sc->shdma_chan; schan->max_xfer_len = 64 * 1024 * 1024 - 1; @@ -349,10 +346,8 @@ static int sudmac_probe(struct platform_device *pdev) err = -ENOMEM; su_dev = devm_kzalloc(&pdev->dev, sizeof(struct sudmac_device), GFP_KERNEL); - if (!su_dev) { - dev_err(&pdev->dev, "Not enough memory\n"); + if (!su_dev) return err; - } dma_dev = &su_dev->shdma_dev.dma_dev; diff --git a/drivers/dma/sirf-dma.c b/drivers/dma/sirf-dma.c index e48350e..8ea51c7 100644 --- a/drivers/dma/sirf-dma.c +++ b/drivers/dma/sirf-dma.c @@ -854,10 +854,9 @@ static int sirfsoc_dma_probe(struct platform_device *op) int ret, i; sdma = devm_kzalloc(dev, sizeof(*sdma), GFP_KERNEL); - if (!sdma) { - dev_err(dev, "Memory exhausted!\n"); + if (!sdma) return -ENOMEM; - } + data = (struct sirfsoc_dmadata *) (of_match_device(op->dev.driver->of_match_table, &op->dev)->data); diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c index 378cc47..8b18e44 100644 --- a/drivers/dma/ste_dma40.c +++ b/drivers/dma/ste_dma40.c @@ -3237,10 +3237,8 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev) (num_phy_chans + num_log_chans + num_memcpy_chans) * sizeof(struct d40_chan), GFP_KERNEL); - if (base == NULL) { - d40_err(&pdev->dev, "Out of memory\n"); + if (base == NULL) goto failure; - } base->rev = rev; base->clk = clk; diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c index 7f4af8c..032884f 100644 --- a/drivers/dma/tegra20-apb-dma.c +++ b/drivers/dma/tegra20-apb-dma.c @@ -300,10 +300,8 @@ static struct tegra_dma_desc *tegra_dma_desc_get( /* Allocate DMA desc */ dma_desc = kzalloc(sizeof(*dma_desc), GFP_NOWAIT); - if (!dma_desc) { - dev_err(tdc2dev(tdc), "dma_desc alloc failed\n"); + if (!dma_desc) return NULL; - } dma_async_tx_descriptor_init(&dma_desc->txd, &tdc->dma_chan); dma_desc->txd.tx_submit = tegra_dma_tx_submit; @@ -340,8 +338,7 @@ static struct tegra_dma_sg_req *tegra_dma_sg_req_get( spin_unlock_irqrestore(&tdc->lock, flags); sg_req = kzalloc(sizeof(struct tegra_dma_sg_req), GFP_NOWAIT); - if (!sg_req) - dev_err(tdc2dev(tdc), "sg_req alloc failed\n"); + return sg_req; } @@ -1319,10 +1316,8 @@ static int tegra_dma_probe(struct platform_device *pdev) tdma = devm_kzalloc(&pdev->dev, sizeof(*tdma) + cdata->nr_channels * sizeof(struct tegra_dma_channel), GFP_KERNEL); - if (!tdma) { - dev_err(&pdev->dev, "Error: memory allocation failed\n"); + if (!tdma) return -ENOMEM; - } tdma->dev = &pdev->dev; tdma->chip_data = cdata; diff --git a/drivers/dma/timb_dma.c b/drivers/dma/timb_dma.c index 559cd40..e82745a 100644 --- a/drivers/dma/timb_dma.c +++ b/drivers/dma/timb_dma.c @@ -337,18 +337,14 @@ static struct timb_dma_desc *td_alloc_init_desc(struct timb_dma_chan *td_chan) int err; td_desc = kzalloc(sizeof(struct timb_dma_desc), GFP_KERNEL); - if (!td_desc) { - dev_err(chan2dev(chan), "Failed to alloc descriptor\n"); + if (!td_desc) goto out; - } td_desc->desc_list_len = td_chan->desc_elems * TIMB_DMA_DESC_SIZE; td_desc->desc_list = kzalloc(td_desc->desc_list_len, GFP_KERNEL); - if (!td_desc->desc_list) { - dev_err(chan2dev(chan), "Failed to alloc descriptor\n"); + if (!td_desc->desc_list) goto err; - } dma_async_tx_descriptor_init(&td_desc->txd, chan); td_desc->txd.tx_submit = td_tx_submit; -- 1.9.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 8/8] dmaengine: Remove site specific OOM error messages on kzalloc 2016-06-07 17:38 ` [PATCH 8/8] dmaengine: Remove site specific OOM error messages on kzalloc Peter Griffin @ 2016-06-08 8:22 ` Jon Hunter 2016-06-08 12:12 ` Linus Walleij 1 sibling, 0 replies; 20+ messages in thread From: Jon Hunter @ 2016-06-08 8:22 UTC (permalink / raw) To: linux-arm-kernel On 07/06/16 18:38, Peter Griffin wrote: > If kzalloc() fails it will issue it's own error message including > a dump_stack(). So remove the site specific error messages. > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > --- > drivers/dma/amba-pl08x.c | 10 +--------- > drivers/dma/bestcomm/bestcomm.c | 2 -- > drivers/dma/edma.c | 16 ++++------------ > drivers/dma/fsldma.c | 2 -- > drivers/dma/k3dma.c | 10 ++++------ > drivers/dma/mmp_tdma.c | 5 ++--- > drivers/dma/moxart-dma.c | 4 +--- > drivers/dma/nbpfaxi.c | 5 ++--- > drivers/dma/pl330.c | 5 +---- > drivers/dma/ppc4xx/adma.c | 2 -- > drivers/dma/s3c24xx-dma.c | 5 +---- > drivers/dma/sh/shdmac.c | 9 ++------- > drivers/dma/sh/sudmac.c | 9 ++------- > drivers/dma/sirf-dma.c | 5 ++--- > drivers/dma/ste_dma40.c | 4 +--- > drivers/dma/tegra20-apb-dma.c | 11 +++-------- > drivers/dma/timb_dma.c | 8 ++------ > 17 files changed, 28 insertions(+), 84 deletions(-) [snip] > diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c > index 7f4af8c..032884f 100644 > --- a/drivers/dma/tegra20-apb-dma.c > +++ b/drivers/dma/tegra20-apb-dma.c > @@ -300,10 +300,8 @@ static struct tegra_dma_desc *tegra_dma_desc_get( > > /* Allocate DMA desc */ > dma_desc = kzalloc(sizeof(*dma_desc), GFP_NOWAIT); > - if (!dma_desc) { > - dev_err(tdc2dev(tdc), "dma_desc alloc failed\n"); > + if (!dma_desc) > return NULL; > - } > > dma_async_tx_descriptor_init(&dma_desc->txd, &tdc->dma_chan); > dma_desc->txd.tx_submit = tegra_dma_tx_submit; > @@ -340,8 +338,7 @@ static struct tegra_dma_sg_req *tegra_dma_sg_req_get( > spin_unlock_irqrestore(&tdc->lock, flags); > > sg_req = kzalloc(sizeof(struct tegra_dma_sg_req), GFP_NOWAIT); > - if (!sg_req) > - dev_err(tdc2dev(tdc), "sg_req alloc failed\n"); > + > return sg_req; > } > > @@ -1319,10 +1316,8 @@ static int tegra_dma_probe(struct platform_device *pdev) > > tdma = devm_kzalloc(&pdev->dev, sizeof(*tdma) + cdata->nr_channels * > sizeof(struct tegra_dma_channel), GFP_KERNEL); > - if (!tdma) { > - dev_err(&pdev->dev, "Error: memory allocation failed\n"); > + if (!tdma) > return -ENOMEM; > - } > > tdma->dev = &pdev->dev; > tdma->chip_data = cdata; For the tegra portion ... Acked-by: Jon Hunter <jonathanh@nvidia.com> Cheers Jon -- nvpublic ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 8/8] dmaengine: Remove site specific OOM error messages on kzalloc 2016-06-07 17:38 ` [PATCH 8/8] dmaengine: Remove site specific OOM error messages on kzalloc Peter Griffin 2016-06-08 8:22 ` Jon Hunter @ 2016-06-08 12:12 ` Linus Walleij 2016-06-09 15:30 ` Lee Jones 2016-06-15 17:08 ` Vinod Koul 1 sibling, 2 replies; 20+ messages in thread From: Linus Walleij @ 2016-06-08 12:12 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jun 7, 2016 at 7:38 PM, Peter Griffin <peter.griffin@linaro.org> wrote: > If kzalloc() fails it will issue it's own error message including > a dump_stack(). So remove the site specific error messages. > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> Acked-by: Linus Walleij <linus.walleij@linaro.org> A few subsystems may use a cleanup like this... I wonder how many unnecessary prints I've introduced myself :P Yours, Linus Walleij ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 8/8] dmaengine: Remove site specific OOM error messages on kzalloc 2016-06-08 12:12 ` Linus Walleij @ 2016-06-09 15:30 ` Lee Jones 2016-06-15 17:08 ` Vinod Koul 1 sibling, 0 replies; 20+ messages in thread From: Lee Jones @ 2016-06-09 15:30 UTC (permalink / raw) To: linux-arm-kernel On Wed, 08 Jun 2016, Linus Walleij wrote: > On Tue, Jun 7, 2016 at 7:38 PM, Peter Griffin <peter.griffin@linaro.org> wrote: > > > If kzalloc() fails it will issue it's own error message including > > a dump_stack(). So remove the site specific error messages. > > > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > > Acked-by: Linus Walleij <linus.walleij@linaro.org> > > A few subsystems may use a cleanup like this... > I wonder how many unnecessary prints I've introduced > myself :P Wolfram did some analysis on this a while back. IIRC he also presented on his findings at ELC(?). -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org ? Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 8/8] dmaengine: Remove site specific OOM error messages on kzalloc 2016-06-08 12:12 ` Linus Walleij 2016-06-09 15:30 ` Lee Jones @ 2016-06-15 17:08 ` Vinod Koul 1 sibling, 0 replies; 20+ messages in thread From: Vinod Koul @ 2016-06-15 17:08 UTC (permalink / raw) To: linux-arm-kernel On Wed, Jun 08, 2016 at 02:12:27PM +0200, Linus Walleij wrote: > On Tue, Jun 7, 2016 at 7:38 PM, Peter Griffin <peter.griffin@linaro.org> wrote: > > > If kzalloc() fails it will issue it's own error message including > > a dump_stack(). So remove the site specific error messages. > > > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > > Acked-by: Linus Walleij <linus.walleij@linaro.org> > > A few subsystems may use a cleanup like this... > I wonder how many unnecessary prints I've introduced > myself :P Yes that should be case with mine too :) I think adding a coccinelle script and doing this treewise might be great Thanks -- ~Vinod ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/8] Various dmaengine cleanups 2016-06-07 17:38 [PATCH 0/8] Various dmaengine cleanups Peter Griffin ` (7 preceding siblings ...) 2016-06-07 17:38 ` [PATCH 8/8] dmaengine: Remove site specific OOM error messages on kzalloc Peter Griffin @ 2016-06-21 16:05 ` Vinod Koul 8 siblings, 0 replies; 20+ messages in thread From: Vinod Koul @ 2016-06-21 16:05 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jun 07, 2016 at 06:38:33PM +0100, Peter Griffin wrote: > Hi Vinod, > > This series is a bunch of cleanup updates to various > dmaengine drivers, based on some of the review feeback to my fdma series. Good cleanup, Applied, thanks -- ~Vinod ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2016-06-28 4:04 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-06-07 17:38 [PATCH 0/8] Various dmaengine cleanups Peter Griffin 2016-06-07 17:38 ` [PATCH 1/8] dmaengine: fsl-edma: Fix clock handling error paths Peter Griffin 2016-06-07 17:38 ` [PATCH 2/8] dmaengine: fsl-edma: print error code in error messages Peter Griffin 2016-06-07 17:38 ` [PATCH 3/8] dmaengine: coh901318: Only calculate residue if txstate exists Peter Griffin 2016-06-08 12:15 ` Linus Walleij 2016-06-07 17:38 ` [PATCH 4/8] dmaengine: s3c24xx: Simplify code in s3c24xx_dma_tx_status() Peter Griffin 2016-06-07 17:38 ` [PATCH 5/8] dmaengine: ste_dma40: Only calculate residue if txstate exists Peter Griffin 2016-06-08 12:15 ` Linus Walleij 2016-06-07 17:38 ` [PATCH 6/8] dmaengine: sun6i-dma: Only calculate residue if state exists Peter Griffin 2016-06-07 17:38 ` [PATCH 7/8] dmaengine: tegra20-apb-dma: Only calculate residue if txstate exists Peter Griffin 2016-06-08 8:51 ` Jon Hunter 2016-06-21 16:01 ` Vinod Koul 2016-06-21 17:19 ` Jon Hunter 2016-06-28 4:04 ` Vinod Koul 2016-06-07 17:38 ` [PATCH 8/8] dmaengine: Remove site specific OOM error messages on kzalloc Peter Griffin 2016-06-08 8:22 ` Jon Hunter 2016-06-08 12:12 ` Linus Walleij 2016-06-09 15:30 ` Lee Jones 2016-06-15 17:08 ` Vinod Koul 2016-06-21 16:05 ` [PATCH 0/8] Various dmaengine cleanups Vinod Koul
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).