[U-Boot] [PATCH 0/6] add dma uclass implementation and adopt ti-edma3 to it

[U-Boot] [PATCH 0/6] add dma uclass implementation and adopt ti-edma3 to it

[Mugunthan V N]

This patch series enables adds support for DMA driver model and
to adopt driver model. This has been tested on am437x-sk evms
(logs [1]).
Also pushed a branch for testing [2]

[1]: http://pastebin.ubuntu.com/13596990/
[2]: git://git.ti.com/~mugunthanvnm/ti-u-boot/mugunth-ti-u-boot.git dm-dma

Mugunthan V N (6):
  dm: implement a DMA uclass
  dma: Kconfig: Add TI_EDMA3 entry
  sf: sf_ops: use dma to copy data from mmap region if platform supports
  spi: ti_qspi: compile out spi_flash_copy_mmap when DM_DMA is defined
  drivers: dma: ti-edma3: convert driver to adopt driver model
  defconfig: am437x_sk_evm: enable dma driver model

 configs/am437x_sk_evm_defconfig |  1 +
 drivers/dma/Kconfig             | 22 ++++++++++++
 drivers/dma/Makefile            |  2 ++
 drivers/dma/dma-uclass.c        | 63 ++++++++++++++++++++++++++++++++++
 drivers/dma/ti-edma3.c          | 75 +++++++++++++++++++++++++++++++++++++++--
 drivers/mtd/spi/sf_ops.c        |  5 +++
 drivers/spi/ti_qspi.c           |  2 +-
 include/dm/uclass-id.h          |  1 +
 include/dma.h                   | 64 +++++++++++++++++++++++++++++++++++
 9 files changed, 232 insertions(+), 3 deletions(-)
 create mode 100644 drivers/dma/dma-uclass.c
 create mode 100644 include/dma.h

-- 
2.6.3.368.gf34be46

[U-Boot] [PATCH 1/6] dm: implement a DMA uclass

[Mugunthan V N]

Implement a DMA uclass so that the devices like ethernet, spi,
mmc etc can offload the data transfers from/to the device and
memory.

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 drivers/dma/Kconfig      | 15 ++++++++++++
 drivers/dma/Makefile     |  2 ++
 drivers/dma/dma-uclass.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++
 include/dm/uclass-id.h   |  1 +
 include/dma.h            | 64 ++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 145 insertions(+)
 create mode 100644 drivers/dma/dma-uclass.c
 create mode 100644 include/dma.h

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index e69de29..af15199 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -0,0 +1,15 @@
+menu "DMA Support"
+
+config DM_DMA
+	bool "Enable Driver Model for DMA drivers"
+	depends on DM
+	help
+	  Enable driver model for DMA. DMA engines can do
+	  asynchronous data transfers without involving the host
+	  CPU. Currently, this framework can be used to offload
+	  memory copies to and from devices like qspi, ethernet
+	  etc Drivers provide methods to access the DMA devices
+	  buses that is used to transfer data to and from memory.
+	  The uclass interface is defined in include/dma.h.
+
+endmenu # menu "DMA Support"
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index f95fe70..4ff22aa 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -5,6 +5,8 @@
 # SPDX-License-Identifier:	GPL-2.0+
 #
 
+obj-$(CONFIG_DM_DMA) += dma-uclass.o
+
 obj-$(CONFIG_FSLDMAFEC) += MCD_tasksInit.o MCD_dmaApi.o MCD_tasks.o
 obj-$(CONFIG_APBH_DMA) += apbh_dma.o
 obj-$(CONFIG_FSL_DMA) += fsl_dma.o
diff --git a/drivers/dma/dma-uclass.c b/drivers/dma/dma-uclass.c
new file mode 100644
index 0000000..1afcbaa4
--- /dev/null
+++ b/drivers/dma/dma-uclass.c
@@ -0,0 +1,63 @@
+/*
+ * Direct Memory Access U-Class driver
+ *
+ * (C) Copyright 2015
+ *     Texas Instruments Incorporated, <www.ti.com>
+ *
+ * Author: Mugunthan V N <mugunthanvnm@ti.com>
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#include <common.h>
+#include <dma.h>
+#include <dm.h>
+#include <dm/uclass-internal.h>
+#include <errno.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+int dma_memcpy(void *dst, void *src, size_t len)
+{
+	struct udevice *dev;
+	const struct dma_ops *ops;
+	int ret;
+
+	for (ret = uclass_find_first_device(UCLASS_DMA, &dev); dev && !ret;
+	     ret = uclass_find_next_device(&dev)) {
+		struct dma_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+
+		if (uc_priv->supported & SUPPORTS_MEM_TO_MEM)
+			break;
+	}
+
+	if (!dev) {
+		printf("No DMA device found that supports mem to mem transfers\n");
+		return -ENODEV;
+	}
+
+	ops = device_get_ops(dev);
+	if (!ops->transfer)
+		return -ENOSYS;
+
+	/* Invalidate the area, so no writeback into the RAM races with DMA */
+	invalidate_dcache_range((unsigned long)dst, (unsigned long)dst +
+				roundup(len, ARCH_DMA_MINALIGN));
+
+	return ops->transfer(dev, DMA_MEM_TO_MEM, dst, src, len);
+}
+
+int dma_post_bind(struct udevice *dev)
+{
+	struct udevice *devp;
+	uclass_get_device_by_of_offset(UCLASS_DMA, dev->of_offset, &devp);
+	return 0;
+}
+
+UCLASS_DRIVER(dma) = {
+	.id		= UCLASS_DMA,
+	.name		= "dma",
+	.flags		= DM_UC_FLAG_SEQ_ALIAS,
+	.per_device_auto_alloc_size = sizeof(struct dma_dev_priv),
+	.post_bind	= dma_post_bind,
+};
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 27fa0b6..9f5fcae 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -30,6 +30,7 @@ enum uclass_id {
 	UCLASS_CPU,		/* CPU, typically part of an SoC */
 	UCLASS_CROS_EC,		/* Chrome OS EC */
 	UCLASS_DISPLAY_PORT,	/* Display port video */
+	UCLASS_DMA,		/* Direct Memory Access */
 	UCLASS_RAM,		/* RAM controller */
 	UCLASS_ETH,		/* Ethernet device */
 	UCLASS_GPIO,		/* Bank of general-purpose I/O pins */
diff --git a/include/dma.h b/include/dma.h
new file mode 100644
index 0000000..d53e435
--- /dev/null
+++ b/include/dma.h
@@ -0,0 +1,64 @@
+/*
+ * (C) Copyright 2015
+ *     Texas Instruments Incorporated, <www.ti.com>
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#ifndef _DMA_H_
+#define _DMA_H_
+
+/*
+ * enum dma_direction - dma transfer direction indicator
+ * @DMA_MEM_TO_MEM: Memcpy mode
+ * @DMA_MEM_TO_DEV: From Memory to Device
+ * @DMA_DEV_TO_MEM: From Device to Memory
+ * @DMA_DEV_TO_DEV: From Device to Device
+ */
+enum dma_direction {
+	DMA_MEM_TO_MEM,
+	DMA_MEM_TO_DEV,
+	DMA_DEV_TO_MEM,
+	DMA_DEV_TO_DEV,
+	DMA_TRANS_NONE,
+};
+
+#define SUPPORTS_MEM_TO_MEM	BIT(0)
+#define SUPPORTS_MEM_TO_DEV	BIT(1)
+#define SUPPORTS_DEV_TO_MEM	BIT(2)
+#define SUPPORTS_DEV_TO_DEV	BIT(3)
+
+/*
+ * struct dma_ops - Driver model DMA operations
+ *
+ * The uclass interface is implemented by all DMA devices which use
+ * driver model.
+ */
+struct dma_ops {
+	/*
+	 * Get the current timer count
+	 *
+	 * @dev: The DMA device
+	 * @direction: direction of data transfer should be one from
+		       enum dma_direction
+	 * @dst: Destination pointer
+	 * @src: Source pointer
+	 * @len: Length of the data to be copied.
+	 * @return: 0 if OK, -ve on error
+	 */
+	int (*transfer)(struct udevice *dev, int direction, void *dst,
+			void *src, size_t len);
+};
+
+/*
+ * struct dma_dev_priv - information about a device used by the uclass
+ *
+ * @supported: mode of transfers that DMA can support
+ */
+struct dma_dev_priv {
+	unsigned long supported;
+};
+
+int dma_memcpy(void *dst, void *src, size_t len);
+
+#endif	/* _DMA_H_ */
-- 
2.6.3.368.gf34be46

[U-Boot] [PATCH 2/6] dma: Kconfig: Add TI_EDMA3 entry

[Mugunthan V N]

Add TI_EDMA3 entry on Kconfig with help description.

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 drivers/dma/Kconfig | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index af15199..1e2a8e1 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -12,4 +12,11 @@ config DM_DMA
 	  buses that is used to transfer data to and from memory.
 	  The uclass interface is defined in include/dma.h.
 
+config TI_EDMA3
+	bool "TI EDMA3 driver"
+	help
+	  Enable the TI EDMA3 driver for DRA7xx and AM43xx evms.
+	  This driver support data transfer between memory
+	  regions.
+
 endmenu # menu "DMA Support"
-- 
2.6.3.368.gf34be46

[U-Boot] [PATCH 3/6] sf: sf_ops: use dma to copy data from mmap region if platform supports

[Mugunthan V N]

Add dma memcpy api to the default spi_flash_copy_mmap(), so that
dma will be used to copy data when DM_DMA is defined for the
platform.

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 drivers/mtd/spi/sf_ops.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index 3a56d7f..3c83f8a 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -16,6 +16,7 @@
 #include <watchdog.h>
 #include <linux/compiler.h>
 #include <linux/log2.h>
+#include <dma.h>
 
 #include "sf_internal.h"
 
@@ -389,6 +390,10 @@ int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
 
 void __weak spi_flash_copy_mmap(void *data, void *offset, size_t len)
 {
+#ifdef CONFIG_DM_DMA
+	if (!dma_memcpy(data, offset, len))
+		return;
+#endif
 	memcpy(data, offset, len);
 }
 
-- 
2.6.3.368.gf34be46

[U-Boot] [PATCH 4/6] spi: ti_qspi: compile out spi_flash_copy_mmap when DM_DMA is defined

[Mugunthan V N]

When DM_DMA is defined the default spi_flash_copy_mmap() can
handle dma memory copy, so compile out spi_flash_copy_mmap() from
ti_qspi driver when DM_DMA config is defined.

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 drivers/spi/ti_qspi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c
index 945290e..f6cf9f4 100644
--- a/drivers/spi/ti_qspi.c
+++ b/drivers/spi/ti_qspi.c
@@ -277,7 +277,7 @@ static int __ti_qspi_xfer(struct ti_qspi_priv *priv, unsigned int bitlen,
 }
 
 /* TODO: control from sf layer to here through dm-spi */
-#ifdef CONFIG_TI_EDMA3
+#if defined(CONFIG_TI_EDMA3) && !defined(CONFIG_DM_DMA)
 void spi_flash_copy_mmap(void *data, void *offset, size_t len)
 {
 	unsigned int			addr = (unsigned int) (data);
-- 
2.6.3.368.gf34be46

[U-Boot] [PATCH 5/6] drivers: dma: ti-edma3: convert driver to adopt driver model

[Mugunthan V N]

adopt ti-edma3 driver to device driver model

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 drivers/dma/ti-edma3.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 73 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/ti-edma3.c b/drivers/dma/ti-edma3.c
index d6a427f..0eb8602 100644
--- a/drivers/dma/ti-edma3.c
+++ b/drivers/dma/ti-edma3.c
@@ -11,6 +11,9 @@
 
 #include <asm/io.h>
 #include <common.h>
+#include <dma.h>
+#include <dm/device.h>
+#include <asm/omap_common.h>
 #include <asm/ti-common/ti-edma3.h>
 
 #define EDMA3_SL_BASE(slot)			(0x4000 + ((slot) << 5))
@@ -31,6 +34,11 @@
 #define EDMA3_QEESR				0x108c
 #define EDMA3_QSECR				0x1094
 
+/* ti qspi priv */
+struct ti_edma3_priv {
+	u32 base;
+};
+
 /**
  * qedma3_start - start qdma on a channel
  * @base: base address of edma
@@ -383,8 +391,8 @@ void qedma3_stop(u32 base, struct edma3_channel_config *cfg)
 	__raw_writel(0, base + EDMA3_QCHMAP(cfg->chnum));
 }
 
-void edma3_transfer(unsigned long edma3_base_addr, unsigned int
-		    edma_slot_num, void *dst, void *src, size_t len)
+void __edma3_transfer(unsigned long edma3_base_addr, unsigned int edma_slot_num,
+		      void *dst, void *src, size_t len)
 {
 	struct edma3_slot_config        slot;
 	struct edma3_channel_config     edma_channel;
@@ -460,3 +468,66 @@ void edma3_transfer(unsigned long edma3_base_addr, unsigned int
 		qedma3_stop(edma3_base_addr, &edma_channel);
 	}
 }
+
+#ifndef CONFIG_DM_DMA
+
+void edma3_transfer(unsigned long edma3_base_addr, unsigned int edma_slot_num,
+		    void *dst, void *src, size_t len)
+{
+	__edma3_transfer(edma3_base_addr, edma_slot_num, dst, src, len);
+}
+
+#else
+
+static int ti_edma3_transfer(struct udevice *dev, int direction, void *dst,
+			     void *src, size_t len)
+{
+	struct ti_edma3_priv *priv = dev_get_priv(dev);
+
+	/* enable edma3 clocks */
+	enable_edma3_clocks();
+
+	__edma3_transfer(priv->base, 1, dst, src, len);
+
+	/* disable edma3 clocks */
+	disable_edma3_clocks();
+
+	return 0;
+}
+
+static int ti_edma3_probe(struct udevice *bus)
+{
+	/* Nothing to do in probe */
+	return 0;
+}
+
+static int ti_edma3_ofdata_to_platdata(struct udevice *dev)
+{
+	struct ti_edma3_priv *priv = dev_get_priv(dev);
+	struct dma_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+
+	priv->base = dev_get_addr(dev);
+	uc_priv->supported = SUPPORTS_MEM_TO_MEM;
+
+	return 0;
+}
+
+static const struct dma_ops ti_edma3_ops = {
+	.transfer	= ti_edma3_transfer,
+};
+
+static const struct udevice_id ti_edma3_ids[] = {
+	{ .compatible = "ti,edma3" },
+	{ }
+};
+
+U_BOOT_DRIVER(ti_edma3) = {
+	.name	= "ti_edma3",
+	.id	= UCLASS_DMA,
+	.of_match = ti_edma3_ids,
+	.ops	= &ti_edma3_ops,
+	.ofdata_to_platdata = ti_edma3_ofdata_to_platdata,
+	.priv_auto_alloc_size = sizeof(struct ti_edma3_priv),
+	.probe	= ti_edma3_probe,
+};
+#endif /* CONFIG_DM_DMA */
-- 
2.6.3.368.gf34be46

[U-Boot] [PATCH 6/6] defconfig: am437x_sk_evm: enable dma driver model

[Mugunthan V N]

enable dma driver model for am437x_sk_evm as ti-edma3 supports
driver model

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 configs/am437x_sk_evm_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/am437x_sk_evm_defconfig b/configs/am437x_sk_evm_defconfig
index 8f78eeb..fc617d6 100644
--- a/configs/am437x_sk_evm_defconfig
+++ b/configs/am437x_sk_evm_defconfig
@@ -21,3 +21,4 @@ CONFIG_TI_QSPI=y
 CONFIG_DM_SPI=y
 CONFIG_DM_SPI_FLASH=y
 CONFIG_SPI_FLASH_BAR=y
+CONFIG_DM_DMA=y
-- 
2.6.3.368.gf34be46

[U-Boot] [PATCH 1/6] dm: implement a DMA uclass

[Simon Glass]

Hi Mugunthan,

On 1 December 2015 at 04:13, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> Implement a DMA uclass so that the devices like ethernet, spi,
> mmc etc can offload the data transfers from/to the device and
> memory.
>
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
>  drivers/dma/Kconfig      | 15 ++++++++++++
>  drivers/dma/Makefile     |  2 ++
>  drivers/dma/dma-uclass.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/dm/uclass-id.h   |  1 +
>  include/dma.h            | 64 ++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 145 insertions(+)
>  create mode 100644 drivers/dma/dma-uclass.c
>  create mode 100644 include/dma.h
>
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index e69de29..af15199 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -0,0 +1,15 @@
> +menu "DMA Support"
> +
> +config DM_DMA
> +       bool "Enable Driver Model for DMA drivers"
> +       depends on DM
> +       help
> +         Enable driver model for DMA. DMA engines can do
> +         asynchronous data transfers without involving the host
> +         CPU. Currently, this framework can be used to offload
> +         memory copies to and from devices like qspi, ethernet
> +         etc Drivers provide methods to access the DMA devices
> +         buses that is used to transfer data to and from memory.
> +         The uclass interface is defined in include/dma.h.
> +
> +endmenu # menu "DMA Support"
> diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> index f95fe70..4ff22aa 100644
> --- a/drivers/dma/Makefile
> +++ b/drivers/dma/Makefile
> @@ -5,6 +5,8 @@
>  # SPDX-License-Identifier:     GPL-2.0+
>  #
>
> +obj-$(CONFIG_DM_DMA) += dma-uclass.o

You can just use CONFIG_DMA. The DM_ prefix is for when we have an
existing pre-DM interface. Eventually all the CONFIG_DM_... options
will be removed. Unless you have a special reason?

> +
>  obj-$(CONFIG_FSLDMAFEC) += MCD_tasksInit.o MCD_dmaApi.o MCD_tasks.o
>  obj-$(CONFIG_APBH_DMA) += apbh_dma.o
>  obj-$(CONFIG_FSL_DMA) += fsl_dma.o
> diff --git a/drivers/dma/dma-uclass.c b/drivers/dma/dma-uclass.c
> new file mode 100644
> index 0000000..1afcbaa4
> --- /dev/null
> +++ b/drivers/dma/dma-uclass.c
> @@ -0,0 +1,63 @@
> +/*
> + * Direct Memory Access U-Class driver
> + *
> + * (C) Copyright 2015
> + *     Texas Instruments Incorporated, <www.ti.com>
> + *
> + * Author: Mugunthan V N <mugunthanvnm@ti.com>
> + *
> + * SPDX-License-Identifier:     GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <dma.h>
> +#include <dm.h>
> +#include <dm/uclass-internal.h>
> +#include <errno.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +int dma_memcpy(void *dst, void *src, size_t len)
> +{
> +       struct udevice *dev;
> +       const struct dma_ops *ops;
> +       int ret;
> +
> +       for (ret = uclass_find_first_device(UCLASS_DMA, &dev); dev && !ret;
> +            ret = uclass_find_next_device(&dev)) {
> +               struct dma_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> +
> +               if (uc_priv->supported & SUPPORTS_MEM_TO_MEM)
> +                       break;
> +       }
> +
> +       if (!dev) {
> +               printf("No DMA device found that supports mem to mem transfers\n");
> +               return -ENODEV;
> +       }

Can you put the above code in a separate function like:

static int get_dma_device(struct udevice **devp)

also also please call device_probe() on it before returning it.

> +
> +       ops = device_get_ops(dev);
> +       if (!ops->transfer)
> +               return -ENOSYS;
> +
> +       /* Invalidate the area, so no writeback into the RAM races with DMA */
> +       invalidate_dcache_range((unsigned long)dst, (unsigned long)dst +
> +                               roundup(len, ARCH_DMA_MINALIGN));
> +
> +       return ops->transfer(dev, DMA_MEM_TO_MEM, dst, src, len);
> +}
> +
> +int dma_post_bind(struct udevice *dev)
> +{
> +       struct udevice *devp;
> +       uclass_get_device_by_of_offset(UCLASS_DMA, dev->of_offset, &devp);

You are not allowed to do this in the post_bind() method. See above
for how it should get probed.

> +       return 0;
> +}
> +
> +UCLASS_DRIVER(dma) = {
> +       .id             = UCLASS_DMA,
> +       .name           = "dma",
> +       .flags          = DM_UC_FLAG_SEQ_ALIAS,
> +       .per_device_auto_alloc_size = sizeof(struct dma_dev_priv),
> +       .post_bind      = dma_post_bind,
> +};
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 27fa0b6..9f5fcae 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -30,6 +30,7 @@ enum uclass_id {
>         UCLASS_CPU,             /* CPU, typically part of an SoC */
>         UCLASS_CROS_EC,         /* Chrome OS EC */
>         UCLASS_DISPLAY_PORT,    /* Display port video */
> +       UCLASS_DMA,             /* Direct Memory Access */
>         UCLASS_RAM,             /* RAM controller */
>         UCLASS_ETH,             /* Ethernet device */
>         UCLASS_GPIO,            /* Bank of general-purpose I/O pins */
> diff --git a/include/dma.h b/include/dma.h
> new file mode 100644
> index 0000000..d53e435
> --- /dev/null
> +++ b/include/dma.h
> @@ -0,0 +1,64 @@
> +/*
> + * (C) Copyright 2015
> + *     Texas Instruments Incorporated, <www.ti.com>
> + *
> + * SPDX-License-Identifier:     GPL-2.0+
> + */
> +
> +#ifndef _DMA_H_
> +#define _DMA_H_
> +
> +/*
> + * enum dma_direction - dma transfer direction indicator
> + * @DMA_MEM_TO_MEM: Memcpy mode
> + * @DMA_MEM_TO_DEV: From Memory to Device
> + * @DMA_DEV_TO_MEM: From Device to Memory
> + * @DMA_DEV_TO_DEV: From Device to Device
> + */
> +enum dma_direction {
> +       DMA_MEM_TO_MEM,
> +       DMA_MEM_TO_DEV,
> +       DMA_DEV_TO_MEM,
> +       DMA_DEV_TO_DEV,
> +       DMA_TRANS_NONE,

What does that mean?

> +};
> +
> +#define SUPPORTS_MEM_TO_MEM    BIT(0)
> +#define SUPPORTS_MEM_TO_DEV    BIT(1)
> +#define SUPPORTS_DEV_TO_MEM    BIT(2)
> +#define SUPPORTS_DEV_TO_DEV    BIT(3)

DMA_ prefix on these. Also you could put them in an enum so they are grouped.

> +
> +/*
> + * struct dma_ops - Driver model DMA operations
> + *
> + * The uclass interface is implemented by all DMA devices which use
> + * driver model.
> + */
> +struct dma_ops {
> +       /*
> +        * Get the current timer count

Incorrect comment.

> +        *
> +        * @dev: The DMA device
> +        * @direction: direction of data transfer should be one from
> +                      enum dma_direction
> +        * @dst: Destination pointer
> +        * @src: Source pointer

Any alignment restructions?

> +        * @len: Length of the data to be copied.

In bytes?

> +        * @return: 0 if OK, -ve on error
> +        */
> +       int (*transfer)(struct udevice *dev, int direction, void *dst,

s/int direction/enum dma_direction direction/

> +                       void *src, size_t len);
> +};
> +
> +/*
> + * struct dma_dev_priv - information about a device used by the uclass
> + *
> + * @supported: mode of transfers that DMA can support

Please reference where these flags come from.

> + */
> +struct dma_dev_priv {
> +       unsigned long supported;

int or uint should be enough.

> +};
> +
> +int dma_memcpy(void *dst, void *src, size_t len);
> +
> +#endif /* _DMA_H_ */
> --
> 2.6.3.368.gf34be46
>

Regards,
Simon

[U-Boot] [PATCH 2/6] dma: Kconfig: Add TI_EDMA3 entry

[Simon Glass]

On 1 December 2015 at 04:13, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> Add TI_EDMA3 entry on Kconfig with help description.
>
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
>  drivers/dma/Kconfig | 7 +++++++
>  1 file changed, 7 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 3/6] sf: sf_ops: use dma to copy data from mmap region if platform supports

[Simon Glass]

Hi,

On 1 December 2015 at 04:13, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> Add dma memcpy api to the default spi_flash_copy_mmap(), so that
> dma will be used to copy data when DM_DMA is defined for the
> platform.
>
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
>  drivers/mtd/spi/sf_ops.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
> index 3a56d7f..3c83f8a 100644
> --- a/drivers/mtd/spi/sf_ops.c
> +++ b/drivers/mtd/spi/sf_ops.c
> @@ -16,6 +16,7 @@
>  #include <watchdog.h>
>  #include <linux/compiler.h>
>  #include <linux/log2.h>
> +#include <dma.h>
>
>  #include "sf_internal.h"
>
> @@ -389,6 +390,10 @@ int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
>
>  void __weak spi_flash_copy_mmap(void *data, void *offset, size_t len)
>  {
> +#ifdef CONFIG_DM_DMA
> +       if (!dma_memcpy(data, offset, len))
> +               return;
> +#endif
>         memcpy(data, offset, len);
>  }
>
> --
> 2.6.3.368.gf34be46
>

This looks like one driver (SPI flash) calling a weak function in
another (SPI). Why isn't this done with the driver interface?

Regards,
Simon

[U-Boot] [PATCH 4/6] spi: ti_qspi: compile out spi_flash_copy_mmap when DM_DMA is defined

[Simon Glass]

Hi,

On 1 December 2015 at 04:13, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> When DM_DMA is defined the default spi_flash_copy_mmap() can
> handle dma memory copy, so compile out spi_flash_copy_mmap() from
> ti_qspi driver when DM_DMA config is defined.
>
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
>  drivers/spi/ti_qspi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c
> index 945290e..f6cf9f4 100644
> --- a/drivers/spi/ti_qspi.c
> +++ b/drivers/spi/ti_qspi.c
> @@ -277,7 +277,7 @@ static int __ti_qspi_xfer(struct ti_qspi_priv *priv, unsigned int bitlen,
>  }
>
>  /* TODO: control from sf layer to here through dm-spi */
> -#ifdef CONFIG_TI_EDMA3
> +#if defined(CONFIG_TI_EDMA3) && !defined(CONFIG_DM_DMA)
>  void spi_flash_copy_mmap(void *data, void *offset, size_t len)
>  {
>         unsigned int                    addr = (unsigned int) (data);
> --
> 2.6.3.368.gf34be46
>

This is also odd - shouldn't it be an option for the SPI driver,
perhaps in the device tree?

Regards,
Simon

[U-Boot] [PATCH 5/6] drivers: dma: ti-edma3: convert driver to adopt driver model

[Simon Glass]

Hi Mugunthan,

On 1 December 2015 at 04:13, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> adopt ti-edma3 driver to device driver model
>
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
>  drivers/dma/ti-edma3.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 73 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/ti-edma3.c b/drivers/dma/ti-edma3.c
> index d6a427f..0eb8602 100644
> --- a/drivers/dma/ti-edma3.c
> +++ b/drivers/dma/ti-edma3.c
> @@ -11,6 +11,9 @@
>
>  #include <asm/io.h>
>  #include <common.h>
> +#include <dma.h>
> +#include <dm/device.h>
> +#include <asm/omap_common.h>
>  #include <asm/ti-common/ti-edma3.h>
>
>  #define EDMA3_SL_BASE(slot)                    (0x4000 + ((slot) << 5))
> @@ -31,6 +34,11 @@
>  #define EDMA3_QEESR                            0x108c
>  #define EDMA3_QSECR                            0x1094
>
> +/* ti qspi priv */

May as well drop this comment as it doesn't help.

> +struct ti_edma3_priv {
> +       u32 base;
> +};
> +
>  /**
>   * qedma3_start - start qdma on a channel
>   * @base: base address of edma
> @@ -383,8 +391,8 @@ void qedma3_stop(u32 base, struct edma3_channel_config *cfg)
>         __raw_writel(0, base + EDMA3_QCHMAP(cfg->chnum));
>  }
>
> -void edma3_transfer(unsigned long edma3_base_addr, unsigned int
> -                   edma_slot_num, void *dst, void *src, size_t len)
> +void __edma3_transfer(unsigned long edma3_base_addr, unsigned int edma_slot_num,
> +                     void *dst, void *src, size_t len)
>  {
>         struct edma3_slot_config        slot;
>         struct edma3_channel_config     edma_channel;
> @@ -460,3 +468,66 @@ void edma3_transfer(unsigned long edma3_base_addr, unsigned int
>                 qedma3_stop(edma3_base_addr, &edma_channel);
>         }
>  }
> +
> +#ifndef CONFIG_DM_DMA
> +
> +void edma3_transfer(unsigned long edma3_base_addr, unsigned int edma_slot_num,
> +                   void *dst, void *src, size_t len)
> +{
> +       __edma3_transfer(edma3_base_addr, edma_slot_num, dst, src, len);
> +}
> +
> +#else
> +
> +static int ti_edma3_transfer(struct udevice *dev, int direction, void *dst,
> +                            void *src, size_t len)
> +{
> +       struct ti_edma3_priv *priv = dev_get_priv(dev);
> +
> +       /* enable edma3 clocks */
> +       enable_edma3_clocks();
> +
> +       __edma3_transfer(priv->base, 1, dst, src, len);

What happens to direction?

> +
> +       /* disable edma3 clocks */
> +       disable_edma3_clocks();
> +
> +       return 0;
> +}
> +
> +static int ti_edma3_probe(struct udevice *bus)
> +{
> +       /* Nothing to do in probe */

You can omit this function.

> +       return 0;
> +}
> +
> +static int ti_edma3_ofdata_to_platdata(struct udevice *dev)
> +{
> +       struct ti_edma3_priv *priv = dev_get_priv(dev);
> +       struct dma_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> +
> +       priv->base = dev_get_addr(dev);
> +       uc_priv->supported = SUPPORTS_MEM_TO_MEM;
> +
> +       return 0;
> +}
> +
> +static const struct dma_ops ti_edma3_ops = {
> +       .transfer       = ti_edma3_transfer,
> +};
> +
> +static const struct udevice_id ti_edma3_ids[] = {
> +       { .compatible = "ti,edma3" },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(ti_edma3) = {
> +       .name   = "ti_edma3",
> +       .id     = UCLASS_DMA,
> +       .of_match = ti_edma3_ids,
> +       .ops    = &ti_edma3_ops,
> +       .ofdata_to_platdata = ti_edma3_ofdata_to_platdata,
> +       .priv_auto_alloc_size = sizeof(struct ti_edma3_priv),
> +       .probe  = ti_edma3_probe,
> +};
> +#endif /* CONFIG_DM_DMA */
> --
> 2.6.3.368.gf34be46
>


Regards,
Simon

[U-Boot] [PATCH 6/6] defconfig: am437x_sk_evm: enable dma driver model

[Simon Glass]

On 1 December 2015 at 04:13, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> enable dma driver model for am437x_sk_evm as ti-edma3 supports
> driver model
>
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
>  configs/am437x_sk_evm_defconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/configs/am437x_sk_evm_defconfig b/configs/am437x_sk_evm_defconfig
> index 8f78eeb..fc617d6 100644
> --- a/configs/am437x_sk_evm_defconfig
> +++ b/configs/am437x_sk_evm_defconfig
> @@ -21,3 +21,4 @@ CONFIG_TI_QSPI=y
>  CONFIG_DM_SPI=y
>  CONFIG_DM_SPI_FLASH=y
>  CONFIG_SPI_FLASH_BAR=y
> +CONFIG_DM_DMA=y
> --
> 2.6.3.368.gf34be46
>

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 1/6] dm: implement a DMA uclass

[Mugunthan V N]

On Wednesday 02 December 2015 03:24 AM, Simon Glass wrote:
> Hi Mugunthan,
> 
> On 1 December 2015 at 04:13, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>> Implement a DMA uclass so that the devices like ethernet, spi,
>> mmc etc can offload the data transfers from/to the device and
>> memory.
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>> ---
>>  drivers/dma/Kconfig      | 15 ++++++++++++
>>  drivers/dma/Makefile     |  2 ++
>>  drivers/dma/dma-uclass.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++
>>  include/dm/uclass-id.h   |  1 +
>>  include/dma.h            | 64 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 145 insertions(+)
>>  create mode 100644 drivers/dma/dma-uclass.c
>>  create mode 100644 include/dma.h
>>
>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>> index e69de29..af15199 100644
>> --- a/drivers/dma/Kconfig
>> +++ b/drivers/dma/Kconfig
>> @@ -0,0 +1,15 @@
>> +menu "DMA Support"
>> +
>> +config DM_DMA
>> +       bool "Enable Driver Model for DMA drivers"
>> +       depends on DM
>> +       help
>> +         Enable driver model for DMA. DMA engines can do
>> +         asynchronous data transfers without involving the host
>> +         CPU. Currently, this framework can be used to offload
>> +         memory copies to and from devices like qspi, ethernet
>> +         etc Drivers provide methods to access the DMA devices
>> +         buses that is used to transfer data to and from memory.
>> +         The uclass interface is defined in include/dma.h.
>> +
>> +endmenu # menu "DMA Support"
>> diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
>> index f95fe70..4ff22aa 100644
>> --- a/drivers/dma/Makefile
>> +++ b/drivers/dma/Makefile
>> @@ -5,6 +5,8 @@
>>  # SPDX-License-Identifier:     GPL-2.0+
>>  #
>>
>> +obj-$(CONFIG_DM_DMA) += dma-uclass.o
> 
> You can just use CONFIG_DMA. The DM_ prefix is for when we have an
> existing pre-DM interface. Eventually all the CONFIG_DM_... options
> will be removed. Unless you have a special reason?

For consistency I used CONFIG_DM_DMA, will rename it to CONFIG_DMA.

> 
>> +
>>  obj-$(CONFIG_FSLDMAFEC) += MCD_tasksInit.o MCD_dmaApi.o MCD_tasks.o
>>  obj-$(CONFIG_APBH_DMA) += apbh_dma.o
>>  obj-$(CONFIG_FSL_DMA) += fsl_dma.o
>> diff --git a/drivers/dma/dma-uclass.c b/drivers/dma/dma-uclass.c
>> new file mode 100644
>> index 0000000..1afcbaa4
>> --- /dev/null
>> +++ b/drivers/dma/dma-uclass.c
>> @@ -0,0 +1,63 @@
>> +/*
>> + * Direct Memory Access U-Class driver
>> + *
>> + * (C) Copyright 2015
>> + *     Texas Instruments Incorporated, <www.ti.com>
>> + *
>> + * Author: Mugunthan V N <mugunthanvnm@ti.com>
>> + *
>> + * SPDX-License-Identifier:     GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <dma.h>
>> +#include <dm.h>
>> +#include <dm/uclass-internal.h>
>> +#include <errno.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +int dma_memcpy(void *dst, void *src, size_t len)
>> +{
>> +       struct udevice *dev;
>> +       const struct dma_ops *ops;
>> +       int ret;
>> +
>> +       for (ret = uclass_find_first_device(UCLASS_DMA, &dev); dev && !ret;
>> +            ret = uclass_find_next_device(&dev)) {
>> +               struct dma_dev_priv *uc_priv = dev_get_uclass_priv(dev);
>> +
>> +               if (uc_priv->supported & SUPPORTS_MEM_TO_MEM)
>> +                       break;
>> +       }
>> +
>> +       if (!dev) {
>> +               printf("No DMA device found that supports mem to mem transfers\n");
>> +               return -ENODEV;
>> +       }
> 
> Can you put the above code in a separate function like:
> 
> static int get_dma_device(struct udevice **devp)
> 
> also also please call device_probe() on it before returning it.

Okay, will do it in next version.

> 
>> +
>> +       ops = device_get_ops(dev);
>> +       if (!ops->transfer)
>> +               return -ENOSYS;
>> +
>> +       /* Invalidate the area, so no writeback into the RAM races with DMA */
>> +       invalidate_dcache_range((unsigned long)dst, (unsigned long)dst +
>> +                               roundup(len, ARCH_DMA_MINALIGN));
>> +
>> +       return ops->transfer(dev, DMA_MEM_TO_MEM, dst, src, len);
>> +}
>> +
>> +int dma_post_bind(struct udevice *dev)
>> +{
>> +       struct udevice *devp;
>> +       uclass_get_device_by_of_offset(UCLASS_DMA, dev->of_offset, &devp);
> 
> You are not allowed to do this in the post_bind() method. See above
> for how it should get probed.
> 

Okay, agreed.

>> +       return 0;
>> +}
>> +
>> +UCLASS_DRIVER(dma) = {
>> +       .id             = UCLASS_DMA,
>> +       .name           = "dma",
>> +       .flags          = DM_UC_FLAG_SEQ_ALIAS,
>> +       .per_device_auto_alloc_size = sizeof(struct dma_dev_priv),
>> +       .post_bind      = dma_post_bind,
>> +};
>> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
>> index 27fa0b6..9f5fcae 100644
>> --- a/include/dm/uclass-id.h
>> +++ b/include/dm/uclass-id.h
>> @@ -30,6 +30,7 @@ enum uclass_id {
>>         UCLASS_CPU,             /* CPU, typically part of an SoC */
>>         UCLASS_CROS_EC,         /* Chrome OS EC */
>>         UCLASS_DISPLAY_PORT,    /* Display port video */
>> +       UCLASS_DMA,             /* Direct Memory Access */
>>         UCLASS_RAM,             /* RAM controller */
>>         UCLASS_ETH,             /* Ethernet device */
>>         UCLASS_GPIO,            /* Bank of general-purpose I/O pins */
>> diff --git a/include/dma.h b/include/dma.h
>> new file mode 100644
>> index 0000000..d53e435
>> --- /dev/null
>> +++ b/include/dma.h
>> @@ -0,0 +1,64 @@
>> +/*
>> + * (C) Copyright 2015
>> + *     Texas Instruments Incorporated, <www.ti.com>
>> + *
>> + * SPDX-License-Identifier:     GPL-2.0+
>> + */
>> +
>> +#ifndef _DMA_H_
>> +#define _DMA_H_
>> +
>> +/*
>> + * enum dma_direction - dma transfer direction indicator
>> + * @DMA_MEM_TO_MEM: Memcpy mode
>> + * @DMA_MEM_TO_DEV: From Memory to Device
>> + * @DMA_DEV_TO_MEM: From Device to Memory
>> + * @DMA_DEV_TO_DEV: From Device to Device
>> + */
>> +enum dma_direction {
>> +       DMA_MEM_TO_MEM,
>> +       DMA_MEM_TO_DEV,
>> +       DMA_DEV_TO_MEM,
>> +       DMA_DEV_TO_DEV,
>> +       DMA_TRANS_NONE,
> 
> What does that mean?

DMA driver will be having only one transfer api and to identify what
kind of transfer that the user driver is expecting, these enums are used.

> 
>> +};
>> +
>> +#define SUPPORTS_MEM_TO_MEM    BIT(0)
>> +#define SUPPORTS_MEM_TO_DEV    BIT(1)
>> +#define SUPPORTS_DEV_TO_MEM    BIT(2)
>> +#define SUPPORTS_DEV_TO_DEV    BIT(3)
> 
> DMA_ prefix on these. Also you could put them in an enum so they are grouped.

Will add DMA prefix to these defines.

Since a dma driver can support multiple transfer types I kept this as
bit level definitions, so that driver can intimate DMA framework like
uc_priv->supported = SUPPORTS_MEM_TO_MEM | SUPPORTS_MEM_TO_DEV;

> 
>> +
>> +/*
>> + * struct dma_ops - Driver model DMA operations
>> + *
>> + * The uclass interface is implemented by all DMA devices which use
>> + * driver model.
>> + */
>> +struct dma_ops {
>> +       /*
>> +        * Get the current timer count
> 
> Incorrect comment.
> 
>> +        *
>> +        * @dev: The DMA device
>> +        * @direction: direction of data transfer should be one from
>> +                      enum dma_direction
>> +        * @dst: Destination pointer
>> +        * @src: Source pointer
> 
> Any alignment restructions?

I should be aligned to DMA device alignment restrictions.

> 
>> +        * @len: Length of the data to be copied.
> 
> In bytes?

Okay

> 
>> +        * @return: 0 if OK, -ve on error
>> +        */
>> +       int (*transfer)(struct udevice *dev, int direction, void *dst,
> 
> s/int direction/enum dma_direction direction/

Okay

> 
>> +                       void *src, size_t len);
>> +};
>> +
>> +/*
>> + * struct dma_dev_priv - information about a device used by the uclass
>> + *
>> + * @supported: mode of transfers that DMA can support
> 
> Please reference where these flags come from.

Okay

> 
>> + */
>> +struct dma_dev_priv {
>> +       unsigned long supported;
> 
> int or uint should be enough.

Will used uint.

> 
>> +};
>> +
>> +int dma_memcpy(void *dst, void *src, size_t len);
>> +
>> +#endif /* _DMA_H_ */
>> --
>> 2.6.3.368.gf34be46
>>
> 
> Regards,
> Simon
> 

Regards
Mugunthan V N

[U-Boot] [PATCH 1/6] dm: implement a DMA uclass

[Mugunthan V N]

On Wednesday 02 December 2015 01:47 PM, Mugunthan V N wrote:
>>> +       DMA_TRANS_NONE,
>> > 
>> > What does that mean?
> DMA driver will be having only one transfer api and to identify what
> kind of transfer that the user driver is expecting, these enums are used.
> 

Oops, I was taking your comment for the whole enums. Thought of having
an error enum as DMA_TRANS_NONE, but it doesn't add value, will remove
it in next version.

Regards
Mugunthan V N

[U-Boot] [PATCH 3/6] sf: sf_ops: use dma to copy data from mmap region if platform supports

[Mugunthan V N]

On Wednesday 02 December 2015 03:24 AM, Simon Glass wrote:
> Hi,
> 
> On 1 December 2015 at 04:13, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>> Add dma memcpy api to the default spi_flash_copy_mmap(), so that
>> dma will be used to copy data when DM_DMA is defined for the
>> platform.
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>> ---
>>  drivers/mtd/spi/sf_ops.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
>> index 3a56d7f..3c83f8a 100644
>> --- a/drivers/mtd/spi/sf_ops.c
>> +++ b/drivers/mtd/spi/sf_ops.c
>> @@ -16,6 +16,7 @@
>>  #include <watchdog.h>
>>  #include <linux/compiler.h>
>>  #include <linux/log2.h>
>> +#include <dma.h>
>>
>>  #include "sf_internal.h"
>>
>> @@ -389,6 +390,10 @@ int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
>>
>>  void __weak spi_flash_copy_mmap(void *data, void *offset, size_t len)
>>  {
>> +#ifdef CONFIG_DM_DMA
>> +       if (!dma_memcpy(data, offset, len))
>> +               return;
>> +#endif
>>         memcpy(data, offset, len);
>>  }
>>
>> --
>> 2.6.3.368.gf34be46
>>
> 
> This looks like one driver (SPI flash) calling a weak function in
> another (SPI). Why isn't this done with the driver interface?
> 

This is suppose to be with SPI flash driver, as the flash driver can
only decide whether to use memory map transfers or spi transfers.

It was kept as a weak function so that platforms with DMA support can
have their own spi_flash_copy_mmap() to transfer data with DMA support.
But for some reasons this implementations landed in spi driver files
which is not the best place, it should have to be done in DMA drivers.

With CONFIG_DMA, this should go away with the above #ifdef CONFIG_DMA
and the weak function attribute can be removed later.

Regards
Mugunthan V N

[U-Boot] [PATCH 5/6] drivers: dma: ti-edma3: convert driver to adopt driver model

[Mugunthan V N]

On Wednesday 02 December 2015 03:24 AM, Simon Glass wrote:
> Hi Mugunthan,
> 
> On 1 December 2015 at 04:13, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>> adopt ti-edma3 driver to device driver model
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>> ---
>>  drivers/dma/ti-edma3.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 73 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/dma/ti-edma3.c b/drivers/dma/ti-edma3.c
>> index d6a427f..0eb8602 100644
>> --- a/drivers/dma/ti-edma3.c
>> +++ b/drivers/dma/ti-edma3.c
>> @@ -11,6 +11,9 @@
>>
>>  #include <asm/io.h>
>>  #include <common.h>
>> +#include <dma.h>
>> +#include <dm/device.h>
>> +#include <asm/omap_common.h>
>>  #include <asm/ti-common/ti-edma3.h>
>>
>>  #define EDMA3_SL_BASE(slot)                    (0x4000 + ((slot) << 5))
>> @@ -31,6 +34,11 @@
>>  #define EDMA3_QEESR                            0x108c
>>  #define EDMA3_QSECR                            0x1094
>>
>> +/* ti qspi priv */
> 
> May as well drop this comment as it doesn't help.
> 
>> +struct ti_edma3_priv {
>> +       u32 base;
>> +};
>> +
>>  /**
>>   * qedma3_start - start qdma on a channel
>>   * @base: base address of edma
>> @@ -383,8 +391,8 @@ void qedma3_stop(u32 base, struct edma3_channel_config *cfg)
>>         __raw_writel(0, base + EDMA3_QCHMAP(cfg->chnum));
>>  }
>>
>> -void edma3_transfer(unsigned long edma3_base_addr, unsigned int
>> -                   edma_slot_num, void *dst, void *src, size_t len)
>> +void __edma3_transfer(unsigned long edma3_base_addr, unsigned int edma_slot_num,
>> +                     void *dst, void *src, size_t len)
>>  {
>>         struct edma3_slot_config        slot;
>>         struct edma3_channel_config     edma_channel;
>> @@ -460,3 +468,66 @@ void edma3_transfer(unsigned long edma3_base_addr, unsigned int
>>                 qedma3_stop(edma3_base_addr, &edma_channel);
>>         }
>>  }
>> +
>> +#ifndef CONFIG_DM_DMA
>> +
>> +void edma3_transfer(unsigned long edma3_base_addr, unsigned int edma_slot_num,
>> +                   void *dst, void *src, size_t len)
>> +{
>> +       __edma3_transfer(edma3_base_addr, edma_slot_num, dst, src, len);
>> +}
>> +
>> +#else
>> +
>> +static int ti_edma3_transfer(struct udevice *dev, int direction, void *dst,
>> +                            void *src, size_t len)
>> +{
>> +       struct ti_edma3_priv *priv = dev_get_priv(dev);
>> +
>> +       /* enable edma3 clocks */
>> +       enable_edma3_clocks();
>> +
>> +       __edma3_transfer(priv->base, 1, dst, src, len);
> 
> What happens to direction?

Missed to check the direction, will add in next version.

> 
>> +
>> +       /* disable edma3 clocks */
>> +       disable_edma3_clocks();
>> +
>> +       return 0;
>> +}
>> +
>> +static int ti_edma3_probe(struct udevice *bus)
>> +{
>> +       /* Nothing to do in probe */
> 
> You can omit this function.

Okay.

> 
>> +       return 0;
>> +}
>> +
>> +static int ti_edma3_ofdata_to_platdata(struct udevice *dev)
>> +{
>> +       struct ti_edma3_priv *priv = dev_get_priv(dev);
>> +       struct dma_dev_priv *uc_priv = dev_get_uclass_priv(dev);
>> +
>> +       priv->base = dev_get_addr(dev);
>> +       uc_priv->supported = SUPPORTS_MEM_TO_MEM;
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct dma_ops ti_edma3_ops = {
>> +       .transfer       = ti_edma3_transfer,
>> +};
>> +
>> +static const struct udevice_id ti_edma3_ids[] = {
>> +       { .compatible = "ti,edma3" },
>> +       { }
>> +};
>> +
>> +U_BOOT_DRIVER(ti_edma3) = {
>> +       .name   = "ti_edma3",
>> +       .id     = UCLASS_DMA,
>> +       .of_match = ti_edma3_ids,
>> +       .ops    = &ti_edma3_ops,
>> +       .ofdata_to_platdata = ti_edma3_ofdata_to_platdata,
>> +       .priv_auto_alloc_size = sizeof(struct ti_edma3_priv),
>> +       .probe  = ti_edma3_probe,
>> +};
>> +#endif /* CONFIG_DM_DMA */
>> --
>> 2.6.3.368.gf34be46
>>
> 

Regards
Mugunthan V N

[U-Boot] [PATCH 3/6] sf: sf_ops: use dma to copy data from mmap region if platform supports

[Simon Glass]

Hi Magunthan,

On 2 December 2015 at 02:45, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> On Wednesday 02 December 2015 03:24 AM, Simon Glass wrote:
>> Hi,
>>
>> On 1 December 2015 at 04:13, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>>> Add dma memcpy api to the default spi_flash_copy_mmap(), so that
>>> dma will be used to copy data when DM_DMA is defined for the
>>> platform.
>>>
>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>>> ---
>>>  drivers/mtd/spi/sf_ops.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
>>> index 3a56d7f..3c83f8a 100644
>>> --- a/drivers/mtd/spi/sf_ops.c
>>> +++ b/drivers/mtd/spi/sf_ops.c
>>> @@ -16,6 +16,7 @@
>>>  #include <watchdog.h>
>>>  #include <linux/compiler.h>
>>>  #include <linux/log2.h>
>>> +#include <dma.h>
>>>
>>>  #include "sf_internal.h"
>>>
>>> @@ -389,6 +390,10 @@ int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
>>>
>>>  void __weak spi_flash_copy_mmap(void *data, void *offset, size_t len)
>>>  {
>>> +#ifdef CONFIG_DM_DMA
>>> +       if (!dma_memcpy(data, offset, len))
>>> +               return;
>>> +#endif
>>>         memcpy(data, offset, len);
>>>  }
>>>
>>> --
>>> 2.6.3.368.gf34be46
>>>
>>
>> This looks like one driver (SPI flash) calling a weak function in
>> another (SPI). Why isn't this done with the driver interface?
>>
>
> This is suppose to be with SPI flash driver, as the flash driver can
> only decide whether to use memory map transfers or spi transfers.
>
> It was kept as a weak function so that platforms with DMA support can
> have their own spi_flash_copy_mmap() to transfer data with DMA support.
> But for some reasons this implementations landed in spi driver files
> which is not the best place, it should have to be done in DMA drivers.
>
> With CONFIG_DMA, this should go away with the above #ifdef CONFIG_DMA
> and the weak function attribute can be removed later.

OK that sounds good, please add a TODO for this.

Reviewed-by: Simon Glass <sjg@chromium.org>

Regards,
Simon