2015-11-13 16:39:52

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V3 0/6] DMA: tegra-apb: Clean-up

Some clean-up changes for the tegra-apb DMA driver.

Changes for V3 noted in the applicable patches.

These have been compile and boot tested for ARM and ARM64. Summary of the
ARM results are below.

Test summary
------------

Build: zImage:
Pass: ( 2/ 2): multi_v7_defconfig, tegra_defconfig

Build: Image:
Pass: ( 1/ 1): defconfig

Boot to userspace: defconfig:
Pass: ( 1/ 1): qemu-vexpress64

Boot to userspace: multi_v7_defconfig:
Pass: ( 4/ 4): tegra114-dalmore-a04, tegra124-jetson-tk1,
tegra20-trimslice, tegra30-beaver

Boot to userspace: tegra_defconfig:
Pass: ( 4/ 4): tegra114-dalmore-a04, tegra124-jetson-tk1,
tegra20-trimslice, tegra30-beaver

Jon Hunter (6):
dmaengine: tegra-apb: Correct runtime-pm usage
dmaengine: tegra-apb: Use dev_get_drvdata()
dmaengine: tegra-apb: Save and restore word count
dmaengine: tegra-apb: Only save channel state for those in use
dmaengine: tegra-apb: Update driver to use GFP_NOWAIT
dmaengine: tegra-apb: Free interrupts before killing tasklets

drivers/dma/tegra20-apb-dma.c | 73 ++++++++++++++++++++++++-------------------
1 file changed, 40 insertions(+), 33 deletions(-)

--
2.1.4


2015-11-13 16:39:56

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V3 1/6] dmaengine: tegra-apb: Correct runtime-pm usage

The tegra-apb DMA driver enables runtime-pm but never calls
pm_runtime_get/put and hence the runtime-pm callbacks are never invoked.
The driver manages the clocks by directly calling clk_prepare_enable()
and clk_unprepare_disable().

Fix this by replacing the clk_prepare_enable() and clk_disable_unprepare()
with pm_runtime_get_sync() and pm_runtime_put(), respectively. Note that
the consequence of this is that if runtime-pm is disabled, then the clocks
will remain on the entire time the driver is loaded. However, if
runtime-pm is disabled, then power is not most likely not a concern.

Signed-off-by: Jon Hunter <[email protected]>
---
V3 changes:
- Removed unnecessary update to local variables in suspend/resume
V2 changes:
- Fixed return value for allocating channel
- Fixed test for return value from pm_runtime_get_sync()

drivers/dma/tegra20-apb-dma.c | 43 ++++++++++++++++++-------------------------
1 file changed, 18 insertions(+), 25 deletions(-)

diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
index c8f79dcaaee8..f68bccf55a24 100644
--- a/drivers/dma/tegra20-apb-dma.c
+++ b/drivers/dma/tegra20-apb-dma.c
@@ -1186,10 +1186,12 @@ static int tegra_dma_alloc_chan_resources(struct dma_chan *dc)

dma_cookie_init(&tdc->dma_chan);
tdc->config_init = false;
- ret = clk_prepare_enable(tdma->dma_clk);
+
+ ret = pm_runtime_get_sync(tdma->dev);
if (ret < 0)
- dev_err(tdc2dev(tdc), "clk_prepare_enable failed: %d\n", ret);
- return ret;
+ return ret;
+
+ return 0;
}

static void tegra_dma_free_chan_resources(struct dma_chan *dc)
@@ -1232,7 +1234,7 @@ static void tegra_dma_free_chan_resources(struct dma_chan *dc)
list_del(&sg_req->node);
kfree(sg_req);
}
- clk_disable_unprepare(tdma->dma_clk);
+ pm_runtime_put(tdma->dev);

tdc->slave_id = 0;
}
@@ -1356,20 +1358,14 @@ static int tegra_dma_probe(struct platform_device *pdev)
spin_lock_init(&tdma->global_lock);

pm_runtime_enable(&pdev->dev);
- if (!pm_runtime_enabled(&pdev->dev)) {
+ if (!pm_runtime_enabled(&pdev->dev))
ret = tegra_dma_runtime_resume(&pdev->dev);
- if (ret) {
- dev_err(&pdev->dev, "dma_runtime_resume failed %d\n",
- ret);
- goto err_pm_disable;
- }
- }
+ else
+ ret = pm_runtime_get_sync(&pdev->dev);

- /* Enable clock before accessing registers */
- ret = clk_prepare_enable(tdma->dma_clk);
if (ret < 0) {
- dev_err(&pdev->dev, "clk_prepare_enable failed: %d\n", ret);
- goto err_pm_disable;
+ pm_runtime_disable(&pdev->dev);
+ return ret;
}

/* Reset DMA controller */
@@ -1382,7 +1378,7 @@ static int tegra_dma_probe(struct platform_device *pdev)
tdma_write(tdma, TEGRA_APBDMA_CONTROL, 0);
tdma_write(tdma, TEGRA_APBDMA_IRQ_MASK_SET, 0xFFFFFFFFul);

- clk_disable_unprepare(tdma->dma_clk);
+ pm_runtime_put(&pdev->dev);

INIT_LIST_HEAD(&tdma->dma_dev.channels);
for (i = 0; i < cdata->nr_channels; i++) {
@@ -1485,7 +1481,6 @@ err_irq:
tasklet_kill(&tdc->tasklet);
}

-err_pm_disable:
pm_runtime_disable(&pdev->dev);
if (!pm_runtime_status_suspended(&pdev->dev))
tegra_dma_runtime_suspend(&pdev->dev);
@@ -1543,7 +1538,7 @@ static int tegra_dma_pm_suspend(struct device *dev)
int ret;

/* Enable clock before accessing register */
- ret = tegra_dma_runtime_resume(dev);
+ ret = pm_runtime_get_sync(dev);
if (ret < 0)
return ret;

@@ -1560,7 +1555,7 @@ static int tegra_dma_pm_suspend(struct device *dev)
}

/* Disable clock */
- tegra_dma_runtime_suspend(dev);
+ pm_runtime_put(dev);
return 0;
}

@@ -1571,7 +1566,7 @@ static int tegra_dma_pm_resume(struct device *dev)
int ret;

/* Enable clock before accessing register */
- ret = tegra_dma_runtime_resume(dev);
+ ret = pm_runtime_get_sync(dev);
if (ret < 0)
return ret;

@@ -1592,16 +1587,14 @@ static int tegra_dma_pm_resume(struct device *dev)
}

/* Disable clock */
- tegra_dma_runtime_suspend(dev);
+ pm_runtime_put(dev);
return 0;
}
#endif

static const struct dev_pm_ops tegra_dma_dev_pm_ops = {
-#ifdef CONFIG_PM
- .runtime_suspend = tegra_dma_runtime_suspend,
- .runtime_resume = tegra_dma_runtime_resume,
-#endif
+ SET_RUNTIME_PM_OPS(tegra_dma_runtime_suspend, tegra_dma_runtime_resume,
+ NULL)
SET_SYSTEM_SLEEP_PM_OPS(tegra_dma_pm_suspend, tegra_dma_pm_resume)
};

--
2.1.4

2015-11-13 16:40:04

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V3 2/6] dmaengine: tegra-apb: Use dev_get_drvdata()

In the tegra_dma_runtime_suspend/resume functions, the pdev structure
is not needed, and so just call dev_get_drvdata() to get the device
data structure.

Signed-off-by: Jon Hunter <[email protected]>
---
drivers/dma/tegra20-apb-dma.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
index f68bccf55a24..355dbc354818 100644
--- a/drivers/dma/tegra20-apb-dma.c
+++ b/drivers/dma/tegra20-apb-dma.c
@@ -1509,8 +1509,7 @@ static int tegra_dma_remove(struct platform_device *pdev)

static int tegra_dma_runtime_suspend(struct device *dev)
{
- struct platform_device *pdev = to_platform_device(dev);
- struct tegra_dma *tdma = platform_get_drvdata(pdev);
+ struct tegra_dma *tdma = dev_get_drvdata(dev);

clk_disable_unprepare(tdma->dma_clk);
return 0;
@@ -1518,8 +1517,7 @@ static int tegra_dma_runtime_suspend(struct device *dev)

static int tegra_dma_runtime_resume(struct device *dev)
{
- struct platform_device *pdev = to_platform_device(dev);
- struct tegra_dma *tdma = platform_get_drvdata(pdev);
+ struct tegra_dma *tdma = dev_get_drvdata(dev);
int ret;

ret = clk_prepare_enable(tdma->dma_clk);
--
2.1.4

2015-11-13 16:40:06

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V3 3/6] dmaengine: tegra-apb: Save and restore word count

Newer tegra devices have a separate word count register per channel that
contains the number of words to be transferred. This register is not
saved or restored by the suspend/resume helpers for these newer devices
and so ensure that it is.

Signed-off-by: Jon Hunter <[email protected]>
---
drivers/dma/tegra20-apb-dma.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
index 355dbc354818..18a561376c63 100644
--- a/drivers/dma/tegra20-apb-dma.c
+++ b/drivers/dma/tegra20-apb-dma.c
@@ -1550,6 +1550,9 @@ static int tegra_dma_pm_suspend(struct device *dev)
ch_reg->apb_ptr = tdc_read(tdc, TEGRA_APBDMA_CHAN_APBPTR);
ch_reg->ahb_seq = tdc_read(tdc, TEGRA_APBDMA_CHAN_AHBSEQ);
ch_reg->apb_seq = tdc_read(tdc, TEGRA_APBDMA_CHAN_APBSEQ);
+ if (tdma->chip_data->support_separate_wcount_reg)
+ ch_reg->wcount = tdc_read(tdc,
+ TEGRA_APBDMA_CHAN_WCOUNT);
}

/* Disable clock */
@@ -1576,6 +1579,9 @@ static int tegra_dma_pm_resume(struct device *dev)
struct tegra_dma_channel *tdc = &tdma->channels[i];
struct tegra_dma_channel_regs *ch_reg = &tdc->channel_reg;

+ if (tdma->chip_data->support_separate_wcount_reg)
+ tdc_write(tdc, TEGRA_APBDMA_CHAN_WCOUNT,
+ ch_reg->wcount);
tdc_write(tdc, TEGRA_APBDMA_CHAN_APBSEQ, ch_reg->apb_seq);
tdc_write(tdc, TEGRA_APBDMA_CHAN_APBPTR, ch_reg->apb_ptr);
tdc_write(tdc, TEGRA_APBDMA_CHAN_AHBSEQ, ch_reg->ahb_seq);
--
2.1.4

2015-11-13 16:41:38

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V3 4/6] dmaengine: tegra-apb: Only save channel state for those in use

Currently the tegra-apb DMA driver suspend/resume helpers, save and
restore the registers for all channels regardless of whether they are
in use or not. Change this so that only channels that have been
allocated and configured are saved and restored.

Signed-off-by: Jon Hunter <[email protected]>
---
V3 changes:
- Updated comment to be on a single line

drivers/dma/tegra20-apb-dma.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
index 18a561376c63..d4cabd6931e5 100644
--- a/drivers/dma/tegra20-apb-dma.c
+++ b/drivers/dma/tegra20-apb-dma.c
@@ -1545,6 +1545,10 @@ static int tegra_dma_pm_suspend(struct device *dev)
struct tegra_dma_channel *tdc = &tdma->channels[i];
struct tegra_dma_channel_regs *ch_reg = &tdc->channel_reg;

+ /* Only save the state of DMA channels that are in use */
+ if (!tdc->config_init)
+ continue;
+
ch_reg->csr = tdc_read(tdc, TEGRA_APBDMA_CHAN_CSR);
ch_reg->ahb_ptr = tdc_read(tdc, TEGRA_APBDMA_CHAN_AHBPTR);
ch_reg->apb_ptr = tdc_read(tdc, TEGRA_APBDMA_CHAN_APBPTR);
@@ -1579,6 +1583,10 @@ static int tegra_dma_pm_resume(struct device *dev)
struct tegra_dma_channel *tdc = &tdma->channels[i];
struct tegra_dma_channel_regs *ch_reg = &tdc->channel_reg;

+ /* Only restore the state of DMA channels that are in use */
+ if (!tdc->config_init)
+ continue;
+
if (tdma->chip_data->support_separate_wcount_reg)
tdc_write(tdc, TEGRA_APBDMA_CHAN_WCOUNT,
ch_reg->wcount);
--
2.1.4

2015-11-13 16:40:51

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V3 5/6] dmaengine: tegra-apb: Update driver to use GFP_NOWAIT

The tegra20-apb-dma driver currently uses the flag GFP_ATOMIC when
allocating memory for structures used in conjunction with the DMA
descriptors. It is preferred that dmaengine drivers use GFP_NOWAIT
instead and so the emergency memory pool will not be used by these
drivers.

Signed-off-by: Jon Hunter <[email protected]>
---
drivers/dma/tegra20-apb-dma.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
index d4cabd6931e5..754c1f54d4fe 100644
--- a/drivers/dma/tegra20-apb-dma.c
+++ b/drivers/dma/tegra20-apb-dma.c
@@ -296,7 +296,7 @@ static struct tegra_dma_desc *tegra_dma_desc_get(
spin_unlock_irqrestore(&tdc->lock, flags);

/* Allocate DMA desc */
- dma_desc = kzalloc(sizeof(*dma_desc), GFP_ATOMIC);
+ dma_desc = kzalloc(sizeof(*dma_desc), GFP_NOWAIT);
if (!dma_desc) {
dev_err(tdc2dev(tdc), "dma_desc alloc failed\n");
return NULL;
@@ -336,7 +336,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_ATOMIC);
+ 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;
--
2.1.4

2015-11-13 16:40:13

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V3 6/6] dmaengine: tegra-apb: Free interrupts before killing tasklets

On probe failure or driver removal, before killing any tasklets, ensure
that the channel interrupt is freed to ensure that another channel
interrupt cannot occur and schedule the tasklet again.

Signed-off-by: Jon Hunter <[email protected]>
---
drivers/dma/tegra20-apb-dma.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
index 754c1f54d4fe..935da8192f59 100644
--- a/drivers/dma/tegra20-apb-dma.c
+++ b/drivers/dma/tegra20-apb-dma.c
@@ -1396,8 +1396,7 @@ static int tegra_dma_probe(struct platform_device *pdev)
}
tdc->irq = res->start;
snprintf(tdc->name, sizeof(tdc->name), "apbdma.%d", i);
- ret = devm_request_irq(&pdev->dev, tdc->irq,
- tegra_dma_isr, 0, tdc->name, tdc);
+ ret = request_irq(tdc->irq, tegra_dma_isr, 0, tdc->name, tdc);
if (ret) {
dev_err(&pdev->dev,
"request_irq failed with err %d channel %d\n",
@@ -1478,6 +1477,8 @@ err_unregister_dma_dev:
err_irq:
while (--i >= 0) {
struct tegra_dma_channel *tdc = &tdma->channels[i];
+
+ free_irq(tdc->irq, tdc);
tasklet_kill(&tdc->tasklet);
}

@@ -1497,6 +1498,7 @@ static int tegra_dma_remove(struct platform_device *pdev)

for (i = 0; i < tdma->chip_data->nr_channels; ++i) {
tdc = &tdma->channels[i];
+ free_irq(tdc->irq, tdc);
tasklet_kill(&tdc->tasklet);
}

--
2.1.4

2015-11-14 13:27:53

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH V3 1/6] dmaengine: tegra-apb: Correct runtime-pm usage

On Fri, Nov 13, 2015 at 6:39 PM, Jon Hunter <[email protected]> wrote:
> The tegra-apb DMA driver enables runtime-pm but never calls
> pm_runtime_get/put and hence the runtime-pm callbacks are never invoked.
> The driver manages the clocks by directly calling clk_prepare_enable()
> and clk_unprepare_disable().
>
> Fix this by replacing the clk_prepare_enable() and clk_disable_unprepare()
> with pm_runtime_get_sync() and pm_runtime_put(), respectively. Note that
> the consequence of this is that if runtime-pm is disabled, then the clocks
> will remain on the entire time the driver is loaded. However, if
> runtime-pm is disabled, then power is not most likely not a concern.
>

Have you tested these changes somehow?

See my comments below.

> Signed-off-by: Jon Hunter <[email protected]>
> ---
> V3 changes:
> - Removed unnecessary update to local variables in suspend/resume
> V2 changes:
> - Fixed return value for allocating channel
> - Fixed test for return value from pm_runtime_get_sync()
>
> drivers/dma/tegra20-apb-dma.c | 43 ++++++++++++++++++-------------------------
> 1 file changed, 18 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
> index c8f79dcaaee8..f68bccf55a24 100644
> --- a/drivers/dma/tegra20-apb-dma.c
> +++ b/drivers/dma/tegra20-apb-dma.c
> @@ -1186,10 +1186,12 @@ static int tegra_dma_alloc_chan_resources(struct dma_chan *dc)
>
> dma_cookie_init(&tdc->dma_chan);
> tdc->config_init = false;
> - ret = clk_prepare_enable(tdma->dma_clk);
> +
> + ret = pm_runtime_get_sync(tdma->dev);
> if (ret < 0)
> - dev_err(tdc2dev(tdc), "clk_prepare_enable failed: %d\n", ret);
> - return ret;
> + return ret;
> +

> + return 0;

This is a non-reachable code.

> }
>
> static void tegra_dma_free_chan_resources(struct dma_chan *dc)
> @@ -1232,7 +1234,7 @@ static void tegra_dma_free_chan_resources(struct dma_chan *dc)
> list_del(&sg_req->node);
> kfree(sg_req);
> }
> - clk_disable_unprepare(tdma->dma_clk);
> + pm_runtime_put(tdma->dev);
>
> tdc->slave_id = 0;
> }
> @@ -1356,20 +1358,14 @@ static int tegra_dma_probe(struct platform_device *pdev)
> spin_lock_init(&tdma->global_lock);
>
> pm_runtime_enable(&pdev->dev);
> - if (!pm_runtime_enabled(&pdev->dev)) {
> + if (!pm_runtime_enabled(&pdev->dev))
> ret = tegra_dma_runtime_resume(&pdev->dev);

I didn't check, but does this bring device to powered on state?

> - if (ret) {
> - dev_err(&pdev->dev, "dma_runtime_resume failed %d\n",
> - ret);
> - goto err_pm_disable;
> - }
> - }
> + else
> + ret = pm_runtime_get_sync(&pdev->dev);
>
> - /* Enable clock before accessing registers */
> - ret = clk_prepare_enable(tdma->dma_clk);
> if (ret < 0) {
> - dev_err(&pdev->dev, "clk_prepare_enable failed: %d\n", ret);
> - goto err_pm_disable;
> + pm_runtime_disable(&pdev->dev);
> + return ret;
> }
>
> /* Reset DMA controller */
> @@ -1382,7 +1378,7 @@ static int tegra_dma_probe(struct platform_device *pdev)
> tdma_write(tdma, TEGRA_APBDMA_CONTROL, 0);
> tdma_write(tdma, TEGRA_APBDMA_IRQ_MASK_SET, 0xFFFFFFFFul);
>
> - clk_disable_unprepare(tdma->dma_clk);
> + pm_runtime_put(&pdev->dev);
>
> INIT_LIST_HEAD(&tdma->dma_dev.channels);
> for (i = 0; i < cdata->nr_channels; i++) {
> @@ -1485,7 +1481,6 @@ err_irq:
> tasklet_kill(&tdc->tasklet);
> }
>
> -err_pm_disable:
> pm_runtime_disable(&pdev->dev);
> if (!pm_runtime_status_suspended(&pdev->dev))
> tegra_dma_runtime_suspend(&pdev->dev);
> @@ -1543,7 +1538,7 @@ static int tegra_dma_pm_suspend(struct device *dev)
> int ret;
>
> /* Enable clock before accessing register */
> - ret = tegra_dma_runtime_resume(dev);
> + ret = pm_runtime_get_sync(dev);
> if (ret < 0)
> return ret;
>
> @@ -1560,7 +1555,7 @@ static int tegra_dma_pm_suspend(struct device *dev)
> }
>
> /* Disable clock */
> - tegra_dma_runtime_suspend(dev);
> + pm_runtime_put(dev);
> return 0;
> }
>
> @@ -1571,7 +1566,7 @@ static int tegra_dma_pm_resume(struct device *dev)
> int ret;
>
> /* Enable clock before accessing register */
> - ret = tegra_dma_runtime_resume(dev);
> + ret = pm_runtime_get_sync(dev);
> if (ret < 0)
> return ret;
>
> @@ -1592,16 +1587,14 @@ static int tegra_dma_pm_resume(struct device *dev)
> }
>
> /* Disable clock */
> - tegra_dma_runtime_suspend(dev);
> + pm_runtime_put(dev);
> return 0;
> }
> #endif
>
> static const struct dev_pm_ops tegra_dma_dev_pm_ops = {
> -#ifdef CONFIG_PM
> - .runtime_suspend = tegra_dma_runtime_suspend,
> - .runtime_resume = tegra_dma_runtime_resume,
> -#endif
> + SET_RUNTIME_PM_OPS(tegra_dma_runtime_suspend, tegra_dma_runtime_resume,
> + NULL)
> SET_SYSTEM_SLEEP_PM_OPS(tegra_dma_pm_suspend, tegra_dma_pm_resume)
> };
>
> --
> 2.1.4
>



--
With Best Regards,
Andy Shevchenko

2015-11-16 09:34:58

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH V3 1/6] dmaengine: tegra-apb: Correct runtime-pm usage


On 14/11/15 13:27, Andy Shevchenko wrote:
> On Fri, Nov 13, 2015 at 6:39 PM, Jon Hunter <[email protected]> wrote:
>> The tegra-apb DMA driver enables runtime-pm but never calls
>> pm_runtime_get/put and hence the runtime-pm callbacks are never invoked.
>> The driver manages the clocks by directly calling clk_prepare_enable()
>> and clk_unprepare_disable().
>>
>> Fix this by replacing the clk_prepare_enable() and clk_disable_unprepare()
>> with pm_runtime_get_sync() and pm_runtime_put(), respectively. Note that
>> the consequence of this is that if runtime-pm is disabled, then the clocks
>> will remain on the entire time the driver is loaded. However, if
>> runtime-pm is disabled, then power is not most likely not a concern.
>>
>
> Have you tested these changes somehow?

Yes.

> See my comments below.
>
>> Signed-off-by: Jon Hunter <[email protected]>
>> ---
>> V3 changes:
>> - Removed unnecessary update to local variables in suspend/resume
>> V2 changes:
>> - Fixed return value for allocating channel
>> - Fixed test for return value from pm_runtime_get_sync()
>>
>> drivers/dma/tegra20-apb-dma.c | 43 ++++++++++++++++++-------------------------
>> 1 file changed, 18 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
>> index c8f79dcaaee8..f68bccf55a24 100644
>> --- a/drivers/dma/tegra20-apb-dma.c
>> +++ b/drivers/dma/tegra20-apb-dma.c
>> @@ -1186,10 +1186,12 @@ static int tegra_dma_alloc_chan_resources(struct dma_chan *dc)
>>
>> dma_cookie_init(&tdc->dma_chan);
>> tdc->config_init = false;
>> - ret = clk_prepare_enable(tdma->dma_clk);
>> +
>> + ret = pm_runtime_get_sync(tdma->dev);
>> if (ret < 0)
>> - dev_err(tdc2dev(tdc), "clk_prepare_enable failed: %d\n", ret);
>> - return ret;
>> + return ret;
>> +
>
>> + return 0;
>
> This is a non-reachable code.

No it is not.

>> }
>>
>> static void tegra_dma_free_chan_resources(struct dma_chan *dc)
>> @@ -1232,7 +1234,7 @@ static void tegra_dma_free_chan_resources(struct dma_chan *dc)
>> list_del(&sg_req->node);
>> kfree(sg_req);
>> }
>> - clk_disable_unprepare(tdma->dma_clk);
>> + pm_runtime_put(tdma->dev);
>>
>> tdc->slave_id = 0;
>> }
>> @@ -1356,20 +1358,14 @@ static int tegra_dma_probe(struct platform_device *pdev)
>> spin_lock_init(&tdma->global_lock);
>>
>> pm_runtime_enable(&pdev->dev);
>> - if (!pm_runtime_enabled(&pdev->dev)) {
>> + if (!pm_runtime_enabled(&pdev->dev))
>> ret = tegra_dma_runtime_resume(&pdev->dev);
>
> I didn't check, but does this bring device to powered on state?

Yes.

Jon

2015-11-30 16:45:32

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH V3 0/6] DMA: tegra-apb: Clean-up

Hi Vinod,

Any more comments on this?

Cheers
Jon

On 13/11/15 16:39, Jon Hunter wrote:
> Some clean-up changes for the tegra-apb DMA driver.
>
> Changes for V3 noted in the applicable patches.
>
> These have been compile and boot tested for ARM and ARM64. Summary of the
> ARM results are below.
>
> Test summary
> ------------
>
> Build: zImage:
> Pass: ( 2/ 2): multi_v7_defconfig, tegra_defconfig
>
> Build: Image:
> Pass: ( 1/ 1): defconfig
>
> Boot to userspace: defconfig:
> Pass: ( 1/ 1): qemu-vexpress64
>
> Boot to userspace: multi_v7_defconfig:
> Pass: ( 4/ 4): tegra114-dalmore-a04, tegra124-jetson-tk1,
> tegra20-trimslice, tegra30-beaver
>
> Boot to userspace: tegra_defconfig:
> Pass: ( 4/ 4): tegra114-dalmore-a04, tegra124-jetson-tk1,
> tegra20-trimslice, tegra30-beaver
>
> Jon Hunter (6):
> dmaengine: tegra-apb: Correct runtime-pm usage
> dmaengine: tegra-apb: Use dev_get_drvdata()
> dmaengine: tegra-apb: Save and restore word count
> dmaengine: tegra-apb: Only save channel state for those in use
> dmaengine: tegra-apb: Update driver to use GFP_NOWAIT
> dmaengine: tegra-apb: Free interrupts before killing tasklets
>
> drivers/dma/tegra20-apb-dma.c | 73 ++++++++++++++++++++++++-------------------
> 1 file changed, 40 insertions(+), 33 deletions(-)
>

2015-12-05 10:40:39

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH V3 0/6] DMA: tegra-apb: Clean-up

On Fri, Nov 13, 2015 at 04:39:37PM +0000, Jon Hunter wrote:
> Some clean-up changes for the tegra-apb DMA driver.
>
> Changes for V3 noted in the applicable patches.
>
> These have been compile and boot tested for ARM and ARM64. Summary of the
> ARM results are below.

Applied, thanks

--
~Vinod