[PATCH 12/15] spi/s3c64xx: Add support DMA engine API

[alim.akhtar@gmail.com (Alim Akhtar)]

On Tue, Aug 9, 2011 at 9:43 AM, Alim Akhtar <alim.akhtar@gmail.com> wrote:
> On Mon, Aug 8, 2011 at 11:17 PM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>> On Wed, Jul 27, 2011 at 11:01 AM, Boojin Kim <boojin.kim@samsung.com> wrote:
>>> 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 <boojin.kim@samsung.com>
>>> Acked-by: Grant Likely <grant.likely@secretlab.ca>
>>> Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
>>> ---
>>> ?drivers/spi/spi_s3c64xx.c | ?141 ++++++++++++++++++++++-----------------------
>>> ?1 files changed, 69 insertions(+), 72 deletions(-)
>>>
>>> diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
>>> index 8945e20..a4cf76a 100644
>>> --- a/drivers/spi/spi_s3c64xx.c
>>> +++ b/drivers/spi/spi_s3c64xx.c
>>> @@ -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)
>>> ? ? ? ?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?
>>
>>
>>> + ? ? ? 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:
>>> @@ -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;
>>> @@ -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);
>>> ?}
>>
>> 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
> ------------------------------------------------------------------------------------------------------------------
> diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
> index a4cf76a..f7b9c37 100644
> --- a/drivers/spi/spi_s3c64xx.c
> +++ b/drivers/spi/spi_s3c64xx.c
> @@ -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>
>
> @@ -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;
> ?}
> @@ -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");
> @@ -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;
> @@ -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:
> ----------------------------------------------------------------------------------------------------------------------
>
Hi All,
Any suggestions/comments on my concerns?

Thanks!
> Regards,
> Alim
>
>
>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>