Hello,
This series fixes some problems that I spotted recently, secondly the
driver's code gets a cleanup. Please review and apply, thanks in advance!
Changelog:
v7: - Updated patch "Keep clock enabled only during of DMA transfer" by
fixing RPM refcount problem of v6 that should happen if oneshot
transfer is terminated with EOC being set. The patch was change in
accordance to Jon's Hunter recommendation. In a result there are
these new small additional patches:
dmaengine: tegra-apb: Remove handling of unrealistic error condition
dmaengine: tegra-apb: Don't stop cyclic DMA in a case of error condition
- Updated commit's message of the "Clean up suspend-resume" patch in
accordance to Jon's request, now saying that channel's pausing isn't
supported by the driver.
- Added new very minor patch to clean up tdc_start_head_req():
dmaengine: tegra-apb: Remove pending_sg_req checking from tdc_start_head_req
v6: - Added stable tag and changed order of the patch "Prevent race
conditions of tasklet vs free list", making it patch #2, as was
requested by Jon Hunter in the review comment to v5.
- Factored out the tdc->config_init cleanup into separate patch, as was
requested by Jon Hunter in the review comment to v5:
dmaengine: tegra-apb: Remove unneeded initialization of tdc->config_init
- Added new very minor patch to enable compile-testing for the driver:
dmaengine: tegra-apb: Support COMPILE_TEST
v5: - Fixed touching hardware registers after RPM-suspending in the patch
"Keep clock enabled only during of DMA transfer", now RPM is kept
resumed in the tegra_dma_terminate_all() while needed. Thanks to
Jon Hunter for pointing at this problem in a review comment to v4.
- The "Clean up runtime PM teardown" patch is replaced with the "Remove
assumptions about unavailable runtime PM" patch because I recalled that
now RPM is always available on all Tegra SoCs.
- The "Clean up suspend-resume" patch got a minor improvement, now
tasklet_kill() is invoked before checking of the busy state in
tegra_dma_dev_suspend(), this should allow us to catch problems if DMA
callback issues a new DMA transfer.
- Added Jon's acks to the reviewed patches.
v4: - Addressed Jon's request to *not* remove the runtime PM usage, instead
there is now new patch that makes RPM more practical:
dmaengine: tegra-apb: Keep clock enabled only during of DMA transfer
- Added new minor patch to clean up RPM's teardown:
dmaengine: tegra-apb: Clean up runtime PM teardown
v3: - In the review comment to v1 Michał Mirosław suggested that "Prevent
race conditions on channel's freeing" does changes that deserve to
be separated into two patches. I factored out and improved tasklet
releasing into this new patch:
dmaengine: tegra-apb: Clean up tasklet releasing
- The "Fix use-after-free" patch got an improved commit message.
v2: - I took another look at the driver and spotted few more things that
could be improved, which resulted in these new patches:
dmaengine: tegra-apb: Remove runtime PM usage
dmaengine: tegra-apb: Clean up suspend-resume
dmaengine: tegra-apb: Add missing of_dma_controller_free
dmaengine: tegra-apb: Allow to compile as a loadable kernel module
dmaengine: tegra-apb: Remove MODULE_ALIAS
Dmitry Osipenko (19):
dmaengine: tegra-apb: Fix use-after-free
dmaengine: tegra-apb: Prevent race conditions of tasklet vs free list
dmaengine: tegra-apb: Implement synchronization hook
dmaengine: tegra-apb: Prevent race conditions on channel's freeing
dmaengine: tegra-apb: Clean up tasklet releasing
dmaengine: tegra-apb: Use devm_platform_ioremap_resource
dmaengine: tegra-apb: Use devm_request_irq
dmaengine: tegra-apb: Fix coding style problems
dmaengine: tegra-apb: Remove unneeded initialization of
tdc->config_init
dmaengine: tegra-apb: Remove assumptions about unavailable runtime PM
dmaengine: tegra-apb: Remove pending_sg_req checking from
tdc_start_head_req
dmaengine: tegra-apb: Remove handling of unrealistic error condition
dmaengine: tegra-apb: Don't stop cyclic DMA in a case of error
condition
dmaengine: tegra-apb: Keep clock enabled only during of DMA transfer
dmaengine: tegra-apb: Clean up suspend-resume
dmaengine: tegra-apb: Add missing of_dma_controller_free
dmaengine: tegra-apb: Allow to compile as a loadable kernel module
dmaengine: tegra-apb: Remove MODULE_ALIAS
dmaengine: tegra-apb: Support COMPILE_TEST
drivers/dma/Kconfig | 4 +-
drivers/dma/tegra20-apb-dma.c | 516 +++++++++++++++++-----------------
2 files changed, 258 insertions(+), 262 deletions(-)
--
2.24.0
The ISR tasklet could be kept scheduled after DMA transfer termination,
let's add synchronization hook which blocks until tasklet is finished.
Acked-by: Jon Hunter <[email protected]>
Signed-off-by: Dmitry Osipenko <[email protected]>
---
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 4a750e29bfb5..f56881500a23 100644
--- a/drivers/dma/tegra20-apb-dma.c
+++ b/drivers/dma/tegra20-apb-dma.c
@@ -798,6 +798,13 @@ static int tegra_dma_terminate_all(struct dma_chan *dc)
return 0;
}
+static void tegra_dma_synchronize(struct dma_chan *dc)
+{
+ struct tegra_dma_channel *tdc = to_tegra_dma_chan(dc);
+
+ tasklet_kill(&tdc->tasklet);
+}
+
static unsigned int tegra_dma_sg_bytes_xferred(struct tegra_dma_channel *tdc,
struct tegra_dma_sg_req *sg_req)
{
@@ -1506,6 +1513,7 @@ static int tegra_dma_probe(struct platform_device *pdev)
tdma->dma_dev.residue_granularity = DMA_RESIDUE_GRANULARITY_BURST;
tdma->dma_dev.device_config = tegra_dma_slave_config;
tdma->dma_dev.device_terminate_all = tegra_dma_terminate_all;
+ tdma->dma_dev.device_synchronize = tegra_dma_synchronize;
tdma->dma_dev.device_tx_status = tegra_dma_tx_status;
tdma->dma_dev.device_issue_pending = tegra_dma_issue_pending;
--
2.24.0
Use resource-managed variant of request_irq for brevity.
Acked-by: Jon Hunter <[email protected]>
Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/dma/tegra20-apb-dma.c | 35 +++++++++++------------------------
1 file changed, 11 insertions(+), 24 deletions(-)
diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
index f44291207928..dff21e80ffa4 100644
--- a/drivers/dma/tegra20-apb-dma.c
+++ b/drivers/dma/tegra20-apb-dma.c
@@ -182,7 +182,6 @@ struct tegra_dma_channel {
char name[12];
bool config_init;
int id;
- int irq;
void __iomem *chan_addr;
spinlock_t lock;
bool busy;
@@ -1380,7 +1379,6 @@ static const struct tegra_dma_chip_data tegra148_dma_chip_data = {
static int tegra_dma_probe(struct platform_device *pdev)
{
- struct resource *res;
struct tegra_dma *tdma;
int ret;
int i;
@@ -1446,25 +1444,27 @@ static int tegra_dma_probe(struct platform_device *pdev)
INIT_LIST_HEAD(&tdma->dma_dev.channels);
for (i = 0; i < cdata->nr_channels; i++) {
struct tegra_dma_channel *tdc = &tdma->channels[i];
+ int irq;
tdc->chan_addr = tdma->base_addr +
TEGRA_APBDMA_CHANNEL_BASE_ADD_OFFSET +
(i * cdata->channel_reg_size);
- res = platform_get_resource(pdev, IORESOURCE_IRQ, i);
- if (!res) {
- ret = -EINVAL;
+ irq = platform_get_irq(pdev, i);
+ if (irq < 0) {
+ ret = irq;
dev_err(&pdev->dev, "No irq resource for chan %d\n", i);
- goto err_irq;
+ goto err_pm_disable;
}
- tdc->irq = res->start;
+
snprintf(tdc->name, sizeof(tdc->name), "apbdma.%d", i);
- ret = request_irq(tdc->irq, tegra_dma_isr, 0, tdc->name, tdc);
+ ret = devm_request_irq(&pdev->dev, irq, tegra_dma_isr, 0,
+ tdc->name, tdc);
if (ret) {
dev_err(&pdev->dev,
"request_irq failed with err %d channel %d\n",
ret, i);
- goto err_irq;
+ goto err_pm_disable;
}
tdc->dma_chan.device = &tdma->dma_dev;
@@ -1517,7 +1517,7 @@ static int tegra_dma_probe(struct platform_device *pdev)
if (ret < 0) {
dev_err(&pdev->dev,
"Tegra20 APB DMA driver registration failed %d\n", ret);
- goto err_irq;
+ goto err_pm_disable;
}
ret = of_dma_controller_register(pdev->dev.of_node,
@@ -1534,13 +1534,7 @@ static int tegra_dma_probe(struct platform_device *pdev)
err_unregister_dma_dev:
dma_async_device_unregister(&tdma->dma_dev);
-err_irq:
- while (--i >= 0) {
- struct tegra_dma_channel *tdc = &tdma->channels[i];
-
- free_irq(tdc->irq, tdc);
- }
-
+err_pm_disable:
pm_runtime_disable(&pdev->dev);
if (!pm_runtime_status_suspended(&pdev->dev))
tegra_dma_runtime_suspend(&pdev->dev);
@@ -1550,16 +1544,9 @@ static int tegra_dma_probe(struct platform_device *pdev)
static int tegra_dma_remove(struct platform_device *pdev)
{
struct tegra_dma *tdma = platform_get_drvdata(pdev);
- int i;
- struct tegra_dma_channel *tdc;
dma_async_device_unregister(&tdma->dma_dev);
- for (i = 0; i < tdma->chip_data->nr_channels; ++i) {
- tdc = &tdma->channels[i];
- free_irq(tdc->irq, tdc);
- }
-
pm_runtime_disable(&pdev->dev);
if (!pm_runtime_status_suspended(&pdev->dev))
tegra_dma_runtime_suspend(&pdev->dev);
--
2.24.0
There is nothing arch-specific in the driver's code, so let's enable
compile-testing for the driver.
Acked-by: Jon Hunter <[email protected]>
Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/dma/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 98daf3aafdcc..7fc725d928b2 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -617,7 +617,7 @@ config TXX9_DMAC
config TEGRA20_APB_DMA
tristate "NVIDIA Tegra20 APB DMA support"
- depends on ARCH_TEGRA
+ depends on ARCH_TEGRA || COMPILE_TEST
select DMA_ENGINE
help
Support for the NVIDIA Tegra20 APB DMA controller driver. The
--
2.24.0
The runtime PM is always available on all Tegra SoCs since the commit
40b2bb1b132a ("ARM: tegra: enforce PM requirement"), so there is no
need to handle the case of unavailable RPM in the code anymore.
Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/dma/tegra20-apb-dma.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
index 7158bd3145c4..22b88ccff05d 100644
--- a/drivers/dma/tegra20-apb-dma.c
+++ b/drivers/dma/tegra20-apb-dma.c
@@ -1429,11 +1429,8 @@ 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))
- ret = tegra_dma_runtime_resume(&pdev->dev);
- else
- ret = pm_runtime_get_sync(&pdev->dev);
+ ret = pm_runtime_get_sync(&pdev->dev);
if (ret < 0)
goto err_pm_disable;
@@ -1546,8 +1543,6 @@ static int tegra_dma_probe(struct platform_device *pdev)
err_pm_disable:
pm_runtime_disable(&pdev->dev);
- if (!pm_runtime_status_suspended(&pdev->dev))
- tegra_dma_runtime_suspend(&pdev->dev);
return ret;
}
@@ -1557,10 +1552,7 @@ static int tegra_dma_remove(struct platform_device *pdev)
struct tegra_dma *tdma = platform_get_drvdata(pdev);
dma_async_device_unregister(&tdma->dma_dev);
-
pm_runtime_disable(&pdev->dev);
- if (!pm_runtime_status_suspended(&pdev->dev))
- tegra_dma_runtime_suspend(&pdev->dev);
return 0;
}
--
2.24.0
On 02/02/2020 22:28, Dmitry Osipenko wrote:
> The runtime PM is always available on all Tegra SoCs since the commit
> 40b2bb1b132a ("ARM: tegra: enforce PM requirement"), so there is no
> need to handle the case of unavailable RPM in the code anymore.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
> drivers/dma/tegra20-apb-dma.c | 10 +---------
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
> index 7158bd3145c4..22b88ccff05d 100644
> --- a/drivers/dma/tegra20-apb-dma.c
> +++ b/drivers/dma/tegra20-apb-dma.c
> @@ -1429,11 +1429,8 @@ 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))
> - ret = tegra_dma_runtime_resume(&pdev->dev);
> - else
> - ret = pm_runtime_get_sync(&pdev->dev);
>
> + ret = pm_runtime_get_sync(&pdev->dev);
> if (ret < 0)
> goto err_pm_disable;
>
> @@ -1546,8 +1543,6 @@ static int tegra_dma_probe(struct platform_device *pdev)
>
> err_pm_disable:
> pm_runtime_disable(&pdev->dev);
> - if (!pm_runtime_status_suspended(&pdev->dev))
> - tegra_dma_runtime_suspend(&pdev->dev);
>
> return ret;
> }
> @@ -1557,10 +1552,7 @@ static int tegra_dma_remove(struct platform_device *pdev)
> struct tegra_dma *tdma = platform_get_drvdata(pdev);
>
> dma_async_device_unregister(&tdma->dma_dev);
> -
> pm_runtime_disable(&pdev->dev);
> - if (!pm_runtime_status_suspended(&pdev->dev))
> - tegra_dma_runtime_suspend(&pdev->dev);
>
> return 0;
> }
>
Acked-by: Jon Hunter <[email protected]>
Cheers
Jon
--
nvpublic