Skip to contentSkip to conversation

dw_dmac: few cleanups to the driver

PATCH In review Andy Shevchenko 4 participants 10 messages Oct 18, 2012 – Oct 25, 2012 dmaengine

lore
Share & tools
lore
#0 lore raw

v3 cover dw_dmac: few cleanups to the driver

There are few cleanups to the driver which partialy acked and reviewed.

Since v2:
- add one more clean up (patch 6/6)
- make a separate patch to WARN a user in dwc_handle_error (suggested by Felipe Balbi)
- add a description to patch 4/6

Andy Shevchenko (4):
dw_dmac: change dev_printk() to corresponding macros
dw_dmac: don't call platform_get_drvdata twice
dw_dmac: change dev_crit to dev_WARN in dwc_handle_error
dw_dmac: introduce to_dw_desc() macro

Heikki Krogerus (2):
dmaengine: dw_dmac: remove CLK dependency
dmaengine: dw_dmac: amend description and indentation

 drivers/dma/Kconfig        |    1 -
 drivers/dma/dw_dmac.c      |   33 ++++++++++++++-------------------
 drivers/dma/dw_dmac_regs.h |    2 ++
 3 files changed, 16 insertions(+), 20 deletions(-)
Signature · 1 line
1.7.10.4
#1 lore raw

PATCH v3 1/6 dmaengine: dw_dmac: remove CLK dependency

From: Heikki Krogerus <[email protected]>

This driver could be used on different platforms. Thus, the HAVE_CLK dependency is dropped away.

---
 drivers/dma/Kconfig |    1 -
 1 file changed, 1 deletion(-)
drivers/dma/Kconfig +0 -1
index 677cd6e..df32537 100644
@@ -83,7 +83,6 @@ config INTEL_IOP_ADMA
 
 config DW_DMAC
 	tristate "Synopsys DesignWare AHB DMA support"
-	depends on HAVE_CLK
 	select DMA_ENGINE
 	default y if CPU_AT32AP7000
 	help
Signature · 1 line
1.7.10.4
#2 lore raw

PATCH v3 2/6 dmaengine: dw_dmac: amend description and indentation

In reply to Andy Shevchenko

From: Heikki Krogerus <[email protected]>

The driver will be used as a core part for various implementations of the DesignWare DMA device. The patch adjusts description on the top and corrects paragraph indentation in few places across the code.

---
 drivers/dma/dw_dmac.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/dma/dw_dmac.c +3 -3
index c4b0eb3..c27c125 100644
@@ -1,6 +1,5 @@
 /*
- * Driver for the Synopsys DesignWare DMA Controller (aka DMACA on
- * AVR32 systems.)
+ * Core driver for the Synopsys DesignWare DMA Controller
  *
  * Copyright (C) 2007-2008 Atmel Corporation
  * Copyright (C) 2010-2011 ST Microelectronics
@@ -9,6 +8,7 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+
 #include <linux/bitops.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
@@ -222,7 +222,6 @@ static inline void dwc_dump_chan_regs(struct dw_dma_chan *dwc)
 		channel_readl(dwc, CTL_LO));
 }
 
-
 static inline void dwc_chan_disable(struct dw_dma *dw, struct dw_dma_chan *dwc)
 {
 	channel_clear_bit(dw, CH_EN, dwc->mask);
@@ -1679,6 +1678,7 @@ static int dw_resume_noirq(struct device *dev)
 
 	clk_prepare_enable(dw->clk);
 	dma_writel(dw, CFG, DW_CFG_DMA_EN);
+
 	return 0;
 }
 
Signature · 1 line
1.7.10.4
#3 lore raw

PATCH v3 3/6 dw_dmac: change dev_printk() to corresponding macros

In reply to Andy Shevchenko

Change printk(KERN_INFO ..., dev_name(...), ...) to dev_info() as well.

---
 drivers/dma/dw_dmac.c |   15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)
drivers/dma/dw_dmac.c +6 -9
index c27c125..d4ddf48 100644
@@ -456,9 +456,8 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
 
 static inline void dwc_dump_lli(struct dw_dma_chan *dwc, struct dw_lli *lli)
 {
-	dev_printk(KERN_CRIT, chan2dev(&dwc->chan),
-			"  desc: s0x%x d0x%x l0x%x c0x%x:%x\n",
-			lli->sar, lli->dar, lli->llp, lli->ctlhi, lli->ctllo);
+	dev_crit(chan2dev(&dwc->chan), "  desc: s0x%x d0x%x l0x%x c0x%x:%x\n",
+		 lli->sar, lli->dar, lli->llp, lli->ctlhi, lli->ctllo);
 }
 
 static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
@@ -492,10 +491,8 @@ static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
 	 * controller flagged an error instead of scribbling over
 	 * random memory locations.
 	 */
-	dev_printk(KERN_CRIT, chan2dev(&dwc->chan),
-			"Bad descriptor submitted for DMA!\n");
-	dev_printk(KERN_CRIT, chan2dev(&dwc->chan),
-			"  cookie: %d\n", bad_desc->txd.cookie);
+	dev_crit(chan2dev(&dwc->chan), "Bad descriptor submitted for DMA!\n");
+	dev_crit(chan2dev(&dwc->chan), "  cookie: %d\n", bad_desc->txd.cookie);
 	dwc_dump_lli(dwc, &bad_desc->lli);
 	list_for_each_entry(child, &bad_desc->tx_list, desc_node)
 		dwc_dump_lli(dwc, &child->lli);
@@ -1625,8 +1622,8 @@ static int __devinit dw_probe(struct platform_device *pdev)
 
 	dma_writel(dw, CFG, DW_CFG_DMA_EN);
 
-	printk(KERN_INFO "%s: DesignWare DMA Controller, %d channels\n",
-			dev_name(&pdev->dev), nr_channels);
+	dev_info(&pdev->dev, "DesignWare DMA Controller, %d channels\n",
+		 nr_channels);
 
 	dma_async_device_register(&dw->dma);
 
Signature · 1 line
1.7.10.4
#4 lore raw

PATCH v3 4/6 dw_dmac: don't call platform_get_drvdata twice

In reply to Andy Shevchenko

There is no need to call platform_get_drvdata twice as we have it already in dw variable.

---
 drivers/dma/dw_dmac.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/dma/dw_dmac.c +2 -2
index d4ddf48..180ffd5 100644
@@ -1653,7 +1653,7 @@ static void dw_shutdown(struct platform_device *pdev)
 {
 	struct dw_dma	*dw = platform_get_drvdata(pdev);
 
-	dw_dma_off(platform_get_drvdata(pdev));
+	dw_dma_off(dw);
 	clk_disable_unprepare(dw->clk);
 }
 
@@ -1662,7 +1662,7 @@ static int dw_suspend_noirq(struct device *dev)
 	struct platform_device *pdev = to_platform_device(dev);
 	struct dw_dma	*dw = platform_get_drvdata(pdev);
 
-	dw_dma_off(platform_get_drvdata(pdev));
+	dw_dma_off(dw);
 	clk_disable_unprepare(dw->clk);
 
 	return 0;
Signature · 1 line
1.7.10.4
#5 lore raw

PATCH v3 5/6 dw_dmac: change dev_crit to dev_WARN in dwc_handle_error

In reply to Andy Shevchenko

In case of handling a bad descriptor the dwc_handle_error() will dump a stack as well. It's a lot more verbose and more likely to get user's attention.

---
 drivers/dma/dw_dmac.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/dma/dw_dmac.c +3 -3
index 180ffd5..826ff8d 100644
@@ -485,14 +485,14 @@ static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
 		dwc_dostart(dwc, dwc_first_active(dwc));
 
 	/*
-	 * KERN_CRITICAL may seem harsh, but since this only happens
+	 * WARN may seem harsh, but since this only happens
 	 * when someone submits a bad physical address in a
 	 * descriptor, we should consider ourselves lucky that the
 	 * controller flagged an error instead of scribbling over
 	 * random memory locations.
 	 */
-	dev_crit(chan2dev(&dwc->chan), "Bad descriptor submitted for DMA!\n");
-	dev_crit(chan2dev(&dwc->chan), "  cookie: %d\n", bad_desc->txd.cookie);
+	dev_WARN(chan2dev(&dwc->chan), "Bad descriptor submitted for DMA!\n"
+				       "  cookie: %d\n", bad_desc->txd.cookie);
 	dwc_dump_lli(dwc, &bad_desc->lli);
 	list_for_each_entry(child, &bad_desc->tx_list, desc_node)
 		dwc_dump_lli(dwc, &child->lli);
Signature · 1 line
1.7.10.4
#6 lore raw

PATCH v3 6/6 dw_dmac: introduce to_dw_desc() macro

In reply to Andy Shevchenko

The to_dw_desc() macro helps to retrieve the dw_desc node from the corresponding list_head structure.

---
 drivers/dma/dw_dmac.c      |    6 ++----
 drivers/dma/dw_dmac_regs.h |    2 ++
 2 files changed, 4 insertions(+), 4 deletions(-)
drivers/dma/dw_dmac.c +2 -4
index 826ff8d..cb89c2a 100644
@@ -93,7 +93,7 @@ static struct device *chan2parent(struct dma_chan *chan)
 
 static struct dw_desc *dwc_first_active(struct dw_dma_chan *dwc)
 {
-	return list_entry(dwc->active_list.next, struct dw_desc, desc_node);
+	return to_dw_desc(dwc->active_list.next);
 }
 
 static struct dw_desc *dwc_desc_get(struct dw_dma_chan *dwc)
@@ -600,9 +600,7 @@ static void dw_dma_tasklet(unsigned long data)
 			if (test_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags)) {
 				if (dwc->tx_node_active != dwc->tx_list) {
 					struct dw_desc *desc =
-						list_entry(dwc->tx_node_active,
-							   struct dw_desc,
-							   desc_node);
+						to_dw_desc(dwc->tx_node_active);
 
 					dma_writel(dw, CLEAR.XFER, dwc->mask);
 
drivers/dma/dw_dmac_regs.h +2 -0
index ff39fa6..de90a1b 100644
@@ -287,6 +287,8 @@ struct dw_desc {
 	size_t				len;
 };
 
+#define to_dw_desc(h)	list_entry(h, struct dw_desc, desc_node)
+
 static inline struct dw_desc *
 txd_to_dw_desc(struct dma_async_tx_descriptor *txd)
 {
Signature · 1 line
1.7.10.4
Acked-by on the v3 series
#7 lore raw
In reply to Andy Shevchenko
Quoted 7 lines · Andy Shevchenko

There are few cleanups to the driver which partialy acked and reviewed.

Since v2:
- add one more clean up (patch 6/6)
- make a separate patch to WARN a user in dwc_handle_error (suggested by Felipe Balbi)
- add a description to patch 4/6

Reviewed-by
#8 lore raw

Hi,

Quoted 11 lines · viresh kumar
Quoted 7 lines · Andy Shevchenko

There are few cleanups to the driver which partialy acked and reviewed.

Since v2:
- add one more clean up (patch 6/6)
- make a separate patch to WARN a user in dwc_handle_error (suggested by Felipe Balbi)
- add a description to patch 4/6

FWIW:

(for the entire series)

Signature · 1 line
balbi
#9 lore raw
In reply to Andy Shevchenko
Quoted 14 lines · Andy Shevchenko

There are few cleanups to the driver which partialy acked and reviewed.

Since v2:
- add one more clean up (patch 6/6)
- make a separate patch to WARN a user in dwc_handle_error (suggested by Felipe Balbi)
- add a description to patch 4/6

Andy Shevchenko (4):
dw_dmac: change dev_printk() to corresponding macros
dw_dmac: don't call platform_get_drvdata twice
dw_dmac: change dev_crit to dev_WARN in dwc_handle_error
dw_dmac: introduce to_dw_desc() macro

Quoted 9 lines

Heikki Krogerus (2):
dmaengine: dw_dmac: remove CLK dependency
dmaengine: dw_dmac: amend description and indentation

 drivers/dma/Kconfig        |    1 -
 drivers/dma/dw_dmac.c      |   33 ++++++++++++++-------------------
 drivers/dma/dw_dmac_regs.h |    2 ++
 3 files changed, 16 insertions(+), 20 deletions(-)

Applied thanks

Signature · 2 lines
Vinod Koul
Intel Corp.