NVIDIA Tegra APB DMA driver fixes and improvements
Subsystems (3)
v8 cover NVIDIA Tegra APB DMA driver fixes and improvements
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:
v8: - Added RPM-put after every tegra_dma_stop(), like it was suggested by
Jon Hunter in a review comments to v6/v7. Dropped this patch in a result:
dmaengine: tegra-apb: Don't stop cyclic DMA in a case of error condition
- Squashed all removals of duplicated pending_sg_req-list-empty checks into a single patch and added another removal which was suggested by Jon Hunter during review of v7:
dmaengine: tegra-apb: Remove duplicated pending_sg_req checks
- Added patch to improve error message about DMA underflow condition, which was suggested by Jon Hunter in a review comment to v7:
dmaengine: tegra-apb: Improve error message about DMA underflow
- Added another new minor cleanup-patch:
dmaengine: tegra-apb: Remove unused function argument
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 duplicated pending_sg_req checks
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
dmaengine: tegra-apb: Remove unused function argument
dmaengine: tegra-apb: Improve error message about DMA underflow
drivers/dma/Kconfig | 4 +- drivers/dma/tegra20-apb-dma.c | 514 +++++++++++++++++----------------- 2 files changed, 255 insertions(+), 263 deletions(-)
Signature · 1 line
2.24.0
PATCH v8 1/19 dmaengine: tegra-apb: Fix use-after-free
I was doing some experiments with I2C and noticed that Tegra APB DMA driver crashes sometime after I2C DMA transfer termination. The crash happens because tegra_dma_terminate_all() bails out immediately if pending list is empty, and thus, it doesn't release the half-completed descriptors which are getting re-used before ISR tasklet kicks-in.
tegra-i2c 7000c400.i2c: DMA transfer timeout elants_i2c 0-0010: elants_i2c_irq: failed to read data: -110
------------[ cut here ]------------ WARNING: CPU: 0 PID: 142 at lib/list_debug.c:45 __list_del_entry_valid+0x45/0xac list_del corruption, ddbaac44->next is LIST_POISON1 (00000100) Modules linked in: CPU: 0 PID: 142 Comm: kworker/0:2 Not tainted 5.5.0-rc2-next-20191220-00175-gc3605715758d-dirty #538 Hardware name: NVIDIA Tegra SoC (Flattened Device Tree) Workqueue: events_freezable_power_ thermal_zone_device_check [<c010e5c5>] (unwind_backtrace) from [<c010a1c5>] (show_stack+0x11/0x14) [<c010a1c5>] (show_stack) from [<c0973925>] (dump_stack+0x85/0x94) [<c0973925>] (dump_stack) from [<c011f529>] (__warn+0xc1/0xc4) [<c011f529>] (__warn) from [<c011f7e9>] (warn_slowpath_fmt+0x61/0x78) [<c011f7e9>] (warn_slowpath_fmt) from [<c042497d>] (__list_del_entry_valid+0x45/0xac) [<c042497d>] (__list_del_entry_valid) from [<c047a87f>] (tegra_dma_tasklet+0x5b/0x154) [<c047a87f>] (tegra_dma_tasklet) from [<c0124799>] (tasklet_action_common.constprop.0+0x41/0x7c) [<c0124799>] (tasklet_action_common.constprop.0) from [<c01022ab>] (__do_softirq+0xd3/0x2a8) [<c01022ab>] (__do_softirq) from [<c0124683>] (irq_exit+0x7b/0x98) [<c0124683>] (irq_exit) from [<c0168c19>] (__handle_domain_irq+0x45/0x80) [<c0168c19>] (__handle_domain_irq) from [<c043e429>] (gic_handle_irq+0x45/0x7c) [<c043e429>] (gic_handle_irq) from [<c0101aa5>] (__irq_svc+0x65/0x94) Exception stack(0xde2ebb90 to 0xde2ebbd8)
--- drivers/dma/tegra20-apb-dma.c | 4 ---- 1 file changed, 4 deletions(-)
drivers/dma/tegra20-apb-dma.c +0 -4
@@ -756,10 +756,6 @@ static int tegra_dma_terminate_all(struct dma_chan *dc)bool was_busy;spin_lock_irqsave (&tdc->lock, flags); -if (list_empty (&tdc->pending_sg_req)) { -spin_unlock_irqrestore (&tdc->lock, flags); -return 0 ; - }if (!tdc->busy) goto skip_dma_stop;
Signature · 1 line
2.24.0
PATCH v8 2/19 dmaengine: tegra-apb: Prevent race conditions of tasklet vs free list
In reply to Dmitry OsipenkoThe interrupt handler puts a half-completed DMA descriptor on a free list and then schedules tasklet to process bottom half of the descriptor that executes client's callback, this creates possibility to pick up the busy descriptor from the free list. Thus, let's disallow descriptor's re-use until it is fully processed.
--- drivers/dma/tegra20-apb-dma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/dma/tegra20-apb-dma.c +1 -1
@@ -281,7 +281,7 @@ static struct tegra_dma_desc *tegra_dma_desc_get(/* Do not allocate if desc are waiting for ack */ list_for_each_entry (dma_desc, &tdc->free_dma_desc, node) { -if (async_tx_test_ack (&dma_desc->txd)) { +if (async_tx_test_ack (&dma_desc->txd) && !dma_desc->cb_count) {list_del (&dma_desc->node);spin_unlock_irqrestore (&tdc->lock, flags); dma_desc->txd.flags =0 ;
Signature · 1 line
2.24.0
PATCH v8 3/19 dmaengine: tegra-apb: Implement synchronization hook
In reply to Dmitry OsipenkoThe ISR tasklet could be kept scheduled after DMA transfer termination, let's add synchronization hook which blocks until tasklet is finished.
--- drivers/dma/tegra20-apb-dma.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/dma/tegra20-apb-dma.c +8 -0
@@ -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;
Signature · 1 line
2.24.0
PATCH v8 4/19 dmaengine: tegra-apb: Prevent race conditions on channel's freeing
In reply to Dmitry OsipenkoIt's incorrect to check the channel's "busy" state without taking a lock. That shouldn't cause any real troubles, nevertheless it's always better not to have any race conditions in the code.
--- drivers/dma/tegra20-apb-dma.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/dma/tegra20-apb-dma.c +1 -2
@@ -1294,8 +1294,7 @@ static void tegra_dma_free_chan_resources(struct dma_chan *dc)dev_dbg (tdc2dev (tdc),"Freeing channel %d\n" , tdc->id); -if (tdc->busy) -tegra_dma_terminate_all (dc); +tegra_dma_terminate_all (dc);spin_lock_irqsave (&tdc->lock, flags);list_splice_init (&tdc->pending_sg_req, &sg_req_list);
Signature · 1 line
2.24.0
PATCH v8 5/19 dmaengine: tegra-apb: Clean up tasklet releasing
In reply to Dmitry OsipenkoThere is no need to kill tasklet when driver's probe fails because tasklet can't be scheduled at this time. It is also cleaner to kill tasklet on channel's freeing rather than to kill it on driver's removal, otherwise tasklet could perform a dummy execution after channel's releasing, which isn't very nice.
--- drivers/dma/tegra20-apb-dma.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
drivers/dma/tegra20-apb-dma.c +1 -5
@@ -1287,7 +1287,6 @@ static void tegra_dma_free_chan_resources(struct dma_chan *dc)struct tegra_dma_sg_req *sg_req;struct list_head dma_desc_list;struct list_head sg_req_list; -unsigned long flags;INIT_LIST_HEAD (&dma_desc_list);INIT_LIST_HEAD (&sg_req_list);
@@ -1295,15 +1294,14 @@ static void tegra_dma_free_chan_resources(struct dma_chan *dc)dev_dbg (tdc2dev (tdc),"Freeing channel %d\n" , tdc->id);tegra_dma_terminate_all (dc); +tasklet_kill (&tdc->tasklet); -spin_lock_irqsave (&tdc->lock, flags);list_splice_init (&tdc->pending_sg_req, &sg_req_list);list_splice_init (&tdc->free_sg_req, &sg_req_list);list_splice_init (&tdc->free_dma_desc, &dma_desc_list);INIT_LIST_HEAD (&tdc->cb_desc); tdc->config_init = false; tdc->isr_handler =NULL ; -spin_unlock_irqrestore (&tdc->lock, flags);while (!list_empty (&dma_desc_list)) { dma_desc = list_first_entry(&dma_desc_list,
@@ -1542,7 +1540,6 @@ static int tegra_dma_probe(struct platform_device *pdev)struct tegra_dma_channel *tdc = &tdma->channels[i];free_irq (tdc->irq, tdc); -tasklet_kill (&tdc->tasklet); }pm_runtime_disable (&pdev->dev);
@@ -1562,7 +1559,6 @@ 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); }pm_runtime_disable (&pdev->dev);
Signature · 1 line
2.24.0
PATCH v8 6/19 dmaengine: tegra-apb: Use devm_platform_ioremap_resource
In reply to Dmitry OsipenkoUse devm_platform_ioremap_resource to keep code cleaner a tad.
--- drivers/dma/tegra20-apb-dma.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/dma/tegra20-apb-dma.c +1 -2
@@ -1402,8 +1402,7 @@ static int tegra_dma_probe(struct platform_device *pdev) tdma->chip_data = cdata;platform_set_drvdata (pdev, tdma); - res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); - tdma->base_addr =devm_ioremap_resource (&pdev->dev, res); + tdma->base_addr =devm_platform_ioremap_resource (pdev,0 );if (IS_ERR (tdma->base_addr))return PTR_ERR (tdma->base_addr);
Signature · 1 line
2.24.0
PATCH v8 7/19 dmaengine: tegra-apb: Use devm_request_irq
In reply to Dmitry OsipenkoUse resource-managed variant of request_irq for brevity.
--- drivers/dma/tegra20-apb-dma.c | 35 +++++++++++------------------------ 1 file changed, 11 insertions(+), 24 deletions(-)
drivers/dma/tegra20-apb-dma.c +11 -24
@@ -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);
Signature · 1 line
2.24.0
PATCH v8 8/19 dmaengine: tegra-apb: Fix coding style problems
In reply to Dmitry OsipenkoThis patch fixes few dozens of coding style problems reported by checkpatch and prettifies code where makes sense.
--- drivers/dma/tegra20-apb-dma.c | 275 ++++++++++++++++++---------------- 1 file changed, 144 insertions(+), 131 deletions(-)
drivers/dma/tegra20-apb-dma.c +144 -131
@@ -59,7 +59,7 @@#define TEGRA_APBDMA_STATUS_COUNT_MASK 0xFFFC#define TEGRA_APBDMA_CHAN_CSRE 0x00C -#define TEGRA_APBDMA_CHAN_CSRE_PAUSE (1 << 31) +#define TEGRA_APBDMA_CHAN_CSRE_PAUSE BIT(31)/* AHB memory address */ #define TEGRA_APBDMA_CHAN_AHBPTR 0x010
@@ -120,21 +120,21 @@ struct tegra_dma;* @support_separate_wcount_reg: Support separate word count register. */ struct tegra_dma_chip_data { -int nr_channels; -int channel_reg_size; -int max_dma_count; +unsigned int nr_channels; +unsigned int channel_reg_size; +unsigned int max_dma_count;bool support_channel_pause;bool support_separate_wcount_reg; };/* DMA channel registers */ struct tegra_dma_channel_regs { -unsigned long csr; -unsigned long ahb_ptr; -unsigned long apb_ptr; -unsigned long ahb_seq; -unsigned long apb_seq; -unsigned long wcount; +u32 csr; +u32 ahb_ptr; +u32 apb_ptr; +u32 ahb_seq; +u32 apb_seq; +u32 wcount; };/*
@@ -168,7 +168,7 @@ struct tegra_dma_desc {struct list_head node;struct list_head tx_list;struct list_head cb_node; -int cb_count; +unsigned int cb_count; };struct tegra_dma_channel ;
@@ -181,7 +181,7 @@ struct tegra_dma_channel {struct dma_chan dma_chan;char name[12 ];bool config_init; -int id; +unsigned int id;void __iomem *chan_addr;spinlock_t lock;bool busy;
@@ -201,7 +201,7 @@ struct tegra_dma_channel {/* Channel-slave specific configuration */ unsigned int slave_id;struct dma_slave_config dma_sconfig; -struct tegra_dma_channel_regs channel_reg; +struct tegra_dma_channel_regs channel_reg; };/* tegra_dma: Tegra DMA specific information */
@@ -239,7 +239,7 @@ static inline u32 tdma_read(struct tegra_dma *tdma, u32 reg) }static inline void tdc_write (struct tegra_dma_channel *tdc, -u32 reg,u32 val) +u32 reg,u32 val) {writel (val, tdc->chan_addr + reg); }
@@ -254,8 +254,8 @@ static inline struct tegra_dma_channel *to_tegra_dma_chan(struct dma_chan *dc) return container_of(dc, struct tegra_dma_channel, dma_chan); } -static inline struct tegra_dma_desc *txd_to_tegra_dma_desc ( -struct dma_async_tx_descriptor *td) +static inline struct tegra_dma_desc * +txd_to_tegra_dma_desc (struct dma_async_tx_descriptor *td) { return container_of(td, struct tegra_dma_desc, txd); }
@@ -270,8 +270,7 @@ static int tegra_dma_runtime_suspend(struct device *dev);static int tegra_dma_runtime_resume (struct device *dev);/* Get DMA desc from free list, if not there then allocate it. */ -static struct tegra_dma_desc *tegra_dma_desc_get ( -struct tegra_dma_channel *tdc) +static struct tegra_dma_desc *tegra_dma_desc_get (struct tegra_dma_channel *tdc) {struct tegra_dma_desc *dma_desc;unsigned long flags;
@@ -298,11 +297,12 @@ static struct tegra_dma_desc *tegra_dma_desc_get(dma_async_tx_descriptor_init (&dma_desc->txd, &tdc->dma_chan); dma_desc->txd.tx_submit = tegra_dma_tx_submit; dma_desc->txd.flags =0 ; +return dma_desc; }static void tegra_dma_desc_put (struct tegra_dma_channel *tdc, -struct tegra_dma_desc *dma_desc) +struct tegra_dma_desc *dma_desc) {unsigned long flags;
@@ -313,29 +313,29 @@ static void tegra_dma_desc_put(struct tegra_dma_channel *tdc,spin_unlock_irqrestore (&tdc->lock, flags); } -static struct tegra_dma_sg_req *tegra_dma_sg_req_get ( -struct tegra_dma_channel *tdc) +static struct tegra_dma_sg_req * +tegra_dma_sg_req_get (struct tegra_dma_channel *tdc) { -struct tegra_dma_sg_req *sg_req =NULL ; +struct tegra_dma_sg_req *sg_req;unsigned long flags;spin_lock_irqsave (&tdc->lock, flags);if (!list_empty (&tdc->free_sg_req)) { - sg_req =list_first_entry (&tdc->free_sg_req, -typeof (*sg_req), node); + sg_req =list_first_entry (&tdc->free_sg_req,typeof (*sg_req), + node);list_del (&sg_req->node);spin_unlock_irqrestore (&tdc->lock, flags);return sg_req; }spin_unlock_irqrestore (&tdc->lock, flags); - sg_req =kzalloc (sizeof (struct tegra_dma_sg_req ),GFP_NOWAIT ); + sg_req =kzalloc (sizeof (*sg_req),GFP_NOWAIT );return sg_req; }static int tegra_dma_slave_config (struct dma_chan *dc, -struct dma_slave_config *sconfig) +struct dma_slave_config *sconfig) {struct tegra_dma_channel *tdc =to_tegra_dma_chan (dc);
@@ -352,11 +352,12 @@ static int tegra_dma_slave_config(struct dma_chan *dc, tdc->slave_id = sconfig->slave_id; } tdc->config_init = true; +return 0 ; }static void tegra_dma_global_pause (struct tegra_dma_channel *tdc, -bool wait_for_burst_complete) +bool wait_for_burst_complete) {struct tegra_dma *tdma = tdc->tdma;
@@ -391,13 +392,13 @@ static void tegra_dma_global_resume(struct tegra_dma_channel *tdc) }static void tegra_dma_pause (struct tegra_dma_channel *tdc, -bool wait_for_burst_complete) +bool wait_for_burst_complete) {struct tegra_dma *tdma = tdc->tdma;if (tdma->chip_data->support_channel_pause) {tdc_write (tdc,TEGRA_APBDMA_CHAN_CSRE , -TEGRA_APBDMA_CHAN_CSRE_PAUSE ); +TEGRA_APBDMA_CHAN_CSRE_PAUSE );if (wait_for_burst_complete)udelay (TEGRA_APBDMA_BURST_COMPLETE_TIME ); }else {
@@ -409,17 +410,15 @@ static void tegra_dma_resume(struct tegra_dma_channel *tdc) {struct tegra_dma *tdma = tdc->tdma; -if (tdma->chip_data->support_channel_pause) { +if (tdma->chip_data->support_channel_pause)tdc_write (tdc,TEGRA_APBDMA_CHAN_CSRE ,0 ); - }else { +else tegra_dma_global_resume (tdc); - } }static void tegra_dma_stop (struct tegra_dma_channel *tdc) { -u32 csr; -u32 status; +u32 csr, status;/* Disable interrupts */ csr =tdc_read (tdc,TEGRA_APBDMA_CHAN_CSR );
@@ -440,7 +439,7 @@ static void tegra_dma_stop(struct tegra_dma_channel *tdc) }static void tegra_dma_start (struct tegra_dma_channel *tdc, -struct tegra_dma_sg_req *sg_req) +struct tegra_dma_sg_req *sg_req) {struct tegra_dma_channel_regs *ch_regs = &sg_req->ch_regs;
@@ -454,11 +453,11 @@ static void tegra_dma_start(struct tegra_dma_channel *tdc,/* Start DMA */ tdc_write (tdc,TEGRA_APBDMA_CHAN_CSR , - ch_regs->csr |TEGRA_APBDMA_CSR_ENB ); + ch_regs->csr |TEGRA_APBDMA_CSR_ENB ); }static void tegra_dma_configure_for_next (struct tegra_dma_channel *tdc, -struct tegra_dma_sg_req *nsg_req) +struct tegra_dma_sg_req *nsg_req) {unsigned long status;
@@ -492,9 +491,9 @@ static void tegra_dma_configure_for_next(struct tegra_dma_channel *tdc,tdc_write (tdc,TEGRA_APBDMA_CHAN_AHBPTR , nsg_req->ch_regs.ahb_ptr);if (tdc->tdma->chip_data->support_separate_wcount_reg)tdc_write (tdc,TEGRA_APBDMA_CHAN_WCOUNT , - nsg_req->ch_regs.wcount); + nsg_req->ch_regs.wcount);tdc_write (tdc,TEGRA_APBDMA_CHAN_CSR , - nsg_req->ch_regs.csr |TEGRA_APBDMA_CSR_ENB ); + nsg_req->ch_regs.csr |TEGRA_APBDMA_CSR_ENB ); nsg_req->configured = true; nsg_req->words_xferred =0 ;
@@ -508,8 +507,7 @@ static void tdc_start_head_req(struct tegra_dma_channel *tdc)if (list_empty (&tdc->pending_sg_req))return ; - sg_req =list_first_entry (&tdc->pending_sg_req, -typeof (*sg_req), node); + sg_req =list_first_entry (&tdc->pending_sg_req,typeof (*sg_req), node);tegra_dma_start (tdc, sg_req); sg_req->configured = true; sg_req->words_xferred =0 ;
@@ -518,34 +516,35 @@ static void tdc_start_head_req(struct tegra_dma_channel *tdc)static void tdc_configure_next_head_desc (struct tegra_dma_channel *tdc) { -struct tegra_dma_sg_req *hsgreq; -struct tegra_dma_sg_req *hnsgreq; +struct tegra_dma_sg_req *hsgreq, *hnsgreq;if (list_empty (&tdc->pending_sg_req))return ; hsgreq =list_first_entry (&tdc->pending_sg_req,typeof (*hsgreq), node);if (!list_is_last (&hsgreq->node, &tdc->pending_sg_req)) { - hnsgreq =list_first_entry (&hsgreq->node, -typeof (*hnsgreq), node); + hnsgreq =list_first_entry (&hsgreq->node,typeof (*hnsgreq), + node);tegra_dma_configure_for_next (tdc, hnsgreq); } } -static inline int get_current_xferred_count (struct tegra_dma_channel *tdc, -struct tegra_dma_sg_req *sg_req,unsigned long status) +static inline unsigned int +get_current_xferred_count (struct tegra_dma_channel *tdc, +struct tegra_dma_sg_req *sg_req, +unsigned long status) {return sg_req->req_len - (status &TEGRA_APBDMA_STATUS_COUNT_MASK ) -4 ; }static void tegra_dma_abort_all (struct tegra_dma_channel *tdc) { -struct tegra_dma_sg_req *sgreq;struct tegra_dma_desc *dma_desc; +struct tegra_dma_sg_req *sgreq;while (!list_empty (&tdc->pending_sg_req)) { - sgreq =list_first_entry (&tdc->pending_sg_req, -typeof (*sgreq), node); + sgreq =list_first_entry (&tdc->pending_sg_req,typeof (*sgreq), + node);list_move_tail (&sgreq->node, &tdc->free_sg_req);if (sgreq->last_sg) { dma_desc = sgreq->dma_desc;
@@ -555,7 +554,7 @@ static void tegra_dma_abort_all(struct tegra_dma_channel *tdc)/* Add in cb list if it is not there. */ if (!dma_desc->cb_count)list_add_tail (&dma_desc->cb_node, - &tdc->cb_desc); + &tdc->cb_desc); dma_desc->cb_count++; } }
@@ -563,9 +562,10 @@ static void tegra_dma_abort_all(struct tegra_dma_channel *tdc) }static bool handle_continuous_head_request (struct tegra_dma_channel *tdc, -struct tegra_dma_sg_req *last_sg_req,bool to_terminate) +struct tegra_dma_sg_req *last_sg_req, +bool to_terminate) { -struct tegra_dma_sg_req *hsgreq =NULL ; +struct tegra_dma_sg_req *hsgreq;if (list_empty (&tdc->pending_sg_req)) {dev_err (tdc2dev (tdc),"DMA is running without req\n" );
@@ -589,14 +589,15 @@ static bool handle_continuous_head_request(struct tegra_dma_channel *tdc,/* Configure next request */ if (!to_terminate)tdc_configure_next_head_desc (tdc); +return true; }static void handle_once_dma_done (struct tegra_dma_channel *tdc, -bool to_terminate) +bool to_terminate) { -struct tegra_dma_sg_req *sgreq;struct tegra_dma_desc *dma_desc; +struct tegra_dma_sg_req *sgreq; tdc->busy = false; sgreq =list_first_entry (&tdc->pending_sg_req,typeof (*sgreq), node);
@@ -622,10 +623,10 @@ static void handle_once_dma_done(struct tegra_dma_channel *tdc, }static void handle_cont_sngl_cycle_dma_done (struct tegra_dma_channel *tdc, -bool to_terminate) +bool to_terminate) { -struct tegra_dma_sg_req *sgreq;struct tegra_dma_desc *dma_desc; +struct tegra_dma_sg_req *sgreq;bool st; sgreq =list_first_entry (&tdc->pending_sg_req,typeof (*sgreq), node);
@@ -657,13 +658,13 @@ static void tegra_dma_tasklet(unsigned long data)struct tegra_dma_channel *tdc = (struct tegra_dma_channel *)data;struct dmaengine_desc_callback cb;struct tegra_dma_desc *dma_desc; +unsigned int cb_count;unsigned long flags; -int cb_count;spin_lock_irqsave (&tdc->lock, flags);while (!list_empty (&tdc->cb_desc)) { - dma_desc =list_first_entry (&tdc->cb_desc, -typeof (*dma_desc), cb_node); + dma_desc =list_first_entry (&tdc->cb_desc,typeof (*dma_desc), + cb_node);list_del (&dma_desc->cb_node);dmaengine_desc_get_callback (&dma_desc->txd, &cb); cb_count = dma_desc->cb_count;
@@ -681,8 +682,8 @@ static void tegra_dma_tasklet(unsigned long data)static irqreturn_t tegra_dma_isr (int irq,void *dev_id) {struct tegra_dma_channel *tdc = dev_id; -unsigned long status;unsigned long flags; +u32 status;spin_lock_irqsave (&tdc->lock, flags);
@@ -697,8 +698,9 @@ static irqreturn_t tegra_dma_isr(int irq, void *dev_id) }spin_unlock_irqrestore (&tdc->lock, flags); -dev_info (tdc2dev (tdc), -"Interrupt already served status 0x%08lx\n" , status); +dev_info (tdc2dev (tdc),"Interrupt already served status 0x%08x\n" , + status); +return IRQ_NONE ; }
@@ -714,6 +716,7 @@ static dma_cookie_t tegra_dma_tx_submit(struct dma_async_tx_descriptor *txd) cookie =dma_cookie_assign (&dma_desc->txd);list_splice_tail_init (&dma_desc->tx_list, &tdc->pending_sg_req);spin_unlock_irqrestore (&tdc->lock, flags); +return cookie; }
@@ -747,11 +750,10 @@ static void tegra_dma_issue_pending(struct dma_chan *dc)static int tegra_dma_terminate_all (struct dma_chan *dc) {struct tegra_dma_channel *tdc =to_tegra_dma_chan (dc); -struct tegra_dma_sg_req *sgreq;struct tegra_dma_desc *dma_desc; +struct tegra_dma_sg_req *sgreq;unsigned long flags; -unsigned long status; -unsigned long wcount; +u32 status, wcount;bool was_busy;spin_lock_irqsave (&tdc->lock, flags);
@@ -777,8 +779,8 @@ static int tegra_dma_terminate_all(struct dma_chan *dc)tegra_dma_stop (tdc);if (!list_empty (&tdc->pending_sg_req) && was_busy) { - sgreq =list_first_entry (&tdc->pending_sg_req, -typeof (*sgreq), node); + sgreq =list_first_entry (&tdc->pending_sg_req,typeof (*sgreq), + node); sgreq->dma_desc->bytes_transferred +=get_current_xferred_count (tdc, sgreq, wcount); }
@@ -788,12 +790,13 @@ static int tegra_dma_terminate_all(struct dma_chan *dc)tegra_dma_abort_all (tdc);while (!list_empty (&tdc->cb_desc)) { - dma_desc =list_first_entry (&tdc->cb_desc, -typeof (*dma_desc), cb_node); + dma_desc =list_first_entry (&tdc->cb_desc,typeof (*dma_desc), + cb_node);list_del (&dma_desc->cb_node); dma_desc->cb_count =0 ; }spin_unlock_irqrestore (&tdc->lock, flags); +return 0 ; }
@@ -807,7 +810,7 @@ static void tegra_dma_synchronize(struct dma_chan *dc)static unsigned int tegra_dma_sg_bytes_xferred (struct tegra_dma_channel *tdc,struct tegra_dma_sg_req *sg_req) { -unsigned long status, wcount =0 ; +u32 status, wcount =0 ;if (!list_is_first (&sg_req->node, &tdc->pending_sg_req))return 0 ;
@@ -864,7 +867,8 @@ static unsigned int tegra_dma_sg_bytes_xferred(struct tegra_dma_channel *tdc, }static enum dma_status tegra_dma_tx_status (struct dma_chan *dc, -dma_cookie_t cookie,struct dma_tx_state *txstate) +dma_cookie_t cookie, +struct dma_tx_state *txstate) {struct tegra_dma_channel *tdc =to_tegra_dma_chan (dc);struct tegra_dma_desc *dma_desc;
@@ -911,11 +915,12 @@ static enum dma_status tegra_dma_tx_status(struct dma_chan *dc,trace_tegra_dma_tx_status (&tdc->dma_chan, cookie, txstate);spin_unlock_irqrestore (&tdc->lock, flags); +return ret; } -static inline int get_bus_width (struct tegra_dma_channel *tdc, -enum dma_slave_buswidth slave_bw) +static inline unsigned int get_bus_width (struct tegra_dma_channel *tdc, +enum dma_slave_buswidth slave_bw) {switch (slave_bw) {case DMA_SLAVE_BUSWIDTH_1_BYTE :
@@ -928,16 +933,17 @@ static inline int get_bus_width(struct tegra_dma_channel *tdc,return TEGRA_APBDMA_APBSEQ_BUS_WIDTH_64 ;default :dev_warn (tdc2dev (tdc), -"slave bw is not supported, using 32bits\n" ); +"slave bw is not supported, using 32bits\n" );return TEGRA_APBDMA_APBSEQ_BUS_WIDTH_32 ; } } -static inline int get_burst_size (struct tegra_dma_channel *tdc, -u32 burst_size,enum dma_slave_buswidth slave_bw,int len) +static inline unsigned int get_burst_size (struct tegra_dma_channel *tdc, +u32 burst_size, +enum dma_slave_buswidth slave_bw, +u32 len) { -int burst_byte; -int burst_ahb_width; +unsigned int burst_byte, burst_ahb_width;/* * burst_size from client is in terms of the bus_width.
@@ -964,9 +970,12 @@ static inline int get_burst_size(struct tegra_dma_channel *tdc, }static int get_transfer_param (struct tegra_dma_channel *tdc, -enum dma_transfer_direction direction,unsigned long *apb_addr, -unsigned long *apb_seq,unsigned long *csr,unsigned int *burst_size, -enum dma_slave_buswidth *slave_bw) +enum dma_transfer_direction direction, +u32 *apb_addr, +u32 *apb_seq, +u32 *csr, +unsigned int *burst_size, +enum dma_slave_buswidth *slave_bw) {switch (direction) {case DMA_MEM_TO_DEV :
@@ -987,13 +996,15 @@ static int get_transfer_param(struct tegra_dma_channel *tdc,default :dev_err (tdc2dev (tdc),"DMA direction is not supported\n" ); -return -EINVAL ; +break ; } +return -EINVAL ; }static void tegra_dma_prep_wcount (struct tegra_dma_channel *tdc, -struct tegra_dma_channel_regs *ch_regs,u32 len) +struct tegra_dma_channel_regs *ch_regs, +u32 len) {u32 len_field = (len -4 ) &0xFFFC ;
@@ -1003,20 +1014,23 @@ static void tegra_dma_prep_wcount(struct tegra_dma_channel *tdc, ch_regs->csr |= len_field; } -static struct dma_async_tx_descriptor *tegra_dma_prep_slave_sg ( -struct dma_chan *dc,struct scatterlist *sgl,unsigned int sg_len, -enum dma_transfer_direction direction,unsigned long flags, -void *context) +static struct dma_async_tx_descriptor * +tegra_dma_prep_slave_sg (struct dma_chan *dc, +struct scatterlist *sgl, +unsigned int sg_len, +enum dma_transfer_direction direction, +unsigned long flags, +void *context) {struct tegra_dma_channel *tdc =to_tegra_dma_chan (dc); +struct tegra_dma_sg_req *sg_req =NULL ; +u32 csr, ahb_seq, apb_ptr, apb_seq; +enum dma_slave_buswidth slave_bw;struct tegra_dma_desc *dma_desc; -unsigned int i; -struct scatterlist *sg; -unsigned long csr, ahb_seq, apb_ptr, apb_seq;struct list_head req_list; -struct tegra_dma_sg_req *sg_req =NULL ; -u32 burst_size; -enum dma_slave_buswidth slave_bw; +struct scatterlist *sg; +unsigned int burst_size; +unsigned int i;if (!tdc->config_init) {dev_err (tdc2dev (tdc),"DMA channel is not configured\n" );
@@ -1028,7 +1042,7 @@ static struct dma_async_tx_descriptor *tegra_dma_prep_slave_sg( }if (get_transfer_param (tdc, direction, &apb_ptr, &apb_seq, &csr, - &burst_size, &slave_bw) <0 ) + &burst_size, &slave_bw) <0 )return NULL ;INIT_LIST_HEAD (&req_list);
@@ -1074,7 +1088,7 @@ static struct dma_async_tx_descriptor *tegra_dma_prep_slave_sg( len =sg_dma_len (sg);if ((len &3 ) || (mem &3 ) || - (len > tdc->tdma->chip_data->max_dma_count)) { + len > tdc->tdma->chip_data->max_dma_count) {dev_err (tdc2dev (tdc),"DMA length/memory address is not supported\n" );tegra_dma_desc_put (tdc, dma_desc);
@@ -1126,20 +1140,21 @@ static struct dma_async_tx_descriptor *tegra_dma_prep_slave_sg(return &dma_desc->txd; } -static struct dma_async_tx_descriptor *tegra_dma_prep_dma_cyclic ( -struct dma_chan *dc,dma_addr_t buf_addr,size_t buf_len, -size_t period_len,enum dma_transfer_direction direction, -unsigned long flags) +static struct dma_async_tx_descriptor * +tegra_dma_prep_dma_cyclic (struct dma_chan *dc,dma_addr_t buf_addr, +size_t buf_len, +size_t period_len, +enum dma_transfer_direction direction, +unsigned long flags) {struct tegra_dma_channel *tdc =to_tegra_dma_chan (dc); -struct tegra_dma_desc *dma_desc =NULL ;struct tegra_dma_sg_req *sg_req =NULL ; -unsigned long csr, ahb_seq, apb_ptr, apb_seq; -int len; -size_t remain_len; -dma_addr_t mem = buf_addr; -u32 burst_size; +u32 csr, ahb_seq, apb_ptr, apb_seq;enum dma_slave_buswidth slave_bw; +struct tegra_dma_desc *dma_desc; +dma_addr_t mem = buf_addr; +unsigned int burst_size; +size_t len, remain_len;if (!buf_len || !period_len) {dev_err (tdc2dev (tdc),"Invalid buffer/period len\n" );
@@ -1173,13 +1188,13 @@ static struct dma_async_tx_descriptor *tegra_dma_prep_dma_cyclic( len = period_len;if ((len &3 ) || (buf_addr &3 ) || - (len > tdc->tdma->chip_data->max_dma_count)) { + len > tdc->tdma->chip_data->max_dma_count) {dev_err (tdc2dev (tdc),"Req len/mem address is not correct\n" );return NULL ; }if (get_transfer_param (tdc, direction, &apb_ptr, &apb_seq, &csr, - &burst_size, &slave_bw) <0 ) + &burst_size, &slave_bw) <0 )return NULL ; ahb_seq =TEGRA_APBDMA_AHBSEQ_INTR_ENB ;
@@ -1303,8 +1318,8 @@ static void tegra_dma_free_chan_resources(struct dma_chan *dc) tdc->isr_handler =NULL ;while (!list_empty (&dma_desc_list)) { - dma_desc =list_first_entry (&dma_desc_list, -typeof (*dma_desc), node); + dma_desc =list_first_entry (&dma_desc_list,typeof (*dma_desc), + node);list_del (&dma_desc->node);kfree (dma_desc); }
@@ -1323,8 +1338,8 @@ static struct dma_chan *tegra_dma_of_xlate(struct of_phandle_args *dma_spec, struct of_dma *ofdma) {struct tegra_dma *tdma = ofdma->of_dma_data; -struct dma_chan *chan;struct tegra_dma_channel *tdc; +struct dma_chan *chan;if (dma_spec->args[0 ] >TEGRA_APBDMA_CSR_REQ_SEL_MASK ) {dev_err (tdma->dev,"Invalid slave id: %d\n" , dma_spec->args[0 ]);
@@ -1379,20 +1394,16 @@ static const struct tegra_dma_chip_data tegra148_dma_chip_data = {static int tegra_dma_probe (struct platform_device *pdev) { +const struct tegra_dma_chip_data *cdata;struct tegra_dma *tdma; +unsigned int i; +size_t size;int ret; -int i; -const struct tegra_dma_chip_data *cdata; cdata =of_device_get_match_data (&pdev->dev); -if (!cdata) { -dev_err (&pdev->dev,"Error: No device match data found\n" ); -return -ENODEV ; - } + size =struct_size (tdma, channels, cdata->nr_channels); - tdma =devm_kzalloc (&pdev->dev, -struct_size (tdma, channels, cdata->nr_channels), -GFP_KERNEL ); + tdma =devm_kzalloc (&pdev->dev, size,GFP_KERNEL );if (!tdma)return -ENOMEM ;
@@ -1424,10 +1435,8 @@ static int tegra_dma_probe(struct platform_device *pdev)else ret =pm_runtime_get_sync (&pdev->dev); -if (ret <0 ) { -pm_runtime_disable (&pdev->dev); -return ret; - } +if (ret <0 ) + goto err_pm_disable;/* Reset DMA controller */ reset_control_assert (tdma->rst);
@@ -1470,13 +1479,13 @@ static int tegra_dma_probe(struct platform_device *pdev) tdc->dma_chan.device = &tdma->dma_dev;dma_cookie_init (&tdc->dma_chan);list_add_tail (&tdc->dma_chan.device_node, - &tdma->dma_dev.channels); + &tdma->dma_dev.channels); tdc->tdma = tdma; tdc->id = i; tdc->slave_id =TEGRA_APBDMA_SLAVE_ID_INVALID ;tasklet_init (&tdc->tasklet, tegra_dma_tasklet, - (unsigned long )tdc); + (unsigned long )tdc);spin_lock_init (&tdc->lock);INIT_LIST_HEAD (&tdc->pending_sg_req);
@@ -1528,16 +1537,19 @@ static int tegra_dma_probe(struct platform_device *pdev) goto err_unregister_dma_dev; } -dev_info (&pdev->dev,"Tegra20 APB DMA driver register %d channels\n" , - cdata->nr_channels); +dev_info (&pdev->dev,"Tegra20 APB DMA driver registered %u channels\n" , + cdata->nr_channels); +return 0 ; err_unregister_dma_dev:dma_async_device_unregister (&tdma->dma_dev); + err_pm_disable:pm_runtime_disable (&pdev->dev);if (!pm_runtime_status_suspended (&pdev->dev))tegra_dma_runtime_suspend (&pdev->dev); +return ret; }
@@ -1557,7 +1569,7 @@ static int tegra_dma_remove(struct platform_device *pdev)static int tegra_dma_runtime_suspend (struct device *dev) {struct tegra_dma *tdma =dev_get_drvdata (dev); -int i; +unsigned int i; tdma->reg_gen =tdma_read (tdma,TEGRA_APBDMA_GENERAL );for (i =0 ; i < tdma->chip_data->nr_channels; i++) {
@@ -1586,7 +1598,8 @@ static int tegra_dma_runtime_suspend(struct device *dev)static int tegra_dma_runtime_resume (struct device *dev) {struct tegra_dma *tdma =dev_get_drvdata (dev); -int i, ret; +unsigned int i; +int ret; ret =clk_prepare_enable (tdma->dma_clk);if (ret <0 ) {
@@ -1614,7 +1627,7 @@ static int tegra_dma_runtime_resume(struct device *dev)tdc_write (tdc,TEGRA_APBDMA_CHAN_AHBSEQ , ch_reg->ahb_seq);tdc_write (tdc,TEGRA_APBDMA_CHAN_AHBPTR , ch_reg->ahb_ptr);tdc_write (tdc,TEGRA_APBDMA_CHAN_CSR , - (ch_reg->csr & ~TEGRA_APBDMA_CSR_ENB )); + ch_reg->csr & ~TEGRA_APBDMA_CSR_ENB ); }return 0 ;
Signature · 1 line
2.24.0
PATCH v8 9/19 dmaengine: tegra-apb: Remove unneeded initialization of tdc->config_init
In reply to Dmitry OsipenkoThere is no need to re-initialize the already initialized variables. The tdc->config_init=false after driver's probe and after channel's freeing, so there is no need to re-initialize it on the channel's allocation.
--- drivers/dma/tegra20-apb-dma.c | 1 - 1 file changed, 1 deletion(-)
drivers/dma/tegra20-apb-dma.c +0 -1
@@ -1284,7 +1284,6 @@ static int tegra_dma_alloc_chan_resources(struct dma_chan *dc)int ret;dma_cookie_init (&tdc->dma_chan); - tdc->config_init = false; ret =pm_runtime_get_sync (tdma->dev);if (ret <0 )
Signature · 1 line
2.24.0
PATCH v8 10/19 dmaengine: tegra-apb: Remove assumptions about unavailable runtime PM
In reply to Dmitry OsipenkoThe 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.
--- drivers/dma/tegra20-apb-dma.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-)
drivers/dma/tegra20-apb-dma.c +1 -9
@@ -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 ; }
Signature · 1 line
2.24.0
PATCH v8 11/19 dmaengine: tegra-apb: Remove duplicated pending_sg_req checks
In reply to Dmitry OsipenkoThere are few place in the code which check whether pending_sg_req list is empty despite of the check already being done. Let's remove the duplicated checks to keep code clean.
--- drivers/dma/tegra20-apb-dma.c | 12 ------------ 1 file changed, 12 deletions(-)
drivers/dma/tegra20-apb-dma.c +0 -12
@@ -504,9 +504,6 @@ static void tdc_start_head_req(struct tegra_dma_channel *tdc) {struct tegra_dma_sg_req *sg_req; -if (list_empty (&tdc->pending_sg_req)) -return ; - sg_req =list_first_entry (&tdc->pending_sg_req,typeof (*sg_req), node);tegra_dma_start (tdc, sg_req); sg_req->configured = true;
@@ -518,9 +515,6 @@ static void tdc_configure_next_head_desc(struct tegra_dma_channel *tdc) {struct tegra_dma_sg_req *hsgreq, *hnsgreq; -if (list_empty (&tdc->pending_sg_req)) -return ; - hsgreq =list_first_entry (&tdc->pending_sg_req,typeof (*hsgreq), node);if (!list_is_last (&hsgreq->node, &tdc->pending_sg_req)) { hnsgreq = list_first_entry(&hsgreq->node, typeof(*hnsgreq),
@@ -567,12 +561,6 @@ static bool handle_continuous_head_request(struct tegra_dma_channel *tdc, {struct tegra_dma_sg_req *hsgreq; -if (list_empty (&tdc->pending_sg_req)) { -dev_err (tdc2dev (tdc),"DMA is running without req\n" ); -tegra_dma_stop (tdc); -return false; - } -/* * Check that head req on list should be in flight. * If it is not in flight then abort transfer as
Signature · 1 line
2.24.0
Re: [PATCH v8 11/19] dmaengine: tegra-apb: Remove duplicated pending_sg_req checks
Quoted 37 lines · Dmitry Osipenko
There are few place in the code which check whether
pending_sg_reqlist is empty despite of the check already being done. Let's remove the duplicated checks to keep code clean.Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/dma/tegra20-apb-dma.c | 12 ------------ 1 file changed, 12 deletions(-)drivers/dma/tegra20-apb-dma.c +0
-6@@ -504,9 +504,6 @@ static void tdc_start_head_req(struct tegra_dma_channel *tdc) {struct tegra_dma_sg_req *sg_req; -if (list_empty (&tdc->pending_sg_req)) -return ; - sg_req =list_first_entry (&tdc->pending_sg_req,typeof (*sg_req), node);tegra_dma_start (tdc, sg_req); sg_req->configured = true;@@ -518,9 +515,6 @@ static void tdc_configure_next_head_desc(struct tegra_dma_channel *tdc) {struct tegra_dma_sg_req *hsgreq, *hnsgreq; -if (list_empty (&tdc->pending_sg_req)) -return ; - hsgreq =list_first_entry (&tdc->pending_sg_req,typeof (*hsgreq), node);if (!list_is_last (&hsgreq->node, &tdc->pending_sg_req)) { hnsgreq = list_first_entry(&hsgreq->node, typeof(*hnsgreq),@@ -567,12 +561,6 @@ static bool handle_continuous_head_request(struct tegra_dma_channel *tdc, {struct tegra_dma_sg_req *hsgreq;
drivers/dma/tegra20-apb-dma.c static bool handle_continuous_head_request(struct tegra_dma_channel *tdc, · line 566
Quoted 10 lines
drivers/dma/tegra20-apb-dma.c +0
-6-if (list_empty (&tdc->pending_sg_req)) { -dev_err (tdc2dev (tdc),"DMA is running without req\n" ); -tegra_dma_stop (tdc); -return false; - } -/* * Check that head req on list should be in flight. * If it is not in flight then abort transfer as
Thanks
Jon
Signature · 1 line
nvpublic
PATCH v8 12/19 dmaengine: tegra-apb: Keep clock enabled only during of DMA transfer
In reply to Dmitry OsipenkoIt's a bit impractical to enable hardware's clock at the time of DMA channel's allocation because most of DMA client drivers allocate DMA channel at the time of the driver's probing, and thus, DMA clock is kept always-enabled in practice, defeating the whole purpose of runtime PM.
--- drivers/dma/tegra20-apb-dma.c | 36 ++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-)
drivers/dma/tegra20-apb-dma.c +25 -11
@@ -569,6 +569,7 @@ static bool handle_continuous_head_request(struct tegra_dma_channel *tdc, hsgreq =list_first_entry (&tdc->pending_sg_req,typeof (*hsgreq), node);if (!hsgreq->configured) {tegra_dma_stop (tdc); +pm_runtime_put (tdc->tdma->dev);dev_err (tdc2dev (tdc),"Error in DMA transfer, aborting DMA\n" );tegra_dma_abort_all (tdc);return false;
@@ -604,9 +605,14 @@ static void handle_once_dma_done(struct tegra_dma_channel *tdc,list_add_tail (&sgreq->node, &tdc->free_sg_req);/* Do not start DMA if it is going to be terminate */ -if (to_terminate ||list_empty (&tdc->pending_sg_req)) +if (to_terminate)return ; +if (list_empty (&tdc->pending_sg_req)) { +pm_runtime_put (tdc->tdma->dev); +return ; + } +tdc_start_head_req (tdc); }
@@ -712,6 +718,7 @@ static void tegra_dma_issue_pending(struct dma_chan *dc) {struct tegra_dma_channel *tdc =to_tegra_dma_chan (dc);unsigned long flags; +int err;spin_lock_irqsave (&tdc->lock, flags);if (list_empty (&tdc->pending_sg_req)) {
@@ -719,6 +726,12 @@ static void tegra_dma_issue_pending(struct dma_chan *dc) goto end; }if (!tdc->busy) { + err =pm_runtime_get_sync (tdc->tdma->dev); +if (err <0 ) { +dev_err (tdc2dev (tdc),"Failed to enable DMA\n" ); + goto end; + } +tdc_start_head_req (tdc);/* Continuous single mode: Configure next req */
@@ -774,6 +787,8 @@ static int tegra_dma_terminate_all(struct dma_chan *dc) }tegra_dma_resume (tdc); +pm_runtime_put (tdc->tdma->dev); + skip_dma_stop:tegra_dma_abort_all (tdc);
@@ -1268,22 +1283,15 @@ tegra_dma_prep_dma_cyclic(struct dma_chan *dc, dma_addr_t buf_addr,static int tegra_dma_alloc_chan_resources (struct dma_chan *dc) {struct tegra_dma_channel *tdc =to_tegra_dma_chan (dc); -struct tegra_dma *tdma = tdc->tdma; -int ret;dma_cookie_init (&tdc->dma_chan); - ret =pm_runtime_get_sync (tdma->dev); -if (ret <0 ) -return ret; -return 0 ; }static void tegra_dma_free_chan_resources (struct dma_chan *dc) {struct tegra_dma_channel *tdc =to_tegra_dma_chan (dc); -struct tegra_dma *tdma = tdc->tdma;struct tegra_dma_desc *dma_desc;struct tegra_dma_sg_req *sg_req;struct list_head dma_desc_list;
@@ -1316,7 +1324,6 @@ static void tegra_dma_free_chan_resources(struct dma_chan *dc)list_del (&sg_req->node);kfree (sg_req); } -pm_runtime_put (tdma->dev); tdc->slave_id =TEGRA_APBDMA_SLAVE_ID_INVALID ; }
@@ -1416,6 +1423,11 @@ static int tegra_dma_probe(struct platform_device *pdev)spin_lock_init (&tdma->global_lock); + ret =clk_prepare (tdma->dma_clk); +if (ret) +return ret; + +pm_runtime_irq_safe (&pdev->dev);pm_runtime_enable (&pdev->dev); ret =pm_runtime_get_sync (&pdev->dev);
@@ -1531,6 +1543,7 @@ static int tegra_dma_probe(struct platform_device *pdev) err_pm_disable:pm_runtime_disable (&pdev->dev); +clk_unprepare (tdma->dma_clk);return ret; }
@@ -1541,6 +1554,7 @@ static int tegra_dma_remove(struct platform_device *pdev)dma_async_device_unregister (&tdma->dma_dev);pm_runtime_disable (&pdev->dev); +clk_unprepare (tdma->dma_clk);return 0 ; }
@@ -1569,7 +1583,7 @@ static int tegra_dma_runtime_suspend(struct device *dev) TEGRA_APBDMA_CHAN_WCOUNT); } -clk_disable_unprepare (tdma->dma_clk); +clk_disable (tdma->dma_clk);return 0 ; }
@@ -1580,7 +1594,7 @@ static int tegra_dma_runtime_resume(struct device *dev)unsigned int i;int ret; - ret =clk_prepare_enable (tdma->dma_clk); + ret =clk_enable (tdma->dma_clk);if (ret <0 ) {dev_err (dev,"clk_enable failed: %d\n" , ret);return ret;
Signature · 1 line
2.24.0
Re: [PATCH v8 12/19] dmaengine: tegra-apb: Keep clock enabled only during of DMA transfer
Quoted 138 lines · Dmitry Osipenko
It's a bit impractical to enable hardware's clock at the time of DMA channel's allocation because most of DMA client drivers allocate DMA channel at the time of the driver's probing, and thus, DMA clock is kept always-enabled in practice, defeating the whole purpose of runtime PM.
Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/dma/tegra20-apb-dma.c | 36 ++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-)drivers/dma/tegra20-apb-dma.c +24
-10@@ -569,6 +569,7 @@ static bool handle_continuous_head_request(struct tegra_dma_channel *tdc, hsgreq =list_first_entry (&tdc->pending_sg_req,typeof (*hsgreq), node);if (!hsgreq->configured) {tegra_dma_stop (tdc); +pm_runtime_put (tdc->tdma->dev);dev_err (tdc2dev (tdc),"Error in DMA transfer, aborting DMA\n" );tegra_dma_abort_all (tdc);return false;@@ -604,9 +605,14 @@ static void handle_once_dma_done(struct tegra_dma_channel *tdc,list_add_tail (&sgreq->node, &tdc->free_sg_req);/* Do not start DMA if it is going to be terminate */ -if (to_terminate ||list_empty (&tdc->pending_sg_req)) +if (to_terminate)return ; +if (list_empty (&tdc->pending_sg_req)) { +pm_runtime_put (tdc->tdma->dev); +return ; + } +tdc_start_head_req (tdc); }@@ -712,6 +718,7 @@ static void tegra_dma_issue_pending(struct dma_chan *dc) {struct tegra_dma_channel *tdc =to_tegra_dma_chan (dc);unsigned long flags; +int err;spin_lock_irqsave (&tdc->lock, flags);if (list_empty (&tdc->pending_sg_req)) {@@ -719,6 +726,12 @@ static void tegra_dma_issue_pending(struct dma_chan *dc) goto end; }if (!tdc->busy) { + err =pm_runtime_get_sync (tdc->tdma->dev); +if (err <0 ) { +dev_err (tdc2dev (tdc),"Failed to enable DMA\n" ); + goto end; + } +tdc_start_head_req (tdc);/* Continuous single mode: Configure next req */ @@ -774,6 +787,8 @@ static int tegra_dma_terminate_all(struct dma_chan *dc) }tegra_dma_resume (tdc); +pm_runtime_put (tdc->tdma->dev); + skip_dma_stop:tegra_dma_abort_all (tdc);@@ -1268,22 +1283,15 @@ tegra_dma_prep_dma_cyclic(struct dma_chan *dc, dma_addr_t buf_addr,static int tegra_dma_alloc_chan_resources (struct dma_chan *dc) {struct tegra_dma_channel *tdc =to_tegra_dma_chan (dc); -struct tegra_dma *tdma = tdc->tdma; -int ret;dma_cookie_init (&tdc->dma_chan); - ret =pm_runtime_get_sync (tdma->dev); -if (ret <0 ) -return ret; -return 0 ; }static void tegra_dma_free_chan_resources (struct dma_chan *dc) {struct tegra_dma_channel *tdc =to_tegra_dma_chan (dc); -struct tegra_dma *tdma = tdc->tdma;struct tegra_dma_desc *dma_desc;struct tegra_dma_sg_req *sg_req;struct list_head dma_desc_list;@@ -1316,7 +1324,6 @@ static void tegra_dma_free_chan_resources(struct dma_chan *dc)list_del (&sg_req->node);kfree (sg_req); } -pm_runtime_put (tdma->dev); tdc->slave_id =TEGRA_APBDMA_SLAVE_ID_INVALID ; }@@ -1416,6 +1423,11 @@ static int tegra_dma_probe(struct platform_device *pdev)spin_lock_init (&tdma->global_lock); + ret =clk_prepare (tdma->dma_clk); +if (ret) +return ret; + +pm_runtime_irq_safe (&pdev->dev);pm_runtime_enable (&pdev->dev); ret =pm_runtime_get_sync (&pdev->dev);@@ -1531,6 +1543,7 @@ static int tegra_dma_probe(struct platform_device *pdev) err_pm_disable:pm_runtime_disable (&pdev->dev); +clk_unprepare (tdma->dma_clk);return ret; }@@ -1541,6 +1554,7 @@ static int tegra_dma_remove(struct platform_device *pdev)dma_async_device_unregister (&tdma->dma_dev);pm_runtime_disable (&pdev->dev); +clk_unprepare (tdma->dma_clk);return 0 ; }@@ -1569,7 +1583,7 @@ static int tegra_dma_runtime_suspend(struct device *dev) TEGRA_APBDMA_CHAN_WCOUNT); } -clk_disable_unprepare (tdma->dma_clk); +clk_disable (tdma->dma_clk);return 0 ; }
drivers/dma/tegra20-apb-dma.c static int tegra_dma_runtime_resume(struct device *dev) · line 1600
Quoted 8 lines
drivers/dma/tegra20-apb-dma.c +1
-1@@ -1580,7 +1594,7 @@ static int tegra_dma_runtime_resume(struct device *dev)unsigned int i;int ret; - ret =clk_prepare_enable (tdma->dma_clk); + ret =clk_enable (tdma->dma_clk);if (ret <0 ) {dev_err (dev,"clk_enable failed: %d\n" , ret);return ret;
Thanks!
Jon
Signature · 1 line
nvpublic
PATCH v8 13/19 dmaengine: tegra-apb: Clean up suspend-resume
In reply to Dmitry OsipenkoIt is enough to check whether hardware is busy on suspend and to reset it across of suspend-resume because:
1. Channel's configuration is fully re-programmed on each DMA transfer anyways.
2. Context save-restore of an active channel won't end up well without pausing transfer prior to the context's saving, but note that every channel shall be idling at the time of suspend, so save-restore is not needed at all.
3. The only case where context save-restore may be useful is when channel is in a paused state during suspend. But channel's pausing could be supported only on Tegra114+ and this functionality wasn't implemented by the driver for years now because there is no need for it in upstream kernel.
--- drivers/dma/tegra20-apb-dma.c | 136 +++++++++++++++++----------------- 1 file changed, 69 insertions(+), 67 deletions(-)
drivers/dma/tegra20-apb-dma.c +69 -67
@@ -221,9 +221,6 @@ struct tegra_dma {*/ u32 global_pause_count; -/* Some register need to be cache before suspend */ -u32 reg_gen; -/* Last member of the structure */ struct tegra_dma_channel channels[0 ]; };
@@ -1386,6 +1383,36 @@ static const struct tegra_dma_chip_data tegra148_dma_chip_data = { .support_separate_wcount_reg = true, }; +static int tegra_dma_init_hw (struct tegra_dma *tdma) +{ +int err; + + err =reset_control_assert (tdma->rst); +if (err) { +dev_err (tdma->dev,"failed to assert reset: %d\n" , err); +return err; + } + + err =clk_enable (tdma->dma_clk); +if (err) { +dev_err (tdma->dev,"failed to enable clk: %d\n" , err); +return err; + } + +/* reset DMA controller */ +udelay (2 ); +reset_control_deassert (tdma->rst); + +/* enable global DMA registers */ +tdma_write (tdma,TEGRA_APBDMA_GENERAL ,TEGRA_APBDMA_GENERAL_ENABLE ); +tdma_write (tdma,TEGRA_APBDMA_CONTROL ,0 ); +tdma_write (tdma,TEGRA_APBDMA_IRQ_MASK_SET ,0xFFFFFFFF ); + +clk_disable (tdma->dma_clk); + +return 0 ; +} +static int tegra_dma_probe (struct platform_device *pdev) {const struct tegra_dma_chip_data *cdata;
@@ -1427,25 +1454,13 @@ static int tegra_dma_probe(struct platform_device *pdev)if (ret)return ret; + ret =tegra_dma_init_hw (tdma); +if (ret) + goto err_clk_unprepare; +pm_runtime_irq_safe (&pdev->dev);pm_runtime_enable (&pdev->dev); - ret =pm_runtime_get_sync (&pdev->dev); -if (ret <0 ) - goto err_pm_disable; - -/* Reset DMA controller */ -reset_control_assert (tdma->rst); -udelay (2 ); -reset_control_deassert (tdma->rst); - -/* Enable global DMA registers */ -tdma_write (tdma,TEGRA_APBDMA_GENERAL ,TEGRA_APBDMA_GENERAL_ENABLE ); -tdma_write (tdma,TEGRA_APBDMA_CONTROL ,0 ); -tdma_write (tdma,TEGRA_APBDMA_IRQ_MASK_SET ,0xFFFFFFFFul ); - -pm_runtime_put (&pdev->dev); -INIT_LIST_HEAD (&tdma->dma_dev.channels);for (i =0 ; i < cdata->nr_channels; i++) {struct tegra_dma_channel *tdc = &tdma->channels[i];
@@ -1543,6 +1558,8 @@ static int tegra_dma_probe(struct platform_device *pdev) err_pm_disable:pm_runtime_disable (&pdev->dev); + +err_clk_unprepare:clk_unprepare (tdma->dma_clk);return ret;
@@ -1562,26 +1579,6 @@ static int tegra_dma_remove(struct platform_device *pdev)static int tegra_dma_runtime_suspend (struct device *dev) {struct tegra_dma *tdma =dev_get_drvdata (dev); -unsigned int i; - - tdma->reg_gen =tdma_read (tdma,TEGRA_APBDMA_GENERAL ); -for (i =0 ; i < tdma->chip_data->nr_channels; i++) { -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 ); - 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 ); - }clk_disable (tdma->dma_clk);
@@ -1591,46 +1588,51 @@ static int tegra_dma_runtime_suspend(struct device *dev)static int tegra_dma_runtime_resume (struct device *dev) {struct tegra_dma *tdma =dev_get_drvdata (dev); -unsigned int i; -int ret; - ret =clk_enable (tdma->dma_clk); -if (ret <0 ) { -dev_err (dev,"clk_enable failed: %d\n" , ret); -return ret; - } +return clk_enable (tdma->dma_clk); +} -tdma_write (tdma,TEGRA_APBDMA_GENERAL , tdma->reg_gen); -tdma_write (tdma,TEGRA_APBDMA_CONTROL ,0 ); -tdma_write (tdma,TEGRA_APBDMA_IRQ_MASK_SET ,0xFFFFFFFFul ); +static int __maybe_unused tegra_dma_dev_suspend (struct device *dev) +{ +struct tegra_dma *tdma =dev_get_drvdata (dev); +unsigned long flags; +unsigned int i; +bool busy;for (i =0 ; i < tdma->chip_data->nr_channels; i++) {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); -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); -tdc_write (tdc,TEGRA_APBDMA_CHAN_AHBPTR , ch_reg->ahb_ptr); -tdc_write (tdc,TEGRA_APBDMA_CHAN_CSR , - ch_reg->csr & ~TEGRA_APBDMA_CSR_ENB ); + +tasklet_kill (&tdc->tasklet); + +spin_lock_irqsave (&tdc->lock, flags); + busy = tdc->busy; +spin_unlock_irqrestore (&tdc->lock, flags); + +if (busy) { +dev_err (tdma->dev,"channel %u busy\n" , i); +return -EBUSY ; + } } -return 0 ; +return pm_runtime_force_suspend (dev); +} + +static int __maybe_unused tegra_dma_dev_resume (struct device *dev) +{ +struct tegra_dma *tdma =dev_get_drvdata (dev); +int err; + + err =tegra_dma_init_hw (tdma); +if (err) +return err; + +return pm_runtime_force_resume (dev); }static const struct dev_pm_ops tegra_dma_dev_pm_ops = { SET_RUNTIME_PM_OPS(tegra_dma_runtime_suspend, tegra_dma_runtime_resume, NULL) -SET_SYSTEM_SLEEP_PM_OPS (pm_runtime_force_suspend, - pm_runtime_force_resume) +SET_SYSTEM_SLEEP_PM_OPS (tegra_dma_dev_suspend, tegra_dma_dev_resume) };static const struct of_device_id tegra_dma_of_match[] = {
Signature · 1 line
2.24.0
PATCH v8 14/19 dmaengine: tegra-apb: Add missing of_dma_controller_free
In reply to Dmitry OsipenkoThe DMA controller shall be released on driver's removal.
--- drivers/dma/tegra20-apb-dma.c | 1 + 1 file changed, 1 insertion(+)
drivers/dma/tegra20-apb-dma.c +1 -0
@@ -1569,6 +1569,7 @@ static int tegra_dma_remove(struct platform_device *pdev) {struct tegra_dma *tdma =platform_get_drvdata (pdev); +of_dma_controller_free (pdev->dev.of_node);dma_async_device_unregister (&tdma->dma_dev);pm_runtime_disable (&pdev->dev);clk_unprepare (tdma->dma_clk);
Signature · 1 line
2.24.0
PATCH v8 15/19 dmaengine: tegra-apb: Allow to compile as a loadable kernel module
In reply to Dmitry OsipenkoThe driver's removal was fixed by a recent commit and module load/unload is working well now, tested on Tegra30.
--- drivers/dma/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/dma/Kconfig +1 -1
@@ -616,7 +616,7 @@ config TXX9_DMAC integrated in chips such as the Toshiba TX4927/38/39. config TEGRA20_APB_DMA - bool "NVIDIA Tegra20 APB DMA support" + tristate "NVIDIA Tegra20 APB DMA support" depends on ARCH_TEGRA select DMA_ENGINE help
Signature · 1 line
2.24.0
PATCH v8 16/19 dmaengine: tegra-apb: Remove MODULE_ALIAS
In reply to Dmitry OsipenkoTegra APB DMA driver is an Open Firmware driver, so it uses OF alias naming scheme which overrides MODULE_ALIAS, meaning that MODULE_ALIAS does nothing and could be removed safely.
--- drivers/dma/tegra20-apb-dma.c | 1 - 1 file changed, 1 deletion(-)
drivers/dma/tegra20-apb-dma.c +0 -1
@@ -1666,7 +1666,6 @@ static struct platform_driver tegra_dmac_driver = {module_platform_driver (tegra_dmac_driver); -MODULE_ALIAS ("platform:tegra20-apbdma" );MODULE_DESCRIPTION ("NVIDIA Tegra APB DMA Controller driver" );MODULE_AUTHOR ("Laxman Dewangan <[email protected]>" );MODULE_LICENSE ("GPL v2" );
Signature · 1 line
2.24.0
PATCH v8 17/19 dmaengine: tegra-apb: Support COMPILE_TEST
In reply to Dmitry OsipenkoThere is nothing arch-specific in the driver's code, so let's enable compile-testing for the driver.
--- drivers/dma/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/dma/Kconfig +1 -1
@@ -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
Signature · 1 line
2.24.0
PATCH v8 18/19 dmaengine: tegra-apb: Remove unused function argument
In reply to Dmitry OsipenkoRemove unused function argument from handle_continuous_head_request().
--- drivers/dma/tegra20-apb-dma.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/dma/tegra20-apb-dma.c +1 -2
@@ -553,7 +553,6 @@ static void tegra_dma_abort_all(struct tegra_dma_channel *tdc) }static bool handle_continuous_head_request (struct tegra_dma_channel *tdc, -struct tegra_dma_sg_req *last_sg_req,bool to_terminate) {struct tegra_dma_sg_req *hsgreq;
@@ -638,7 +637,7 @@ static void handle_cont_sngl_cycle_dma_done(struct tegra_dma_channel *tdc,if (!list_is_last (&sgreq->node, &tdc->pending_sg_req)) {list_move_tail (&sgreq->node, &tdc->pending_sg_req); sgreq->configured = false; - st =handle_continuous_head_request (tdc, sgreq, to_terminate); + st =handle_continuous_head_request (tdc, to_terminate);if (!st) dma_desc->dma_status =DMA_ERROR ; }
Signature · 1 line
2.24.0
Re: [PATCH v8 18/19] dmaengine: tegra-apb: Remove unused function argument
Quoted 21 lines · Dmitry Osipenko
Remove unused function argument from
handle_continuous_head_request().Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/dma/tegra20-apb-dma.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)drivers/dma/tegra20-apb-dma.c +0
-1@@ -553,7 +553,6 @@ static void tegra_dma_abort_all(struct tegra_dma_channel *tdc) }static bool handle_continuous_head_request (struct tegra_dma_channel *tdc, -struct tegra_dma_sg_req *last_sg_req,bool to_terminate) {struct tegra_dma_sg_req *hsgreq;
drivers/dma/tegra20-apb-dma.c static void handle_cont_sngl_cycle_dma_done(struct tegra_dma_channel *tdc, · line 643
Quoted 8 lines
drivers/dma/tegra20-apb-dma.c +1
-1@@ -638,7 +637,7 @@ static void handle_cont_sngl_cycle_dma_done(struct tegra_dma_channel *tdc,if (!list_is_last (&sgreq->node, &tdc->pending_sg_req)) {list_move_tail (&sgreq->node, &tdc->pending_sg_req); sgreq->configured = false; - st =handle_continuous_head_request (tdc, sgreq, to_terminate); + st =handle_continuous_head_request (tdc, to_terminate);if (!st) dma_desc->dma_status =DMA_ERROR ; }
Thanks!
Jon
Signature · 1 line
nvpublic
Re: [PATCH v8 18/19] dmaengine: tegra-apb: Remove unused function argument
14.02.2020 17:16, Jon Hunter пишет:
...
Quoted 5 lines
Acked-by Jon Hunter <[email protected]>Thanks!
Jon
Jon, thank you very much!
In the patchwork I see that you acked all the patches, but my Gmail missed 2 of 4 emails, maybe the missing emails will arrive a day later :)
Jon Hunter
maintainerRe: [PATCH v8 18/19] dmaengine: tegra-apb: Remove unused function argument
Quoted 4 lines · Dmitry Osipenko
14.02.2020 17:16, Jon Hunter пишет:
...Quoted 2 lines
Acked-by Jon Hunter <[email protected]>
Quoted 8 lines
Quoted 3 lines
Thanks!
JonJon, thank you very much!
In the patchwork I see that you acked all the patches, but my Gmail missed 2 of 4 emails, maybe the missing emails will arrive a day later :)
Yes all should be ACK'ed now. I did receive an email from our mail server saying that there was an issue and the message was delayed. So not sure if you ever got it.
Jon
Signature · 1 line
nvpublic
Re: [PATCH v8 18/19] dmaengine: tegra-apb: Remove unused function argument
17.02.2020 14:15, Jon Hunter пишет:
Quoted 10 lines · Dmitry Osipenko
Quoted 8 lines
14.02.2020 17:16, Jon Hunter пишет:
...Quoted 5 lines
Acked-by Jon Hunter <[email protected]>Thanks!
Jon
Quoted 8 lines
Quoted 4 lines
Jon, thank you very much!
In the patchwork I see that you acked all the patches, but my Gmail missed 2 of 4 emails, maybe the missing emails will arrive a day later :)
Yes all should be ACK'ed now. I did receive an email from our mail server saying that there was an issue and the message was delayed. So not sure if you ever got it.
The emails arrived a day later, just like it was predicted :) Thank you very much for reviewing the patches!
PATCH v8 19/19 dmaengine: tegra-apb: Improve error message about DMA underflow
In reply to Dmitry OsipenkoTechnically it is possible that DMA could be misconfigured in a way that cyclic DMA transfer is processed slower than it takes to complete the cycle and in this case the DMA is getting aborted with a not very informative message about the problem, let's improve it.
--- drivers/dma/tegra20-apb-dma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/dma/tegra20-apb-dma.c +1 -1
@@ -566,7 +566,7 @@ static bool handle_continuous_head_request(struct tegra_dma_channel *tdc,if (!hsgreq->configured) {tegra_dma_stop (tdc);pm_runtime_put (tdc->tdma->dev); -dev_err (tdc2dev (tdc),"Error in DMA transfer, aborting DMA\n" ); +dev_err (tdc2dev (tdc),"DMA transfer underflow, aborting DMA\n" );tegra_dma_abort_all (tdc);return false; }
Signature · 1 line
2.24.0
Re: [PATCH v8 19/19] dmaengine: tegra-apb: Improve error message about DMA underflow
Quoted 17 lines · Dmitry Osipenko
Technically it is possible that DMA could be misconfigured in a way that cyclic DMA transfer is processed slower than it takes to complete the cycle and in this case the DMA is getting aborted with a not very informative message about the problem, let's improve it.
Suggested-by Jon Hunter <[email protected]>Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/dma/tegra20-apb-dma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/dma/tegra20-apb-dma.c +0
-0
drivers/dma/tegra20-apb-dma.c static bool handle_continuous_head_request(struct tegra_dma_channel *tdc, · line 572
Quoted 8 lines
drivers/dma/tegra20-apb-dma.c +1
-1@@ -566,7 +566,7 @@ static bool handle_continuous_head_request(struct tegra_dma_channel *tdc,if (!hsgreq->configured) {tegra_dma_stop (tdc);pm_runtime_put (tdc->tdma->dev); -dev_err (tdc2dev (tdc),"Error in DMA transfer, aborting DMA\n" ); +dev_err (tdc2dev (tdc),"DMA transfer underflow, aborting DMA\n" );tegra_dma_abort_all (tdc);return false; }
Thanks!
Jon
Signature · 1 line
nvpublic
Quoted 4 lines · Dmitry Osipenko
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!
Applied, thanks
One note I would like to add thanking you and Jon for the series :)
This version was pleasure to read. A patch should do *one* thing and this series really illustrates this principal and as a result I enjoyed reading the series and was able to do a quick review of the series, notwithstanding the fact that it had 19 patches. So thanks to you and Jon (i know he pushed for split etc) for the wonderful read.
Signature · 1 line
~Vinod
25.02.2020 09:32, Vinod Koul пишет:
Quoted 8 lines · Dmitry Osipenko
Quoted 4 lines
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!
Applied, thanks
Quoted 8 lines
One note I would like to add thanking you and Jon for the series :)
This version was pleasure to read. A patch should do *one* thing and this series really illustrates this principal and as a result I enjoyed reading the series and was able to do a quick review of the series, notwithstanding the fact that it had 19 patches. So thanks to you and Jon (i know he pushed for split etc) for the wonderful read.
Thank you very much!