dw_dmac: move to own folder and split

PATCH In review Andy Shevchenko 3 participants 10 messages Jun 4, 2013 – Jun 5, 2013

Subsystems (2)

dmaengine Synopsys DesignWare DMAC driver

lore

Share & tools

lore

Andy Shevchenko

author reviewer

2013-06-04 12:36 UTC #0 lore raw

cover dw_dmac: move to own folder and split

This is second attempt to do split as suggested by Arnd et al [1,2]. There are three most important patches:
2/4 moves driver to its own folder
3/4 does the actual split
4/4 introduces PCI part of the driver

During this files are renamed: mostly means that dw_dmac prefix is eliminated. I hope to get it ready to be included in v3.11.

[1] http://www.spinics.net/lists/kernel/msg1408174.html
[2] http://www.spinics.net/lists/kernel/msg1408495.html

P.S. Viresh, I didn't apply your Ack to the 3/4, because I amended it a bit: I moved dwc_get_{s,d}ms to the "internal.h" as it shared between core.c and platform.c.

Cnages v0->v1:
- apply Viresh's ACKs
- append PCI part of the driver
- test altogether on Intel Medfield
- add 1/4 to the series as it based on it, but Vinod didn't apply it yet to his tree.

Andy Shevchenko (4):
dw_dmac: don't check resource with devm_ioremap_resource
dma: move dw_dmac driver to an own directory
dma: dw: split driver to library part and platform code
dma: dw: add PCI part of the driver

 MAINTAINERS                               |   3 +-
 drivers/dma/Kconfig                       |  20 +-
 drivers/dma/Makefile                      |   2 +-
 drivers/dma/dw/Kconfig                    |  36 ++++
 drivers/dma/dw/Makefile                   |   8 +
 drivers/dma/{dw_dmac.c => dw/core.c}      | 314 ++++--------------------------
 drivers/dma/dw/internal.h                 |  74 +++++++
 drivers/dma/dw/pci.c                      | 101 ++++++++++
 drivers/dma/dw/platform.c                 | 308 +++++++++++++++++++++++++++++
 drivers/dma/{dw_dmac_regs.h => dw/regs.h} |   1 +
 10 files changed, 565 insertions(+), 302 deletions(-)
 create mode 100644 drivers/dma/dw/Kconfig
 create mode 100644 drivers/dma/dw/Makefile
 rename drivers/dma/{dw_dmac.c => dw/core.c} (85%)
 create mode 100644 drivers/dma/dw/internal.h
 create mode 100644 drivers/dma/dw/pci.c
 create mode 100644 drivers/dma/dw/platform.c
 rename drivers/dma/{dw_dmac_regs.h => dw/regs.h} (99%)

Signature · 1 line

1.8.2.rc0.22.gb3600c3

Andy Shevchenko

author reviewer

2013-06-04 12:36 UTC #1 lore raw

PATCH 1/4 dw_dmac: don't check resource with devm_ioremap_resource

devm_ioremap_resource does sanity checks on the given resource. No need to duplicate this in the driver.

Signed-off-by Andy Shevchenko <[email protected]>

Acked-by Viresh Kumar <[email protected]>

---
 drivers/dma/dw_dmac.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

drivers/dma/dw_dmac.c +1 -4

index 724083d..2b65ba6 100644
@@ -1667,14 +1667,11 @@ static int dw_probe(struct platform_device *pdev)
 	int			err;
 	int			i;
 
-	io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!io)
-		return -EINVAL;
-
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
 		return irq;
 
+	io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	regs = devm_ioremap_resource(&pdev->dev, io);
 	if (IS_ERR(regs))
 		return PTR_ERR(regs);

Signature · 1 line

1.8.2.rc0.22.gb3600c3

Andy Shevchenko

author reviewer

2013-06-04 12:36 UTC #2 lore raw

PATCH 2/4 dma: move dw_dmac driver to an own directory

In reply to Andy Shevchenko

The dw_dmac driver is going to be split into multiple files. To make this more convenient move it to an own directory.

Signed-off-by Andy Shevchenko <[email protected]>

Acked-by Viresh Kumar <[email protected]>

---
 MAINTAINERS                         |  3 +--
 drivers/dma/Kconfig                 | 20 +-------------------
 drivers/dma/Makefile                |  2 +-
 drivers/dma/dw/Kconfig              | 23 +++++++++++++++++++++++
 drivers/dma/dw/Makefile             |  1 +
 drivers/dma/{ => dw}/dw_dmac.c      |  2 +-
 drivers/dma/{ => dw}/dw_dmac_regs.h |  0
 7 files changed, 28 insertions(+), 23 deletions(-)
 create mode 100644 drivers/dma/dw/Kconfig
 create mode 100644 drivers/dma/dw/Makefile
 rename drivers/dma/{ => dw}/dw_dmac.c (99%)
 rename drivers/dma/{ => dw}/dw_dmac_regs.h (100%)

MAINTAINERS +1 -2

index c488924..e0b03a9 100644
@@ -6995,8 +6995,7 @@ SYNOPSYS DESIGNWARE DMAC DRIVER
 M:	Viresh Kumar <[email protected]>
 S:	Maintained
 F:	include/linux/dw_dmac.h
-F:	drivers/dma/dw_dmac_regs.h
-F:	drivers/dma/dw_dmac.c
+F:	drivers/dma/dw/
 
 SYNOPSYS DESIGNWARE MMC/SD/SDIO DRIVER
 M:	Seungwon Jeon <[email protected]>

drivers/dma/Kconfig +1 ~~-19~~

index e992489..146a1d8 100644
@@ -79,25 +79,7 @@ config INTEL_IOP_ADMA
 	help
 	  Enable support for the Intel(R) IOP Series RAID engines.
 
-config DW_DMAC
-	tristate "Synopsys DesignWare AHB DMA support"
-	depends on GENERIC_HARDIRQS
-	select DMA_ENGINE
-	default y if CPU_AT32AP7000
-	help
-	  Support the Synopsys DesignWare AHB DMA controller.  This
-	  can be integrated in chips such as the Atmel AT32ap7000.
-
-config DW_DMAC_BIG_ENDIAN_IO
-	bool "Use big endian I/O register access"
-	default y if AVR32
-	depends on DW_DMAC
-	help
-	  Say yes here to use big endian I/O access when reading and writing
-	  to the DMA controller registers. This is needed on some platforms,
-	  like the Atmel AVR32 architecture.
-
-	  If unsure, use the default setting.
+source "drivers/dma/dw/Kconfig"
 
 config AT_HDMAC
 	tristate "Atmel AHB DMA support"

drivers/dma/Makefile +1 -1

index a2b0df5..ac44ca0 100644
@@ -15,7 +15,7 @@ obj-$(CONFIG_FSL_DMA) += fsldma.o
 obj-$(CONFIG_MPC512X_DMA) += mpc512x_dma.o
 obj-$(CONFIG_PPC_BESTCOMM) += bestcomm/
 obj-$(CONFIG_MV_XOR) += mv_xor.o
-obj-$(CONFIG_DW_DMAC) += dw_dmac.o
+obj-$(CONFIG_DW_DMAC) += dw/
 obj-$(CONFIG_AT_HDMAC) += at_hdmac.o
 obj-$(CONFIG_MX3_IPU) += ipu/
 obj-$(CONFIG_TXX9_DMAC) += txx9dmac.o

drivers/dma/dw/Kconfig new file +23 -0

new file mode 100644
index 0000000..38a215a
@@ -0,0 +1,23 @@
+#
+# DMA engine configuration for dw
+#
+
+config DW_DMAC
+	tristate "Synopsys DesignWare AHB DMA support"
+	depends on GENERIC_HARDIRQS
+	select DMA_ENGINE
+	default y if CPU_AT32AP7000
+	help
+	  Support the Synopsys DesignWare AHB DMA controller. This
+	  can be integrated in chips such as the Atmel AT32ap7000.
+
+config DW_DMAC_BIG_ENDIAN_IO
+	bool "Use big endian I/O register access"
+	default y if AVR32
+	depends on DW_DMAC
+	help
+	  Say yes here to use big endian I/O access when reading and writing
+	  to the DMA controller registers. This is needed on some platforms,
+	  like the Atmel AVR32 architecture.
+
+	  If unsure, use the default setting.

drivers/dma/dw/Makefile new file +1 -0

new file mode 100644
index 0000000..dd8d993
@@ -0,0 +1 @@
+obj-$(CONFIG_DW_DMAC) += dw_dmac.o

drivers/dma/dw/dw_dmac.c renamed from drivers/dma/dw_dmac.c +1 -1

similarity index 99%
rename from drivers/dma/dw_dmac.c
rename to drivers/dma/dw/dw_dmac.c
index 2b65ba6..15f3f4f 100644
@@ -28,8 +28,8 @@
 #include <linux/acpi.h>
 #include <linux/acpi_dma.h>
 
+#include "../dmaengine.h"
 #include "dw_dmac_regs.h"
-#include "dmaengine.h"
 
 /*
  * This supports the Synopsys "DesignWare AHB Central DMA Controller",

drivers/dma/dw/dw_dmac_regs.h renamed from drivers/dma/dw_dmac_regs.h +0 -0

similarity index 100%
rename from drivers/dma/dw_dmac_regs.h
rename to drivers/dma/dw/dw_dmac_regs.h

Signature · 1 line

1.8.2.rc0.22.gb3600c3

Andy Shevchenko

author reviewer

2013-06-04 12:36 UTC #3 lore raw

PATCH 3/4 dma: dw: split driver to library part and platform code

In reply to Andy Shevchenko

To simplify the driver development let's split driver to library and platform code parts. It helps us to add PCI driver in future.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/dma/Makefile                      |   2 +-
 drivers/dma/dw/Kconfig                    |   8 +-
 drivers/dma/dw/Makefile                   |   6 +-
 drivers/dma/dw/{dw_dmac.c => core.c}      | 309 ++++--------------------------
 drivers/dma/dw/internal.h                 |  74 +++++++
 drivers/dma/dw/platform.c                 | 308 +++++++++++++++++++++++++++++
 drivers/dma/dw/{dw_dmac_regs.h => regs.h} |   1 +
 7 files changed, 428 insertions(+), 280 deletions(-)
 rename drivers/dma/dw/{dw_dmac.c => core.c} (85%)
 create mode 100644 drivers/dma/dw/internal.h
 create mode 100644 drivers/dma/dw/platform.c
 rename drivers/dma/dw/{dw_dmac_regs.h => regs.h} (99%)

drivers/dma/Makefile +1 -1

index ac44ca0..6e2a521 100644
@@ -15,7 +15,7 @@ obj-$(CONFIG_FSL_DMA) += fsldma.o
 obj-$(CONFIG_MPC512X_DMA) += mpc512x_dma.o
 obj-$(CONFIG_PPC_BESTCOMM) += bestcomm/
 obj-$(CONFIG_MV_XOR) += mv_xor.o
-obj-$(CONFIG_DW_DMAC) += dw/
+obj-$(CONFIG_DW_DMAC_CORE) += dw/
 obj-$(CONFIG_AT_HDMAC) += at_hdmac.o
 obj-$(CONFIG_MX3_IPU) += ipu/
 obj-$(CONFIG_TXX9_DMAC) += txx9dmac.o

drivers/dma/dw/Kconfig +6 -2

index 38a215a..efd9e02 100644
@@ -2,10 +2,14 @@
 # DMA engine configuration for dw
 #
 
-config DW_DMAC
+config DW_DMAC_CORE
 	tristate "Synopsys DesignWare AHB DMA support"
 	depends on GENERIC_HARDIRQS
 	select DMA_ENGINE
+
+config DW_DMAC
+	tristate "Synopsys DesignWare AHB DMA platform driver"
+	select DW_DMAC_CORE
 	default y if CPU_AT32AP7000
 	help
 	  Support the Synopsys DesignWare AHB DMA controller. This

@@ -14,7 +18,7 @@ config DW_DMAC
 config DW_DMAC_BIG_ENDIAN_IO
 	bool "Use big endian I/O register access"
 	default y if AVR32
-	depends on DW_DMAC
+	depends on DW_DMAC_CORE
 	help
 	  Say yes here to use big endian I/O access when reading and writing
 	  to the DMA controller registers. This is needed on some platforms,

drivers/dma/dw/Makefile +5 -1

index dd8d993..47f3674 100644
@@ -1 +1,5 @@
-obj-$(CONFIG_DW_DMAC) += dw_dmac.o
+obj-$(CONFIG_DW_DMAC_CORE)	+= dw_dmac_core.o
+dw_dmac_core-objs	:= core.o
+
+obj-$(CONFIG_DW_DMAC)		+= dw_dmac.o
+dw_dmac-objs		:= platform.o

drivers/dma/dw/core.c renamed from drivers/dma/dw/dw_dmac.c +33 ~~-276~~

443 lines not on this page · raw message

drivers/dma/dw/internal.h new file +74 -0

74 lines not on this page · raw message

drivers/dma/dw/platform.c new file +308 -0

308 lines not on this page · raw message

drivers/dma/dw/regs.h renamed from drivers/dma/dw/dw_dmac_regs.h +1 -0

similarity index 99%
rename from drivers/dma/dw/dw_dmac_regs.h
rename to drivers/dma/dw/regs.h
index 9d41720..07c5a6e 100644
@@ -9,6 +9,7 @@
  * published by the Free Software Foundation.
  */
 
+#include <linux/interrupt.h>
 #include <linux/dmaengine.h>
 #include <linux/dw_dmac.h>

Signature · 1 line

1.8.2.rc0.22.gb3600c3

Arnd Bergmann

2013-06-04 13:00 UTC #4 lore raw

Re: [PATCH v1 3/4] dma: dw: split driver to library part and platform code

Quoted 4 lines · On Tuesday 04 June 2013 15:36:21 Andy Shevchenko wrote:

To simplify the driver development let's split driver to library and platform code parts. It helps us to add PCI driver in future.
Signed-off-by Andy Shevchenko <[email protected]>

Quoted 8 lines

---
 drivers/dma/Makefile                      |   2 +-
 drivers/dma/dw/Kconfig                    |   8 +-
 drivers/dma/dw/Makefile                   |   6 +-
 drivers/dma/dw/{dw_dmac.c => core.c}      | 309 ++++--------------------------
 drivers/dma/dw/internal.h                 |  74 +++++++
 drivers/dma/dw/platform.c                 | 308 +++++++++++++++++++++++++++++
 drivers/dma/dw/{dw_dmac_regs.h => regs.h} |   1 +

Looks good to me, just one issue I found:

drivers/dma/dw/internal.h line 48

Quoted 11 lines

drivers/dma/dw/internal.h new file +11 -0

+#ifdef CONFIG_PM_SLEEP
+
+int dw_dma_suspend_noirq(struct dw_dma_chip *chip);
+int dw_dma_resume_noirq(struct dw_dma_chip *chip);
+
+#else /* !CONFIG_PM_SLEEP */
+
+#define dw_dma_suspend_noirq	NULL
+#define dw_dma_resume_noirq	NULL
+
+#endif /* !CONFIG_PM_SLEEP */

This won't work ...

Quoted 5 lines

drivers/dma/dw/platform.c new file +5 -0

+static int dw_suspend_noirq(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct dw_dma_chip *chip = platform_get_drvdata(pdev);
+

drivers/dma/dw/platform.c line 272

Quoted 10 lines

drivers/dma/dw/platform.c new file +10 -0

+	return dw_dma_suspend_noirq(chip);
+}
+
+static int dw_resume_noirq(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct dw_dma_chip *chip = platform_get_drvdata(pdev);
+
+	return dw_dma_resume_noirq(chip);
+}

when you try to call NULL as a function.

Please try to get it to build with CONFIG_PM_SLEEP disabled.

Arnd

Viresh Kumar

Acked-by on v1 3/4

2013-06-05 08:45 UTC #5 lore raw

Re: [PATCH v1 3/4] dma: dw: split driver to library part and platform code

In reply to Andy Shevchenko

Quoted 4 lines · Andy Shevchenko

To simplify the driver development let's split driver to library and platform code parts. It helps us to add PCI driver in future.
Signed-off-by Andy Shevchenko <[email protected]>

Quoted 13 lines

---
 drivers/dma/Makefile                      |   2 +-
 drivers/dma/dw/Kconfig                    |   8 +-
 drivers/dma/dw/Makefile                   |   6 +-
 drivers/dma/dw/{dw_dmac.c => core.c}      | 309 ++++--------------------------
 drivers/dma/dw/internal.h                 |  74 +++++++
 drivers/dma/dw/platform.c                 | 308 +++++++++++++++++++++++++++++
 drivers/dma/dw/{dw_dmac_regs.h => regs.h} |   1 +
 7 files changed, 428 insertions(+), 280 deletions(-)
 rename drivers/dma/dw/{dw_dmac.c => core.c} (85%)
 create mode 100644 drivers/dma/dw/internal.h
 create mode 100644 drivers/dma/dw/platform.c
 rename drivers/dma/dw/{dw_dmac_regs.h => regs.h} (99%)

Acked-by Viresh Kumar <[email protected]>

Andy Shevchenko

author reviewer

2013-06-04 12:36 UTC #6 lore raw

PATCH 4/4 dma: dw: add PCI part of the driver

In reply to Andy Shevchenko

This is the PCI part of the DesignWare DMAC driver. The controller is usually used in the Intel hardware such as Intel Medfield.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/dma/dw/Kconfig  |   9 +++++
 drivers/dma/dw/Makefile |   3 ++
 drivers/dma/dw/pci.c    | 101 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 113 insertions(+)
 create mode 100644 drivers/dma/dw/pci.c

drivers/dma/dw/Kconfig +9 -0

index efd9e02..db2b41f 100644
@@ -15,6 +15,15 @@ config DW_DMAC
 	  Support the Synopsys DesignWare AHB DMA controller. This
 	  can be integrated in chips such as the Atmel AT32ap7000.
 
+config DW_DMAC_PCI
+	tristate "Synopsys DesignWare AHB DMA PCI driver"
+	depends on PCI
+	select DW_DMAC_CORE
+	help
+	  Support the Synopsys DesignWare AHB DMA controller on the
+	  platfroms that enumerate it as a PCI device. For example,
+	  Intel Medfield has integrated this GPDMA controller.
+
 config DW_DMAC_BIG_ENDIAN_IO
 	bool "Use big endian I/O register access"
 	default y if AVR32

drivers/dma/dw/Makefile +3 -0

index 47f3674..3eebd1c 100644
@@ -3,3 +3,6 @@ dw_dmac_core-objs	:= core.o
 
 obj-$(CONFIG_DW_DMAC)		+= dw_dmac.o
 dw_dmac-objs		:= platform.o
+
+obj-$(CONFIG_DW_DMAC_PCI)	+= dw_dmac_pci.o
+dw_dmac_pci-objs	:= pci.o

drivers/dma/dw/pci.c new file +101 -0

101 lines not on this page · raw message

Signature · 1 line

1.8.2.rc0.22.gb3600c3

Viresh Kumar

Acked-by on v1 4/4

2013-06-05 09:42 UTC #7 lore raw

Re: [PATCH v1 4/4] dma: dw: add PCI part of the driver

Quoted 10 lines · Andy Shevchenko

This is the PCI part of the DesignWare DMAC driver. The controller is usually used in the Intel hardware such as Intel Medfield.
Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/dma/dw/Kconfig  |   9 +++++
 drivers/dma/dw/Makefile |   3 ++
 drivers/dma/dw/pci.c    | 101 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 113 insertions(+)
 create mode 100644 drivers/dma/dw/pci.c

Looks fine.

Acked-by Viresh Kumar <[email protected]>

Arnd Bergmann

Acked-by on the v1 series

2013-06-04 13:01 UTC #8 lore raw

In reply to Andy Shevchenko

Quoted 11 lines · On Tuesday 04 June 2013 15:36:18 Andy Shevchenko wrote:

This is second attempt to do split as suggested by Arnd et al [1,2]. There are three most important patches:
2/4 moves driver to its own folder
3/4 does the actual split
4/4 introduces PCI part of the driver
During this files are renamed: mostly means that dw_dmac prefix is eliminated. I hope to get it ready to be included in v3.11.
[1] http://www.spinics.net/lists/kernel/msg1408174.html
[2] http://www.spinics.net/lists/kernel/msg1408495.html

Whole series

Acked-by Arnd Bergmann <[email protected]>

provided you make it work with !CONFIG_PM.

Arnd

Andy Shevchenko

author reviewer

2013-06-04 13:43 UTC #9 lore raw

Quoted 10 lines · Arnd Bergmann

Quoted 9 lines · On Tuesday 04 June 2013 15:36:18 Andy Shevchenko wrote:
This is second attempt to do split as suggested by Arnd et al [1,2]. There are three most important patches:
2/4 moves driver to its own folder
3/4 does the actual split
4/4 introduces PCI part of the driver
During this files are renamed: mostly means that dw_dmac prefix is eliminated. I hope to get it ready to be included in v3.11.

Quoted 8 lines

Quoted 2 lines
[1] http://www.spinics.net/lists/kernel/msg1408174.html
[2] http://www.spinics.net/lists/kernel/msg1408495.html
Whole series
Acked-by Arnd Bergmann <[email protected]>
provided you make it work with !CONFIG_PM.

Thank you for review.

I will wait for a while, if no more comments I will resubmit v2.

Signature · 2 lines

Andy Shevchenko <[email protected]>
Intel Finland Oy