[PATCH 0/7] Add support for FDMA DMA controller found on STi chipsets

[PATCH 0/7] Add support for FDMA DMA controller found on STi chipsets

[Peter Griffin]

Hi,

This patchset adds support for the Flexible Direct Memory Access (FDMA) core
found on STi chipsets from STMicroelectronics. The FDMA is a slim core CPU
with a dedicated firmware. It is a general purpose DMA controller supporting
16 independent channels and data can be moved from memory to memory or between
memory and paced latency critical real time targets.

To increase the number of peripheral request, support is also added for the
FDMA crossbar hardware, which can be used to multiplex up to 96 peripheral
requests, to the 3 fdma engines.

regards,

Peter.

Peter Griffin (7):
  dmaengine: st_fdma: Add STMicroelectronics FDMA DT binding
    documentation
  dmaengine: st_fdma: Add STMicroelectronics FDMA xbar DT binding
    documentation
  dmaengine: st_fdma: Add STMicroelectronics FDMA engine driver support
  dmaengine: st_fdma: Add xbar support
  ARM: STi: DT: STiH407: Add FDMA driver and xbar driver dt nodes.
  MAINTAINERS: Add FDMA driver files to STi section.
  ARM: multi_v7_defconfig: Enable STi FDMA driver

 Documentation/devicetree/bindings/dma/st_fdma.txt |  100 ++
 MAINTAINERS                                       |    2 +
 arch/arm/boot/dts/stih407-family.dtsi             |   59 +
 arch/arm/configs/multi_v7_defconfig               |    2 +
 drivers/dma/Kconfig                               |   26 +
 drivers/dma/Makefile                              |    2 +
 drivers/dma/st_fdma.c                             | 1203 +++++++++++++++++++++
 drivers/dma/st_fdma.h                             |  266 +++++
 drivers/dma/st_fdma_xbar.c                        |  149 +++
 include/linux/platform_data/dma-st_fdma.h         |   70 ++
 10 files changed, 1879 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/st_fdma.txt
 create mode 100644 drivers/dma/st_fdma.c
 create mode 100644 drivers/dma/st_fdma.h
 create mode 100644 drivers/dma/st_fdma_xbar.c
 create mode 100644 include/linux/platform_data/dma-st_fdma.h

-- 
1.9.1

[PATCH 1/7] dmaengine: st_fdma: Add STMicroelectronics FDMA DT binding documentation

[Peter Griffin]

This patch adds the DT binding documentation for the FDMA constroller
found on STi based chipsets from STMicroelectronics.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 Documentation/devicetree/bindings/dma/st_fdma.txt | 76 +++++++++++++++++++++++
 1 file changed, 76 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/st_fdma.txt

diff --git a/Documentation/devicetree/bindings/dma/st_fdma.txt b/Documentation/devicetree/bindings/dma/st_fdma.txt
new file mode 100644
index 0000000..1ec7470
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/st_fdma.txt
@@ -0,0 +1,76 @@
+* STMicroelectronics Flexible Direct Memory Access Device Tree bindings
+
+The FDMA is a general-purpose direct memory access controller capable of
+supporting 16 independent DMA channels. It accepts up to 32 DMA requests.
+The FDMA is based on a Slim processor which require a firmware.
+
+* FDMA Controller
+
+Required properties:
+- compatible	: Should be "st,fdma_mpe31"
+- reg		: Should contain DMA registers location and length
+- interrupts	: Should contain one interrupt shared by all channel
+- dma-channels	: Number of channels supported by the controller
+- #dma-cells	: Must be <3>.
+- st,fw-name	: Should contain the name of the firmware to be loaded
+- clocks	: Must contain an entry for each name in clock-names
+- clock-names	: Must contain "fdma_slim, fdma_hi, fdma_low, fdma_ic" entries
+See: Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+
+Example:
+
+	fdma1: fdma-app@1 {
+		compatible = "st,fdma_mpe31";
+		reg = <0x8e40000 0x20000>;
+		interrupts = <GIC_SPI 7 IRQ_TYPE_NONE>;
+		dma-channels = <16>;
+		#dma-cells = <3>;
+		st,fw-name = "fdma_STiH407_1.elf";
+		clocks = <&CLK_S_C0_FLEXGEN CLK_FDMA>,
+			 <&CLK_S_C0_FLEXGEN CLK_TX_ICN_DMU>,
+			 <&CLK_S_C0_FLEXGEN CLK_TX_ICN_DMU>,
+			 <&CLK_S_C0_FLEXGEN CLK_EXT2F_A9>;
+		clock-names = "fdma_slim",
+			      "fdma_hi",
+			      "fdma_low",
+			      "fdma_ic";
+
+* DMA client
+
+Required properties:
+- dmas: Comma separated list of dma channel requests
+- dma-names: Names of the aforementioned requested channels
+
+Each dmas request consists of 4 cells:
+1. A phandle pointing to the FDMA controller
+2. The request line number
+3. A 32bit mask specifying (see include/linux/platform_data/dma-st-fdma.h)
+ -bit 2-0: Holdoff value, dreq will be masked for
+	0x0: 0-0.5us
+	0x1: 0.5-1us
+	0x2: 1-1.5us
+ -bit 17: data swap
+	0x0: disabled
+	0x1: enabled
+ -bit 21: Increment Address
+	0x0: no address increment between transfers
+	0x1: increment address between transfers
+ -bit 22: 2 STBus Initiator Coprocessor interface
+	0x0: high priority port
+	0x1: low priority port
+4. transfers type
+ 0 free running
+ 1 paced
+
+Example:
+
+	snd_uni_player2: snd-uni-player@2 {
+		compatible = "st,snd_uni_player";
+		status = "okay";
+		reg = <0x8D82000  0x158>;
+		interrupts = <GIC_SPI 86 IRQ_TYPE_NONE>;
+		version = <5>;
+		dmas = <&fdma0 4 0 1>;
+		dma-names = "tx";
+		description = "Uni Player #2 (DAC)";
-- 
1.9.1

[PATCH 2/7] dmaengine: st_fdma: Add STMicroelectronics FDMA xbar DT binding documentation

[Peter Griffin]

This patch adds the DT binding documentation for the FDMA xbar hw
found on STi based chipsets from STMicroelectronics.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 Documentation/devicetree/bindings/dma/st_fdma.txt | 24 +++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/Documentation/devicetree/bindings/dma/st_fdma.txt b/Documentation/devicetree/bindings/dma/st_fdma.txt
index 1ec7470..655ee57 100644
--- a/Documentation/devicetree/bindings/dma/st_fdma.txt
+++ b/Documentation/devicetree/bindings/dma/st_fdma.txt
@@ -3,6 +3,24 @@
 The FDMA is a general-purpose direct memory access controller capable of
 supporting 16 independent DMA channels. It accepts up to 32 DMA requests.
 The FDMA is based on a Slim processor which require a firmware.
+To increase the number of peripheral request, the FDMA crossbar can mutiplex
+up to 96 peripheral requests to one of 3 fdma controlers engine.
+
+* FDMA crossbar
+
+Required properties:
+- compatible		: Should be "st,fdma-xbar-1.0"
+- reg			: Should contain XBAR registers location and length
+- dma-requests		: Should contain the number of peripheral request supported
+- #st,fdma-xbar-cells	: Must be <1>
+
+Example:
+	xbar0: fdma-xbar-mpe@0 {
+		compatible = "st,fdma-xbar-1.0";
+		reg = <0x8e80000 0x1000>;
+		dma-requests = <79>;
+		#st,fdma-xbar-cells = <1>;
+	};
 
 * FDMA Controller
 
@@ -18,6 +36,11 @@ Required properties:
 See: Documentation/devicetree/bindings/clock/clock-bindings.txt
 
 
+Optional properties:
+- st,fdma-xbar	: Allow to plug controller behind the crossbar at offset X
+  1. A phandle pointing to the FDMA crossbar
+  2. Output offset <2..0>
+
 Example:
 
 	fdma1: fdma-app@1 {
@@ -26,6 +49,7 @@ Example:
 		interrupts = <GIC_SPI 7 IRQ_TYPE_NONE>;
 		dma-channels = <16>;
 		#dma-cells = <3>;
+		st,fdma-xbar = <&xbar0 0>;
 		st,fw-name = "fdma_STiH407_1.elf";
 		clocks = <&CLK_S_C0_FLEXGEN CLK_FDMA>,
 			 <&CLK_S_C0_FLEXGEN CLK_TX_ICN_DMU>,
-- 
1.9.1

[PATCH 3/7] dmaengine: st_fdma: Add STMicroelectronics FDMA engine driver support

[Peter Griffin]

This patch adds support for the Flexible Direct Memory Access (FDMA) core
driver. The FDMA is a slim core CPU with a dedicated firmware.
It is a general purpose DMA controller capable of supporting 16
independent DMA channels. Data moves maybe from memory to memory
or between memory and paced latency critical real time targets and it
is found on al STi based chipsets.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 drivers/dma/Kconfig                       |   15 +
 drivers/dma/Makefile                      |    1 +
 drivers/dma/st_fdma.c                     | 1182 +++++++++++++++++++++++++++++
 drivers/dma/st_fdma.h                     |  234 ++++++
 include/linux/platform_data/dma-st_fdma.h |   70 ++
 5 files changed, 1502 insertions(+)
 create mode 100644 drivers/dma/st_fdma.c
 create mode 100644 drivers/dma/st_fdma.h
 create mode 100644 include/linux/platform_data/dma-st_fdma.h

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 88d474b..7a016e0 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -507,4 +507,19 @@ config QCOM_BAM_DMA
 	  Enable support for the QCOM BAM DMA controller.  This controller
 	  provides DMA capabilities for a variety of on-chip devices.
 
+config ST_FDMA
+	bool "ST FDMA dmaengine support"
+	depends on ARCH_STI
+	select DMA_ENGINE
+	select FW_LOADER
+	select FW_LOADER_USER_HELPER_FALLBACK
+	select LIBELF_32
+	select DMA_VIRTUAL_CHANNELS
+	help
+	  Enable support for ST FDMA controller.
+	  It supports 16 independent DMA channels, accepts up to 32 DMA requests
+
+	  Say Y here if you have such a chipset.
+	  If unsure, say N.
+
 endif
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index 6a4d6f2..f68e6d8 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_MOXART_DMA) += moxart-dma.o
 obj-$(CONFIG_FSL_RAID) += fsl_raid.o
 obj-$(CONFIG_FSL_EDMA) += fsl-edma.o
 obj-$(CONFIG_QCOM_BAM_DMA) += qcom_bam_dma.o
+obj-$(CONFIG_ST_FDMA) += st_fdma.o
 obj-y += xilinx/
 obj-$(CONFIG_INTEL_MIC_X100_DMA) += mic_x100_dma.o
 obj-$(CONFIG_NBPFAXI_DMA) += nbpfaxi.o
diff --git a/drivers/dma/st_fdma.c b/drivers/dma/st_fdma.c
new file mode 100644
index 0000000..07a6df1
--- /dev/null
+++ b/drivers/dma/st_fdma.c
@@ -0,0 +1,1182 @@
+/*
+ * st_fdma.c
+ *
+ * Copyright (C) 2014 STMicroelectronics
+ * Author: Ludovic Barre <Ludovic.barre@st.com>
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_dma.h>
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+#include <linux/clk.h>
+#include <linux/dmaengine.h>
+#include <linux/dmapool.h>
+#include <linux/firmware.h>
+#include <linux/elf.h>
+#include <linux/atomic.h>
+
+#include "st_fdma.h"
+#include "dmaengine.h"
+#include "virt-dma.h"
+
+static char *fdma_clk_name[CLK_MAX_NUM] = {
+	[CLK_SLIM]	= "fdma_slim",
+	[CLK_HI]	= "fdma_hi",
+	[CLK_LOW]	= "fdma_low",
+	[CLK_IC]	= "fdma_ic",
+};
+
+static int st_fdma_clk_get(struct st_fdma_dev *fdev)
+{
+	int i;
+
+	for (i = 0; i < CLK_MAX_NUM; i++) {
+		fdev->clks[i] = devm_clk_get(fdev->dev, fdma_clk_name[i]);
+		if (IS_ERR(fdev->clks[i])) {
+			dev_err(fdev->dev,
+				"failed to get clock: %s\n", fdma_clk_name[i]);
+			return PTR_ERR(fdev->clks[i]);
+		}
+	}
+
+	if (i != CLK_MAX_NUM) {
+		dev_err(fdev->dev, "all clocks are not defined\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int st_fdma_clk_enable(struct st_fdma_dev *fdev)
+{
+	int i, ret;
+
+	for (i = 0; i < CLK_MAX_NUM; i++) {
+		ret = clk_prepare_enable(fdev->clks[i]);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+static void st_fdma_clk_disable(struct st_fdma_dev *fdev)
+{
+	int i;
+
+	for (i = 0; i < CLK_MAX_NUM; i++)
+		clk_disable_unprepare(fdev->clks[i]);
+}
+
+static inline struct st_fdma_chan *to_st_fdma_chan(struct dma_chan *c)
+{
+	return container_of(c, struct st_fdma_chan, vchan.chan);
+}
+
+static struct st_fdma_desc *to_st_fdma_desc(struct virt_dma_desc *vd)
+{
+	return container_of(vd, struct st_fdma_desc, vdesc);
+}
+
+void *st_fdma_seg_to_mem(struct st_fdma_dev *fdev, u64 da, int len)
+{
+	int i;
+	resource_size_t base = fdev->io_res->start;
+	const struct st_fdma_ram *fdma_mem = fdev->drvdata->fdma_mem;
+	void *ptr = NULL;
+
+	for (i = 0; i < fdev->drvdata->num_mem; i++) {
+		int mem_off = da - (base + fdma_mem[i].offset);
+
+		/* next mem if da is too small */
+		if (mem_off < 0)
+			continue;
+
+		/* next mem if da is too large */
+		if (mem_off + len > fdma_mem[i].size)
+			continue;
+
+		ptr = fdev->io_base + fdma_mem[i].offset + mem_off;
+		break;
+	}
+
+	return ptr;
+}
+
+static int
+st_fdma_elf_sanity_check(struct st_fdma_dev *fdev, const struct firmware *fw)
+{
+	const char *fw_name = fdev->pdata->fw_name;
+	struct elf32_hdr *ehdr;
+	char class;
+
+	if (!fw) {
+		dev_err(fdev->dev, "failed to load %s\n", fw_name);
+		return -EINVAL;
+	}
+
+	if (fw->size < sizeof(struct elf32_hdr)) {
+		dev_err(fdev->dev, "Image is too small\n");
+		return -EINVAL;
+	}
+
+	ehdr = (struct elf32_hdr *)fw->data;
+
+	/* We only support ELF32 at this point */
+	class = ehdr->e_ident[EI_CLASS];
+	if (class != ELFCLASS32) {
+		dev_err(fdev->dev, "Unsupported class: %d\n", class);
+		return -EINVAL;
+	}
+
+	if (ehdr->e_ident[EI_DATA] != ELFDATA2LSB) {
+		dev_err(fdev->dev, "Unsupported firmware endianness\n");
+		return -EINVAL;
+	}
+
+	if (fw->size < ehdr->e_shoff + sizeof(struct elf32_shdr)) {
+		dev_err(fdev->dev, "Image is too small\n");
+		return -EINVAL;
+	}
+
+	if (memcmp(ehdr->e_ident, ELFMAG, SELFMAG)) {
+		dev_err(fdev->dev, "Image is corrupted (bad magic)\n");
+		return -EINVAL;
+	}
+
+	if (ehdr->e_phnum != fdev->drvdata->num_mem) {
+		dev_err(fdev->dev, "spurious nb of segments\n");
+		return -EINVAL;
+	}
+
+	if (ehdr->e_type != ET_EXEC) {
+		dev_err(fdev->dev, "Unsupported ELF header type\n");
+		return -EINVAL;
+	}
+
+	if (ehdr->e_machine != EM_SLIM) {
+		dev_err(fdev->dev, "Unsupported ELF header machine\n");
+		return -EINVAL;
+	}
+	if (ehdr->e_phoff > fw->size) {
+		dev_err(fdev->dev, "Firmware size is too small\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int
+st_fdma_elf_load_segments(struct st_fdma_dev *fdev, const struct firmware *fw)
+{
+	struct device *dev = fdev->dev;
+	struct elf32_hdr *ehdr;
+	struct elf32_phdr *phdr;
+	int i, mem_loaded = 0;
+	const u8 *elf_data = fw->data;
+
+	ehdr = (struct elf32_hdr *)elf_data;
+	phdr = (struct elf32_phdr *)(elf_data + ehdr->e_phoff);
+
+	/*
+	 * go through the available ELF segments
+	 * the program header's paddr member to contain device addresses.
+	 * We then go through the physically contiguous memory regions which we
+	 * allocated (and mapped) earlier on the probe,
+	 * and "translate" device address to kernel addresses,
+	 * so we can copy the segments where they are expected.
+	 */
+	for (i = 0; i < ehdr->e_phnum; i++, phdr++) {
+		u32 da = phdr->p_paddr;
+		u32 memsz = phdr->p_memsz;
+		u32 filesz = phdr->p_filesz;
+		u32 offset = phdr->p_offset;
+		void *dst;
+
+		if (phdr->p_type != PT_LOAD)
+			continue;
+
+		dev_dbg(dev, "phdr: type %d da %#x ofst:%#x memsz %#x filesz %#x\n",
+			phdr->p_type, da, offset, memsz, filesz);
+
+		if (filesz > memsz) {
+			dev_err(dev, "bad phdr filesz 0x%x memsz 0x%x\n",
+				filesz, memsz);
+			break;
+		}
+
+		if (offset + filesz > fw->size) {
+			dev_err(dev, "truncated fw: need 0x%x avail 0x%zx\n",
+				offset + filesz, fw->size);
+			break;
+		}
+
+		dst = st_fdma_seg_to_mem(fdev, da, memsz);
+		if (!dst) {
+			dev_err(dev, "bad phdr da 0x%x mem 0x%x\n", da, memsz);
+			break;
+		}
+
+		if (phdr->p_filesz)
+			memcpy(dst, elf_data + phdr->p_offset, filesz);
+
+		if (memsz > filesz)
+			memset(dst + filesz, 0, memsz - filesz);
+
+		mem_loaded++;
+	}
+
+	return (mem_loaded != fdev->drvdata->num_mem) ? -EINVAL : 0;
+}
+
+static void st_fdma_enable(struct st_fdma_dev *fdev)
+{
+	unsigned long hw_id, hw_ver, fw_rev;
+	u32 val;
+
+	/* disable CPU pipeline clock & reset cpu pipeline */
+	val = FDMA_CLK_GATE_DIS | FDMA_CLK_GATE_RESET;
+	fdma_write(fdev, val, CLK_GATE);
+	/* disable SLIM core STBus sync */
+	fdma_write(fdev, FDMA_STBUS_SYNC_DIS, STBUS_SYNC);
+	/* enable cpu pipeline clock */
+	fdma_write(fdev, !FDMA_CLK_GATE_DIS, CLK_GATE);
+
+	/* clear int & cmd mailbox */
+	fdma_write(fdev, ~0UL, INT_CLR);
+	fdma_write(fdev, ~0UL, CMD_CLR);
+	/* enable all channels cmd & int */
+	fdma_write(fdev, ~0UL, INT_MASK);
+	fdma_write(fdev, ~0UL, CMD_MASK);
+
+	/* enable cpu */
+	writel(FDMA_EN_RUN, fdev->io_base + FDMA_EN_OFST);
+
+	hw_id = fdma_read(fdev, ID);
+	hw_ver = fdma_read(fdev, VER);
+	fw_rev = fdma_read(fdev, REV_ID);
+
+	dev_info(fdev->dev, "fw rev:%ld.%ld on SLIM %ld.%ld\n",
+		 FDMA_REV_ID_MAJ(fw_rev), FDMA_REV_ID_MIN(fw_rev),
+		 hw_id, hw_ver);
+}
+
+static int st_fdma_disable(struct st_fdma_dev *fdev)
+{
+	/* mask all (cmd & int) channels */
+	fdma_write(fdev, 0UL, INT_MASK);
+	fdma_write(fdev, 0UL, CMD_MASK);
+
+	/* disable cpu pipeline clock */
+	fdma_write(fdev, FDMA_CLK_GATE_DIS, CLK_GATE);
+	writel(!FDMA_EN_RUN, fdev->io_base + FDMA_EN_OFST);
+
+	return readl(fdev->io_base + FDMA_EN_OFST);
+}
+
+static void st_fdma_fw_cb(const struct firmware *fw, void *context)
+{
+	struct st_fdma_dev *fdev = context;
+	int ret;
+
+	ret = st_fdma_elf_sanity_check(fdev, fw);
+	if (ret)
+		goto out;
+
+	st_fdma_disable(fdev);
+	ret = st_fdma_elf_load_segments(fdev, fw);
+	if (ret)
+		goto out;
+
+	st_fdma_enable(fdev);
+	atomic_set(&fdev->fw_loaded, 1);
+out:
+	release_firmware(fw);
+	complete_all(&fdev->fw_ack);
+}
+
+static int st_fdma_get_fw(struct st_fdma_dev *fdev)
+{
+	int ret;
+
+	init_completion(&fdev->fw_ack);
+	atomic_set(&fdev->fw_loaded, 0);
+
+	ret = request_firmware_nowait(THIS_MODULE, FW_ACTION_HOTPLUG,
+				      fdev->pdata->fw_name, fdev->dev,
+				      GFP_KERNEL, fdev, st_fdma_fw_cb);
+	if (ret) {
+		dev_err(fdev->dev, "request_firmware_nowait err: %d\n", ret);
+		complete_all(&fdev->fw_ack);
+	}
+
+	return ret;
+}
+
+static int st_fdma_dreq_get(struct st_fdma_chan *fchan)
+{
+	struct st_fdma_dev *fdev = fchan->fdev;
+	u32 req_line_cfg = fchan->cfg.req_line;
+	u32 dreq_line;
+	int try = 0;
+
+	/*
+	 * dreq_mask is shared for n channels of fdma, so all accesses must be
+	 * atomic. if the dreq_mask it change between ffz ant set_bit,
+	 * we retry
+	 */
+	do {
+		if (fdev->dreq_mask == ~0L) {
+			dev_err(fdev->dev, "No req lines available\n");
+			return -EINVAL;
+		}
+
+		if (try || req_line_cfg >= ST_FDMA_NR_DREQS) {
+			dev_err(fdev->dev, "Invalid or used req line\n");
+			return -EINVAL;
+		} else {
+			dreq_line = req_line_cfg;
+		}
+
+		try++;
+	} while (test_and_set_bit(dreq_line, &fdev->dreq_mask));
+
+	dev_dbg(fdev->dev, "get dreq_line:%d mask:%#lx\n",
+		dreq_line, fdev->dreq_mask);
+
+	return dreq_line;
+}
+
+static void st_fdma_dreq_put(struct st_fdma_chan *fchan)
+{
+	struct st_fdma_dev *fdev = fchan->fdev;
+
+	dev_dbg(fdev->dev, "put dreq_line:%#x\n", fchan->dreq_line);
+	clear_bit(fchan->dreq_line, &fdev->dreq_mask);
+}
+
+static void st_fdma_xfer_desc(struct st_fdma_chan *fchan)
+{
+	struct virt_dma_desc *vdesc;
+	unsigned long nbytes, ch_cmd, cmd;
+
+	vdesc = vchan_next_desc(&fchan->vchan);
+	if (!vdesc)
+		return;
+
+	fchan->fdesc = to_st_fdma_desc(vdesc);
+	nbytes = fchan->fdesc->node[0].desc->nbytes;
+	cmd = FDMA_CMD_START(fchan->vchan.chan.chan_id);
+	ch_cmd = fchan->fdesc->node[0].pdesc | FDMA_CH_CMD_STA_START;
+
+	/* start the channel for the descriptor */
+	fnode_write(fchan, nbytes, CNTN);
+	fchan_write(fchan, ch_cmd, CH_CMD);
+	writel(cmd, fchan->fdev->io_base + FDMA_CMD_SET_OFST);
+
+	dev_dbg(fchan->fdev->dev, "start chan:%d\n", fchan->vchan.chan.chan_id);
+}
+
+static void st_fdma_ch_sta_update(struct st_fdma_chan *fchan,
+				  unsigned long int_sta)
+{
+	unsigned long ch_sta, ch_err;
+	int ch_id = fchan->vchan.chan.chan_id;
+	struct st_fdma_dev *fdev = fchan->fdev;
+
+	ch_sta = fchan_read(fchan, CH_CMD);
+	ch_err = ch_sta & FDMA_CH_CMD_ERR_MASK;
+	ch_sta &= FDMA_CH_CMD_STA_MASK;
+
+	if (int_sta & FDMA_INT_STA_ERR) {
+		dev_warn(fdev->dev, "chan:%d, error:%ld\n", ch_id, ch_err);
+		fchan->status = DMA_ERROR;
+		return;
+	}
+
+	switch (ch_sta) {
+	case FDMA_CH_CMD_STA_PAUSED:
+		fchan->status = DMA_PAUSED;
+		break;
+	case FDMA_CH_CMD_STA_RUNNING:
+		fchan->status = DMA_IN_PROGRESS;
+		break;
+	}
+}
+
+static irqreturn_t st_fdma_irq_handler(int irq, void *dev_id)
+{
+	struct st_fdma_dev *fdev = dev_id;
+	irqreturn_t ret = IRQ_NONE;
+	struct st_fdma_chan *fchan = &fdev->chans[0];
+	unsigned long int_sta, clr;
+
+	int_sta = fdma_read(fdev, INT_STA);
+	clr = int_sta;
+
+	for (; int_sta != 0 ; int_sta >>= 2, fchan++) {
+		if (!(int_sta & (FDMA_INT_STA_CH | FDMA_INT_STA_ERR)))
+			continue;
+
+		spin_lock(&fchan->vchan.lock);
+		st_fdma_ch_sta_update(fchan, int_sta);
+
+		if (fchan->fdesc) {
+			if (!fchan->fdesc->iscyclic) {
+				list_del(&fchan->fdesc->vdesc.node);
+				vchan_cookie_complete(&fchan->fdesc->vdesc);
+				fchan->fdesc = NULL;
+				fchan->status = DMA_COMPLETE;
+			} else {
+				vchan_cyclic_callback(&fchan->fdesc->vdesc);
+			}
+
+			/* Start the next descriptor (if available) */
+			if (!fchan->fdesc)
+				st_fdma_xfer_desc(fchan);
+		}
+
+		spin_unlock(&fchan->vchan.lock);
+		ret = IRQ_HANDLED;
+	}
+
+	fdma_write(fdev, clr, INT_CLR);
+
+	return ret;
+}
+
+static struct dma_chan *st_fdma_of_xlate(struct of_phandle_args *dma_spec,
+					 struct of_dma *ofdma)
+{
+	struct st_fdma_dev *fdev = ofdma->of_dma_data;
+	struct st_fdma_cfg cfg;
+
+	if (dma_spec->args_count < 1)
+		return NULL;
+
+	cfg.of_node = dma_spec->np;
+	cfg.req_line = dma_spec->args[0];
+	cfg.req_ctrl = 0;
+	cfg.type = ST_FDMA_TYPE_FREE_RUN;
+
+	if (dma_spec->args_count > 1)
+		cfg.req_ctrl = dma_spec->args[1] & REQ_CTRL_CFG_MASK;
+
+	if (dma_spec->args_count > 2)
+		cfg.type = dma_spec->args[2];
+
+	dev_dbg(fdev->dev, "xlate req_line:%d type:%d req_ctrl:%#x\n",
+		cfg.req_line, cfg.type, cfg.req_ctrl);
+
+	return dma_request_channel(fdev->dma_device.cap_mask,
+			st_fdma_filter_fn, &cfg);
+}
+
+static void st_fdma_free_desc(struct virt_dma_desc *vdesc)
+{
+	struct st_fdma_desc *fdesc;
+	int i;
+
+	fdesc = to_st_fdma_desc(vdesc);
+	for (i = 0; i < fdesc->n_nodes; i++)
+			dma_pool_free(fdesc->fchan->node_pool,
+				      fdesc->node[i].desc,
+				      fdesc->node[i].pdesc);
+	kfree(fdesc);
+}
+
+static struct st_fdma_desc *st_fdma_alloc_desc(struct st_fdma_chan *fchan,
+					       int sg_len)
+{
+	struct st_fdma_desc *fdesc;
+	int i;
+
+	fdesc = kzalloc(sizeof(*fdesc) +
+			sizeof(struct st_fdma_sw_node) * sg_len, GFP_NOWAIT);
+	if (!fdesc)
+		return NULL;
+
+	fdesc->fchan = fchan;
+	fdesc->n_nodes = sg_len;
+	for (i = 0; i < sg_len; i++) {
+		fdesc->node[i].desc = dma_pool_alloc(fchan->node_pool,
+				GFP_NOWAIT, &fdesc->node[i].pdesc);
+		if (!fdesc->node[i].desc)
+			goto err;
+	}
+	return fdesc;
+
+err:
+	while (--i >= 0)
+		dma_pool_free(fchan->node_pool, fdesc->node[i].desc,
+			      fdesc->node[i].pdesc);
+	kfree(fdesc);
+	return NULL;
+}
+
+static int st_fdma_alloc_chan_res(struct dma_chan *chan)
+{
+	struct st_fdma_chan *fchan = to_st_fdma_chan(chan);
+
+	if (fchan->cfg.type == ST_FDMA_TYPE_FREE_RUN) {
+		fchan->dreq_line = 0;
+	} else {
+		fchan->dreq_line = st_fdma_dreq_get(fchan);
+		if (IS_ERR_VALUE(fchan->dreq_line))
+			return -EINVAL;
+	}
+
+	/* Create the dma pool for descriptor allocation */
+	fchan->node_pool = dmam_pool_create(dev_name(&chan->dev->device),
+					    fchan->fdev->dev,
+					    sizeof(struct st_fdma_hw_node),
+					    __alignof__(struct st_fdma_hw_node),
+					    0);
+
+	if (!fchan->node_pool) {
+		dev_err(fchan->fdev->dev, "unable to allocate desc pool\n");
+		return -ENOMEM;
+	}
+
+	dev_dbg(fchan->fdev->dev, "alloc ch_id:%d type:%d\n",
+		fchan->vchan.chan.chan_id, fchan->cfg.type);
+
+	return 0;
+}
+
+static void st_fdma_free_chan_res(struct dma_chan *chan)
+{
+	struct st_fdma_chan *fchan = to_st_fdma_chan(chan);
+	unsigned long flags;
+	LIST_HEAD(head);
+
+	dev_dbg(fchan->fdev->dev, "freeing chan:%d\n",
+		fchan->vchan.chan.chan_id);
+
+	if (fchan->cfg.type != ST_FDMA_TYPE_FREE_RUN)
+		st_fdma_dreq_put(fchan);
+
+	spin_lock_irqsave(&fchan->vchan.lock, flags);
+	fchan->fdesc = NULL;
+	vchan_get_all_descriptors(&fchan->vchan, &head);
+	spin_unlock_irqrestore(&fchan->vchan.lock, flags);
+
+	dma_pool_destroy(fchan->node_pool);
+	fchan->node_pool = NULL;
+	memset(&fchan->cfg, 0, sizeof(struct st_fdma_cfg));
+}
+
+static struct dma_async_tx_descriptor *st_fdma_prep_dma_memcpy(
+	struct dma_chan *chan,	dma_addr_t dst, dma_addr_t src,
+	size_t len, unsigned long flags)
+{
+	struct st_fdma_chan *fchan;
+	struct st_fdma_desc *fdesc;
+	struct st_fdma_hw_node *hw_node;
+
+	if (!len)
+		return NULL;
+
+	fchan = to_st_fdma_chan(chan);
+
+	if (!atomic_read(&fchan->fdev->fw_loaded)) {
+		dev_err(fchan->fdev->dev, "%s: fdma fw not loaded\n", __func__);
+		return NULL;
+	}
+
+	/* We only require a single descriptor */
+	fdesc = st_fdma_alloc_desc(fchan, 1);
+	if (!fdesc) {
+		dev_err(fchan->fdev->dev, "no memory for desc\n");
+		return NULL;
+	}
+
+	hw_node = fdesc->node[0].desc;
+	hw_node->next = 0;
+	hw_node->control = NODE_CTRL_REQ_MAP_FREE_RUN;
+	hw_node->control |= NODE_CTRL_SRC_INCR;
+	hw_node->control |= NODE_CTRL_DST_INCR;
+	hw_node->control |= NODE_CTRL_INT_EON;
+	hw_node->nbytes = len;
+	hw_node->saddr = src;
+	hw_node->daddr = dst;
+	hw_node->generic.length = len;
+	hw_node->generic.sstride = 0;
+	hw_node->generic.dstride = 0;
+
+	return vchan_tx_prep(&fchan->vchan, &fdesc->vdesc, flags);
+}
+
+static struct dma_async_tx_descriptor *st_fdma_prep_dma_cyclic(
+		struct dma_chan *chan, dma_addr_t buf_addr, size_t len,
+		size_t period_len, enum dma_transfer_direction direction,
+		unsigned long flags)
+{
+	struct st_fdma_chan *fchan;
+	struct st_fdma_desc *fdesc;
+	int sg_len, i;
+
+	if (!chan || !len || !period_len)
+		return NULL;
+
+	fchan = to_st_fdma_chan(chan);
+
+	if (!atomic_read(&fchan->fdev->fw_loaded)) {
+		dev_err(fchan->fdev->dev, "%s: fdma fw not loaded\n", __func__);
+		return NULL;
+	}
+
+	if (!is_slave_direction(direction)) {
+		dev_err(fchan->fdev->dev, "bad direction?\n");
+		return NULL;
+	}
+
+	/* the buffer length must be a multiple of period_len */
+	if (len % period_len != 0) {
+		dev_err(fchan->fdev->dev, "len is not multiple of period\n");
+		return NULL;
+	}
+
+	sg_len = len / period_len;
+	fdesc = st_fdma_alloc_desc(fchan, sg_len);
+	if (!fdesc) {
+		dev_err(fchan->fdev->dev, "no memory for desc\n");
+		return NULL;
+	}
+
+	fdesc->iscyclic = true;
+
+	for (i = 0; i < sg_len; i++) {
+		struct st_fdma_hw_node *hw_node = fdesc->node[i].desc;
+
+		hw_node->next = fdesc->node[(i + 1) % sg_len].pdesc;
+
+		hw_node->control = NODE_CTRL_REQ_MAP_DREQ(fchan->dreq_line);
+		hw_node->control |= NODE_CTRL_INT_EON;
+
+		if (direction == DMA_MEM_TO_DEV) {
+			hw_node->control |= NODE_CTRL_SRC_INCR;
+			hw_node->control |= NODE_CTRL_DST_STATIC;
+			hw_node->saddr = buf_addr + (i * period_len);
+			hw_node->daddr = fchan->cfg.dev_addr;
+		} else {
+			hw_node->control |= NODE_CTRL_SRC_STATIC;
+			hw_node->control |= NODE_CTRL_DST_INCR;
+			hw_node->saddr = fchan->cfg.dev_addr;
+			hw_node->daddr = buf_addr + (i * period_len);
+		}
+
+		hw_node->nbytes = period_len;
+		hw_node->generic.length = period_len;
+		hw_node->generic.sstride = 0;
+		hw_node->generic.dstride = 0;
+	}
+
+	return vchan_tx_prep(&fchan->vchan, &fdesc->vdesc, flags);
+}
+
+static struct dma_async_tx_descriptor *st_fdma_prep_slave_sg(
+		struct dma_chan *chan, struct scatterlist *sgl,
+		unsigned int sg_len, enum dma_transfer_direction direction,
+		unsigned long flags, void *context)
+{
+	struct st_fdma_chan *fchan;
+	struct st_fdma_desc *fdesc;
+	struct st_fdma_hw_node *hw_node;
+	struct scatterlist *sg;
+	int i;
+
+	if (!chan || !sgl || !sg_len)
+		return NULL;
+
+	fchan = to_st_fdma_chan(chan);
+
+	if (!atomic_read(&fchan->fdev->fw_loaded)) {
+		dev_err(fchan->fdev->dev, "%s: fdma fw not loaded\n", __func__);
+		return NULL;
+	}
+
+	if (!is_slave_direction(direction)) {
+		dev_err(fchan->fdev->dev, "bad direction?\n");
+		return NULL;
+	}
+
+	fdesc = st_fdma_alloc_desc(fchan, sg_len);
+	if (!fdesc) {
+		dev_err(fchan->fdev->dev, "no memory for desc\n");
+		return NULL;
+	}
+
+	fdesc->iscyclic = false;
+
+	for_each_sg(sgl, sg, sg_len, i) {
+		hw_node = fdesc->node[i].desc;
+
+		hw_node->next = fdesc->node[(i + 1) % sg_len].pdesc;
+		hw_node->control = NODE_CTRL_REQ_MAP_DREQ(fchan->dreq_line);
+
+		if (direction == DMA_MEM_TO_DEV) {
+			hw_node->control |= NODE_CTRL_SRC_INCR;
+			hw_node->control |= NODE_CTRL_DST_STATIC;
+			hw_node->saddr = sg_dma_address(sg);
+			hw_node->daddr = fchan->cfg.dev_addr;
+		} else {
+			hw_node->control |= NODE_CTRL_SRC_STATIC;
+			hw_node->control |= NODE_CTRL_DST_INCR;
+			hw_node->saddr = fchan->cfg.dev_addr;
+			hw_node->daddr = sg_dma_address(sg);
+		}
+
+		hw_node->nbytes = sg_dma_len(sg);
+		hw_node->generic.length = sg_dma_len(sg);
+		hw_node->generic.sstride = 0;
+		hw_node->generic.dstride = 0;
+	}
+	/* interrupt at end of last node */
+	hw_node->control |= NODE_CTRL_INT_EON;
+
+	return vchan_tx_prep(&fchan->vchan, &fdesc->vdesc, flags);
+}
+
+static size_t st_fdma_desc_residue(struct st_fdma_chan *fchan,
+				   struct virt_dma_desc *vdesc,
+				   bool in_progress)
+{
+	struct st_fdma_desc *fdesc = fchan->fdesc;
+	size_t residue = 0;
+	dma_addr_t cur_addr = 0;
+	int i;
+
+	if (in_progress) {
+		cur_addr = fchan_read(fchan, CH_CMD);
+		cur_addr &= FDMA_CH_CMD_DATA_MASK;
+	}
+
+	for (i = fchan->fdesc->n_nodes - 1 ; i >= 0; i--) {
+		if (cur_addr == fdesc->node[i].pdesc) {
+			residue += fnode_read(fchan, CNTN);
+			break;
+		}
+		residue += fdesc->node[i].desc->nbytes;
+	}
+
+	return residue;
+}
+
+static enum dma_status st_fdma_tx_status(struct dma_chan *chan,
+					 dma_cookie_t cookie,
+					 struct dma_tx_state *txstate)
+{
+	struct st_fdma_chan *fchan = to_st_fdma_chan(chan);
+	struct virt_dma_desc *vd;
+	enum dma_status ret;
+	unsigned long flags;
+
+	ret = dma_cookie_status(chan, cookie, txstate);
+	if (ret == DMA_COMPLETE)
+		return ret;
+
+	if (!txstate)
+		return fchan->status;
+
+	spin_lock_irqsave(&fchan->vchan.lock, flags);
+	vd = vchan_find_desc(&fchan->vchan, cookie);
+	if (fchan->fdesc && cookie == fchan->fdesc->vdesc.tx.cookie)
+		txstate->residue = st_fdma_desc_residue(fchan, vd, true);
+	else if (vd)
+		txstate->residue = st_fdma_desc_residue(fchan, vd, false);
+	else
+		txstate->residue = 0;
+
+	spin_unlock_irqrestore(&fchan->vchan.lock, flags);
+
+	return fchan->status;
+}
+
+static void st_fdma_issue_pending(struct dma_chan *chan)
+{
+	struct st_fdma_chan *fchan = to_st_fdma_chan(chan);
+	unsigned long flags;
+
+	spin_lock_irqsave(&fchan->vchan.lock, flags);
+
+	if (vchan_issue_pending(&fchan->vchan) && !fchan->fdesc)
+		st_fdma_xfer_desc(fchan);
+
+	spin_unlock_irqrestore(&fchan->vchan.lock, flags);
+}
+
+static int st_fdma_pause(struct dma_chan *chan)
+{
+	unsigned long flags;
+	LIST_HEAD(head);
+	struct st_fdma_chan *fchan = to_st_fdma_chan(chan);
+	int ch_id = fchan->vchan.chan.chan_id;
+	unsigned long cmd = FDMA_CMD_PAUSE(ch_id);
+
+	dev_dbg(fchan->fdev->dev, "pause chan:%d\n", ch_id);
+
+	spin_lock_irqsave(&fchan->vchan.lock, flags);
+	if (fchan->fdesc)
+		fdma_write(fchan->fdev, cmd, CMD_SET);
+	spin_unlock_irqrestore(&fchan->vchan.lock, flags);
+
+	return 0;
+}
+
+static int st_fdma_resume(struct dma_chan *chan)
+{
+	unsigned long flags;
+	unsigned long val;
+	struct st_fdma_chan *fchan = to_st_fdma_chan(chan);
+	int ch_id = fchan->vchan.chan.chan_id;
+
+	dev_dbg(fchan->fdev->dev, "resume chan:%d\n", ch_id);
+
+	spin_lock_irqsave(&fchan->vchan.lock, flags);
+	if (fchan->fdesc) {
+		val = fchan_read(fchan, CH_CMD);
+		val &= FDMA_CH_CMD_DATA_MASK;
+		fchan_write(fchan, val, CH_CMD);
+	}
+	spin_unlock_irqrestore(&fchan->vchan.lock, flags);
+
+	return 0;
+}
+
+static int st_fdma_terminate_all(struct dma_chan *chan)
+{
+	unsigned long flags;
+	LIST_HEAD(head);
+	struct st_fdma_chan *fchan = to_st_fdma_chan(chan);
+	int ch_id = fchan->vchan.chan.chan_id;
+	unsigned long cmd = FDMA_CMD_PAUSE(ch_id);
+
+	dev_dbg(fchan->fdev->dev, "terminate chan:%d\n", ch_id);
+
+	spin_lock_irqsave(&fchan->vchan.lock, flags);
+	fdma_write(fchan->fdev, cmd, CMD_SET);
+	fchan->fdesc = NULL;
+	vchan_get_all_descriptors(&fchan->vchan, &head);
+	spin_unlock_irqrestore(&fchan->vchan.lock, flags);
+	vchan_dma_desc_free_list(&fchan->vchan, &head);
+
+	return 0;
+}
+
+static int st_fdma_slave_config(struct dma_chan *chan,
+				struct dma_slave_config *slave_cfg)
+{
+	u32 maxburst = 0, addr = 0;
+	enum dma_slave_buswidth width;
+	struct st_fdma_chan *fchan = to_st_fdma_chan(chan);
+	int ch_id = fchan->vchan.chan.chan_id;
+	struct st_fdma_dev *fdev = fchan->fdev;
+
+	if (slave_cfg->direction == DMA_DEV_TO_MEM) {
+		fchan->cfg.req_ctrl &= ~REQ_CTRL_WNR;
+		maxburst = slave_cfg->src_maxburst;
+		width = slave_cfg->src_addr_width;
+		addr = slave_cfg->src_addr;
+	} else if (slave_cfg->direction == DMA_MEM_TO_DEV) {
+		fchan->cfg.req_ctrl |= REQ_CTRL_WNR;
+		maxburst = slave_cfg->dst_maxburst;
+		width = slave_cfg->dst_addr_width;
+		addr = slave_cfg->dst_addr;
+	} else {
+		return -EINVAL;
+	}
+
+	if (width == DMA_SLAVE_BUSWIDTH_1_BYTE)
+		fchan->cfg.req_ctrl |= REQ_CTRL_OPCODE_LD_ST1;
+	else if (width == DMA_SLAVE_BUSWIDTH_2_BYTES)
+		fchan->cfg.req_ctrl |= REQ_CTRL_OPCODE_LD_ST2;
+	else if (width == DMA_SLAVE_BUSWIDTH_4_BYTES)
+		fchan->cfg.req_ctrl |= REQ_CTRL_OPCODE_LD_ST4;
+	else if (width == DMA_SLAVE_BUSWIDTH_8_BYTES)
+		fchan->cfg.req_ctrl |= REQ_CTRL_OPCODE_LD_ST8;
+	else
+		return -EINVAL;
+
+	fchan->cfg.req_ctrl |= REQ_CTRL_NUM_OPS(maxburst-1);
+	dreq_write(fchan, fchan->cfg.req_ctrl, REQ_CTRL);
+
+	fchan->cfg.dev_addr = addr;
+	fchan->cfg.dir = slave_cfg->direction;
+
+	dev_dbg(fdev->dev, "chan:%d dma_slave_config addr:%#x req_ctrl:%#x\n",
+		ch_id, addr, fchan->cfg.req_ctrl);
+
+	return 0;
+}
+
+static const struct st_fdma_ram fdma_mpe31_mem[] = {
+	{ .name = "dmem", .offset = 0x10000, .size = 0x3000 },
+	{ .name = "imem", .offset = 0x18000, .size = 0x8000 },
+};
+
+static const struct st_fdma_driverdata fdma_mpe31 = {
+	.fdma_mem = fdma_mpe31_mem,
+	.num_mem = ARRAY_SIZE(fdma_mpe31_mem),
+};
+
+static const struct of_device_id st_fdma_match[] = {
+	{ .compatible = "st,fdma_mpe31", .data = &fdma_mpe31 },
+	{},
+};
+MODULE_DEVICE_TABLE(of, st_fdma_match);
+
+static struct st_fdma_platform_data *
+st_fdma_parse_dt(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct st_fdma_platform_data *pdata;
+	int ret;
+
+	if (!np)
+		goto err;
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		goto err;
+
+	ret = of_property_read_u32(np, "dma-channels", &pdata->nr_channels);
+	if (ret)
+		goto err;
+
+	ret = of_property_read_string(np, "st,fw-name", &pdata->fw_name);
+	if (ret)
+		goto err;
+
+	return pdata;
+
+err:
+	return NULL;
+}
+#define FDMA_DMA_BUSWIDTHS	(BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) | \
+				 BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | \
+				 BIT(DMA_SLAVE_BUSWIDTH_3_BYTES) | \
+				 BIT(DMA_SLAVE_BUSWIDTH_4_BYTES))
+
+
+static int st_fdma_probe(struct platform_device *pdev)
+{
+	struct st_fdma_dev *fdev;
+	const struct of_device_id *match;
+	struct device_node *np = pdev->dev.of_node;
+	const struct st_fdma_driverdata *drvdata;
+	const struct st_fdma_platform_data *pdata;
+	int irq, ret, i;
+
+	match = of_match_device((st_fdma_match), &pdev->dev);
+	if (!match || !match->data) {
+		dev_err(&pdev->dev, "No device match found\n");
+		return -ENODEV;
+	}
+
+	drvdata = match->data;
+
+	pdata = st_fdma_parse_dt(pdev);
+	if (!pdata) {
+		dev_err(&pdev->dev, "unable to find platform data\n");
+		return -EINVAL;
+	}
+
+	fdev = devm_kzalloc(&pdev->dev, sizeof(*fdev), GFP_KERNEL);
+	if (!fdev)
+		return -ENOMEM;
+
+	fdev->chans = devm_kzalloc(&pdev->dev,
+				   pdata->nr_channels
+				   * sizeof(struct st_fdma_chan), GFP_KERNEL);
+	if (!fdev->chans)
+		return -ENOMEM;
+
+	fdev->dev = &pdev->dev;
+	fdev->drvdata = drvdata;
+	fdev->pdata = pdata;
+	platform_set_drvdata(pdev, fdev);
+
+	fdev->io_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	fdev->io_base = devm_ioremap_resource(&pdev->dev, fdev->io_res);
+	if (IS_ERR(fdev->io_base))
+		return PTR_ERR(fdev->io_base);
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "Failed to get irq resource\n");
+		return -EINVAL;
+	}
+
+	ret = devm_request_irq(&pdev->dev, irq, st_fdma_irq_handler, 0,
+			       dev_name(&pdev->dev), fdev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to request irq\n");
+		goto err;
+	}
+
+	ret = st_fdma_clk_get(fdev);
+	if (ret)
+		goto err;
+
+	ret = st_fdma_clk_enable(fdev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to enable clocks\n");
+		goto err_clk;
+	}
+
+	/* Initialise list of FDMA channels */
+	INIT_LIST_HEAD(&fdev->dma_device.channels);
+	for (i = 0; i < fdev->pdata->nr_channels; i++) {
+		struct st_fdma_chan *fchan = &fdev->chans[i];
+
+		fchan->fdev = fdev;
+		fchan->vchan.desc_free = st_fdma_free_desc;
+		vchan_init(&fchan->vchan, &fdev->dma_device);
+	}
+
+	ret = st_fdma_get_fw(fdev);
+	if (ret)
+		goto err_clk;
+
+	/* Initialise the FDMA dreq (reserve 0 & 31 for FDMA use) */
+	fdev->dreq_mask = BIT(0) | BIT(31);
+
+	dma_cap_set(DMA_SLAVE, fdev->dma_device.cap_mask);
+	dma_cap_set(DMA_CYCLIC, fdev->dma_device.cap_mask);
+	dma_cap_set(DMA_MEMCPY, fdev->dma_device.cap_mask);
+
+	fdev->dma_device.dev = &pdev->dev;
+	fdev->dma_device.device_alloc_chan_resources = st_fdma_alloc_chan_res;
+	fdev->dma_device.device_free_chan_resources = st_fdma_free_chan_res;
+	fdev->dma_device.device_prep_dma_cyclic	= st_fdma_prep_dma_cyclic;
+	fdev->dma_device.device_prep_slave_sg = st_fdma_prep_slave_sg;
+	fdev->dma_device.device_prep_dma_memcpy = st_fdma_prep_dma_memcpy;
+	fdev->dma_device.device_tx_status = st_fdma_tx_status;
+	fdev->dma_device.device_issue_pending = st_fdma_issue_pending;
+	fdev->dma_device.device_terminate_all = st_fdma_terminate_all;
+	fdev->dma_device.device_config = st_fdma_slave_config;
+	fdev->dma_device.device_pause = st_fdma_pause;
+	fdev->dma_device.device_resume = st_fdma_resume;
+
+	fdev->dma_device.src_addr_widths = FDMA_DMA_BUSWIDTHS;
+	fdev->dma_device.dst_addr_widths = FDMA_DMA_BUSWIDTHS;
+	fdev->dma_device.directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
+	fdev->dma_device.residue_granularity = DMA_RESIDUE_GRANULARITY_BURST;
+
+	ret = dma_async_device_register(&fdev->dma_device);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to register DMA device\n");
+		goto err_clk;
+	}
+
+	ret = of_dma_controller_register(np, st_fdma_of_xlate, fdev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to register controller\n");
+		goto err_dma_dev;
+	}
+
+	dev_info(&pdev->dev, "ST FDMA engine driver, irq:%d\n", irq);
+
+	return 0;
+
+err_dma_dev:
+	dma_async_device_unregister(&fdev->dma_device);
+err_clk:
+	st_fdma_clk_disable(fdev);
+err:
+	return ret;
+}
+
+static int __exit st_fdma_remove(struct platform_device *pdev)
+{
+	struct st_fdma_dev *fdev = platform_get_drvdata(pdev);
+
+	wait_for_completion(&fdev->fw_ack);
+
+	st_fdma_clk_disable(fdev);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int st_fdma_pm_suspend(struct device *dev)
+{
+	struct st_fdma_dev *fdev = dev_get_drvdata(dev);
+	int ret;
+
+	if (atomic_read(&fdev->fw_loaded)) {
+		ret = st_fdma_disable(fdev);
+		if (ret & FDMA_EN_RUN) {
+			dev_warn(fdev->dev, "Failed to disable channels");
+			return -EBUSY;
+		}
+	}
+
+	st_fdma_clk_disable(fdev);
+
+	return 0;
+}
+
+static int st_fdma_pm_resume(struct device *dev)
+{
+	struct st_fdma_dev *fdev = dev_get_drvdata(dev);
+	int ret;
+
+	ret = st_fdma_clk_enable(fdev);
+	if (ret) {
+		dev_err(fdev->dev, "Failed to enable clocks\n");
+		goto out;
+	}
+
+	ret = st_fdma_get_fw(fdev);
+out:
+	return ret;
+}
+
+const struct dev_pm_ops st_fdma_pm = {
+	.suspend_late = st_fdma_pm_suspend,
+	.resume_early = st_fdma_pm_resume,
+};
+
+#define ST_FDMA_PM	(&st_fdma_pm)
+#else
+#define ST_FDMA_PM	NULL
+#endif
+
+static struct platform_driver st_fdma_platform_driver = {
+	.driver = {
+		.name = "st-fdma",
+		.of_match_table = st_fdma_match,
+		.pm = ST_FDMA_PM,
+	},
+	.probe = st_fdma_probe,
+	.remove = st_fdma_remove,
+};
+module_platform_driver(st_fdma_platform_driver);
+
+bool st_fdma_filter_fn(struct dma_chan *chan, void *param)
+{
+	struct st_fdma_cfg *config = param;
+	struct st_fdma_chan *fchan = to_st_fdma_chan(chan);
+
+	if (!param)
+		return false;
+
+	if (fchan->fdev->dma_device.dev->of_node != config->of_node)
+		return false;
+
+	fchan->cfg = *config;
+
+	return true;
+}
+EXPORT_SYMBOL(st_fdma_filter_fn);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("STMicroelectronics FDMA engine driver");
+MODULE_AUTHOR("Ludovic.barre <Ludovic.barre@st.com>");
diff --git a/drivers/dma/st_fdma.h b/drivers/dma/st_fdma.h
new file mode 100644
index 0000000..533c811
--- /dev/null
+++ b/drivers/dma/st_fdma.h
@@ -0,0 +1,234 @@
+/*
+ * st_fdma.h
+ *
+ * Copyright (C) 2014 STMicroelectronics
+ * Author: Ludovic Barre <Ludovic.barre@st.com>
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+#ifndef __DMA_ST_FDMA_H
+#define __DMA_ST_FDMA_H
+
+#include <linux/dmaengine.h>
+#include <linux/interrupt.h>
+#include <linux/platform_data/dma-st_fdma.h>
+
+#include "virt-dma.h"
+
+#define ST_FDMA_NR_DREQS 32
+#define EM_SLIM	102	/* No official SLIM ELF ID */
+
+enum {
+	CLK_SLIM,
+	CLK_HI,
+	CLK_LOW,
+	CLK_IC,
+	CLK_MAX_NUM,
+};
+
+#define MEM_NAME_SZ 10
+
+struct st_fdma_ram {
+	char name[MEM_NAME_SZ];
+	u32 offset;
+	u32 size;
+};
+
+/**
+ * struct st_fdma_generic_node - Free running/paced generic node
+ *
+ * @length: Length in bytes of a line in a 2D mem to mem
+ * @sstride: Stride, in bytes, between source lines in a 2D data move
+ * @dstride: Stride, in bytes, between destination lines in a 2D data move
+ */
+struct st_fdma_generic_node {
+	u32 length;
+	u32 sstride;
+	u32 dstride;
+};
+
+/**
+ * struct st_fdma_hw_node - Node structure used by fdma hw
+ *
+ * @next: Pointer to next node
+ * @control: Transfer Control Parameters
+ * @nbytes: Number of Bytes to read
+ * @saddr: Source address
+ * @daddr: Destination address
+ *
+ * @generic: generic node for free running/paced transfert type
+ * 2 others transfert type are possible, but not yet implemented
+ *
+ * The NODE structures must be aligned to a 32 byte boundary
+ */
+struct st_fdma_hw_node {
+	u32 next;
+	u32 control;
+	u32 nbytes;
+	u32 saddr;
+	u32 daddr;
+	union {
+		struct st_fdma_generic_node generic;
+	};
+} __aligned(32);
+
+/*
+ * node control parameters
+ */
+#define NODE_CTRL_REQ_MAP_MASK		GENMASK(4, 0)
+#define NODE_CTRL_REQ_MAP_FREE_RUN	0x0
+#define NODE_CTRL_REQ_MAP_DREQ(n)	((n) & NODE_CTRL_REQ_MAP_MASK)
+#define NODE_CTRL_REQ_MAP_EXT		NODE_CTRL_REQ_MAP_MASK
+#define NODE_CTRL_SRC_MASK		GENMASK(6, 5)
+#define NODE_CTRL_SRC_STATIC		BIT(5)
+#define NODE_CTRL_SRC_INCR		BIT(6)
+#define NODE_CTRL_DST_MASK		GENMASK(8, 7)
+#define NODE_CTRL_DST_STATIC		BIT(7)
+#define NODE_CTRL_DST_INCR		BIT(8)
+#define NODE_CTRL_SECURE		BIT(15)
+#define NODE_CTRL_PAUSE_EON		BIT(30)
+#define NODE_CTRL_INT_EON		BIT(31)
+
+/**
+ * struct st_fdma_sw_node - descriptor structure for link list
+ *
+ * @pdesc: Physical address of desc
+ * @node: link used for putting this into a channel queue
+ */
+struct st_fdma_sw_node {
+	dma_addr_t pdesc;
+	struct st_fdma_hw_node *desc;
+};
+
+struct st_fdma_driverdata {
+	const struct st_fdma_ram *fdma_mem;
+	u32 num_mem;
+};
+
+struct st_fdma_desc {
+	struct virt_dma_desc vdesc;
+	struct st_fdma_chan *fchan;
+	bool iscyclic;
+	unsigned int n_nodes;
+	struct st_fdma_sw_node node[];
+};
+
+struct st_fdma_chan {
+	struct st_fdma_dev *fdev;
+	struct dma_pool *node_pool;
+	struct st_fdma_cfg cfg;
+	int dreq_line;
+
+	struct virt_dma_chan vchan;
+	struct st_fdma_desc *fdesc;
+	enum dma_status	status;
+};
+
+struct st_fdma_dev {
+	struct device *dev;
+	const struct st_fdma_driverdata *drvdata;
+	const struct st_fdma_platform_data *pdata;
+	struct dma_device dma_device;
+
+	void __iomem *io_base;
+	struct resource *io_res;
+	struct clk *clks[CLK_MAX_NUM];
+
+	struct st_fdma_chan *chans;
+
+	spinlock_t dreq_lock;
+	unsigned long dreq_mask;
+
+	struct completion fw_ack;
+	atomic_t fw_loaded;
+};
+
+/* Registers*/
+/* FDMA interface */
+#define FDMA_ID_OFST		0x00000
+#define FDMA_VER_OFST		0x00004
+
+#define FDMA_EN_OFST		0x00008
+#define FDMA_EN_RUN			BIT(0)
+
+#define FDMA_CLK_GATE_OFST	0x0000C
+#define FDMA_CLK_GATE_DIS		BIT(0)
+#define FDMA_CLK_GATE_RESET		BIT(2)
+
+#define FDMA_SLIM_PC_OFST	0x00020
+
+#define FDMA_REV_ID_OFST	0x10000
+#define FDMA_REV_ID_MIN_MASK		GENMASK(15, 8)
+#define FDMA_REV_ID_MIN(id)		((id & FDMA_REV_ID_MIN_MASK) >> 8)
+#define FDMA_REV_ID_MAJ_MASK		GENMASK(23, 16)
+#define FDMA_REV_ID_MAJ(id)		((id & FDMA_REV_ID_MAJ_MASK) >> 16)
+
+#define FDMA_STBUS_SYNC_OFST	0x17F88
+#define FDMA_STBUS_SYNC_DIS		BIT(0)
+
+#define FDMA_CMD_STA_OFST	0x17FC0
+#define FDMA_CMD_SET_OFST	0x17FC4
+#define FDMA_CMD_CLR_OFST	0x17FC8
+#define FDMA_CMD_MASK_OFST	0x17FCC
+#define FDMA_CMD_START(ch)		(0x1 << (ch << 1))
+#define FDMA_CMD_PAUSE(ch)		(0x2 << (ch << 1))
+#define FDMA_CMD_FLUSH(ch)		(0x3 << (ch << 1))
+
+#define FDMA_INT_STA_OFST	0x17FD0
+#define FDMA_INT_STA_CH			0x1
+#define FDMA_INT_STA_ERR		0x2
+
+#define FDMA_INT_SET_OFST	0x17FD4
+#define FDMA_INT_CLR_OFST	0x17FD8
+#define FDMA_INT_MASK_OFST	0x17FDC
+
+#define fdma_read(fdev, name) \
+	readl_relaxed((fdev)->io_base + FDMA_##name##_OFST)
+
+#define fdma_write(fdev, val, name) \
+	writel_relaxed((val), (fdev)->io_base + FDMA_##name##_OFST)
+
+/* fchan interface */
+#define FDMA_CH_CMD_OFST	0x10200
+#define FDMA_CH_CMD_STA_MASK		GENMASK(1, 0)
+#define FDMA_CH_CMD_STA_IDLE		(0x0)
+#define FDMA_CH_CMD_STA_START		(0x1)
+#define FDMA_CH_CMD_STA_RUNNING		(0x2)
+#define FDMA_CH_CMD_STA_PAUSED		(0x3)
+#define FDMA_CH_CMD_ERR_MASK		GENMASK(4, 2)
+#define FDMA_CH_CMD_ERR_INT		(0x0 << 2)
+#define FDMA_CH_CMD_ERR_NAND		(0x1 << 2)
+#define FDMA_CH_CMD_ERR_MCHI		(0x2 << 2)
+#define FDMA_CH_CMD_DATA_MASK		GENMASK(31, 5)
+#define fchan_read(fchan, name) \
+	readl_relaxed((fchan)->fdev->io_base \
+			+ (fchan)->vchan.chan.chan_id * 0x4 \
+			+ FDMA_##name##_OFST)
+
+#define fchan_write(fchan, val, name) \
+	writel_relaxed((val), (fchan)->fdev->io_base \
+			+ (fchan)->vchan.chan.chan_id * 0x4 \
+			+ FDMA_##name##_OFST)
+
+/* req interface */
+#define FDMA_REQ_CTRL_OFST	0x10240
+#define dreq_write(fchan, val, name) \
+	writel_relaxed((val), (fchan)->fdev->io_base \
+			+ fchan->dreq_line * 0x04 \
+			+ FDMA_##name##_OFST)
+/* node interface */
+#define FDMA_NODE_SZ 128
+#define FDMA_PTRN_OFST		0x10800
+#define FDMA_CNTN_OFST		0x10808
+#define FDMA_SADDRN_OFST	0x1080c
+#define FDMA_DADDRN_OFST	0x10810
+#define fnode_read(fchan, name) \
+	readl_relaxed((fchan)->fdev->io_base \
+			+ (fchan)->vchan.chan.chan_id * FDMA_NODE_SZ \
+			+ FDMA_##name##_OFST)
+
+#define fnode_write(fchan, val, name) \
+	writel_relaxed((val), (fchan)->fdev->io_base \
+			+ (fchan)->vchan.chan.chan_id * FDMA_NODE_SZ \
+			+ FDMA_##name##_OFST)
+
+#endif	/* __DMA_ST_FDMA_H */
diff --git a/include/linux/platform_data/dma-st_fdma.h b/include/linux/platform_data/dma-st_fdma.h
new file mode 100644
index 0000000..3a1098b
--- /dev/null
+++ b/include/linux/platform_data/dma-st_fdma.h
@@ -0,0 +1,70 @@
+/*
+ * dma-st-fdma.h
+ *
+ * Copyright (C) 2014 STMicroelectronics
+ * Author: Ludovic Barre <Ludovic.barre@st.com>
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+#ifndef __PLAT_ST_FDMA_H
+#define __PLAT_ST_FDMA_H
+
+#include <linux/dmaengine.h>
+
+/*
+ * Channel type
+ */
+enum st_fdma_type {
+	ST_FDMA_TYPE_FREE_RUN,
+	ST_FDMA_TYPE_PACED,
+};
+
+struct st_fdma_cfg {
+	struct device_node *of_node;
+	enum st_fdma_type type;
+	dma_addr_t dev_addr;
+	enum dma_transfer_direction dir;
+	int req_line; /* request line */
+	u32 req_ctrl; /* Request control */
+};
+
+/**
+ * struct st_fdma_platform_data - platform specific data for FDMA engine
+ *
+ * @nr_channels:	Number of channels supported by hardware
+ * @fw_name:		The firmware name
+ */
+struct st_fdma_platform_data {
+	u32 nr_channels;
+	const char *fw_name;
+};
+
+/*
+ * request control bits
+ */
+/*replace by genmask >3.13*/
+#define REQ_CTRL_NUM_OPS_MASK		GENMASK(31, 24)
+#define REQ_CTRL_NUM_OPS(n)		(REQ_CTRL_NUM_OPS_MASK & ((n) << 24))
+#define REQ_CTRL_INITIATOR_MASK		BIT(22)
+#define	REQ_CTRL_INIT0			(0x0 << 22)
+#define	REQ_CTRL_INIT1			(0x1 << 22)
+#define REQ_CTRL_INC_ADDR_ON		BIT(21)
+#define REQ_CTRL_DATA_SWAP_ON		BIT(17)
+#define REQ_CTRL_WNR			BIT(14)
+#define REQ_CTRL_OPCODE_MASK		GENMASK(7, 4)
+#define REQ_CTRL_OPCODE_LD_ST1		(0x0 << 4)
+#define REQ_CTRL_OPCODE_LD_ST2		(0x1 << 4)
+#define REQ_CTRL_OPCODE_LD_ST4		(0x2 << 4)
+#define REQ_CTRL_OPCODE_LD_ST8		(0x3 << 4)
+#define REQ_CTRL_OPCODE_LD_ST16		(0x4 << 4)
+#define REQ_CTRL_OPCODE_LD_ST32		(0x5 << 4)
+#define REQ_CTRL_OPCODE_LD_ST64		(0x6 << 4)
+#define REQ_CTRL_HOLDOFF_MASK		GENMASK(2, 0)
+#define REQ_CTRL_HOLDOFF(n)		((n) & REQ_CTRL_HOLDOFF_MASK)
+
+/* bits used by client to configure request control */
+#define REQ_CTRL_CFG_MASK (REQ_CTRL_HOLDOFF_MASK | REQ_CTRL_DATA_SWAP_ON \
+			| REQ_CTRL_INC_ADDR_ON | REQ_CTRL_INITIATOR_MASK)
+
+bool st_fdma_filter_fn(struct dma_chan *chan, void *param);
+
+#endif	/* __PLAT_ST_FDMA_H */
-- 
1.9.1

[PATCH 4/7] dmaengine: st_fdma: Add xbar support

[Peter Griffin]

To increase the number of peripheral requests, the FDMA crossbar
can multiplex up to 96 peripheral requests to one of 3 fdma
controllers.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 drivers/dma/Kconfig        |  11 ++++
 drivers/dma/Makefile       |   1 +
 drivers/dma/st_fdma.c      |  25 +++++++-
 drivers/dma/st_fdma.h      |  32 ++++++++++
 drivers/dma/st_fdma_xbar.c | 149 +++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 216 insertions(+), 2 deletions(-)
 create mode 100644 drivers/dma/st_fdma_xbar.c

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 7a016e0..21802b1 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -522,4 +522,15 @@ config ST_FDMA
 	  Say Y here if you have such a chipset.
 	  If unsure, say N.
 
+config ST_FDMA_XBAR
+	bool "ST FDMA crossbar"
+	depends on ST_FDMA
+	default y
+	help
+	  Enable support for ST FDMA crossbar.
+	  xbar add flexibility and increase the number of peripheral request
+	  can be used by fdma xbar can multiplex until 96 peripheral requests
+	  to one of 3 fdma controller
+
+
 endif
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index f68e6d8..19f18b1 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_MOXART_DMA) += moxart-dma.o
 obj-$(CONFIG_FSL_RAID) += fsl_raid.o
 obj-$(CONFIG_FSL_EDMA) += fsl-edma.o
 obj-$(CONFIG_QCOM_BAM_DMA) += qcom_bam_dma.o
+obj-$(CONFIG_ST_FDMA_XBAR) += st_fdma_xbar.o
 obj-$(CONFIG_ST_FDMA) += st_fdma.o
 obj-y += xilinx/
 obj-$(CONFIG_INTEL_MIC_X100_DMA) += mic_x100_dma.o
diff --git a/drivers/dma/st_fdma.c b/drivers/dma/st_fdma.c
index 07a6df1..a870902 100644
--- a/drivers/dma/st_fdma.c
+++ b/drivers/dma/st_fdma.c
@@ -336,7 +336,9 @@ static int st_fdma_dreq_get(struct st_fdma_chan *fchan)
 			return -EINVAL;
 		}
 
-		if (try || req_line_cfg >= ST_FDMA_NR_DREQS) {
+		if (fdev->xbar_dev)
+			dreq_line = ffz(fdev->dreq_mask);
+		else if (try || req_line_cfg >= ST_FDMA_NR_DREQS) {
 			dev_err(fdev->dev, "Invalid or used req line\n");
 			return -EINVAL;
 		} else {
@@ -903,6 +905,15 @@ static int st_fdma_slave_config(struct dma_chan *chan,
 	else
 		return -EINVAL;
 
+	if (fdev->xbar_dev) {
+		if (st_fdma_xbar_mux(fdev->xbar_dev, fchan->cfg.req_line,
+				     fchan->dreq_line)) {
+			dev_err(fdev->dev, "Error routing req line\n");
+			clear_bit(fchan->dreq_line, &fdev->dreq_mask);
+			return -EINVAL;
+		}
+	}
+
 	fchan->cfg.req_ctrl |= REQ_CTRL_NUM_OPS(maxburst-1);
 	dreq_write(fchan, fchan->cfg.req_ctrl, REQ_CTRL);
 
@@ -1044,6 +1055,13 @@ static int st_fdma_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_clk;
 
+	fdev->xbar_dev = st_fdma_xbar_request(&pdev->dev);
+	if (IS_ERR(fdev->xbar_dev)) {
+		dev_err(&pdev->dev, "Failed to request xbar:%ld\n",
+			PTR_ERR(fdev->xbar_dev));
+		goto err_clk;
+	}
+
 	/* Initialise the FDMA dreq (reserve 0 & 31 for FDMA use) */
 	fdev->dreq_mask = BIT(0) | BIT(31);
 
@@ -1081,7 +1099,10 @@ static int st_fdma_probe(struct platform_device *pdev)
 		goto err_dma_dev;
 	}
 
-	dev_info(&pdev->dev, "ST FDMA engine driver, irq:%d\n", irq);
+	dev_info(&pdev->dev, "ST FDMA engine driver, irq:%d xbar(%s, id:%d)\n",
+		 irq, fdev->xbar_dev ?
+		 dev_name(&fdev->xbar_dev->pdev->dev) : "no",
+		 fdev->xbar_dev ? fdev->xbar_dev->fdma_id : 0);
 
 	return 0;
 
diff --git a/drivers/dma/st_fdma.h b/drivers/dma/st_fdma.h
index 533c811..dd46780 100644
--- a/drivers/dma/st_fdma.h
+++ b/drivers/dma/st_fdma.h
@@ -17,6 +17,20 @@
 #define ST_FDMA_NR_DREQS 32
 #define EM_SLIM	102	/* No official SLIM ELF ID */
 
+struct st_fdma_xbar {
+	void __iomem *io_base;
+	struct clk *clk;
+	u32 nr_requests;
+	u32 max_nr_requests;
+};
+
+struct st_fdma_xbar_dev {
+	struct platform_device *pdev;
+	struct st_fdma_xbar *xbar;
+	struct device *fdev;
+	u32 fdma_id;
+};
+
 enum {
 	CLK_SLIM,
 	CLK_HI,
@@ -135,6 +149,7 @@ struct st_fdma_dev {
 
 	struct st_fdma_chan *chans;
 
+	struct st_fdma_xbar_dev *xbar_dev;
 	spinlock_t dreq_lock;
 	unsigned long dreq_mask;
 
@@ -142,6 +157,23 @@ struct st_fdma_dev {
 	atomic_t fw_loaded;
 };
 
+#if defined(CONFIG_ST_FDMA_XBAR)
+struct st_fdma_xbar_dev *st_fdma_xbar_request(struct device *device);
+int st_fdma_xbar_mux(struct st_fdma_xbar_dev *xbar_dev,
+		     u32 in_dreq, u32 fdma_dreq);
+#else
+struct st_fdma_xbar_dev *st_fdma_xbar_request(struct device *device)
+{
+	return NULL;
+}
+
+int st_fdma_xbar_mux(struct st_fdma_xbar_dev *xbar_dev,
+		     u32 in_dreq, u32 fdma_dreq)
+{
+	return -ENODEV;
+}
+#endif
+
 /* Registers*/
 /* FDMA interface */
 #define FDMA_ID_OFST		0x00000
diff --git a/drivers/dma/st_fdma_xbar.c b/drivers/dma/st_fdma_xbar.c
new file mode 100644
index 0000000..c3b8e0f
--- /dev/null
+++ b/drivers/dma/st_fdma_xbar.c
@@ -0,0 +1,149 @@
+/*
+ * st_fdma_xbar.c
+ *
+ * Copyright (C) 2014 STMicroelectronics
+ * Author: Ludovic Barre <Ludovic.barre@st.com>
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+
+#include "st_fdma.h"
+
+#define XBAR_1_0_MAX_REQ	96
+
+int st_fdma_xbar_mux(struct st_fdma_xbar_dev *xbar_dev,
+		     u32 per_req, u32 fdma_req)
+{
+	u32 out_req;
+
+	if (per_req >= xbar_dev->xbar->nr_requests)
+		return -EINVAL;
+
+	out_req = (xbar_dev->fdma_id * ST_FDMA_NR_DREQS) + fdma_req;
+	writel(per_req, xbar_dev->xbar->io_base + (out_req << 2));
+
+	dev_dbg(&xbar_dev->pdev->dev, "in_req:%d out_req:%d\n",
+		per_req, out_req);
+
+	return 0;
+}
+
+struct st_fdma_xbar_dev *st_fdma_xbar_request(struct device *device)
+{
+	struct device_node *np = device->of_node;
+	struct st_fdma_xbar_dev *xbar_dev;
+	struct of_phandle_args args;
+	int err;
+
+	err = of_parse_phandle_with_args(np, "st,fdma-xbar",
+					 "#st,fdma-xbar-cells", 0, &args);
+	if (err)
+		return NULL;
+
+	xbar_dev = kzalloc(sizeof(*xbar_dev), GFP_KERNEL);
+	if (!xbar_dev) {
+		of_node_put(args.np);
+		err = -ENOMEM;
+		goto out;
+	}
+
+	xbar_dev->pdev = of_find_device_by_node(args.np);
+	if (!xbar_dev->pdev) {
+		of_node_put(args.np);
+		err = -ENODEV;
+		goto free;
+	}
+
+	of_node_put(args.np);
+
+	xbar_dev->xbar = platform_get_drvdata(xbar_dev->pdev);
+	if (!xbar_dev->xbar) {
+		err = -EPROBE_DEFER;
+		goto pdev_put;
+	}
+
+	xbar_dev->fdma_id = args.args[0];
+	xbar_dev->fdev = device;
+
+	return xbar_dev;
+
+pdev_put:
+	platform_device_put(xbar_dev->pdev);
+free:
+	kfree(xbar_dev);
+out:
+	return ERR_PTR(err);
+}
+
+void st_fdma_xbar_free(struct st_fdma_xbar_dev *device)
+{
+	platform_device_put(device->pdev);
+	kfree(device);
+}
+
+static const struct of_device_id st_fdma_xbar_match[] = {
+	{ .compatible = "st,fdma-xbar-1.0", .data = (void *)XBAR_1_0_MAX_REQ },
+	{},
+};
+MODULE_DEVICE_TABLE(of, st_fdma_xbar_match);
+
+static int st_fdma_xbar_probe(struct platform_device *pdev)
+{
+	const struct of_device_id *match;
+	struct device_node *np = pdev->dev.of_node;
+	struct st_fdma_xbar *xbar;
+	struct resource *res;
+
+	match = of_match_device((st_fdma_xbar_match), &pdev->dev);
+	if (!match) {
+		dev_err(&pdev->dev, "No device match found\n");
+		return -ENODEV;
+	}
+
+	xbar = devm_kzalloc(&pdev->dev, sizeof(*xbar), GFP_KERNEL);
+	if (!xbar)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	xbar->io_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(xbar->io_base))
+		return PTR_ERR(xbar->io_base);
+
+	xbar->max_nr_requests = (unsigned long)match->data;
+
+	if (of_property_read_u32(np, "dma-requests", &xbar->nr_requests))
+		xbar->nr_requests = xbar->max_nr_requests;
+
+	if (xbar->nr_requests > xbar->max_nr_requests) {
+		dev_err(&pdev->dev, "Nr requests not supported\n");
+		return -EINVAL;
+	}
+
+	platform_set_drvdata(pdev, xbar);
+
+	return 0;
+}
+
+static int st_fdma_xbar_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static struct platform_driver st_fdma_xbar_driver = {
+	.driver	= {
+		.name = "st-fdma-xbar",
+		.of_match_table = st_fdma_xbar_match,
+	},
+	.probe = st_fdma_xbar_probe,
+	.remove = st_fdma_xbar_remove,
+};
+module_platform_driver(st_fdma_xbar_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("STMicroelectronics FDMA cross bar");
+MODULE_AUTHOR("Ludovic.barre <Ludovic.barre@st.com>");
-- 
1.9.1

[PATCH 5/7] ARM: STi: DT: STiH407: Add FDMA driver and xbar driver dt nodes.

[Peter Griffin]

These nodes are required to get the fdma and xbar driver working
on STiH407 based silicon.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 arch/arm/boot/dts/stih407-family.dtsi | 59 +++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
index 838b812..b6d9e20 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi
@@ -565,5 +565,64 @@
 						  <&phy_port2 PHY_TYPE_USB3>;
 			};
 		};
+
+		xbar0: fdma-xbar-mpe@0 {
+			compatible = "st,fdma-xbar-1.0";
+			reg = <0x8e80000 0x1000>;
+			dma-requests = <79>;
+			#st,fdma-xbar-cells = <1>;
+		};
+
+		fdma0: fdma-audio@0 {
+			compatible = "st,fdma_mpe31";
+			reg = <0x8e20000 0x20000>;
+			interrupts = <GIC_SPI 5 IRQ_TYPE_NONE>;
+			dma-channels = <16>;
+			#dma-cells = <3>;
+			st,fw-name = "fdma_STiH407_0.elf";
+			clocks = <&clk_s_c0_flexgen CLK_FDMA>,
+				 <&clk_s_c0_flexgen CLK_EXT2F_A9>,
+				 <&clk_s_c0_flexgen CLK_EXT2F_A9>,
+				 <&clk_s_c0_flexgen CLK_EXT2F_A9>;
+			clock-names = "fdma_slim",
+				      "fdma_hi",
+				      "fdma_low",
+				      "fdma_ic";
+		};
+
+		fdma1: fdma-app@1 {
+			compatible = "st,fdma_mpe31";
+			reg = <0x8e40000 0x20000>;
+			interrupts = <GIC_SPI 7 IRQ_TYPE_NONE>;
+			dma-channels = <16>;
+			#dma-cells = <3>;
+			st,fdma-xbar = <&xbar0 0>;
+			st,fw-name = "fdma_STiH407_1.elf";
+			clocks = <&clk_s_c0_flexgen CLK_FDMA>,
+				 <&clk_s_c0_flexgen CLK_TX_ICN_DMU>,
+				 <&clk_s_c0_flexgen CLK_TX_ICN_DMU>,
+				 <&clk_s_c0_flexgen CLK_EXT2F_A9>;
+			clock-names = "fdma_slim",
+				      "fdma_hi",
+				      "fdma_low",
+				      "fdma_ic";
+		};
+
+		fdma2: fdma-free_running@2 {
+			compatible = "st,fdma_mpe31";
+			reg = <0x8e60000 0x20000>;
+			interrupts = <GIC_SPI 9 IRQ_TYPE_NONE>;
+			dma-channels = <16>;
+			#dma-cells = <3>;
+			st,fw-name = "fdma_STiH407_2.elf";
+			clocks = <&clk_s_c0_flexgen CLK_FDMA>,
+				 <&clk_s_c0_flexgen CLK_EXT2F_A9>,
+				 <&clk_s_c0_flexgen CLK_TX_ICN_DISP_0>,
+				 <&clk_s_c0_flexgen CLK_EXT2F_A9>;
+			clock-names = "fdma_slim",
+				      "fdma_hi",
+				      "fdma_low",
+				      "fdma_ic";
+		};
 	};
 };
-- 
1.9.1

[PATCH 6/7] MAINTAINERS: Add FDMA driver files to STi section.

[Peter Griffin]

This patch adds the FDMA driver files to the STi
section of the maintainers file.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8133cef..8cb1ad9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1503,6 +1503,8 @@ S:	Maintained
 F:	arch/arm/mach-sti/
 F:	arch/arm/boot/dts/sti*
 F:	drivers/clocksource/arm_global_timer.c
+F:	drivers/dma/st_fdma.*
+F:	drivers/dma/st_fdma_xbar.c
 F:	drivers/i2c/busses/i2c-st.c
 F:	drivers/media/rc/st_rc.c
 F:	drivers/mmc/host/sdhci-st.c
-- 
1.9.1

[PATCH 7/7] ARM: multi_v7_defconfig: Enable STi FDMA driver

[Peter Griffin]

This DMA controller is found on all STi chipsets.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 arch/arm/configs/multi_v7_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 6d83a1b..370dd85 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -588,6 +588,8 @@ CONFIG_IMX_DMA=y
 CONFIG_MXS_DMA=y
 CONFIG_DMA_OMAP=y
 CONFIG_XILINX_VDMA=y
+CONFIG_ST_FDMA=y
+CONFIG_ST_FDMA_XBAR=y
 CONFIG_STAGING=y
 CONFIG_SENSORS_ISL29018=y
 CONFIG_SENSORS_ISL29028=y
-- 
1.9.1

Re: [PATCH 3/7] dmaengine: st_fdma: Add STMicroelectronics FDMA engine driver support

[Paul Bolle]

On wo, 2015-07-08 at 17:11 +0100, Peter Griffin wrote:
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
 
> +config ST_FDMA
> +	bool "ST FDMA dmaengine support"
> +	depends on ARCH_STI
> +	select DMA_ENGINE
> +	select FW_LOADER
> +	select FW_LOADER_USER_HELPER_FALLBACK
> +	select LIBELF_32
> +	select DMA_VIRTUAL_CHANNELS
> +	help
> +	  Enable support for ST FDMA controller.
> +	  It supports 16 independent DMA channels, accepts up to 32 DMA requests
> +
> +	  Say Y here if you have such a chipset.
> +	  If unsure, say N.

> --- a/drivers/dma/Makefile
> +++ b/drivers/dma/Makefile
> 
> +obj-$(CONFIG_ST_FDMA) += st_fdma.o

ST_FDMA is a bool symbol, so st_fdma.o can only be built-in.

> --- /dev/null
> +++ b/drivers/dma/st_fdma.c

> +#include <linux/module.h>

Needed?

> +void *st_fdma_seg_to_mem(struct st_fdma_dev *fdev, u64 da, int len)

static?

> +{
> +	[...]
> +}

> +static int
> +st_fdma_elf_load_segments(struct st_fdma_dev *fdev, const struct 
> firmware *fw)
> +{
> +	[...]
> +		dst = st_fdma_seg_to_mem(fdev, da, memsz);
> +		[...]
> +}

> +static const struct of_device_id st_fdma_match[] = {
> +	{ .compatible = "st,fdma_mpe31", .data = &fdma_mpe31 },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, st_fdma_match);

MODULE_DEVICE_TABLE() is preprocessed away for built-in only code.

> +static int __exit st_fdma_remove(struct platform_device *pdev)
> +{
> +	struct st_fdma_dev *fdev = platform_get_drvdata(pdev);
> +
> +	wait_for_completion(&fdev->fw_ack);
> +
> +	st_fdma_clk_disable(fdev);
> +
> +	return 0;
> +}

Since this driver is built-in only this means st_fdma_remove() can never
be used, right?

> +static struct platform_driver st_fdma_platform_driver = {
> +	.driver = {
> +		.name = "st-fdma",
> +		.of_match_table = st_fdma_match,
> +		.pm = ST_FDMA_PM,
> +	},
> +	.probe = st_fdma_probe,
> +	.remove = st_fdma_remove,
> +};
> +module_platform_driver(st_fdma_platform_driver);

So can .remove be dropped?

Since v4.2-rc1 there's builtin_platform_driver() for built-in only code.

> +bool st_fdma_filter_fn(struct dma_chan *chan, void *param)
> +{
> +	[...]
> +}
> +EXPORT_SYMBOL(st_fdma_filter_fn);

This series adds no users of this export. I suppose they will be added
in another series. Is that correct?

> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("STMicroelectronics FDMA engine driver");
> +MODULE_AUTHOR("Ludovic.barre <Ludovic.barre@st.com>");

These macros will, basically, be preprocessed away for built-in only
code.

Thanks,


Paul Bolle

Re: [PATCH 4/7] dmaengine: st_fdma: Add xbar support

[Paul Bolle]

On wo, 2015-07-08 at 17:11 +0100, Peter Griffin wrote:
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
 
> +config ST_FDMA_XBAR
> +	bool "ST FDMA crossbar"
> +	depends on ST_FDMA
> +	default y
> +	help
> +	  Enable support for ST FDMA crossbar.
> +	  xbar add flexibility and increase the number of peripheral request
> +	  can be used by fdma xbar can multiplex until 96 peripheral requests
> +	  to one of 3 fdma controller

> diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> index f68e6d8..19f18b1 100644
> --- a/drivers/dma/Makefile
> +++ b/drivers/dma/Makefile

> +obj-$(CONFIG_ST_FDMA_XBAR) += st_fdma_xbar.o

ST_FDMA_XBAR is a bool symbol, so st_fdma_xbar.o can only be built-in.

> --- /dev/null
> +++ b/drivers/dma/st_fdma_xbar.c

> +#include <linux/module.h>

Needed?

> +void st_fdma_xbar_free(struct st_fdma_xbar_dev *device)
> +{
> +	platform_device_put(device->pdev);
> +	kfree(device);
> +}

Unused.

> +static const struct of_device_id st_fdma_xbar_match[] = {
> +	{ .compatible = "st,fdma-xbar-1.0", .data = (void *)XBAR_1_0_MAX_REQ },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, st_fdma_xbar_match);

See my remark on 3/7: MODULE_DEVICE_TABLE() will be preprocessed away.

> +static struct platform_driver st_fdma_xbar_driver = {
> +	.driver	= {
> +		.name = "st-fdma-xbar",
> +		.of_match_table = st_fdma_xbar_match,
> +	},
> +	.probe = st_fdma_xbar_probe,
> +	.remove = st_fdma_xbar_remove,
> +};
> +module_platform_driver(st_fdma_xbar_driver);

See my remark on 3/7: builtin_platform_driver(). 

> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("STMicroelectronics FDMA cross bar");
> +MODULE_AUTHOR("Ludovic.barre <Ludovic.barre@st.com>");

See my remark on 3/7: will be preprocessed away.

Thanks,


Paul Bolle

Re: [PATCH 1/7] dmaengine: st_fdma: Add STMicroelectronics FDMA DT binding documentation

[Vinod Koul]

On Wed, Jul 08, 2015 at 05:11:22PM +0100, Peter Griffin wrote:
> This patch adds the DT binding documentation for the FDMA constroller
> found on STi based chipsets from STMicroelectronics.
> 
> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  Documentation/devicetree/bindings/dma/st_fdma.txt | 76 +++++++++++++++++++++++
>  1 file changed, 76 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dma/st_fdma.txt
> 
> diff --git a/Documentation/devicetree/bindings/dma/st_fdma.txt b/Documentation/devicetree/bindings/dma/st_fdma.txt
> new file mode 100644
> index 0000000..1ec7470
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/st_fdma.txt
> @@ -0,0 +1,76 @@
> +* STMicroelectronics Flexible Direct Memory Access Device Tree bindings
> +
> +The FDMA is a general-purpose direct memory access controller capable of
> +supporting 16 independent DMA channels. It accepts up to 32 DMA requests.
> +The FDMA is based on a Slim processor which require a firmware.
> +
> +* FDMA Controller
> +
> +Required properties:
> +- compatible	: Should be "st,fdma_mpe31"
> +- reg		: Should contain DMA registers location and length
> +- interrupts	: Should contain one interrupt shared by all channel

s/channel/channels

> +- dma-channels	: Number of channels supported by the controller
> +- #dma-cells	: Must be <3>.

any reason three?

-- 
~Vinod

Re: [PATCH 3/7] dmaengine: st_fdma: Add STMicroelectronics FDMA engine driver support

[Vinod Koul]

On Wed, Jul 08, 2015 at 05:11:24PM +0100, Peter Griffin wrote:
> +static int
> +st_fdma_elf_sanity_check(struct st_fdma_dev *fdev, const struct firmware *fw)
> +{
> +	const char *fw_name = fdev->pdata->fw_name;
> +	struct elf32_hdr *ehdr;
> +	char class;
> +
> +	if (!fw) {
> +		dev_err(fdev->dev, "failed to load %s\n", fw_name);
> +		return -EINVAL;
> +	}
> +
> +	if (fw->size < sizeof(struct elf32_hdr)) {
sizeof(*ehdr) ?

> +		dev_err(fdev->dev, "Image is too small\n");
> +		return -EINVAL;
> +	}
> +
> +	ehdr = (struct elf32_hdr *)fw->data;
> +
> +	/* We only support ELF32 at this point */
> +	class = ehdr->e_ident[EI_CLASS];
> +	if (class != ELFCLASS32) {
> +		dev_err(fdev->dev, "Unsupported class: %d\n", class);
> +		return -EINVAL;
> +	}
> +
> +	if (ehdr->e_ident[EI_DATA] != ELFDATA2LSB) {
> +		dev_err(fdev->dev, "Unsupported firmware endianness\n");
would be worth printing the value for debug

> +		return -EINVAL;
> +	}
> +
> +	if (fw->size < ehdr->e_shoff + sizeof(struct elf32_shdr)) {
> +		dev_err(fdev->dev, "Image is too small\n");
Again printing size helps when you get a log trace and have no idea why size
was small. Similar one other places


> +		dst = st_fdma_seg_to_mem(fdev, da, memsz);
> +		if (!dst) {
> +			dev_err(dev, "bad phdr da 0x%x mem 0x%x\n", da, memsz);
> +			break;
> +		}
> +
> +		if (phdr->p_filesz)
> +			memcpy(dst, elf_data + phdr->p_offset, filesz);
> +
> +		if (memsz > filesz)
> +			memset(dst + filesz, 0, memsz - filesz);
> +
> +		mem_loaded++;
> +	}
> +
> +	return (mem_loaded != fdev->drvdata->num_mem) ? -EINVAL : 0;
so you are not expecting any segment with PT_LOAD otherwise this check will
fail as num_mem is assigned from e_phnum.
Also perhaps EIO will be better return?

> +}
> +
> +static void st_fdma_enable(struct st_fdma_dev *fdev)
> +{
> +	unsigned long hw_id, hw_ver, fw_rev;
> +	u32 val;
> +
> +	/* disable CPU pipeline clock & reset cpu pipeline */
> +	val = FDMA_CLK_GATE_DIS | FDMA_CLK_GATE_RESET;
> +	fdma_write(fdev, val, CLK_GATE);

empty line here

> +	/* disable SLIM core STBus sync */
> +	fdma_write(fdev, FDMA_STBUS_SYNC_DIS, STBUS_SYNC);
> +	/* enable cpu pipeline clock */
> +	fdma_write(fdev, !FDMA_CLK_GATE_DIS, CLK_GATE);
> +
> +	/* clear int & cmd mailbox */
> +	fdma_write(fdev, ~0UL, INT_CLR);
> +	fdma_write(fdev, ~0UL, CMD_CLR);

here too

> +static int st_fdma_get_fw(struct st_fdma_dev *fdev)
> +{
> +	int ret;
> +
> +	init_completion(&fdev->fw_ack);
> +	atomic_set(&fdev->fw_loaded, 0);
> +
> +	ret = request_firmware_nowait(THIS_MODULE, FW_ACTION_HOTPLUG,
> +				      fdev->pdata->fw_name, fdev->dev,
> +				      GFP_KERNEL, fdev, st_fdma_fw_cb);

Isn't doing this in device probe too stringent and holding up load...

> +	fdesc = st_fdma_alloc_desc(fchan, sg_len);
> +	if (!fdesc) {
> +		dev_err(fchan->fdev->dev, "no memory for desc\n");
> +		return NULL;
> +	}
> +
> +	fdesc->iscyclic = false;
> +
> +	for_each_sg(sgl, sg, sg_len, i) {
> +		hw_node = fdesc->node[i].desc;
> +
> +		hw_node->next = fdesc->node[(i + 1) % sg_len].pdesc;
> +		hw_node->control = NODE_CTRL_REQ_MAP_DREQ(fchan->dreq_line);
> +
> +		if (direction == DMA_MEM_TO_DEV) {
> +			hw_node->control |= NODE_CTRL_SRC_INCR;
> +			hw_node->control |= NODE_CTRL_DST_STATIC;
> +			hw_node->saddr = sg_dma_address(sg);
> +			hw_node->daddr = fchan->cfg.dev_addr;
> +		} else {
> +			hw_node->control |= NODE_CTRL_SRC_STATIC;
> +			hw_node->control |= NODE_CTRL_DST_INCR;
> +			hw_node->saddr = fchan->cfg.dev_addr;
> +			hw_node->daddr = sg_dma_address(sg);
> +		}
> +
> +		hw_node->nbytes = sg_dma_len(sg);
> +		hw_node->generic.length = sg_dma_len(sg);
> +		hw_node->generic.sstride = 0;
> +		hw_node->generic.dstride = 0;

This looks quite similar to previous one, I think some bits can be reused

> +static int st_fdma_slave_config(struct dma_chan *chan,
> +				struct dma_slave_config *slave_cfg)
> +{
> +	u32 maxburst = 0, addr = 0;
> +	enum dma_slave_buswidth width;
> +	struct st_fdma_chan *fchan = to_st_fdma_chan(chan);
> +	int ch_id = fchan->vchan.chan.chan_id;
> +	struct st_fdma_dev *fdev = fchan->fdev;
> +
> +	if (slave_cfg->direction == DMA_DEV_TO_MEM) {

This is depreciated, you can't use direction here. Please save the fields and
then use them in prep_ call

Also this is quite big patch, consider splitting it up for faster review

-- 
~Vinod

Re: [PATCH 1/7] dmaengine: st_fdma: Add STMicroelectronics FDMA DT binding documentation

[Peter Griffin]

Hi Vinod,

Thanks for reviewing.

On Wed, 19 Aug 2015, Vinod Koul wrote:

> On Wed, Jul 08, 2015 at 05:11:22PM +0100, Peter Griffin wrote:
> > This patch adds the DT binding documentation for the FDMA constroller
> > found on STi based chipsets from STMicroelectronics.
> > 
> > Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > ---
> >  Documentation/devicetree/bindings/dma/st_fdma.txt | 76 +++++++++++++++++++++++
> >  1 file changed, 76 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/dma/st_fdma.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/dma/st_fdma.txt b/Documentation/devicetree/bindings/dma/st_fdma.txt
> > new file mode 100644
> > index 0000000..1ec7470
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/dma/st_fdma.txt
> > @@ -0,0 +1,76 @@
> > +* STMicroelectronics Flexible Direct Memory Access Device Tree bindings
> > +
> > +The FDMA is a general-purpose direct memory access controller capable of
> > +supporting 16 independent DMA channels. It accepts up to 32 DMA requests.
> > +The FDMA is based on a Slim processor which require a firmware.
> > +
> > +* FDMA Controller
> > +
> > +Required properties:
> > +- compatible	: Should be "st,fdma_mpe31"
> > +- reg		: Should contain DMA registers location and length
> > +- interrupts	: Should contain one interrupt shared by all channel
> 
> s/channel/channels

Will fix in v2.

> 
> > +- dma-channels	: Number of channels supported by the controller
> > +- #dma-cells	: Must be <3>.
> 
> any reason three?

Yes, it is documented further down in the file under DMA Client.

To make it clearer in V2 I could change to

"- #dma-cells	: Must be <3>. See DMA client section below"?

regards,

Peter.

Re: [PATCH 1/7] dmaengine: st_fdma: Add STMicroelectronics FDMA DT binding documentation

[Lee Jones]

On Tue, 25 Aug 2015, Peter Griffin wrote:

> Hi Vinod,
> 
> Thanks for reviewing.
> 
> On Wed, 19 Aug 2015, Vinod Koul wrote:
> 
> > On Wed, Jul 08, 2015 at 05:11:22PM +0100, Peter Griffin wrote:
> > > This patch adds the DT binding documentation for the FDMA constroller
> > > found on STi based chipsets from STMicroelectronics.
> > > 
> > > Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
> > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > > ---
> > >  Documentation/devicetree/bindings/dma/st_fdma.txt | 76 +++++++++++++++++++++++
> > >  1 file changed, 76 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/dma/st_fdma.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/dma/st_fdma.txt b/Documentation/devicetree/bindings/dma/st_fdma.txt
> > > new file mode 100644
> > > index 0000000..1ec7470
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/dma/st_fdma.txt
> > > @@ -0,0 +1,76 @@
> > > +* STMicroelectronics Flexible Direct Memory Access Device Tree bindings
> > > +
> > > +The FDMA is a general-purpose direct memory access controller capable of
> > > +supporting 16 independent DMA channels. It accepts up to 32 DMA requests.
> > > +The FDMA is based on a Slim processor which require a firmware.
> > > +
> > > +* FDMA Controller
> > > +
> > > +Required properties:
> > > +- compatible	: Should be "st,fdma_mpe31"
> > > +- reg		: Should contain DMA registers location and length
> > > +- interrupts	: Should contain one interrupt shared by all channel
> > 
> > s/channel/channels
> 
> Will fix in v2.
> 
> > 
> > > +- dma-channels	: Number of channels supported by the controller
> > > +- #dma-cells	: Must be <3>.
> > 
> > any reason three?
> 
> Yes, it is documented further down in the file under DMA Client.
> 
> To make it clearer in V2 I could change to
> 
> "- #dma-cells	: Must be <3>. See DMA client section below"?

- #dma-cells	: Must be <3>
			1st cell: Phandle to       ... <blah>
			2nd cell: DMA channel      ... <blah>
			3rd cell: Flags describing ... <blah>   

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 6/7] MAINTAINERS: Add FDMA driver files to STi section.

[Lee Jones]

On Wed, 08 Jul 2015, Peter Griffin wrote:

> This patch adds the FDMA driver files to the STi
> section of the maintainers file.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  MAINTAINERS | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8133cef..8cb1ad9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1503,6 +1503,8 @@ S:	Maintained
>  F:	arch/arm/mach-sti/
>  F:	arch/arm/boot/dts/sti*
>  F:	drivers/clocksource/arm_global_timer.c
> +F:	drivers/dma/st_fdma.*
> +F:	drivers/dma/st_fdma_xbar.c

How about you replace both lines with:

F:	drivers/dma/st_fdma*

>  F:	drivers/i2c/busses/i2c-st.c
>  F:	drivers/media/rc/st_rc.c
>  F:	drivers/mmc/host/sdhci-st.c

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 5/7] ARM: STi: DT: STiH407: Add FDMA driver and xbar driver dt nodes.

[Lee Jones]

On Wed, 08 Jul 2015, Peter Griffin wrote:

> These nodes are required to get the fdma and xbar driver working
> on STiH407 based silicon.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  arch/arm/boot/dts/stih407-family.dtsi | 59 +++++++++++++++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
> index 838b812..b6d9e20 100644
> --- a/arch/arm/boot/dts/stih407-family.dtsi
> +++ b/arch/arm/boot/dts/stih407-family.dtsi
> @@ -565,5 +565,64 @@
>  						  <&phy_port2 PHY_TYPE_USB3>;
>  			};
>  		};
> +
> +		xbar0: fdma-xbar-mpe@0 {

The unit address should match the one in the reg property.

> +			compatible = "st,fdma-xbar-1.0";

Is that a version number in the compatible string?

> +			reg = <0x8e80000 0x1000>;
> +			dma-requests = <79>;
> +			#st,fdma-xbar-cells = <1>;

Why can't FDMA use the generic DMA properties?

> +		};
> +
> +		fdma0: fdma-audio@0 {

Same comment as above.

> +			compatible = "st,fdma_mpe31";

DT doesn't usually like underscores.

> +			reg = <0x8e20000 0x20000>;
> +			interrupts = <GIC_SPI 5 IRQ_TYPE_NONE>;
> +			dma-channels = <16>;
> +			#dma-cells = <3>;
> +			st,fw-name = "fdma_STiH407_0.elf";

I've bought this up before somewhere (although there doesn't appear to
be a Firmware Maintainer), surely we should define a generic binding
for passing firmware names?  Many vendors are going to require it.

> +			clocks = <&clk_s_c0_flexgen CLK_FDMA>,
> +				 <&clk_s_c0_flexgen CLK_EXT2F_A9>,
> +				 <&clk_s_c0_flexgen CLK_EXT2F_A9>,
> +				 <&clk_s_c0_flexgen CLK_EXT2F_A9>;
> +			clock-names = "fdma_slim",
> +				      "fdma_hi",
> +				      "fdma_low",
> +				      "fdma_ic";
> +		};
> +
> +		fdma1: fdma-app@1 {

As above.

> +			compatible = "st,fdma_mpe31";

As above.

> +			reg = <0x8e40000 0x20000>;
> +			interrupts = <GIC_SPI 7 IRQ_TYPE_NONE>;
> +			dma-channels = <16>;
> +			#dma-cells = <3>;
> +			st,fdma-xbar = <&xbar0 0>;
> +			st,fw-name = "fdma_STiH407_1.elf";
> +			clocks = <&clk_s_c0_flexgen CLK_FDMA>,
> +				 <&clk_s_c0_flexgen CLK_TX_ICN_DMU>,
> +				 <&clk_s_c0_flexgen CLK_TX_ICN_DMU>,
> +				 <&clk_s_c0_flexgen CLK_EXT2F_A9>;
> +			clock-names = "fdma_slim",
> +				      "fdma_hi",
> +				      "fdma_low",
> +				      "fdma_ic";
> +		};
> +
> +		fdma2: fdma-free_running@2 {
> +			compatible = "st,fdma_mpe31";
> +			reg = <0x8e60000 0x20000>;
> +			interrupts = <GIC_SPI 9 IRQ_TYPE_NONE>;
> +			dma-channels = <16>;
> +			#dma-cells = <3>;
> +			st,fw-name = "fdma_STiH407_2.elf";
> +			clocks = <&clk_s_c0_flexgen CLK_FDMA>,
> +				 <&clk_s_c0_flexgen CLK_EXT2F_A9>,
> +				 <&clk_s_c0_flexgen CLK_TX_ICN_DISP_0>,
> +				 <&clk_s_c0_flexgen CLK_EXT2F_A9>;
> +			clock-names = "fdma_slim",
> +				      "fdma_hi",
> +				      "fdma_low",
> +				      "fdma_ic";
> +		};
>  	};
>  };

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 3/7] dmaengine: st_fdma: Add STMicroelectronics FDMA engine driver support

[Peter Griffin]

Hi Paul,

Thanks for reviewing.

On Thu, 09 Jul 2015, Paul Bolle wrote:

> On wo, 2015-07-08 at 17:11 +0100, Peter Griffin wrote:
> > --- a/drivers/dma/Kconfig
> > +++ b/drivers/dma/Kconfig
>  
> > +config ST_FDMA
> > +	bool "ST FDMA dmaengine support"
> > +	depends on ARCH_STI
> > +	select DMA_ENGINE
> > +	select FW_LOADER
> > +	select FW_LOADER_USER_HELPER_FALLBACK
> > +	select LIBELF_32
> > +	select DMA_VIRTUAL_CHANNELS
> > +	help
> > +	  Enable support for ST FDMA controller.
> > +	  It supports 16 independent DMA channels, accepts up to 32 DMA requests
> > +
> > +	  Say Y here if you have such a chipset.
> > +	  If unsure, say N.
> 
> > --- a/drivers/dma/Makefile
> > +++ b/drivers/dma/Makefile
> > 
> > +obj-$(CONFIG_ST_FDMA) += st_fdma.o
> 
> ST_FDMA is a bool symbol, so st_fdma.o can only be built-in.

Yes good spot, that is a mistake it should be tristate. Will fix in v2.

> 
> > --- /dev/null
> > +++ b/drivers/dma/st_fdma.c
> 
> > +#include <linux/module.h>
> 
> Needed?
> 
> > +void *st_fdma_seg_to_mem(struct st_fdma_dev *fdev, u64 da, int len)
> 
> static?

Fixed in v2.

> 
> > +{
> > +	[...]
> > +}
> 
> > +static int
> > +st_fdma_elf_load_segments(struct st_fdma_dev *fdev, const struct 
> > firmware *fw)
> > +{
> > +	[...]
> > +		dst = st_fdma_seg_to_mem(fdev, da, memsz);
> > +		[...]
> > +}
> 
> > +static const struct of_device_id st_fdma_match[] = {
> > +	{ .compatible = "st,fdma_mpe31", .data = &fdma_mpe31 },
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, st_fdma_match);
> 
> MODULE_DEVICE_TABLE() is preprocessed away for built-in only code.
> 
> > +static int __exit st_fdma_remove(struct platform_device *pdev)
> > +{
> > +	struct st_fdma_dev *fdev = platform_get_drvdata(pdev);
> > +
> > +	wait_for_completion(&fdev->fw_ack);
> > +
> > +	st_fdma_clk_disable(fdev);
> > +
> > +	return 0;
> > +}
> 
> Since this driver is built-in only this means st_fdma_remove() can never
> be used, right?

Will be capable of being a module in v2.

> 
> > +static struct platform_driver st_fdma_platform_driver = {
> > +	.driver = {
> > +		.name = "st-fdma",
> > +		.of_match_table = st_fdma_match,
> > +		.pm = ST_FDMA_PM,
> > +	},
> > +	.probe = st_fdma_probe,
> > +	.remove = st_fdma_remove,
> > +};
> > +module_platform_driver(st_fdma_platform_driver);
> 
> So can .remove be dropped?
> 
> Since v4.2-rc1 there's builtin_platform_driver() for built-in only code.

Thanks for the pointer, I wasn't aware of that function.

> 
> > +bool st_fdma_filter_fn(struct dma_chan *chan, void *param)
> > +{
> > +	[...]
> > +}
> > +EXPORT_SYMBOL(st_fdma_filter_fn);
> 
> This series adds no users of this export. I suppose they will be added
> in another series. Is that correct?

No the export is not required. Will fix in v2.

> 
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_DESCRIPTION("STMicroelectronics FDMA engine driver");
> > +MODULE_AUTHOR("Ludovic.barre <Ludovic.barre@st.com>");
> 
> These macros will, basically, be preprocessed away for built-in only
> code.

The driver will be capable of being a module in v2.

regards,

Peter.

Re: [PATCH 3/7] dmaengine: st_fdma: Add STMicroelectronics FDMA engine driver support

[Maxime Coquelin]

Hi Paul,

On 07/09/2015 10:17 AM, Paul Bolle wrote:
>> >+static int __exit st_fdma_remove(struct platform_device *pdev)
>> >+{
>> >+	struct st_fdma_dev *fdev = platform_get_drvdata(pdev);
>> >+
>> >+	wait_for_completion(&fdev->fw_ack);
>> >+
>> >+	st_fdma_clk_disable(fdev);
>> >+
>> >+	return 0;
>> >+}
> Since this driver is built-in only this means st_fdma_remove() can never
> be used, right?
>

It's not because a driver is built-in only that it does not need a 
remove callback.
An instance can be probed/removed any time via driver's bind/unbind 
SysFS entries.
Am I missing something?

Kind regards,
Maxime

Re: [PATCH 3/7] dmaengine: st_fdma: Add STMicroelectronics FDMA engine driver support

[Paul Bolle]

Hi Maxime,

On ma, 2015-08-31 at 09:49 +0200, Maxime Coquelin wrote:
> On 07/09/2015 10:17 AM, Paul Bolle wrote:
> > > > +static int __exit st_fdma_remove(struct platform_device *pdev)
> > > > +{
> > > > +	struct st_fdma_dev *fdev = platform_get_drvdata(pdev);
> > > > +
> > > > +	wait_for_completion(&fdev->fw_ack);
> > > > +
> > > > +	st_fdma_clk_disable(fdev);
> > > > +
> > > > +	return 0;
> > > > +}
> > Since this driver is built-in only this means st_fdma_remove() can 
> > never be used, right?

> It's not because a driver is built-in only that it does not need a 
> remove callback.
> An instance can be probed/removed any time via driver's bind/unbind 
> SysFS entries.
> Am I missing something?

(This discussion is moot because Peter already stated that a new version
will be modular.)

It follows from the __exit tag that st_fdma_remove() should never be
part of the kernel image (in this version of the patch), doesn't it?

(I don't know what happens in this situation if an unbind sysfs entry is
used to remove a driver. I've never tried that.)


Paul Bolle

Re: [PATCH 3/7] dmaengine: st_fdma: Add STMicroelectronics FDMA engine driver support

[Maxime Coquelin]

On 08/31/2015 10:08 AM, Paul Bolle wrote:
> Hi Maxime,
>
> On ma, 2015-08-31 at 09:49 +0200, Maxime Coquelin wrote:
>> On 07/09/2015 10:17 AM, Paul Bolle wrote:
>>>>> +static int __exit st_fdma_remove(struct platform_device *pdev)
>>>>> +{
>>>>> +	struct st_fdma_dev *fdev = platform_get_drvdata(pdev);
>>>>> +
>>>>> +	wait_for_completion(&fdev->fw_ack);
>>>>> +
>>>>> +	st_fdma_clk_disable(fdev);
>>>>> +
>>>>> +	return 0;
>>>>> +}
>>> Since this driver is built-in only this means st_fdma_remove() can
>>> never be used, right?
>> It's not because a driver is built-in only that it does not need a
>> remove callback.
>> An instance can be probed/removed any time via driver's bind/unbind
>> SysFS entries.
>> Am I missing something?
> (This discussion is moot because Peter already stated that a new version
> will be modular.)
>
> It follows from the __exit tag that st_fdma_remove() should never be
> part of the kernel image (in this version of the patch), doesn't it?
Yes, you are right.
The remove callback is relevant, but without the __exit tag.
>
> (I don't know what happens in this situation if an unbind sysfs entry is
> used to remove a driver. I've never tried that.)
>
>
Not checked whether the function is omitted when built-in, but in any 
case, I thnk remove callbacks should not be tagged with __exit.

Regards,
Maxime

Re: [PATCH 6/7] MAINTAINERS: Add FDMA driver files to STi section.

[Peter Griffin]

Hi Lee,

On Wed, 26 Aug 2015, Lee Jones wrote:

> On Wed, 08 Jul 2015, Peter Griffin wrote:
> 
> > This patch adds the FDMA driver files to the STi
> > section of the maintainers file.
> > 
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > ---
> >  MAINTAINERS | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 8133cef..8cb1ad9 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1503,6 +1503,8 @@ S:	Maintained
> >  F:	arch/arm/mach-sti/
> >  F:	arch/arm/boot/dts/sti*
> >  F:	drivers/clocksource/arm_global_timer.c
> > +F:	drivers/dma/st_fdma.*
> > +F:	drivers/dma/st_fdma_xbar.c
> 
> How about you replace both lines with:
> 
> F:	drivers/dma/st_fdma*

Ok, will fix in next version.

regards,

Peter.

Re: [PATCH 3/7] dmaengine: st_fdma: Add STMicroelectronics FDMA engine driver support

[Peter Griffin]

Hi Vinod,

Thanks for reviewing.

On Wed, 19 Aug 2015, Vinod Koul wrote:

> On Wed, Jul 08, 2015 at 05:11:24PM +0100, Peter Griffin wrote:
> > +static int
> > +st_fdma_elf_sanity_check(struct st_fdma_dev *fdev, const struct firmware *fw)
> > +{
> > +	const char *fw_name = fdev->pdata->fw_name;
> > +	struct elf32_hdr *ehdr;
> > +	char class;
> > +
> > +	if (!fw) {
> > +		dev_err(fdev->dev, "failed to load %s\n", fw_name);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (fw->size < sizeof(struct elf32_hdr)) {
> sizeof(*ehdr) ?

Ok fixed in v2.

> 
> > +		dev_err(fdev->dev, "Image is too small\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	ehdr = (struct elf32_hdr *)fw->data;
> > +
> > +	/* We only support ELF32 at this point */
> > +	class = ehdr->e_ident[EI_CLASS];
> > +	if (class != ELFCLASS32) {
> > +		dev_err(fdev->dev, "Unsupported class: %d\n", class);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (ehdr->e_ident[EI_DATA] != ELFDATA2LSB) {
> > +		dev_err(fdev->dev, "Unsupported firmware endianness\n");
> would be worth printing the value for debug

Fixed in v2

> 
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (fw->size < ehdr->e_shoff + sizeof(struct elf32_shdr)) {
> > +		dev_err(fdev->dev, "Image is too small\n");
> Again printing size helps when you get a log trace and have no idea why size
> was small. Similar one other places

Fixed in v2

> 
> 
> > +		dst = st_fdma_seg_to_mem(fdev, da, memsz);
> > +		if (!dst) {
> > +			dev_err(dev, "bad phdr da 0x%x mem 0x%x\n", da, memsz);
> > +			break;
> > +		}
> > +
> > +		if (phdr->p_filesz)
> > +			memcpy(dst, elf_data + phdr->p_offset, filesz);
> > +
> > +		if (memsz > filesz)
> > +			memset(dst + filesz, 0, memsz - filesz);
> > +
> > +		mem_loaded++;
> > +	}
> > +
> > +	return (mem_loaded != fdev->drvdata->num_mem) ? -EINVAL : 0;
> so you are not expecting any segment with PT_LOAD otherwise this check will
> fail as num_mem is assigned from e_phnum.

That is correct.

> Also perhaps EIO will be better return?

Updated to -EIO in v2.

> 
> > +}
> > +
> > +static void st_fdma_enable(struct st_fdma_dev *fdev)
> > +{
> > +	unsigned long hw_id, hw_ver, fw_rev;
> > +	u32 val;
> > +
> > +	/* disable CPU pipeline clock & reset cpu pipeline */
> > +	val = FDMA_CLK_GATE_DIS | FDMA_CLK_GATE_RESET;
> > +	fdma_write(fdev, val, CLK_GATE);
> 
> empty line here

Removed in v2.

> 
> > +	/* disable SLIM core STBus sync */
> > +	fdma_write(fdev, FDMA_STBUS_SYNC_DIS, STBUS_SYNC);
> > +	/* enable cpu pipeline clock */
> > +	fdma_write(fdev, !FDMA_CLK_GATE_DIS, CLK_GATE);
> > +
> > +	/* clear int & cmd mailbox */
> > +	fdma_write(fdev, ~0UL, INT_CLR);
> > +	fdma_write(fdev, ~0UL, CMD_CLR);
> 
> here too

Removed in v2.
> 
> > +static int st_fdma_get_fw(struct st_fdma_dev *fdev)
> > +{
> > +	int ret;
> > +
> > +	init_completion(&fdev->fw_ack);
> > +	atomic_set(&fdev->fw_loaded, 0);
> > +
> > +	ret = request_firmware_nowait(THIS_MODULE, FW_ACTION_HOTPLUG,
> > +				      fdev->pdata->fw_name, fdev->dev,
> > +				      GFP_KERNEL, fdev, st_fdma_fw_cb);
> 
> Isn't doing this in device probe too stringent and holding up load...

No I don't think so, as we are using the _nowait variant which is
asynchronous and sleeps for the smallest period possible so as not
to increase boot time of built-in code.

We could use GFP_ATOMIC which means request_firmware_nowait can't sleep
at all. Although no other drivers which use request_firmware_nowait
in their probe functions do that which I can see.

> 
> > +	fdesc = st_fdma_alloc_desc(fchan, sg_len);
> > +	if (!fdesc) {
> > +		dev_err(fchan->fdev->dev, "no memory for desc\n");
> > +		return NULL;
> > +	}
> > +
> > +	fdesc->iscyclic = false;
> > +
> > +	for_each_sg(sgl, sg, sg_len, i) {
> > +		hw_node = fdesc->node[i].desc;
> > +
> > +		hw_node->next = fdesc->node[(i + 1) % sg_len].pdesc;
> > +		hw_node->control = NODE_CTRL_REQ_MAP_DREQ(fchan->dreq_line);
> > +
> > +		if (direction == DMA_MEM_TO_DEV) {
> > +			hw_node->control |= NODE_CTRL_SRC_INCR;
> > +			hw_node->control |= NODE_CTRL_DST_STATIC;
> > +			hw_node->saddr = sg_dma_address(sg);
> > +			hw_node->daddr = fchan->cfg.dev_addr;
> > +		} else {
> > +			hw_node->control |= NODE_CTRL_SRC_STATIC;
> > +			hw_node->control |= NODE_CTRL_DST_INCR;
> > +			hw_node->saddr = fchan->cfg.dev_addr;
> > +			hw_node->daddr = sg_dma_address(sg);
> > +		}
> > +
> > +		hw_node->nbytes = sg_dma_len(sg);
> > +		hw_node->generic.length = sg_dma_len(sg);
> > +		hw_node->generic.sstride = 0;
> > +		hw_node->generic.dstride = 0;
> 
> This looks quite similar to previous one, I think some bits can be reused

Will abstract out common parts in v2.

> 
> > +static int st_fdma_slave_config(struct dma_chan *chan,
> > +				struct dma_slave_config *slave_cfg)
> > +{
> > +	u32 maxburst = 0, addr = 0;
> > +	enum dma_slave_buswidth width;
> > +	struct st_fdma_chan *fchan = to_st_fdma_chan(chan);
> > +	int ch_id = fchan->vchan.chan.chan_id;
> > +	struct st_fdma_dev *fdev = fchan->fdev;
> > +
> > +	if (slave_cfg->direction == DMA_DEV_TO_MEM) {
> 
> This is depreciated, you can't use direction here. Please save the fields and
> then use them in prep_ call

Ok, will save the fields and use in prep_ calls in v2.

> 
> Also this is quite big patch, consider splitting it up for faster review

Will split into smaller patches in v2.

regards,

Peter.

Re: [PATCH 1/7] dmaengine: st_fdma: Add STMicroelectronics FDMA DT binding documentation

[Peter Griffin]

Hi Lee,

On Wed, 26 Aug 2015, Lee Jones wrote:

> On Tue, 25 Aug 2015, Peter Griffin wrote:
> 
> > Hi Vinod,
> > 
> > Thanks for reviewing.
> > 
> > On Wed, 19 Aug 2015, Vinod Koul wrote:
> > 
> > > On Wed, Jul 08, 2015 at 05:11:22PM +0100, Peter Griffin wrote:
> > > > This patch adds the DT binding documentation for the FDMA constroller
> > > > found on STi based chipsets from STMicroelectronics.
> > > > 
> > > > Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
> > > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > > > ---
> > > >  Documentation/devicetree/bindings/dma/st_fdma.txt | 76 +++++++++++++++++++++++
> > > >  1 file changed, 76 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/dma/st_fdma.txt
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/dma/st_fdma.txt b/Documentation/devicetree/bindings/dma/st_fdma.txt
> > > > new file mode 100644
> > > > index 0000000..1ec7470
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/dma/st_fdma.txt
> > > > @@ -0,0 +1,76 @@
> > > > +* STMicroelectronics Flexible Direct Memory Access Device Tree bindings
> > > > +
> > > > +The FDMA is a general-purpose direct memory access controller capable of
> > > > +supporting 16 independent DMA channels. It accepts up to 32 DMA requests.
> > > > +The FDMA is based on a Slim processor which require a firmware.
> > > > +
> > > > +* FDMA Controller
> > > > +
> > > > +Required properties:
> > > > +- compatible	: Should be "st,fdma_mpe31"
> > > > +- reg		: Should contain DMA registers location and length
> > > > +- interrupts	: Should contain one interrupt shared by all channel
> > > 
> > > s/channel/channels
> > 
> > Will fix in v2.
> > 
> > > 
> > > > +- dma-channels	: Number of channels supported by the controller
> > > > +- #dma-cells	: Must be <3>.
> > > 
> > > any reason three?
> > 
> > Yes, it is documented further down in the file under DMA Client.
> > 
> > To make it clearer in V2 I could change to
> > 
> > "- #dma-cells	: Must be <3>. See DMA client section below"?
> 
> - #dma-cells	: Must be <3>
> 			1st cell: Phandle to       ... <blah>
> 			2nd cell: DMA channel      ... <blah>
> 			3rd cell: Flags describing ... <blah>  

I'd prefer not to document it twice in the same file as it is
almost 20 lines long. IMHO it makes more sense to document it under
the dma-client section like it is done presently, because when writing a
dma-client node you need to know what these things mean.

When writing a controller node all you only need to know is
"Must be <3>". A more avid reader now has a nice pointer to scroll
down if they wish to find out more.

regards,

Peter.

Re: [PATCH 5/7] ARM: STi: DT: STiH407: Add FDMA driver and xbar driver dt nodes.

[Peter Griffin]

Hi Lee,

On Wed, 26 Aug 2015, Lee Jones wrote:

> On Wed, 08 Jul 2015, Peter Griffin wrote:
> 
> > These nodes are required to get the fdma and xbar driver working
> > on STiH407 based silicon.
> > 
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > ---
> >  arch/arm/boot/dts/stih407-family.dtsi | 59 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 59 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
> > index 838b812..b6d9e20 100644
> > --- a/arch/arm/boot/dts/stih407-family.dtsi
> > +++ b/arch/arm/boot/dts/stih407-family.dtsi
> > @@ -565,5 +565,64 @@
> >  						  <&phy_port2 PHY_TYPE_USB3>;
> >  			};
> >  		};
> > +
> > +		xbar0: fdma-xbar-mpe@0 {
> 
> The unit address should match the one in the reg property.

Will fix in v2.

> 
> > +			compatible = "st,fdma-xbar-1.0";
> 
> Is that a version number in the compatible string?

Yes it is the version number of the IP.

> 
> > +			reg = <0x8e80000 0x1000>;
> > +			dma-requests = <79>;
> > +			#st,fdma-xbar-cells = <1>;
> 
> Why can't FDMA use the generic DMA properties?

Well the FDMA controller does, but this is a seperate piece of hw
which is really a router rather than a controller.

However I plan on temporarily removing xbar from this series,
as I think it should be re-worked to use the of_dma_router_register
and friends provided by the TI xbar driver.

It looks like the two xbar hw are serving a very similar purpose,
so once that re-work is done it can also use generic DMA propoerties
as I believe using that framework it will get registered as a controller.
> 
> > +		};
> > +
> > +		fdma0: fdma-audio@0 {
> 
> Same comment as above.

Will fix in v2.

> 
> > +			compatible = "st,fdma_mpe31";
> 
> DT doesn't usually like underscores.

Obviously I can trivially change it, but out of interest is there
a technical reason for doing so? Clearly it works OK with
an underscore and a quick grep reveals a lot of ste, and some tegra
and vexpress compatibles using underscore as well.

> 
> > +			reg = <0x8e20000 0x20000>;
> > +			interrupts = <GIC_SPI 5 IRQ_TYPE_NONE>;
> > +			dma-channels = <16>;
> > +			#dma-cells = <3>;
> > +			st,fw-name = "fdma_STiH407_0.elf";
> 
> I've bought this up before somewhere (although there doesn't appear to
> be a Firmware Maintainer), surely we should define a generic binding
> for passing firmware names?  Many vendors are going to require it.

A generic binding would certainly be useful.

>From what I can see TI are using: -

    ti,pm-firmware in wkup_m3_rproc.c

and Freescale are using: -

fsl,sdma-ram-script-name in imx-sdma.c

So other platforms are putting it in device tree, there are probably other
examples.

Most other STi drivers keep the firmware name in the C code, which is
usually my first preference. However with fdma it is more comlicated
as there is a seperate firmware file for each instance of the IP block.

I will see if there is a way to remove this from DT and keep it in
the driver.

> 
> > +			clocks = <&clk_s_c0_flexgen CLK_FDMA>,
> > +				 <&clk_s_c0_flexgen CLK_EXT2F_A9>,
> > +				 <&clk_s_c0_flexgen CLK_EXT2F_A9>,
> > +				 <&clk_s_c0_flexgen CLK_EXT2F_A9>;
> > +			clock-names = "fdma_slim",
> > +				      "fdma_hi",
> > +				      "fdma_low",
> > +				      "fdma_ic";
> > +		};
> > +
> > +		fdma1: fdma-app@1 {
> 
> As above.

Will fix
> 
> > +			compatible = "st,fdma_mpe31";
> 
> As above.

Will fix
> 
> > +			reg = <0x8e40000 0x20000>;
> > +			interrupts = <GIC_SPI 7 IRQ_TYPE_NONE>;
> > +			dma-channels = <16>;
> > +			#dma-cells = <3>;
> > +			st,fdma-xbar = <&xbar0 0>;
> > +			st,fw-name = "fdma_STiH407_1.elf";
> > +			clocks = <&clk_s_c0_flexgen CLK_FDMA>,
> > +				 <&clk_s_c0_flexgen CLK_TX_ICN_DMU>,
> > +				 <&clk_s_c0_flexgen CLK_TX_ICN_DMU>,
> > +				 <&clk_s_c0_flexgen CLK_EXT2F_A9>;
> > +			clock-names = "fdma_slim",
> > +				      "fdma_hi",
> > +				      "fdma_low",
> > +				      "fdma_ic";
> > +		};
> > +
> > +		fdma2: fdma-free_running@2 {
> > +			compatible = "st,fdma_mpe31";
> > +			reg = <0x8e60000 0x20000>;
> > +			interrupts = <GIC_SPI 9 IRQ_TYPE_NONE>;
> > +			dma-channels = <16>;
> > +			#dma-cells = <3>;
> > +			st,fw-name = "fdma_STiH407_2.elf";
> > +			clocks = <&clk_s_c0_flexgen CLK_FDMA>,
> > +				 <&clk_s_c0_flexgen CLK_EXT2F_A9>,
> > +				 <&clk_s_c0_flexgen CLK_TX_ICN_DISP_0>,
> > +				 <&clk_s_c0_flexgen CLK_EXT2F_A9>;
> > +			clock-names = "fdma_slim",
> > +				      "fdma_hi",
> > +				      "fdma_low",
> > +				      "fdma_ic";
> > +		};
> >  	};
> >  };
> 
> -- 
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

regards,

Peter.