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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Aug 9, 2011 at 9:43 AM, Alim Akhtar <alim.akhtar@xxxxxxxxx> wrote:
> On Mon, Aug 8, 2011 at 11:17 PM, Jassi Brar <jassisinghbrar@xxxxxxxxx> wrote:
>> On Wed, Jul 27, 2011 at 11:01 AM, Boojin Kim <boojin.kim@xxxxxxxxxxx> 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@xxxxxxxxxxx>
>>> Acked-by: Grant Likely <grant.likely@xxxxxxxxxxxx>
>>> Signed-off-by: Kukjin Kim <kgene.kim@xxxxxxxxxxx>
>>> ---
>>>  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@xxxxxxxxxxxxxxxxxxx
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux SoC Development]     [Linux Rockchip Development]     [Linux USB Development]     [Video for Linux]     [Linux Audio Users]     [Linux SCSI]     [Yosemite News]

  Powered by Linux