* [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, linux-kernel, kernel, vinod.koul,
linus.walleij, dan.j.williams, leoli, zw, baohua, maxime.ripard,
wens, ldewangan, jonathanh, swarren, thierry.reding, gnurou
Cc: peter.griffin, lee.jones, dmaengine, linux-tegra, linuxppc-dev
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, linux-kernel, kernel, vinod.koul,
linus.walleij, dan.j.williams, leoli, zw, baohua, maxime.ripard,
wens, ldewangan, jonathanh, swarren, thierry.reding, gnurou
Cc: peter.griffin, lee.jones, dmaengine, linux-tegra, linuxppc-dev
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, linux-kernel, kernel, vinod.koul,
linus.walleij, dan.j.williams, leoli, zw, baohua, maxime.ripard,
wens, ldewangan, jonathanh, swarren, thierry.reding, gnurou
Cc: peter.griffin, lee.jones, dmaengine, linux-tegra, linuxppc-dev
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, linux-kernel, kernel, vinod.koul,
linus.walleij, dan.j.williams, leoli, zw, baohua, maxime.ripard,
wens, ldewangan, jonathanh, swarren, thierry.reding, gnurou
Cc: peter.griffin, lee.jones, dmaengine, linux-tegra, linuxppc-dev
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 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, linux-kernel, kernel, vinod.koul,
linus.walleij, dan.j.williams, leoli, zw, baohua, maxime.ripard,
wens, ldewangan, jonathanh, swarren, thierry.reding, gnurou
Cc: peter.griffin, lee.jones, dmaengine, linux-tegra, linuxppc-dev
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, linux-kernel, kernel, vinod.koul,
linus.walleij, dan.j.williams, leoli, zw, baohua, maxime.ripard,
wens, ldewangan, jonathanh, swarren, thierry.reding, gnurou
Cc: peter.griffin, lee.jones, dmaengine, linux-tegra, linuxppc-dev
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 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, linux-kernel, kernel, vinod.koul,
linus.walleij, dan.j.williams, leoli, zw, baohua, maxime.ripard,
wens, ldewangan, jonathanh, swarren, thierry.reding, gnurou
Cc: peter.griffin, lee.jones, dmaengine, linux-tegra, linuxppc-dev
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, linux-kernel, kernel, vinod.koul,
linus.walleij, dan.j.williams, leoli, zw, baohua, maxime.ripard,
wens, ldewangan, jonathanh, swarren, thierry.reding, gnurou
Cc: peter.griffin, lee.jones, dmaengine, linux-tegra, linuxppc-dev
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 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, linux-kernel, kernel, vinod.koul,
linus.walleij, dan.j.williams, leoli, zw, baohua, maxime.ripard,
wens, ldewangan, jonathanh, swarren, thierry.reding, gnurou
Cc: peter.griffin, lee.jones, dmaengine, linux-tegra, linuxppc-dev
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
* Re: [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: Peter Griffin, linux-arm-kernel, linux-kernel, kernel,
vinod.koul, linus.walleij, dan.j.williams, leoli, zw, baohua,
maxime.ripard, wens, ldewangan, swarren, thierry.reding, gnurou
Cc: lee.jones, dmaengine, linux-tegra, linuxppc-dev
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
* Re: [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: Peter Griffin, linux-arm-kernel, linux-kernel, kernel,
vinod.koul, linus.walleij, dan.j.williams, leoli, zw, baohua,
maxime.ripard, wens, ldewangan, swarren, thierry.reding, gnurou
Cc: lee.jones, dmaengine, linux-tegra, linuxppc-dev
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
* Re: [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: Peter Griffin
Cc: linux-arm-kernel, linux-kernel, open list:ARM/STI ARCHITECTURE,
Vinod Koul, Dan Williams, Leo Li, Zhang Wei, Barry Song,
Maxime Ripard, Chen-Yu Tsai, Laxman Dewangan, Jon Hunter,
Stephen Warren, Thierry Reding, Alexandre Courbot, Lee Jones,
dmaengine, linux-tegra, linuxppc-dev@lists.ozlabs.org list
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
* Re: [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: Peter Griffin
Cc: linux-arm-kernel, linux-kernel, open list:ARM/STI ARCHITECTURE,
Vinod Koul, Dan Williams, Leo Li, Zhang Wei, Barry Song,
Maxime Ripard, Chen-Yu Tsai, Laxman Dewangan, Jon Hunter,
Stephen Warren, Thierry Reding, Alexandre Courbot, Lee Jones,
dmaengine, linux-tegra, linuxppc-dev@lists.ozlabs.org list
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
* Re: [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: Peter Griffin
Cc: linux-arm-kernel, linux-kernel, open list:ARM/STI ARCHITECTURE,
Vinod Koul, Dan Williams, Leo Li, Zhang Wei, Barry Song,
Maxime Ripard, Chen-Yu Tsai, Laxman Dewangan, Jon Hunter,
Stephen Warren, Thierry Reding, Alexandre Courbot, Lee Jones,
dmaengine, linux-tegra, linuxppc-dev@lists.ozlabs.org list
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
* Re: [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: Linus Walleij
Cc: Peter Griffin, linux-arm-kernel, linux-kernel,
open list:ARM/STI ARCHITECTURE, Vinod Koul, Dan Williams, Leo Li,
Zhang Wei, Barry Song, Maxime Ripard, Chen-Yu Tsai,
Laxman Dewangan, Jon Hunter, Stephen Warren, Thierry Reding,
Alexandre Courbot, dmaengine, linux-tegra,
linuxppc-dev@lists.ozlabs.org list
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
* Re: [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: Linus Walleij
Cc: Peter Griffin, linux-arm-kernel, linux-kernel,
open list:ARM/STI ARCHITECTURE, Dan Williams, Leo Li, Zhang Wei,
Barry Song, Maxime Ripard, Chen-Yu Tsai, Laxman Dewangan,
Jon Hunter, Stephen Warren, Thierry Reding, Alexandre Courbot,
Lee Jones, dmaengine, linux-tegra,
linuxppc-dev@lists.ozlabs.org list
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
* Re: [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: Jon Hunter
Cc: Peter Griffin, linux-arm-kernel, linux-kernel, kernel,
linus.walleij, dan.j.williams, leoli, zw, baohua, maxime.ripard,
wens, ldewangan, swarren, thierry.reding, gnurou, lee.jones,
dmaengine, linux-tegra, linuxppc-dev
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
* Re: [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: Peter Griffin
Cc: linux-arm-kernel, linux-kernel, kernel, linus.walleij,
dan.j.williams, leoli, zw, baohua, maxime.ripard, wens,
ldewangan, jonathanh, swarren, thierry.reding, gnurou, lee.jones,
dmaengine, linux-tegra, linuxppc-dev
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
* Re: [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: Vinod Koul
Cc: Peter Griffin, linux-arm-kernel, linux-kernel, kernel,
linus.walleij, dan.j.williams, leoli, zw, baohua, maxime.ripard,
wens, ldewangan, swarren, thierry.reding, gnurou, lee.jones,
dmaengine, linux-tegra, linuxppc-dev
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
* Re: [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: Jon Hunter
Cc: Peter Griffin, linux-arm-kernel, linux-kernel, kernel,
linus.walleij, dan.j.williams, leoli, zw, baohua, maxime.ripard,
wens, ldewangan, swarren, thierry.reding, gnurou, lee.jones,
dmaengine, linux-tegra, linuxppc-dev
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
end of thread, other threads:[~2016-06-28 3:57 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).