To use DMA generic APIs for Samsung DMA

This patch update runtime PM for PL330 DMAC to reduce power consumption.

Signed-off-by Chanwoo Choi <[email protected]>

Signed-off-by Kyungmin Park <[email protected]>

---
The following patch support runtime PM for PL330 DMAC, but the clock of PL330 is always on. If the clock of PL330 is always on, additional power (10mA) is consumed.

include/linux/amba/pl330.h +0 -0

index 17b0ada..d9a63fd 100644
@@ -12,6 +12,9 @@

?#ifndef ? ? ? ?__AMBA_PL330_H_
?#define ? ? ? ?__AMBA_PL330_H_

include/linux/amba/pl330.h +3 -0

+#include <linux/dmaengine.h>
+#include <linux/interrupt.h>
+

?#include <asm/hardware/pl330.h>

?struct dma_pl330_peri {

include/linux/amba/pl330.h +51 -0

@@ -42,4 +45,55 @@ struct dma_pl330_platdata {
 ? ? ? ?unsigned mcbuf_sz;
 ?};
 
+struct dma_pl330_chan {
+ ? ? ? /* Schedule desc completion */
+ ? ? ? struct tasklet_struct task;
+
+ ? ? ? /* DMA-Engine Channel */
+ ? ? ? struct dma_chan chan;
+
+ ? ? ? /* Last completed cookie */
+ ? ? ? dma_cookie_t completed;
+
+ ? ? ? /* List of to be xfered descriptors */
+ ? ? ? struct list_head work_list;
+
+ ? ? ? /* Pointer to the DMAC that manages this channel,
+ ? ? ? ?* NULL if the channel is available to be acquired.
+ ? ? ? ?* As the parent, this DMAC also provides descriptors
+ ? ? ? ?* to the channel.
+ ? ? ? ?*/
+ ? ? ? struct dma_pl330_dmac *dmac;
+
+ ? ? ? /* To protect channel manipulation */
+ ? ? ? spinlock_t lock;
+
+ ? ? ? /* Token of a hardware channel thread of PL330 DMAC
+ ? ? ? ?* NULL if the channel is available to be acquired.
+ ? ? ? ?*/
+ ? ? ? void *pl330_chid;
+
+ ? ? ? /* taks for cyclic capability */
+ ? ? ? struct tasklet_struct *cyclic_task;
+
+ ? ? ? bool cyclic;
+};
+
+struct dma_pl330_dmac {
+ ? ? ? struct pl330_info pif;
+
+ ? ? ? /* DMA-Engine Device */
+ ? ? ? struct dma_device ddma;
+
+ ? ? ? /* Pool of descriptors available for the DMAC's channels */
+ ? ? ? struct list_head desc_pool;
+ ? ? ? /* To protect desc_pool manipulation */
+ ? ? ? spinlock_t pool_lock;
+
+ ? ? ? /* Peripheral channels connected to this DMAC */
+ ? ? ? struct dma_pl330_chan peripherals[0]; /* keep at end */
+
+ ? ? ? struct clk *clk;
+};
+

?#endif /* __AMBA_PL330_H_ */

arch/arm/plat-samsung/dma-ops.c +4 -0

+#ifdef CONFIG_PM_RUNTIME
+	struct dma_pl330_chan *pch;
+	struct dma_pl330_dmac *pdmac;
+#endif
 
 	dma_cap_zero(mask);

Quoted 6 lines · Chanwoo Choi

arch/arm/plat-samsung/dma-ops.c +4 -0

+#ifdef CONFIG_PM_RUNTIME
+	struct dma_pl330_chan *pch;
+	struct dma_pl330_dmac *pdmac;
+#endif
 
 	dma_cap_zero(mask);

arch/arm/plat-samsung/dma-ops.c static unsigned samsung_dmadev_request(enum dma_ch dma_ch, · line 52

Quoted 10 lines

arch/arm/plat-samsung/dma-ops.c +6 -0

 	dma_cap_set(info->cap, mask);
 
 	chan = dma_request_channel(mask, pl330_filter, (void *)dma_ch);
 
+#ifdef CONFIG_PM_RUNTIME
+	pch = container_of(chan, struct dma_pl330_chan, chan);
+	pdmac = pch->dmac;
+
+	pm_runtime_get_sync(pdmac->pif.dev);
+#endif

This is not the right way to do this. The pm runtime stuff should be internal to the PL330 driver. Take a moment to think about this:

You need to wake it up just before a transfer starts. You need to put it back to sleep just after the last transfer has finished. That gives you a hint where the calls need to be.

That can only be done from within the PL330 driver.

Signed-off-by Boojin Kim <[email protected]>

Cc Vinod Koul <[email protected]>

Cc Dan Williams <[email protected]>

Signed-off-by Kukjin Kim <[email protected]>

This patch makes Samsung EXYNOS4 to use DMA PL330 driver on DMADEVICE. The EXYNOS4 uses DMA generic APIs instead of SAMSUNG specific S3C-PL330 APIs.

Signed-off-by Boojin Kim <[email protected]>

Signed-off-by Kukjin Kim <[email protected]>

---

?arch/arm/mach-exynos4/Kconfig | ? ?2 +-
?arch/arm/mach-exynos4/clock.c | ? 16 ++-
?arch/arm/mach-exynos4/dma.c ? | ?299 ++++++++++++++++++++++++++---------------

?3 files changed, 198 insertions(+), 119 deletions(-)

arch/arm/mach-exynos4/dma.c +0 -0

index 564bb53..d57d662 100644

+struct amba_device exynos4_device_pdma0 = {
+ ? ? ? .dev = {
+ ? ? ? ? ? ? ? .init_name = "dma-pl330.0",
? ? ? ? ? ? ? ?.dma_mask = &dma_dmamask,
? ? ? ? ? ? ? ?.coherent_dma_mask = DMA_BIT_MASK(32), ? ? ? ? ? ? ? ?.platform_data = &exynos4_pdma0_pdata, ? ? ? ?},
+ ? ? ? .res = {
+ ? ? ? ? ? ? ? .start = EXYNOS4_PA_PDMA0,
+ ? ? ? ? ? ? ? .end = EXYNOS4_PA_PDMA0 + SZ_4K,
+ ? ? ? ? ? ? ? .flags = IORESOURCE_MEM,
+ ? ? ? },
+ ? ? ? .irq = {IRQ_PDMA0, NO_IRQ},
+ ? ? ? .periphid = 0x00041330,
?};

+struct amba_device exynos4_device_pdma1 = {
+ ? ? ? .dev = {
+ ? ? ? ? ? ? ? .init_name = "dma-pl330.1",
? ? ? ? ? ? ? ?.dma_mask = &dma_dmamask,
? ? ? ? ? ? ? ?.coherent_dma_mask = DMA_BIT_MASK(32), ? ? ? ? ? ? ? ?.platform_data = &exynos4_pdma1_pdata, ? ? ? ?},

arch/arm/mach-exynos4/dma.c +7 -5

-};
-
-static struct platform_device *exynos4_dmacs[] __initdata = {
- ? ? ? &exynos4_device_pdma0,
- ? ? ? &exynos4_device_pdma1,
+ ? ? ? .res = {
+ ? ? ? ? ? ? ? .start = EXYNOS4_PA_PDMA1,
+ ? ? ? ? ? ? ? .end = EXYNOS4_PA_PDMA1 + SZ_4K,
+ ? ? ? ? ? ? ? .flags = IORESOURCE_MEM,
+ ? ? ? },
+ ? ? ? .irq = {IRQ_PDMA1, NO_IRQ},
+ ? ? ? .periphid = 0x00041330,

?};

?static int __init exynos4_dma_init(void)
?{
- ? ? ? platform_add_devices(exynos4_dmacs, ARRAY_SIZE(exynos4_dmacs));
+ ? ? ? amba_device_register(&exynos4_device_pdma0, &iomem_resource);

This patch makes Samsung EXYNOS4 to use DMA PL330 driver on DMADEVICE. The EXYNOS4 uses DMA generic APIs instead of SAMSUNG specific S3C-PL330 APIs.

Signed-off-by Boojin Kim <[email protected]>

Signed-off-by Kukjin Kim <[email protected]>

---

?arch/arm/mach-exynos4/Kconfig | ? ?2 +-
?arch/arm/mach-exynos4/clock.c | ? 16 ++-
?arch/arm/mach-exynos4/dma.c ? | ?299 ++++++++++++++++++++++++++---------------

?3 files changed, 198 insertions(+), 119 deletions(-)

arch/arm/mach-exynos4/dma.c +0 -0

index 564bb53..d57d662 100644

+struct dma_pl330_peri pdma0_peri[28] = {
+ ? ? ? {
+ ? ? ? ? ? ? ? .peri_id = (u8)DMACH_PCM0_RX,
+ ? ? ? ? ? ? ? .rqtype = DEVTOMEM,
+ ? ? ? }, {

Quoted 5 lines

+struct dma_pl330_peri pdma0_peri[28] = {
+ ? ? ? {
+ ? ? ? ? ? ? ? .peri_id = (u8)DMACH_PCM0_RX,
+ ? ? ? ? ? ? ? .rqtype = DEVTOMEM,
+ ? ? ? }, {

The client driver specifies the direction of the transfer (DMA_TO_DEVICE or DMA_FROM_DEVICE) when requesting for a dma channel. So, is the rqtype parameter required in the platform data for the dma driver?

This patch adds to support DMA generic API to transfer raw SPI data. Basiclly the spi driver uses DMA generic API if architecture supports it. Otherwise, uses Samsung specific S3C-PL330 APIs.

Signed-off-by Boojin Kim <[email protected]>

Acked-by Grant Likely <[email protected]>

Signed-off-by Kukjin Kim <[email protected]>

---
?drivers/spi/spi_s3c64xx.c | ?141 ++++++++++++++++++++++----------------------- ?1 files changed, 69 insertions(+), 72 deletions(-)

drivers/spi/spi_s3c64xx.c +3 -0

index 8945e20..a4cf76a 100644
@@ -172,6 +172,9 @@ struct s3c64xx_spi_driver_data {
 ? ? ? ?unsigned ? ? ? ? ? ? ? ? ? ? ? ?state;
 ? ? ? ?unsigned ? ? ? ? ? ? ? ? ? ? ? ?cur_mode, cur_bpw;
 ? ? ? ?unsigned ? ? ? ? ? ? ? ? ? ? ? ?cur_speed;
+ ? ? ? unsigned ? ? ? ? ? ? ? ? ? ? ? ?rx_ch;
+ ? ? ? unsigned ? ? ? ? ? ? ? ? ? ? ? ?tx_ch;
+ ? ? ? struct samsung_dma_ops ? ? ? ? ?*ops;
 ?};
 
 ?static struct s3c2410_dma_client s3c64xx_spi_dma_client = {

drivers/spi/spi_s3c64xx.c +37 -4

+ ? ? ? unsigned long flags;
+
+ ? ? ? spin_lock_irqsave(&sdd->lock, flags);
+
+ ? ? ? sdd->state &= ~RXBUSY;
+ ? ? ? /* If the other done */
+ ? ? ? if (!(sdd->state & TXBUSY))
+ ? ? ? ? ? ? ? complete(&sdd->xfer_completion);
+
+ ? ? ? spin_unlock_irqrestore(&sdd->lock, flags);
+}
+
+static void s3c64xx_spi_dma_txcb(void *data)
+{
+ ? ? ? struct s3c64xx_spi_driver_data *sdd
+ ? ? ? ? ? ? ? = (struct s3c64xx_spi_driver_data *)data;
+ ? ? ? unsigned long flags;
+
+ ? ? ? spin_lock_irqsave(&sdd->lock, flags);
+
+ ? ? ? sdd->state &= ~TXBUSY;
+ ? ? ? /* If the other done */
+ ? ? ? if (!(sdd->state & RXBUSY))
+ ? ? ? ? ? ? ? complete(&sdd->xfer_completion);
+
+ ? ? ? spin_unlock_irqrestore(&sdd->lock, flags);
+}
+
 ?static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct spi_device *spi,
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct spi_transfer *xfer, int dma_mode)

@@ -234,6 +269,7 @@ static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
 ? ? ? ?struct s3c64xx_spi_info *sci = sdd->cntrlr_info;
 ? ? ? ?void __iomem *regs = sdd->regs;
 ? ? ? ?u32 modecfg, chcfg;
+ ? ? ? struct samsung_dma_prep_info info;
 
 ? ? ? ?modecfg = readl(regs + S3C64XX_SPI_MODE_CFG);
 ? ? ? ?modecfg &= ~(S3C64XX_SPI_MODE_TXDMA_ON | S3C64XX_SPI_MODE_RXDMA_ON);

@@ -259,10 +295,14 @@ static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
 ? ? ? ? ? ? ? ?chcfg |= S3C64XX_SPI_CH_TXCH_ON;
 ? ? ? ? ? ? ? ?if (dma_mode) {
 ? ? ? ? ? ? ? ? ? ? ? ?modecfg |= S3C64XX_SPI_MODE_TXDMA_ON;
- ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_config(sdd->tx_dmach, sdd->cur_bpw / 8);
- ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_enqueue(sdd->tx_dmach, (void *)sdd,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? xfer->tx_dma, xfer->len);
- ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_ctrl(sdd->tx_dmach, S3C2410_DMAOP_START);
+ ? ? ? ? ? ? ? ? ? ? ? info.cap = DMA_SLAVE;
+ ? ? ? ? ? ? ? ? ? ? ? info.direction = DMA_TO_DEVICE;
+ ? ? ? ? ? ? ? ? ? ? ? info.buf = xfer->tx_dma;
+ ? ? ? ? ? ? ? ? ? ? ? info.len = xfer->len;
+ ? ? ? ? ? ? ? ? ? ? ? info.fp = s3c64xx_spi_dma_txcb;
+ ? ? ? ? ? ? ? ? ? ? ? info.fp_param = sdd;
+ ? ? ? ? ? ? ? ? ? ? ? sdd->ops->prepare(sdd->tx_ch, &info);
+ ? ? ? ? ? ? ? ? ? ? ? sdd->ops->trigger(sdd->tx_ch);
 ? ? ? ? ? ? ? ?} else {
 ? ? ? ? ? ? ? ? ? ? ? ?switch (sdd->cur_bpw) {

? ? ? ? ? ? ? ? ? ? ? ?case 32:

drivers/spi/spi_s3c64xx.c +8 -44

@@ -294,10 +334,14 @@ static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
 ? ? ? ? ? ? ? ? ? ? ? ?writel(((xfer->len * 8 / sdd->cur_bpw) & 0xffff)
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| S3C64XX_SPI_PACKET_CNT_EN,
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?regs + S3C64XX_SPI_PACKET_CNT);
- ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_config(sdd->rx_dmach, sdd->cur_bpw / 8);
- ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_enqueue(sdd->rx_dmach, (void *)sdd,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? xfer->rx_dma, xfer->len);
- ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_ctrl(sdd->rx_dmach, S3C2410_DMAOP_START);
+ ? ? ? ? ? ? ? ? ? ? ? info.cap = DMA_SLAVE;
+ ? ? ? ? ? ? ? ? ? ? ? info.direction = DMA_FROM_DEVICE;
+ ? ? ? ? ? ? ? ? ? ? ? info.buf = xfer->rx_dma;
+ ? ? ? ? ? ? ? ? ? ? ? info.len = xfer->len;
+ ? ? ? ? ? ? ? ? ? ? ? info.fp = s3c64xx_spi_dma_rxcb;
+ ? ? ? ? ? ? ? ? ? ? ? info.fp_param = sdd;
+ ? ? ? ? ? ? ? ? ? ? ? sdd->ops->prepare(sdd->rx_ch, &info);
+ ? ? ? ? ? ? ? ? ? ? ? sdd->ops->trigger(sdd->rx_ch);
 ? ? ? ? ? ? ? ?}
 ? ? ? ?}
 

@@ -483,46 +527,6 @@ static void s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd)
 ? ? ? ?}
 ?}
 
-static void s3c64xx_spi_dma_rxcb(struct s3c2410_dma_chan *chan, void *buf_id,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int size, enum s3c2410_dma_buffresult res)
-{
- ? ? ? struct s3c64xx_spi_driver_data *sdd = buf_id;
- ? ? ? unsigned long flags;
-
- ? ? ? spin_lock_irqsave(&sdd->lock, flags);
-
- ? ? ? if (res == S3C2410_RES_OK)
- ? ? ? ? ? ? ? sdd->state &= ~RXBUSY;
- ? ? ? else
- ? ? ? ? ? ? ? dev_err(&sdd->pdev->dev, "DmaAbrtRx-%d\n", size);
-
- ? ? ? /* If the other done */
- ? ? ? if (!(sdd->state & TXBUSY))
- ? ? ? ? ? ? ? complete(&sdd->xfer_completion);
-
- ? ? ? spin_unlock_irqrestore(&sdd->lock, flags);
-}
-
-static void s3c64xx_spi_dma_txcb(struct s3c2410_dma_chan *chan, void *buf_id,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int size, enum s3c2410_dma_buffresult res)
-{
- ? ? ? struct s3c64xx_spi_driver_data *sdd = buf_id;
- ? ? ? unsigned long flags;
-
- ? ? ? spin_lock_irqsave(&sdd->lock, flags);
-
- ? ? ? if (res == S3C2410_RES_OK)
- ? ? ? ? ? ? ? sdd->state &= ~TXBUSY;
- ? ? ? else
- ? ? ? ? ? ? ? dev_err(&sdd->pdev->dev, "DmaAbrtTx-%d \n", size);
-
- ? ? ? /* If the other done */
- ? ? ? if (!(sdd->state & RXBUSY))
- ? ? ? ? ? ? ? complete(&sdd->xfer_completion);
-
- ? ? ? spin_unlock_irqrestore(&sdd->lock, flags);
-}
-
 ?#define XFER_DMAADDR_INVALID DMA_BIT_MASK(32)
 
 ?static int s3c64xx_spi_map_mssg(struct s3c64xx_spi_driver_data *sdd,

@@ -697,12 +701,10 @@ static void handle_msg(struct s3c64xx_spi_driver_data *sdd,
 ? ? ? ? ? ? ? ? ? ? ? ?if (use_dma) {

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?if (xfer->tx_buf != NULL
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&& (sdd->state & TXBUSY))
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_ctrl(sdd->tx_dmach,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? S3C2410_DMAOP_FLUSH);
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sdd->ops->stop(sdd->tx_ch); ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?if (xfer->rx_buf != NULL
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&& (sdd->state & RXBUSY))
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_ctrl(sdd->rx_dmach,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? S3C2410_DMAOP_FLUSH);
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sdd->ops->stop(sdd->rx_ch); ? ? ? ? ? ? ? ? ? ? ? ?}

? ? ? ? ? ? ? ? ? ? ? ?goto out;

drivers/spi/spi_s3c64xx.c +13 -18

@@ -742,24 +744,19 @@ out:
 
 ?static int acquire_dma(struct s3c64xx_spi_driver_data *sdd)
 ?{
- ? ? ? if (s3c2410_dma_request(sdd->rx_dmach,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &s3c64xx_spi_dma_client, NULL) < 0) {
- ? ? ? ? ? ? ? dev_err(&sdd->pdev->dev, "cannot get RxDMA\n");
- ? ? ? ? ? ? ? return 0;
- ? ? ? }
- ? ? ? s3c2410_dma_set_buffdone_fn(sdd->rx_dmach, s3c64xx_spi_dma_rxcb);
- ? ? ? s3c2410_dma_devconfig(sdd->rx_dmach, S3C2410_DMASRC_HW,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sdd->sfr_start + S3C64XX_SPI_RX_DATA);
-
- ? ? ? if (s3c2410_dma_request(sdd->tx_dmach,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &s3c64xx_spi_dma_client, NULL) < 0) {
- ? ? ? ? ? ? ? dev_err(&sdd->pdev->dev, "cannot get TxDMA\n");
- ? ? ? ? ? ? ? s3c2410_dma_free(sdd->rx_dmach, &s3c64xx_spi_dma_client);
- ? ? ? ? ? ? ? return 0;
- ? ? ? }
- ? ? ? s3c2410_dma_set_buffdone_fn(sdd->tx_dmach, s3c64xx_spi_dma_txcb);
- ? ? ? s3c2410_dma_devconfig(sdd->tx_dmach, S3C2410_DMASRC_MEM,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sdd->sfr_start + S3C64XX_SPI_TX_DATA);
+
+ ? ? ? struct samsung_dma_info info;
+ ? ? ? sdd->ops = samsung_dma_get_ops();
+
+ ? ? ? info.cap = DMA_SLAVE;
+ ? ? ? info.client = &s3c64xx_spi_dma_client;
+ ? ? ? info.direction = DMA_FROM_DEVICE;
+ ? ? ? info.fifo = sdd->sfr_start + S3C64XX_SPI_RX_DATA;
+ ? ? ? info.width = sdd->cur_bpw / 8;
+ ? ? ? sdd->rx_ch = sdd->ops->request(sdd->rx_dmach, &info);
+ ? ? ? info.direction = DMA_TO_DEVICE;
+ ? ? ? info.fifo = sdd->sfr_start + S3C64XX_SPI_TX_DATA;
+ ? ? ? sdd->tx_ch = sdd->ops->request(sdd->tx_dmach, &info);
 
 ? ? ? ?return 1;
 ?}

@@ -800,8 +797,8 @@ static void s3c64xx_spi_work(struct work_struct *work)
 ? ? ? ?spin_unlock_irqrestore(&sdd->lock, flags);
 

? ? ? ?/* Free DMA channels */
- ? ? ? s3c2410_dma_free(sdd->tx_dmach, &s3c64xx_spi_dma_client);
- ? ? ? s3c2410_dma_free(sdd->rx_dmach, &s3c64xx_spi_dma_client);
+ ? ? ? sdd->ops->release(sdd->rx_ch, &s3c64xx_spi_dma_client);
+ ? ? ? sdd->ops->release(sdd->tx_ch, &s3c64xx_spi_dma_client); ?}

Quoted 28 lines

This patch adds to support DMA generic API to transfer raw SPI data. Basiclly the spi driver uses DMA generic API if architecture supports it. Otherwise, uses Samsung specific S3C-PL330 APIs.

Signed-off-by Boojin Kim <[email protected]>

Acked-by Grant Likely <[email protected]>

Signed-off-by Kukjin Kim <[email protected]>

---
 drivers/spi/spi_s3c64xx.c |  141

++++++++++++++++++++++----------------------- 1 files changed, 69 insertions(+), 72 deletions(-)

drivers/spi/spi_s3c64xx.c +3 -0

index 8945e20..a4cf76a 100644
@@ -172,6 +172,9 @@ struct s3c64xx_spi_driver_data {
       unsigned                        state;
       unsigned                        cur_mode, cur_bpw;
       unsigned                        cur_speed;
+       unsigned                        rx_ch;
+       unsigned                        tx_ch;
+       struct samsung_dma_ops          *ops;
 };
 
 static struct s3c2410_dma_client s3c64xx_spi_dma_client = {

drivers/spi/spi_s3c64xx.c static void flush_fifo(struct · line 236

Quoted 7 lines

drivers/spi/spi_s3c64xx.c +4 -0

@@ -227,6 +230,38 @@ static void flush_fifo(struct
 s3c64xx_spi_driver_data *sdd) writel(val, regs + S3C64XX_SPI_CH_CFG);
 }
 
+static void s3c64xx_spi_dma_rxcb(void *data)
+{
+       struct s3c64xx_spi_driver_data *sdd
+               = (struct s3c64xx_spi_driver_data *)data;

void* doesn't need typecasting (same for all such occurances) IIRC someone already pointed it out?

Quoted 181 lines

drivers/spi/spi_s3c64xx.c +28 -0

+       unsigned long flags;
+
+       spin_lock_irqsave(&sdd->lock, flags);
+
+       sdd->state &= ~RXBUSY;
+       /* If the other done */
+       if (!(sdd->state & TXBUSY))
+               complete(&sdd->xfer_completion);
+
+       spin_unlock_irqrestore(&sdd->lock, flags);
+}
+
+static void s3c64xx_spi_dma_txcb(void *data)
+{
+       struct s3c64xx_spi_driver_data *sdd
+               = (struct s3c64xx_spi_driver_data *)data;
+       unsigned long flags;
+
+       spin_lock_irqsave(&sdd->lock, flags);
+
+       sdd->state &= ~TXBUSY;
+       /* If the other done */
+       if (!(sdd->state & RXBUSY))
+               complete(&sdd->xfer_completion);
+
+       spin_unlock_irqrestore(&sdd->lock, flags);
+}
+
 static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
                               struct spi_device *spi,
                               struct spi_transfer *xfer, int dma_mode)

@@ -234,6 +269,7 @@ static void enable_datapath(struct

s3c64xx_spi_driver_data *sdd, struct s3c64xx_spi_info *sci =

sdd->cntrlr_info;
       void __iomem *regs = sdd->regs;
       u32 modecfg, chcfg;

+ struct samsung_dma_prep_info info;

       modecfg = readl(regs + S3C64XX_SPI_MODE_CFG);
       modecfg &= ~(S3C64XX_SPI_MODE_TXDMA_ON |
S3C64XX_SPI_MODE_RXDMA_ON); @@ -259,10 +295,14 @@ static void
enable_datapath(struct s3c64xx_spi_driver_data *sdd, chcfg |=
S3C64XX_SPI_CH_TXCH_ON;
               if (dma_mode) {
                       modecfg |= S3C64XX_SPI_MODE_TXDMA_ON;

- s3c2410_dma_config(sdd->tx_dmach, sdd->cur_bpw /
8); - s3c2410_dma_enqueue(sdd->tx_dmach, (void
*)sdd, - xfer->tx_dma,
xfer->len); - s3c2410_dma_ctrl(sdd->tx_dmach,
S3C2410_DMAOP_START); + info.cap = DMA_SLAVE;

drivers/spi/spi_s3c64xx.c +7 -0

+                       info.direction = DMA_TO_DEVICE;
+                       info.buf = xfer->tx_dma;
+                       info.len = xfer->len;
+                       info.fp = s3c64xx_spi_dma_txcb;
+                       info.fp_param = sdd;
+                       sdd->ops->prepare(sdd->tx_ch, &info);
+                       sdd->ops->trigger(sdd->tx_ch);
               } else {
                       switch (sdd->cur_bpw) {
                       case 32:

@@ -294,10 +334,14 @@ static void enable_datapath(struct

s3c64xx_spi_driver_data *sdd, writel(((xfer->len * 8 / sdd->cur_bpw) & 0xffff) | S3C64XX_SPI_PACKET_CNT_EN, regs + S3C64XX_SPI_PACKET_CNT); -

                    s3c2410_dma_config(sdd->rx_dmach, sdd->cur_bpw / 8);

drivers/spi/spi_s3c64xx.c +0 -2

-                       s3c2410_dma_enqueue(sdd->rx_dmach, (void *)sdd,
-                                               xfer->rx_dma,

xfer->len); - s3c2410_dma_ctrl(sdd->rx_dmach,
S3C2410_DMAOP_START); + info.cap = DMA_SLAVE;

drivers/spi/spi_s3c64xx.c +7 -19

+                       info.direction = DMA_FROM_DEVICE;
+                       info.buf = xfer->rx_dma;
+                       info.len = xfer->len;
+                       info.fp = s3c64xx_spi_dma_rxcb;
+                       info.fp_param = sdd;
+                       sdd->ops->prepare(sdd->rx_ch, &info);
+                       sdd->ops->trigger(sdd->rx_ch);
               }
       }

@@ -483,46 +527,6 @@ static void s3c64xx_spi_config(struct
 s3c64xx_spi_driver_data *sdd) }
 }
 
-static void s3c64xx_spi_dma_rxcb(struct s3c2410_dma_chan *chan, void
 *buf_id, -                                int size, enum
 s3c2410_dma_buffresult res) -{
-       struct s3c64xx_spi_driver_data *sdd = buf_id;
-       unsigned long flags;
-
-       spin_lock_irqsave(&sdd->lock, flags);
-
-       if (res == S3C2410_RES_OK)
-               sdd->state &= ~RXBUSY;
-       else
-               dev_err(&sdd->pdev->dev, "DmaAbrtRx-%d\n", size);
-
-       /* If the other done */
-       if (!(sdd->state & TXBUSY))
-               complete(&sdd->xfer_completion);
-
-       spin_unlock_irqrestore(&sdd->lock, flags);
-}
-
-static void s3c64xx_spi_dma_txcb(struct s3c2410_dma_chan *chan, void
 *buf_id, -                                int size, enum

s3c2410_dma_buffresult res) -{

drivers/spi/spi_s3c64xx.c +0 -19

-       struct s3c64xx_spi_driver_data *sdd = buf_id;
-       unsigned long flags;
-
-       spin_lock_irqsave(&sdd->lock, flags);
-
-       if (res == S3C2410_RES_OK)
-               sdd->state &= ~TXBUSY;
-       else
-               dev_err(&sdd->pdev->dev, "DmaAbrtTx-%d \n", size);
-
-       /* If the other done */
-       if (!(sdd->state & RXBUSY))
-               complete(&sdd->xfer_completion);
-
-       spin_unlock_irqrestore(&sdd->lock, flags);
-}
-
 #define XFER_DMAADDR_INVALID DMA_BIT_MASK(32)
 
 static int s3c64xx_spi_map_mssg(struct s3c64xx_spi_driver_data *sdd,

@@ -697,12 +701,10 @@ static void handle_msg(struct
 s3c64xx_spi_driver_data *sdd, if (use_dma) {
                               if (xfer->tx_buf != NULL
                                               && (sdd->state & TXBUSY))
-                                       s3c2410_dma_ctrl(sdd->tx_dmach,
-                                                      

S3C2410_DMAOP_FLUSH); + sdd->ops->stop(sdd->tx_ch); if (xfer->rx_buf != NULL

                                               && (sdd->state & RXBUSY))

drivers/spi/spi_s3c64xx.c +0 -2

-                                       s3c2410_dma_ctrl(sdd->rx_dmach,
-                                                      

S3C2410_DMAOP_FLUSH); +
sdd->ops->stop(sdd->rx_ch); }

                       goto out;

drivers/spi/spi_s3c64xx.c +0 -7

@@ -742,24 +744,19 @@ out:
 
 static int acquire_dma(struct s3c64xx_spi_driver_data *sdd)
 {
-       if (s3c2410_dma_request(sdd->rx_dmach,
-                                       &s3c64xx_spi_dma_client, NULL) <
 0) { -               dev_err(&sdd->pdev->dev, "cannot get RxDMA\n");
-               return 0;
-       }
-       s3c2410_dma_set_buffdone_fn(sdd->rx_dmach, s3c64xx_spi_dma_rxcb);
-       s3c2410_dma_devconfig(sdd->rx_dmach, S3C2410_DMASRC_HW,
-                                       sdd->sfr_start +

S3C64XX_SPI_RX_DATA); -

drivers/spi/spi_s3c64xx.c +0 -2

-       if (s3c2410_dma_request(sdd->tx_dmach,
-                                       &s3c64xx_spi_dma_client, NULL) <

0) { - dev_err(&sdd->pdev->dev, "cannot get TxDMA\n");

drivers/spi/spi_s3c64xx.c +0 -6

-               s3c2410_dma_free(sdd->rx_dmach, &s3c64xx_spi_dma_client);
-               return 0;
-       }
-       s3c2410_dma_set_buffdone_fn(sdd->tx_dmach, s3c64xx_spi_dma_txcb);
-       s3c2410_dma_devconfig(sdd->tx_dmach, S3C2410_DMASRC_MEM,
-                                       sdd->sfr_start +

S3C64XX_SPI_TX_DATA); +

drivers/spi/spi_s3c64xx.c +12 -0

+       struct samsung_dma_info info;
+       sdd->ops = samsung_dma_get_ops();
+
+       info.cap = DMA_SLAVE;
+       info.client = &s3c64xx_spi_dma_client;
+       info.direction = DMA_FROM_DEVICE;
+       info.fifo = sdd->sfr_start + S3C64XX_SPI_RX_DATA;
+       info.width = sdd->cur_bpw / 8;
+       sdd->rx_ch = sdd->ops->request(sdd->rx_dmach, &info);
+       info.direction = DMA_TO_DEVICE;
+       info.fifo = sdd->sfr_start + S3C64XX_SPI_TX_DATA;
+       sdd->tx_ch = sdd->ops->request(sdd->tx_dmach, &info);
 
       return 1;
 }

@@ -800,8 +797,8 @@ static void s3c64xx_spi_work(struct work_struct
 *work) spin_unlock_irqrestore(&sdd->lock, flags);
 

drivers/spi/spi_s3c64xx.c static void s3c64xx_spi_work(struct work_struct *work) · line 802

Quoted 6 lines

drivers/spi/spi_s3c64xx.c +2 -2

       /* Free DMA channels */
-       s3c2410_dma_free(sdd->tx_dmach, &s3c64xx_spi_dma_client);
-       s3c2410_dma_free(sdd->rx_dmach, &s3c64xx_spi_dma_client);
+       sdd->ops->release(sdd->rx_ch, &s3c64xx_spi_dma_client);
+       sdd->ops->release(sdd->tx_ch, &s3c64xx_spi_dma_client);
 }

Btw, this spi driver is for S3C64xx(with pl080) and S5P(with pl330) series, both of which
have DMAENGINE drivers. May be it could be directly switched to using those, rather than
via Samsung's wrapper driver.
Or am I overlooking something ?

Am Montag 08 August 2011, 19:47:58 schrieb Jassi Brar:

Quoted 6 lines

Btw, this spi driver is for S3C64xx(with pl080) and S5P(with pl330) series, both of which
have DMAENGINE drivers. May be it could be directly switched to using those, rather than
via Samsung's wrapper driver.
Or am I overlooking something ?

yep, at least the S3C2416/2450 HSSPI which should use this driver.

Quoted 8 lines

Am Montag 08 August 2011, 19:47:58 schrieb Jassi Brar:

Quoted 6 lines

Btw, this spi driver is for S3C64xx(with pl080) and S5P(with pl330) series, both of which
have DMAENGINE drivers. May be it could be directly switched to using those, rather than
via Samsung's wrapper driver.
Or am I overlooking something ?

Quoted 1 line

yep, at least the S3C2416/2450 HSSPI which should use this driver.

Seems you keep some patches private ;) the spi driver isn't enabled for 2416 in mainline.

Quoted 27 lines · Boojin Kim

This patch adds to support DMA generic API to transfer raw SPI data. Basiclly the spi driver uses DMA generic API if architecture supports it. Otherwise, uses Samsung specific S3C-PL330 APIs.

Signed-off-by Boojin Kim <[email protected]>

Acked-by Grant Likely <[email protected]>

Signed-off-by Kukjin Kim <[email protected]>

---
?drivers/spi/spi_s3c64xx.c | ?141 ++++++++++++++++++++++----------------------- ?1 files changed, 69 insertions(+), 72 deletions(-)

drivers/spi/spi_s3c64xx.c +3 -0

index 8945e20..a4cf76a 100644
@@ -172,6 +172,9 @@ struct s3c64xx_spi_driver_data {
 ? ? ? ?unsigned ? ? ? ? ? ? ? ? ? ? ? ?state;
 ? ? ? ?unsigned ? ? ? ? ? ? ? ? ? ? ? ?cur_mode, cur_bpw;
 ? ? ? ?unsigned ? ? ? ? ? ? ? ? ? ? ? ?cur_speed;
+ ? ? ? unsigned ? ? ? ? ? ? ? ? ? ? ? ?rx_ch;
+ ? ? ? unsigned ? ? ? ? ? ? ? ? ? ? ? ?tx_ch;
+ ? ? ? struct samsung_dma_ops ? ? ? ? ?*ops;
 ?};
 
 ?static struct s3c2410_dma_client s3c64xx_spi_dma_client = {

drivers/spi/spi_s3c64xx.c static void flush_fifo(struct s3c64xx_spi_driver_data *sdd) · line 236

Quoted 7 lines

drivers/spi/spi_s3c64xx.c +4 -0

@@ -227,6 +230,38 @@ static void flush_fifo(struct s3c64xx_spi_driver_data *sdd)
 ? ? ? ?writel(val, regs + S3C64XX_SPI_CH_CFG);
 ?}
 
+static void s3c64xx_spi_dma_rxcb(void *data)
+{
+ ? ? ? struct s3c64xx_spi_driver_data *sdd
+ ? ? ? ? ? ? ? = (struct s3c64xx_spi_driver_data *)data;

void* doesn't need typecasting (same for all such occurances) IIRC someone already pointed it out?

Quoted 180 lines

drivers/spi/spi_s3c64xx.c +37 -4

+ ? ? ? unsigned long flags;
+
+ ? ? ? spin_lock_irqsave(&sdd->lock, flags);
+
+ ? ? ? sdd->state &= ~RXBUSY;
+ ? ? ? /* If the other done */
+ ? ? ? if (!(sdd->state & TXBUSY))
+ ? ? ? ? ? ? ? complete(&sdd->xfer_completion);
+
+ ? ? ? spin_unlock_irqrestore(&sdd->lock, flags);
+}
+
+static void s3c64xx_spi_dma_txcb(void *data)
+{
+ ? ? ? struct s3c64xx_spi_driver_data *sdd
+ ? ? ? ? ? ? ? = (struct s3c64xx_spi_driver_data *)data;
+ ? ? ? unsigned long flags;
+
+ ? ? ? spin_lock_irqsave(&sdd->lock, flags);
+
+ ? ? ? sdd->state &= ~TXBUSY;
+ ? ? ? /* If the other done */
+ ? ? ? if (!(sdd->state & RXBUSY))
+ ? ? ? ? ? ? ? complete(&sdd->xfer_completion);
+
+ ? ? ? spin_unlock_irqrestore(&sdd->lock, flags);
+}
+
 ?static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct spi_device *spi,
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct spi_transfer *xfer, int dma_mode)

@@ -234,6 +269,7 @@ static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
 ? ? ? ?struct s3c64xx_spi_info *sci = sdd->cntrlr_info;
 ? ? ? ?void __iomem *regs = sdd->regs;
 ? ? ? ?u32 modecfg, chcfg;
+ ? ? ? struct samsung_dma_prep_info info;
 
 ? ? ? ?modecfg = readl(regs + S3C64XX_SPI_MODE_CFG);
 ? ? ? ?modecfg &= ~(S3C64XX_SPI_MODE_TXDMA_ON | S3C64XX_SPI_MODE_RXDMA_ON);

@@ -259,10 +295,14 @@ static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
 ? ? ? ? ? ? ? ?chcfg |= S3C64XX_SPI_CH_TXCH_ON;
 ? ? ? ? ? ? ? ?if (dma_mode) {
 ? ? ? ? ? ? ? ? ? ? ? ?modecfg |= S3C64XX_SPI_MODE_TXDMA_ON;
- ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_config(sdd->tx_dmach, sdd->cur_bpw / 8);
- ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_enqueue(sdd->tx_dmach, (void *)sdd,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? xfer->tx_dma, xfer->len);
- ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_ctrl(sdd->tx_dmach, S3C2410_DMAOP_START);
+ ? ? ? ? ? ? ? ? ? ? ? info.cap = DMA_SLAVE;
+ ? ? ? ? ? ? ? ? ? ? ? info.direction = DMA_TO_DEVICE;
+ ? ? ? ? ? ? ? ? ? ? ? info.buf = xfer->tx_dma;
+ ? ? ? ? ? ? ? ? ? ? ? info.len = xfer->len;
+ ? ? ? ? ? ? ? ? ? ? ? info.fp = s3c64xx_spi_dma_txcb;
+ ? ? ? ? ? ? ? ? ? ? ? info.fp_param = sdd;
+ ? ? ? ? ? ? ? ? ? ? ? sdd->ops->prepare(sdd->tx_ch, &info);
+ ? ? ? ? ? ? ? ? ? ? ? sdd->ops->trigger(sdd->tx_ch);
 ? ? ? ? ? ? ? ?} else {
 ? ? ? ? ? ? ? ? ? ? ? ?switch (sdd->cur_bpw) {

? ? ? ? ? ? ? ? ? ? ? ?case 32:

drivers/spi/spi_s3c64xx.c +8 -44

@@ -294,10 +334,14 @@ static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
 ? ? ? ? ? ? ? ? ? ? ? ?writel(((xfer->len * 8 / sdd->cur_bpw) & 0xffff)
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| S3C64XX_SPI_PACKET_CNT_EN,
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?regs + S3C64XX_SPI_PACKET_CNT);
- ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_config(sdd->rx_dmach, sdd->cur_bpw / 8);
- ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_enqueue(sdd->rx_dmach, (void *)sdd,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? xfer->rx_dma, xfer->len);
- ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_ctrl(sdd->rx_dmach, S3C2410_DMAOP_START);
+ ? ? ? ? ? ? ? ? ? ? ? info.cap = DMA_SLAVE;
+ ? ? ? ? ? ? ? ? ? ? ? info.direction = DMA_FROM_DEVICE;
+ ? ? ? ? ? ? ? ? ? ? ? info.buf = xfer->rx_dma;
+ ? ? ? ? ? ? ? ? ? ? ? info.len = xfer->len;
+ ? ? ? ? ? ? ? ? ? ? ? info.fp = s3c64xx_spi_dma_rxcb;
+ ? ? ? ? ? ? ? ? ? ? ? info.fp_param = sdd;
+ ? ? ? ? ? ? ? ? ? ? ? sdd->ops->prepare(sdd->rx_ch, &info);
+ ? ? ? ? ? ? ? ? ? ? ? sdd->ops->trigger(sdd->rx_ch);
 ? ? ? ? ? ? ? ?}
 ? ? ? ?}
 

@@ -483,46 +527,6 @@ static void s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd)
 ? ? ? ?}
 ?}
 
-static void s3c64xx_spi_dma_rxcb(struct s3c2410_dma_chan *chan, void *buf_id,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int size, enum s3c2410_dma_buffresult res)
-{
- ? ? ? struct s3c64xx_spi_driver_data *sdd = buf_id;
- ? ? ? unsigned long flags;
-
- ? ? ? spin_lock_irqsave(&sdd->lock, flags);
-
- ? ? ? if (res == S3C2410_RES_OK)
- ? ? ? ? ? ? ? sdd->state &= ~RXBUSY;
- ? ? ? else
- ? ? ? ? ? ? ? dev_err(&sdd->pdev->dev, "DmaAbrtRx-%d\n", size);
-
- ? ? ? /* If the other done */
- ? ? ? if (!(sdd->state & TXBUSY))
- ? ? ? ? ? ? ? complete(&sdd->xfer_completion);
-
- ? ? ? spin_unlock_irqrestore(&sdd->lock, flags);
-}
-
-static void s3c64xx_spi_dma_txcb(struct s3c2410_dma_chan *chan, void *buf_id,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int size, enum s3c2410_dma_buffresult res)
-{
- ? ? ? struct s3c64xx_spi_driver_data *sdd = buf_id;
- ? ? ? unsigned long flags;
-
- ? ? ? spin_lock_irqsave(&sdd->lock, flags);
-
- ? ? ? if (res == S3C2410_RES_OK)
- ? ? ? ? ? ? ? sdd->state &= ~TXBUSY;
- ? ? ? else
- ? ? ? ? ? ? ? dev_err(&sdd->pdev->dev, "DmaAbrtTx-%d \n", size);
-
- ? ? ? /* If the other done */
- ? ? ? if (!(sdd->state & RXBUSY))
- ? ? ? ? ? ? ? complete(&sdd->xfer_completion);
-
- ? ? ? spin_unlock_irqrestore(&sdd->lock, flags);
-}
-
 ?#define XFER_DMAADDR_INVALID DMA_BIT_MASK(32)
 
 ?static int s3c64xx_spi_map_mssg(struct s3c64xx_spi_driver_data *sdd,

@@ -697,12 +701,10 @@ static void handle_msg(struct s3c64xx_spi_driver_data *sdd,
 ? ? ? ? ? ? ? ? ? ? ? ?if (use_dma) {

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?if (xfer->tx_buf != NULL
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&& (sdd->state & TXBUSY))
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_ctrl(sdd->tx_dmach,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? S3C2410_DMAOP_FLUSH);
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sdd->ops->stop(sdd->tx_ch); ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?if (xfer->rx_buf != NULL
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&& (sdd->state & RXBUSY))
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_ctrl(sdd->rx_dmach,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? S3C2410_DMAOP_FLUSH);
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sdd->ops->stop(sdd->rx_ch); ? ? ? ? ? ? ? ? ? ? ? ?}

? ? ? ? ? ? ? ? ? ? ? ?goto out;

drivers/spi/spi_s3c64xx.c +13 -18

@@ -742,24 +744,19 @@ out:
 
 ?static int acquire_dma(struct s3c64xx_spi_driver_data *sdd)
 ?{
- ? ? ? if (s3c2410_dma_request(sdd->rx_dmach,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &s3c64xx_spi_dma_client, NULL) < 0) {
- ? ? ? ? ? ? ? dev_err(&sdd->pdev->dev, "cannot get RxDMA\n");
- ? ? ? ? ? ? ? return 0;
- ? ? ? }
- ? ? ? s3c2410_dma_set_buffdone_fn(sdd->rx_dmach, s3c64xx_spi_dma_rxcb);
- ? ? ? s3c2410_dma_devconfig(sdd->rx_dmach, S3C2410_DMASRC_HW,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sdd->sfr_start + S3C64XX_SPI_RX_DATA);
-
- ? ? ? if (s3c2410_dma_request(sdd->tx_dmach,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &s3c64xx_spi_dma_client, NULL) < 0) {
- ? ? ? ? ? ? ? dev_err(&sdd->pdev->dev, "cannot get TxDMA\n");
- ? ? ? ? ? ? ? s3c2410_dma_free(sdd->rx_dmach, &s3c64xx_spi_dma_client);
- ? ? ? ? ? ? ? return 0;
- ? ? ? }
- ? ? ? s3c2410_dma_set_buffdone_fn(sdd->tx_dmach, s3c64xx_spi_dma_txcb);
- ? ? ? s3c2410_dma_devconfig(sdd->tx_dmach, S3C2410_DMASRC_MEM,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sdd->sfr_start + S3C64XX_SPI_TX_DATA);
+
+ ? ? ? struct samsung_dma_info info;
+ ? ? ? sdd->ops = samsung_dma_get_ops();
+
+ ? ? ? info.cap = DMA_SLAVE;
+ ? ? ? info.client = &s3c64xx_spi_dma_client;
+ ? ? ? info.direction = DMA_FROM_DEVICE;
+ ? ? ? info.fifo = sdd->sfr_start + S3C64XX_SPI_RX_DATA;
+ ? ? ? info.width = sdd->cur_bpw / 8;
+ ? ? ? sdd->rx_ch = sdd->ops->request(sdd->rx_dmach, &info);
+ ? ? ? info.direction = DMA_TO_DEVICE;
+ ? ? ? info.fifo = sdd->sfr_start + S3C64XX_SPI_TX_DATA;
+ ? ? ? sdd->tx_ch = sdd->ops->request(sdd->tx_dmach, &info);
 
 ? ? ? ?return 1;
 ?}

@@ -800,8 +797,8 @@ static void s3c64xx_spi_work(struct work_struct *work)
 ? ? ? ?spin_unlock_irqrestore(&sdd->lock, flags);
 

Quoted 6 lines

? ? ? ?/* Free DMA channels */
- ? ? ? s3c2410_dma_free(sdd->tx_dmach, &s3c64xx_spi_dma_client);
- ? ? ? s3c2410_dma_free(sdd->rx_dmach, &s3c64xx_spi_dma_client);
+ ? ? ? sdd->ops->release(sdd->rx_ch, &s3c64xx_spi_dma_client);
+ ? ? ? sdd->ops->release(sdd->tx_ch, &s3c64xx_spi_dma_client); ?}

Btw, this spi driver is for S3C64xx(with pl080) and S5P(with pl330) series, both of which
have DMAENGINE drivers. May be it could be directly switched to using those, rather than
via Samsung's wrapper driver.
Or am I overlooking something ?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 219 lines · Jassi Brar

Quoted 27 lines · Boojin Kim

This patch adds to support DMA generic API to transfer raw SPI data. Basiclly the spi driver uses DMA generic API if architecture supports it. Otherwise, uses Samsung specific S3C-PL330 APIs.

Signed-off-by Boojin Kim <[email protected]>

Acked-by Grant Likely <[email protected]>

Signed-off-by Kukjin Kim <[email protected]>

---
?drivers/spi/spi_s3c64xx.c | ?141 ++++++++++++++++++++++----------------------- ?1 files changed, 69 insertions(+), 72 deletions(-)

drivers/spi/spi_s3c64xx.c +3 -0

index 8945e20..a4cf76a 100644
@@ -172,6 +172,9 @@ struct s3c64xx_spi_driver_data {
 ? ? ? ?unsigned ? ? ? ? ? ? ? ? ? ? ? ?state;
 ? ? ? ?unsigned ? ? ? ? ? ? ? ? ? ? ? ?cur_mode, cur_bpw;
 ? ? ? ?unsigned ? ? ? ? ? ? ? ? ? ? ? ?cur_speed;
+ ? ? ? unsigned ? ? ? ? ? ? ? ? ? ? ? ?rx_ch;
+ ? ? ? unsigned ? ? ? ? ? ? ? ? ? ? ? ?tx_ch;
+ ? ? ? struct samsung_dma_ops ? ? ? ? ?*ops;
 ?};
 
 ?static struct s3c2410_dma_client s3c64xx_spi_dma_client = {

drivers/spi/spi_s3c64xx.c static void flush_fifo(struct s3c64xx_spi_driver_data *sdd) · line 236

Quoted 7 lines

drivers/spi/spi_s3c64xx.c +4 -0

@@ -227,6 +230,38 @@ static void flush_fifo(struct s3c64xx_spi_driver_data *sdd)
 ? ? ? ?writel(val, regs + S3C64XX_SPI_CH_CFG);
 ?}
 
+static void s3c64xx_spi_dma_rxcb(void *data)
+{
+ ? ? ? struct s3c64xx_spi_driver_data *sdd
+ ? ? ? ? ? ? ? = (struct s3c64xx_spi_driver_data *)data;

void* doesn't need typecasting (same for all such occurances) IIRC someone already pointed it out?

Quoted 180 lines

drivers/spi/spi_s3c64xx.c +37 -4

+ ? ? ? unsigned long flags;
+
+ ? ? ? spin_lock_irqsave(&sdd->lock, flags);
+
+ ? ? ? sdd->state &= ~RXBUSY;
+ ? ? ? /* If the other done */
+ ? ? ? if (!(sdd->state & TXBUSY))
+ ? ? ? ? ? ? ? complete(&sdd->xfer_completion);
+
+ ? ? ? spin_unlock_irqrestore(&sdd->lock, flags);
+}
+
+static void s3c64xx_spi_dma_txcb(void *data)
+{
+ ? ? ? struct s3c64xx_spi_driver_data *sdd
+ ? ? ? ? ? ? ? = (struct s3c64xx_spi_driver_data *)data;
+ ? ? ? unsigned long flags;
+
+ ? ? ? spin_lock_irqsave(&sdd->lock, flags);
+
+ ? ? ? sdd->state &= ~TXBUSY;
+ ? ? ? /* If the other done */
+ ? ? ? if (!(sdd->state & RXBUSY))
+ ? ? ? ? ? ? ? complete(&sdd->xfer_completion);
+
+ ? ? ? spin_unlock_irqrestore(&sdd->lock, flags);
+}
+
 ?static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct spi_device *spi,
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct spi_transfer *xfer, int dma_mode)

@@ -234,6 +269,7 @@ static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
 ? ? ? ?struct s3c64xx_spi_info *sci = sdd->cntrlr_info;
 ? ? ? ?void __iomem *regs = sdd->regs;
 ? ? ? ?u32 modecfg, chcfg;
+ ? ? ? struct samsung_dma_prep_info info;
 
 ? ? ? ?modecfg = readl(regs + S3C64XX_SPI_MODE_CFG);
 ? ? ? ?modecfg &= ~(S3C64XX_SPI_MODE_TXDMA_ON | S3C64XX_SPI_MODE_RXDMA_ON);

@@ -259,10 +295,14 @@ static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
 ? ? ? ? ? ? ? ?chcfg |= S3C64XX_SPI_CH_TXCH_ON;
 ? ? ? ? ? ? ? ?if (dma_mode) {
 ? ? ? ? ? ? ? ? ? ? ? ?modecfg |= S3C64XX_SPI_MODE_TXDMA_ON;
- ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_config(sdd->tx_dmach, sdd->cur_bpw / 8);
- ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_enqueue(sdd->tx_dmach, (void *)sdd,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? xfer->tx_dma, xfer->len);
- ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_ctrl(sdd->tx_dmach, S3C2410_DMAOP_START);
+ ? ? ? ? ? ? ? ? ? ? ? info.cap = DMA_SLAVE;
+ ? ? ? ? ? ? ? ? ? ? ? info.direction = DMA_TO_DEVICE;
+ ? ? ? ? ? ? ? ? ? ? ? info.buf = xfer->tx_dma;
+ ? ? ? ? ? ? ? ? ? ? ? info.len = xfer->len;
+ ? ? ? ? ? ? ? ? ? ? ? info.fp = s3c64xx_spi_dma_txcb;
+ ? ? ? ? ? ? ? ? ? ? ? info.fp_param = sdd;
+ ? ? ? ? ? ? ? ? ? ? ? sdd->ops->prepare(sdd->tx_ch, &info);
+ ? ? ? ? ? ? ? ? ? ? ? sdd->ops->trigger(sdd->tx_ch);
 ? ? ? ? ? ? ? ?} else {
 ? ? ? ? ? ? ? ? ? ? ? ?switch (sdd->cur_bpw) {

? ? ? ? ? ? ? ? ? ? ? ?case 32:

drivers/spi/spi_s3c64xx.c +8 -44

@@ -294,10 +334,14 @@ static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
 ? ? ? ? ? ? ? ? ? ? ? ?writel(((xfer->len * 8 / sdd->cur_bpw) & 0xffff)
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| S3C64XX_SPI_PACKET_CNT_EN,
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?regs + S3C64XX_SPI_PACKET_CNT);
- ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_config(sdd->rx_dmach, sdd->cur_bpw / 8);
- ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_enqueue(sdd->rx_dmach, (void *)sdd,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? xfer->rx_dma, xfer->len);
- ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_ctrl(sdd->rx_dmach, S3C2410_DMAOP_START);
+ ? ? ? ? ? ? ? ? ? ? ? info.cap = DMA_SLAVE;
+ ? ? ? ? ? ? ? ? ? ? ? info.direction = DMA_FROM_DEVICE;
+ ? ? ? ? ? ? ? ? ? ? ? info.buf = xfer->rx_dma;
+ ? ? ? ? ? ? ? ? ? ? ? info.len = xfer->len;
+ ? ? ? ? ? ? ? ? ? ? ? info.fp = s3c64xx_spi_dma_rxcb;
+ ? ? ? ? ? ? ? ? ? ? ? info.fp_param = sdd;
+ ? ? ? ? ? ? ? ? ? ? ? sdd->ops->prepare(sdd->rx_ch, &info);
+ ? ? ? ? ? ? ? ? ? ? ? sdd->ops->trigger(sdd->rx_ch);
 ? ? ? ? ? ? ? ?}
 ? ? ? ?}
 

@@ -483,46 +527,6 @@ static void s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd)
 ? ? ? ?}
 ?}
 
-static void s3c64xx_spi_dma_rxcb(struct s3c2410_dma_chan *chan, void *buf_id,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int size, enum s3c2410_dma_buffresult res)
-{
- ? ? ? struct s3c64xx_spi_driver_data *sdd = buf_id;
- ? ? ? unsigned long flags;
-
- ? ? ? spin_lock_irqsave(&sdd->lock, flags);
-
- ? ? ? if (res == S3C2410_RES_OK)
- ? ? ? ? ? ? ? sdd->state &= ~RXBUSY;
- ? ? ? else
- ? ? ? ? ? ? ? dev_err(&sdd->pdev->dev, "DmaAbrtRx-%d\n", size);
-
- ? ? ? /* If the other done */
- ? ? ? if (!(sdd->state & TXBUSY))
- ? ? ? ? ? ? ? complete(&sdd->xfer_completion);
-
- ? ? ? spin_unlock_irqrestore(&sdd->lock, flags);
-}
-
-static void s3c64xx_spi_dma_txcb(struct s3c2410_dma_chan *chan, void *buf_id,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int size, enum s3c2410_dma_buffresult res)
-{
- ? ? ? struct s3c64xx_spi_driver_data *sdd = buf_id;
- ? ? ? unsigned long flags;
-
- ? ? ? spin_lock_irqsave(&sdd->lock, flags);
-
- ? ? ? if (res == S3C2410_RES_OK)
- ? ? ? ? ? ? ? sdd->state &= ~TXBUSY;
- ? ? ? else
- ? ? ? ? ? ? ? dev_err(&sdd->pdev->dev, "DmaAbrtTx-%d \n", size);
-
- ? ? ? /* If the other done */
- ? ? ? if (!(sdd->state & RXBUSY))
- ? ? ? ? ? ? ? complete(&sdd->xfer_completion);
-
- ? ? ? spin_unlock_irqrestore(&sdd->lock, flags);
-}
-
 ?#define XFER_DMAADDR_INVALID DMA_BIT_MASK(32)
 
 ?static int s3c64xx_spi_map_mssg(struct s3c64xx_spi_driver_data *sdd,

@@ -697,12 +701,10 @@ static void handle_msg(struct s3c64xx_spi_driver_data *sdd,
 ? ? ? ? ? ? ? ? ? ? ? ?if (use_dma) {

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?if (xfer->tx_buf != NULL
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&& (sdd->state & TXBUSY))
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_ctrl(sdd->tx_dmach,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? S3C2410_DMAOP_FLUSH);
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sdd->ops->stop(sdd->tx_ch); ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?if (xfer->rx_buf != NULL
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&& (sdd->state & RXBUSY))
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? s3c2410_dma_ctrl(sdd->rx_dmach,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? S3C2410_DMAOP_FLUSH);
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sdd->ops->stop(sdd->rx_ch); ? ? ? ? ? ? ? ? ? ? ? ?}

? ? ? ? ? ? ? ? ? ? ? ?goto out;

drivers/spi/spi_s3c64xx.c +13 -18

@@ -742,24 +744,19 @@ out:
 
 ?static int acquire_dma(struct s3c64xx_spi_driver_data *sdd)
 ?{
- ? ? ? if (s3c2410_dma_request(sdd->rx_dmach,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &s3c64xx_spi_dma_client, NULL) < 0) {
- ? ? ? ? ? ? ? dev_err(&sdd->pdev->dev, "cannot get RxDMA\n");
- ? ? ? ? ? ? ? return 0;
- ? ? ? }
- ? ? ? s3c2410_dma_set_buffdone_fn(sdd->rx_dmach, s3c64xx_spi_dma_rxcb);
- ? ? ? s3c2410_dma_devconfig(sdd->rx_dmach, S3C2410_DMASRC_HW,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sdd->sfr_start + S3C64XX_SPI_RX_DATA);
-
- ? ? ? if (s3c2410_dma_request(sdd->tx_dmach,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &s3c64xx_spi_dma_client, NULL) < 0) {
- ? ? ? ? ? ? ? dev_err(&sdd->pdev->dev, "cannot get TxDMA\n");
- ? ? ? ? ? ? ? s3c2410_dma_free(sdd->rx_dmach, &s3c64xx_spi_dma_client);
- ? ? ? ? ? ? ? return 0;
- ? ? ? }
- ? ? ? s3c2410_dma_set_buffdone_fn(sdd->tx_dmach, s3c64xx_spi_dma_txcb);
- ? ? ? s3c2410_dma_devconfig(sdd->tx_dmach, S3C2410_DMASRC_MEM,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sdd->sfr_start + S3C64XX_SPI_TX_DATA);
+
+ ? ? ? struct samsung_dma_info info;
+ ? ? ? sdd->ops = samsung_dma_get_ops();
+
+ ? ? ? info.cap = DMA_SLAVE;
+ ? ? ? info.client = &s3c64xx_spi_dma_client;
+ ? ? ? info.direction = DMA_FROM_DEVICE;
+ ? ? ? info.fifo = sdd->sfr_start + S3C64XX_SPI_RX_DATA;
+ ? ? ? info.width = sdd->cur_bpw / 8;
+ ? ? ? sdd->rx_ch = sdd->ops->request(sdd->rx_dmach, &info);
+ ? ? ? info.direction = DMA_TO_DEVICE;
+ ? ? ? info.fifo = sdd->sfr_start + S3C64XX_SPI_TX_DATA;
+ ? ? ? sdd->tx_ch = sdd->ops->request(sdd->tx_dmach, &info);
 
 ? ? ? ?return 1;
 ?}

@@ -800,8 +797,8 @@ static void s3c64xx_spi_work(struct work_struct *work)
 ? ? ? ?spin_unlock_irqrestore(&sdd->lock, flags);
 

Quoted 13 lines

Quoted 6 lines

? ? ? ?/* Free DMA channels */
- ? ? ? s3c2410_dma_free(sdd->tx_dmach, &s3c64xx_spi_dma_client);
- ? ? ? s3c2410_dma_free(sdd->rx_dmach, &s3c64xx_spi_dma_client);
+ ? ? ? sdd->ops->release(sdd->rx_ch, &s3c64xx_spi_dma_client);
+ ? ? ? sdd->ops->release(sdd->tx_ch, &s3c64xx_spi_dma_client); ?}

Btw, this spi driver is for S3C64xx(with pl080) and S5P(with pl330) series, both of which
have DMAENGINE drivers. May be it could be directly switched to using those, rather than
via Samsung's wrapper driver.
Or am I overlooking something ?

Exactly, spi_s3c64xx.c (including Sound driver) can not be used with pl080 and pl330 (DMAENGINE drivers) as it is.
Recently I was trying to use PL080 DMAENGINE driver, and i was ended up using some #ifdef in the spi driver.
something like
#ifdef CONFIG_PL080
sdd->tx_dmac.bus_id = dmatx_res->name; sdd->rx_dmac.bus_id = dmarx_res->name;
#else
sdd->tx_dmac.bus_id = dmatx_res->start; sdd->tx_dmac.bus_id = dmatx_res->start;
#endif

This is because, Pl080 handle the channel as a char *(name) and pl330 expect the channel to be enum (a number).

I think the solution could be changes in the either of these drivers to follow some symmetry or we will be ending up with
two client driver for the same device or writing up some unnecessary warper code or finally using some #ifdef.

Please suggest if you guys have any other idea/approach to handle such kind of situation.

Below is the git diff of spi_s3c64xx driver using with PL080 DMAENGINE driver. This patch is against kgene's next-samsung-dma-v4
------------------------------------------------------------------------------------------------------------------

drivers/spi/spi_s3c64xx.c +0 -0

index a4cf76a..f7b9c37 100644
@@ -24,6 +24,7 @@

?#include <linux/delay.h>
?#include <linux/clk.h>
?#include <linux/dma-mapping.h>
+#include <linux/amba/pl08x.h>
?#include <linux/platform_device.h>
?#include <linux/spi/spi.h>

drivers/spi/spi_s3c64xx.c +14 -1

@@ -165,8 +166,13 @@ struct s3c64xx_spi_driver_data {
 ? ? ? ?struct work_struct ? ? ? ? ? ? ?work;
 ? ? ? ?struct list_head ? ? ? ? ? ? ? ?queue;
 ? ? ? ?spinlock_t ? ? ? ? ? ? ? ? ? ? ?lock;
+#ifdef CONFIG_DMADEV_PL080
+ ? ? ? struct pl08x_channel_data ? ? ? rx_dmach;
+ ? ? ? struct pl08x_channel_data ? ? ? tx_dmach;
+#else
 ? ? ? ?enum dma_ch ? ? ? ? ? ? ? ? ? ? rx_dmach;
 ? ? ? ?enum dma_ch ? ? ? ? ? ? ? ? ? ? tx_dmach;
+#endif
 ? ? ? ?unsigned long ? ? ? ? ? ? ? ? ? sfr_start;
 ? ? ? ?struct completion ? ? ? ? ? ? ? xfer_completion;
 ? ? ? ?unsigned ? ? ? ? ? ? ? ? ? ? ? ?state;

@@ -753,10 +759,18 @@ static int acquire_dma(struct
 s3c64xx_spi_driver_data *sdd)
 ? ? ? ?info.direction = DMA_FROM_DEVICE;
 ? ? ? ?info.fifo = sdd->sfr_start + S3C64XX_SPI_RX_DATA;
 ? ? ? ?info.width = sdd->cur_bpw / 8;
+#ifdef CONFIG_DMADEV_PL080
+ ? ? ? sdd->rx_ch = sdd->ops->request(sdd->rx_dmach.bus_id, &info);
+#else
 ? ? ? ?sdd->rx_ch = sdd->ops->request(sdd->rx_dmach, &info);
+#endif
 ? ? ? ?info.direction = DMA_TO_DEVICE;
 ? ? ? ?info.fifo = sdd->sfr_start + S3C64XX_SPI_TX_DATA;
- ? ? ? sdd->tx_ch = sdd->ops->request(sdd->tx_dmach, &info);
+#ifdef CONFIG_DMADEV_PL080
+ ? ? ? sdd->tx_ch = sdd->ops->request(sdd->tx_dmach.bus_id, &info);
+#else
+ ? ? ? ? ? ? ? sdd->tx_ch = sdd->ops->request(sdd->tx_dmach, &info);
+#endif
 
 ? ? ? ?return 1;

?}

drivers/spi/spi_s3c64xx.c +0 -0

@@ -982,7 +996,6 @@ static int __init s3c64xx_spi_probe(struct
 platform_device *pdev)
 ? ? ? ?}
 

? ? ? ?/* Check for availability of necessary resource */
-
? ? ? ?dmatx_res = platform_get_resource(pdev, IORESOURCE_DMA, 0); ? ? ? ?if (dmatx_res == NULL) {
? ? ? ? ? ? ? ?dev_err(&pdev->dev, "Unable to get SPI-Tx dma resource\n");

drivers/spi/spi_s3c64xx.c +5 -1

@@ -1015,9 +1028,13 @@ static int __init s3c64xx_spi_probe(struct
 platform_device *pdev)
 ? ? ? ?sdd->cntrlr_info = sci;
 ? ? ? ?sdd->pdev = pdev;
 ? ? ? ?sdd->sfr_start = mem_res->start;
+#ifdef CONFIG_DMADEV_PL080
+ ? ? ? sdd->tx_dmach.bus_id = dmatx_res->name;
+ ? ? ? sdd->rx_dmach.bus_id = dmarx_res->name;
+#else
 ? ? ? ?sdd->tx_dmach = dmatx_res->start;
 ? ? ? ?sdd->rx_dmach = dmarx_res->start;
-
+#endif
 ? ? ? ?sdd->cur_bpw = 8;
 

? ? ? ?master->bus_num = pdev->id;

drivers/spi/spi_s3c64xx.c +0 -1

@@ -1105,7 +1122,6 @@ static int __init s3c64xx_spi_probe(struct
 platform_device *pdev)
 ? ? ? ?dev_dbg(&pdev->dev, "\tIOmem=[0x%x-0x%x]\tDMA=[Rx-%d, Tx-%d]\n",
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?mem_res->end, mem_res->start,
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sdd->rx_dmach, sdd->tx_dmach);
-
 ? ? ? ?return 0;
 

?err8:
----------------------------------------------------------------------------------------------------------------------

Regards,
Alim

Quoted 5 lines

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Exactly, spi_s3c64xx.c (including Sound driver) can not be used with pl080 and pl330 (DMAENGINE drivers) as it is.
Recently I was trying to use PL080 DMAENGINE driver, and i was ended up using some #ifdef in the spi driver.
something like
#ifdef CONFIG_PL080
sdd->tx_dmac.bus_id = dmatx_res->name; sdd->rx_dmac.bus_id = dmarx_res->name;
#else
sdd->tx_dmac.bus_id = dmatx_res->start; sdd->tx_dmac.bus_id = dmatx_res->start;
#endif

This is because, Pl080 handle the channel as a char *(name) and pl330 expect the channel to be enum (a number).

Quoted 13 lines · Alim Akhtar

Exactly, spi_s3c64xx.c (including Sound driver) can not be used with pl080 and pl330 (DMAENGINE drivers) as it is.
Recently I was trying to use PL080 DMAENGINE driver, and i was ended up using some #ifdef in the spi driver.
something like
#ifdef CONFIG_PL080
sdd->tx_dmac.bus_id = dmatx_res->name; sdd->rx_dmac.bus_id = dmarx_res->name;
#else
sdd->tx_dmac.bus_id = dmatx_res->start; sdd->tx_dmac.bus_id = dmatx_res->start;
#endif

Quoted 2 lines

This is because, Pl080 handle the channel as a char *(name) and pl330 expect the channel to be enum (a number).

The alternative is that you do as the Primecell drivers do (which was designed from the outset to be independent of the specific dmaengine), and pass the DMA filter parameters as pointers via platform data.

Sent: Tuesday, August 09, 2011 2:48 AM
To: Boojin Kim
Cc: linux-arm-kernel at lists.infradead.org; linux-samsung- soc at vger.kernel.org; Vinod Koul; Dan Williams; Kukjin Kim; Mark Brown; Grant Likely; Russell King
Subject: Re: [PATCH 12/15] spi/s3c64xx: Add support DMA engine API

Quoted 10 lines · Boojin Kim

This patch adds to support DMA generic API to transfer raw SPI data. Basiclly the spi driver uses DMA generic API if architecture supports it. Otherwise, uses Samsung specific S3C-PL330 APIs.

Signed-off-by Boojin Kim <[email protected]>

Acked-by Grant Likely <[email protected]>

Signed-off-by Kukjin Kim <[email protected]>

---
 drivers/spi/spi_s3c64xx.c |  141 ++++++++++++++++++++++-------------

----------

drivers/spi/spi_s3c64xx.c static void flush_fifo(struct

Quoted 17 lines

 1 files changed, 69 insertions(+), 72 deletions(-)

drivers/spi/spi_s3c64xx.c +3 -0

index 8945e20..a4cf76a 100644
@@ -172,6 +172,9 @@ struct s3c64xx_spi_driver_data {
       unsigned                        state;
       unsigned                        cur_mode, cur_bpw;
       unsigned                        cur_speed;
+       unsigned                        rx_ch;
+       unsigned                        tx_ch;
+       struct samsung_dma_ops          *ops;
 };
 
 static struct s3c2410_dma_client s3c64xx_spi_dma_client = {

@@ -227,6 +230,38 @@ static void flush_fifo(struct

s3c64xx_spi_driver_data *sdd)

Quoted 3 lines

drivers/spi/spi_s3c64xx.c +0 -0

       writel(val, regs + S3C64XX_SPI_CH_CFG);
 }
 

drivers/spi/spi_s3c64xx.c static void flush_fifo(struct s3c64xx_spi_driver_data *sdd) · line 236

Quoted 4 lines

drivers/spi/spi_s3c64xx.c +4 -0

+static void s3c64xx_spi_dma_rxcb(void *data)
+{
+       struct s3c64xx_spi_driver_data *sdd
+               = (struct s3c64xx_spi_driver_data *)data;

void* doesn't need typecasting (same for all such occurances) IIRC someone already pointed it out?

drivers/spi/spi_s3c64xx.c static void enable_datapath(struct

Quoted 34 lines

drivers/spi/spi_s3c64xx.c +28 -0

+       unsigned long flags;
+
+       spin_lock_irqsave(&sdd->lock, flags);
+
+       sdd->state &= ~RXBUSY;
+       /* If the other done */
+       if (!(sdd->state & TXBUSY))
+               complete(&sdd->xfer_completion);
+
+       spin_unlock_irqrestore(&sdd->lock, flags);
+}
+
+static void s3c64xx_spi_dma_txcb(void *data)
+{
+       struct s3c64xx_spi_driver_data *sdd
+               = (struct s3c64xx_spi_driver_data *)data;
+       unsigned long flags;
+
+       spin_lock_irqsave(&sdd->lock, flags);
+
+       sdd->state &= ~TXBUSY;
+       /* If the other done */
+       if (!(sdd->state & RXBUSY))
+               complete(&sdd->xfer_completion);
+
+       spin_unlock_irqrestore(&sdd->lock, flags);
+}
+
 static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
                               struct spi_device *spi,
                               struct spi_transfer *xfer, int dma_mode)

@@ -234,6 +269,7 @@ static void enable_datapath(struct

s3c64xx_spi_driver_data *sdd,

drivers/spi/spi_s3c64xx.c static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,

Quoted 7 lines

drivers/spi/spi_s3c64xx.c +1 -0

       struct s3c64xx_spi_info *sci = sdd->cntrlr_info;
       void __iomem *regs = sdd->regs;
       u32 modecfg, chcfg;
+       struct samsung_dma_prep_info info;
 
       modecfg = readl(regs + S3C64XX_SPI_MODE_CFG);
       modecfg &= ~(S3C64XX_SPI_MODE_TXDMA_ON |

S3C64XX_SPI_MODE_RXDMA_ON);

drivers/spi/spi_s3c64xx.c static void enable_datapath(struct

Quoted 1 line

drivers/spi/spi_s3c64xx.c +0 -0

@@ -259,10 +295,14 @@ static void enable_datapath(struct

s3c64xx_spi_driver_data *sdd,

drivers/spi/spi_s3c64xx.c static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,

Quoted 4 lines

drivers/spi/spi_s3c64xx.c +0 -1

               chcfg |= S3C64XX_SPI_CH_TXCH_ON;
               if (dma_mode) {
                       modecfg |= S3C64XX_SPI_MODE_TXDMA_ON;
-                       s3c2410_dma_config(sdd->tx_dmach, sdd->cur_bpw

/ 8);

drivers/spi/spi_s3c64xx.c static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,

Quoted 3 lines

drivers/spi/spi_s3c64xx.c +0 -3

-                       s3c2410_dma_enqueue(sdd->tx_dmach, (void *)sdd,
-                                               xfer->tx_dma, xfer->len);
-                       s3c2410_dma_ctrl(sdd->tx_dmach,

S3C2410_DMAOP_START);

drivers/spi/spi_s3c64xx.c static void enable_datapath(struct

Quoted 12 lines

drivers/spi/spi_s3c64xx.c +8 -0

+                       info.cap = DMA_SLAVE;
+                       info.direction = DMA_TO_DEVICE;
+                       info.buf = xfer->tx_dma;
+                       info.len = xfer->len;
+                       info.fp = s3c64xx_spi_dma_txcb;
+                       info.fp_param = sdd;
+                       sdd->ops->prepare(sdd->tx_ch, &info);
+                       sdd->ops->trigger(sdd->tx_ch);
               } else {
                       switch (sdd->cur_bpw) {
                       case 32:

@@ -294,10 +334,14 @@ static void enable_datapath(struct

s3c64xx_spi_driver_data *sdd,

drivers/spi/spi_s3c64xx.c static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,

Quoted 4 lines

drivers/spi/spi_s3c64xx.c +0 -1

                       writel(((xfer->len * 8 / sdd->cur_bpw) & 0xffff)
                                       | S3C64XX_SPI_PACKET_CNT_EN,
                                       regs + S3C64XX_SPI_PACKET_CNT);
-                       s3c2410_dma_config(sdd->rx_dmach, sdd->cur_bpw

/ 8);

drivers/spi/spi_s3c64xx.c static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,

Quoted 3 lines

drivers/spi/spi_s3c64xx.c +0 -3

-                       s3c2410_dma_enqueue(sdd->rx_dmach, (void *)sdd,
-                                               xfer->rx_dma, xfer->len);
-                       s3c2410_dma_ctrl(sdd->rx_dmach,

S3C2410_DMAOP_START);

drivers/spi/spi_s3c64xx.c static void s3c64xx_spi_config(struct

Quoted 12 lines

drivers/spi/spi_s3c64xx.c +8 -0

+                       info.cap = DMA_SLAVE;
+                       info.direction = DMA_FROM_DEVICE;
+                       info.buf = xfer->rx_dma;
+                       info.len = xfer->len;
+                       info.fp = s3c64xx_spi_dma_rxcb;
+                       info.fp_param = sdd;
+                       sdd->ops->prepare(sdd->rx_ch, &info);
+                       sdd->ops->trigger(sdd->rx_ch);
               }
       }
 

@@ -483,46 +527,6 @@ static void s3c64xx_spi_config(struct

s3c64xx_spi_driver_data *sdd)

drivers/spi/spi_s3c64xx.c static void s3c64xx_spi_config(struct · line 486

Quoted 4 lines

drivers/spi/spi_s3c64xx.c +0 -1

       }
 }
 
-static void s3c64xx_spi_dma_rxcb(struct s3c2410_dma_chan *chan,

void *buf_id,

drivers/spi/spi_s3c64xx.c static void s3c64xx_spi_config(struct · line 487

Quoted 1 line

drivers/spi/spi_s3c64xx.c +0 -1

-                                int size, enum s3c2410_dma_buffresult

res)

Quoted 9 lines

drivers/spi/spi_s3c64xx.c +0 -9

-{
-       struct s3c64xx_spi_driver_data *sdd = buf_id;
-       unsigned long flags;
-
-       spin_lock_irqsave(&sdd->lock, flags);
-
-       if (res == S3C2410_RES_OK)
-               sdd->state &= ~RXBUSY;
-       else

drivers/spi/spi_s3c64xx.c static void s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd)

Quoted 10 lines

drivers/spi/spi_s3c64xx.c +0 -10

-               dev_err(&sdd->pdev->dev, "DmaAbrtRx-%d\n", size);
-
-       /* If the other done */
-       if (!(sdd->state & TXBUSY))
-               complete(&sdd->xfer_completion);
-
-       spin_unlock_irqrestore(&sdd->lock, flags);
-}
-
-static void s3c64xx_spi_dma_txcb(struct s3c2410_dma_chan *chan,

void *buf_id,

drivers/spi/spi_s3c64xx.c static void s3c64xx_spi_config(struct · line 507

Quoted 1 line

drivers/spi/spi_s3c64xx.c +0 -1

-                                int size, enum s3c2410_dma_buffresult

res)

drivers/spi/spi_s3c64xx.c static void handle_msg(struct

Quoted 22 lines

drivers/spi/spi_s3c64xx.c +0 -18

-{
-       struct s3c64xx_spi_driver_data *sdd = buf_id;
-       unsigned long flags;
-
-       spin_lock_irqsave(&sdd->lock, flags);
-
-       if (res == S3C2410_RES_OK)
-               sdd->state &= ~TXBUSY;
-       else
-               dev_err(&sdd->pdev->dev, "DmaAbrtTx-%d \n", size);
-
-       /* If the other done */
-       if (!(sdd->state & RXBUSY))
-               complete(&sdd->xfer_completion);
-
-       spin_unlock_irqrestore(&sdd->lock, flags);
-}
-
 #define XFER_DMAADDR_INVALID DMA_BIT_MASK(32)
 
 static int s3c64xx_spi_map_mssg(struct s3c64xx_spi_driver_data *sdd,

@@ -697,12 +701,10 @@ static void handle_msg(struct

s3c64xx_spi_driver_data *sdd,

drivers/spi/spi_s3c64xx.c static void handle_msg(struct s3c64xx_spi_driver_data *sdd,

Quoted 5 lines

drivers/spi/spi_s3c64xx.c +0 -2

                       if (use_dma) {
                               if (xfer->tx_buf != NULL
                                               && (sdd->state & TXBUSY))
-                                       s3c2410_dma_ctrl(sdd->tx_dmach,
-

                                                       S3C2410_DMAOP_FLUSH);

drivers/spi/spi_s3c64xx.c static void handle_msg(struct s3c64xx_spi_driver_data *sdd,

Quoted 5 lines

drivers/spi/spi_s3c64xx.c +1 -2

+                                       sdd->ops->stop(sdd->tx_ch);
                               if (xfer->rx_buf != NULL
                                               && (sdd->state & RXBUSY))
-                                       s3c2410_dma_ctrl(sdd->rx_dmach,
-

                                                       S3C2410_DMAOP_FLUSH);

drivers/spi/spi_s3c64xx.c out: · line 746

Quoted 10 lines

drivers/spi/spi_s3c64xx.c +1 -2

+                                       sdd->ops->stop(sdd->rx_ch);
                       }
 
                       goto out;

@@ -742,24 +744,19 @@ out:
 
 static int acquire_dma(struct s3c64xx_spi_driver_data *sdd)
 {
-       if (s3c2410_dma_request(sdd->rx_dmach,
-                                       &s3c64xx_spi_dma_client, NULL) <

0) {

drivers/spi/spi_s3c64xx.c out:

Quoted 4 lines

drivers/spi/spi_s3c64xx.c +0 -4

-               dev_err(&sdd->pdev->dev, "cannot get RxDMA\n");
-               return 0;
-       }
-       s3c2410_dma_set_buffdone_fn(sdd->rx_dmach,

s3c64xx_spi_dma_rxcb);

drivers/spi/spi_s3c64xx.c out: · line 752

Quoted 2 lines

drivers/spi/spi_s3c64xx.c +0 -2

-       s3c2410_dma_devconfig(sdd->rx_dmach, S3C2410_DMASRC_HW,
-                                       sdd->sfr_start +

S3C64XX_SPI_RX_DATA);

drivers/spi/spi_s3c64xx.c out:

Quoted 3 lines

drivers/spi/spi_s3c64xx.c +0 -3

-
-       if (s3c2410_dma_request(sdd->tx_dmach,
-                                       &s3c64xx_spi_dma_client, NULL) <

0) {

drivers/spi/spi_s3c64xx.c out: · line 757

Quoted 2 lines

drivers/spi/spi_s3c64xx.c +0 -2

-               dev_err(&sdd->pdev->dev, "cannot get TxDMA\n");
-               s3c2410_dma_free(sdd->rx_dmach,

&s3c64xx_spi_dma_client);

drivers/spi/spi_s3c64xx.c out:

Quoted 3 lines

drivers/spi/spi_s3c64xx.c +0 -3

-               return 0;
-       }
-       s3c2410_dma_set_buffdone_fn(sdd->tx_dmach,

s3c64xx_spi_dma_txcb);

drivers/spi/spi_s3c64xx.c out: · line 762

Quoted 2 lines

drivers/spi/spi_s3c64xx.c +0 -2

-       s3c2410_dma_devconfig(sdd->tx_dmach, S3C2410_DMASRC_MEM,
-                                       sdd->sfr_start +

S3C64XX_SPI_TX_DATA);

drivers/spi/spi_s3c64xx.c static void s3c64xx_spi_work(struct work_struct

Quoted 17 lines

drivers/spi/spi_s3c64xx.c +13 -0

+
+       struct samsung_dma_info info;
+       sdd->ops = samsung_dma_get_ops();
+
+       info.cap = DMA_SLAVE;
+       info.client = &s3c64xx_spi_dma_client;
+       info.direction = DMA_FROM_DEVICE;
+       info.fifo = sdd->sfr_start + S3C64XX_SPI_RX_DATA;
+       info.width = sdd->cur_bpw / 8;
+       sdd->rx_ch = sdd->ops->request(sdd->rx_dmach, &info);
+       info.direction = DMA_TO_DEVICE;
+       info.fifo = sdd->sfr_start + S3C64XX_SPI_TX_DATA;
+       sdd->tx_ch = sdd->ops->request(sdd->tx_dmach, &info);
 
       return 1;
 }

@@ -800,8 +797,8 @@ static void s3c64xx_spi_work(struct work_struct

*work)

drivers/spi/spi_s3c64xx.c static void s3c64xx_spi_work(struct work_struct *work) · line 802

Quoted 8 lines

drivers/spi/spi_s3c64xx.c +2 -2

       spin_unlock_irqrestore(&sdd->lock, flags);
 
       /* Free DMA channels */
-       s3c2410_dma_free(sdd->tx_dmach, &s3c64xx_spi_dma_client);
-       s3c2410_dma_free(sdd->rx_dmach, &s3c64xx_spi_dma_client);
+       sdd->ops->release(sdd->rx_ch, &s3c64xx_spi_dma_client);
+       sdd->ops->release(sdd->tx_ch, &s3c64xx_spi_dma_client);
 }

Btw, this spi driver is for S3C64xx(with pl080) and S5P(with pl330) series, both of which
have DMAENGINE drivers. May be it could be directly switched to using those, rather than
via Samsung's wrapper driver.
Or am I overlooking something ?

This patch modifies to merge the dma control code.
Original s3c64xx spi driver has each dma control code for rx and tx channel. This patch merges these dma control codes into one.
With this patch, a dma setup function and callback function handle for both rx and tx channel.

?static void dma_enqueue(struct snd_pcm_substream *substream) ?{
? ? ? ?struct runtime_data *prtd = substream->runtime->private_data; ? ? ? ?dma_addr_t pos = prtd->dma_pos;
? ? ? ?unsigned int limit;
- ? ? ? int ret;
+ ? ? ? struct samsung_dma_prep_info dma_info;

? ? ? ?pr_debug("Entered %s\n", __func__);

- ? ? ? if (s3c_dma_has_circular())
- ? ? ? ? ? ? ? limit = (prtd->dma_end - prtd->dma_start) / prtd->dma_period;
- ? ? ? else
- ? ? ? ? ? ? ? limit = prtd->dma_limit;
+ ? ? ? limit = (prtd->dma_end - prtd->dma_start) / prtd->dma_period;

? ? ? ?pr_debug("%s: loaded %d, limit %d\n",
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?__func__, prtd->dma_loaded, limit);

- ? ? ? while (prtd->dma_loaded < limit) {
- ? ? ? ? ? ? ? unsigned long len = prtd->dma_period;
+ ? ? ? dma_info.cap = (samsung_dma_has_circular() ? DMA_CYCLIC : DMA_SLAVE);
+ ? ? ? dma_info.direction =
+ ? ? ? ? ? ? ? (substream->stream == SNDRV_PCM_STREAM_PLAYBACK
+ ? ? ? ? ? ? ? ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
+ ? ? ? dma_info.fp = audio_buffdone;
+ ? ? ? dma_info.fp_param = substream;
+ ? ? ? dma_info.period = prtd->dma_period;
+ ? ? ? dma_info.len = prtd->dma_period*limit;

+ ? ? ? while (prtd->dma_loaded < limit) {
? ? ? ? ? ? ? ?pr_debug("dma_loaded: %d\n", prtd->dma_loaded);

- ? ? ? ? ? ? ? if ((pos + len) > prtd->dma_end) {
- ? ? ? ? ? ? ? ? ? ? ? len ?= prtd->dma_end - pos;
- ? ? ? ? ? ? ? ? ? ? ? pr_debug("%s: corrected dma len %ld\n", __func__, len);
+ ? ? ? ? ? ? ? if ((pos + dma_info.period) > prtd->dma_end) {
+ ? ? ? ? ? ? ? ? ? ? ? dma_info.period ?= prtd->dma_end - pos;
+ ? ? ? ? ? ? ? ? ? ? ? pr_debug("%s: corrected dma len %ld\n",
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __func__, dma_info.period); ? ? ? ? ? ? ? ?}

- ? ? ? ? ? ? ? ret = s3c2410_dma_enqueue(prtd->params->channel,
- ? ? ? ? ? ? ? ? ? ? ? substream, pos, len);
+ ? ? ? ? ? ? ? dma_info.buf = pos;
+ ? ? ? ? ? ? ? prtd->params->ops->prepare(prtd->params->ch, &dma_info);

- ? ? ? ? ? ? ? if (ret == 0) {
- ? ? ? ? ? ? ? ? ? ? ? prtd->dma_loaded++;
- ? ? ? ? ? ? ? ? ? ? ? pos += prtd->dma_period;
- ? ? ? ? ? ? ? ? ? ? ? if (pos >= prtd->dma_end)
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pos = prtd->dma_start;
- ? ? ? ? ? ? ? } else
- ? ? ? ? ? ? ? ? ? ? ? break;
+ ? ? ? ? ? ? ? prtd->dma_loaded++;
+ ? ? ? ? ? ? ? pos += prtd->dma_period;
+ ? ? ? ? ? ? ? if (pos >= prtd->dma_end)
+ ? ? ? ? ? ? ? ? ? ? ? pos = prtd->dma_start; ? ? ? ?}

? ? ? ?prtd->dma_pos = pos;
?}

sound/soc/samsung/dma.c +1 -3

-static void audio_buffdone(struct s3c2410_dma_chan *channel,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? void *dev_id, int size,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? enum s3c2410_dma_buffresult result)
+static void audio_buffdone(void *data)

?{
- ? ? ? struct snd_pcm_substream *substream = dev_id;
- ? ? ? struct runtime_data *prtd;
+ ? ? ? struct snd_pcm_substream *substream = data;
+ ? ? ? struct runtime_data *prtd = substream->runtime->private_data;

? ? ? ?pr_debug("Entered %s\n", __func__);

- ? ? ? if (result == S3C2410_RES_ABORT || result == S3C2410_RES_ERR)
- ? ? ? ? ? ? ? return;
-
- ? ? ? prtd = substream->runtime->private_data;
+ ? ? ? if (prtd->state & ST_RUNNING) {
+ ? ? ? ? ? ? ? prtd->dma_pos += prtd->dma_period;
+ ? ? ? ? ? ? ? if (prtd->dma_pos >= prtd->dma_end)
+ ? ? ? ? ? ? ? ? ? ? ? prtd->dma_pos = prtd->dma_start;

- ? ? ? if (substream)
- ? ? ? ? ? ? ? snd_pcm_period_elapsed(substream);
+ ? ? ? ? ? ? ? if (substream)
+ ? ? ? ? ? ? ? ? ? ? ? snd_pcm_period_elapsed(substream);

- ? ? ? spin_lock(&prtd->lock);
- ? ? ? if (prtd->state & ST_RUNNING && !s3c_dma_has_circular()) {
- ? ? ? ? ? ? ? prtd->dma_loaded--;
- ? ? ? ? ? ? ? dma_enqueue(substream);
+ ? ? ? ? ? ? ? spin_lock(&prtd->lock);
+ ? ? ? ? ? ? ? if (!samsung_dma_has_circular()) {
+ ? ? ? ? ? ? ? ? ? ? ? prtd->dma_loaded--;
+ ? ? ? ? ? ? ? ? ? ? ? dma_enqueue(substream);
+ ? ? ? ? ? ? ? }
+ ? ? ? ? ? ? ? spin_unlock(&prtd->lock);
? ? ? ?}
-
- ? ? ? spin_unlock(&prtd->lock);
?}

(quoted diff) +0 -0

@@ -265,14 +250,14 @@ static int dma_trigger(struct snd_pcm_substream *substream, int cmd)

? ? ? ?case SNDRV_PCM_TRIGGER_RESUME:
? ? ? ?case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
? ? ? ? ? ? ? ?prtd->state |= ST_RUNNING;
- ? ? ? ? ? ? ? s3c2410_dma_ctrl(prtd->params->channel, S3C2410_DMAOP_START);
+ ? ? ? ? ? ? ? prtd->params->ops->trigger(prtd->params->ch);
? ? ? ? ? ? ? ?break;

? ? ? ?case SNDRV_PCM_TRIGGER_STOP:
? ? ? ?case SNDRV_PCM_TRIGGER_SUSPEND:
? ? ? ?case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
? ? ? ? ? ? ? ?prtd->state &= ~ST_RUNNING;
- ? ? ? ? ? ? ? s3c2410_dma_ctrl(prtd->params->channel, S3C2410_DMAOP_STOP);
+ ? ? ? ? ? ? ? prtd->params->ops->stop(prtd->params->ch);
? ? ? ? ? ? ? ?break;

(quoted diff) +2 -11

@@ -291,21 +276,12 @@ dma_pointer(struct snd_pcm_substream *substream)
 ? ? ? ?struct snd_pcm_runtime *runtime = substream->runtime;
 ? ? ? ?struct runtime_data *prtd = runtime->private_data;
 ? ? ? ?unsigned long res;
- ? ? ? dma_addr_t src, dst;
 
 ? ? ? ?pr_debug("Entered %s\n", __func__);
 
- ? ? ? spin_lock(&prtd->lock);
- ? ? ? s3c2410_dma_getposition(prtd->params->channel, &src, &dst);
-
- ? ? ? if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
- ? ? ? ? ? ? ? res = dst - prtd->dma_start;
- ? ? ? else
- ? ? ? ? ? ? ? res = src - prtd->dma_start;
-
- ? ? ? spin_unlock(&prtd->lock);
+ ? ? ? res = prtd->dma_pos - prtd->dma_start;
 
- ? ? ? pr_debug("Pointer %x %x\n", src, dst);
+ ? ? ? pr_debug("Pointer offset: %lu\n", res);
 

? ? ? ?/* we seem to be getting the odd error from the pcm library due ? ? ? ? * to out-of-bounds pointers. this is maybe due to the dma engine

Sent: Tuesday, August 09, 2011 4:28 AM
To: Boojin Kim
Cc: linux-arm-kernel at lists.infradead.org; linux-samsung- soc at vger.kernel.org; Vinod Koul; Dan Williams; Kukjin Kim; Mark Brown; Grant Likely; Russell King; Liam Girdwood
Subject: Re: [PATCH 14/15] ASoC: Samsung: Update DMA interface

Quoted 13 lines · Boojin Kim

static void dma_enqueue(struct snd_pcm_substream *substream)

 {
       struct runtime_data *prtd = substream->runtime->private_data;
       dma_addr_t pos = prtd->dma_pos;
       unsigned int limit;

sound/soc/samsung/dma.c +1 -3

-       int ret;
+       struct samsung_dma_prep_info dma_info;
 
       pr_debug("Entered %s\n", __func__);
 
-       if (s3c_dma_has_circular())
-               limit = (prtd->dma_end - prtd->dma_start) / prtd-

dma_period;

sound/soc/samsung/dma.c struct runtime_data { · line 81

Quoted 3 lines

sound/soc/samsung/dma.c +1 -2

-       else
-               limit = prtd->dma_limit;
+       limit = (prtd->dma_end - prtd->dma_start) / prtd->dma_period;

'dma_limit' is rendered useless, so you might want to remove it from 'struct runtime_data'
as well.

sound/soc/samsung/dma.c struct runtime_data {

Quoted 7 lines

       pr_debug("%s: loaded %d, limit %d\n",
                               __func__, prtd->dma_loaded, limit);

sound/soc/samsung/dma.c +1 -2

-       while (prtd->dma_loaded < limit) {
-               unsigned long len = prtd->dma_period;
+       dma_info.cap = (samsung_dma_has_circular() ? DMA_CYCLIC :

DMA_SLAVE);

Quoted 4 lines

sound/soc/samsung/dma.c +4 -0

+       dma_info.direction =
+               (substream->stream == SNDRV_PCM_STREAM_PLAYBACK
+               ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
+       dma_info.fp = audio_buffdone;

sound/soc/samsung/dma.c struct runtime_data {

Quoted 10 lines

sound/soc/samsung/dma.c +4 -3

+       dma_info.fp_param = substream;
+       dma_info.period = prtd->dma_period;
+       dma_info.len = prtd->dma_period*limit;
 
+       while (prtd->dma_loaded < limit) {
               pr_debug("dma_loaded: %d\n", prtd->dma_loaded);
 
-               if ((pos + len) > prtd->dma_end) {
-                       len  = prtd->dma_end - pos;
-                       pr_debug("%s: corrected dma len %ld\n",

__func__, len);

sound/soc/samsung/dma.c struct runtime_data {

Quoted 10 lines

sound/soc/samsung/dma.c +6 -2

+               if ((pos + dma_info.period) > prtd->dma_end) {
+                       dma_info.period  = prtd->dma_end - pos;
+                       pr_debug("%s: corrected dma len %ld\n",
+                                       __func__, dma_info.period);
               }
 
-               ret = s3c2410_dma_enqueue(prtd->params->channel,
-                       substream, pos, len);
+               dma_info.buf = pos;
+               prtd->params->ops->prepare(prtd->params->ch,

&dma_info);

Quoted 47 lines

sound/soc/samsung/dma.c +13 -22

-               if (ret == 0) {
-                       prtd->dma_loaded++;
-                       pos += prtd->dma_period;
-                       if (pos >= prtd->dma_end)
-                               pos = prtd->dma_start;
-               } else
-                       break;
+               prtd->dma_loaded++;
+               pos += prtd->dma_period;
+               if (pos >= prtd->dma_end)
+                       pos = prtd->dma_start;
       }
 
       prtd->dma_pos = pos;
 }
 
-static void audio_buffdone(struct s3c2410_dma_chan *channel,
-                               void *dev_id, int size,
-                               enum s3c2410_dma_buffresult result)
+static void audio_buffdone(void *data)
 {
-       struct snd_pcm_substream *substream = dev_id;
-       struct runtime_data *prtd;
+       struct snd_pcm_substream *substream = data;
+       struct runtime_data *prtd = substream->runtime->private_data;
 
       pr_debug("Entered %s\n", __func__);
 
-       if (result == S3C2410_RES_ABORT || result == S3C2410_RES_ERR)
-               return;
-
-       prtd = substream->runtime->private_data;
+       if (prtd->state & ST_RUNNING) {
+               prtd->dma_pos += prtd->dma_period;
+               if (prtd->dma_pos >= prtd->dma_end)
+                       prtd->dma_pos = prtd->dma_start;
 
-       if (substream)
-               snd_pcm_period_elapsed(substream);
+               if (substream)
+                       snd_pcm_period_elapsed(substream);
 
-       spin_lock(&prtd->lock);
-       if (prtd->state & ST_RUNNING && !s3c_dma_has_circular()) {
-               prtd->dma_loaded--;
-               dma_enqueue(substream);

sound/soc/samsung/dma.c struct runtime_data { · line 138

Quoted 10 lines

sound/soc/samsung/dma.c +6 -2

+               spin_lock(&prtd->lock);
+               if (!samsung_dma_has_circular()) {
+                       prtd->dma_loaded--;
+                       dma_enqueue(substream);
+               }
+               spin_unlock(&prtd->lock);
       }
-
-       spin_unlock(&prtd->lock);
 }

Since we set integer-constraint on number of periods, you could also discard bothering fractional boundaries. That would make things a lot simpler.

Review static int dma_trigger(struct

Quoted 1 line

(quoted diff) +0 -0

@@ -265,14 +250,14 @@ static int dma_trigger(struct

snd_pcm_substream *substream, int cmd)

sound/soc/samsung/dma.c static int dma_trigger(struct snd_pcm_substream *substream, int cmd)

Quoted 4 lines

sound/soc/samsung/dma.c +0 -1

       case SNDRV_PCM_TRIGGER_RESUME:
       case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
               prtd->state |= ST_RUNNING;
-               s3c2410_dma_ctrl(prtd->params->channel,

S3C2410_DMAOP_START);

sound/soc/samsung/dma.c static int dma_trigger(struct snd_pcm_substream *substream, int cmd)

Quoted 8 lines

sound/soc/samsung/dma.c +1 -1

+               prtd->params->ops->trigger(prtd->params->ch);
               break;
 
       case SNDRV_PCM_TRIGGER_STOP:
       case SNDRV_PCM_TRIGGER_SUSPEND:
       case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
               prtd->state &= ~ST_RUNNING;
-               s3c2410_dma_ctrl(prtd->params->channel,

S3C2410_DMAOP_STOP);

sound/soc/samsung/dma.c static int dma_trigger(struct snd_pcm_substream *substream, int cmd) · line 261

Quoted 2 lines

sound/soc/samsung/dma.c +1 -0

+               prtd->params->ops->stop(prtd->params->ch);
               break;

I wish you agreed and used

       prtd->params->ops->cmd(ch, enum some_cmd_options)
rather than having 4 separate callbacks
      prtd->params->ops->trigger(prtd->params->ch)
      prtd->params->ops->stop(prtd->params->ch)
     prtd->params->ops->flush(prtd->params->ch)
     prtd->params->ops->started(prtd->params->ch)

Review dma_pointer(struct snd_pcm_substream

Quoted 1 line

(quoted diff) +0 -0

@@ -291,21 +276,12 @@ dma_pointer(struct snd_pcm_substream

*substream)

Quoted 12 lines

sound/soc/samsung/dma.c +0 -6

       struct snd_pcm_runtime *runtime = substream->runtime;
       struct runtime_data *prtd = runtime->private_data;
       unsigned long res;
-       dma_addr_t src, dst;
 
       pr_debug("Entered %s\n", __func__);
 
-       spin_lock(&prtd->lock);
-       s3c2410_dma_getposition(prtd->params->channel, &src, &dst);
-
-       if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
-               res = dst - prtd->dma_start;

sound/soc/samsung/dma.c dma_pointer(struct snd_pcm_substream *substream)

Quoted 10 lines

sound/soc/samsung/dma.c +2 -5

-       else
-               res = src - prtd->dma_start;
-
-       spin_unlock(&prtd->lock);
+       res = prtd->dma_pos - prtd->dma_start;
 
-       pr_debug("Pointer %x %x\n", src, dst);
+       pr_debug("Pointer offset: %lu\n", res);
 
       /* we seem to be getting the odd error from the pcm library

due