2013-05-11 11:30:16

by Heiko Stübner

[permalink] [raw]
Subject: [RFC 0/4] ARM: S3C24XX: add dmaengine based dma-driver

This series tries to provide a basic dmaengine driver for the s3c24xx
SoCs.

The driver currently has some limitations, in that it does not support the
earlier s3c24xx socs, that cannot use every channel, but have special
channel requirements for specific slave-targets.

Another limitation is, that it currently does not support scatter-gather
lists with more than element, due to me not understanding sg at first.
While it does not hinder the usability, as all applicable Samsung drivers
currently use only 1-element-lists, I plan to fix this in the next revision.

In any case, I would be thankful for pointers to the obvious mistakes I'll
probably have made, due to this being my first travel into dmaengine-land.


Heiko Stuebner (4):
ARM: S3C24XX: number the dma clocks
dma: add dmaengine driver for Samsung s3c24xx SoCs
ARM: S3C24XX: add platform-devices for new dma driver for s3c2412 and s3c2443
ARM: SAMSUNG: set s3c24xx_dma_filter for s3c64xx-spi0 device

arch/arm/mach-s3c24xx/clock-s3c2412.c | 8 +-
arch/arm/mach-s3c24xx/common-s3c2443.c | 12 +-
arch/arm/mach-s3c24xx/common.c | 103 +++
arch/arm/mach-s3c24xx/common.h | 3 +
arch/arm/plat-samsung/devs.c | 5 +-
drivers/dma/Kconfig | 12 +-
drivers/dma/Makefile | 1 +
drivers/dma/s3c24xx-dma.c | 1129 +++++++++++++++++++++++++++++
include/linux/platform_data/dma-s3c24xx.h | 54 ++
9 files changed, 1315 insertions(+), 12 deletions(-)
create mode 100644 drivers/dma/s3c24xx-dma.c
create mode 100644 include/linux/platform_data/dma-s3c24xx.h

--
1.7.2.3


2013-05-11 11:30:49

by Heiko Stübner

[permalink] [raw]
Subject: [RFC 1/4] ARM: S3C24XX: number the dma clocks

Each dma channel has its own clock. The upcoming dma driver wants to
handle these itself and therefore needs to be able to get the correct
clock for a channel.

Therefore rename the dma clocks to "dma.X" for s3c2412, s3c2416 and
s3c2443. This does not change the behaviour for the old dma driver at
all, as all dma clocks are still always on and not handled by the old
dma driver at all.

Signed-off-by: Heiko Stuebner <[email protected]>
are always enabled in the original code.
---
arch/arm/mach-s3c24xx/clock-s3c2412.c | 8 ++++----
arch/arm/mach-s3c24xx/common-s3c2443.c | 12 ++++++------
2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-s3c24xx/clock-s3c2412.c b/arch/arm/mach-s3c24xx/clock-s3c2412.c
index 2cc017d..26e2bdc 100644
--- a/arch/arm/mach-s3c24xx/clock-s3c2412.c
+++ b/arch/arm/mach-s3c24xx/clock-s3c2412.c
@@ -484,22 +484,22 @@ static struct clk init_clocks_disable[] = {

static struct clk init_clocks[] = {
{
- .name = "dma",
+ .name = "dma.0",
.parent = &clk_h,
.enable = s3c2412_clkcon_enable,
.ctrlbit = S3C2412_CLKCON_DMA0,
}, {
- .name = "dma",
+ .name = "dma.1",
.parent = &clk_h,
.enable = s3c2412_clkcon_enable,
.ctrlbit = S3C2412_CLKCON_DMA1,
}, {
- .name = "dma",
+ .name = "dma.2",
.parent = &clk_h,
.enable = s3c2412_clkcon_enable,
.ctrlbit = S3C2412_CLKCON_DMA2,
}, {
- .name = "dma",
+ .name = "dma.3",
.parent = &clk_h,
.enable = s3c2412_clkcon_enable,
.ctrlbit = S3C2412_CLKCON_DMA3,
diff --git a/arch/arm/mach-s3c24xx/common-s3c2443.c b/arch/arm/mach-s3c24xx/common-s3c2443.c
index 869bb9c..7f78533 100644
--- a/arch/arm/mach-s3c24xx/common-s3c2443.c
+++ b/arch/arm/mach-s3c24xx/common-s3c2443.c
@@ -500,32 +500,32 @@ static struct clk init_clocks_off[] = {

static struct clk init_clocks[] = {
{
- .name = "dma",
+ .name = "dma.0",
.parent = &clk_h,
.enable = s3c2443_clkcon_enable_h,
.ctrlbit = S3C2443_HCLKCON_DMA0,
}, {
- .name = "dma",
+ .name = "dma.1",
.parent = &clk_h,
.enable = s3c2443_clkcon_enable_h,
.ctrlbit = S3C2443_HCLKCON_DMA1,
}, {
- .name = "dma",
+ .name = "dma.2",
.parent = &clk_h,
.enable = s3c2443_clkcon_enable_h,
.ctrlbit = S3C2443_HCLKCON_DMA2,
}, {
- .name = "dma",
+ .name = "dma.3",
.parent = &clk_h,
.enable = s3c2443_clkcon_enable_h,
.ctrlbit = S3C2443_HCLKCON_DMA3,
}, {
- .name = "dma",
+ .name = "dma.4",
.parent = &clk_h,
.enable = s3c2443_clkcon_enable_h,
.ctrlbit = S3C2443_HCLKCON_DMA4,
}, {
- .name = "dma",
+ .name = "dma.5",
.parent = &clk_h,
.enable = s3c2443_clkcon_enable_h,
.ctrlbit = S3C2443_HCLKCON_DMA5,
--
1.7.2.3

2013-05-11 11:31:33

by Heiko Stübner

[permalink] [raw]
Subject: [RFC 2/4] dma: add dmaengine driver for Samsung s3c24xx SoCs

This adds a new driver to support the s3c24xx dma using the dmaengine
and make the old one in mach-s3c24xx obsolete in the long run.

Conceptually the s3c24xx-dma feels like a distant relative of the pl08x
with numerous virtual channels being mapped to a lot less physical ones.
The driver therefore borrows a lot from the amba-pl08x driver in this
regard. Functionality-wise the driver gains a memcpy ability in addition
to the slave_sg one.

The driver currently only supports the "newer" SoCs which can use any
physical channel for any dma slave. Support for the older SoCs where
each channel only supports a subset of possible dma slaves will have to
be added later.

Tested on a s3c2416-based board, memcpy using the dmatest module and
slave_sg partially using the spi-s3c64xx driver.

Signed-off-by: Heiko Stuebner <[email protected]>
---
drivers/dma/Kconfig | 12 +-
drivers/dma/Makefile | 1 +
drivers/dma/s3c24xx-dma.c | 1129 +++++++++++++++++++++++++++++
include/linux/platform_data/dma-s3c24xx.h | 54 ++
4 files changed, 1195 insertions(+), 1 deletions(-)
create mode 100644 drivers/dma/s3c24xx-dma.c
create mode 100644 include/linux/platform_data/dma-s3c24xx.h

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index aeaea32..cf422ae 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -174,7 +174,17 @@ config TEGRA20_APB_DMA
This DMA controller transfers data from memory to peripheral fifo
or vice versa. It does not support memory to memory data transfer.

-
+config S3C24XX_DMAC
+ tristate "Samsung S3C24XX DMA support"
+ depends on ARCH_S3C24XX && !S3C24XX_DMA
+ select DMA_ENGINE
+ select DMA_VIRTUAL_CHANNELS
+ help
+ Support for the Samsung S3C24XX DMA controller driver. The
+ DMA controller is having multiple DMA channels which can be
+ configured for different peripherals like audio, UART, SPI.
+ The DMA controller can transfer data from memory to peripheral,
+ periphal to memory, periphal to periphal and memory to memory.

config SH_DMAE
tristate "Renesas SuperH DMAC support"
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index 488e3ff..2758969 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_SIRF_DMA) += sirf-dma.o
obj-$(CONFIG_TI_EDMA) += edma.o
obj-$(CONFIG_STE_DMA40) += ste_dma40.o ste_dma40_ll.o
obj-$(CONFIG_TEGRA20_APB_DMA) += tegra20-apb-dma.o
+obj-$(CONFIG_S3C24XX_DMAC) += s3c24xx-dma.o
obj-$(CONFIG_PL330_DMA) += pl330.o
obj-$(CONFIG_PCH_DMA) += pch_dma.o
obj-$(CONFIG_AMBA_PL08X) += amba-pl08x.o
diff --git a/drivers/dma/s3c24xx-dma.c b/drivers/dma/s3c24xx-dma.c
new file mode 100644
index 0000000..2b1edf6
--- /dev/null
+++ b/drivers/dma/s3c24xx-dma.c
@@ -0,0 +1,1129 @@
+/*
+ * S3C24XX DMA handling
+ *
+ * Copyright (c) 2013 Heiko Stuebner <[email protected]>
+ *
+ * based on amba-pl08x.c
+ *
+ * Copyright (c) 2006 ARM Ltd.
+ * Copyright (c) 2010 ST-Ericsson SA
+ *
+ * Author: Peter Pearse <[email protected]>
+ * Author: Linus Walleij <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ *
+ * The DMA controllers in S3C24XX SoCs have a varying number of DMA signals
+ * that can be routed to any of the 4 to 8 hardware-channels.
+ *
+ * Therefore on these DMA controllers the number of channels
+ * and the number of incoming DMA signals are two totally different things.
+ * It is usually not possible to theoretically handle all physical signals,
+ * so a multiplexing scheme with possible denial of use is necessary.
+ *
+ * Open items:
+ * - handle scatterlists with more than one item
+ * - bursts
+ */
+
+#include <linux/platform_device.h>
+#include <linux/types.h>
+#include <linux/dmaengine.h>
+#include <linux/interrupt.h>
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/platform_data/dma-s3c24xx.h>
+
+#include "dmaengine.h"
+#include "virt-dma.h"
+
+#define MAX_DMA_CHANNELS 8
+
+#define DISRC (0x00)
+#define DISRCC (0x04)
+#define DISRCC_INC_INCREMENT (0 << 0)
+#define DISRCC_INC_FIXED (1 << 0)
+#define DISRCC_LOC_AHB (0 << 1)
+#define DISRCC_LOC_APB (1 << 1)
+
+#define DIDST (0x08)
+#define DIDSTC (0x0C)
+#define DIDSTC_INC_INCREMENT (0 << 0)
+#define DIDSTC_INC_FIXED (1 << 0)
+#define DIDSTC_LOC_AHB (0 << 1)
+#define DIDSTC_LOC_APB (1 << 1)
+#define DIDSTC_INT_TC0 (0 << 2)
+#define DIDSTC_INT_RELOAD (1 << 2)
+
+#define DCON (0x10)
+
+#define DCON_TC_MASK 0xfffff
+#define DCON_DSZ_BYTE (0 << 20)
+#define DCON_DSZ_HALFWORD (1 << 20)
+#define DCON_DSZ_WORD (2 << 20)
+#define DCON_DSZ_MASK (3 << 20)
+#define DCON_DSZ_SHIFT 20
+#define DCON_AUTORELOAD (0 << 22)
+#define DCON_NORELOAD (1 << 22)
+#define DCON_HWTRIG (1 << 23)
+#define DCON_SERV_SINGLE (0 << 27)
+#define DCON_SERV_WHOLE (1 << 27)
+#define DCON_TSZ_UNIT (0 << 28)
+#define DCON_TSZ_BURST4 (1 << 28)
+#define DCON_INT (1 << 29)
+#define DCON_SYNC_PCLK (0 << 30)
+#define DCON_SYNC_HCLK (1 << 30)
+#define DCON_DEMAND (0 << 31)
+#define DCON_HANDSHAKE (1 << 31)
+
+#define DSTAT (0x14)
+#define DSTAT_STAT_BUSY (1 << 20)
+#define DSTAT_CURRTC_MASK 0xfffff
+
+#define DMASKTRIG (0x20)
+#define DMASKTRIG_STOP (1 << 2)
+#define DMASKTRIG_ON (1 << 1)
+#define DMASKTRIG_SWTRIG (1 << 0)
+
+#define DMAREQSEL (0x24)
+#define DMAREQSEL_HW (1 << 0)
+
+/*
+ * struct soc_data - vendor-specific config parameters for individual SoCs
+ * @stride: spacing between the registers of each channel
+ * @has_reqsel: does the controller use the newer requestselection mechanism
+ * @has_clocks: are controllable dma-clocks present
+ */
+struct soc_data {
+ int stride;
+ bool has_reqsel;
+ bool has_clocks;
+};
+
+/*
+ * enum s3c24xx_dma_chan_state - holds the virtual channel states
+ * @S3C24XX_DMA_CHAN_IDLE: the channel is idle
+ * @S3C24XX_DMA_CHAN_RUNNING: the channel has allocated a physical transport
+ * channel and is running a transfer on it
+ * @S3C24XX_DMA_CHAN_WAITING: the channel is waiting for a physical transport
+ * channel to become available (only pertains to memcpy channels)
+ */
+enum s3c24xx_dma_chan_state {
+ S3C24XX_DMA_CHAN_IDLE,
+ S3C24XX_DMA_CHAN_RUNNING,
+ S3C24XX_DMA_CHAN_WAITING,
+};
+
+/*
+ * struct s3c24xx_txd - wrapper for struct dma_async_tx_descriptor
+ * @vd: virtual DMA descriptor
+ * @cctl: control reg values for current txd
+ * @ccfg: config reg values for current txd
+ */
+struct s3c24xx_txd {
+ struct virt_dma_desc vd;
+
+ dma_addr_t src_addr;
+ dma_addr_t dst_addr;
+ u8 width;
+ size_t len;
+
+ u32 disrcc;
+ u32 didstc;
+ u32 dcon;
+};
+
+struct s3c24xx_dma_chan;
+
+/*
+ * struct s3c24xx_dma_phy - holder for the physical channels
+ * @id: physical index to this channel
+ * @valid: does the channel have all required elements
+ * @base: virtual memory base (remapped) for the this channel
+ * @irq: interrupt for this channel
+ * @clk: clock for this channel
+ * @lock: a lock to use when altering an instance of this struct
+ * @serving: virtual channel currently being served by this physicalchannel
+ * @host: a pointer to the host (internal use)
+ */
+struct s3c24xx_dma_phy {
+ unsigned int id;
+ bool valid;
+ void __iomem *base;
+ unsigned int irq;
+ struct clk *clk;
+ spinlock_t lock;
+ struct s3c24xx_dma_chan *serving;
+ struct s3c24xx_dma_engine *host;
+};
+
+/*
+ * struct s3c24xx_dma_chan - this structure wraps a DMA ENGINE channel
+ * @id: the id of the channel
+ * @name: name of the channel
+ * @vc: wrappped virtual channel
+ * @phy: the physical channel utilized by this channel, if there is one
+ * @runtime_addr: address for RX/TX according to the runtime config
+ * @at: active transaction on this channel
+ * @lock: a lock for this channel data
+ * @host: a pointer to the host (internal use)
+ * @state: whether the channel is idle, running etc
+ * @slave: whether this channel is a device (slave) or for memcpy
+ */
+struct s3c24xx_dma_chan {
+ int id;
+ const char *name;
+ struct virt_dma_chan vc;
+ struct s3c24xx_dma_phy *phy;
+ struct dma_slave_config cfg;
+ struct s3c24xx_txd *at;
+ struct s3c24xx_dma_engine *host;
+ enum s3c24xx_dma_chan_state state;
+ bool slave;
+};
+
+/*
+ * struct s3c24xx_dma_engine - the local state holder for the S3C24XX
+ * @pdev: the corresponding platform device
+ * @pdata: platform data passed in from the platform/machine
+ * @base: virtual memory base (remapped)
+ * @slave: slave engine for this instance
+ * @memcpy: memcpy engine for this instance
+ * @phy_chans: array of data for the physical channels
+ */
+
+struct s3c24xx_dma_engine {
+ struct platform_device *pdev;
+ const struct s3c24xx_dma_platdata *pdata;
+ struct soc_data *sdata;
+ void __iomem *base;
+ struct dma_device slave;
+ struct dma_device memcpy;
+
+ struct s3c24xx_dma_phy *phy_chans;
+};
+
+/*
+ * Physical channel handling
+ */
+
+/*
+ * Check whether a certain channel is busy or not.
+ */
+static int s3c24xx_dma_phy_busy(struct s3c24xx_dma_phy *phy)
+{
+ unsigned int val = readl(phy->base + DSTAT);
+ return val & DSTAT_STAT_BUSY;
+}
+
+/*
+ * Allocate a physical channel for a virtual channel
+ *
+ * Try to locate a physical channel to be used for this transfer. If all
+ * are taken return NULL and the requester will have to cope by using
+ * some fallback PIO mode or retrying later.
+ */
+static
+struct s3c24xx_dma_phy *s3c24xx_dma_get_phy(struct s3c24xx_dma_chan *s3cchan)
+{
+ struct s3c24xx_dma_engine *s3cdma = s3cchan->host;
+ struct s3c24xx_dma_phy *phy = NULL;
+ unsigned long flags;
+ int i;
+ int ret;
+
+ for (i = 0; i < s3cdma->pdata->num_phy_channels; i++) {
+ phy = &s3cdma->phy_chans[i];
+
+ if (!phy || !phy->valid)
+ continue;
+
+ spin_lock_irqsave(&phy->lock, flags);
+
+ if (!phy->serving) {
+ phy->serving = s3cchan;
+ spin_unlock_irqrestore(&phy->lock, flags);
+ break;
+ }
+
+ spin_unlock_irqrestore(&phy->lock, flags);
+ }
+
+ /* No physical channel available, cope with it */
+ if (i == s3cdma->pdata->num_phy_channels) {
+ dev_warn(&s3cdma->pdev->dev, "no phy channel available\n");
+ return NULL;
+ }
+
+ /* start the phy clock */
+ if (s3cdma->sdata->has_clocks) {
+ ret = clk_enable(phy->clk);
+ if (ret) {
+ phy->serving = NULL;
+ return NULL;
+ }
+ }
+
+ return phy;
+}
+
+/*
+ * Mark the physical channel as free.
+ *
+ * This drops the link between the physical and virtual channel.
+ */
+static inline void s3c24xx_dma_put_phy(struct s3c24xx_dma_phy *phy)
+{
+ struct s3c24xx_dma_engine *s3cdma = phy->host;
+
+ if (s3cdma->sdata->has_clocks)
+ clk_disable(phy->clk);
+ phy->serving = NULL;
+}
+
+/*
+ * Stops the channel by writing the stop bit.
+ * This should not be used for an on-going transfer, but as a method of
+ * shutting down a channel (eg, when it's no longer used) or terminating a
+ * transfer.
+ */
+static void s3c24xx_dma_terminate_phy(struct s3c24xx_dma_phy *phy)
+{
+ writel(DMASKTRIG_STOP, phy->base + DMASKTRIG);
+}
+
+/*
+ * Virtual channel handling
+ */
+
+static inline
+struct s3c24xx_dma_chan *to_s3c24xx_dma_chan(struct dma_chan *chan)
+{
+ return container_of(chan, struct s3c24xx_dma_chan, vc.chan);
+}
+
+static u32 s3c24xx_dma_getbytes_chan(struct s3c24xx_dma_chan *s3cchan)
+{
+ struct s3c24xx_dma_phy *phy = s3cchan->phy;
+ struct s3c24xx_txd *txd = s3cchan->at;
+ u32 tc = readl(phy->base + DSTAT) & DSTAT_CURRTC_MASK;
+
+ switch (txd->dcon & DCON_DSZ_MASK) {
+ case DCON_DSZ_BYTE:
+ return tc;
+ case DCON_DSZ_HALFWORD:
+ return tc * 2;
+ case DCON_DSZ_WORD:
+ return tc * 4;
+ default:
+ break;
+ }
+
+ BUG();
+ return 0;
+}
+
+static int s3c24xx_dma_set_runtime_config(struct s3c24xx_dma_chan *s3cchan,
+ struct dma_slave_config *config)
+{
+ if (!s3cchan->slave)
+ return -EINVAL;
+
+ /* Reject definitely invalid configurations */
+ if (config->src_addr_width == DMA_SLAVE_BUSWIDTH_8_BYTES ||
+ config->dst_addr_width == DMA_SLAVE_BUSWIDTH_8_BYTES)
+ return -EINVAL;
+
+ s3cchan->cfg = *config;
+
+ return 0;
+}
+
+/*
+ * Transfer handling
+ */
+
+static inline
+struct s3c24xx_txd *to_s3c24xx_txd(struct dma_async_tx_descriptor *tx)
+{
+ return container_of(tx, struct s3c24xx_txd, vd.tx);
+}
+
+/*
+ * Set the initial DMA register values.
+ * Put them into the hardware and start the transfer.
+ */
+static void s3c24xx_dma_start_next_txd(struct s3c24xx_dma_chan *s3cchan)
+{
+ struct s3c24xx_dma_engine *s3cdma = s3cchan->host;
+ struct s3c24xx_dma_phy *phy = s3cchan->phy;
+ struct virt_dma_desc *vd = vchan_next_desc(&s3cchan->vc);
+ struct s3c24xx_txd *txd = to_s3c24xx_txd(&vd->tx);
+ u32 dcon = txd->dcon;
+ u32 val;
+
+ list_del(&txd->vd.node);
+
+ s3cchan->at = txd;
+
+ /* Wait for channel inactive */
+ while (s3c24xx_dma_phy_busy(phy))
+ cpu_relax();
+
+ /* transfer-size and -count from len and width */
+ switch (txd->width) {
+ case 1:
+ dcon |= DCON_DSZ_BYTE | txd->len;
+ break;
+ case 2:
+ dcon |= DCON_DSZ_HALFWORD | (txd->len / 2);
+ break;
+ case 4:
+ dcon |= DCON_DSZ_WORD | (txd->len / 4);
+ break;
+ }
+
+ if (s3cchan->slave) {
+ if (s3cdma->sdata->has_reqsel) {
+ int reqsel = s3cdma->pdata->reqsel_map[s3cchan->id];
+ writel((reqsel << 1) | DMAREQSEL_HW,
+ phy->base + DMAREQSEL);
+ } else {
+ dev_err(&s3cdma->pdev->dev, "non-reqsel unsupported\n");
+ return;
+ dcon |= DCON_HWTRIG;
+ }
+ } else {
+ if (s3cdma->sdata->has_reqsel) {
+ writel(0, phy->base + DMAREQSEL);
+ } else {
+ dev_err(&s3cdma->pdev->dev, "non-reqsel unsupported\n");
+ return;
+ }
+ }
+
+ writel(txd->src_addr, phy->base + DISRC);
+ writel(txd->disrcc, phy->base + DISRCC);
+ writel(txd->dst_addr, phy->base + DIDST);
+ writel(txd->didstc, phy->base + DIDSTC);
+
+ writel(dcon, phy->base + DCON);
+
+ val = readl(phy->base + DMASKTRIG);
+ val &= ~DMASKTRIG_STOP;
+ val |= DMASKTRIG_ON;
+ writel(val, phy->base + DMASKTRIG);
+
+ /* trigger the dma operation for memcpy transfers */
+ if (!s3cchan->slave) {
+ val = readl(phy->base + DMASKTRIG);
+ val |= DMASKTRIG_SWTRIG;
+ writel(val, phy->base + DMASKTRIG);
+ }
+}
+
+static void s3c24xx_dma_free_txd_list(struct s3c24xx_dma_engine *s3cdma,
+ struct s3c24xx_dma_chan *s3cchan)
+{
+ LIST_HEAD(head);
+
+ vchan_get_all_descriptors(&s3cchan->vc, &head);
+ vchan_dma_desc_free_list(&s3cchan->vc, &head);
+}
+
+/*
+ * Try to allocate a physical channel. When successful, assign it to
+ * this virtual channel, and initiate the next descriptor. The
+ * virtual channel lock must be held at this point.
+ */
+static void s3c24xx_dma_phy_alloc_and_start(struct s3c24xx_dma_chan *s3cchan)
+{
+ struct s3c24xx_dma_engine *s3cdma = s3cchan->host;
+ struct s3c24xx_dma_phy *phy;
+
+ phy = s3c24xx_dma_get_phy(s3cchan);
+ if (!phy) {
+ dev_dbg(&s3cdma->pdev->dev, "no physical channel available for xfer on %s\n",
+ s3cchan->name);
+ s3cchan->state = S3C24XX_DMA_CHAN_WAITING;
+ return;
+ }
+
+ dev_dbg(&s3cdma->pdev->dev, "allocated physical channel %d for xfer on %s\n",
+ phy->id, s3cchan->name);
+
+ s3cchan->phy = phy;
+ s3cchan->state = S3C24XX_DMA_CHAN_RUNNING;
+
+ s3c24xx_dma_start_next_txd(s3cchan);
+}
+
+static void s3c24xx_dma_phy_reassign_start(struct s3c24xx_dma_phy *phy,
+ struct s3c24xx_dma_chan *s3cchan)
+{
+ struct s3c24xx_dma_engine *s3cdma = s3cchan->host;
+
+ dev_dbg(&s3cdma->pdev->dev, "reassigned physical channel %d for xfer on %s\n",
+ phy->id, s3cchan->name);
+
+ /*
+ * We do this without taking the lock; we're really only concerned
+ * about whether this pointer is NULL or not, and we're guaranteed
+ * that this will only be called when it _already_ is non-NULL.
+ */
+ phy->serving = s3cchan;
+ s3cchan->phy = phy;
+ s3cchan->state = S3C24XX_DMA_CHAN_RUNNING;
+ s3c24xx_dma_start_next_txd(s3cchan);
+}
+
+/*
+ * Free a physical DMA channel, potentially reallocating it to another
+ * virtual channel if we have any pending.
+ */
+static void s3c24xx_dma_phy_free(struct s3c24xx_dma_chan *s3cchan)
+{
+ struct s3c24xx_dma_engine *s3cdma = s3cchan->host;
+ struct s3c24xx_dma_chan *p, *next;
+
+retry:
+ next = NULL;
+
+ /* Find a waiting virtual channel for the next transfer. */
+ list_for_each_entry(p, &s3cdma->memcpy.channels, vc.chan.device_node)
+ if (p->state == S3C24XX_DMA_CHAN_WAITING) {
+ next = p;
+ break;
+ }
+
+ if (!next) {
+ list_for_each_entry(p, &s3cdma->slave.channels,
+ vc.chan.device_node)
+ if (p->state == S3C24XX_DMA_CHAN_WAITING) {
+ next = p;
+ break;
+ }
+ }
+
+ /* Ensure that the physical channel is stopped */
+ s3c24xx_dma_terminate_phy(s3cchan->phy);
+
+ if (next) {
+ bool success;
+
+ /*
+ * Eww. We know this isn't going to deadlock
+ * but lockdep probably doesn't.
+ */
+ spin_lock(&next->vc.lock);
+ /* Re-check the state now that we have the lock */
+ success = next->state == S3C24XX_DMA_CHAN_WAITING;
+ if (success)
+ s3c24xx_dma_phy_reassign_start(s3cchan->phy, next);
+ spin_unlock(&next->vc.lock);
+
+ /* If the state changed, try to find another channel */
+ if (!success)
+ goto retry;
+ } else {
+ /* No more jobs, so free up the physical channel */
+ s3c24xx_dma_put_phy(s3cchan->phy);
+ }
+
+ s3cchan->phy = NULL;
+ s3cchan->state = S3C24XX_DMA_CHAN_IDLE;
+}
+
+static void s3c24xx_dma_desc_free(struct virt_dma_desc *vd)
+{
+ struct s3c24xx_txd *txd = to_s3c24xx_txd(&vd->tx);
+ kfree(txd);
+}
+
+static irqreturn_t s3c24xx_dma_irq(int irq, void *data)
+{
+ struct s3c24xx_dma_phy *phy = data;
+ struct s3c24xx_dma_chan *s3cchan = phy->serving;
+ struct s3c24xx_txd *txd;
+
+ dev_dbg(&phy->host->pdev->dev, "interrupt on channel %d\n", phy->id);
+
+ if (!s3cchan) {
+ dev_err(&phy->host->pdev->dev, "interrupt on unused channel %d\n",
+ phy->id);
+ return IRQ_NONE;
+ }
+
+ spin_lock(&s3cchan->vc.lock);
+ txd = s3cchan->at;
+ if (txd) {
+ s3cchan->at = NULL;
+ vchan_cookie_complete(&txd->vd);
+
+ /*
+ * And start the next descriptor (if any),
+ * otherwise free this channel.
+ */
+ if (vchan_next_desc(&s3cchan->vc))
+ s3c24xx_dma_start_next_txd(s3cchan);
+ else
+ s3c24xx_dma_phy_free(s3cchan);
+ }
+ spin_unlock(&s3cchan->vc.lock);
+
+ return IRQ_HANDLED;
+}
+
+/*
+ * The DMA ENGINE API
+ */
+
+static int s3c24xx_dma_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
+ unsigned long arg)
+{
+ struct s3c24xx_dma_chan *s3cchan = to_s3c24xx_dma_chan(chan);
+ struct s3c24xx_dma_engine *s3cdma = s3cchan->host;
+ unsigned long flags;
+ int ret = 0;
+
+ /* Controls applicable to inactive channels */
+ if (cmd == DMA_SLAVE_CONFIG)
+ return s3c24xx_dma_set_runtime_config(s3cchan,
+ (struct dma_slave_config *)arg);
+
+ /*
+ * Anything succeeds on channels with no physical allocation and
+ * no queued transfers.
+ */
+ spin_lock_irqsave(&s3cchan->vc.lock, flags);
+ if (!s3cchan->phy && !s3cchan->at) {
+ spin_unlock_irqrestore(&s3cchan->vc.lock, flags);
+ return 0;
+ }
+
+ switch (cmd) {
+ case DMA_TERMINATE_ALL:
+ s3cchan->state = S3C24XX_DMA_CHAN_IDLE;
+
+ /* Mark physical channel as free */
+ if (s3cchan->phy)
+ s3c24xx_dma_phy_free(s3cchan);
+
+ /* Dequeue current job */
+ if (s3cchan->at) {
+ s3c24xx_dma_desc_free(&s3cchan->at->vd);
+ s3cchan->at = NULL;
+ }
+
+ /* Dequeue jobs not yet fired as well */
+ s3c24xx_dma_free_txd_list(s3cdma, s3cchan);
+ break;
+ default:
+ /* Unknown command */
+ ret = -ENXIO;
+ break;
+ }
+
+ spin_unlock_irqrestore(&s3cchan->vc.lock, flags);
+
+ return ret;
+}
+
+static int s3c24xx_dma_alloc_chan_resources(struct dma_chan *chan)
+{
+ return 0;
+}
+
+static void s3c24xx_dma_free_chan_resources(struct dma_chan *chan)
+{
+ /* Ensure all queued descriptors are freed */
+ vchan_free_chan_resources(to_virt_chan(chan));
+}
+
+static enum dma_status s3c24xx_dma_tx_status(struct dma_chan *chan,
+ dma_cookie_t cookie, struct dma_tx_state *txstate)
+{
+ struct s3c24xx_dma_chan *s3cchan = to_s3c24xx_dma_chan(chan);
+ struct virt_dma_desc *vd;
+ unsigned long flags;
+ enum dma_status ret;
+ size_t bytes = 0;
+
+ ret = dma_cookie_status(chan, cookie, txstate);
+ if (ret == DMA_SUCCESS)
+ return ret;
+
+ /*
+ * There's no point calculating the residue if there's
+ * no txstate to store the value.
+ */
+ if (!txstate)
+ return ret;
+
+ spin_lock_irqsave(&s3cchan->vc.lock, flags);
+ ret = dma_cookie_status(chan, cookie, txstate);
+ if (ret != DMA_SUCCESS) {
+ vd = vchan_find_desc(&s3cchan->vc, cookie);
+ if (vd) {
+ /* On the issued list, so hasn't been processed yet */
+ struct s3c24xx_txd *txd = to_s3c24xx_txd(&vd->tx);
+ bytes = txd->len;
+ } else {
+ bytes = s3c24xx_dma_getbytes_chan(s3cchan);
+ }
+ }
+ spin_unlock_irqrestore(&s3cchan->vc.lock, flags);
+
+ /*
+ * This cookie not complete yet
+ * Get number of bytes left in the active transactions and queue
+ */
+ dma_set_residue(txstate, bytes);
+
+ /* Whether waiting or running, we're in progress */
+ return ret;
+}
+
+/*
+ * Initialize a descriptor to be used by memcpy submit
+ */
+static struct dma_async_tx_descriptor *s3c24xx_dma_prep_memcpy(
+ struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
+ size_t len, unsigned long flags)
+{
+ struct s3c24xx_dma_chan *s3cchan = to_s3c24xx_dma_chan(chan);
+ struct s3c24xx_dma_engine *s3cdma = s3cchan->host;
+ struct s3c24xx_txd *txd;
+
+ dev_dbg(&s3cdma->pdev->dev, "prepare memcpy of %d bytes from %s\n",
+ len, s3cchan->name);
+
+ if ((len & DCON_TC_MASK) != len) {
+ dev_err(&s3cdma->pdev->dev, "memcpy size %d to large\n", len);
+ return NULL;
+ }
+
+ txd = kzalloc(sizeof(*txd), GFP_NOWAIT);
+ if (!txd) {
+ dev_err(&s3cdma->pdev->dev, "no memory for descriptor\n");
+ return NULL;
+ }
+
+ txd->src_addr = src;
+ txd->dst_addr = dest;
+ txd->width = 1;
+ txd->len = len;
+
+ txd->disrcc = DISRCC_LOC_AHB | DISRCC_INC_INCREMENT;
+ txd->didstc = DIDSTC_LOC_AHB | DIDSTC_INC_INCREMENT;
+ txd->dcon = DCON_DEMAND | DCON_SYNC_HCLK | DCON_INT | DCON_SERV_WHOLE |
+ DCON_NORELOAD;
+
+ return vchan_tx_prep(&s3cchan->vc, &txd->vd, flags);
+}
+
+static struct dma_async_tx_descriptor *s3c24xx_dma_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 s3c24xx_dma_chan *s3cchan = to_s3c24xx_dma_chan(chan);
+ struct s3c24xx_dma_engine *s3cdma = s3cchan->host;
+ struct s3c24xx_txd *txd;
+ struct scatterlist *sg;
+ dma_addr_t slave_addr;
+ u32 hwcfg = 0;
+ int tmp;
+
+ dev_dbg(&s3cdma->pdev->dev, "prepare transaction of %d bytes from %s\n",
+ sg_dma_len(sgl), s3cchan->name);
+
+ /* FIXME: temporarily, until we can transfer more than one element */
+ if (sg_nents(sgl) > 1) {
+ dev_err(&s3cdma->pdev->dev, "currently only scatterlists of size 1 supported\n");
+ BUG();
+ return NULL;
+ }
+
+ txd = kzalloc(sizeof(*txd), GFP_NOWAIT);
+ if (!txd) {
+ dev_err(&s3cdma->pdev->dev, "no memory for descriptor\n");
+ return NULL;
+ }
+
+ switch (s3cchan->id) {
+ case S3C24XX_DMACH_XD0:
+ case S3C24XX_DMACH_XD1:
+ txd->dcon |= DCON_HANDSHAKE | DCON_SYNC_HCLK;
+ hwcfg |= DISRCC_LOC_AHB;
+ break;
+ case S3C24XX_DMACH_SDI:
+ /* note, ensure if need HANDSHAKE or not */
+ txd->dcon |= DCON_SYNC_PCLK;
+ hwcfg |= DISRCC_LOC_APB;
+ break;
+
+ default:
+ txd->dcon |= DCON_HANDSHAKE | DCON_SYNC_PCLK;
+ hwcfg |= DISRCC_LOC_APB;
+ }
+
+ /* always assume our peripheral desintation is a fixed
+ * address in memory. */
+ hwcfg |= DISRCC_INC_FIXED;
+
+ /* individual dma operations are requested by the slave,
+ * so serve only single atomic operations (DCON_SERV_SINGLE).
+ */
+ txd->dcon |= DCON_INT | DCON_SERV_SINGLE | DCON_NORELOAD;
+
+ if (direction == DMA_MEM_TO_DEV) {
+ txd->disrcc = DISRCC_LOC_AHB | DISRCC_INC_INCREMENT;
+ txd->didstc = hwcfg;
+ slave_addr = s3cchan->cfg.dst_addr;
+ txd->width = s3cchan->cfg.dst_addr_width;
+ } else if (direction == DMA_DEV_TO_MEM) {
+ txd->disrcc = hwcfg;
+ txd->didstc = DIDSTC_LOC_AHB | DIDSTC_INC_INCREMENT;
+ slave_addr = s3cchan->cfg.src_addr;
+ txd->width = s3cchan->cfg.src_addr_width;
+ } else {
+ kfree(txd);
+ dev_err(&s3cdma->pdev->dev,
+ "direction %d unsupported\n", direction);
+ return NULL;
+ }
+
+ for_each_sg(sgl, sg, sg_len, tmp) {
+ txd->len = sg_dma_len(sg);
+ if (direction == DMA_MEM_TO_DEV) {
+ txd->src_addr = sg_dma_address(sg);
+ txd->dst_addr = slave_addr;
+ } else { /* DMA_DEV_TO_MEM */
+ txd->src_addr = slave_addr;
+ txd->dst_addr = sg_dma_address(sg);
+ }
+ break;
+ }
+
+ return vchan_tx_prep(&s3cchan->vc, &txd->vd, flags);
+}
+
+/*
+ * Slave transactions callback to the slave device to allow
+ * synchronization of slave DMA signals with the DMAC enable
+ */
+static void s3c24xx_dma_issue_pending(struct dma_chan *chan)
+{
+ struct s3c24xx_dma_chan *s3cchan = to_s3c24xx_dma_chan(chan);
+ unsigned long flags;
+
+ spin_lock_irqsave(&s3cchan->vc.lock, flags);
+ if (vchan_issue_pending(&s3cchan->vc)) {
+ if (!s3cchan->phy && s3cchan->state != S3C24XX_DMA_CHAN_WAITING)
+ s3c24xx_dma_phy_alloc_and_start(s3cchan);
+ }
+ spin_unlock_irqrestore(&s3cchan->vc.lock, flags);
+}
+
+/*
+ * Bringup and teardown
+ */
+
+/*
+ * Initialise the DMAC memcpy/slave channels.
+ * Make a local wrapper to hold required data
+ */
+static int s3c24xx_dma_init_virtual_channels(struct s3c24xx_dma_engine *s3cdma,
+ struct dma_device *dmadev, unsigned int channels, bool slave)
+{
+ struct s3c24xx_dma_chan *chan;
+ int i;
+
+ INIT_LIST_HEAD(&dmadev->channels);
+
+ /*
+ * Register as many many memcpy as we have physical channels,
+ * we won't always be able to use all but the code will have
+ * to cope with that situation.
+ */
+ for (i = 0; i < channels; i++) {
+ chan = devm_kzalloc(dmadev->dev, sizeof(*chan), GFP_KERNEL);
+ if (!chan) {
+ dev_err(dmadev->dev,
+ "%s no memory for channel\n", __func__);
+ return -ENOMEM;
+ }
+
+ chan->id = i;
+ chan->host = s3cdma;
+ chan->state = S3C24XX_DMA_CHAN_IDLE;
+
+ if (slave) {
+ chan->slave = true;
+ chan->name = kasprintf(GFP_KERNEL, "slave%d", i);
+ if (!chan->name)
+ return -ENOMEM;
+ } else {
+ chan->name = kasprintf(GFP_KERNEL, "memcpy%d", i);
+ if (!chan->name)
+ return -ENOMEM;
+ }
+ dev_dbg(dmadev->dev,
+ "initialize virtual channel \"%s\"\n",
+ chan->name);
+
+ chan->vc.desc_free = s3c24xx_dma_desc_free;
+ vchan_init(&chan->vc, dmadev);
+ }
+ dev_info(dmadev->dev, "initialized %d virtual %s channels\n",
+ i, slave ? "slave" : "memcpy");
+ return i;
+}
+
+static void s3c24xx_dma_free_virtual_channels(struct dma_device *dmadev)
+{
+ struct s3c24xx_dma_chan *chan = NULL;
+ struct s3c24xx_dma_chan *next;
+
+ list_for_each_entry_safe(chan,
+ next, &dmadev->channels, vc.chan.device_node)
+ list_del(&chan->vc.chan.device_node);
+}
+
+/* s3c2412 and s3c2413 have a 0x40 stride and dmareqsel mechanism */
+static struct soc_data soc_s3c2412 = {
+ .stride = 0x40,
+ .has_reqsel = true,
+ .has_clocks = true,
+};
+
+/* s3c2443 and following have a 0x100 stride and dmareqsel mechanism */
+static struct soc_data soc_s3c2443 = {
+ .stride = 0x100,
+ .has_reqsel = true,
+ .has_clocks = true,
+};
+
+static struct platform_device_id s3c24xx_dma_driver_ids[] = {
+ {
+ .name = "s3c2412-dma",
+ .driver_data = (kernel_ulong_t)&soc_s3c2412,
+ }, {
+ .name = "s3c2443-dma",
+ .driver_data = (kernel_ulong_t)&soc_s3c2443,
+ },
+ { },
+};
+
+static struct soc_data *s3c24xx_dma_get_soc_data(struct platform_device *pdev)
+{
+ return (struct soc_data *)
+ platform_get_device_id(pdev)->driver_data;
+}
+
+static int s3c24xx_dma_probe(struct platform_device *pdev)
+{
+ const struct s3c24xx_dma_platdata *pdata = dev_get_platdata(&pdev->dev);
+ struct soc_data *sdata;
+ struct s3c24xx_dma_engine *s3cdma;
+ struct resource *res;
+ int ret;
+ int i;
+
+ if (!pdata) {
+ pdata = s3c24xx_dma_get_of_pdata(&pdev->dev);
+ if (IS_ERR(pdata))
+ return PTR_ERR(pdata);
+ }
+
+ /* Basic sanity check */
+ if (pdata->num_phy_channels > MAX_DMA_CHANNELS) {
+ dev_err(&pdev->dev, "to many dma channels %d, max %d\n",
+ pdata->num_phy_channels, MAX_DMA_CHANNELS);
+ return -EINVAL;
+ }
+
+ sdata = s3c24xx_dma_get_soc_data(pdev);
+ if (!sdata)
+ return -EINVAL;
+
+ s3cdma = devm_kzalloc(&pdev->dev, sizeof(*s3cdma), GFP_KERNEL);
+ if (!s3cdma)
+ return -ENOMEM;
+
+ s3cdma->pdev = pdev;
+ s3cdma->pdata = pdata;
+ s3cdma->sdata = sdata;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ s3cdma->base = devm_request_and_ioremap(&pdev->dev, res);
+ if (!s3cdma->base) {
+ dev_err(&pdev->dev, "could not map dma registers\n");
+ return -EBUSY;
+ }
+
+ s3cdma->phy_chans = devm_kzalloc(&pdev->dev,
+ sizeof(struct s3c24xx_dma_phy) *
+ pdata->num_phy_channels,
+ GFP_KERNEL);
+ if (!s3cdma->phy_chans)
+ return -ENOMEM;
+
+ /* aquire irqs and clocks for all physical channels */
+ for (i = 0; i < pdata->num_phy_channels; i++) {
+ struct s3c24xx_dma_phy *phy = &s3cdma->phy_chans[i];
+ char clk_name[6];
+
+ phy->id = i;
+ phy->base = s3cdma->base + (i * sdata->stride);
+ phy->host = s3cdma;
+
+ sprintf(clk_name, "dma.%d", i);
+ phy->clk = devm_clk_get(&pdev->dev, clk_name);
+ if (IS_ERR(phy->clk) && sdata->has_clocks) {
+ dev_err(&pdev->dev, "unable to aquire clock for channel %d, error %lu",
+ i, PTR_ERR(phy->clk));
+ continue;
+ }
+
+ if (sdata->has_clocks) {
+ ret = clk_prepare(phy->clk);
+ if (ret) {
+ dev_err(&pdev->dev, "clock for phy %d failed, error %d\n",
+ i, ret);
+ continue;
+ }
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_IRQ, i);
+ if (res)
+ phy->irq = res->start;
+ if (!phy->irq) {
+ dev_err(&pdev->dev, "failed to get irq %d\n", i);
+ continue;
+ }
+
+ ret = devm_request_irq(&pdev->dev, phy->irq, s3c24xx_dma_irq,
+ 0, pdev->name, phy);
+ if (ret) {
+ dev_err(&pdev->dev, "Unable to request irq for channel %d, error %d\n",
+ i, ret);
+ continue;
+ }
+
+ spin_lock_init(&phy->lock);
+ phy->valid = true;
+
+ dev_dbg(&pdev->dev, "physical channel %d is %s\n",
+ i, s3c24xx_dma_phy_busy(phy) ? "BUSY" : "FREE");
+ }
+
+ /* Initialize memcpy engine */
+ dma_cap_set(DMA_MEMCPY, s3cdma->memcpy.cap_mask);
+ dma_cap_set(DMA_PRIVATE, s3cdma->memcpy.cap_mask);
+ s3cdma->memcpy.dev = &pdev->dev;
+ s3cdma->memcpy.device_alloc_chan_resources =
+ s3c24xx_dma_alloc_chan_resources;
+ s3cdma->memcpy.device_free_chan_resources =
+ s3c24xx_dma_free_chan_resources;
+ s3cdma->memcpy.device_prep_dma_memcpy = s3c24xx_dma_prep_memcpy;
+ s3cdma->memcpy.device_tx_status = s3c24xx_dma_tx_status;
+ s3cdma->memcpy.device_issue_pending = s3c24xx_dma_issue_pending;
+ s3cdma->memcpy.device_control = s3c24xx_dma_control;
+
+ /* Initialize slave engine for SoC internal dedicated periphals */
+ dma_cap_set(DMA_SLAVE, s3cdma->slave.cap_mask);
+ dma_cap_set(DMA_PRIVATE, s3cdma->slave.cap_mask);
+ s3cdma->slave.dev = &pdev->dev;
+ s3cdma->slave.device_alloc_chan_resources =
+ s3c24xx_dma_alloc_chan_resources;
+ s3cdma->slave.device_free_chan_resources =
+ s3c24xx_dma_free_chan_resources;
+ s3cdma->slave.device_tx_status = s3c24xx_dma_tx_status;
+ s3cdma->slave.device_issue_pending = s3c24xx_dma_issue_pending;
+ s3cdma->slave.device_prep_slave_sg = s3c24xx_dma_prep_slave_sg;
+ s3cdma->slave.device_control = s3c24xx_dma_control;
+
+ /* Register as many memcpy channels as there are physical channels */
+ ret = s3c24xx_dma_init_virtual_channels(s3cdma, &s3cdma->memcpy,
+ pdata->num_phy_channels, false);
+ if (ret <= 0) {
+ dev_warn(&pdev->dev,
+ "%s failed to enumerate memcpy channels - %d\n",
+ __func__, ret);
+ goto err_memcpy;
+ }
+
+ /* Register slave channels */
+ ret = s3c24xx_dma_init_virtual_channels(s3cdma, &s3cdma->slave,
+ S3C24XX_DMACH_MAX, true);
+ if (ret <= 0) {
+ dev_warn(&pdev->dev,
+ "%s failed to enumerate slave channels - %d\n",
+ __func__, ret);
+ goto err_slave;
+ }
+
+ ret = dma_async_device_register(&s3cdma->memcpy);
+ if (ret) {
+ dev_warn(&pdev->dev,
+ "%s failed to register memcpy as an async device - %d\n",
+ __func__, ret);
+ goto err_memcpy_reg;
+ }
+
+ ret = dma_async_device_register(&s3cdma->slave);
+ if (ret) {
+ dev_warn(&pdev->dev,
+ "%s failed to register slave as an async device - %d\n",
+ __func__, ret);
+ goto err_slave_reg;
+ }
+
+ platform_set_drvdata(pdev, s3cdma);
+ dev_info(&pdev->dev, "Loaded dma driver with %d physical channels\n",
+ pdata->num_phy_channels);
+
+ return 0;
+
+err_slave_reg:
+ dma_async_device_unregister(&s3cdma->memcpy);
+err_memcpy_reg:
+ s3c24xx_dma_free_virtual_channels(&s3cdma->slave);
+err_slave:
+ s3c24xx_dma_free_virtual_channels(&s3cdma->memcpy);
+err_memcpy:
+
+ return ret;
+}
+
+static struct platform_driver s3c24xx_dma_driver = {
+ .driver = {
+ .name = "s3c24xx-dma",
+ },
+ .id_table = s3c24xx_dma_driver_ids,
+};
+
+bool s3c24xx_dma_filter(struct dma_chan *chan, void *param)
+{
+ struct s3c24xx_dma_chan *s3cchan;
+
+ if (chan->device->dev->driver != &s3c24xx_dma_driver.driver)
+ return false;
+
+ s3cchan = to_s3c24xx_dma_chan(chan);
+
+ return s3cchan->id == (int)param;
+}
+EXPORT_SYMBOL(s3c24xx_dma_filter);
+
+static int __init s3c24xx_dma_module_init(void)
+{
+ return platform_driver_probe(&s3c24xx_dma_driver, s3c24xx_dma_probe);
+}
+subsys_initcall(s3c24xx_dma_module_init);
diff --git a/include/linux/platform_data/dma-s3c24xx.h b/include/linux/platform_data/dma-s3c24xx.h
new file mode 100644
index 0000000..72f9c7f
--- /dev/null
+++ b/include/linux/platform_data/dma-s3c24xx.h
@@ -0,0 +1,54 @@
+/*
+ * S3C24XX DMA handling
+ *
+ * Copyright (c) 2013 Heiko Stuebner <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+enum s3c24xx_dma_requests {
+ S3C24XX_DMACH_XD0 = 0,
+ S3C24XX_DMACH_XD1,
+ S3C24XX_DMACH_SDI,
+ S3C24XX_DMACH_SPI0,
+ S3C24XX_DMACH_SPI1,
+ S3C24XX_DMACH_UART0,
+ S3C24XX_DMACH_UART1,
+ S3C24XX_DMACH_UART2,
+ S3C24XX_DMACH_TIMER,
+ S3C24XX_DMACH_I2S_RX,
+ S3C24XX_DMACH_I2S_TX,
+ S3C24XX_DMACH_PCM_IN,
+ S3C24XX_DMACH_PCM_OUT,
+ S3C24XX_DMACH_MIC_IN,
+ S3C24XX_DMACH_USB_EP1,
+ S3C24XX_DMACH_USB_EP2,
+ S3C24XX_DMACH_USB_EP3,
+ S3C24XX_DMACH_USB_EP4,
+ S3C24XX_DMACH_UART0_SRC2,
+ S3C24XX_DMACH_UART1_SRC2,
+ S3C24XX_DMACH_UART2_SRC2,
+ S3C24XX_DMACH_UART3,
+ S3C24XX_DMACH_UART3_SRC2,
+ S3C24XX_DMACH_SPI0_TX,
+ S3C24XX_DMACH_SPI0_RX,
+ S3C24XX_DMACH_SPI1_TX,
+ S3C24XX_DMACH_SPI1_RX,
+ S3C24XX_DMACH_MAX,
+};
+
+/**
+ * struct s3c24xx_dma_platdata - platform specific settings
+ * @num_phy_channels: number of physical channels
+ * @reqsel_map: map the virtual channels to dma request sources
+ */
+struct s3c24xx_dma_platdata {
+ int num_phy_channels;
+ int *reqsel_map;
+};
+
+struct dma_chan;
+bool s3c24xx_dma_filter(struct dma_chan *chan, void *param);
--
1.7.2.3

2013-05-11 11:32:14

by Heiko Stübner

[permalink] [raw]
Subject: [RFC 3/4] ARM: S3C24XX: add platform-devices for new dma driver for s3c2412 and s3c2443

This includes defining the mapping for the request sources.

Signed-off-by: Heiko Stuebner <[email protected]>
---
arch/arm/mach-s3c24xx/common.c | 103 ++++++++++++++++++++++++++++++++++++++++
arch/arm/mach-s3c24xx/common.h | 3 +
2 files changed, 106 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-s3c24xx/common.c b/arch/arm/mach-s3c24xx/common.c
index c157103..9f64037 100644
--- a/arch/arm/mach-s3c24xx/common.c
+++ b/arch/arm/mach-s3c24xx/common.c
@@ -30,6 +30,7 @@
#include <linux/platform_device.h>
#include <linux/delay.h>
#include <linux/io.h>
+#include <linux/platform_data/dma-s3c24xx.h>

#include <mach/hardware.h>
#include <mach/regs-clock.h>
@@ -302,3 +303,105 @@ void __init_or_cpufreq s3c24xx_setup_clocks(unsigned long fclk,
clk_p.rate = pclk;
clk_f.rate = fclk;
}
+
+#if defined(CONFIG_CPU_S3C2410) || defined(CONFIG_CPU_S3C2412) || \
+ defined(CONFIG_CPUS_3C2440) || defined(CONFIG_CPUS_3C2442)
+static struct resource s3c2410_dma_resource[] = {
+ [0] = DEFINE_RES_MEM(S3C24XX_PA_DMA, S3C24XX_SZ_DMA),
+ [1] = DEFINE_RES_IRQ(IRQ_DMA0),
+ [2] = DEFINE_RES_IRQ(IRQ_DMA1),
+ [3] = DEFINE_RES_IRQ(IRQ_DMA2),
+ [4] = DEFINE_RES_IRQ(IRQ_DMA3),
+};
+#endif
+
+#ifdef CONFIG_CPU_S3C2412
+static int s3c2412_dma_reqsel[S3C24XX_DMACH_MAX] = {
+ [S3C24XX_DMACH_XD0] = 17,
+ [S3C24XX_DMACH_XD1] = 18,
+ [S3C24XX_DMACH_SDI] = 10,
+ [S3C24XX_DMACH_SPI0_RX] = 1,
+ [S3C24XX_DMACH_SPI0_TX] = 0,
+ [S3C24XX_DMACH_SPI1_RX] = 3,
+ [S3C24XX_DMACH_SPI1_TX] = 2,
+ [S3C24XX_DMACH_UART0] = 19,
+ [S3C24XX_DMACH_UART1] = 21,
+ [S3C24XX_DMACH_UART2] = 23,
+ [S3C24XX_DMACH_UART0_SRC2] = 20,
+ [S3C24XX_DMACH_UART1_SRC2] = 22,
+ [S3C24XX_DMACH_UART2_SRC2] = 24,
+ [S3C24XX_DMACH_TIMER] = 9,
+ [S3C24XX_DMACH_I2S_RX] = 5,
+ [S3C24XX_DMACH_I2S_TX] = 4,
+ [S3C24XX_DMACH_USB_EP1] = 13,
+ [S3C24XX_DMACH_USB_EP2] = 14,
+ [S3C24XX_DMACH_USB_EP3] = 15,
+ [S3C24XX_DMACH_USB_EP4] = 16,
+};
+
+static struct s3c24xx_dma_platdata s3c2412_dma_platdata = {
+ .num_phy_channels = 4,
+ .reqsel_map = s3c2412_dma_reqsel,
+};
+
+struct platform_device s3c2412_device_dma = {
+ .name = "s3c2412-dma",
+ .id = 0,
+ .num_resources = ARRAY_SIZE(s3c2410_dma_resource),
+ .resource = s3c2410_dma_resource,
+ .dev = {
+ .platform_data = &s3c2412_dma_platdata,
+ },
+};
+#endif
+
+#if defined(CONFIG_CPUS_3C2443) || defined(CONFIG_CPU_S3C2416)
+static struct resource s3c2443_dma_resource[] = {
+ [0] = DEFINE_RES_MEM(S3C24XX_PA_DMA, S3C24XX_SZ_DMA),
+ [1] = DEFINE_RES_IRQ(IRQ_S3C2443_DMA0),
+ [2] = DEFINE_RES_IRQ(IRQ_S3C2443_DMA1),
+ [3] = DEFINE_RES_IRQ(IRQ_S3C2443_DMA2),
+ [4] = DEFINE_RES_IRQ(IRQ_S3C2443_DMA3),
+ [5] = DEFINE_RES_IRQ(IRQ_S3C2443_DMA4),
+ [6] = DEFINE_RES_IRQ(IRQ_S3C2443_DMA5),
+};
+
+static int s3c2443_dma_reqsel[S3C24XX_DMACH_MAX] = {
+ [S3C24XX_DMACH_XD0] = 17,
+ [S3C24XX_DMACH_XD1] = 18,
+ [S3C24XX_DMACH_SDI] = 10,
+ [S3C24XX_DMACH_SPI0_RX] = 1,
+ [S3C24XX_DMACH_SPI0_TX] = 0,
+ [S3C24XX_DMACH_SPI1_RX] = 3,
+ [S3C24XX_DMACH_SPI1_TX] = 2,
+ [S3C24XX_DMACH_UART0] = 19,
+ [S3C24XX_DMACH_UART1] = 21,
+ [S3C24XX_DMACH_UART2] = 23,
+ [S3C24XX_DMACH_UART3] = 25,
+ [S3C24XX_DMACH_UART0_SRC2] = 20,
+ [S3C24XX_DMACH_UART1_SRC2] = 22,
+ [S3C24XX_DMACH_UART2_SRC2] = 24,
+ [S3C24XX_DMACH_UART3_SRC2] = 26,
+ [S3C24XX_DMACH_TIMER] = 9,
+ [S3C24XX_DMACH_I2S_RX] = 5,
+ [S3C24XX_DMACH_I2S_TX] = 4,
+ [S3C24XX_DMACH_PCM_IN] = 28,
+ [S3C24XX_DMACH_PCM_OUT] = 27,
+ [S3C24XX_DMACH_MIC_IN] = 29,
+};
+
+static struct s3c24xx_dma_platdata s3c2443_dma_platdata = {
+ .num_phy_channels = 6,
+ .reqsel_map = s3c2443_dma_reqsel,
+};
+
+struct platform_device s3c2443_device_dma = {
+ .name = "s3c2443-dma",
+ .id = 0,
+ .num_resources = ARRAY_SIZE(s3c2443_dma_resource),
+ .resource = s3c2443_dma_resource,
+ .dev = {
+ .platform_data = &s3c2443_dma_platdata,
+ },
+};
+#endif
diff --git a/arch/arm/mach-s3c24xx/common.h b/arch/arm/mach-s3c24xx/common.h
index 307c371..7e134d9 100644
--- a/arch/arm/mach-s3c24xx/common.h
+++ b/arch/arm/mach-s3c24xx/common.h
@@ -107,4 +107,7 @@ extern void s3c2443_init_irq(void);

extern struct syscore_ops s3c24xx_irq_syscore_ops;

+extern struct platform_device s3c2412_device_dma;
+extern struct platform_device s3c2443_device_dma;
+
#endif /* __ARCH_ARM_MACH_S3C24XX_COMMON_H */
--
1.7.2.3

2013-05-11 11:32:40

by Heiko Stübner

[permalink] [raw]
Subject: [RFC 4/4] ARM: SAMSUNG: set s3c24xx_dma_filter for s3c64xx-spi0 device

The spi-s3c64xx device is also used on the s3c2416 and s3c2443 SoCs.
The driver also already uses only generic dma-engine operations.

Therefore add another elif to set the s3c24xx filter.

Signed-off-by: Heiko Stuebner <[email protected]>
---
arch/arm/plat-samsung/devs.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/arch/arm/plat-samsung/devs.c b/arch/arm/plat-samsung/devs.c
index 30c2fe2..bb13f99 100644
--- a/arch/arm/plat-samsung/devs.c
+++ b/arch/arm/plat-samsung/devs.c
@@ -32,6 +32,7 @@
#include <linux/ioport.h>
#include <linux/platform_data/s3c-hsudc.h>
#include <linux/platform_data/s3c-hsotg.h>
+#include <linux/platform_data/dma-s3c24xx.h>

#include <media/s5p_hdmi.h>

@@ -1513,8 +1514,10 @@ void __init s3c64xx_spi0_set_platdata(int (*cfg_gpio)(void), int src_clk_nr,
pd.num_cs = num_cs;
pd.src_clk_nr = src_clk_nr;
pd.cfg_gpio = (cfg_gpio) ? cfg_gpio : s3c64xx_spi0_cfg_gpio;
-#ifdef CONFIG_PL330_DMA
+#if defined(CONFIG_PL330_DMA)
pd.filter = pl330_filter;
+#elif defined(CONFIG_S3C24XX_DMAC)
+ pd.filter = s3c24xx_dma_filter;
#endif

s3c_set_platdata(&pd, sizeof(pd), &s3c64xx_device_spi0);
--
1.7.2.3

2013-05-14 12:47:24

by Linus Walleij

[permalink] [raw]
Subject: Re: [RFC 2/4] dma: add dmaengine driver for Samsung s3c24xx SoCs

On Sat, May 11, 2013 at 1:31 PM, Heiko St?bner <[email protected]> wrote:

> Conceptually the s3c24xx-dma feels like a distant relative of the pl08x
> with numerous virtual channels being mapped to a lot less physical ones.
> The driver therefore borrows a lot from the amba-pl08x driver in this
> regard. Functionality-wise the driver gains a memcpy ability in addition
> to the slave_sg one.
>
> The driver currently only supports the "newer" SoCs which can use any
> physical channel for any dma slave. Support for the older SoCs where
> each channel only supports a subset of possible dma slaves will have to
> be added later.
>
> Tested on a s3c2416-based board, memcpy using the dmatest module and
> slave_sg partially using the spi-s3c64xx driver.
>
> Signed-off-by: Heiko Stuebner <[email protected]>

So have I understood correctly if I assume that *some* S3C
variants, i.e. this: arch/arm/mach-s3c64xx/dma.c
have a vanilla, unmodified, or just slightly modified
PL08x block, while this DMAC is something probably based on
the PL08x where some ASIC engineer has had a good time hacking
around in the VHDL code to meet some feature requirements.
Correct? Or plausible guess?

Exactly *how* far away from the pl08x hardware is it?

I guess you have already come to the conclusion that the
amba-pl08x.c driver cannot be augmented to handle this hardware
after some educated reading of that code?

But are really no parts reusable?

For example, if the LLIs have the same layout, could we split
out the LLI handling from amba-pl08x into a separate file and reuse
that?

The more you share with amba-pl08x the better for everyone,
I am positively sure. And please include Russell on the review
chain, he wrote the virtual channel abstraction.

If I was given the option amongst S3C work I would definatley
have augmented the amba-pl08x.c to handle the stuff in
arch/arm/mach-s3c64xx/dma.c *first* then started to work on
this driver, but I guess you might not be working on s3c64xx?

Yours,
Linus Walleij

2013-05-14 13:51:37

by Heiko Stübner

[permalink] [raw]
Subject: Re: [RFC 2/4] dma: add dmaengine driver for Samsung s3c24xx SoCs

Am Dienstag, 14. Mai 2013, 14:47:19 schrieb Linus Walleij:
> On Sat, May 11, 2013 at 1:31 PM, Heiko St?bner <[email protected]> wrote:
> > Conceptually the s3c24xx-dma feels like a distant relative of the pl08x
> > with numerous virtual channels being mapped to a lot less physical ones.
> > The driver therefore borrows a lot from the amba-pl08x driver in this
> > regard. Functionality-wise the driver gains a memcpy ability in addition
> > to the slave_sg one.
> >
> > The driver currently only supports the "newer" SoCs which can use any
> > physical channel for any dma slave. Support for the older SoCs where
> > each channel only supports a subset of possible dma slaves will have to
> > be added later.
> >
> > Tested on a s3c2416-based board, memcpy using the dmatest module and
> > slave_sg partially using the spi-s3c64xx driver.
> >
> > Signed-off-by: Heiko Stuebner <[email protected]>
>
> So have I understood correctly if I assume that *some* S3C
> variants, i.e. this: arch/arm/mach-s3c64xx/dma.c
> have a vanilla, unmodified, or just slightly modified
> PL08x block, while this DMAC is something probably based on
> the PL08x where some ASIC engineer has had a good time hacking
> around in the VHDL code to meet some feature requirements.
> Correct? Or plausible guess?

You're guess is at good as mine :-) . The public s3c64xx (ARM11 based)
documentation says that it is using s PL080 as dma controller while the
s3c24xx (ARM9 based) SoCs have this one, which doesn't come with any label in
the manuals.
Similar to the s3c64xx using a vic, while the s3c24xx uses something
homegrown.

The relationship description was more based on the concepts used, i.e. the
virtual channel concept and general handling of dma transfers feel somehow
similar - as I said these are my first steps into this, so I still need to
understand a lot.


> Exactly *how* far away from the pl08x hardware is it?
>
> I guess you have already come to the conclusion that the
> amba-pl08x.c driver cannot be augmented to handle this hardware
> after some educated reading of that code?
>
> But are really no parts reusable?
>
> For example, if the LLIs have the same layout, could we split
> out the LLI handling from amba-pl08x into a separate file and reuse
> that?

The s3c24xx-dma doesn't have these. You simply put source and destination
addresses on the bus, tell it the number of transfers and start it, while the
pl080 seems to split these then into the LLIs, which seems to be some kind of
hardware feature.

For the rest, the register-layout of the controller is completely different (I
checked the PL080 manual), each channel has its own interrupt and the above
mentioned missing LLIs.

Both drivers share somehow how they handle end of transfers, i.e. how they
reuse the physical channel for a possible waiting virtual channel. But it
didn't look like anything worth factoring out.


> The more you share with amba-pl08x the better for everyone,
> I am positively sure. And please include Russell on the review
> chain, he wrote the virtual channel abstraction.

I think the best feature is already factored out - the virtual channel
handling, which was very nice to have.

I used the list from get_maintainer.pl, so sorry for forgetting Russel.


> If I was given the option amongst S3C work I would definatley
> have augmented the amba-pl08x.c to handle the stuff in
> arch/arm/mach-s3c64xx/dma.c *first* then started to work on
> this driver, but I guess you might not be working on s3c64xx?

correct, my pet-project is s3c2416-based [0], so I haven't seen any other
Samsung stuff at all.


Heiko


[0] http://www.youtube.com/user/mmind81

2013-05-14 14:21:55

by Tomasz Figa

[permalink] [raw]
Subject: Re: [RFC 2/4] dma: add dmaengine driver for Samsung s3c24xx SoCs

Hi Linus, Heiko,

On Tuesday 14 of May 2013 14:47:19 Linus Walleij wrote:
> On Sat, May 11, 2013 at 1:31 PM, Heiko St?bner <[email protected]> wrote:
> > Conceptually the s3c24xx-dma feels like a distant relative of the pl08x
> > with numerous virtual channels being mapped to a lot less physical ones.
> > The driver therefore borrows a lot from the amba-pl08x driver in this
> > regard. Functionality-wise the driver gains a memcpy ability in addition
> > to the slave_sg one.
> >
> > The driver currently only supports the "newer" SoCs which can use any
> > physical channel for any dma slave. Support for the older SoCs where
> > each channel only supports a subset of possible dma slaves will have to
> > be added later.
> >
> > Tested on a s3c2416-based board, memcpy using the dmatest module and
> > slave_sg partially using the spi-s3c64xx driver.
> >
> > Signed-off-by: Heiko Stuebner <[email protected]>
>
> So have I understood correctly if I assume that *some* S3C
> variants, i.e. this: arch/arm/mach-s3c64xx/dma.c
> have a vanilla, unmodified, or just slightly modified
> PL08x block, while this DMAC is something probably based on
> the PL08x where some ASIC engineer has had a good time hacking
> around in the VHDL code to meet some feature requirements.
> Correct? Or plausible guess?
>
> Exactly *how* far away from the pl08x hardware is it?

AFAIK the DMAC of S3C24xx is completely different from PL08x. I think Heiko
just meant that it uses similar concepts, like virtual channels.

> I guess you have already come to the conclusion that the
> amba-pl08x.c driver cannot be augmented to handle this hardware
> after some educated reading of that code?
>
> But are really no parts reusable?
>
> For example, if the LLIs have the same layout, could we split
> out the LLI handling from amba-pl08x into a separate file and reuse
> that?
>
> The more you share with amba-pl08x the better for everyone,
> I am positively sure. And please include Russell on the review
> chain, he wrote the virtual channel abstraction.
>
> If I was given the option amongst S3C work I would definatley
> have augmented the amba-pl08x.c to handle the stuff in
> arch/arm/mach-s3c64xx/dma.c *first* then started to work on
> this driver, but I guess you might not be working on s3c64xx?

Well, I still hope that someone will pick up the work on adapting PL08x driver
to support PL080s of S3C64xx as well, but most likely it will end up with me
doing it, as a part of DT support for S3C64xx.

Best regards,
--
Tomasz Figa
Samsung Poland R&D Center
SW Solution Development, Kernel and System Framework

2013-05-15 18:38:09

by Linus Walleij

[permalink] [raw]
Subject: Re: [RFC 2/4] dma: add dmaengine driver for Samsung s3c24xx SoCs

On Tue, May 14, 2013 at 3:51 PM, Heiko St?bner <[email protected]> wrote:
> Am Dienstag, 14. Mai 2013, 14:47:19 schrieb Linus Walleij:
>> On Sat, May 11, 2013 at 1:31 PM, Heiko St?bner <[email protected]> wrote:
>> > Conceptually the s3c24xx-dma feels like a distant relative of the pl08x
>> > with numerous virtual channels being mapped to a lot less physical ones.
>> > The driver therefore borrows a lot from the amba-pl08x driver in this
>> > regard. Functionality-wise the driver gains a memcpy ability in addition
>> > to the slave_sg one.
>> >
>> > The driver currently only supports the "newer" SoCs which can use any
>> > physical channel for any dma slave. Support for the older SoCs where
>> > each channel only supports a subset of possible dma slaves will have to
>> > be added later.
>> >
>> > Tested on a s3c2416-based board, memcpy using the dmatest module and
>> > slave_sg partially using the spi-s3c64xx driver.
>> >
>> > Signed-off-by: Heiko Stuebner <[email protected]>
>>
>> So have I understood correctly if I assume that *some* S3C
>> variants, i.e. this: arch/arm/mach-s3c64xx/dma.c
>> have a vanilla, unmodified, or just slightly modified
>> PL08x block, while this DMAC is something probably based on
>> the PL08x where some ASIC engineer has had a good time hacking
>> around in the VHDL code to meet some feature requirements.
>> Correct? Or plausible guess?
>
> You're guess is at good as mine :-) . The public s3c64xx (ARM11 based)
> documentation says that it is using s PL080 as dma controller while the
> s3c24xx (ARM9 based) SoCs have this one, which doesn't come with any label in
> the manuals.
> Similar to the s3c64xx using a vic, while the s3c24xx uses something
> homegrown.
>
> The relationship description was more based on the concepts used, i.e. the
> virtual channel concept and general handling of dma transfers feel somehow
> similar - as I said these are my first steps into this, so I still need to
> understand a lot.

OK then, a separate driver seems required, will look a bit closer at
the patch as such.

Yours,
Linus Walleij

2013-05-15 18:53:34

by Linus Walleij

[permalink] [raw]
Subject: Re: [RFC 2/4] dma: add dmaengine driver for Samsung s3c24xx SoCs

On Sat, May 11, 2013 at 1:31 PM, Heiko St?bner <[email protected]> wrote:

> This adds a new driver to support the s3c24xx dma using the dmaengine
> and make the old one in mach-s3c24xx obsolete in the long run.
>
> Conceptually the s3c24xx-dma feels like a distant relative of the pl08x
> with numerous virtual channels being mapped to a lot less physical ones.
> The driver therefore borrows a lot from the amba-pl08x driver in this
> regard. Functionality-wise the driver gains a memcpy ability in addition
> to the slave_sg one.
>
> The driver currently only supports the "newer" SoCs which can use any
> physical channel for any dma slave. Support for the older SoCs where
> each channel only supports a subset of possible dma slaves will have to
> be added later.
>
> Tested on a s3c2416-based board, memcpy using the dmatest module and
> slave_sg partially using the spi-s3c64xx driver.
>
> Signed-off-by: Heiko Stuebner <[email protected]>
(...)
> +static u32 s3c24xx_dma_getbytes_chan(struct s3c24xx_dma_chan *s3cchan)
> +{
> + struct s3c24xx_dma_phy *phy = s3cchan->phy;
> + struct s3c24xx_txd *txd = s3cchan->at;
> + u32 tc = readl(phy->base + DSTAT) & DSTAT_CURRTC_MASK;
> +
> + switch (txd->dcon & DCON_DSZ_MASK) {
> + case DCON_DSZ_BYTE:
> + return tc;
> + case DCON_DSZ_HALFWORD:
> + return tc * 2;
> + case DCON_DSZ_WORD:
> + return tc * 4;
> + default:
> + break;
> + }
> +
> + BUG();

Isn't that a bit nasty. This macro should be used with care and we
should recover if possible. dev_err()?

> +/*
> + * Set the initial DMA register values.
> + * Put them into the hardware and start the transfer.
> + */
> +static void s3c24xx_dma_start_next_txd(struct s3c24xx_dma_chan *s3cchan)
> +{
> + struct s3c24xx_dma_engine *s3cdma = s3cchan->host;
> + struct s3c24xx_dma_phy *phy = s3cchan->phy;
> + struct virt_dma_desc *vd = vchan_next_desc(&s3cchan->vc);
> + struct s3c24xx_txd *txd = to_s3c24xx_txd(&vd->tx);
> + u32 dcon = txd->dcon;
> + u32 val;
> +
> + list_del(&txd->vd.node);
> +
> + s3cchan->at = txd;
> +
> + /* Wait for channel inactive */
> + while (s3c24xx_dma_phy_busy(phy))
> + cpu_relax();
> +
> + /* transfer-size and -count from len and width */
> + switch (txd->width) {
> + case 1:
> + dcon |= DCON_DSZ_BYTE | txd->len;
> + break;
> + case 2:
> + dcon |= DCON_DSZ_HALFWORD | (txd->len / 2);
> + break;
> + case 4:
> + dcon |= DCON_DSZ_WORD | (txd->len / 4);
> + break;
> + }
> +
> + if (s3cchan->slave) {
> + if (s3cdma->sdata->has_reqsel) {
> + int reqsel = s3cdma->pdata->reqsel_map[s3cchan->id];
> + writel((reqsel << 1) | DMAREQSEL_HW,
> + phy->base + DMAREQSEL);
> + } else {
> + dev_err(&s3cdma->pdev->dev, "non-reqsel unsupported\n");
> + return;
> + dcon |= DCON_HWTRIG;
> + }
> + } else {
> + if (s3cdma->sdata->has_reqsel) {
> + writel(0, phy->base + DMAREQSEL);
> + } else {
> + dev_err(&s3cdma->pdev->dev, "non-reqsel unsupported\n");
> + return;
> + }
> + }
> +
> + writel(txd->src_addr, phy->base + DISRC);
> + writel(txd->disrcc, phy->base + DISRCC);
> + writel(txd->dst_addr, phy->base + DIDST);
> + writel(txd->didstc, phy->base + DIDSTC);
> +
> + writel(dcon, phy->base + DCON);
> +
> + val = readl(phy->base + DMASKTRIG);
> + val &= ~DMASKTRIG_STOP;
> + val |= DMASKTRIG_ON;
> + writel(val, phy->base + DMASKTRIG);
> +
> + /* trigger the dma operation for memcpy transfers */
> + if (!s3cchan->slave) {
> + val = readl(phy->base + DMASKTRIG);
> + val |= DMASKTRIG_SWTRIG;
> + writel(val, phy->base + DMASKTRIG);
> + }
> +}

Since you have a few readl() and writel() in potentially
time-critical code, consider using readl_relaxed() and
writel_relaxed().

> +static irqreturn_t s3c24xx_dma_irq(int irq, void *data)
> +{
> + struct s3c24xx_dma_phy *phy = data;
> + struct s3c24xx_dma_chan *s3cchan = phy->serving;
> + struct s3c24xx_txd *txd;
> +
> + dev_dbg(&phy->host->pdev->dev, "interrupt on channel %d\n", phy->id);
> +
> + if (!s3cchan) {
> + dev_err(&phy->host->pdev->dev, "interrupt on unused channel %d\n",
> + phy->id);
> + return IRQ_NONE;
> + }
> +
> + spin_lock(&s3cchan->vc.lock);
> + txd = s3cchan->at;
> + if (txd) {
> + s3cchan->at = NULL;
> + vchan_cookie_complete(&txd->vd);
> +
> + /*
> + * And start the next descriptor (if any),
> + * otherwise free this channel.
> + */
> + if (vchan_next_desc(&s3cchan->vc))
> + s3c24xx_dma_start_next_txd(s3cchan);
> + else
> + s3c24xx_dma_phy_free(s3cchan);
> + }
> + spin_unlock(&s3cchan->vc.lock);
> +
> + return IRQ_HANDLED;
> +}

OK so one IRQ per channel. Is there really no status
register to check or flag to clear on these IRQs?

Apart from these smallish things it looks good to me:
Acked-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2013-05-15 20:32:07

by Heiko Stübner

[permalink] [raw]
Subject: Re: [RFC 2/4] dma: add dmaengine driver for Samsung s3c24xx SoCs

Am Mittwoch, 15. Mai 2013, 20:53:32 schrieb Linus Walleij:
> On Sat, May 11, 2013 at 1:31 PM, Heiko St?bner <[email protected]> wrote:
> > This adds a new driver to support the s3c24xx dma using the dmaengine
> > and make the old one in mach-s3c24xx obsolete in the long run.
> >
> > Conceptually the s3c24xx-dma feels like a distant relative of the pl08x
> > with numerous virtual channels being mapped to a lot less physical ones.
> > The driver therefore borrows a lot from the amba-pl08x driver in this
> > regard. Functionality-wise the driver gains a memcpy ability in addition
> > to the slave_sg one.
> >
> > The driver currently only supports the "newer" SoCs which can use any
> > physical channel for any dma slave. Support for the older SoCs where
> > each channel only supports a subset of possible dma slaves will have to
> > be added later.
> >
> > Tested on a s3c2416-based board, memcpy using the dmatest module and
> > slave_sg partially using the spi-s3c64xx driver.
> >
> > Signed-off-by: Heiko Stuebner <[email protected]>
>
> (...)
>
> > +static u32 s3c24xx_dma_getbytes_chan(struct s3c24xx_dma_chan *s3cchan)
> > +{
> > + struct s3c24xx_dma_phy *phy = s3cchan->phy;
> > + struct s3c24xx_txd *txd = s3cchan->at;
> > + u32 tc = readl(phy->base + DSTAT) & DSTAT_CURRTC_MASK;
> > +
> > + switch (txd->dcon & DCON_DSZ_MASK) {
> > + case DCON_DSZ_BYTE:
> > + return tc;
> > + case DCON_DSZ_HALFWORD:
> > + return tc * 2;
> > + case DCON_DSZ_WORD:
> > + return tc * 4;
> > + default:
> > + break;
> > + }
> > +
> > + BUG();
>
> Isn't that a bit nasty. This macro should be used with care and we
> should recover if possible. dev_err()?

runtime_config already denies any settings not in the 1,2 or 4bytes range -
the default-part should therefore never be reached. So if any other value
magically appears in the register and triggers the default-part, something is
seriously wrong. So my guess is, the BUG might be appropriate.

On the other hand the whole default+BUG part could also simply go away, for
the same reasons.


> > +/*
> > + * Set the initial DMA register values.
> > + * Put them into the hardware and start the transfer.
> > + */
> > +static void s3c24xx_dma_start_next_txd(struct s3c24xx_dma_chan *s3cchan)
> > +{
> > + struct s3c24xx_dma_engine *s3cdma = s3cchan->host;
> > + struct s3c24xx_dma_phy *phy = s3cchan->phy;
> > + struct virt_dma_desc *vd = vchan_next_desc(&s3cchan->vc);
> > + struct s3c24xx_txd *txd = to_s3c24xx_txd(&vd->tx);
> > + u32 dcon = txd->dcon;
> > + u32 val;
> > +
> > + list_del(&txd->vd.node);
> > +
> > + s3cchan->at = txd;
> > +
> > + /* Wait for channel inactive */
> > + while (s3c24xx_dma_phy_busy(phy))
> > + cpu_relax();
> > +
> > + /* transfer-size and -count from len and width */
> > + switch (txd->width) {
> > + case 1:
> > + dcon |= DCON_DSZ_BYTE | txd->len;
> > + break;
> > + case 2:
> > + dcon |= DCON_DSZ_HALFWORD | (txd->len / 2);
> > + break;
> > + case 4:
> > + dcon |= DCON_DSZ_WORD | (txd->len / 4);
> > + break;
> > + }
> > +
> > + if (s3cchan->slave) {
> > + if (s3cdma->sdata->has_reqsel) {
> > + int reqsel =
> > s3cdma->pdata->reqsel_map[s3cchan->id]; +
> > writel((reqsel << 1) | DMAREQSEL_HW,
> > + phy->base + DMAREQSEL);
> > + } else {
> > + dev_err(&s3cdma->pdev->dev, "non-reqsel
> > unsupported\n"); + return;
> > + dcon |= DCON_HWTRIG;
> > + }
> > + } else {
> > + if (s3cdma->sdata->has_reqsel) {
> > + writel(0, phy->base + DMAREQSEL);
> > + } else {
> > + dev_err(&s3cdma->pdev->dev, "non-reqsel
> > unsupported\n"); + return;
> > + }
> > + }
> > +
> > + writel(txd->src_addr, phy->base + DISRC);
> > + writel(txd->disrcc, phy->base + DISRCC);
> > + writel(txd->dst_addr, phy->base + DIDST);
> > + writel(txd->didstc, phy->base + DIDSTC);
> > +
> > + writel(dcon, phy->base + DCON);
> > +
> > + val = readl(phy->base + DMASKTRIG);
> > + val &= ~DMASKTRIG_STOP;
> > + val |= DMASKTRIG_ON;
> > + writel(val, phy->base + DMASKTRIG);
> > +
> > + /* trigger the dma operation for memcpy transfers */
> > + if (!s3cchan->slave) {
> > + val = readl(phy->base + DMASKTRIG);
> > + val |= DMASKTRIG_SWTRIG;
> > + writel(val, phy->base + DMASKTRIG);
> > + }
> > +}
>
> Since you have a few readl() and writel() in potentially
> time-critical code, consider using readl_relaxed() and
> writel_relaxed().

You're right of course.

If I understand the writel semantics and the thread from you from 2011 [0]
correctly, only the writel to DMASKTRIG mustn't be relaxed to make sure the
settings registers are written to before, so like:

writel_relaxed(txd->src_addr, phy->base + DISRC);
writel_relaxed(txd->disrcc, phy->base + DISRCC);
writel_relaxed(txd->dst_addr, phy->base + DIDST);
writel_relaxed(txd->didstc, phy->base + DIDSTC);
writel_relaxed(dcon, phy->base + DCON);

val = readl_relaxed(phy->base + DMASKTRIG);
val &= ~DMASKTRIG_STOP;
val |= DMASKTRIG_ON;
writel(val, phy->base + DMASKTRIG);


And note to self: check if the memcpy-speciality can move into the first
DMASKTRIG write.

> > +static irqreturn_t s3c24xx_dma_irq(int irq, void *data)
> > +{
> > + struct s3c24xx_dma_phy *phy = data;
> > + struct s3c24xx_dma_chan *s3cchan = phy->serving;
> > + struct s3c24xx_txd *txd;
> > +
> > + dev_dbg(&phy->host->pdev->dev, "interrupt on channel %d\n",
> > phy->id); +
> > + if (!s3cchan) {
> > + dev_err(&phy->host->pdev->dev, "interrupt on unused
> > channel %d\n", + phy->id);
> > + return IRQ_NONE;
> > + }
> > +
> > + spin_lock(&s3cchan->vc.lock);
> > + txd = s3cchan->at;
> > + if (txd) {
> > + s3cchan->at = NULL;
> > + vchan_cookie_complete(&txd->vd);
> > +
> > + /*
> > + * And start the next descriptor (if any),
> > + * otherwise free this channel.
> > + */
> > + if (vchan_next_desc(&s3cchan->vc))
> > + s3c24xx_dma_start_next_txd(s3cchan);
> > + else
> > + s3c24xx_dma_phy_free(s3cchan);
> > + }
> > + spin_unlock(&s3cchan->vc.lock);
> > +
> > + return IRQ_HANDLED;
> > +}
>
> OK so one IRQ per channel. Is there really no status
> register to check or flag to clear on these IRQs?

Nope there are none. The only status you get from the controller is busy/non-
busy and remaining transfers for the transaction - the DSTAT register in the
code. And not listed in the code the current memory addresses (source and
destination) in use. There are no other registers at all.

And the irq itself only triggers when all transfers of the transaction are
done (transfer_count is 0).


> Apart from these smallish things it looks good to me:
> Acked-by: Linus Walleij <[email protected]>

really? Cool.
Part of me was expecting tomatoes or other vegetables to be thrown ;-) .


Thanks for looking thru the driver.
Heiko


[0] http://comments.gmane.org/gmane.linux.ports.arm.kernel/117626

2013-05-15 21:20:15

by Sylwester Nawrocki

[permalink] [raw]
Subject: Re: [RFC 2/4] dma: add dmaengine driver for Samsung s3c24xx SoCs

On 05/15/2013 10:31 PM, Heiko St?bner wrote:
>>> + BUG();
>> >
>> > Isn't that a bit nasty. This macro should be used with care and we
>> > should recover if possible. dev_err()?
>
> runtime_config already denies any settings not in the 1,2 or 4bytes range -
> the default-part should therefore never be reached. So if any other value
> magically appears in the register and triggers the default-part, something is
> seriously wrong. So my guess is, the BUG might be appropriate.
>
> On the other hand the whole default+BUG part could also simply go away, for
> the same reasons.

IMHO BUG() is not needed at all. As Linus suggested dev_err() is such case
or WARN_ON() would be more appropriate. This has been discussed in the past
extensively, not sure if you are aware of the other Linus' opinion on
BUG()/BUG_ON() proliferation: https://lkml.org/lkml/2012/9/27/461

Regards,
Sylwester

2013-05-15 21:48:46

by Heiko Stübner

[permalink] [raw]
Subject: Re: [RFC 2/4] dma: add dmaengine driver for Samsung s3c24xx SoCs

Am Mittwoch, 15. Mai 2013, 23:20:08 schrieb Sylwester Nawrocki:
> On 05/15/2013 10:31 PM, Heiko St?bner wrote:
> >>> + BUG();
> >>>
> >> > Isn't that a bit nasty. This macro should be used with care and we
> >> > should recover if possible. dev_err()?
> >
> > runtime_config already denies any settings not in the 1,2 or 4bytes range
> > - the default-part should therefore never be reached. So if any other
> > value magically appears in the register and triggers the default-part,
> > something is seriously wrong. So my guess is, the BUG might be
> > appropriate.
> >
> > On the other hand the whole default+BUG part could also simply go away,
> > for the same reasons.
>
> IMHO BUG() is not needed at all. As Linus suggested dev_err() is such case
> or WARN_ON() would be more appropriate. This has been discussed in the past
> extensively, not sure if you are aware of the other Linus' opinion on
> BUG()/BUG_ON() proliferation: https://lkml.org/lkml/2012/9/27/461

Very interesting read and I'll keep this in mind in the future. What about the
other option ... i.e. simply getting rid of the whole "error handling", as the
other code paths should already make sure that only valid values get written
into the register.

Can the value change in the register somehow on its own without kernel
intervention, or does this not happen?


Thanks
Heiko

2013-05-15 22:02:46

by Tomasz Figa

[permalink] [raw]
Subject: Re: [RFC 2/4] dma: add dmaengine driver for Samsung s3c24xx SoCs

On Wednesday 15 of May 2013 23:48:31 Heiko St?bner wrote:
> Am Mittwoch, 15. Mai 2013, 23:20:08 schrieb Sylwester Nawrocki:
> > On 05/15/2013 10:31 PM, Heiko St?bner wrote:
> > >>> + BUG();
> > >>>
> > >> > Isn't that a bit nasty. This macro should be used with care and
> > >> > we
> > >> > should recover if possible. dev_err()?
> > >
> > > runtime_config already denies any settings not in the 1,2 or 4bytes
> > > range - the default-part should therefore never be reached. So if
> > > any other value magically appears in the register and triggers the
> > > default-part, something is seriously wrong. So my guess is, the BUG
> > > might be appropriate.
> > >
> > > On the other hand the whole default+BUG part could also simply go
> > > away,
> > > for the same reasons.
> >
> > IMHO BUG() is not needed at all. As Linus suggested dev_err() is such
> > case or WARN_ON() would be more appropriate. This has been discussed
> > in the past extensively, not sure if you are aware of the other
> > Linus' opinion on BUG()/BUG_ON() proliferation:
> > https://lkml.org/lkml/2012/9/27/461
> Very interesting read and I'll keep this in mind in the future. What
> about the other option ... i.e. simply getting rid of the whole "error
> handling", as the other code paths should already make sure that only
> valid values get written into the register.
>
> Can the value change in the register somehow on its own without kernel
> intervention, or does this not happen?

Hmm, it depends on hardware, I guess. Not sure how it works on this
particular IP.

Still, the mentioned BUG() was about a value in a driver-filled struct,
wasn't it?

/* Quoting the the code for reference */

> +static u32 s3c24xx_dma_getbytes_chan(struct s3c24xx_dma_chan *s3cchan)
> +{
> + struct s3c24xx_dma_phy *phy = s3cchan->phy;
> + struct s3c24xx_txd *txd = s3cchan->at;
> + u32 tc = readl(phy->base + DSTAT) & DSTAT_CURRTC_MASK;
> +
> + switch (txd->dcon & DCON_DSZ_MASK) {
> + case DCON_DSZ_BYTE:
> + return tc;
> + case DCON_DSZ_HALFWORD:
> + return tc * 2;
> + case DCON_DSZ_WORD:
> + return tc * 4;
> + default:
> + break;
> + }
> +
> + BUG();

(Btw. I don't see anything setting the DCON_DSZ bits in this field. Am I
missing something?)

Best regards,
Tomasz

2013-05-15 22:45:16

by Heiko Stübner

[permalink] [raw]
Subject: Re: [RFC 2/4] dma: add dmaengine driver for Samsung s3c24xx SoCs

Am Donnerstag, 16. Mai 2013, 00:02:40 schrieb Tomasz Figa:
> On Wednesday 15 of May 2013 23:48:31 Heiko St?bner wrote:
> > Am Mittwoch, 15. Mai 2013, 23:20:08 schrieb Sylwester Nawrocki:
> > > On 05/15/2013 10:31 PM, Heiko St?bner wrote:
> > > >>> + BUG();
> > > >>>
> > > >> > Isn't that a bit nasty. This macro should be used with care and
> > > >> > we
> > > >> > should recover if possible. dev_err()?
> > > >
> > > > runtime_config already denies any settings not in the 1,2 or 4bytes
> > > > range - the default-part should therefore never be reached. So if
> > > > any other value magically appears in the register and triggers the
> > > > default-part, something is seriously wrong. So my guess is, the BUG
> > > > might be appropriate.
> > > >
> > > > On the other hand the whole default+BUG part could also simply go
> > > > away,
> > > > for the same reasons.
> > >
> > > IMHO BUG() is not needed at all. As Linus suggested dev_err() is such
> > > case or WARN_ON() would be more appropriate. This has been discussed
> > > in the past extensively, not sure if you are aware of the other
> > > Linus' opinion on BUG()/BUG_ON() proliferation:
> > > https://lkml.org/lkml/2012/9/27/461
> >
> > Very interesting read and I'll keep this in mind in the future. What
> > about the other option ... i.e. simply getting rid of the whole "error
> > handling", as the other code paths should already make sure that only
> > valid values get written into the register.
> >
> > Can the value change in the register somehow on its own without kernel
> > intervention, or does this not happen?
>
> Hmm, it depends on hardware, I guess. Not sure how it works on this
> particular IP.
>
> Still, the mentioned BUG() was about a value in a driver-filled struct,
> wasn't it?
>
> /* Quoting the the code for reference */
>
> > +static u32 s3c24xx_dma_getbytes_chan(struct s3c24xx_dma_chan *s3cchan)
> > +{
> > + struct s3c24xx_dma_phy *phy = s3cchan->phy;
> > + struct s3c24xx_txd *txd = s3cchan->at;
> > + u32 tc = readl(phy->base + DSTAT) & DSTAT_CURRTC_MASK;
> > +
> > + switch (txd->dcon & DCON_DSZ_MASK) {
> > + case DCON_DSZ_BYTE:
> > + return tc;
> > + case DCON_DSZ_HALFWORD:
> > + return tc * 2;
> > + case DCON_DSZ_WORD:
> > + return tc * 4;
> > + default:
> > + break;
> > + }
> > +
> > + BUG();
>
> (Btw. I don't see anything setting the DCON_DSZ bits in this field. Am I
> missing something?)

this is for calculating the remaining bytes of the transaction. which is used
in s3c24xx_dma_tx_status.

And when looking at it again, I can't really fathom why I did it this way with
decoding the DSZ from the dcon value of the s3c24xx_txd again instead of
simply using the width value of the same struct ....

So it can be much simpler as
(...)
u32 tc = readl(phy->base + DSTAT) & DSTAT_CURRTC_MASK;
return tc * txd->width;

getting rid of this stuff alltogether


still puzzled how I came up with this strangeness in the first place
Heiko

2013-05-15 23:26:14

by Tomasz Figa

[permalink] [raw]
Subject: Re: [RFC 2/4] dma: add dmaengine driver for Samsung s3c24xx SoCs

On Thursday 16 of May 2013 00:45:03 Heiko St?bner wrote:
> Am Donnerstag, 16. Mai 2013, 00:02:40 schrieb Tomasz Figa:
> > On Wednesday 15 of May 2013 23:48:31 Heiko St?bner wrote:
> > > Am Mittwoch, 15. Mai 2013, 23:20:08 schrieb Sylwester Nawrocki:
> > > > On 05/15/2013 10:31 PM, Heiko St?bner wrote:
> > > > >>> + BUG();
> > > > >>>
> > > > >> > Isn't that a bit nasty. This macro should be used with care
> > > > >> > and
> > > > >> > we
> > > > >> > should recover if possible. dev_err()?
> > > > >
> > > > > runtime_config already denies any settings not in the 1,2 or
> > > > > 4bytes
> > > > > range - the default-part should therefore never be reached. So
> > > > > if
> > > > > any other value magically appears in the register and triggers
> > > > > the
> > > > > default-part, something is seriously wrong. So my guess is, the
> > > > > BUG
> > > > > might be appropriate.
> > > > >
> > > > > On the other hand the whole default+BUG part could also simply
> > > > > go
> > > > > away,
> > > > > for the same reasons.
> > > >
> > > > IMHO BUG() is not needed at all. As Linus suggested dev_err() is
> > > > such
> > > > case or WARN_ON() would be more appropriate. This has been
> > > > discussed
> > > > in the past extensively, not sure if you are aware of the other
> > > > Linus' opinion on BUG()/BUG_ON() proliferation:
> > > > https://lkml.org/lkml/2012/9/27/461
> > >
> > > Very interesting read and I'll keep this in mind in the future. What
> > > about the other option ... i.e. simply getting rid of the whole
> > > "error
> > > handling", as the other code paths should already make sure that
> > > only
> > > valid values get written into the register.
> > >
> > > Can the value change in the register somehow on its own without
> > > kernel
> > > intervention, or does this not happen?
> >
> > Hmm, it depends on hardware, I guess. Not sure how it works on this
> > particular IP.
> >
> > Still, the mentioned BUG() was about a value in a driver-filled
> > struct,
> > wasn't it?
> >
> > /* Quoting the the code for reference */
> >
> > > +static u32 s3c24xx_dma_getbytes_chan(struct s3c24xx_dma_chan
> > > *s3cchan)
> > > +{
> > > + struct s3c24xx_dma_phy *phy = s3cchan->phy;
> > > + struct s3c24xx_txd *txd = s3cchan->at;
> > > + u32 tc = readl(phy->base + DSTAT) & DSTAT_CURRTC_MASK;
> > > +
> > > + switch (txd->dcon & DCON_DSZ_MASK) {
> > > + case DCON_DSZ_BYTE:
> > > + return tc;
> > > + case DCON_DSZ_HALFWORD:
> > > + return tc * 2;
> > > + case DCON_DSZ_WORD:
> > > + return tc * 4;
> > > + default:
> > > + break;
> > > + }
> > > +
> > > + BUG();
> >
> > (Btw. I don't see anything setting the DCON_DSZ bits in this field. Am
> > I missing something?)
>
> this is for calculating the remaining bytes of the transaction. which is
> used in s3c24xx_dma_tx_status.
>
> And when looking at it again, I can't really fathom why I did it this
> way with decoding the DSZ from the dcon value of the s3c24xx_txd again
> instead of simply using the width value of the same struct ....
>
> So it can be much simpler as
> (...)
> u32 tc = readl(phy->base + DSTAT) & DSTAT_CURRTC_MASK;
> return tc * txd->width;
>
> getting rid of this stuff alltogether
>
>
> still puzzled how I came up with this strangeness in the first place

Hehe, happens.

I'm still yet to review the whole series, but I'm failing to find enough
time. Hopefully will get to do it soon.

Best regards,
Tomasz

2013-05-16 02:18:44

by Jingoo Han

[permalink] [raw]
Subject: Re: [RFC 2/4] dma: add dmaengine driver for Samsung s3c24xx SoCs

On Tuesday, May 14, 2013 11:22 PM Tomasz Figa wrote:
>
> Hi Linus, Heiko,
>
> On Tuesday 14 of May 2013 14:47:19 Linus Walleij wrote:
> > On Sat, May 11, 2013 at 1:31 PM, Heiko St?bner <[email protected]> wrote:
> > > Conceptually the s3c24xx-dma feels like a distant relative of the pl08x
> > > with numerous virtual channels being mapped to a lot less physical ones.
> > > The driver therefore borrows a lot from the amba-pl08x driver in this
> > > regard. Functionality-wise the driver gains a memcpy ability in addition
> > > to the slave_sg one.
> > >
> > > The driver currently only supports the "newer" SoCs which can use any
> > > physical channel for any dma slave. Support for the older SoCs where
> > > each channel only supports a subset of possible dma slaves will have to
> > > be added later.
> > >
> > > Tested on a s3c2416-based board, memcpy using the dmatest module and
> > > slave_sg partially using the spi-s3c64xx driver.
> > >
> > > Signed-off-by: Heiko Stuebner <[email protected]>
> >
> > So have I understood correctly if I assume that *some* S3C
> > variants, i.e. this: arch/arm/mach-s3c64xx/dma.c
> > have a vanilla, unmodified, or just slightly modified
> > PL08x block, while this DMAC is something probably based on
> > the PL08x where some ASIC engineer has had a good time hacking
> > around in the VHDL code to meet some feature requirements.
> > Correct? Or plausible guess?
> >
> > Exactly *how* far away from the pl08x hardware is it?
>
> AFAIK the DMAC of S3C24xx is completely different from PL08x. I think Heiko
> just meant that it uses similar concepts, like virtual channels.

Yes, right.
the DMAC of S3C24xx is completely different from PL08x.
As Heiko mentioned, the DMAC of S3C24xx is 'home grown' as other IPs of S3C24xx.

Best regards,
Jingoo Han????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-05-17 12:20:51

by Linus Walleij

[permalink] [raw]
Subject: Re: [RFC 2/4] dma: add dmaengine driver for Samsung s3c24xx SoCs

On Wed, May 15, 2013 at 10:31 PM, Heiko St?bner <[email protected]> wrote:

> If I understand the writel semantics and the thread from you from 2011 [0]
> correctly, only the writel to DMASKTRIG mustn't be relaxed to make sure the
> settings registers are written to before, so like:
>
> writel_relaxed(txd->src_addr, phy->base + DISRC);
> writel_relaxed(txd->disrcc, phy->base + DISRCC);
> writel_relaxed(txd->dst_addr, phy->base + DIDST);
> writel_relaxed(txd->didstc, phy->base + DIDSTC);
> writel_relaxed(dcon, phy->base + DCON);
>
> val = readl_relaxed(phy->base + DMASKTRIG);
> val &= ~DMASKTRIG_STOP;
> val |= DMASKTRIG_ON;
> writel(val, phy->base + DMASKTRIG);

Yep. That will drain write buffers etc and make sure all outstanding writes hit
the hardware.

Yours,
Linus Walleij