* [v7,1/2] dmaengine: 8250_mtk_dma: add MediaTek uart DMA support
From: Long Cheng @ 2018-12-25 12:06 UTC (permalink / raw)
To: Nicolas Boichat
Cc: Vinod Koul, Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee,
Sean Wang, Matthias Brugger, Dan Williams, Greg Kroah-Hartman,
Jiri Slaby, Sean Wang, dmaengine, devicetree,
linux-arm Mailing List, linux-mediatek, lkml, linux-serial,
srv_heupstream, Yingjoe Chen, YT Shen
Thanks for your comments.
On Tue, 2018-12-25 at 15:16 +0800, Nicolas Boichat wrote:
> Not a full review, a few comments below.
>
> Thanks,
>
would you like help to full review at this version patch? and then i
can modify these at next version patch. thanks.
> On Tue, Dec 25, 2018 at 9:27 AM Long Cheng <long.cheng@mediatek.com> wrote:
> >
> > In DMA engine framework, add 8250 uart dma to support MediaTek uart.
> > If MediaTek uart enabled(SERIAL_8250_MT6577), and want to improve
> > the performance, can enable the function.
> >
> > Signed-off-by: Long Cheng <long.cheng@mediatek.com>
> > ---
> > drivers/dma/mediatek/8250_mtk_dma.c | 694 +++++++++++++++++++++++++++++++++++
> > drivers/dma/mediatek/Kconfig | 11 +
> > drivers/dma/mediatek/Makefile | 1 +
> > 3 files changed, 706 insertions(+)
> > create mode 100644 drivers/dma/mediatek/8250_mtk_dma.c
> >
> > diff --git a/drivers/dma/mediatek/8250_mtk_dma.c b/drivers/dma/mediatek/8250_mtk_dma.c
> > new file mode 100644
> > index 0000000..c4090f2
> > --- /dev/null
> > +++ b/drivers/dma/mediatek/8250_mtk_dma.c
> > @@ -0,0 +1,694 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * MediaTek 8250 DMA driver.
> > + *
> > + * Copyright (c) 2018 MediaTek Inc.
> > + * Author: Long Cheng <long.cheng@mediatek.com>
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/dmaengine.h>
> > +#include <linux/dma-mapping.h>
> > +#include <linux/err.h>
> > +#include <linux/init.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/list.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of_dma.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/iopoll.h>
>
> Alphabetical order.
ok, i will order.
>
> > +
> > +#include "../virt-dma.h"
> > +
> > +#define MTK_UART_APDMA_CHANNELS (CONFIG_SERIAL_8250_NR_UARTS * 2)
> > +
> > +#define VFF_EN_B BIT(0)
> > +#define VFF_STOP_B BIT(0)
> > +#define VFF_FLUSH_B BIT(0)
> > +#define VFF_4G_SUPPORT_B BIT(0)
> > +#define VFF_RX_INT_EN0_B BIT(0) /*rx valid size >= vff thre*/
> > +#define VFF_RX_INT_EN1_B BIT(1)
> > +#define VFF_TX_INT_EN_B BIT(0) /*tx left size >= vff thre*/
> > +#define VFF_WARM_RST_B BIT(0)
> > +#define VFF_RX_INT_FLAG_CLR_B (BIT(0) | BIT(1))
> > +#define VFF_TX_INT_FLAG_CLR_B 0
> > +#define VFF_STOP_CLR_B 0
> > +#define VFF_FLUSH_CLR_B 0
> > +#define VFF_INT_EN_CLR_B 0
> > +#define VFF_4G_SUPPORT_CLR_B 0
> > +
> > +/* interrupt trigger level for tx */
> > +#define VFF_TX_THRE(n) ((n) * 7 / 8)
> > +/* interrupt trigger level for rx */
> > +#define VFF_RX_THRE(n) ((n) * 3 / 4)
> > +
> > +#define MTK_UART_APDMA_RING_SIZE 0xffffU
> > +/* invert this bit when wrap ring head again*/
> > +#define MTK_UART_APDMA_RING_WRAP 0x10000U
> > +
> > +#define VFF_INT_FLAG 0x00
> > +#define VFF_INT_EN 0x04
> > +#define VFF_EN 0x08
> > +#define VFF_RST 0x0c
> > +#define VFF_STOP 0x10
> > +#define VFF_FLUSH 0x14
> > +#define VFF_ADDR 0x1c
> > +#define VFF_LEN 0x24
> > +#define VFF_THRE 0x28
> > +#define VFF_WPT 0x2c
> > +#define VFF_RPT 0x30
> > +/*TX: the buffer size HW can read. RX: the buffer size SW can read.*/
> > +#define VFF_VALID_SIZE 0x3c
> > +/*TX: the buffer size SW can write. RX: the buffer size HW can write.*/
> > +#define VFF_LEFT_SIZE 0x40
> > +#define VFF_DEBUG_STATUS 0x50
> > +#define VFF_4G_SUPPORT 0x54
> > +
> > +struct mtk_uart_apdmadev {
> > + struct dma_device ddev;
> > + struct clk *clk;
> > + bool support_33bits;
> > + unsigned int dma_irq[MTK_UART_APDMA_CHANNELS];
> > +};
> > +
> > +struct mtk_uart_apdma_desc {
> > + struct virt_dma_desc vd;
> > +
> > + unsigned int avail_len;
> > +};
> > +
> > +struct mtk_chan {
> > + struct virt_dma_chan vc;
> > + struct dma_slave_config cfg;
> > + void __iomem *base;
> > + struct mtk_uart_apdma_desc *desc;
> > +
> > + bool requested;
> > +
> > + unsigned int rx_status;
> > +};
> > +
> > +static inline struct mtk_uart_apdmadev *
> > +to_mtk_uart_apdma_dev(struct dma_device *d)
> > +{
> > + return container_of(d, struct mtk_uart_apdmadev, ddev);
> > +}
> > +
> > +static inline struct mtk_chan *to_mtk_uart_apdma_chan(struct dma_chan *c)
> > +{
> > + return container_of(c, struct mtk_chan, vc.chan);
> > +}
> > +
> > +static inline struct mtk_uart_apdma_desc *to_mtk_uart_apdma_desc
> > + (struct dma_async_tx_descriptor *t)
> > +{
> > + return container_of(t, struct mtk_uart_apdma_desc, vd.tx);
> > +}
> > +
> > +static void mtk_uart_apdma_chan_write(struct mtk_chan *c,
> > + unsigned int reg, unsigned int val)
> > +{
> > + writel(val, c->base + reg);
> > +}
> > +
> > +static unsigned int
> > +mtk_uart_apdma_chan_read(struct mtk_chan *c, unsigned int reg)
> > +{
> > + return readl(c->base + reg);
> > +}
> > +
> > +static void mtk_uart_apdma_desc_free(struct virt_dma_desc *vd)
> > +{
> > + struct dma_chan *chan = vd->tx.chan;
> > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > +
> > + kfree(c->desc);
> > + c->desc = NULL;
>
> Why? I'm afraid this may mask double-free.
>
i will remove it.
> > +}
> > +
> > +static void mtk_uart_apdma_start_tx(struct mtk_chan *c)
> > +{
> > + unsigned int txcount = c->desc->avail_len;
>
> Variable name is confusing... I'd rather have a boolean `transmitted`
> (or something), set to 0 here, and set to 1 in the loop (but again,
> maybe this is not necessary at all, see below).
ok, i modify the function and remove it.
>
> > + unsigned int len, send, left, wpt, wrap;
> > +
> > + if (mtk_uart_apdma_chan_read(c, VFF_LEFT_SIZE) == 0U) {
> > + mtk_uart_apdma_chan_write(c, VFF_INT_EN, VFF_TX_INT_EN_B);
>
> I'd add a "return;" here and de-indent the rest of the function (that
> should help to make it more readable).
add
>
> > + } else {
> > + len = mtk_uart_apdma_chan_read(c, VFF_LEN);
>
> You make an assumption below that len is always a power of 2. I wonder
> if we want to validate that with some BUG/WARN call.
>
> Also, I assume VFF_LEN is constant? Maybe we can read it once only in
> the probe function?
>
no, the value of VFF_LEN isn't constant.
> > +
> > + while (((left = mtk_uart_apdma_chan_read(c,
> > + VFF_LEFT_SIZE)) > 0U)
> > + && (c->desc->avail_len != 0U)) {
>
> Why do we need a while loop here? LEFT_SIZE will always be smaller
> than LEN, right? Shouldn't we load as much as we can, then let an
> interrupt happen and reload the FIFO at that stage? Or does this help
> gain a bit of extra performance?
i will remove while, use 'if' instead of.
>
> > + send = min_t(unsigned int, left, c->desc->avail_len);
> > + wpt = mtk_uart_apdma_chan_read(c, VFF_WPT);
> > + wrap = wpt & MTK_UART_APDMA_RING_WRAP ?
>
> wrap = wrp ^ MTK_UART_APDMA_RING_WRAP ?
>
wrap = wpt & MTK_UART_APDMA_RING_WRAP ?
i had test it.
> > + 0U : MTK_UART_APDMA_RING_WRAP;
> > +
> > + if ((wpt & (len - 1U)) + send < len)
> > + mtk_uart_apdma_chan_write(c,
> > + VFF_WPT, wpt + send);
> > + else
> > + mtk_uart_apdma_chan_write(c, VFF_WPT,
> > + ((wpt + send) & (len - 1U))
> > + | wrap);
> > +
> > + c->desc->avail_len -= send;
> > + }
> > +
> > + if (txcount != c->desc->avail_len) {
>
> We know mtk_uart_apdma_chan_read(c, VFF_LEFT_SIZE) > 0 at least once,
> so this test should always be true.
>
> Unless we have c->desc->avail_len == 0, but then we won't even call
> this function... So this is always true.
>
yes. you are right. i will remove it.
> > + mtk_uart_apdma_chan_write(c,
> > + VFF_INT_EN, VFF_TX_INT_EN_B);
> > + if (mtk_uart_apdma_chan_read(c,
> > + VFF_FLUSH) == 0U)
> > + mtk_uart_apdma_chan_write(c,
> > + VFF_FLUSH, VFF_FLUSH_B);
> > + }
> > + }
> > +}
> > +
> > +static void mtk_uart_apdma_start_rx(struct mtk_chan *c)
> > +{
> > + struct mtk_uart_apdma_desc *d = c->desc;
> > + unsigned int rx_len, wg, rg, count;
> > +
> > + if (mtk_uart_apdma_chan_read(c, VFF_VALID_SIZE) == 0U)
> > + return;
> > +
> > + if (d && vchan_next_desc(&c->vc)) {
> > + rx_len = mtk_uart_apdma_chan_read(c, VFF_LEN);
> > + rg = mtk_uart_apdma_chan_read(c, VFF_RPT);
> > + wg = mtk_uart_apdma_chan_read(c, VFF_WPT);
> > + count = ((rg ^ wg) & MTK_UART_APDMA_RING_WRAP) ?
> > + ((wg & MTK_UART_APDMA_RING_SIZE) +
> > + rx_len - (rg & MTK_UART_APDMA_RING_SIZE)) :
> > + ((wg & MTK_UART_APDMA_RING_SIZE) -
> > + (rg & MTK_UART_APDMA_RING_SIZE));
>
> This is hard to read, please use a if/else.
>
ok.
> > +
> > + c->rx_status = count;
> > + mtk_uart_apdma_chan_write(c, VFF_RPT, wg);
> > +
> > + list_del(&d->vd.node);
> > + vchan_cookie_complete(&d->vd);
> > + }
> > +}
> > +
> > +static irqreturn_t mtk_uart_apdma_irq_handler(int irq, void *dev_id)
> > +{
> > + struct dma_chan *chan = (struct dma_chan *)dev_id;
> > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > + struct mtk_uart_apdma_desc *d;
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&c->vc.lock, flags);
> > + switch (c->cfg.direction) {
> > + case DMA_DEV_TO_MEM:
>
> You use an if/else below in a similar case, please be consistent (IMHO
> if/else is a bit more compact, so I'd use that here).
>
sean wang had review it. the link
http://lists.infradead.org/pipermail/linux-mediatek/2018-December/016292.html
> > + mtk_uart_apdma_chan_write(c,
> > + VFF_INT_FLAG, VFF_RX_INT_FLAG_CLR_B);
> > + mtk_uart_apdma_start_rx(c);
> > + break;
> > + case DMA_MEM_TO_DEV:
> > + d = c->desc;
> > +
> > + mtk_uart_apdma_chan_write(c,
> > + VFF_INT_FLAG, VFF_TX_INT_FLAG_CLR_B);
> > +
> > + if (d->avail_len != 0U) {
> > + mtk_uart_apdma_start_tx(c);
> > + } else {
> > + list_del(&d->vd.node);
> > + vchan_cookie_complete(&d->vd);
> > + }
> > + break;
> > + default:
> > + break;
> > + }
> > + spin_unlock_irqrestore(&c->vc.lock, flags);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int mtk_uart_apdma_alloc_chan_resources(struct dma_chan *chan)
> > +{
> > + struct mtk_uart_apdmadev *mtkd = to_mtk_uart_apdma_dev(chan->device);
> > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > + u32 status;
> > + int ret = -EBUSY;
> > +
> > + pm_runtime_get_sync(mtkd->ddev.dev);
> > +
> > + mtk_uart_apdma_chan_write(c, VFF_ADDR, 0);
> > + mtk_uart_apdma_chan_write(c, VFF_THRE, 0);
> > + mtk_uart_apdma_chan_write(c, VFF_LEN, 0);
> > + mtk_uart_apdma_chan_write(c, VFF_RST, VFF_WARM_RST_B);
> > +
> > + ret = readx_poll_timeout(readl,
> > + c->base + VFF_EN,
> > + status, status == 0, 10, 100);
> > + if (ret) {
> > + dev_err(c->vc.chan.device->dev,
> > + "dma reset: fail, timeout\n");
> > + goto exit;
> > + }
> > +
> > + if (!c->requested) {
> > + c->requested = true;
> > + ret = request_irq(mtkd->dma_irq[chan->chan_id],
> > + mtk_uart_apdma_irq_handler,
> > + IRQF_TRIGGER_NONE,
> > + KBUILD_MODNAME, chan);
> > + if (ret < 0) {
> > + dev_err(chan->device->dev, "Can't request dma IRQ\n");
> > + return -EINVAL;
> > + }
> > + }
> > +
> > + if (mtkd->support_33bits)
> > + mtk_uart_apdma_chan_write(c,
> > + VFF_4G_SUPPORT, VFF_4G_SUPPORT_CLR_B);
> > +
> > +exit:
> > + return ret;
> > +}
> > +
> > +static void mtk_uart_apdma_free_chan_resources(struct dma_chan *chan)
> > +{
> > + struct mtk_uart_apdmadev *mtkd = to_mtk_uart_apdma_dev(chan->device);
> > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > +
> > + if (c->requested) {
> > + c->requested = false;
> > + free_irq(mtkd->dma_irq[chan->chan_id], chan);
> > + }
> > +
> > + tasklet_kill(&c->vc.task);
> > +
> > + vchan_free_chan_resources(&c->vc);
> > +
> > + pm_runtime_put_sync(mtkd->ddev.dev);
> > +}
> > +
> > +static enum dma_status mtk_uart_apdma_tx_status(struct dma_chan *chan,
> > + dma_cookie_t cookie,
> > + struct dma_tx_state *txstate)
> > +{
> > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > + enum dma_status ret;
> > + unsigned long flags;
> > +
> > + if (!txstate)
> > + return DMA_ERROR;
> > +
> > + ret = dma_cookie_status(chan, cookie, txstate);
> > + spin_lock_irqsave(&c->vc.lock, flags);
> > + if (ret == DMA_IN_PROGRESS) {
> > + c->rx_status = mtk_uart_apdma_chan_read(c, VFF_RPT)
> > + & MTK_UART_APDMA_RING_SIZE;
> > + dma_set_residue(txstate, c->rx_status);
> > + } else if (ret == DMA_COMPLETE && c->cfg.direction == DMA_DEV_TO_MEM) {
> > + dma_set_residue(txstate, c->rx_status);
> > + } else {
> > + dma_set_residue(txstate, 0);
> > + }
> > + spin_unlock_irqrestore(&c->vc.lock, flags);
> > +
> > + return ret;
> > +}
> > +
> > +/*
> > + * dmaengine_prep_slave_single will call the function. and sglen is 1.
> > + * 8250 uart using one ring buffer, and deal with one sg.
> > + */
> > +static struct dma_async_tx_descriptor *mtk_uart_apdma_prep_slave_sg
> > + (struct dma_chan *chan, struct scatterlist *sgl,
> > + unsigned int sglen, enum dma_transfer_direction dir,
> > + unsigned long tx_flags, void *context)
> > +{
> > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > + struct mtk_uart_apdma_desc *d;
> > +
> > + if ((dir != DMA_DEV_TO_MEM) &&
> > + (dir != DMA_MEM_TO_DEV)) {
> > + dev_err(chan->device->dev, "bad direction\n");
> > + return NULL;
> > + }
> > +
> > + /* Now allocate and setup the descriptor */
> > + d = kzalloc(sizeof(*d), GFP_ATOMIC);
> > + if (!d)
> > + return NULL;
> > +
> > + /* sglen is 1 */
> > + d->avail_len = sg_dma_len(sgl);
> > +
> > + return vchan_tx_prep(&c->vc, &d->vd, tx_flags);
> > +}
> > +
> > +static void mtk_uart_apdma_issue_pending(struct dma_chan *chan)
> > +{
> > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > + struct virt_dma_desc *vd;
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&c->vc.lock, flags);
> > + if (c->cfg.direction == DMA_DEV_TO_MEM) {
> > + if (vchan_issue_pending(&c->vc) && !c->desc) {
> > + vd = vchan_next_desc(&c->vc);
> > + c->desc = to_mtk_uart_apdma_desc(&vd->tx);
> > + mtk_uart_apdma_start_rx(c);
> > + }
> > + } else if (c->cfg.direction == DMA_MEM_TO_DEV) {
> > + if (vchan_issue_pending(&c->vc) && !c->desc) {
> > + vd = vchan_next_desc(&c->vc);
> > + c->desc = to_mtk_uart_apdma_desc(&vd->tx);
> > + mtk_uart_apdma_start_tx(c);
> > + }
> > + }
> > + spin_unlock_irqrestore(&c->vc.lock, flags);
> > +}
> > +
> > +static int mtk_uart_apdma_slave_config(struct dma_chan *chan,
> > + struct dma_slave_config *cfg)
> > +{
> > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > + struct mtk_uart_apdmadev *mtkd =
> > + to_mtk_uart_apdma_dev(c->vc.chan.device);
> > +
> > + c->cfg = *cfg;
> > +
> > + if (cfg->direction == DMA_DEV_TO_MEM) {
> > + unsigned int rx_len = cfg->src_addr_width * 1024;
> > +
> > + mtk_uart_apdma_chan_write(c, VFF_ADDR, cfg->src_addr);
> > + mtk_uart_apdma_chan_write(c, VFF_LEN, rx_len);
> > + mtk_uart_apdma_chan_write(c, VFF_THRE, VFF_RX_THRE(rx_len));
> > + mtk_uart_apdma_chan_write(c,
> > + VFF_INT_EN, VFF_RX_INT_EN0_B
> > + | VFF_RX_INT_EN1_B);
> > + mtk_uart_apdma_chan_write(c, VFF_RPT, 0);
> > + mtk_uart_apdma_chan_write(c,
> > + VFF_INT_FLAG, VFF_RX_INT_FLAG_CLR_B);
> > + mtk_uart_apdma_chan_write(c, VFF_EN, VFF_EN_B);
> > + } else if (cfg->direction == DMA_MEM_TO_DEV) {
> > + unsigned int tx_len = cfg->dst_addr_width * 1024;
> > +
> > + mtk_uart_apdma_chan_write(c, VFF_ADDR, cfg->dst_addr);
> > + mtk_uart_apdma_chan_write(c, VFF_LEN, tx_len);
> > + mtk_uart_apdma_chan_write(c, VFF_THRE, VFF_TX_THRE(tx_len));
> > + mtk_uart_apdma_chan_write(c, VFF_WPT, 0);
> > + mtk_uart_apdma_chan_write(c,
> > + VFF_INT_FLAG, VFF_TX_INT_FLAG_CLR_B);
> > + mtk_uart_apdma_chan_write(c, VFF_EN, VFF_EN_B);
> > + }
> > +
> > + if (mtkd->support_33bits)
> > + mtk_uart_apdma_chan_write(c, VFF_4G_SUPPORT, VFF_4G_SUPPORT_B);
> > +
> > + if (mtk_uart_apdma_chan_read(c, VFF_EN) != VFF_EN_B) {
> > + dev_err(chan->device->dev,
> > + "config dma dir[%d] fail\n", cfg->direction);
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_uart_apdma_terminate_all(struct dma_chan *chan)
> > +{
> > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > + unsigned long flags;
> > + u32 status;
> > + int ret;
> > +
> > + spin_lock_irqsave(&c->vc.lock, flags);
> > +
> > + mtk_uart_apdma_chan_write(c, VFF_FLUSH, VFF_FLUSH_CLR_B);
> > + /* Wait for flush */
> > + ret = readx_poll_timeout(readl,
> > + c->base + VFF_FLUSH,
> > + status,
> > + (status & VFF_FLUSH_B) != VFF_FLUSH_B,
> > + 10, 100);
> > + if (ret)
> > + dev_err(c->vc.chan.device->dev,
> > + "dma stop: polling FLUSH fail, DEBUG=0x%x\n",
> > + mtk_uart_apdma_chan_read(c, VFF_DEBUG_STATUS));
> > +
> > + /*set stop as 1 -> wait until en is 0 -> set stop as 0*/
> > + mtk_uart_apdma_chan_write(c, VFF_STOP, VFF_STOP_B);
> > + ret = readx_poll_timeout(readl,
> > + c->base + VFF_EN,
> > + status, status == 0, 10, 100);
> > + if (ret)
> > + dev_err(c->vc.chan.device->dev,
> > + "dma stop: polling VFF_EN fail, DEBUG=0x%x\n",
> > + mtk_uart_apdma_chan_read(c, VFF_DEBUG_STATUS));
> > +
> > + mtk_uart_apdma_chan_write(c, VFF_STOP, VFF_STOP_CLR_B);
> > + mtk_uart_apdma_chan_write(c, VFF_INT_EN, VFF_INT_EN_CLR_B);
> > +
> > + switch (c->cfg.direction) {
> > + case DMA_DEV_TO_MEM:
>
> Ditto, if/else?
>
ditto.
http://lists.infradead.org/pipermail/linux-mediatek/2018-December/016292.html
> > + mtk_uart_apdma_chan_write(c,
> > + VFF_INT_FLAG, VFF_RX_INT_FLAG_CLR_B);
> > + break;
> > + case DMA_MEM_TO_DEV:
> > + mtk_uart_apdma_chan_write(c,
> > + VFF_INT_FLAG, VFF_TX_INT_FLAG_CLR_B);
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + spin_unlock_irqrestore(&c->vc.lock, flags);
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_uart_apdma_device_pause(struct dma_chan *chan)
> > +{
> > + /* just for check caps pass */
> > + return 0;
> > +}
> > +
> > +static int mtk_uart_apdma_device_resume(struct dma_chan *chan)
> > +{
> > + /* just for check caps pass */
> > + return 0;
> > +}
> > +
> > +static void mtk_uart_apdma_free(struct mtk_uart_apdmadev *mtkd)
> > +{
> > + while (list_empty(&mtkd->ddev.channels) == 0) {
> > + struct mtk_chan *c = list_first_entry(&mtkd->ddev.channels,
> > + struct mtk_chan, vc.chan.device_node);
> > +
> > + list_del(&c->vc.chan.device_node);
> > + tasklet_kill(&c->vc.task);
> > + }
> > +}
> > +
> > +static const struct of_device_id mtk_uart_apdma_match[] = {
> > + { .compatible = "mediatek,mt6577-uart-dma", },
> > + { /* sentinel */ },
> > +};
> > +MODULE_DEVICE_TABLE(of, mtk_uart_apdma_match);
> > +
> > +static int mtk_uart_apdma_probe(struct platform_device *pdev)
> > +{
> > + struct mtk_uart_apdmadev *mtkd;
> > + struct resource *res;
> > + struct mtk_chan *c;
> > + unsigned int i;
> > + int rc;
> > +
> > + mtkd = devm_kzalloc(&pdev->dev, sizeof(*mtkd), GFP_KERNEL);
> > + if (!mtkd)
> > + return -ENOMEM;
> > +
> > + mtkd->clk = devm_clk_get(&pdev->dev, NULL);
> > + if (IS_ERR(mtkd->clk)) {
> > + dev_err(&pdev->dev, "No clock specified\n");
> > + rc = PTR_ERR(mtkd->clk);
> > + goto err_no_dma;
> > + }
> > +
> > + if (of_property_read_bool(pdev->dev.of_node, "dma-33bits")) {
> > + dev_info(&pdev->dev, "Support dma 33bits\n");
>
> Drop this, a bit verbose.
>
> > + mtkd->support_33bits = true;
> > + }
> > +
> > + rc = dma_set_mask_and_coherent(&pdev->dev,
> > + DMA_BIT_MASK(32 | mtkd->support_33bits));
> > + if (rc)
> > + goto err_no_dma;
> > +
> > + dma_cap_set(DMA_SLAVE, mtkd->ddev.cap_mask);
> > + mtkd->ddev.device_alloc_chan_resources =
> > + mtk_uart_apdma_alloc_chan_resources;
> > + mtkd->ddev.device_free_chan_resources =
> > + mtk_uart_apdma_free_chan_resources;
> > + mtkd->ddev.device_tx_status = mtk_uart_apdma_tx_status;
> > + mtkd->ddev.device_issue_pending = mtk_uart_apdma_issue_pending;
> > + mtkd->ddev.device_prep_slave_sg = mtk_uart_apdma_prep_slave_sg;
> > + mtkd->ddev.device_config = mtk_uart_apdma_slave_config;
> > + mtkd->ddev.device_pause = mtk_uart_apdma_device_pause;
> > + mtkd->ddev.device_resume = mtk_uart_apdma_device_resume;
> > + mtkd->ddev.device_terminate_all = mtk_uart_apdma_terminate_all;
> > + mtkd->ddev.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE);
> > + mtkd->ddev.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE);
> > + mtkd->ddev.directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
> > + mtkd->ddev.residue_granularity = DMA_RESIDUE_GRANULARITY_SEGMENT;
> > + mtkd->ddev.dev = &pdev->dev;
> > + INIT_LIST_HEAD(&mtkd->ddev.channels);
> > +
> > + for (i = 0; i < MTK_UART_APDMA_CHANNELS; i++) {
> > + c = devm_kzalloc(mtkd->ddev.dev, sizeof(*c), GFP_KERNEL);
> > + if (!c) {
> > + rc = -ENODEV;
> > + goto err_no_dma;
> > + }
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> > + if (!res) {
> > + rc = -ENODEV;
> > + goto err_no_dma;
> > + }
> > +
> > + c->base = devm_ioremap_resource(&pdev->dev, res);
> > + if (IS_ERR(c->base)) {
> > + rc = PTR_ERR(c->base);
> > + goto err_no_dma;
> > + }
> > + c->requested = false;
> > + c->vc.desc_free = mtk_uart_apdma_desc_free;
> > + vchan_init(&c->vc, &mtkd->ddev);
> > +
> > + mtkd->dma_irq[i] = platform_get_irq(pdev, i);
> > + if ((int)mtkd->dma_irq[i] < 0) {
> > + dev_err(&pdev->dev, "failed to get IRQ[%d]\n", i);
> > + rc = -EINVAL;
> > + goto err_no_dma;
> > + }
> > + }
> > +
> > + pm_runtime_enable(&pdev->dev);
> > + pm_runtime_set_active(&pdev->dev);
> > +
> > + rc = dma_async_device_register(&mtkd->ddev);
> > + if (rc)
> > + goto rpm_disable;
> > +
> > + platform_set_drvdata(pdev, mtkd);
> > +
> > + if (pdev->dev.of_node) {
> > + /* Device-tree DMA controller registration */
> > + rc = of_dma_controller_register(pdev->dev.of_node,
> > + of_dma_xlate_by_chan_id,
> > + mtkd);
> > + if (rc)
> > + goto dma_remove;
> > + }
> > +
> > + return rc;
> > +
> > +dma_remove:
> > + dma_async_device_unregister(&mtkd->ddev);
> > +rpm_disable:
> > + pm_runtime_disable(&pdev->dev);
> > +err_no_dma:
> > + mtk_uart_apdma_free(mtkd);
> > + return rc;
> > +}
> > +
> > +static int mtk_uart_apdma_remove(struct platform_device *pdev)
> > +{
> > + struct mtk_uart_apdmadev *mtkd = platform_get_drvdata(pdev);
> > +
> > + if (pdev->dev.of_node)
> > + of_dma_controller_free(pdev->dev.of_node);
> > +
> > + pm_runtime_disable(&pdev->dev);
> > + pm_runtime_put_noidle(&pdev->dev);
> > +
> > + dma_async_device_unregister(&mtkd->ddev);
> > + mtk_uart_apdma_free(mtkd);
> > +
> > + return 0;
> > +}
> > +
> > +#ifdef CONFIG_PM_SLEEP
> > +static int mtk_uart_apdma_suspend(struct device *dev)
> > +{
> > + struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
> > +
> > + if (!pm_runtime_suspended(dev))
> > + clk_disable_unprepare(mtkd->clk);
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_uart_apdma_resume(struct device *dev)
> > +{
> > + int ret;
> > + struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
> > +
> > + if (!pm_runtime_suspended(dev)) {
> > + ret = clk_prepare_enable(mtkd->clk);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +#endif /* CONFIG_PM_SLEEP */
> > +
> > +#ifdef CONFIG_PM
> > +static int mtk_uart_apdma_runtime_suspend(struct device *dev)
> > +{
> > + struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
> > +
> > + clk_disable_unprepare(mtkd->clk);
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_uart_apdma_runtime_resume(struct device *dev)
> > +{
> > + int ret;
> > + struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
> > +
> > + ret = clk_prepare_enable(mtkd->clk);
> > + if (ret)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +#endif /* CONFIG_PM */
> > +
> > +static const struct dev_pm_ops mtk_uart_apdma_pm_ops = {
> > + SET_SYSTEM_SLEEP_PM_OPS(mtk_uart_apdma_suspend, mtk_uart_apdma_resume)
> > + SET_RUNTIME_PM_OPS(mtk_uart_apdma_runtime_suspend,
> > + mtk_uart_apdma_runtime_resume, NULL)
> > +};
> > +
> > +static struct platform_driver mtk_uart_apdma_driver = {
> > + .probe = mtk_uart_apdma_probe,
> > + .remove = mtk_uart_apdma_remove,
> > + .driver = {
> > + .name = KBUILD_MODNAME,
> > + .pm = &mtk_uart_apdma_pm_ops,
> > + .of_match_table = of_match_ptr(mtk_uart_apdma_match),
> > + },
> > +};
> > +
> > +module_platform_driver(mtk_uart_apdma_driver);
> > +
> > +MODULE_DESCRIPTION("MediaTek UART APDMA Controller Driver");
> > +MODULE_AUTHOR("Long Cheng <long.cheng@mediatek.com>");
> > +MODULE_LICENSE("GPL v2");
> > +
> > diff --git a/drivers/dma/mediatek/Kconfig b/drivers/dma/mediatek/Kconfig
> > index 27bac0b..1a523c87 100644
> > --- a/drivers/dma/mediatek/Kconfig
> > +++ b/drivers/dma/mediatek/Kconfig
> > @@ -1,4 +1,15 @@
> >
> > +config DMA_MTK_UART
> > + tristate "MediaTek SoCs APDMA support for UART"
> > + depends on OF && SERIAL_8250_MT6577
> > + select DMA_ENGINE
> > + select DMA_VIRTUAL_CHANNELS
> > + help
> > + Support for the UART DMA engine found on MediaTek MTK SoCs.
> > + when SERIAL_8250_MT6577 is enabled, and if you want to use DMA,
> > + you can enable the config. the DMA engine can only be used
> > + with MediaTek SoCs.
> > +
> > config MTK_HSDMA
> > tristate "MediaTek High-Speed DMA controller support"
> > depends on ARCH_MEDIATEK || COMPILE_TEST
> > diff --git a/drivers/dma/mediatek/Makefile b/drivers/dma/mediatek/Makefile
> > index 6e778f8..2f2efd9 100644
> > --- a/drivers/dma/mediatek/Makefile
> > +++ b/drivers/dma/mediatek/Makefile
> > @@ -1 +1,2 @@
> > +obj-$(CONFIG_DMA_MTK_UART) += 8250_mtk_dma.o
> > obj-$(CONFIG_MTK_HSDMA) += mtk-hsdma.o
> > --
> > 1.7.9.5
> >
^ permalink raw reply
* [v7,1/2] dmaengine: 8250_mtk_dma: add MediaTek uart DMA support
From: Nicolas Boichat @ 2018-12-26 0:05 UTC (permalink / raw)
To: Long Cheng
Cc: Vinod Koul, Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee,
Sean Wang, Matthias Brugger, Dan Williams, Greg Kroah-Hartman,
Jiri Slaby, Sean Wang, dmaengine, devicetree,
linux-arm Mailing List, linux-mediatek, lkml, linux-serial,
srv_heupstream, Yingjoe Chen, YT Shen
On Tue, Dec 25, 2018 at 8:06 PM Long Cheng <long.cheng@mediatek.com> wrote:
>
> Thanks for your comments.
>
> On Tue, 2018-12-25 at 15:16 +0800, Nicolas Boichat wrote:
> > Not a full review, a few comments below.
> >
> > Thanks,
> >
>
> would you like help to full review at this version patch? and then i
> can modify these at next version patch. thanks.
Added a few more comments ,-)
> > On Tue, Dec 25, 2018 at 9:27 AM Long Cheng <long.cheng@mediatek.com> wrote:
> > >
> > > In DMA engine framework, add 8250 uart dma to support MediaTek uart.
> > > If MediaTek uart enabled(SERIAL_8250_MT6577), and want to improve
> > > the performance, can enable the function.
> > >
> > > Signed-off-by: Long Cheng <long.cheng@mediatek.com>
> > > ---
> > > drivers/dma/mediatek/8250_mtk_dma.c | 694 +++++++++++++++++++++++++++++++++++
> > > drivers/dma/mediatek/Kconfig | 11 +
> > > drivers/dma/mediatek/Makefile | 1 +
> > > 3 files changed, 706 insertions(+)
> > > create mode 100644 drivers/dma/mediatek/8250_mtk_dma.c
> > >
> > > diff --git a/drivers/dma/mediatek/8250_mtk_dma.c b/drivers/dma/mediatek/8250_mtk_dma.c
> > > new file mode 100644
> > > index 0000000..c4090f2
> > > --- /dev/null
> > > +++ b/drivers/dma/mediatek/8250_mtk_dma.c
> > > @@ -0,0 +1,694 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * MediaTek 8250 DMA driver.
> > > + *
> > > + * Copyright (c) 2018 MediaTek Inc.
> > > + * Author: Long Cheng <long.cheng@mediatek.com>
> > > + */
> > > +
> > > +#include <linux/clk.h>
> > > +#include <linux/dmaengine.h>
> > > +#include <linux/dma-mapping.h>
> > > +#include <linux/err.h>
> > > +#include <linux/init.h>
> > > +#include <linux/interrupt.h>
> > > +#include <linux/list.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of_dma.h>
> > > +#include <linux/of_device.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/spinlock.h>
> > > +#include <linux/pm_runtime.h>
> > > +#include <linux/iopoll.h>
> >
> > Alphabetical order.
>
> ok, i will order.
>
> >
> > > +
> > > +#include "../virt-dma.h"
> > > +
> > > +#define MTK_UART_APDMA_CHANNELS (CONFIG_SERIAL_8250_NR_UARTS * 2)
> > > +
> > > +#define VFF_EN_B BIT(0)
> > > +#define VFF_STOP_B BIT(0)
> > > +#define VFF_FLUSH_B BIT(0)
> > > +#define VFF_4G_SUPPORT_B BIT(0)
> > > +#define VFF_RX_INT_EN0_B BIT(0) /*rx valid size >= vff thre*/
> > > +#define VFF_RX_INT_EN1_B BIT(1)
> > > +#define VFF_TX_INT_EN_B BIT(0) /*tx left size >= vff thre*/
> > > +#define VFF_WARM_RST_B BIT(0)
> > > +#define VFF_RX_INT_FLAG_CLR_B (BIT(0) | BIT(1))
> > > +#define VFF_TX_INT_FLAG_CLR_B 0
> > > +#define VFF_STOP_CLR_B 0
> > > +#define VFF_FLUSH_CLR_B 0
> > > +#define VFF_INT_EN_CLR_B 0
> > > +#define VFF_4G_SUPPORT_CLR_B 0
> > > +
> > > +/* interrupt trigger level for tx */
> > > +#define VFF_TX_THRE(n) ((n) * 7 / 8)
> > > +/* interrupt trigger level for rx */
> > > +#define VFF_RX_THRE(n) ((n) * 3 / 4)
> > > +
> > > +#define MTK_UART_APDMA_RING_SIZE 0xffffU
> > > +/* invert this bit when wrap ring head again*/
> > > +#define MTK_UART_APDMA_RING_WRAP 0x10000U
> > > +
> > > +#define VFF_INT_FLAG 0x00
> > > +#define VFF_INT_EN 0x04
> > > +#define VFF_EN 0x08
> > > +#define VFF_RST 0x0c
> > > +#define VFF_STOP 0x10
> > > +#define VFF_FLUSH 0x14
> > > +#define VFF_ADDR 0x1c
> > > +#define VFF_LEN 0x24
> > > +#define VFF_THRE 0x28
> > > +#define VFF_WPT 0x2c
> > > +#define VFF_RPT 0x30
> > > +/*TX: the buffer size HW can read. RX: the buffer size SW can read.*/
> > > +#define VFF_VALID_SIZE 0x3c
> > > +/*TX: the buffer size SW can write. RX: the buffer size HW can write.*/
> > > +#define VFF_LEFT_SIZE 0x40
> > > +#define VFF_DEBUG_STATUS 0x50
> > > +#define VFF_4G_SUPPORT 0x54
> > > +
> > > +struct mtk_uart_apdmadev {
> > > + struct dma_device ddev;
> > > + struct clk *clk;
> > > + bool support_33bits;
> > > + unsigned int dma_irq[MTK_UART_APDMA_CHANNELS];
> > > +};
> > > +
> > > +struct mtk_uart_apdma_desc {
> > > + struct virt_dma_desc vd;
> > > +
> > > + unsigned int avail_len;
> > > +};
> > > +
> > > +struct mtk_chan {
> > > + struct virt_dma_chan vc;
> > > + struct dma_slave_config cfg;
> > > + void __iomem *base;
> > > + struct mtk_uart_apdma_desc *desc;
> > > +
> > > + bool requested;
> > > +
> > > + unsigned int rx_status;
> > > +};
> > > +
> > > +static inline struct mtk_uart_apdmadev *
> > > +to_mtk_uart_apdma_dev(struct dma_device *d)
> > > +{
> > > + return container_of(d, struct mtk_uart_apdmadev, ddev);
> > > +}
> > > +
> > > +static inline struct mtk_chan *to_mtk_uart_apdma_chan(struct dma_chan *c)
> > > +{
> > > + return container_of(c, struct mtk_chan, vc.chan);
> > > +}
> > > +
> > > +static inline struct mtk_uart_apdma_desc *to_mtk_uart_apdma_desc
> > > + (struct dma_async_tx_descriptor *t)
> > > +{
> > > + return container_of(t, struct mtk_uart_apdma_desc, vd.tx);
> > > +}
> > > +
> > > +static void mtk_uart_apdma_chan_write(struct mtk_chan *c,
> > > + unsigned int reg, unsigned int val)
> > > +{
> > > + writel(val, c->base + reg);
> > > +}
> > > +
> > > +static unsigned int
> > > +mtk_uart_apdma_chan_read(struct mtk_chan *c, unsigned int reg)
> > > +{
> > > + return readl(c->base + reg);
> > > +}
> > > +
> > > +static void mtk_uart_apdma_desc_free(struct virt_dma_desc *vd)
> > > +{
> > > + struct dma_chan *chan = vd->tx.chan;
> > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > +
> > > + kfree(c->desc);
> > > + c->desc = NULL;
> >
> > Why? I'm afraid this may mask double-free.
> >
>
> i will remove it.
>
> > > +}
> > > +
> > > +static void mtk_uart_apdma_start_tx(struct mtk_chan *c)
> > > +{
> > > + unsigned int txcount = c->desc->avail_len;
> >
> > Variable name is confusing... I'd rather have a boolean `transmitted`
> > (or something), set to 0 here, and set to 1 in the loop (but again,
> > maybe this is not necessary at all, see below).
>
> ok, i modify the function and remove it.
>
> >
> > > + unsigned int len, send, left, wpt, wrap;
> > > +
> > > + if (mtk_uart_apdma_chan_read(c, VFF_LEFT_SIZE) == 0U) {
> > > + mtk_uart_apdma_chan_write(c, VFF_INT_EN, VFF_TX_INT_EN_B);
> >
> > I'd add a "return;" here and de-indent the rest of the function (that
> > should help to make it more readable).
>
> add
>
> >
> > > + } else {
> > > + len = mtk_uart_apdma_chan_read(c, VFF_LEN);
> >
> > You make an assumption below that len is always a power of 2. I wonder
> > if we want to validate that with some BUG/WARN call.
> >
> > Also, I assume VFF_LEN is constant? Maybe we can read it once only in
> > the probe function?
> >
>
> no, the value of VFF_LEN isn't constant.
>
> > > +
> > > + while (((left = mtk_uart_apdma_chan_read(c,
> > > + VFF_LEFT_SIZE)) > 0U)
> > > + && (c->desc->avail_len != 0U)) {
> >
> > Why do we need a while loop here? LEFT_SIZE will always be smaller
> > than LEN, right? Shouldn't we load as much as we can, then let an
> > interrupt happen and reload the FIFO at that stage? Or does this help
> > gain a bit of extra performance?
>
> i will remove while, use 'if' instead of.
> >
> > > + send = min_t(unsigned int, left, c->desc->avail_len);
> > > + wpt = mtk_uart_apdma_chan_read(c, VFF_WPT);
> > > + wrap = wpt & MTK_UART_APDMA_RING_WRAP ?
> >
> > wrap = wrp ^ MTK_UART_APDMA_RING_WRAP ?
> >
>
> wrap = wpt & MTK_UART_APDMA_RING_WRAP ?
> i had test it.
Sorry, I got it wrong (and my question mark is confusing), what I mean is this:
wrap = (wrp ^ MTK_UART_APDMA_RING_WRAP) & MTK_UART_APDMA_RING_WRAP;
But then it's marginally shorter than your solution, so maybe keep
yours, it's a little simpler to understand.
> > > + 0U : MTK_UART_APDMA_RING_WRAP;
> > > +
> > > + if ((wpt & (len - 1U)) + send < len)
Is it possible for wpt > len? If not, then wpt + send < len should be enough.
Also, wpt + send is used a few times now, so maybe have it a separate variable?
Something like this might be nicer:
next_wpt = wpt + send;
if ((wpt & (len - 1U)) + send >= len) {
next_wpt = next_wpt & (len - 1U);
if (!(wpt & MTK_UART_APDMA_RING_WRAP))
next_wpt |= MTK_UART_APDMA_RING_WRAP;
}
mtk_uart_apdma_chan_write(c, VFF_WPT, next_wpt);
Or if the assumption than wpt < len holds:
if (next_wpt >= len) {
next_wpt = next_wpt - len;
...
> > > + mtk_uart_apdma_chan_write(c,
> > > + VFF_WPT, wpt + send);
> > > + else
> > > + mtk_uart_apdma_chan_write(c, VFF_WPT,
> > > + ((wpt + send) & (len - 1U))
> > > + | wrap);
> > > +
> > > + c->desc->avail_len -= send;
> > > + }
> > > +
> > > + if (txcount != c->desc->avail_len) {
> >
> > We know mtk_uart_apdma_chan_read(c, VFF_LEFT_SIZE) > 0 at least once,
> > so this test should always be true.
> >
> > Unless we have c->desc->avail_len == 0, but then we won't even call
> > this function... So this is always true.
> >
>
> yes. you are right. i will remove it.
>
> > > + mtk_uart_apdma_chan_write(c,
> > > + VFF_INT_EN, VFF_TX_INT_EN_B);
> > > + if (mtk_uart_apdma_chan_read(c,
> > > + VFF_FLUSH) == 0U)
> > > + mtk_uart_apdma_chan_write(c,
> > > + VFF_FLUSH, VFF_FLUSH_B);
> > > + }
> > > + }
> > > +}
> > > +
> > > +static void mtk_uart_apdma_start_rx(struct mtk_chan *c)
> > > +{
> > > + struct mtk_uart_apdma_desc *d = c->desc;
> > > + unsigned int rx_len, wg, rg, count;
> > > +
> > > + if (mtk_uart_apdma_chan_read(c, VFF_VALID_SIZE) == 0U)
> > > + return;
> > > +
> > > + if (d && vchan_next_desc(&c->vc)) {
I'd negate the test to de-indent the rest of the code:
if (!d || !vchan_next_desc(&c->vc))
return;
> > > + rx_len = mtk_uart_apdma_chan_read(c, VFF_LEN);
> > > + rg = mtk_uart_apdma_chan_read(c, VFF_RPT);
> > > + wg = mtk_uart_apdma_chan_read(c, VFF_WPT);
> > > + count = ((rg ^ wg) & MTK_UART_APDMA_RING_WRAP) ?
> > > + ((wg & MTK_UART_APDMA_RING_SIZE) +
> > > + rx_len - (rg & MTK_UART_APDMA_RING_SIZE)) :
> > > + ((wg & MTK_UART_APDMA_RING_SIZE) -
> > > + (rg & MTK_UART_APDMA_RING_SIZE));
> >
> > This is hard to read, please use a if/else.
> >
>
> ok.
>
> > > +
> > > + c->rx_status = count;
> > > + mtk_uart_apdma_chan_write(c, VFF_RPT, wg);
> > > +
> > > + list_del(&d->vd.node);
> > > + vchan_cookie_complete(&d->vd);
> > > + }
> > > +}
> > > +
> > > +static irqreturn_t mtk_uart_apdma_irq_handler(int irq, void *dev_id)
> > > +{
> > > + struct dma_chan *chan = (struct dma_chan *)dev_id;
> > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > + struct mtk_uart_apdma_desc *d;
> > > + unsigned long flags;
> > > +
> > > + spin_lock_irqsave(&c->vc.lock, flags);
> > > + switch (c->cfg.direction) {
> > > + case DMA_DEV_TO_MEM:
> >
> > You use an if/else below in a similar case, please be consistent (IMHO
> > if/else is a bit more compact, so I'd use that here).
> >
> sean wang had review it. the link
> http://lists.infradead.org/pipermail/linux-mediatek/2018-December/016292.html
>
> > > + mtk_uart_apdma_chan_write(c,
> > > + VFF_INT_FLAG, VFF_RX_INT_FLAG_CLR_B);
> > > + mtk_uart_apdma_start_rx(c);
> > > + break;
> > > + case DMA_MEM_TO_DEV:
> > > + d = c->desc;
> > > +
> > > + mtk_uart_apdma_chan_write(c,
> > > + VFF_INT_FLAG, VFF_TX_INT_FLAG_CLR_B);
> > > +
> > > + if (d->avail_len != 0U) {
> > > + mtk_uart_apdma_start_tx(c);
> > > + } else {
> > > + list_del(&d->vd.node);
> > > + vchan_cookie_complete(&d->vd);
> > > + }
> > > + break;
> > > + default:
> > > + break;
> > > + }
> > > + spin_unlock_irqrestore(&c->vc.lock, flags);
> > > +
> > > + return IRQ_HANDLED;
> > > +}
> > > +
> > > +static int mtk_uart_apdma_alloc_chan_resources(struct dma_chan *chan)
> > > +{
> > > + struct mtk_uart_apdmadev *mtkd = to_mtk_uart_apdma_dev(chan->device);
> > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > + u32 status;
> > > + int ret = -EBUSY;
No need to init ret.
> > > +
> > > + pm_runtime_get_sync(mtkd->ddev.dev);
> > > +
> > > + mtk_uart_apdma_chan_write(c, VFF_ADDR, 0);
> > > + mtk_uart_apdma_chan_write(c, VFF_THRE, 0);
> > > + mtk_uart_apdma_chan_write(c, VFF_LEN, 0);
> > > + mtk_uart_apdma_chan_write(c, VFF_RST, VFF_WARM_RST_B);
> > > +
> > > + ret = readx_poll_timeout(readl,
> > > + c->base + VFF_EN,
> > > + status, status == 0, 10, 100);
> > > + if (ret) {
> > > + dev_err(c->vc.chan.device->dev,
> > > + "dma reset: fail, timeout\n");
> > > + goto exit;
Just return ret, since you don't do any cleanup in exit:.
> > > + }
> > > +
> > > + if (!c->requested) {
> > > + c->requested = true;
> > > + ret = request_irq(mtkd->dma_irq[chan->chan_id],
> > > + mtk_uart_apdma_irq_handler,
> > > + IRQF_TRIGGER_NONE,
> > > + KBUILD_MODNAME, chan);
> > > + if (ret < 0) {
> > > + dev_err(chan->device->dev, "Can't request dma IRQ\n");
> > > + return -EINVAL;
> > > + }
> > > + }
> > > +
> > > + if (mtkd->support_33bits)
> > > + mtk_uart_apdma_chan_write(c,
> > > + VFF_4G_SUPPORT, VFF_4G_SUPPORT_CLR_B);
> > > +
> > > +exit:
> > > + return ret;
> > > +}
> > > +
> > > +static void mtk_uart_apdma_free_chan_resources(struct dma_chan *chan)
> > > +{
> > > + struct mtk_uart_apdmadev *mtkd = to_mtk_uart_apdma_dev(chan->device);
> > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > +
> > > + if (c->requested) {
> > > + c->requested = false;
> > > + free_irq(mtkd->dma_irq[chan->chan_id], chan);
> > > + }
> > > +
> > > + tasklet_kill(&c->vc.task);
> > > +
> > > + vchan_free_chan_resources(&c->vc);
> > > +
> > > + pm_runtime_put_sync(mtkd->ddev.dev);
> > > +}
> > > +
> > > +static enum dma_status mtk_uart_apdma_tx_status(struct dma_chan *chan,
> > > + dma_cookie_t cookie,
> > > + struct dma_tx_state *txstate)
> > > +{
> > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > + enum dma_status ret;
> > > + unsigned long flags;
> > > +
> > > + if (!txstate)
> > > + return DMA_ERROR;
> > > +
> > > + ret = dma_cookie_status(chan, cookie, txstate);
> > > + spin_lock_irqsave(&c->vc.lock, flags);
> > > + if (ret == DMA_IN_PROGRESS) {
> > > + c->rx_status = mtk_uart_apdma_chan_read(c, VFF_RPT)
> > > + & MTK_UART_APDMA_RING_SIZE;
> > > + dma_set_residue(txstate, c->rx_status);
> > > + } else if (ret == DMA_COMPLETE && c->cfg.direction == DMA_DEV_TO_MEM) {
> > > + dma_set_residue(txstate, c->rx_status);
> > > + } else {
> > > + dma_set_residue(txstate, 0);
> > > + }
> > > + spin_unlock_irqrestore(&c->vc.lock, flags);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +/*
> > > + * dmaengine_prep_slave_single will call the function. and sglen is 1.
> > > + * 8250 uart using one ring buffer, and deal with one sg.
> > > + */
> > > +static struct dma_async_tx_descriptor *mtk_uart_apdma_prep_slave_sg
> > > + (struct dma_chan *chan, struct scatterlist *sgl,
> > > + unsigned int sglen, enum dma_transfer_direction dir,
Too many spaces (or a tab?) between `sglen,` and `enum`
> > > + unsigned long tx_flags, void *context)
> > > +{
> > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > + struct mtk_uart_apdma_desc *d;
> > > +
> > > + if ((dir != DMA_DEV_TO_MEM) &&
> > > + (dir != DMA_MEM_TO_DEV)) {
> > > + dev_err(chan->device->dev, "bad direction\n");
> > > + return NULL;
> > > + }
> > > +
> > > + /* Now allocate and setup the descriptor */
> > > + d = kzalloc(sizeof(*d), GFP_ATOMIC);
> > > + if (!d)
> > > + return NULL;
> > > +
> > > + /* sglen is 1 */
> > > + d->avail_len = sg_dma_len(sgl);
> > > +
> > > + return vchan_tx_prep(&c->vc, &d->vd, tx_flags);
> > > +}
> > > +
> > > +static void mtk_uart_apdma_issue_pending(struct dma_chan *chan)
> > > +{
> > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > + struct virt_dma_desc *vd;
> > > + unsigned long flags;
> > > +
> > > + spin_lock_irqsave(&c->vc.lock, flags);
> > > + if (c->cfg.direction == DMA_DEV_TO_MEM) {
> > > + if (vchan_issue_pending(&c->vc) && !c->desc) {
> > > + vd = vchan_next_desc(&c->vc);
> > > + c->desc = to_mtk_uart_apdma_desc(&vd->tx);
> > > + mtk_uart_apdma_start_rx(c);
> > > + }
> > > + } else if (c->cfg.direction == DMA_MEM_TO_DEV) {
> > > + if (vchan_issue_pending(&c->vc) && !c->desc) {
> > > + vd = vchan_next_desc(&c->vc);
> > > + c->desc = to_mtk_uart_apdma_desc(&vd->tx);
> > > + mtk_uart_apdma_start_tx(c);
> > > + }
> > > + }
> > > + spin_unlock_irqrestore(&c->vc.lock, flags);
> > > +}
> > > +
> > > +static int mtk_uart_apdma_slave_config(struct dma_chan *chan,
> > > + struct dma_slave_config *cfg)
> > > +{
> > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > + struct mtk_uart_apdmadev *mtkd =
> > > + to_mtk_uart_apdma_dev(c->vc.chan.device);
> > > +
> > > + c->cfg = *cfg;
> > > +
> > > + if (cfg->direction == DMA_DEV_TO_MEM) {
> > > + unsigned int rx_len = cfg->src_addr_width * 1024;
> > > +
> > > + mtk_uart_apdma_chan_write(c, VFF_ADDR, cfg->src_addr);
> > > + mtk_uart_apdma_chan_write(c, VFF_LEN, rx_len);
> > > + mtk_uart_apdma_chan_write(c, VFF_THRE, VFF_RX_THRE(rx_len));
> > > + mtk_uart_apdma_chan_write(c,
> > > + VFF_INT_EN, VFF_RX_INT_EN0_B
> > > + | VFF_RX_INT_EN1_B);
> > > + mtk_uart_apdma_chan_write(c, VFF_RPT, 0);
> > > + mtk_uart_apdma_chan_write(c,
> > > + VFF_INT_FLAG, VFF_RX_INT_FLAG_CLR_B);
> > > + mtk_uart_apdma_chan_write(c, VFF_EN, VFF_EN_B);
> > > + } else if (cfg->direction == DMA_MEM_TO_DEV) {
> > > + unsigned int tx_len = cfg->dst_addr_width * 1024;
> > > +
> > > + mtk_uart_apdma_chan_write(c, VFF_ADDR, cfg->dst_addr);
> > > + mtk_uart_apdma_chan_write(c, VFF_LEN, tx_len);
> > > + mtk_uart_apdma_chan_write(c, VFF_THRE, VFF_TX_THRE(tx_len));
> > > + mtk_uart_apdma_chan_write(c, VFF_WPT, 0);
> > > + mtk_uart_apdma_chan_write(c,
> > > + VFF_INT_FLAG, VFF_TX_INT_FLAG_CLR_B);
> > > + mtk_uart_apdma_chan_write(c, VFF_EN, VFF_EN_B);
> > > + }
> > > +
> > > + if (mtkd->support_33bits)
> > > + mtk_uart_apdma_chan_write(c, VFF_4G_SUPPORT, VFF_4G_SUPPORT_B);
> > > +
> > > + if (mtk_uart_apdma_chan_read(c, VFF_EN) != VFF_EN_B) {
> > > + dev_err(chan->device->dev,
> > > + "config dma dir[%d] fail\n", cfg->direction);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int mtk_uart_apdma_terminate_all(struct dma_chan *chan)
> > > +{
> > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > + unsigned long flags;
> > > + u32 status;
> > > + int ret;
> > > +
> > > + spin_lock_irqsave(&c->vc.lock, flags);
> > > +
> > > + mtk_uart_apdma_chan_write(c, VFF_FLUSH, VFF_FLUSH_CLR_B);
> > > + /* Wait for flush */
> > > + ret = readx_poll_timeout(readl,
> > > + c->base + VFF_FLUSH,
> > > + status,
> > > + (status & VFF_FLUSH_B) != VFF_FLUSH_B,
> > > + 10, 100);
> > > + if (ret)
> > > + dev_err(c->vc.chan.device->dev,
> > > + "dma stop: polling FLUSH fail, DEBUG=0x%x\n",
> > > + mtk_uart_apdma_chan_read(c, VFF_DEBUG_STATUS));
> > > +
> > > + /*set stop as 1 -> wait until en is 0 -> set stop as 0*/
> > > + mtk_uart_apdma_chan_write(c, VFF_STOP, VFF_STOP_B);
> > > + ret = readx_poll_timeout(readl,
> > > + c->base + VFF_EN,
> > > + status, status == 0, 10, 100);
> > > + if (ret)
> > > + dev_err(c->vc.chan.device->dev,
> > > + "dma stop: polling VFF_EN fail, DEBUG=0x%x\n",
> > > + mtk_uart_apdma_chan_read(c, VFF_DEBUG_STATUS));
> > > +
> > > + mtk_uart_apdma_chan_write(c, VFF_STOP, VFF_STOP_CLR_B);
> > > + mtk_uart_apdma_chan_write(c, VFF_INT_EN, VFF_INT_EN_CLR_B);
> > > +
> > > + switch (c->cfg.direction) {
> > > + case DMA_DEV_TO_MEM:
> >
> > Ditto, if/else?
> >
> ditto.
> http://lists.infradead.org/pipermail/linux-mediatek/2018-December/016292.html
Well, I disagree, especially in this case: We turn 4 lines of code into 10...
> > > + mtk_uart_apdma_chan_write(c,
> > > + VFF_INT_FLAG, VFF_RX_INT_FLAG_CLR_B);
> > > + break;
> > > + case DMA_MEM_TO_DEV:
> > > + mtk_uart_apdma_chan_write(c,
> > > + VFF_INT_FLAG, VFF_TX_INT_FLAG_CLR_B);
> > > + break;
> > > + default:
> > > + break;
> > > + }
> > > +
> > > + spin_unlock_irqrestore(&c->vc.lock, flags);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int mtk_uart_apdma_device_pause(struct dma_chan *chan)
> > > +{
> > > + /* just for check caps pass */
> > > + return 0;
> > > +}
> > > +
> > > +static int mtk_uart_apdma_device_resume(struct dma_chan *chan)
> > > +{
> > > + /* just for check caps pass */
> > > + return 0;
> > > +}
Why do you need these? Seems wrong to advertise device_pause/resume
(and the caps) if we don't actually support that?
> > > +
> > > +static void mtk_uart_apdma_free(struct mtk_uart_apdmadev *mtkd)
> > > +{
> > > + while (list_empty(&mtkd->ddev.channels) == 0) {
> > > + struct mtk_chan *c = list_first_entry(&mtkd->ddev.channels,
> > > + struct mtk_chan, vc.chan.device_node);
> > > +
> > > + list_del(&c->vc.chan.device_node);
> > > + tasklet_kill(&c->vc.task);
> > > + }
> > > +}
> > > +
> > > +static const struct of_device_id mtk_uart_apdma_match[] = {
> > > + { .compatible = "mediatek,mt6577-uart-dma", },
> > > + { /* sentinel */ },
> > > +};
> > > +MODULE_DEVICE_TABLE(of, mtk_uart_apdma_match);
> > > +
> > > +static int mtk_uart_apdma_probe(struct platform_device *pdev)
> > > +{
> > > + struct mtk_uart_apdmadev *mtkd;
> > > + struct resource *res;
> > > + struct mtk_chan *c;
> > > + unsigned int i;
> > > + int rc;
> > > +
> > > + mtkd = devm_kzalloc(&pdev->dev, sizeof(*mtkd), GFP_KERNEL);
> > > + if (!mtkd)
> > > + return -ENOMEM;
> > > +
> > > + mtkd->clk = devm_clk_get(&pdev->dev, NULL);
> > > + if (IS_ERR(mtkd->clk)) {
> > > + dev_err(&pdev->dev, "No clock specified\n");
> > > + rc = PTR_ERR(mtkd->clk);
> > > + goto err_no_dma;
> > > + }
> > > +
> > > + if (of_property_read_bool(pdev->dev.of_node, "dma-33bits")) {
> > > + dev_info(&pdev->dev, "Support dma 33bits\n");
> >
> > Drop this, a bit verbose.
> >
> > > + mtkd->support_33bits = true;
> > > + }
> > > +
> > > + rc = dma_set_mask_and_coherent(&pdev->dev,
> > > + DMA_BIT_MASK(32 | mtkd->support_33bits));
> > > + if (rc)
> > > + goto err_no_dma;
> > > +
> > > + dma_cap_set(DMA_SLAVE, mtkd->ddev.cap_mask);
> > > + mtkd->ddev.device_alloc_chan_resources =
> > > + mtk_uart_apdma_alloc_chan_resources;
> > > + mtkd->ddev.device_free_chan_resources =
> > > + mtk_uart_apdma_free_chan_resources;
> > > + mtkd->ddev.device_tx_status = mtk_uart_apdma_tx_status;
> > > + mtkd->ddev.device_issue_pending = mtk_uart_apdma_issue_pending;
> > > + mtkd->ddev.device_prep_slave_sg = mtk_uart_apdma_prep_slave_sg;
> > > + mtkd->ddev.device_config = mtk_uart_apdma_slave_config;
> > > + mtkd->ddev.device_pause = mtk_uart_apdma_device_pause;
> > > + mtkd->ddev.device_resume = mtk_uart_apdma_device_resume;
> > > + mtkd->ddev.device_terminate_all = mtk_uart_apdma_terminate_all;
> > > + mtkd->ddev.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE);
> > > + mtkd->ddev.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE);
> > > + mtkd->ddev.directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
> > > + mtkd->ddev.residue_granularity = DMA_RESIDUE_GRANULARITY_SEGMENT;
> > > + mtkd->ddev.dev = &pdev->dev;
> > > + INIT_LIST_HEAD(&mtkd->ddev.channels);
> > > +
> > > + for (i = 0; i < MTK_UART_APDMA_CHANNELS; i++) {
> > > + c = devm_kzalloc(mtkd->ddev.dev, sizeof(*c), GFP_KERNEL);
> > > + if (!c) {
> > > + rc = -ENODEV;
> > > + goto err_no_dma;
> > > + }
> > > +
> > > + res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> > > + if (!res) {
> > > + rc = -ENODEV;
> > > + goto err_no_dma;
> > > + }
> > > +
> > > + c->base = devm_ioremap_resource(&pdev->dev, res);
> > > + if (IS_ERR(c->base)) {
> > > + rc = PTR_ERR(c->base);
> > > + goto err_no_dma;
> > > + }
> > > + c->requested = false;
> > > + c->vc.desc_free = mtk_uart_apdma_desc_free;
> > > + vchan_init(&c->vc, &mtkd->ddev);
> > > +
> > > + mtkd->dma_irq[i] = platform_get_irq(pdev, i);
> > > + if ((int)mtkd->dma_irq[i] < 0) {
> > > + dev_err(&pdev->dev, "failed to get IRQ[%d]\n", i);
> > > + rc = -EINVAL;
> > > + goto err_no_dma;
> > > + }
> > > + }
> > > +
> > > + pm_runtime_enable(&pdev->dev);
> > > + pm_runtime_set_active(&pdev->dev);
> > > +
> > > + rc = dma_async_device_register(&mtkd->ddev);
> > > + if (rc)
> > > + goto rpm_disable;
> > > +
> > > + platform_set_drvdata(pdev, mtkd);
> > > +
> > > + if (pdev->dev.of_node) {
> > > + /* Device-tree DMA controller registration */
> > > + rc = of_dma_controller_register(pdev->dev.of_node,
> > > + of_dma_xlate_by_chan_id,
> > > + mtkd);
> > > + if (rc)
> > > + goto dma_remove;
> > > + }
> > > +
> > > + return rc;
> > > +
> > > +dma_remove:
> > > + dma_async_device_unregister(&mtkd->ddev);
> > > +rpm_disable:
> > > + pm_runtime_disable(&pdev->dev);
> > > +err_no_dma:
> > > + mtk_uart_apdma_free(mtkd);
It's not very correct to call this before
INIT_LIST_HEAD(&mtkd->ddev.channels);
I'd just call `return rc;` directly for all instances before that line.
> > > + return rc;
> > > +}
> > > +
> > > +static int mtk_uart_apdma_remove(struct platform_device *pdev)
> > > +{
> > > + struct mtk_uart_apdmadev *mtkd = platform_get_drvdata(pdev);
> > > +
> > > + if (pdev->dev.of_node)
> > > + of_dma_controller_free(pdev->dev.of_node);
> > > +
> > > + pm_runtime_disable(&pdev->dev);
> > > + pm_runtime_put_noidle(&pdev->dev);
> > > +
> > > + dma_async_device_unregister(&mtkd->ddev);
> > > + mtk_uart_apdma_free(mtkd);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +#ifdef CONFIG_PM_SLEEP
> > > +static int mtk_uart_apdma_suspend(struct device *dev)
> > > +{
> > > + struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
> > > +
> > > + if (!pm_runtime_suspended(dev))
> > > + clk_disable_unprepare(mtkd->clk);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int mtk_uart_apdma_resume(struct device *dev)
> > > +{
> > > + int ret;
> > > + struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
> > > +
> > > + if (!pm_runtime_suspended(dev)) {
> > > + ret = clk_prepare_enable(mtkd->clk);
> > > + if (ret)
> > > + return ret;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +#endif /* CONFIG_PM_SLEEP */
> > > +
> > > +#ifdef CONFIG_PM
> > > +static int mtk_uart_apdma_runtime_suspend(struct device *dev)
> > > +{
> > > + struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
> > > +
> > > + clk_disable_unprepare(mtkd->clk);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int mtk_uart_apdma_runtime_resume(struct device *dev)
> > > +{
> > > + int ret;
> > > + struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
> > > +
> > > + ret = clk_prepare_enable(mtkd->clk);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + return 0;
> > > +}
> > > +#endif /* CONFIG_PM */
> > > +
> > > +static const struct dev_pm_ops mtk_uart_apdma_pm_ops = {
> > > + SET_SYSTEM_SLEEP_PM_OPS(mtk_uart_apdma_suspend, mtk_uart_apdma_resume)
> > > + SET_RUNTIME_PM_OPS(mtk_uart_apdma_runtime_suspend,
> > > + mtk_uart_apdma_runtime_resume, NULL)
> > > +};
> > > +
> > > +static struct platform_driver mtk_uart_apdma_driver = {
> > > + .probe = mtk_uart_apdma_probe,
> > > + .remove = mtk_uart_apdma_remove,
> > > + .driver = {
> > > + .name = KBUILD_MODNAME,
> > > + .pm = &mtk_uart_apdma_pm_ops,
> > > + .of_match_table = of_match_ptr(mtk_uart_apdma_match),
> > > + },
> > > +};
> > > +
> > > +module_platform_driver(mtk_uart_apdma_driver);
> > > +
> > > +MODULE_DESCRIPTION("MediaTek UART APDMA Controller Driver");
> > > +MODULE_AUTHOR("Long Cheng <long.cheng@mediatek.com>");
> > > +MODULE_LICENSE("GPL v2");
> > > +
> > > diff --git a/drivers/dma/mediatek/Kconfig b/drivers/dma/mediatek/Kconfig
> > > index 27bac0b..1a523c87 100644
> > > --- a/drivers/dma/mediatek/Kconfig
> > > +++ b/drivers/dma/mediatek/Kconfig
> > > @@ -1,4 +1,15 @@
> > >
> > > +config DMA_MTK_UART
> > > + tristate "MediaTek SoCs APDMA support for UART"
> > > + depends on OF && SERIAL_8250_MT6577
> > > + select DMA_ENGINE
> > > + select DMA_VIRTUAL_CHANNELS
> > > + help
> > > + Support for the UART DMA engine found on MediaTek MTK SoCs.
> > > + when SERIAL_8250_MT6577 is enabled, and if you want to use DMA,
> > > + you can enable the config. the DMA engine can only be used
> > > + with MediaTek SoCs.
> > > +
> > > config MTK_HSDMA
> > > tristate "MediaTek High-Speed DMA controller support"
> > > depends on ARCH_MEDIATEK || COMPILE_TEST
> > > diff --git a/drivers/dma/mediatek/Makefile b/drivers/dma/mediatek/Makefile
> > > index 6e778f8..2f2efd9 100644
> > > --- a/drivers/dma/mediatek/Makefile
> > > +++ b/drivers/dma/mediatek/Makefile
> > > @@ -1 +1,2 @@
> > > +obj-$(CONFIG_DMA_MTK_UART) += 8250_mtk_dma.o
> > > obj-$(CONFIG_MTK_HSDMA) += mtk-hsdma.o
> > > --
> > > 1.7.9.5
> > >
>
>
^ permalink raw reply
* [v7,1/2] dmaengine: 8250_mtk_dma: add MediaTek uart DMA support
From: Long Cheng @ 2018-12-26 6:35 UTC (permalink / raw)
To: Nicolas Boichat
Cc: Vinod Koul, Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee,
Sean Wang, Matthias Brugger, Dan Williams, Greg Kroah-Hartman,
Jiri Slaby, Sean Wang, dmaengine, devicetree,
linux-arm Mailing List, linux-mediatek, lkml, linux-serial,
srv_heupstream, Yingjoe Chen, YT Shen, Zhenbao Liu, Long Cheng
On Wed, 2018-12-26 at 08:05 +0800, Nicolas Boichat wrote:
thanks.
> On Tue, Dec 25, 2018 at 8:06 PM Long Cheng <long.cheng@mediatek.com> wrote:
> >
> > Thanks for your comments.
> >
> > On Tue, 2018-12-25 at 15:16 +0800, Nicolas Boichat wrote:
> > > Not a full review, a few comments below.
> > >
> > > Thanks,
> > >
> >
> > would you like help to full review at this version patch? and then i
> > can modify these at next version patch. thanks.
>
> Added a few more comments ,-)
>
> > > On Tue, Dec 25, 2018 at 9:27 AM Long Cheng <long.cheng@mediatek.com> wrote:
> > > >
> > > > In DMA engine framework, add 8250 uart dma to support MediaTek uart.
> > > > If MediaTek uart enabled(SERIAL_8250_MT6577), and want to improve
> > > > the performance, can enable the function.
> > > >
> > > > Signed-off-by: Long Cheng <long.cheng@mediatek.com>
> > > > ---
> > > > drivers/dma/mediatek/8250_mtk_dma.c | 694 +++++++++++++++++++++++++++++++++++
> > > > drivers/dma/mediatek/Kconfig | 11 +
> > > > drivers/dma/mediatek/Makefile | 1 +
> > > > 3 files changed, 706 insertions(+)
> > > > create mode 100644 drivers/dma/mediatek/8250_mtk_dma.c
> > > >
> > > > diff --git a/drivers/dma/mediatek/8250_mtk_dma.c b/drivers/dma/mediatek/8250_mtk_dma.c
> > > > new file mode 100644
> > > > index 0000000..c4090f2
> > > > --- /dev/null
> > > > +++ b/drivers/dma/mediatek/8250_mtk_dma.c
> > > > @@ -0,0 +1,694 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * MediaTek 8250 DMA driver.
> > > > + *
> > > > + * Copyright (c) 2018 MediaTek Inc.
> > > > + * Author: Long Cheng <long.cheng@mediatek.com>
> > > > + */
> > > > +
> > > > +#include <linux/clk.h>
> > > > +#include <linux/dmaengine.h>
> > > > +#include <linux/dma-mapping.h>
> > > > +#include <linux/err.h>
> > > > +#include <linux/init.h>
> > > > +#include <linux/interrupt.h>
> > > > +#include <linux/list.h>
> > > > +#include <linux/kernel.h>
> > > > +#include <linux/module.h>
> > > > +#include <linux/of_dma.h>
> > > > +#include <linux/of_device.h>
> > > > +#include <linux/platform_device.h>
> > > > +#include <linux/slab.h>
> > > > +#include <linux/spinlock.h>
> > > > +#include <linux/pm_runtime.h>
> > > > +#include <linux/iopoll.h>
> > >
> > > Alphabetical order.
> >
> > ok, i will order.
> >
> > >
> > > > +
> > > > +#include "../virt-dma.h"
> > > > +
> > > > +#define MTK_UART_APDMA_CHANNELS (CONFIG_SERIAL_8250_NR_UARTS * 2)
> > > > +
> > > > +#define VFF_EN_B BIT(0)
> > > > +#define VFF_STOP_B BIT(0)
> > > > +#define VFF_FLUSH_B BIT(0)
> > > > +#define VFF_4G_SUPPORT_B BIT(0)
> > > > +#define VFF_RX_INT_EN0_B BIT(0) /*rx valid size >= vff thre*/
> > > > +#define VFF_RX_INT_EN1_B BIT(1)
> > > > +#define VFF_TX_INT_EN_B BIT(0) /*tx left size >= vff thre*/
> > > > +#define VFF_WARM_RST_B BIT(0)
> > > > +#define VFF_RX_INT_FLAG_CLR_B (BIT(0) | BIT(1))
> > > > +#define VFF_TX_INT_FLAG_CLR_B 0
> > > > +#define VFF_STOP_CLR_B 0
> > > > +#define VFF_FLUSH_CLR_B 0
> > > > +#define VFF_INT_EN_CLR_B 0
> > > > +#define VFF_4G_SUPPORT_CLR_B 0
> > > > +
> > > > +/* interrupt trigger level for tx */
> > > > +#define VFF_TX_THRE(n) ((n) * 7 / 8)
> > > > +/* interrupt trigger level for rx */
> > > > +#define VFF_RX_THRE(n) ((n) * 3 / 4)
> > > > +
> > > > +#define MTK_UART_APDMA_RING_SIZE 0xffffU
> > > > +/* invert this bit when wrap ring head again*/
> > > > +#define MTK_UART_APDMA_RING_WRAP 0x10000U
> > > > +
> > > > +#define VFF_INT_FLAG 0x00
> > > > +#define VFF_INT_EN 0x04
> > > > +#define VFF_EN 0x08
> > > > +#define VFF_RST 0x0c
> > > > +#define VFF_STOP 0x10
> > > > +#define VFF_FLUSH 0x14
> > > > +#define VFF_ADDR 0x1c
> > > > +#define VFF_LEN 0x24
> > > > +#define VFF_THRE 0x28
> > > > +#define VFF_WPT 0x2c
> > > > +#define VFF_RPT 0x30
> > > > +/*TX: the buffer size HW can read. RX: the buffer size SW can read.*/
> > > > +#define VFF_VALID_SIZE 0x3c
> > > > +/*TX: the buffer size SW can write. RX: the buffer size HW can write.*/
> > > > +#define VFF_LEFT_SIZE 0x40
> > > > +#define VFF_DEBUG_STATUS 0x50
> > > > +#define VFF_4G_SUPPORT 0x54
> > > > +
> > > > +struct mtk_uart_apdmadev {
> > > > + struct dma_device ddev;
> > > > + struct clk *clk;
> > > > + bool support_33bits;
> > > > + unsigned int dma_irq[MTK_UART_APDMA_CHANNELS];
> > > > +};
> > > > +
> > > > +struct mtk_uart_apdma_desc {
> > > > + struct virt_dma_desc vd;
> > > > +
> > > > + unsigned int avail_len;
> > > > +};
> > > > +
> > > > +struct mtk_chan {
> > > > + struct virt_dma_chan vc;
> > > > + struct dma_slave_config cfg;
> > > > + void __iomem *base;
> > > > + struct mtk_uart_apdma_desc *desc;
> > > > +
> > > > + bool requested;
> > > > +
> > > > + unsigned int rx_status;
> > > > +};
> > > > +
> > > > +static inline struct mtk_uart_apdmadev *
> > > > +to_mtk_uart_apdma_dev(struct dma_device *d)
> > > > +{
> > > > + return container_of(d, struct mtk_uart_apdmadev, ddev);
> > > > +}
> > > > +
> > > > +static inline struct mtk_chan *to_mtk_uart_apdma_chan(struct dma_chan *c)
> > > > +{
> > > > + return container_of(c, struct mtk_chan, vc.chan);
> > > > +}
> > > > +
> > > > +static inline struct mtk_uart_apdma_desc *to_mtk_uart_apdma_desc
> > > > + (struct dma_async_tx_descriptor *t)
> > > > +{
> > > > + return container_of(t, struct mtk_uart_apdma_desc, vd.tx);
> > > > +}
> > > > +
> > > > +static void mtk_uart_apdma_chan_write(struct mtk_chan *c,
> > > > + unsigned int reg, unsigned int val)
> > > > +{
> > > > + writel(val, c->base + reg);
> > > > +}
> > > > +
> > > > +static unsigned int
> > > > +mtk_uart_apdma_chan_read(struct mtk_chan *c, unsigned int reg)
> > > > +{
> > > > + return readl(c->base + reg);
> > > > +}
> > > > +
> > > > +static void mtk_uart_apdma_desc_free(struct virt_dma_desc *vd)
> > > > +{
> > > > + struct dma_chan *chan = vd->tx.chan;
> > > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > > +
> > > > + kfree(c->desc);
> > > > + c->desc = NULL;
> > >
> > > Why? I'm afraid this may mask double-free.
> > >
> >
> > i will remove it.
> >
> > > > +}
> > > > +
> > > > +static void mtk_uart_apdma_start_tx(struct mtk_chan *c)
> > > > +{
> > > > + unsigned int txcount = c->desc->avail_len;
> > >
> > > Variable name is confusing... I'd rather have a boolean `transmitted`
> > > (or something), set to 0 here, and set to 1 in the loop (but again,
> > > maybe this is not necessary at all, see below).
> >
> > ok, i modify the function and remove it.
> >
> > >
> > > > + unsigned int len, send, left, wpt, wrap;
> > > > +
> > > > + if (mtk_uart_apdma_chan_read(c, VFF_LEFT_SIZE) == 0U) {
> > > > + mtk_uart_apdma_chan_write(c, VFF_INT_EN, VFF_TX_INT_EN_B);
> > >
> > > I'd add a "return;" here and de-indent the rest of the function (that
> > > should help to make it more readable).
> >
> > add
> >
> > >
> > > > + } else {
> > > > + len = mtk_uart_apdma_chan_read(c, VFF_LEN);
> > >
> > > You make an assumption below that len is always a power of 2. I wonder
> > > if we want to validate that with some BUG/WARN call.
> > >
> > > Also, I assume VFF_LEN is constant? Maybe we can read it once only in
> > > the probe function?
> > >
> >
> > no, the value of VFF_LEN isn't constant.
> >
> > > > +
> > > > + while (((left = mtk_uart_apdma_chan_read(c,
> > > > + VFF_LEFT_SIZE)) > 0U)
> > > > + && (c->desc->avail_len != 0U)) {
> > >
> > > Why do we need a while loop here? LEFT_SIZE will always be smaller
> > > than LEN, right? Shouldn't we load as much as we can, then let an
> > > interrupt happen and reload the FIFO at that stage? Or does this help
> > > gain a bit of extra performance?
> >
> > i will remove while, use 'if' instead of.
> > >
> > > > + send = min_t(unsigned int, left, c->desc->avail_len);
> > > > + wpt = mtk_uart_apdma_chan_read(c, VFF_WPT);
> > > > + wrap = wpt & MTK_UART_APDMA_RING_WRAP ?
> > >
> > > wrap = wrp ^ MTK_UART_APDMA_RING_WRAP ?
> > >
> >
> > wrap = wpt & MTK_UART_APDMA_RING_WRAP ?
> > i had test it.
>
> Sorry, I got it wrong (and my question mark is confusing), what I mean is this:
>
> wrap = (wrp ^ MTK_UART_APDMA_RING_WRAP) & MTK_UART_APDMA_RING_WRAP;
>
> But then it's marginally shorter than your solution, so maybe keep
> yours, it's a little simpler to understand.
>
i will rename 'MTK_UART_APDMA_RING_WRAP' to 'VFF_RING_WRAP'. let code
shorter. i will keep the code.
> > > > + 0U : MTK_UART_APDMA_RING_WRAP;
> > > > +
> > > > + if ((wpt & (len - 1U)) + send < len)
>
> Is it possible for wpt > len? If not, then wpt + send < len should be enough.
>
> Also, wpt + send is used a few times now, so maybe have it a separate variable?
>
> Something like this might be nicer:
>
> next_wpt = wpt + send;
> if ((wpt & (len - 1U)) + send >= len) {
> next_wpt = next_wpt & (len - 1U);
> if (!(wpt & MTK_UART_APDMA_RING_WRAP))
> next_wpt |= MTK_UART_APDMA_RING_WRAP;
> }
> mtk_uart_apdma_chan_write(c, VFF_WPT, next_wpt);
>
> Or if the assumption than wpt < len holds:
> if (next_wpt >= len) {
> next_wpt = next_wpt - len;
> ...
>
i will modify it like your comments,
next_wpt = wpt + send;
> if ((wpt & (len - 1U)) + send >= len) {
> next_wpt = next_wpt & (len - 1U);
> if (!(wpt & MTK_UART_APDMA_RING_WRAP))
> next_wpt |= MTK_UART_APDMA_RING_WRAP;
>
i think that it's better.
> > > > + mtk_uart_apdma_chan_write(c,
> > > > + VFF_WPT, wpt + send);
> > > > + else
> > > > + mtk_uart_apdma_chan_write(c, VFF_WPT,
> > > > + ((wpt + send) & (len - 1U))
> > > > + | wrap);
> > > > +
> > > > + c->desc->avail_len -= send;
> > > > + }
> > > > +
> > > > + if (txcount != c->desc->avail_len) {
> > >
> > > We know mtk_uart_apdma_chan_read(c, VFF_LEFT_SIZE) > 0 at least once,
> > > so this test should always be true.
> > >
> > > Unless we have c->desc->avail_len == 0, but then we won't even call
> > > this function... So this is always true.
> > >
> >
> > yes. you are right. i will remove it.
> >
> > > > + mtk_uart_apdma_chan_write(c,
> > > > + VFF_INT_EN, VFF_TX_INT_EN_B);
> > > > + if (mtk_uart_apdma_chan_read(c,
> > > > + VFF_FLUSH) == 0U)
> > > > + mtk_uart_apdma_chan_write(c,
> > > > + VFF_FLUSH, VFF_FLUSH_B);
> > > > + }
> > > > + }
> > > > +}
> > > > +
> > > > +static void mtk_uart_apdma_start_rx(struct mtk_chan *c)
> > > > +{
> > > > + struct mtk_uart_apdma_desc *d = c->desc;
> > > > + unsigned int rx_len, wg, rg, count;
> > > > +
> > > > + if (mtk_uart_apdma_chan_read(c, VFF_VALID_SIZE) == 0U)
> > > > + return;
> > > > +
> > > > + if (d && vchan_next_desc(&c->vc)) {
>
> I'd negate the test to de-indent the rest of the code:
> if (!d || !vchan_next_desc(&c->vc))
> return;
>
ok. i will modify it like
if ((mtk_uart_apdma_chan_read(c, VFF_VALID_SIZE) == 0U) ||
!d || !vchan_next_desc(&c->vc))
return;
> > > > + rx_len = mtk_uart_apdma_chan_read(c, VFF_LEN);
> > > > + rg = mtk_uart_apdma_chan_read(c, VFF_RPT);
> > > > + wg = mtk_uart_apdma_chan_read(c, VFF_WPT);
> > > > + count = ((rg ^ wg) & MTK_UART_APDMA_RING_WRAP) ?
> > > > + ((wg & MTK_UART_APDMA_RING_SIZE) +
> > > > + rx_len - (rg & MTK_UART_APDMA_RING_SIZE)) :
> > > > + ((wg & MTK_UART_APDMA_RING_SIZE) -
> > > > + (rg & MTK_UART_APDMA_RING_SIZE));
> > >
> > > This is hard to read, please use a if/else.
> > >
> >
> > ok.
> >
> > > > +
> > > > + c->rx_status = count;
> > > > + mtk_uart_apdma_chan_write(c, VFF_RPT, wg);
> > > > +
> > > > + list_del(&d->vd.node);
> > > > + vchan_cookie_complete(&d->vd);
> > > > + }
> > > > +}
> > > > +
> > > > +static irqreturn_t mtk_uart_apdma_irq_handler(int irq, void *dev_id)
> > > > +{
> > > > + struct dma_chan *chan = (struct dma_chan *)dev_id;
> > > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > > + struct mtk_uart_apdma_desc *d;
> > > > + unsigned long flags;
> > > > +
> > > > + spin_lock_irqsave(&c->vc.lock, flags);
> > > > + switch (c->cfg.direction) {
> > > > + case DMA_DEV_TO_MEM:
> > >
> > > You use an if/else below in a similar case, please be consistent (IMHO
> > > if/else is a bit more compact, so I'd use that here).
> > >
> > sean wang had review it. the link
> > http://lists.infradead.org/pipermail/linux-mediatek/2018-December/016292.html
> >
ok, i will use if/else
> > > > + mtk_uart_apdma_chan_write(c,
> > > > + VFF_INT_FLAG, VFF_RX_INT_FLAG_CLR_B);
> > > > + mtk_uart_apdma_start_rx(c);
> > > > + break;
> > > > + case DMA_MEM_TO_DEV:
> > > > + d = c->desc;
> > > > +
> > > > + mtk_uart_apdma_chan_write(c,
> > > > + VFF_INT_FLAG, VFF_TX_INT_FLAG_CLR_B);
> > > > +
> > > > + if (d->avail_len != 0U) {
> > > > + mtk_uart_apdma_start_tx(c);
> > > > + } else {
> > > > + list_del(&d->vd.node);
> > > > + vchan_cookie_complete(&d->vd);
> > > > + }
> > > > + break;
> > > > + default:
> > > > + break;
> > > > + }
> > > > + spin_unlock_irqrestore(&c->vc.lock, flags);
> > > > +
> > > > + return IRQ_HANDLED;
> > > > +}
> > > > +
> > > > +static int mtk_uart_apdma_alloc_chan_resources(struct dma_chan *chan)
> > > > +{
> > > > + struct mtk_uart_apdmadev *mtkd = to_mtk_uart_apdma_dev(chan->device);
> > > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > > + u32 status;
> > > > + int ret = -EBUSY;
>
> No need to init ret.
>
ok.
> > > > +
> > > > + pm_runtime_get_sync(mtkd->ddev.dev);
> > > > +
> > > > + mtk_uart_apdma_chan_write(c, VFF_ADDR, 0);
> > > > + mtk_uart_apdma_chan_write(c, VFF_THRE, 0);
> > > > + mtk_uart_apdma_chan_write(c, VFF_LEN, 0);
> > > > + mtk_uart_apdma_chan_write(c, VFF_RST, VFF_WARM_RST_B);
> > > > +
> > > > + ret = readx_poll_timeout(readl,
> > > > + c->base + VFF_EN,
> > > > + status, status == 0, 10, 100);
> > > > + if (ret) {
> > > > + dev_err(c->vc.chan.device->dev,
> > > > + "dma reset: fail, timeout\n");
> > > > + goto exit;
>
> Just return ret, since you don't do any cleanup in exit:.
>
ok.
> > > > + }
> > > > +
> > > > + if (!c->requested) {
> > > > + c->requested = true;
> > > > + ret = request_irq(mtkd->dma_irq[chan->chan_id],
> > > > + mtk_uart_apdma_irq_handler,
> > > > + IRQF_TRIGGER_NONE,
> > > > + KBUILD_MODNAME, chan);
> > > > + if (ret < 0) {
> > > > + dev_err(chan->device->dev, "Can't request dma IRQ\n");
> > > > + return -EINVAL;
> > > > + }
> > > > + }
> > > > +
> > > > + if (mtkd->support_33bits)
> > > > + mtk_uart_apdma_chan_write(c,
> > > > + VFF_4G_SUPPORT, VFF_4G_SUPPORT_CLR_B);
> > > > +
> > > > +exit:
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static void mtk_uart_apdma_free_chan_resources(struct dma_chan *chan)
> > > > +{
> > > > + struct mtk_uart_apdmadev *mtkd = to_mtk_uart_apdma_dev(chan->device);
> > > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > > +
> > > > + if (c->requested) {
> > > > + c->requested = false;
> > > > + free_irq(mtkd->dma_irq[chan->chan_id], chan);
> > > > + }
> > > > +
> > > > + tasklet_kill(&c->vc.task);
> > > > +
> > > > + vchan_free_chan_resources(&c->vc);
> > > > +
> > > > + pm_runtime_put_sync(mtkd->ddev.dev);
> > > > +}
> > > > +
> > > > +static enum dma_status mtk_uart_apdma_tx_status(struct dma_chan *chan,
> > > > + dma_cookie_t cookie,
> > > > + struct dma_tx_state *txstate)
> > > > +{
> > > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > > + enum dma_status ret;
> > > > + unsigned long flags;
> > > > +
> > > > + if (!txstate)
> > > > + return DMA_ERROR;
> > > > +
> > > > + ret = dma_cookie_status(chan, cookie, txstate);
> > > > + spin_lock_irqsave(&c->vc.lock, flags);
> > > > + if (ret == DMA_IN_PROGRESS) {
> > > > + c->rx_status = mtk_uart_apdma_chan_read(c, VFF_RPT)
> > > > + & MTK_UART_APDMA_RING_SIZE;
> > > > + dma_set_residue(txstate, c->rx_status);
> > > > + } else if (ret == DMA_COMPLETE && c->cfg.direction == DMA_DEV_TO_MEM) {
> > > > + dma_set_residue(txstate, c->rx_status);
> > > > + } else {
> > > > + dma_set_residue(txstate, 0);
> > > > + }
> > > > + spin_unlock_irqrestore(&c->vc.lock, flags);
> > > > +
> > > > + return ret;
> > > > +}
> > > > +
> > > > +/*
> > > > + * dmaengine_prep_slave_single will call the function. and sglen is 1.
> > > > + * 8250 uart using one ring buffer, and deal with one sg.
> > > > + */
> > > > +static struct dma_async_tx_descriptor *mtk_uart_apdma_prep_slave_sg
> > > > + (struct dma_chan *chan, struct scatterlist *sgl,
> > > > + unsigned int sglen, enum dma_transfer_direction dir,
>
> Too many spaces (or a tab?) between `sglen,` and `enum`
>
ok. i will modify it
> > > > + unsigned long tx_flags, void *context)
> > > > +{
> > > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > > + struct mtk_uart_apdma_desc *d;
> > > > +
> > > > + if ((dir != DMA_DEV_TO_MEM) &&
> > > > + (dir != DMA_MEM_TO_DEV)) {
> > > > + dev_err(chan->device->dev, "bad direction\n");
> > > > + return NULL;
> > > > + }
> > > > +
> > > > + /* Now allocate and setup the descriptor */
> > > > + d = kzalloc(sizeof(*d), GFP_ATOMIC);
> > > > + if (!d)
> > > > + return NULL;
> > > > +
> > > > + /* sglen is 1 */
> > > > + d->avail_len = sg_dma_len(sgl);
> > > > +
> > > > + return vchan_tx_prep(&c->vc, &d->vd, tx_flags);
> > > > +}
> > > > +
> > > > +static void mtk_uart_apdma_issue_pending(struct dma_chan *chan)
> > > > +{
> > > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > > + struct virt_dma_desc *vd;
> > > > + unsigned long flags;
> > > > +
> > > > + spin_lock_irqsave(&c->vc.lock, flags);
> > > > + if (c->cfg.direction == DMA_DEV_TO_MEM) {
> > > > + if (vchan_issue_pending(&c->vc) && !c->desc) {
> > > > + vd = vchan_next_desc(&c->vc);
> > > > + c->desc = to_mtk_uart_apdma_desc(&vd->tx);
> > > > + mtk_uart_apdma_start_rx(c);
> > > > + }
> > > > + } else if (c->cfg.direction == DMA_MEM_TO_DEV) {
> > > > + if (vchan_issue_pending(&c->vc) && !c->desc) {
> > > > + vd = vchan_next_desc(&c->vc);
> > > > + c->desc = to_mtk_uart_apdma_desc(&vd->tx);
> > > > + mtk_uart_apdma_start_tx(c);
> > > > + }
> > > > + }
> > > > + spin_unlock_irqrestore(&c->vc.lock, flags);
> > > > +}
> > > > +
> > > > +static int mtk_uart_apdma_slave_config(struct dma_chan *chan,
> > > > + struct dma_slave_config *cfg)
> > > > +{
> > > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > > + struct mtk_uart_apdmadev *mtkd =
> > > > + to_mtk_uart_apdma_dev(c->vc.chan.device);
> > > > +
> > > > + c->cfg = *cfg;
> > > > +
> > > > + if (cfg->direction == DMA_DEV_TO_MEM) {
> > > > + unsigned int rx_len = cfg->src_addr_width * 1024;
> > > > +
> > > > + mtk_uart_apdma_chan_write(c, VFF_ADDR, cfg->src_addr);
> > > > + mtk_uart_apdma_chan_write(c, VFF_LEN, rx_len);
> > > > + mtk_uart_apdma_chan_write(c, VFF_THRE, VFF_RX_THRE(rx_len));
> > > > + mtk_uart_apdma_chan_write(c,
> > > > + VFF_INT_EN, VFF_RX_INT_EN0_B
> > > > + | VFF_RX_INT_EN1_B);
> > > > + mtk_uart_apdma_chan_write(c, VFF_RPT, 0);
> > > > + mtk_uart_apdma_chan_write(c,
> > > > + VFF_INT_FLAG, VFF_RX_INT_FLAG_CLR_B);
> > > > + mtk_uart_apdma_chan_write(c, VFF_EN, VFF_EN_B);
> > > > + } else if (cfg->direction == DMA_MEM_TO_DEV) {
> > > > + unsigned int tx_len = cfg->dst_addr_width * 1024;
> > > > +
> > > > + mtk_uart_apdma_chan_write(c, VFF_ADDR, cfg->dst_addr);
> > > > + mtk_uart_apdma_chan_write(c, VFF_LEN, tx_len);
> > > > + mtk_uart_apdma_chan_write(c, VFF_THRE, VFF_TX_THRE(tx_len));
> > > > + mtk_uart_apdma_chan_write(c, VFF_WPT, 0);
> > > > + mtk_uart_apdma_chan_write(c,
> > > > + VFF_INT_FLAG, VFF_TX_INT_FLAG_CLR_B);
> > > > + mtk_uart_apdma_chan_write(c, VFF_EN, VFF_EN_B);
> > > > + }
> > > > +
> > > > + if (mtkd->support_33bits)
> > > > + mtk_uart_apdma_chan_write(c, VFF_4G_SUPPORT, VFF_4G_SUPPORT_B);
> > > > +
> > > > + if (mtk_uart_apdma_chan_read(c, VFF_EN) != VFF_EN_B) {
> > > > + dev_err(chan->device->dev,
> > > > + "config dma dir[%d] fail\n", cfg->direction);
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int mtk_uart_apdma_terminate_all(struct dma_chan *chan)
> > > > +{
> > > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > > + unsigned long flags;
> > > > + u32 status;
> > > > + int ret;
> > > > +
> > > > + spin_lock_irqsave(&c->vc.lock, flags);
> > > > +
> > > > + mtk_uart_apdma_chan_write(c, VFF_FLUSH, VFF_FLUSH_CLR_B);
> > > > + /* Wait for flush */
> > > > + ret = readx_poll_timeout(readl,
> > > > + c->base + VFF_FLUSH,
> > > > + status,
> > > > + (status & VFF_FLUSH_B) != VFF_FLUSH_B,
> > > > + 10, 100);
> > > > + if (ret)
> > > > + dev_err(c->vc.chan.device->dev,
> > > > + "dma stop: polling FLUSH fail, DEBUG=0x%x\n",
> > > > + mtk_uart_apdma_chan_read(c, VFF_DEBUG_STATUS));
> > > > +
> > > > + /*set stop as 1 -> wait until en is 0 -> set stop as 0*/
> > > > + mtk_uart_apdma_chan_write(c, VFF_STOP, VFF_STOP_B);
> > > > + ret = readx_poll_timeout(readl,
> > > > + c->base + VFF_EN,
> > > > + status, status == 0, 10, 100);
> > > > + if (ret)
> > > > + dev_err(c->vc.chan.device->dev,
> > > > + "dma stop: polling VFF_EN fail, DEBUG=0x%x\n",
> > > > + mtk_uart_apdma_chan_read(c, VFF_DEBUG_STATUS));
> > > > +
> > > > + mtk_uart_apdma_chan_write(c, VFF_STOP, VFF_STOP_CLR_B);
> > > > + mtk_uart_apdma_chan_write(c, VFF_INT_EN, VFF_INT_EN_CLR_B);
> > > > +
> > > > + switch (c->cfg.direction) {
> > > > + case DMA_DEV_TO_MEM:
> > >
> > > Ditto, if/else?
> > >
> > ditto.
> > http://lists.infradead.org/pipermail/linux-mediatek/2018-December/016292.html
>
> Well, I disagree, especially in this case: We turn 4 lines of code into 10...
>
i see, i will using if/else
> > > > + mtk_uart_apdma_chan_write(c,
> > > > + VFF_INT_FLAG, VFF_RX_INT_FLAG_CLR_B);
> > > > + break;
> > > > + case DMA_MEM_TO_DEV:
> > > > + mtk_uart_apdma_chan_write(c,
> > > > + VFF_INT_FLAG, VFF_TX_INT_FLAG_CLR_B);
> > > > + break;
> > > > + default:
> > > > + break;
> > > > + }
> > > > +
> > > > + spin_unlock_irqrestore(&c->vc.lock, flags);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int mtk_uart_apdma_device_pause(struct dma_chan *chan)
> > > > +{
> > > > + /* just for check caps pass */
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int mtk_uart_apdma_device_resume(struct dma_chan *chan)
> > > > +{
> > > > + /* just for check caps pass */
> > > > + return 0;
> > > > +}
>
> Why do you need these? Seems wrong to advertise device_pause/resume
> (and the caps) if we don't actually support that?
>
in serial 8250_dma.c file, when request DMA, will calll
'dma_get_slave_caps' to get the caps. and the will check caps.cmd_pause.
the pause is device_pause. our device can't support the functions. but
to check pass, need add the fake function. or you can give better
comments. thanks.
> > > > +
> > > > +static void mtk_uart_apdma_free(struct mtk_uart_apdmadev *mtkd)
> > > > +{
> > > > + while (list_empty(&mtkd->ddev.channels) == 0) {
> > > > + struct mtk_chan *c = list_first_entry(&mtkd->ddev.channels,
> > > > + struct mtk_chan, vc.chan.device_node);
> > > > +
> > > > + list_del(&c->vc.chan.device_node);
> > > > + tasklet_kill(&c->vc.task);
> > > > + }
> > > > +}
> > > > +
> > > > +static const struct of_device_id mtk_uart_apdma_match[] = {
> > > > + { .compatible = "mediatek,mt6577-uart-dma", },
> > > > + { /* sentinel */ },
> > > > +};
> > > > +MODULE_DEVICE_TABLE(of, mtk_uart_apdma_match);
> > > > +
> > > > +static int mtk_uart_apdma_probe(struct platform_device *pdev)
> > > > +{
> > > > + struct mtk_uart_apdmadev *mtkd;
> > > > + struct resource *res;
> > > > + struct mtk_chan *c;
> > > > + unsigned int i;
> > > > + int rc;
> > > > +
> > > > + mtkd = devm_kzalloc(&pdev->dev, sizeof(*mtkd), GFP_KERNEL);
> > > > + if (!mtkd)
> > > > + return -ENOMEM;
> > > > +
> > > > + mtkd->clk = devm_clk_get(&pdev->dev, NULL);
> > > > + if (IS_ERR(mtkd->clk)) {
> > > > + dev_err(&pdev->dev, "No clock specified\n");
> > > > + rc = PTR_ERR(mtkd->clk);
> > > > + goto err_no_dma;
> > > > + }
> > > > +
> > > > + if (of_property_read_bool(pdev->dev.of_node, "dma-33bits")) {
> > > > + dev_info(&pdev->dev, "Support dma 33bits\n");
> > >
> > > Drop this, a bit verbose.
> > >
> > > > + mtkd->support_33bits = true;
> > > > + }
> > > > +
> > > > + rc = dma_set_mask_and_coherent(&pdev->dev,
> > > > + DMA_BIT_MASK(32 | mtkd->support_33bits));
> > > > + if (rc)
> > > > + goto err_no_dma;
> > > > +
> > > > + dma_cap_set(DMA_SLAVE, mtkd->ddev.cap_mask);
> > > > + mtkd->ddev.device_alloc_chan_resources =
> > > > + mtk_uart_apdma_alloc_chan_resources;
> > > > + mtkd->ddev.device_free_chan_resources =
> > > > + mtk_uart_apdma_free_chan_resources;
> > > > + mtkd->ddev.device_tx_status = mtk_uart_apdma_tx_status;
> > > > + mtkd->ddev.device_issue_pending = mtk_uart_apdma_issue_pending;
> > > > + mtkd->ddev.device_prep_slave_sg = mtk_uart_apdma_prep_slave_sg;
> > > > + mtkd->ddev.device_config = mtk_uart_apdma_slave_config;
> > > > + mtkd->ddev.device_pause = mtk_uart_apdma_device_pause;
> > > > + mtkd->ddev.device_resume = mtk_uart_apdma_device_resume;
> > > > + mtkd->ddev.device_terminate_all = mtk_uart_apdma_terminate_all;
> > > > + mtkd->ddev.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE);
> > > > + mtkd->ddev.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE);
> > > > + mtkd->ddev.directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
> > > > + mtkd->ddev.residue_granularity = DMA_RESIDUE_GRANULARITY_SEGMENT;
> > > > + mtkd->ddev.dev = &pdev->dev;
> > > > + INIT_LIST_HEAD(&mtkd->ddev.channels);
> > > > +
> > > > + for (i = 0; i < MTK_UART_APDMA_CHANNELS; i++) {
> > > > + c = devm_kzalloc(mtkd->ddev.dev, sizeof(*c), GFP_KERNEL);
> > > > + if (!c) {
> > > > + rc = -ENODEV;
> > > > + goto err_no_dma;
> > > > + }
> > > > +
> > > > + res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> > > > + if (!res) {
> > > > + rc = -ENODEV;
> > > > + goto err_no_dma;
> > > > + }
> > > > +
> > > > + c->base = devm_ioremap_resource(&pdev->dev, res);
> > > > + if (IS_ERR(c->base)) {
> > > > + rc = PTR_ERR(c->base);
> > > > + goto err_no_dma;
> > > > + }
> > > > + c->requested = false;
> > > > + c->vc.desc_free = mtk_uart_apdma_desc_free;
> > > > + vchan_init(&c->vc, &mtkd->ddev);
> > > > +
> > > > + mtkd->dma_irq[i] = platform_get_irq(pdev, i);
> > > > + if ((int)mtkd->dma_irq[i] < 0) {
> > > > + dev_err(&pdev->dev, "failed to get IRQ[%d]\n", i);
> > > > + rc = -EINVAL;
> > > > + goto err_no_dma;
> > > > + }
> > > > + }
> > > > +
> > > > + pm_runtime_enable(&pdev->dev);
> > > > + pm_runtime_set_active(&pdev->dev);
> > > > +
> > > > + rc = dma_async_device_register(&mtkd->ddev);
> > > > + if (rc)
> > > > + goto rpm_disable;
> > > > +
> > > > + platform_set_drvdata(pdev, mtkd);
> > > > +
> > > > + if (pdev->dev.of_node) {
> > > > + /* Device-tree DMA controller registration */
> > > > + rc = of_dma_controller_register(pdev->dev.of_node,
> > > > + of_dma_xlate_by_chan_id,
> > > > + mtkd);
> > > > + if (rc)
> > > > + goto dma_remove;
> > > > + }
> > > > +
> > > > + return rc;
> > > > +
> > > > +dma_remove:
> > > > + dma_async_device_unregister(&mtkd->ddev);
> > > > +rpm_disable:
> > > > + pm_runtime_disable(&pdev->dev);
> > > > +err_no_dma:
> > > > + mtk_uart_apdma_free(mtkd);
>
> It's not very correct to call this before
> INIT_LIST_HEAD(&mtkd->ddev.channels);
>
> I'd just call `return rc;` directly for all instances before that line.
>
ok, i will modify it.
> > > > + return rc;
> > > > +}
> > > > +
> > > > +static int mtk_uart_apdma_remove(struct platform_device *pdev)
> > > > +{
> > > > + struct mtk_uart_apdmadev *mtkd = platform_get_drvdata(pdev);
> > > > +
> > > > + if (pdev->dev.of_node)
> > > > + of_dma_controller_free(pdev->dev.of_node);
> > > > +
> > > > + pm_runtime_disable(&pdev->dev);
> > > > + pm_runtime_put_noidle(&pdev->dev);
> > > > +
> > > > + dma_async_device_unregister(&mtkd->ddev);
> > > > + mtk_uart_apdma_free(mtkd);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +#ifdef CONFIG_PM_SLEEP
> > > > +static int mtk_uart_apdma_suspend(struct device *dev)
> > > > +{
> > > > + struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
> > > > +
> > > > + if (!pm_runtime_suspended(dev))
> > > > + clk_disable_unprepare(mtkd->clk);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int mtk_uart_apdma_resume(struct device *dev)
> > > > +{
> > > > + int ret;
> > > > + struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
> > > > +
> > > > + if (!pm_runtime_suspended(dev)) {
> > > > + ret = clk_prepare_enable(mtkd->clk);
> > > > + if (ret)
> > > > + return ret;
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +#endif /* CONFIG_PM_SLEEP */
> > > > +
> > > > +#ifdef CONFIG_PM
> > > > +static int mtk_uart_apdma_runtime_suspend(struct device *dev)
> > > > +{
> > > > + struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
> > > > +
> > > > + clk_disable_unprepare(mtkd->clk);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int mtk_uart_apdma_runtime_resume(struct device *dev)
> > > > +{
> > > > + int ret;
> > > > + struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
> > > > +
> > > > + ret = clk_prepare_enable(mtkd->clk);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +#endif /* CONFIG_PM */
> > > > +
> > > > +static const struct dev_pm_ops mtk_uart_apdma_pm_ops = {
> > > > + SET_SYSTEM_SLEEP_PM_OPS(mtk_uart_apdma_suspend, mtk_uart_apdma_resume)
> > > > + SET_RUNTIME_PM_OPS(mtk_uart_apdma_runtime_suspend,
> > > > + mtk_uart_apdma_runtime_resume, NULL)
> > > > +};
> > > > +
> > > > +static struct platform_driver mtk_uart_apdma_driver = {
> > > > + .probe = mtk_uart_apdma_probe,
> > > > + .remove = mtk_uart_apdma_remove,
> > > > + .driver = {
> > > > + .name = KBUILD_MODNAME,
> > > > + .pm = &mtk_uart_apdma_pm_ops,
> > > > + .of_match_table = of_match_ptr(mtk_uart_apdma_match),
> > > > + },
> > > > +};
> > > > +
> > > > +module_platform_driver(mtk_uart_apdma_driver);
> > > > +
> > > > +MODULE_DESCRIPTION("MediaTek UART APDMA Controller Driver");
> > > > +MODULE_AUTHOR("Long Cheng <long.cheng@mediatek.com>");
> > > > +MODULE_LICENSE("GPL v2");
> > > > +
> > > > diff --git a/drivers/dma/mediatek/Kconfig b/drivers/dma/mediatek/Kconfig
> > > > index 27bac0b..1a523c87 100644
> > > > --- a/drivers/dma/mediatek/Kconfig
> > > > +++ b/drivers/dma/mediatek/Kconfig
> > > > @@ -1,4 +1,15 @@
> > > >
> > > > +config DMA_MTK_UART
> > > > + tristate "MediaTek SoCs APDMA support for UART"
> > > > + depends on OF && SERIAL_8250_MT6577
> > > > + select DMA_ENGINE
> > > > + select DMA_VIRTUAL_CHANNELS
> > > > + help
> > > > + Support for the UART DMA engine found on MediaTek MTK SoCs.
> > > > + when SERIAL_8250_MT6577 is enabled, and if you want to use DMA,
> > > > + you can enable the config. the DMA engine can only be used
> > > > + with MediaTek SoCs.
> > > > +
> > > > config MTK_HSDMA
> > > > tristate "MediaTek High-Speed DMA controller support"
> > > > depends on ARCH_MEDIATEK || COMPILE_TEST
> > > > diff --git a/drivers/dma/mediatek/Makefile b/drivers/dma/mediatek/Makefile
> > > > index 6e778f8..2f2efd9 100644
> > > > --- a/drivers/dma/mediatek/Makefile
> > > > +++ b/drivers/dma/mediatek/Makefile
> > > > @@ -1 +1,2 @@
> > > > +obj-$(CONFIG_DMA_MTK_UART) += 8250_mtk_dma.o
> > > > obj-$(CONFIG_MTK_HSDMA) += mtk-hsdma.o
> > > > --
> > > > 1.7.9.5
> > > >
> >
> >
^ permalink raw reply
* [v7,1/2] dmaengine: 8250_mtk_dma: add MediaTek uart DMA support
From: Nicolas Boichat @ 2018-12-26 9:28 UTC (permalink / raw)
To: Long Cheng
Cc: Vinod Koul, Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee,
Sean Wang, Matthias Brugger, Dan Williams, Greg Kroah-Hartman,
Jiri Slaby, Sean Wang, dmaengine, devicetree,
linux-arm Mailing List, linux-mediatek, lkml, linux-serial,
srv_heupstream, Yingjoe Chen, YT Shen, Zhenbao Liu
On Wed, Dec 26, 2018 at 2:35 PM Long Cheng <long.cheng@mediatek.com> wrote:
>
> On Wed, 2018-12-26 at 08:05 +0800, Nicolas Boichat wrote:
>
> thanks.
>
> > On Tue, Dec 25, 2018 at 8:06 PM Long Cheng <long.cheng@mediatek.com> wrote:
> > >
> > > Thanks for your comments.
> > >
> > > On Tue, 2018-12-25 at 15:16 +0800, Nicolas Boichat wrote:
> > > > Not a full review, a few comments below.
> > > >
> > > > Thanks,
> > > >
> > >
> > > would you like help to full review at this version patch? and then i
> > > can modify these at next version patch. thanks.
> >
> > Added a few more comments ,-)
> >
> > > > On Tue, Dec 25, 2018 at 9:27 AM Long Cheng <long.cheng@mediatek.com> wrote:
> > > > >
> > > > > In DMA engine framework, add 8250 uart dma to support MediaTek uart.
> > > > > If MediaTek uart enabled(SERIAL_8250_MT6577), and want to improve
> > > > > the performance, can enable the function.
> > > > >
> > > > > Signed-off-by: Long Cheng <long.cheng@mediatek.com>
> > > > > ---
> > > > > drivers/dma/mediatek/8250_mtk_dma.c | 694 +++++++++++++++++++++++++++++++++++
> > > > > drivers/dma/mediatek/Kconfig | 11 +
> > > > > drivers/dma/mediatek/Makefile | 1 +
> > > > > 3 files changed, 706 insertions(+)
> > > > > create mode 100644 drivers/dma/mediatek/8250_mtk_dma.c
> > > > >
> > > > > diff --git a/drivers/dma/mediatek/8250_mtk_dma.c b/drivers/dma/mediatek/8250_mtk_dma.c
> > > > > new file mode 100644
> > > > > index 0000000..c4090f2
> > > > > --- /dev/null
> > > > > +++ b/drivers/dma/mediatek/8250_mtk_dma.c
> > > > > @@ -0,0 +1,694 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > +/*
> > > > > + * MediaTek 8250 DMA driver.
> > > > > + *
> > > > > + * Copyright (c) 2018 MediaTek Inc.
> > > > > + * Author: Long Cheng <long.cheng@mediatek.com>
> > > > > + */
> > > > > +
> > > > > +#include <linux/clk.h>
> > > > > +#include <linux/dmaengine.h>
> > > > > +#include <linux/dma-mapping.h>
> > > > > +#include <linux/err.h>
> > > > > +#include <linux/init.h>
> > > > > +#include <linux/interrupt.h>
> > > > > +#include <linux/list.h>
> > > > > +#include <linux/kernel.h>
> > > > > +#include <linux/module.h>
> > > > > +#include <linux/of_dma.h>
> > > > > +#include <linux/of_device.h>
> > > > > +#include <linux/platform_device.h>
> > > > > +#include <linux/slab.h>
> > > > > +#include <linux/spinlock.h>
> > > > > +#include <linux/pm_runtime.h>
> > > > > +#include <linux/iopoll.h>
> > > >
> > > > Alphabetical order.
> > >
> > > ok, i will order.
> > >
> > > >
> > > > > +
> > > > > +#include "../virt-dma.h"
> > > > > +
> > > > > +#define MTK_UART_APDMA_CHANNELS (CONFIG_SERIAL_8250_NR_UARTS * 2)
> > > > > +
> > > > > +#define VFF_EN_B BIT(0)
> > > > > +#define VFF_STOP_B BIT(0)
> > > > > +#define VFF_FLUSH_B BIT(0)
> > > > > +#define VFF_4G_SUPPORT_B BIT(0)
> > > > > +#define VFF_RX_INT_EN0_B BIT(0) /*rx valid size >= vff thre*/
> > > > > +#define VFF_RX_INT_EN1_B BIT(1)
> > > > > +#define VFF_TX_INT_EN_B BIT(0) /*tx left size >= vff thre*/
> > > > > +#define VFF_WARM_RST_B BIT(0)
> > > > > +#define VFF_RX_INT_FLAG_CLR_B (BIT(0) | BIT(1))
> > > > > +#define VFF_TX_INT_FLAG_CLR_B 0
> > > > > +#define VFF_STOP_CLR_B 0
> > > > > +#define VFF_FLUSH_CLR_B 0
> > > > > +#define VFF_INT_EN_CLR_B 0
> > > > > +#define VFF_4G_SUPPORT_CLR_B 0
> > > > > +
> > > > > +/* interrupt trigger level for tx */
> > > > > +#define VFF_TX_THRE(n) ((n) * 7 / 8)
> > > > > +/* interrupt trigger level for rx */
> > > > > +#define VFF_RX_THRE(n) ((n) * 3 / 4)
> > > > > +
> > > > > +#define MTK_UART_APDMA_RING_SIZE 0xffffU
> > > > > +/* invert this bit when wrap ring head again*/
> > > > > +#define MTK_UART_APDMA_RING_WRAP 0x10000U
> > > > > +
> > > > > +#define VFF_INT_FLAG 0x00
> > > > > +#define VFF_INT_EN 0x04
> > > > > +#define VFF_EN 0x08
> > > > > +#define VFF_RST 0x0c
> > > > > +#define VFF_STOP 0x10
> > > > > +#define VFF_FLUSH 0x14
> > > > > +#define VFF_ADDR 0x1c
> > > > > +#define VFF_LEN 0x24
> > > > > +#define VFF_THRE 0x28
> > > > > +#define VFF_WPT 0x2c
> > > > > +#define VFF_RPT 0x30
> > > > > +/*TX: the buffer size HW can read. RX: the buffer size SW can read.*/
> > > > > +#define VFF_VALID_SIZE 0x3c
> > > > > +/*TX: the buffer size SW can write. RX: the buffer size HW can write.*/
> > > > > +#define VFF_LEFT_SIZE 0x40
> > > > > +#define VFF_DEBUG_STATUS 0x50
> > > > > +#define VFF_4G_SUPPORT 0x54
> > > > > +
> > > > > +struct mtk_uart_apdmadev {
> > > > > + struct dma_device ddev;
> > > > > + struct clk *clk;
> > > > > + bool support_33bits;
> > > > > + unsigned int dma_irq[MTK_UART_APDMA_CHANNELS];
> > > > > +};
> > > > > +
> > > > > +struct mtk_uart_apdma_desc {
> > > > > + struct virt_dma_desc vd;
> > > > > +
> > > > > + unsigned int avail_len;
> > > > > +};
> > > > > +
> > > > > +struct mtk_chan {
> > > > > + struct virt_dma_chan vc;
> > > > > + struct dma_slave_config cfg;
> > > > > + void __iomem *base;
> > > > > + struct mtk_uart_apdma_desc *desc;
> > > > > +
> > > > > + bool requested;
> > > > > +
> > > > > + unsigned int rx_status;
> > > > > +};
> > > > > +
> > > > > +static inline struct mtk_uart_apdmadev *
> > > > > +to_mtk_uart_apdma_dev(struct dma_device *d)
> > > > > +{
> > > > > + return container_of(d, struct mtk_uart_apdmadev, ddev);
> > > > > +}
> > > > > +
> > > > > +static inline struct mtk_chan *to_mtk_uart_apdma_chan(struct dma_chan *c)
> > > > > +{
> > > > > + return container_of(c, struct mtk_chan, vc.chan);
> > > > > +}
> > > > > +
> > > > > +static inline struct mtk_uart_apdma_desc *to_mtk_uart_apdma_desc
> > > > > + (struct dma_async_tx_descriptor *t)
> > > > > +{
> > > > > + return container_of(t, struct mtk_uart_apdma_desc, vd.tx);
> > > > > +}
> > > > > +
> > > > > +static void mtk_uart_apdma_chan_write(struct mtk_chan *c,
> > > > > + unsigned int reg, unsigned int val)
> > > > > +{
> > > > > + writel(val, c->base + reg);
> > > > > +}
> > > > > +
> > > > > +static unsigned int
> > > > > +mtk_uart_apdma_chan_read(struct mtk_chan *c, unsigned int reg)
> > > > > +{
> > > > > + return readl(c->base + reg);
> > > > > +}
> > > > > +
> > > > > +static void mtk_uart_apdma_desc_free(struct virt_dma_desc *vd)
> > > > > +{
> > > > > + struct dma_chan *chan = vd->tx.chan;
> > > > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > > > +
> > > > > + kfree(c->desc);
> > > > > + c->desc = NULL;
> > > >
> > > > Why? I'm afraid this may mask double-free.
> > > >
> > >
> > > i will remove it.
> > >
> > > > > +}
> > > > > +
> > > > > +static void mtk_uart_apdma_start_tx(struct mtk_chan *c)
> > > > > +{
> > > > > + unsigned int txcount = c->desc->avail_len;
> > > >
> > > > Variable name is confusing... I'd rather have a boolean `transmitted`
> > > > (or something), set to 0 here, and set to 1 in the loop (but again,
> > > > maybe this is not necessary at all, see below).
> > >
> > > ok, i modify the function and remove it.
> > >
> > > >
> > > > > + unsigned int len, send, left, wpt, wrap;
> > > > > +
> > > > > + if (mtk_uart_apdma_chan_read(c, VFF_LEFT_SIZE) == 0U) {
> > > > > + mtk_uart_apdma_chan_write(c, VFF_INT_EN, VFF_TX_INT_EN_B);
> > > >
> > > > I'd add a "return;" here and de-indent the rest of the function (that
> > > > should help to make it more readable).
> > >
> > > add
> > >
> > > >
> > > > > + } else {
> > > > > + len = mtk_uart_apdma_chan_read(c, VFF_LEN);
> > > >
> > > > You make an assumption below that len is always a power of 2. I wonder
> > > > if we want to validate that with some BUG/WARN call.
> > > >
> > > > Also, I assume VFF_LEN is constant? Maybe we can read it once only in
> > > > the probe function?
> > > >
> > >
> > > no, the value of VFF_LEN isn't constant.
> > >
> > > > > +
> > > > > + while (((left = mtk_uart_apdma_chan_read(c,
> > > > > + VFF_LEFT_SIZE)) > 0U)
> > > > > + && (c->desc->avail_len != 0U)) {
> > > >
> > > > Why do we need a while loop here? LEFT_SIZE will always be smaller
> > > > than LEN, right? Shouldn't we load as much as we can, then let an
> > > > interrupt happen and reload the FIFO at that stage? Or does this help
> > > > gain a bit of extra performance?
> > >
> > > i will remove while, use 'if' instead of.
> > > >
> > > > > + send = min_t(unsigned int, left, c->desc->avail_len);
> > > > > + wpt = mtk_uart_apdma_chan_read(c, VFF_WPT);
> > > > > + wrap = wpt & MTK_UART_APDMA_RING_WRAP ?
> > > >
> > > > wrap = wrp ^ MTK_UART_APDMA_RING_WRAP ?
> > > >
> > >
> > > wrap = wpt & MTK_UART_APDMA_RING_WRAP ?
> > > i had test it.
> >
> > Sorry, I got it wrong (and my question mark is confusing), what I mean is this:
> >
> > wrap = (wrp ^ MTK_UART_APDMA_RING_WRAP) & MTK_UART_APDMA_RING_WRAP;
> >
> > But then it's marginally shorter than your solution, so maybe keep
> > yours, it's a little simpler to understand.
> >
>
> i will rename 'MTK_UART_APDMA_RING_WRAP' to 'VFF_RING_WRAP'. let code
> shorter. i will keep the code.
>
> > > > > + 0U : MTK_UART_APDMA_RING_WRAP;
> > > > > +
> > > > > + if ((wpt & (len - 1U)) + send < len)
> >
> > Is it possible for wpt > len? If not, then wpt + send < len should be enough.
> >
> > Also, wpt + send is used a few times now, so maybe have it a separate variable?
> >
> > Something like this might be nicer:
> >
> > next_wpt = wpt + send;
> > if ((wpt & (len - 1U)) + send >= len) {
> > next_wpt = next_wpt & (len - 1U);
> > if (!(wpt & MTK_UART_APDMA_RING_WRAP))
> > next_wpt |= MTK_UART_APDMA_RING_WRAP;
> > }
> > mtk_uart_apdma_chan_write(c, VFF_WPT, next_wpt);
> >
> > Or if the assumption than wpt < len holds:
> > if (next_wpt >= len) {
> > next_wpt = next_wpt - len;
> > ...
> >
>
> i will modify it like your comments,
> next_wpt = wpt + send;
> > if ((wpt & (len - 1U)) + send >= len) {
> > next_wpt = next_wpt & (len - 1U);
> > if (!(wpt & MTK_UART_APDMA_RING_WRAP))
> > next_wpt |= MTK_UART_APDMA_RING_WRAP;
> >
> i think that it's better.
>
> > > > > + mtk_uart_apdma_chan_write(c,
> > > > > + VFF_WPT, wpt + send);
> > > > > + else
> > > > > + mtk_uart_apdma_chan_write(c, VFF_WPT,
> > > > > + ((wpt + send) & (len - 1U))
> > > > > + | wrap);
> > > > > +
> > > > > + c->desc->avail_len -= send;
> > > > > + }
> > > > > +
> > > > > + if (txcount != c->desc->avail_len) {
> > > >
> > > > We know mtk_uart_apdma_chan_read(c, VFF_LEFT_SIZE) > 0 at least once,
> > > > so this test should always be true.
> > > >
> > > > Unless we have c->desc->avail_len == 0, but then we won't even call
> > > > this function... So this is always true.
> > > >
> > >
> > > yes. you are right. i will remove it.
> > >
> > > > > + mtk_uart_apdma_chan_write(c,
> > > > > + VFF_INT_EN, VFF_TX_INT_EN_B);
> > > > > + if (mtk_uart_apdma_chan_read(c,
> > > > > + VFF_FLUSH) == 0U)
> > > > > + mtk_uart_apdma_chan_write(c,
> > > > > + VFF_FLUSH, VFF_FLUSH_B);
> > > > > + }
> > > > > + }
> > > > > +}
> > > > > +
> > > > > +static void mtk_uart_apdma_start_rx(struct mtk_chan *c)
> > > > > +{
> > > > > + struct mtk_uart_apdma_desc *d = c->desc;
> > > > > + unsigned int rx_len, wg, rg, count;
> > > > > +
> > > > > + if (mtk_uart_apdma_chan_read(c, VFF_VALID_SIZE) == 0U)
> > > > > + return;
> > > > > +
> > > > > + if (d && vchan_next_desc(&c->vc)) {
> >
> > I'd negate the test to de-indent the rest of the code:
> > if (!d || !vchan_next_desc(&c->vc))
> > return;
> >
>
> ok. i will modify it like
> if ((mtk_uart_apdma_chan_read(c, VFF_VALID_SIZE) == 0U) ||
> !d || !vchan_next_desc(&c->vc))
> return;
>
> > > > > + rx_len = mtk_uart_apdma_chan_read(c, VFF_LEN);
> > > > > + rg = mtk_uart_apdma_chan_read(c, VFF_RPT);
> > > > > + wg = mtk_uart_apdma_chan_read(c, VFF_WPT);
> > > > > + count = ((rg ^ wg) & MTK_UART_APDMA_RING_WRAP) ?
> > > > > + ((wg & MTK_UART_APDMA_RING_SIZE) +
> > > > > + rx_len - (rg & MTK_UART_APDMA_RING_SIZE)) :
> > > > > + ((wg & MTK_UART_APDMA_RING_SIZE) -
> > > > > + (rg & MTK_UART_APDMA_RING_SIZE));
> > > >
> > > > This is hard to read, please use a if/else.
> > > >
> > >
> > > ok.
> > >
> > > > > +
> > > > > + c->rx_status = count;
> > > > > + mtk_uart_apdma_chan_write(c, VFF_RPT, wg);
> > > > > +
> > > > > + list_del(&d->vd.node);
> > > > > + vchan_cookie_complete(&d->vd);
> > > > > + }
> > > > > +}
> > > > > +
> > > > > +static irqreturn_t mtk_uart_apdma_irq_handler(int irq, void *dev_id)
> > > > > +{
> > > > > + struct dma_chan *chan = (struct dma_chan *)dev_id;
> > > > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > > > + struct mtk_uart_apdma_desc *d;
> > > > > + unsigned long flags;
> > > > > +
> > > > > + spin_lock_irqsave(&c->vc.lock, flags);
> > > > > + switch (c->cfg.direction) {
> > > > > + case DMA_DEV_TO_MEM:
> > > >
> > > > You use an if/else below in a similar case, please be consistent (IMHO
> > > > if/else is a bit more compact, so I'd use that here).
> > > >
> > > sean wang had review it. the link
> > > http://lists.infradead.org/pipermail/linux-mediatek/2018-December/016292.html
> > >
>
> ok, i will use if/else
>
> > > > > + mtk_uart_apdma_chan_write(c,
> > > > > + VFF_INT_FLAG, VFF_RX_INT_FLAG_CLR_B);
> > > > > + mtk_uart_apdma_start_rx(c);
> > > > > + break;
> > > > > + case DMA_MEM_TO_DEV:
> > > > > + d = c->desc;
> > > > > +
> > > > > + mtk_uart_apdma_chan_write(c,
> > > > > + VFF_INT_FLAG, VFF_TX_INT_FLAG_CLR_B);
> > > > > +
> > > > > + if (d->avail_len != 0U) {
> > > > > + mtk_uart_apdma_start_tx(c);
> > > > > + } else {
> > > > > + list_del(&d->vd.node);
> > > > > + vchan_cookie_complete(&d->vd);
> > > > > + }
> > > > > + break;
> > > > > + default:
> > > > > + break;
> > > > > + }
> > > > > + spin_unlock_irqrestore(&c->vc.lock, flags);
> > > > > +
> > > > > + return IRQ_HANDLED;
> > > > > +}
> > > > > +
> > > > > +static int mtk_uart_apdma_alloc_chan_resources(struct dma_chan *chan)
> > > > > +{
> > > > > + struct mtk_uart_apdmadev *mtkd = to_mtk_uart_apdma_dev(chan->device);
> > > > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > > > + u32 status;
> > > > > + int ret = -EBUSY;
> >
> > No need to init ret.
> >
>
> ok.
>
> > > > > +
> > > > > + pm_runtime_get_sync(mtkd->ddev.dev);
> > > > > +
> > > > > + mtk_uart_apdma_chan_write(c, VFF_ADDR, 0);
> > > > > + mtk_uart_apdma_chan_write(c, VFF_THRE, 0);
> > > > > + mtk_uart_apdma_chan_write(c, VFF_LEN, 0);
> > > > > + mtk_uart_apdma_chan_write(c, VFF_RST, VFF_WARM_RST_B);
> > > > > +
> > > > > + ret = readx_poll_timeout(readl,
> > > > > + c->base + VFF_EN,
> > > > > + status, status == 0, 10, 100);
> > > > > + if (ret) {
> > > > > + dev_err(c->vc.chan.device->dev,
> > > > > + "dma reset: fail, timeout\n");
> > > > > + goto exit;
> >
> > Just return ret, since you don't do any cleanup in exit:.
> >
>
> ok.
>
> > > > > + }
> > > > > +
> > > > > + if (!c->requested) {
> > > > > + c->requested = true;
> > > > > + ret = request_irq(mtkd->dma_irq[chan->chan_id],
> > > > > + mtk_uart_apdma_irq_handler,
> > > > > + IRQF_TRIGGER_NONE,
> > > > > + KBUILD_MODNAME, chan);
> > > > > + if (ret < 0) {
> > > > > + dev_err(chan->device->dev, "Can't request dma IRQ\n");
> > > > > + return -EINVAL;
> > > > > + }
> > > > > + }
> > > > > +
> > > > > + if (mtkd->support_33bits)
> > > > > + mtk_uart_apdma_chan_write(c,
> > > > > + VFF_4G_SUPPORT, VFF_4G_SUPPORT_CLR_B);
> > > > > +
> > > > > +exit:
> > > > > + return ret;
> > > > > +}
> > > > > +
> > > > > +static void mtk_uart_apdma_free_chan_resources(struct dma_chan *chan)
> > > > > +{
> > > > > + struct mtk_uart_apdmadev *mtkd = to_mtk_uart_apdma_dev(chan->device);
> > > > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > > > +
> > > > > + if (c->requested) {
> > > > > + c->requested = false;
> > > > > + free_irq(mtkd->dma_irq[chan->chan_id], chan);
> > > > > + }
> > > > > +
> > > > > + tasklet_kill(&c->vc.task);
> > > > > +
> > > > > + vchan_free_chan_resources(&c->vc);
> > > > > +
> > > > > + pm_runtime_put_sync(mtkd->ddev.dev);
> > > > > +}
> > > > > +
> > > > > +static enum dma_status mtk_uart_apdma_tx_status(struct dma_chan *chan,
> > > > > + dma_cookie_t cookie,
> > > > > + struct dma_tx_state *txstate)
> > > > > +{
> > > > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > > > + enum dma_status ret;
> > > > > + unsigned long flags;
> > > > > +
> > > > > + if (!txstate)
> > > > > + return DMA_ERROR;
> > > > > +
> > > > > + ret = dma_cookie_status(chan, cookie, txstate);
> > > > > + spin_lock_irqsave(&c->vc.lock, flags);
> > > > > + if (ret == DMA_IN_PROGRESS) {
> > > > > + c->rx_status = mtk_uart_apdma_chan_read(c, VFF_RPT)
> > > > > + & MTK_UART_APDMA_RING_SIZE;
> > > > > + dma_set_residue(txstate, c->rx_status);
> > > > > + } else if (ret == DMA_COMPLETE && c->cfg.direction == DMA_DEV_TO_MEM) {
> > > > > + dma_set_residue(txstate, c->rx_status);
> > > > > + } else {
> > > > > + dma_set_residue(txstate, 0);
> > > > > + }
> > > > > + spin_unlock_irqrestore(&c->vc.lock, flags);
> > > > > +
> > > > > + return ret;
> > > > > +}
> > > > > +
> > > > > +/*
> > > > > + * dmaengine_prep_slave_single will call the function. and sglen is 1.
> > > > > + * 8250 uart using one ring buffer, and deal with one sg.
> > > > > + */
> > > > > +static struct dma_async_tx_descriptor *mtk_uart_apdma_prep_slave_sg
> > > > > + (struct dma_chan *chan, struct scatterlist *sgl,
> > > > > + unsigned int sglen, enum dma_transfer_direction dir,
> >
> > Too many spaces (or a tab?) between `sglen,` and `enum`
> >
>
> ok. i will modify it
>
> > > > > + unsigned long tx_flags, void *context)
> > > > > +{
> > > > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > > > + struct mtk_uart_apdma_desc *d;
> > > > > +
> > > > > + if ((dir != DMA_DEV_TO_MEM) &&
> > > > > + (dir != DMA_MEM_TO_DEV)) {
> > > > > + dev_err(chan->device->dev, "bad direction\n");
> > > > > + return NULL;
> > > > > + }
> > > > > +
> > > > > + /* Now allocate and setup the descriptor */
> > > > > + d = kzalloc(sizeof(*d), GFP_ATOMIC);
> > > > > + if (!d)
> > > > > + return NULL;
> > > > > +
> > > > > + /* sglen is 1 */
> > > > > + d->avail_len = sg_dma_len(sgl);
> > > > > +
> > > > > + return vchan_tx_prep(&c->vc, &d->vd, tx_flags);
> > > > > +}
> > > > > +
> > > > > +static void mtk_uart_apdma_issue_pending(struct dma_chan *chan)
> > > > > +{
> > > > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > > > + struct virt_dma_desc *vd;
> > > > > + unsigned long flags;
> > > > > +
> > > > > + spin_lock_irqsave(&c->vc.lock, flags);
> > > > > + if (c->cfg.direction == DMA_DEV_TO_MEM) {
> > > > > + if (vchan_issue_pending(&c->vc) && !c->desc) {
> > > > > + vd = vchan_next_desc(&c->vc);
> > > > > + c->desc = to_mtk_uart_apdma_desc(&vd->tx);
> > > > > + mtk_uart_apdma_start_rx(c);
> > > > > + }
> > > > > + } else if (c->cfg.direction == DMA_MEM_TO_DEV) {
> > > > > + if (vchan_issue_pending(&c->vc) && !c->desc) {
> > > > > + vd = vchan_next_desc(&c->vc);
> > > > > + c->desc = to_mtk_uart_apdma_desc(&vd->tx);
> > > > > + mtk_uart_apdma_start_tx(c);
> > > > > + }
> > > > > + }
> > > > > + spin_unlock_irqrestore(&c->vc.lock, flags);
> > > > > +}
> > > > > +
> > > > > +static int mtk_uart_apdma_slave_config(struct dma_chan *chan,
> > > > > + struct dma_slave_config *cfg)
> > > > > +{
> > > > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > > > + struct mtk_uart_apdmadev *mtkd =
> > > > > + to_mtk_uart_apdma_dev(c->vc.chan.device);
> > > > > +
> > > > > + c->cfg = *cfg;
> > > > > +
> > > > > + if (cfg->direction == DMA_DEV_TO_MEM) {
> > > > > + unsigned int rx_len = cfg->src_addr_width * 1024;
> > > > > +
> > > > > + mtk_uart_apdma_chan_write(c, VFF_ADDR, cfg->src_addr);
> > > > > + mtk_uart_apdma_chan_write(c, VFF_LEN, rx_len);
> > > > > + mtk_uart_apdma_chan_write(c, VFF_THRE, VFF_RX_THRE(rx_len));
> > > > > + mtk_uart_apdma_chan_write(c,
> > > > > + VFF_INT_EN, VFF_RX_INT_EN0_B
> > > > > + | VFF_RX_INT_EN1_B);
> > > > > + mtk_uart_apdma_chan_write(c, VFF_RPT, 0);
> > > > > + mtk_uart_apdma_chan_write(c,
> > > > > + VFF_INT_FLAG, VFF_RX_INT_FLAG_CLR_B);
> > > > > + mtk_uart_apdma_chan_write(c, VFF_EN, VFF_EN_B);
> > > > > + } else if (cfg->direction == DMA_MEM_TO_DEV) {
> > > > > + unsigned int tx_len = cfg->dst_addr_width * 1024;
> > > > > +
> > > > > + mtk_uart_apdma_chan_write(c, VFF_ADDR, cfg->dst_addr);
> > > > > + mtk_uart_apdma_chan_write(c, VFF_LEN, tx_len);
> > > > > + mtk_uart_apdma_chan_write(c, VFF_THRE, VFF_TX_THRE(tx_len));
> > > > > + mtk_uart_apdma_chan_write(c, VFF_WPT, 0);
> > > > > + mtk_uart_apdma_chan_write(c,
> > > > > + VFF_INT_FLAG, VFF_TX_INT_FLAG_CLR_B);
> > > > > + mtk_uart_apdma_chan_write(c, VFF_EN, VFF_EN_B);
> > > > > + }
> > > > > +
> > > > > + if (mtkd->support_33bits)
> > > > > + mtk_uart_apdma_chan_write(c, VFF_4G_SUPPORT, VFF_4G_SUPPORT_B);
> > > > > +
> > > > > + if (mtk_uart_apdma_chan_read(c, VFF_EN) != VFF_EN_B) {
> > > > > + dev_err(chan->device->dev,
> > > > > + "config dma dir[%d] fail\n", cfg->direction);
> > > > > + return -EINVAL;
> > > > > + }
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static int mtk_uart_apdma_terminate_all(struct dma_chan *chan)
> > > > > +{
> > > > > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > > > + unsigned long flags;
> > > > > + u32 status;
> > > > > + int ret;
> > > > > +
> > > > > + spin_lock_irqsave(&c->vc.lock, flags);
> > > > > +
> > > > > + mtk_uart_apdma_chan_write(c, VFF_FLUSH, VFF_FLUSH_CLR_B);
> > > > > + /* Wait for flush */
> > > > > + ret = readx_poll_timeout(readl,
> > > > > + c->base + VFF_FLUSH,
> > > > > + status,
> > > > > + (status & VFF_FLUSH_B) != VFF_FLUSH_B,
> > > > > + 10, 100);
> > > > > + if (ret)
> > > > > + dev_err(c->vc.chan.device->dev,
> > > > > + "dma stop: polling FLUSH fail, DEBUG=0x%x\n",
> > > > > + mtk_uart_apdma_chan_read(c, VFF_DEBUG_STATUS));
> > > > > +
> > > > > + /*set stop as 1 -> wait until en is 0 -> set stop as 0*/
> > > > > + mtk_uart_apdma_chan_write(c, VFF_STOP, VFF_STOP_B);
> > > > > + ret = readx_poll_timeout(readl,
> > > > > + c->base + VFF_EN,
> > > > > + status, status == 0, 10, 100);
> > > > > + if (ret)
> > > > > + dev_err(c->vc.chan.device->dev,
> > > > > + "dma stop: polling VFF_EN fail, DEBUG=0x%x\n",
> > > > > + mtk_uart_apdma_chan_read(c, VFF_DEBUG_STATUS));
> > > > > +
> > > > > + mtk_uart_apdma_chan_write(c, VFF_STOP, VFF_STOP_CLR_B);
> > > > > + mtk_uart_apdma_chan_write(c, VFF_INT_EN, VFF_INT_EN_CLR_B);
> > > > > +
> > > > > + switch (c->cfg.direction) {
> > > > > + case DMA_DEV_TO_MEM:
> > > >
> > > > Ditto, if/else?
> > > >
> > > ditto.
> > > http://lists.infradead.org/pipermail/linux-mediatek/2018-December/016292.html
> >
> > Well, I disagree, especially in this case: We turn 4 lines of code into 10...
> >
>
> i see, i will using if/else
To clarify, that's my opinion ,-) Down the line it's up to the maintainer.
> > > > > + mtk_uart_apdma_chan_write(c,
> > > > > + VFF_INT_FLAG, VFF_RX_INT_FLAG_CLR_B);
> > > > > + break;
> > > > > + case DMA_MEM_TO_DEV:
> > > > > + mtk_uart_apdma_chan_write(c,
> > > > > + VFF_INT_FLAG, VFF_TX_INT_FLAG_CLR_B);
> > > > > + break;
> > > > > + default:
> > > > > + break;
> > > > > + }
> > > > > +
> > > > > + spin_unlock_irqrestore(&c->vc.lock, flags);
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static int mtk_uart_apdma_device_pause(struct dma_chan *chan)
> > > > > +{
> > > > > + /* just for check caps pass */
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static int mtk_uart_apdma_device_resume(struct dma_chan *chan)
> > > > > +{
> > > > > + /* just for check caps pass */
> > > > > + return 0;
> > > > > +}
> >
> > Why do you need these? Seems wrong to advertise device_pause/resume
> > (and the caps) if we don't actually support that?
> >
>
> in serial 8250_dma.c file, when request DMA, will calll
> 'dma_get_slave_caps' to get the caps. and the will check caps.cmd_pause.
> the pause is device_pause. our device can't support the functions. but
> to check pass, need add the fake function. or you can give better
> comments. thanks.
Well, then I guess this is a hack... I'm not too familiar with
8250_dma.c, but I guess that if it requires pause/resume to be
implemented, then we should implement them, not just stub them out...
Is this something that can be implemented with 8250_mtk DMA?
Also, I don't see where you need resume, so maybe implementing pause is enough?
> > > > > +
> > > > > +static void mtk_uart_apdma_free(struct mtk_uart_apdmadev *mtkd)
> > > > > +{
> > > > > + while (list_empty(&mtkd->ddev.channels) == 0) {
> > > > > + struct mtk_chan *c = list_first_entry(&mtkd->ddev.channels,
> > > > > + struct mtk_chan, vc.chan.device_node);
> > > > > +
> > > > > + list_del(&c->vc.chan.device_node);
> > > > > + tasklet_kill(&c->vc.task);
> > > > > + }
> > > > > +}
> > > > > +
> > > > > +static const struct of_device_id mtk_uart_apdma_match[] = {
> > > > > + { .compatible = "mediatek,mt6577-uart-dma", },
> > > > > + { /* sentinel */ },
> > > > > +};
> > > > > +MODULE_DEVICE_TABLE(of, mtk_uart_apdma_match);
> > > > > +
> > > > > +static int mtk_uart_apdma_probe(struct platform_device *pdev)
> > > > > +{
> > > > > + struct mtk_uart_apdmadev *mtkd;
> > > > > + struct resource *res;
> > > > > + struct mtk_chan *c;
> > > > > + unsigned int i;
> > > > > + int rc;
> > > > > +
> > > > > + mtkd = devm_kzalloc(&pdev->dev, sizeof(*mtkd), GFP_KERNEL);
> > > > > + if (!mtkd)
> > > > > + return -ENOMEM;
> > > > > +
> > > > > + mtkd->clk = devm_clk_get(&pdev->dev, NULL);
> > > > > + if (IS_ERR(mtkd->clk)) {
> > > > > + dev_err(&pdev->dev, "No clock specified\n");
> > > > > + rc = PTR_ERR(mtkd->clk);
> > > > > + goto err_no_dma;
> > > > > + }
> > > > > +
> > > > > + if (of_property_read_bool(pdev->dev.of_node, "dma-33bits")) {
> > > > > + dev_info(&pdev->dev, "Support dma 33bits\n");
> > > >
> > > > Drop this, a bit verbose.
> > > >
> > > > > + mtkd->support_33bits = true;
> > > > > + }
> > > > > +
> > > > > + rc = dma_set_mask_and_coherent(&pdev->dev,
> > > > > + DMA_BIT_MASK(32 | mtkd->support_33bits));
> > > > > + if (rc)
> > > > > + goto err_no_dma;
> > > > > +
> > > > > + dma_cap_set(DMA_SLAVE, mtkd->ddev.cap_mask);
> > > > > + mtkd->ddev.device_alloc_chan_resources =
> > > > > + mtk_uart_apdma_alloc_chan_resources;
> > > > > + mtkd->ddev.device_free_chan_resources =
> > > > > + mtk_uart_apdma_free_chan_resources;
> > > > > + mtkd->ddev.device_tx_status = mtk_uart_apdma_tx_status;
> > > > > + mtkd->ddev.device_issue_pending = mtk_uart_apdma_issue_pending;
> > > > > + mtkd->ddev.device_prep_slave_sg = mtk_uart_apdma_prep_slave_sg;
> > > > > + mtkd->ddev.device_config = mtk_uart_apdma_slave_config;
> > > > > + mtkd->ddev.device_pause = mtk_uart_apdma_device_pause;
> > > > > + mtkd->ddev.device_resume = mtk_uart_apdma_device_resume;
> > > > > + mtkd->ddev.device_terminate_all = mtk_uart_apdma_terminate_all;
> > > > > + mtkd->ddev.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE);
> > > > > + mtkd->ddev.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE);
> > > > > + mtkd->ddev.directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
> > > > > + mtkd->ddev.residue_granularity = DMA_RESIDUE_GRANULARITY_SEGMENT;
> > > > > + mtkd->ddev.dev = &pdev->dev;
> > > > > + INIT_LIST_HEAD(&mtkd->ddev.channels);
> > > > > +
> > > > > + for (i = 0; i < MTK_UART_APDMA_CHANNELS; i++) {
> > > > > + c = devm_kzalloc(mtkd->ddev.dev, sizeof(*c), GFP_KERNEL);
> > > > > + if (!c) {
> > > > > + rc = -ENODEV;
> > > > > + goto err_no_dma;
> > > > > + }
> > > > > +
> > > > > + res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> > > > > + if (!res) {
> > > > > + rc = -ENODEV;
> > > > > + goto err_no_dma;
> > > > > + }
> > > > > +
> > > > > + c->base = devm_ioremap_resource(&pdev->dev, res);
> > > > > + if (IS_ERR(c->base)) {
> > > > > + rc = PTR_ERR(c->base);
> > > > > + goto err_no_dma;
> > > > > + }
> > > > > + c->requested = false;
> > > > > + c->vc.desc_free = mtk_uart_apdma_desc_free;
> > > > > + vchan_init(&c->vc, &mtkd->ddev);
> > > > > +
> > > > > + mtkd->dma_irq[i] = platform_get_irq(pdev, i);
> > > > > + if ((int)mtkd->dma_irq[i] < 0) {
> > > > > + dev_err(&pdev->dev, "failed to get IRQ[%d]\n", i);
> > > > > + rc = -EINVAL;
> > > > > + goto err_no_dma;
> > > > > + }
> > > > > + }
> > > > > +
> > > > > + pm_runtime_enable(&pdev->dev);
> > > > > + pm_runtime_set_active(&pdev->dev);
> > > > > +
> > > > > + rc = dma_async_device_register(&mtkd->ddev);
> > > > > + if (rc)
> > > > > + goto rpm_disable;
> > > > > +
> > > > > + platform_set_drvdata(pdev, mtkd);
> > > > > +
> > > > > + if (pdev->dev.of_node) {
> > > > > + /* Device-tree DMA controller registration */
> > > > > + rc = of_dma_controller_register(pdev->dev.of_node,
> > > > > + of_dma_xlate_by_chan_id,
> > > > > + mtkd);
> > > > > + if (rc)
> > > > > + goto dma_remove;
> > > > > + }
> > > > > +
> > > > > + return rc;
> > > > > +
> > > > > +dma_remove:
> > > > > + dma_async_device_unregister(&mtkd->ddev);
> > > > > +rpm_disable:
> > > > > + pm_runtime_disable(&pdev->dev);
> > > > > +err_no_dma:
> > > > > + mtk_uart_apdma_free(mtkd);
> >
> > It's not very correct to call this before
> > INIT_LIST_HEAD(&mtkd->ddev.channels);
> >
> > I'd just call `return rc;` directly for all instances before that line.
> >
>
> ok, i will modify it.
>
> > > > > + return rc;
> > > > > +}
> > > > > +
> > > > > +static int mtk_uart_apdma_remove(struct platform_device *pdev)
> > > > > +{
> > > > > + struct mtk_uart_apdmadev *mtkd = platform_get_drvdata(pdev);
> > > > > +
> > > > > + if (pdev->dev.of_node)
> > > > > + of_dma_controller_free(pdev->dev.of_node);
> > > > > +
> > > > > + pm_runtime_disable(&pdev->dev);
> > > > > + pm_runtime_put_noidle(&pdev->dev);
> > > > > +
> > > > > + dma_async_device_unregister(&mtkd->ddev);
> > > > > + mtk_uart_apdma_free(mtkd);
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +#ifdef CONFIG_PM_SLEEP
> > > > > +static int mtk_uart_apdma_suspend(struct device *dev)
> > > > > +{
> > > > > + struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
> > > > > +
> > > > > + if (!pm_runtime_suspended(dev))
> > > > > + clk_disable_unprepare(mtkd->clk);
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static int mtk_uart_apdma_resume(struct device *dev)
> > > > > +{
> > > > > + int ret;
> > > > > + struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
> > > > > +
> > > > > + if (!pm_runtime_suspended(dev)) {
> > > > > + ret = clk_prepare_enable(mtkd->clk);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > + }
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +#endif /* CONFIG_PM_SLEEP */
> > > > > +
> > > > > +#ifdef CONFIG_PM
> > > > > +static int mtk_uart_apdma_runtime_suspend(struct device *dev)
> > > > > +{
> > > > > + struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
> > > > > +
> > > > > + clk_disable_unprepare(mtkd->clk);
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static int mtk_uart_apdma_runtime_resume(struct device *dev)
> > > > > +{
> > > > > + int ret;
> > > > > + struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
> > > > > +
> > > > > + ret = clk_prepare_enable(mtkd->clk);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +#endif /* CONFIG_PM */
> > > > > +
> > > > > +static const struct dev_pm_ops mtk_uart_apdma_pm_ops = {
> > > > > + SET_SYSTEM_SLEEP_PM_OPS(mtk_uart_apdma_suspend, mtk_uart_apdma_resume)
> > > > > + SET_RUNTIME_PM_OPS(mtk_uart_apdma_runtime_suspend,
> > > > > + mtk_uart_apdma_runtime_resume, NULL)
> > > > > +};
> > > > > +
> > > > > +static struct platform_driver mtk_uart_apdma_driver = {
> > > > > + .probe = mtk_uart_apdma_probe,
> > > > > + .remove = mtk_uart_apdma_remove,
> > > > > + .driver = {
> > > > > + .name = KBUILD_MODNAME,
> > > > > + .pm = &mtk_uart_apdma_pm_ops,
> > > > > + .of_match_table = of_match_ptr(mtk_uart_apdma_match),
> > > > > + },
> > > > > +};
> > > > > +
> > > > > +module_platform_driver(mtk_uart_apdma_driver);
> > > > > +
> > > > > +MODULE_DESCRIPTION("MediaTek UART APDMA Controller Driver");
> > > > > +MODULE_AUTHOR("Long Cheng <long.cheng@mediatek.com>");
> > > > > +MODULE_LICENSE("GPL v2");
> > > > > +
> > > > > diff --git a/drivers/dma/mediatek/Kconfig b/drivers/dma/mediatek/Kconfig
> > > > > index 27bac0b..1a523c87 100644
> > > > > --- a/drivers/dma/mediatek/Kconfig
> > > > > +++ b/drivers/dma/mediatek/Kconfig
> > > > > @@ -1,4 +1,15 @@
> > > > >
> > > > > +config DMA_MTK_UART
> > > > > + tristate "MediaTek SoCs APDMA support for UART"
> > > > > + depends on OF && SERIAL_8250_MT6577
> > > > > + select DMA_ENGINE
> > > > > + select DMA_VIRTUAL_CHANNELS
> > > > > + help
> > > > > + Support for the UART DMA engine found on MediaTek MTK SoCs.
> > > > > + when SERIAL_8250_MT6577 is enabled, and if you want to use DMA,
> > > > > + you can enable the config. the DMA engine can only be used
> > > > > + with MediaTek SoCs.
> > > > > +
> > > > > config MTK_HSDMA
> > > > > tristate "MediaTek High-Speed DMA controller support"
> > > > > depends on ARCH_MEDIATEK || COMPILE_TEST
> > > > > diff --git a/drivers/dma/mediatek/Makefile b/drivers/dma/mediatek/Makefile
> > > > > index 6e778f8..2f2efd9 100644
> > > > > --- a/drivers/dma/mediatek/Makefile
> > > > > +++ b/drivers/dma/mediatek/Makefile
> > > > > @@ -1 +1,2 @@
> > > > > +obj-$(CONFIG_DMA_MTK_UART) += 8250_mtk_dma.o
> > > > > obj-$(CONFIG_MTK_HSDMA) += mtk-hsdma.o
> > > > > --
> > > > > 1.7.9.5
> > > > >
> > >
> > >
>
>
^ permalink raw reply
* [v7,1/2] dmaengine: 8250_mtk_dma: add MediaTek uart DMA support
From: Long Cheng @ 2018-12-26 10:18 UTC (permalink / raw)
To: Nicolas Boichat
Cc: Vinod Koul, Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee,
Sean Wang, Matthias Brugger, Dan Williams, Greg Kroah-Hartman,
Jiri Slaby, Sean Wang, dmaengine, devicetree,
linux-arm Mailing List, linux-mediatek, lkml, linux-serial,
srv_heupstream, Yingjoe Chen, YT Shen, Zhenbao Liu
On Wed, 2018-12-26 at 17:28 +0800, Nicolas Boichat wrote:
......
> > > > > > +static int mtk_uart_apdma_device_pause(struct dma_chan *chan)
> > > > > > +{
> > > > > > + /* just for check caps pass */
> > > > > > + return 0;
> > > > > > +}
> > > > > > +
> > > > > > +static int mtk_uart_apdma_device_resume(struct dma_chan *chan)
> > > > > > +{
> > > > > > + /* just for check caps pass */
> > > > > > + return 0;
> > > > > > +}
> > >
> > > Why do you need these? Seems wrong to advertise device_pause/resume
> > > (and the caps) if we don't actually support that?
> > >
> >
> > in serial 8250_dma.c file, when request DMA, will calll
> > 'dma_get_slave_caps' to get the caps. and the will check caps.cmd_pause.
> > the pause is device_pause. our device can't support the functions. but
> > to check pass, need add the fake function. or you can give better
> > comments. thanks.
>
> Well, then I guess this is a hack... I'm not too familiar with
> 8250_dma.c, but I guess that if it requires pause/resume to be
> implemented, then we should implement them, not just stub them out...
>
> Is this something that can be implemented with 8250_mtk DMA?
>
the HW can't support it.
> Also, I don't see where you need resume, so maybe implementing pause is enough?
>
yes. pause is enough. for symmetry, add resume function.
anyway, the pause function need to keep.
> > > > > > +
> > > > > > +static void mtk_uart_apdma_free(struct mtk_uart_apdmadev *mtkd)
> > > > > > +{
> > > > > > + while (list_empty(&mtkd->ddev.channels) == 0) {
> > > > > > + struct mtk_chan *c = list_first_entry(&mtkd->ddev.channels,
> > > > > > + struct mtk_chan, vc.chan.device_node);
> > > > > > +
> > > > > > + list_del(&c->vc.chan.device_node);
> > > > > > + tasklet_kill(&c->vc.task);
> > > > > > + }
> > > > > > +}
> > > > > > +
> > > > > > +static const struct of_device_id mtk_uart_apdma_match[] = {
> > > > > > + { .compatible = "mediatek,mt6577-uart-dma", },
> > > > > > + { /* sentinel */ },
> > > > > > +};
> > > > > > +MODULE_DEVICE_TABLE(of, mtk_uart_apdma_match);
> > > > > > +
> > > > > > +static int mtk_uart_apdma_probe(struct platform_device *pdev)
> > > > > > +{
> > > > > > + struct mtk_uart_apdmadev *mtkd;
> > > > > > + struct resource *res;
> > > > > > + struct mtk_chan *c;
> > > > > > + unsigned int i;
> > > > > > + int rc;
> > > > > > +
......
^ permalink raw reply
* [1/2] dt-bindings: dmaengine: Add MediaTek Command-Queue DMA controller bindings
From: shun-chih.yu @ 2018-12-27 13:10 UTC (permalink / raw)
To: Sean Wang, Vinod Koul, Rob Herring, Matthias Brugger,
Dan Williams
Cc: dmaengine, linux-arm-kernel, linux-mediatek, devicetree,
linux-kernel, srv_wsdupstream, Shun-Chih Yu
From: Shun-Chih Yu <shun-chih.yu@mediatek.com>
Document the devicetree bindings for MediaTek Command-Queue DMA controller
which could be found on MT6765 SoC or other similar Mediatek SoCs.
Change-Id: I9736c8cac9be160358feeab935fabaffc5730519
Signed-off-by: Shun-Chih Yu <shun-chih.yu@mediatek.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../devicetree/bindings/dma/mtk-cqdma.txt | 31 ++++++++++++++++++++
1 file changed, 31 insertions(+)
create mode 100644 Documentation/devicetree/bindings/dma/mtk-cqdma.txt
diff --git a/Documentation/devicetree/bindings/dma/mtk-cqdma.txt b/Documentation/devicetree/bindings/dma/mtk-cqdma.txt
new file mode 100644
index 0000000..fb12927
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/mtk-cqdma.txt
@@ -0,0 +1,31 @@
+MediaTek Command-Queue DMA Controller
+==================================
+
+Required properties:
+
+- compatible: Must be "mediatek,mt6765-cqdma" for MT6765.
+- reg: Should contain the base address and length for each channel.
+- interrupts: Should contain references to the interrupts for each channel.
+- clocks: Should be the clock specifiers corresponding to the entry in
+ clock-names property.
+- clock-names: Should contain "cqdma" entries.
+- dma-channels: The number of DMA channels supported by the controller.
+- dma-requests: The number of DMA request supported by the controller.
+- #dma-cells: The length of the DMA specifier, must be <1>. This one cell
+ in dmas property of a client device represents the channel
+ number.
+Example:
+
+ cqdma: dma-controller@10212000 {
+ compatible = "mediatek,mt6765-cqdma";
+ reg = <0 0x10212000 0 0x1000>;
+ interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 114 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&infracfg CLK_IFR_CQ_DMA>;
+ clock-names = "cqdma";
+ dma-channels = <2>;
+ dma-requests = <32>;
+ #dma-cells = <1>;
+ };
+
+DMA clients must use the format described in dma/dma.txt file.
^ permalink raw reply related
* [2/2] dmaengine: mediatek: Add MediaTek Command-Queue DMA controller for MT6765 SoC
From: shun-chih.yu @ 2018-12-27 13:10 UTC (permalink / raw)
To: Sean Wang, Vinod Koul, Rob Herring, Matthias Brugger,
Dan Williams
Cc: dmaengine, linux-arm-kernel, linux-mediatek, devicetree,
linux-kernel, srv_wsdupstream, Shun-Chih Yu
From: Shun-Chih Yu <shun-chih.yu@mediatek.com>
MediaTek Command-Queue DMA controller (CQDMA) on MT6765 SoC is dedicated
to memory-to-memory transfer through queue based descriptor management.
There are only 3 physical channels inside CQDMA, while the driver is
extended to support 32 virtual channels for multiple dma users to issue
dma requests onto the CQDMA simultaneously.
Change-Id: I1e8d116c5ecbbc49190ffc925cb59a0d035d886b
Signed-off-by: Shun-Chih Yu <shun-chih.yu@mediatek.com>
Reviewed-by: Vinod Koul <vkoul@kernel.org>
---
drivers/dma/mediatek/Kconfig | 12 +
drivers/dma/mediatek/Makefile | 1 +
drivers/dma/mediatek/mtk-cqdma.c | 867 ++++++++++++++++++++++++++++++++++++++
3 files changed, 880 insertions(+)
create mode 100644 drivers/dma/mediatek/mtk-cqdma.c
diff --git a/drivers/dma/mediatek/Kconfig b/drivers/dma/mediatek/Kconfig
index 27bac0b..4a1582d 100644
--- a/drivers/dma/mediatek/Kconfig
+++ b/drivers/dma/mediatek/Kconfig
@@ -11,3 +11,15 @@ config MTK_HSDMA
This controller provides the channels which is dedicated to
memory-to-memory transfer to offload from CPU through ring-
based descriptor management.
+
+config MTK_CQDMA
+ tristate "MediaTek Command-Queue DMA controller support"
+ depends on ARCH_MEDIATEK || COMPILE_TEST
+ select DMA_ENGINE
+ select DMA_VIRTUAL_CHANNELS
+ help
+ Enable support for Command-Queue DMA controller on MediaTek
+ SoCs.
+
+ This controller provides the channels which is dedicated to
+ memory-to-memory transfer to offload from CPU.
diff --git a/drivers/dma/mediatek/Makefile b/drivers/dma/mediatek/Makefile
index 6e778f8..41bb381 100644
--- a/drivers/dma/mediatek/Makefile
+++ b/drivers/dma/mediatek/Makefile
@@ -1 +1,2 @@
obj-$(CONFIG_MTK_HSDMA) += mtk-hsdma.o
+obj-$(CONFIG_MTK_CQDMA) += mtk-cqdma.o
diff --git a/drivers/dma/mediatek/mtk-cqdma.c b/drivers/dma/mediatek/mtk-cqdma.c
new file mode 100644
index 0000000..304eb0a
--- /dev/null
+++ b/drivers/dma/mediatek/mtk-cqdma.c
@@ -0,0 +1,867 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018-2019 MediaTek Inc.
+
+/*
+ * Driver for MediaTek Command-Queue DMA Controller
+ *
+ * Author: Shun-Chih Yu <shun-chih.yu@mediatek.com>
+ *
+ */
+
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+#include <linux/err.h>
+#include <linux/iopoll.h>
+#include <linux/interrupt.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_dma.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/refcount.h>
+#include <linux/slab.h>
+
+#include "../virt-dma.h"
+
+#define MTK_CQDMA_USEC_POLL 10
+#define MTK_CQDMA_TIMEOUT_POLL 1000
+#define MTK_CQDMA_DMA_BUSWIDTHS BIT(DMA_SLAVE_BUSWIDTH_4_BYTES)
+#define MTK_CQDMA_ALIGN_SIZE 1
+
+/* The default number of virtual channel */
+#define MTK_CQDMA_NR_VCHANS 32
+
+/* The default number of physical channel */
+#define MTK_CQDMA_NR_PCHANS 3
+
+/* Registers for underlying dma manipulation */
+#define MTK_CQDMA_INT_FLAG 0x0
+#define MTK_CQDMA_INT_EN 0x4
+#define MTK_CQDMA_EN 0x8
+#define MTK_CQDMA_RESET 0xc
+#define MTK_CQDMA_FLUSH 0x14
+#define MTK_CQDMA_SRC 0x1c
+#define MTK_CQDMA_DST 0x20
+#define MTK_CQDMA_LEN1 0x24
+#define MTK_CQDMA_LEN2 0x28
+#define MTK_CQDMA_SRC2 0x60
+#define MTK_CQDMA_DST2 0x64
+
+/* Registers setting */
+#define MTK_CQDMA_EN_BIT BIT(0)
+#define MTK_CQDMA_INT_FLAG_BIT BIT(0)
+#define MTK_CQDMA_INT_EN_BIT BIT(0)
+#define MTK_CQDMA_FLUSH_BIT BIT(0)
+
+#define MTK_CQDMA_WARM_RST_BIT BIT(0)
+#define MTK_CQDMA_HARD_RST_BIT BIT(1)
+
+#define MTK_CQDMA_MAX_LEN GENMASK(27, 0)
+#define MTK_CQDMA_ADDR_LIMIT GENMASK(31, 0)
+#define MTK_CQDMA_ADDR2_SHFIT (32)
+
+/**
+ * struct mtk_cqdma_vdesc - The struct holding info describing virtual
+ * descriptor (CVD)
+ * @vd: An instance for struct virt_dma_desc
+ * @len: The total data size device wants to move
+ * @dest: The destination address device wants to move to
+ * @src: The source address device wants to move from
+ * @ch: The pointer to the corresponding dma channel
+ * @node: To build linked-list for PC queue
+ */
+struct mtk_cqdma_vdesc {
+ struct virt_dma_desc vd;
+ size_t len;
+ dma_addr_t dest;
+ dma_addr_t src;
+ struct dma_chan *ch;
+
+ /* protected by pc.lock */
+ struct list_head node;
+};
+
+/**
+ * struct mtk_cqdma_pchan - The struct holding info describing physical
+ * channel (PC)
+ * @queue: Queue for the CVDs issued to this PC
+ * @base: The mapped register I/O base of this PC
+ * @irq: The IRQ that this PC are using
+ * @refcnt: Track how many VCs are using this PC
+ * @lock: Lock protect agaisting multiple VCs access PC
+ */
+struct mtk_cqdma_pchan {
+ struct list_head queue;
+ void __iomem *base;
+ u32 irq;
+ refcount_t refcnt;
+
+ /* lock to protect PC */
+ spinlock_t lock;
+};
+
+/**
+ * struct mtk_cqdma_vchan - The struct holding info describing virtual
+ * channel (VC)
+ * @vc: An instance for struct virt_dma_chan
+ * @pc: The pointer to the underlying PC
+ * @issue_completion: The wait for all issued descriptors completited
+ * @issue_synchronize: Bool indicating channel synchronization starts
+ */
+struct mtk_cqdma_vchan {
+ struct virt_dma_chan vc;
+ struct mtk_cqdma_pchan *pc;
+ struct completion issue_completion;
+ bool issue_synchronize;
+ /* protected by vc.lock */
+};
+
+/**
+ * struct mtk_cqdma_device - The struct holding info describing CQDMA
+ * device
+ * @ddev: An instance for struct dma_device
+ * @clk: The clock that device internal is using
+ * @dma_requests: The number of VCs the device supports to
+ * @dma_channels: The number of PCs the device supports to
+ * @vc: The pointer to all available VCs
+ * @pc: The pointer to all the underlying PCs
+ */
+struct mtk_cqdma_device {
+ struct dma_device ddev;
+ struct clk *clk;
+
+ u32 dma_requests;
+ u32 dma_channels;
+ struct mtk_cqdma_vchan *vc;
+ struct mtk_cqdma_pchan **pc;
+};
+
+static struct mtk_cqdma_device *to_cqdma_dev(struct dma_chan *chan)
+{
+ return container_of(chan->device, struct mtk_cqdma_device, ddev);
+}
+
+static struct mtk_cqdma_vchan *to_cqdma_vchan(struct dma_chan *chan)
+{
+ return container_of(chan, struct mtk_cqdma_vchan, vc.chan);
+}
+
+static struct mtk_cqdma_vdesc *to_cqdma_vdesc(struct virt_dma_desc *vd)
+{
+ return container_of(vd, struct mtk_cqdma_vdesc, vd);
+}
+
+static struct device *cqdma2dev(struct mtk_cqdma_device *cqdma)
+{
+ return cqdma->ddev.dev;
+}
+
+static u32 mtk_dma_read(struct mtk_cqdma_pchan *pc, u32 reg)
+{
+ return readl(pc->base + reg);
+}
+
+static void mtk_dma_write(struct mtk_cqdma_pchan *pc, u32 reg, u32 val)
+{
+ writel_relaxed(val, pc->base + reg);
+}
+
+static void mtk_dma_rmw(struct mtk_cqdma_pchan *pc, u32 reg,
+ u32 mask, u32 set)
+{
+ u32 val;
+
+ val = mtk_dma_read(pc, reg);
+ val &= ~mask;
+ val |= set;
+ mtk_dma_write(pc, reg, val);
+}
+
+static void mtk_dma_set(struct mtk_cqdma_pchan *pc, u32 reg, u32 val)
+{
+ mtk_dma_rmw(pc, reg, 0, val);
+}
+
+static void mtk_dma_clr(struct mtk_cqdma_pchan *pc, u32 reg, u32 val)
+{
+ mtk_dma_rmw(pc, reg, val, 0);
+}
+
+static void mtk_cqdma_vdesc_free(struct virt_dma_desc *vd)
+{
+ kfree(to_cqdma_vdesc(vd));
+}
+
+static int mtk_cqdma_poll_engine_done(struct mtk_cqdma_pchan *pc, bool atomic)
+{
+ u32 status = 0;
+
+ if (!atomic)
+ return readl_poll_timeout(pc->base + MTK_CQDMA_EN,
+ status,
+ !(status & MTK_CQDMA_EN_BIT),
+ MTK_CQDMA_USEC_POLL,
+ MTK_CQDMA_TIMEOUT_POLL);
+
+ return readl_poll_timeout_atomic(pc->base + MTK_CQDMA_EN,
+ status,
+ !(status & MTK_CQDMA_EN_BIT),
+ MTK_CQDMA_USEC_POLL,
+ MTK_CQDMA_TIMEOUT_POLL);
+}
+
+static int mtk_cqdma_hard_reset(struct mtk_cqdma_pchan *pc)
+{
+ mtk_dma_set(pc, MTK_CQDMA_RESET, MTK_CQDMA_HARD_RST_BIT);
+ mtk_dma_clr(pc, MTK_CQDMA_RESET, MTK_CQDMA_HARD_RST_BIT);
+
+ return mtk_cqdma_poll_engine_done(pc, false);
+}
+
+static void mtk_cqdma_start(struct mtk_cqdma_pchan *pc,
+ struct mtk_cqdma_vdesc *cvd)
+{
+ /* wait for the previous transaction done */
+ if (mtk_cqdma_poll_engine_done(pc, true) < 0)
+ dev_err(cqdma2dev(to_cqdma_dev(cvd->ch)),
+ "cqdma wait transaction timeout\n");
+
+ /* warm reset the dma engine for the new transaction */
+ mtk_dma_set(pc, MTK_CQDMA_RESET, MTK_CQDMA_WARM_RST_BIT);
+ if (mtk_cqdma_poll_engine_done(pc, true) < 0)
+ dev_err(cqdma2dev(to_cqdma_dev(cvd->ch)),
+ "cqdma warm reset timeout\n");
+
+ /* setup the source */
+ mtk_dma_set(pc, MTK_CQDMA_SRC, cvd->src & MTK_CQDMA_ADDR_LIMIT);
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+ mtk_dma_set(pc, MTK_CQDMA_SRC2, cvd->src >> MTK_CQDMA_ADDR2_SHFIT);
+#else
+ mtk_dma_set(pc, MTK_CQDMA_SRC2, 0);
+#endif
+
+ /* setup the destination */
+ mtk_dma_set(pc, MTK_CQDMA_DST, cvd->dest & MTK_CQDMA_ADDR_LIMIT);
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+ mtk_dma_set(pc, MTK_CQDMA_DST2, cvd->dest >> MTK_CQDMA_ADDR2_SHFIT);
+#else
+ mtk_dma_set(pc, MTK_CQDMA_SRC2, 0);
+#endif
+
+ /* setup the length */
+ mtk_dma_set(pc, MTK_CQDMA_LEN1,
+ (cvd->len < MTK_CQDMA_MAX_LEN) ?
+ cvd->len : MTK_CQDMA_MAX_LEN);
+
+ /* start dma engine */
+ mtk_dma_set(pc, MTK_CQDMA_EN, MTK_CQDMA_EN_BIT);
+}
+
+static void mtk_cqdma_issue_vchan_pending(struct mtk_cqdma_vchan *cvc)
+{
+ struct virt_dma_desc *vd, *vd2;
+ struct mtk_cqdma_pchan *pc = cvc->pc;
+ struct mtk_cqdma_vdesc *cvd;
+ bool trigger_engine = false;
+
+ lockdep_assert_held(&cvc->vc.lock);
+ lockdep_assert_held(&pc->lock);
+
+ list_for_each_entry_safe(vd, vd2, &cvc->vc.desc_issued, node) {
+ /* need to trigger dma engine if PC's queue is empty */
+ if (list_empty(&pc->queue))
+ trigger_engine = true;
+
+ cvd = to_cqdma_vdesc(vd);
+
+ /* add VD into PC's queue */
+ list_add_tail(&cvd->node, &pc->queue);
+
+ /* start the dma engine */
+ if (trigger_engine)
+ mtk_cqdma_start(pc, cvd);
+
+ /* remove VD from list desc_issued */
+ list_del(&vd->node);
+ }
+}
+
+/*
+ * return true if this VC is active,
+ * meaning that there are VDs under processing by the PC
+ */
+static bool mtk_cqdma_is_vchan_active(struct mtk_cqdma_vchan *cvc)
+{
+ struct mtk_cqdma_vdesc *cvd;
+
+ list_for_each_entry(cvd, &cvc->pc->queue, node)
+ if (cvc == to_cqdma_vchan(cvd->ch))
+ return true;
+
+ return false;
+}
+
+/*
+ * return the pointer of the CVD that is just consumed by the PC
+ */
+static void mtk_cqdma_consume_work_queue(struct mtk_cqdma_pchan *pc)
+{
+ struct mtk_cqdma_vchan *cvc;
+ struct mtk_cqdma_vdesc *cvd;
+ size_t tlen;
+
+ /* consume a CVD from PC's queue */
+ cvd = list_first_entry_or_null(&pc->queue,
+ struct mtk_cqdma_vdesc, node);
+ if (unlikely(!cvd))
+ return;
+
+ cvc = to_cqdma_vchan(cvd->ch);
+
+ tlen = (cvd->len < MTK_CQDMA_MAX_LEN) ? cvd->len : MTK_CQDMA_MAX_LEN;
+ cvd->len -= tlen;
+ cvd->src += tlen;
+ cvd->dest += tlen;
+
+ /* check whether the CVD completed */
+ if (!cvd->len) {
+ /* delete CVD from PC's queue */
+ list_del(&cvd->node);
+
+ spin_lock(&cvc->vc.lock);
+
+ /* add the VD into list desc_completed */
+ vchan_cookie_complete(&cvd->vd);
+
+ /* setup completion if this VC is under synchronization */
+ if (cvc->issue_synchronize && !mtk_cqdma_is_vchan_active(cvc)) {
+ complete(&cvc->issue_completion);
+ cvc->issue_synchronize = false;
+ }
+
+ spin_unlock(&cvc->vc.lock);
+ }
+
+ /* iterate on the next CVD if the current CVD completed */
+ if (!cvd->len)
+ cvd = list_first_entry_or_null(&pc->queue,
+ struct mtk_cqdma_vdesc, node);
+
+ /* start the next transaction */
+ if (cvd)
+ mtk_cqdma_start(pc, cvd);
+}
+
+static irqreturn_t mtk_cqdma_irq(int irq, void *devid)
+{
+ struct mtk_cqdma_device *cqdma = devid;
+ irqreturn_t ret = IRQ_NONE;
+ u32 i;
+
+ /* clear interrupt flags for each PC */
+ for (i = 0; i < cqdma->dma_channels; ++i) {
+ spin_lock(&cqdma->pc[i]->lock);
+ if (mtk_dma_read(cqdma->pc[i],
+ MTK_CQDMA_INT_FLAG) & MTK_CQDMA_INT_FLAG_BIT) {
+ /* clear interrupt */
+ mtk_dma_clr(cqdma->pc[i], MTK_CQDMA_INT_FLAG,
+ MTK_CQDMA_INT_FLAG_BIT);
+
+ mtk_cqdma_consume_work_queue(cqdma->pc[i]);
+
+ ret = IRQ_HANDLED;
+ }
+ spin_unlock(&cqdma->pc[i]->lock);
+ }
+
+ return ret;
+}
+
+static struct virt_dma_desc *mtk_cqdma_find_active_desc(struct dma_chan *c,
+ dma_cookie_t cookie)
+{
+ struct mtk_cqdma_vchan *cvc = to_cqdma_vchan(c);
+ struct virt_dma_desc *vd;
+ unsigned long flags;
+
+ spin_lock_irqsave(&cvc->pc->lock, flags);
+ list_for_each_entry(vd, &cvc->pc->queue, node)
+ if (vd->tx.cookie == cookie) {
+ spin_unlock_irqrestore(&cvc->pc->lock, flags);
+ return vd;
+ }
+ spin_unlock_irqrestore(&cvc->pc->lock, flags);
+
+ list_for_each_entry(vd, &cvc->vc.desc_issued, node)
+ if (vd->tx.cookie == cookie)
+ return vd;
+
+ return NULL;
+}
+
+static enum dma_status mtk_cqdma_tx_status(struct dma_chan *c,
+ dma_cookie_t cookie,
+ struct dma_tx_state *txstate)
+{
+ struct mtk_cqdma_vchan *cvc = to_cqdma_vchan(c);
+ struct mtk_cqdma_vdesc *cvd;
+ struct virt_dma_desc *vd;
+ enum dma_status ret;
+ unsigned long flags;
+ size_t bytes = 0;
+
+ ret = dma_cookie_status(c, cookie, txstate);
+ if (ret == DMA_COMPLETE || !txstate)
+ return ret;
+
+ spin_lock_irqsave(&cvc->vc.lock, flags);
+ vd = mtk_cqdma_find_active_desc(c, cookie);
+ spin_unlock_irqrestore(&cvc->vc.lock, flags);
+
+ if (vd) {
+ cvd = to_cqdma_vdesc(vd);
+ bytes = cvd->len;
+ }
+
+ dma_set_residue(txstate, bytes);
+
+ return ret;
+}
+
+static void mtk_cqdma_issue_pending(struct dma_chan *c)
+{
+ struct mtk_cqdma_vchan *cvc = to_cqdma_vchan(c);
+ unsigned long pc_flags;
+ unsigned long vc_flags;
+
+ /* acquire PC's lock before VS's lock for lock dependency in ISR */
+ spin_lock_irqsave(&cvc->pc->lock, pc_flags);
+ spin_lock_irqsave(&cvc->vc.lock, vc_flags);
+
+ if (vchan_issue_pending(&cvc->vc))
+ mtk_cqdma_issue_vchan_pending(cvc);
+
+ spin_unlock_irqrestore(&cvc->vc.lock, vc_flags);
+ spin_unlock_irqrestore(&cvc->pc->lock, pc_flags);
+}
+
+static struct dma_async_tx_descriptor *
+mtk_cqdma_prep_dma_memcpy(struct dma_chan *c, dma_addr_t dest,
+ dma_addr_t src, size_t len, unsigned long flags)
+{
+ struct mtk_cqdma_vdesc *cvd;
+
+ cvd = kzalloc(sizeof(*cvd), GFP_NOWAIT);
+ if (!cvd)
+ return NULL;
+
+ /* setup dma channel */
+ cvd->ch = c;
+
+ /* setup sourece, destination, and length */
+ cvd->len = len;
+ cvd->src = src;
+ cvd->dest = dest;
+
+ return vchan_tx_prep(to_virt_chan(c), &cvd->vd, flags);
+}
+
+static void mtk_cqdma_free_inactive_desc(struct dma_chan *c)
+{
+ struct virt_dma_chan *vc = to_virt_chan(c);
+ unsigned long flags;
+ LIST_HEAD(head);
+
+ /*
+ * set desc_allocated, desc_submitted,
+ * and desc_issued as the candicates to be freed
+ */
+ spin_lock_irqsave(&vc->lock, flags);
+ list_splice_tail_init(&vc->desc_allocated, &head);
+ list_splice_tail_init(&vc->desc_submitted, &head);
+ list_splice_tail_init(&vc->desc_issued, &head);
+ spin_unlock_irqrestore(&vc->lock, flags);
+
+ /* free descriptor lists */
+ vchan_dma_desc_free_list(vc, &head);
+}
+
+static void mtk_cqdma_free_active_desc(struct dma_chan *c)
+{
+ struct mtk_cqdma_vchan *cvc = to_cqdma_vchan(c);
+ bool sync_needed = false;
+ unsigned long pc_flags;
+ unsigned long vc_flags;
+
+ /* acquire PC's lock first due to lock dependency in dma ISR */
+ spin_lock_irqsave(&cvc->pc->lock, pc_flags);
+ spin_lock_irqsave(&cvc->vc.lock, vc_flags);
+
+ /* synchronization is required if this VC is active */
+ if (mtk_cqdma_is_vchan_active(cvc)) {
+ cvc->issue_synchronize = true;
+ sync_needed = true;
+ }
+
+ spin_unlock_irqrestore(&cvc->vc.lock, vc_flags);
+ spin_unlock_irqrestore(&cvc->pc->lock, pc_flags);
+
+ /* waiting for the completion of this VC */
+ if (sync_needed)
+ wait_for_completion(&cvc->issue_completion);
+
+ /* free all descriptors in list desc_completed */
+ vchan_synchronize(&cvc->vc);
+
+ WARN_ONCE(!list_empty(&cvc->vc.desc_completed),
+ "Desc pending still in list desc_completed\n");
+}
+
+static int mtk_cqdma_terminate_all(struct dma_chan *c)
+{
+ /* free descriptors not processed yet by hardware */
+ mtk_cqdma_free_inactive_desc(c);
+
+ /* free descriptors being processed by hardware */
+ mtk_cqdma_free_active_desc(c);
+
+ return 0;
+}
+
+static int mtk_cqdma_alloc_chan_resources(struct dma_chan *c)
+{
+ struct mtk_cqdma_device *cqdma = to_cqdma_dev(c);
+ struct mtk_cqdma_vchan *vc = to_cqdma_vchan(c);
+ struct mtk_cqdma_pchan *pc = NULL;
+ u32 i, min_refcnt = U32_MAX, refcnt;
+ unsigned long flags;
+
+ /* allocate PC with the minimum refcount */
+ for (i = 0; i < cqdma->dma_channels; ++i) {
+ refcnt = refcount_read(&cqdma->pc[i]->refcnt);
+ if (refcnt < min_refcnt) {
+ pc = cqdma->pc[i];
+ min_refcnt = refcnt;
+ }
+ }
+
+ if (!pc)
+ return -ENOSPC;
+
+ spin_lock_irqsave(&pc->lock, flags);
+
+ if (!refcount_read(&pc->refcnt)) {
+ /* allocate PC when the refcount is zero */
+ mtk_cqdma_hard_reset(pc);
+
+ /* enable interrupt for this PC */
+ mtk_dma_set(pc, MTK_CQDMA_INT_EN, MTK_CQDMA_INT_EN_BIT);
+
+ /*
+ * refcount_inc would complain increment on 0; use-after-free.
+ * Thus, we need to explicitly set it as 1 initially.
+ */
+ refcount_set(&pc->refcnt, 1);
+ } else {
+ refcount_inc(&pc->refcnt);
+ }
+
+ spin_unlock_irqrestore(&pc->lock, flags);
+
+ vc->pc = pc;
+
+ return 0;
+}
+
+static void mtk_cqdma_free_chan_resources(struct dma_chan *c)
+{
+ struct mtk_cqdma_vchan *cvc = to_cqdma_vchan(c);
+ unsigned long flags;
+
+ /* free all descriptors in all lists on the VC */
+ mtk_cqdma_terminate_all(c);
+
+ spin_lock_irqsave(&cvc->pc->lock, flags);
+
+ /* PC is not freed until there is no VC mapped to it */
+ if (refcount_dec_and_test(&cvc->pc->refcnt)) {
+ /* start the flush operation and stop the engine */
+ mtk_dma_set(cvc->pc, MTK_CQDMA_FLUSH, MTK_CQDMA_FLUSH_BIT);
+
+ /* wait for the completion of flush operation */
+ if (mtk_cqdma_poll_engine_done(cvc->pc, false) < 0)
+ dev_err(cqdma2dev(to_cqdma_dev(c)),
+ "cqdma flush timeout\n");
+
+ /* clear the flush bit and interrupt flag */
+ mtk_dma_clr(cvc->pc, MTK_CQDMA_FLUSH, MTK_CQDMA_FLUSH_BIT);
+ mtk_dma_clr(cvc->pc, MTK_CQDMA_INT_FLAG,
+ MTK_CQDMA_INT_FLAG_BIT);
+
+ /* disable interrupt for this PC */
+ mtk_dma_clr(cvc->pc, MTK_CQDMA_INT_EN, MTK_CQDMA_INT_EN_BIT);
+ }
+
+ spin_unlock_irqrestore(&cvc->pc->lock, flags);
+}
+
+static int mtk_cqdma_hw_init(struct mtk_cqdma_device *cqdma)
+{
+ unsigned long flags;
+ int err;
+ u32 i;
+
+ pm_runtime_enable(cqdma2dev(cqdma));
+ pm_runtime_get_sync(cqdma2dev(cqdma));
+
+ err = clk_prepare_enable(cqdma->clk);
+
+ if (err) {
+ pm_runtime_put_sync(cqdma2dev(cqdma));
+ pm_runtime_disable(cqdma2dev(cqdma));
+ return err;
+ }
+
+ /* reset all PCs */
+ for (i = 0; i < cqdma->dma_channels; ++i) {
+ spin_lock_irqsave(&cqdma->pc[i]->lock, flags);
+ if (mtk_cqdma_hard_reset(cqdma->pc[i]) < 0) {
+ dev_err(cqdma2dev(cqdma), "cqdma hard reset timeout\n");
+ spin_unlock_irqrestore(&cqdma->pc[i]->lock, flags);
+
+ clk_disable_unprepare(cqdma->clk);
+ pm_runtime_put_sync(cqdma2dev(cqdma));
+ pm_runtime_disable(cqdma2dev(cqdma));
+ return -EINVAL;
+ }
+ spin_unlock_irqrestore(&cqdma->pc[i]->lock, flags);
+ }
+
+ return 0;
+}
+
+static void mtk_cqdma_hw_deinit(struct mtk_cqdma_device *cqdma)
+{
+ unsigned long flags;
+ u32 i;
+
+ /* reset all PCs */
+ for (i = 0; i < cqdma->dma_channels; ++i) {
+ spin_lock_irqsave(&cqdma->pc[i]->lock, flags);
+ if (mtk_cqdma_hard_reset(cqdma->pc[i]) < 0)
+ dev_err(cqdma2dev(cqdma), "cqdma hard reset timeout\n");
+ spin_unlock_irqrestore(&cqdma->pc[i]->lock, flags);
+ }
+
+ clk_disable_unprepare(cqdma->clk);
+
+ pm_runtime_put_sync(cqdma2dev(cqdma));
+ pm_runtime_disable(cqdma2dev(cqdma));
+}
+
+static const struct of_device_id mtk_cqdma_match[] = {
+ { .compatible = "mediatek,mt6765-cqdma" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mtk_cqdma_match);
+
+static int mtk_cqdma_probe(struct platform_device *pdev)
+{
+ struct mtk_cqdma_device *cqdma;
+ struct mtk_cqdma_vchan *vc;
+ struct dma_device *dd;
+ struct resource *res;
+ int err;
+ u32 i;
+
+ cqdma = devm_kzalloc(&pdev->dev, sizeof(*cqdma), GFP_KERNEL);
+ if (!cqdma)
+ return -ENOMEM;
+
+ dd = &cqdma->ddev;
+
+ cqdma->clk = devm_clk_get(&pdev->dev, "cqdma");
+ if (IS_ERR(cqdma->clk)) {
+ dev_err(&pdev->dev, "No clock for %s\n",
+ dev_name(&pdev->dev));
+ return PTR_ERR(cqdma->clk);
+ }
+
+ dma_cap_set(DMA_MEMCPY, dd->cap_mask);
+
+ dd->copy_align = MTK_CQDMA_ALIGN_SIZE;
+ dd->device_alloc_chan_resources = mtk_cqdma_alloc_chan_resources;
+ dd->device_free_chan_resources = mtk_cqdma_free_chan_resources;
+ dd->device_tx_status = mtk_cqdma_tx_status;
+ dd->device_issue_pending = mtk_cqdma_issue_pending;
+ dd->device_prep_dma_memcpy = mtk_cqdma_prep_dma_memcpy;
+ dd->device_terminate_all = mtk_cqdma_terminate_all;
+ dd->src_addr_widths = MTK_CQDMA_DMA_BUSWIDTHS;
+ dd->dst_addr_widths = MTK_CQDMA_DMA_BUSWIDTHS;
+ dd->directions = BIT(DMA_MEM_TO_MEM);
+ dd->residue_granularity = DMA_RESIDUE_GRANULARITY_SEGMENT;
+ dd->dev = &pdev->dev;
+ INIT_LIST_HEAD(&dd->channels);
+
+ if (pdev->dev.of_node && of_property_read_u32(pdev->dev.of_node,
+ "dma-requests",
+ &cqdma->dma_requests)) {
+ dev_dbg(&pdev->dev,
+ "Using %u as missing dma-requests property\n",
+ MTK_CQDMA_NR_VCHANS);
+
+ cqdma->dma_requests = MTK_CQDMA_NR_VCHANS;
+ }
+
+ if (pdev->dev.of_node && of_property_read_u32(pdev->dev.of_node,
+ "dma-channels",
+ &cqdma->dma_channels)) {
+ dev_dbg(&pdev->dev,
+ "Using %u as missing dma-channels property\n",
+ MTK_CQDMA_NR_PCHANS);
+
+ cqdma->dma_channels = MTK_CQDMA_NR_PCHANS;
+ }
+
+ cqdma->pc = devm_kcalloc(&pdev->dev, cqdma->dma_channels,
+ sizeof(*cqdma->pc), GFP_KERNEL);
+ if (!cqdma->pc)
+ return -ENOMEM;
+
+ /* initialization for PCs */
+ for (i = 0; i < cqdma->dma_channels; ++i) {
+ cqdma->pc[i] = devm_kcalloc(&pdev->dev, 1,
+ sizeof(**cqdma->pc), GFP_KERNEL);
+ if (!cqdma->pc[i])
+ return -ENOMEM;
+
+ INIT_LIST_HEAD(&cqdma->pc[i]->queue);
+ spin_lock_init(&cqdma->pc[i]->lock);
+ refcount_set(&cqdma->pc[i]->refcnt, 0);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, i);
+ if (!res) {
+ dev_err(&pdev->dev, "No mem resource for %s\n",
+ dev_name(&pdev->dev));
+ return -EINVAL;
+ }
+
+ cqdma->pc[i]->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(cqdma->pc[i]->base))
+ return PTR_ERR(cqdma->pc[i]->base);
+
+ /* allocate IRQ resource */
+ res = platform_get_resource(pdev, IORESOURCE_IRQ, i);
+ if (!res) {
+ dev_err(&pdev->dev, "No irq resource for %s\n",
+ dev_name(&pdev->dev));
+ return -EINVAL;
+ }
+ cqdma->pc[i]->irq = res->start;
+
+ err = devm_request_irq(&pdev->dev, cqdma->pc[i]->irq,
+ mtk_cqdma_irq, 0, dev_name(&pdev->dev),
+ cqdma);
+ if (err) {
+ dev_err(&pdev->dev,
+ "request_irq failed with err %d\n", err);
+ return -EINVAL;
+ }
+ }
+
+ /* allocate resource for VCs */
+ cqdma->vc = devm_kcalloc(&pdev->dev, cqdma->dma_requests,
+ sizeof(*cqdma->vc), GFP_KERNEL);
+ if (!cqdma->vc)
+ return -ENOMEM;
+
+ for (i = 0; i < cqdma->dma_requests; i++) {
+ vc = &cqdma->vc[i];
+ vc->vc.desc_free = mtk_cqdma_vdesc_free;
+ vchan_init(&vc->vc, dd);
+ init_completion(&vc->issue_completion);
+ }
+
+ err = dma_async_device_register(dd);
+ if (err)
+ return err;
+
+ err = of_dma_controller_register(pdev->dev.of_node,
+ of_dma_xlate_by_chan_id, cqdma);
+ if (err) {
+ dev_err(&pdev->dev,
+ "MediaTek CQDMA OF registration failed %d\n", err);
+ goto err_unregister;
+ }
+
+ err = mtk_cqdma_hw_init(cqdma);
+ if (err) {
+ dev_err(&pdev->dev,
+ "MediaTek CQDMA HW initialization failed %d\n", err);
+ goto err_unregister;
+ }
+
+ platform_set_drvdata(pdev, cqdma);
+
+ dev_dbg(&pdev->dev, "MediaTek CQDMA driver registered\n");
+
+ return 0;
+
+err_unregister:
+ dma_async_device_unregister(dd);
+
+ return err;
+}
+
+static int mtk_cqdma_remove(struct platform_device *pdev)
+{
+ struct mtk_cqdma_device *cqdma = platform_get_drvdata(pdev);
+ struct mtk_cqdma_vchan *vc;
+ unsigned long flags;
+ int i;
+
+ /* kill VC task */
+ for (i = 0; i < cqdma->dma_requests; i++) {
+ vc = &cqdma->vc[i];
+
+ list_del(&vc->vc.chan.device_node);
+ tasklet_kill(&vc->vc.task);
+ }
+
+ /* disable interrupt */
+ for (i = 0; i < cqdma->dma_channels; i++) {
+ spin_lock_irqsave(&cqdma->pc[i]->lock, flags);
+ mtk_dma_clr(cqdma->pc[i], MTK_CQDMA_INT_EN,
+ MTK_CQDMA_INT_EN_BIT);
+ spin_unlock_irqrestore(&cqdma->pc[i]->lock, flags);
+
+ /* Waits for any pending IRQ handlers to complete */
+ synchronize_irq(cqdma->pc[i]->irq);
+ }
+
+ /* disable hardware */
+ mtk_cqdma_hw_deinit(cqdma);
+
+ dma_async_device_unregister(&cqdma->ddev);
+ of_dma_controller_free(pdev->dev.of_node);
+
+ return 0;
+}
+
+static struct platform_driver mtk_cqdma_driver = {
+ .probe = mtk_cqdma_probe,
+ .remove = mtk_cqdma_remove,
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .of_match_table = mtk_cqdma_match,
+ },
+};
+module_platform_driver(mtk_cqdma_driver);
+
+MODULE_DESCRIPTION("MediaTek CQDMA Controller Driver");
+MODULE_AUTHOR("Shun-Chih Yu <shun-chih.yu@mediatek.com>");
+MODULE_LICENSE("GPL v2");
^ permalink raw reply related
* [1/3] k3dma: Upgrade k3dma drever to support hisi_asp_dma hardware
From: h00249924 @ 2018-12-28 6:36 UTC (permalink / raw)
To: dmaengine, devicetree, linux-kernel, linux-arm-kernel
Cc: suzhuangluan, kongfei, liyuequan, cash.qianli, huangli295,
hantanglei, wangyoulin1, ninggaoyu, hanxiaolong3, Youlin Wang,
Dan Williams, Vinod Koul
From: Youlin Wang <wwx575822@notesmail.huawei.com>
There is an new "hisi-pcm-asp-dma-1.0" device added in
"arch/arm64/boot/dts/hisilicon/hi3660.dtsi".
So we have to add a matching id in the driver file:
"{ .compatible = "hisilicon,hisi-pcm-asp-dma-1.0", }"
And also hisi-pcm-asp dma device needs no setting to the clock.
So we skip this by "if" sentence on id string matching:
"if (strcasecmp((of_id->compatible), (k3_pdma_dt_ids[0].compatible)) == 0)"
After above this driver will support both k3 and hisi_asp dma hardware.
Signed-off-by: Youlin Wang <wwx575822@notesmail.huawei.com>
Signed-off-by: Tanglei Han <hantanglei@huawei.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Vinod Koul <vkoul@kernel.org>
---
drivers/dma/k3dma.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/k3dma.c b/drivers/dma/k3dma.c
index fdec2b6..10eecc2 100644
--- a/drivers/dma/k3dma.c
+++ b/drivers/dma/k3dma.c
@@ -792,6 +792,7 @@ static int k3_dma_transfer_resume(struct dma_chan *chan)
static const struct of_device_id k3_pdma_dt_ids[] = {
{ .compatible = "hisilicon,k3-dma-1.0", },
+ { .compatible = "hisilicon,hisi-pcm-asp-dma-1.0", },
{}
};
MODULE_DEVICE_TABLE(of, k3_pdma_dt_ids);
@@ -835,10 +836,12 @@ static int k3_dma_probe(struct platform_device *op)
"dma-requests", &d->dma_requests);
}
- d->clk = devm_clk_get(&op->dev, NULL);
- if (IS_ERR(d->clk)) {
- dev_err(&op->dev, "no dma clk\n");
- return PTR_ERR(d->clk);
+ if (strcasecmp((of_id->compatible), (k3_pdma_dt_ids[0].compatible)) == 0) {
+ d->clk = devm_clk_get(&op->dev, NULL);
+ if (IS_ERR(d->clk)) {
+ dev_err(&op->dev, "no dma clk\n");
+ return PTR_ERR(d->clk);
+ }
}
irq = platform_get_irq(op, 0);
^ permalink raw reply related
* [2/3] dmaengine: Extend the k3dma driver binding
From: h00249924 @ 2018-12-28 6:36 UTC (permalink / raw)
To: dmaengine, devicetree, linux-kernel, linux-arm-kernel
Cc: suzhuangluan, kongfei, liyuequan, cash.qianli, huangli295,
hantanglei, wangyoulin1, ninggaoyu, hanxiaolong3, Youlin Wang,
Vinod Koul, Rob Herring, Mark Rutland
From: Youlin Wang <wwx575822@notesmail.huawei.com>
Extend the k3dma driver binding to support hisi-asp hardware variants.
Signed-off-by: Youlin Wang <wwx575822@notesmail.huawei.com>
Signed-off-by: Tanglei Han <hantanglei@huawei.com>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
---
Documentation/devicetree/bindings/dma/k3dma.txt | 33 ++++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/dma/k3dma.txt b/Documentation/devicetree/bindings/dma/k3dma.txt
index 4945aea..cd21b82 100644
--- a/Documentation/devicetree/bindings/dma/k3dma.txt
+++ b/Documentation/devicetree/bindings/dma/k3dma.txt
@@ -3,7 +3,9 @@
See dma.txt first
Required properties:
-- compatible: Should be "hisilicon,k3-dma-1.0"
+- compatible: Must be one of
+- "hisilicon,k3-dma-1.0"
+- "hisilicon,hisi-pcm-asp-dma-1.0"
- reg: Should contain DMA registers location and length.
- interrupts: Should contain one interrupt shared by all channel
- #dma-cells: see dma.txt, should be 1, para number
@@ -43,3 +45,32 @@ For example, i2c0 read channel request line is 18, while write channel use 19
dma-names = "rx", "tx";
};
+
+
+
+Controller:
+ asp_dmac: asp_dmac@E804B000 {
+ compatible = "hisilicon,hisi-pcm-asp-dma-1.0";
+ reg = <0x0 0xe804b000 0x0 0x1000>;
+ #dma-cells = <1>;
+ dma-channels = <16>;
+ dma-requests = <32>;
+ dma-min-chan = <0>;
+ dma-used-chans = <0xFFFE>;
+ dma-share;
+ interrupts = <0 216 4>;
+ interrupt-names = "asp_dma_irq";
+ status = "ok";
+ };
+
+Client:
+ i2s2: hisi_i2s {
+ compatible = "hisilicon,hisi-i2s";
+ reg = <0x0 0xe804f800 0x0 0x400>,
+ <0x0 0xe804e000 0x0 0x400>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2s2_pmx_func &i2s2_cfg_func>;
+ dmas = <&asp_dmac 18 &asp_dmac 19>;
+ dma-names = "rx", "tx";
+ #sound-dai-cells = <0>;
+ };
^ permalink raw reply related
* [3/3] arm64: dts: hi3660: Add hisi asp dma device
From: h00249924 @ 2018-12-28 6:36 UTC (permalink / raw)
To: dmaengine, devicetree, linux-kernel, linux-arm-kernel
Cc: suzhuangluan, kongfei, liyuequan, cash.qianli, huangli295,
hantanglei, wangyoulin1, ninggaoyu, hanxiaolong3, Youlin Wang,
John Stultz, Wei Xu, Rob Herring, Mark Rutland
From: Youlin Wang <wwx575822@notesmail.huawei.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: Youlin Wang <wwx575822@notesmail.huawei.com>
Signed-off-by: Tanglei Han <hantanglei@huawei.com>
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
---
arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
index f432b0a..5223e36 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
@@ -1122,5 +1122,19 @@
};
};
};
+
+ asp_dmac: asp_dmac@E804B000 {
+ compatible = "hisilicon,hisi-pcm-asp-dma-1.0";
+ reg = <0x0 0xe804b000 0x0 0x1000>;
+ #dma-cells = <1>;
+ dma-channels = <16>;
+ dma-requests = <32>;
+ dma-min-chan = <0>;
+ dma-used-chans = <0xFFFE>;
+ dma-share;
+ interrupts = <0 216 4>;
+ interrupt-names = "asp_dma_irq";
+ status = "ok";
+ };
};
};
^ permalink raw reply related
* [2/5] dmaengine: bcm2835: Fix abort of transactions
From: Stefan Wahren @ 2018-12-28 13:20 UTC (permalink / raw)
To: Lukas Wunner
Cc: Frank Pavlic, Martin Sperl, Florian Meier, dmaengine, Eric Anholt,
linux-rpi-kernel, Vinod Koul
Hi Lukas,
> Lukas Wunner <lukas@wunner.de> hat am 22. Dezember 2018 um 08:28 geschrieben:
>
>
> ...
> drivers/dma/bcm2835-dma.c | 33 +++------------------------------
> 1 file changed, 3 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/dma/bcm2835-dma.c b/drivers/dma/bcm2835-dma.c
> index e94f41c56975..17bc7304db3a 100644
> --- a/drivers/dma/bcm2835-dma.c
> +++ b/drivers/dma/bcm2835-dma.c
> @@ -419,25 +419,11 @@ static int bcm2835_dma_abort(void __iomem *chan_base)
> writel(0, chan_base + BCM2835_DMA_CS);
>
> /* Wait for any current AXI transfer to complete */
> - while ((cs & BCM2835_DMA_ISPAUSED) && --timeout) {
> + while ((readl(chan_base + BCM2835_DMA_CS) &
> + BCM2835_DMA_WAITING_FOR_WRITES) && --timeout)
> cpu_relax();
> - cs = readl(chan_base + BCM2835_DMA_CS);
> - }
> -
> - /* We'll un-pause when we set of our next DMA */
> - if (!timeout)
> - return -ETIMEDOUT;
i'm only sceptical about silently ignoring the timeout case. I prefer to have a comment explaining why we proceed with BCM2835_DMA_RESET in this case and some kind of error / debug message like below.
> -
> - if (!(cs & BCM2835_DMA_ACTIVE))
> - return 0;
> -
> - /* Terminate the control block chain */
> - writel(0, chan_base + BCM2835_DMA_NEXTCB);
> -
> - /* Abort the whole DMA */
> - writel(BCM2835_DMA_ABORT | BCM2835_DMA_ACTIVE,
> - chan_base + BCM2835_DMA_CS);
>
> + writel(BCM2835_DMA_RESET, chan_base + BCM2835_DMA_CS);
> return 0;
> }
>
> @@ -787,7 +773,6 @@ static int bcm2835_dma_terminate_all(struct dma_chan *chan)
> struct bcm2835_chan *c = to_bcm2835_dma_chan(chan);
> struct bcm2835_dmadev *d = to_bcm2835_dma_dev(c->vc.chan.device);
> unsigned long flags;
> - int timeout = 10000;
> LIST_HEAD(head);
>
> spin_lock_irqsave(&c->vc.lock, flags);
> @@ -802,18 +787,6 @@ static int bcm2835_dma_terminate_all(struct dma_chan *chan)
> vchan_terminate_vdesc(&c->desc->vd);
> c->desc = NULL;
> bcm2835_dma_abort(c->chan_base);
> -
> - /* Wait for stopping */
> - while (--timeout) {
> - if (!(readl(c->chan_base + BCM2835_DMA_CS) &
> - BCM2835_DMA_ACTIVE))
> - break;
> -
> - cpu_relax();
> - }
> -
> - if (!timeout)
> - dev_err(d->ddev.dev, "DMA transfer could not be terminated\n");
> }
>
Stefan
^ permalink raw reply
* [5/5] dmaengine: bcm2835: Remove dead code
From: Stefan Wahren @ 2018-12-28 13:26 UTC (permalink / raw)
To: Lukas Wunner
Cc: Frank Pavlic, Martin Sperl, Florian Meier, dmaengine, Eric Anholt,
linux-rpi-kernel, Vinod Koul
Hi Lukas,
> Lukas Wunner <lukas@wunner.de> hat am 22. Dezember 2018 um 08:28 geschrieben:
>
>
> The BCM2835 DMA driver deletes a channel from a list upon termination
> without having added it to a list first. Moreover that operation is
> protected by a spinlock which isn't taken anywhere else. These appear
> to be remnants of an older version of the driver which accidentally
> got mainlined. Remove the dead code.
>
> While at it remove an outdated comment claiming the driver only supports
> cyclic transactions. The driver has been supporting other transaction
> types for more than two years.
>
the fact that your mixing two different changes in one patch results in a very general subject.
Please split this up and give them more specific subject lines.
Thanks Stefan
^ permalink raw reply
* [5/5] dmaengine: bcm2835: Remove dead code
From: Lukas Wunner @ 2018-12-28 13:55 UTC (permalink / raw)
To: Stefan Wahren
Cc: Frank Pavlic, Martin Sperl, Florian Meier, dmaengine, Eric Anholt,
linux-rpi-kernel, Vinod Koul
On Fri, Dec 28, 2018 at 02:26:00PM +0100, Stefan Wahren wrote:
> > Lukas Wunner <lukas@wunner.de> hat am 22. Dezember 2018 um 08:28 geschrieben:
> > The BCM2835 DMA driver deletes a channel from a list upon termination
> > without having added it to a list first. Moreover that operation is
> > protected by a spinlock which isn't taken anywhere else. These appear
> > to be remnants of an older version of the driver which accidentally
> > got mainlined. Remove the dead code.
> >
> > While at it remove an outdated comment claiming the driver only supports
> > cyclic transactions. The driver has been supporting other transaction
> > types for more than two years.
>
> the fact that your mixing two different changes in one patch results
> in a very general subject.
In so far as a code comment can be considered code, removal of an
obsolete code comment can be referred to as removal of dead code.
So the subject seems pertinent to everything contained in this
patch from my point of view.
> Please split this up and give them more specific subject lines.
Frankly I don't consider removal of a 2 line code comment worthy a
commit of it's own, so if this is indeed a concern, I'd rather drop
it from the patch and leave the obsolete code comment in the file
for removal at some other date.
Thanks,
Lukas
^ permalink raw reply
* [5/5] dmaengine: bcm2835: Remove dead code
From: Stefan Wahren @ 2018-12-28 15:27 UTC (permalink / raw)
To: Lukas Wunner
Cc: Frank Pavlic, Martin Sperl, Florian Meier, dmaengine, Eric Anholt,
linux-rpi-kernel, Vinod Koul
> Lukas Wunner <lukas@wunner.de> hat am 28. Dezember 2018 um 14:55 geschrieben:
>
>
> On Fri, Dec 28, 2018 at 02:26:00PM +0100, Stefan Wahren wrote:
> > > Lukas Wunner <lukas@wunner.de> hat am 22. Dezember 2018 um 08:28 geschrieben:
> > > The BCM2835 DMA driver deletes a channel from a list upon termination
> > > without having added it to a list first. Moreover that operation is
> > > protected by a spinlock which isn't taken anywhere else. These appear
> > > to be remnants of an older version of the driver which accidentally
> > > got mainlined. Remove the dead code.
> > >
> > > While at it remove an outdated comment claiming the driver only supports
> > > cyclic transactions. The driver has been supporting other transaction
> > > types for more than two years.
> >
> > the fact that your mixing two different changes in one patch results
> > in a very general subject.
>
> In so far as a code comment can be considered code, removal of an
> obsolete code comment can be referred to as removal of dead code.
> So the subject seems pertinent to everything contained in this
> patch from my point of view.
This wasn't the point. It is common in a driver to remove dead code. So in case other commiters came to the idea to name their changes "remove dead code" it is very hard to distinguish those changes.
>
>
> > Please split this up and give them more specific subject lines.
>
> Frankly I don't consider removal of a 2 line code comment worthy a
> commit of it's own, so if this is indeed a concern, I'd rather drop
> it from the patch and leave the obsolete code comment in the file
> for removal at some other date.
Okay
>
> Thanks,
>
> Lukas
^ permalink raw reply
* dmaengine: stm32-mdma: Add a check on read_u32_array
From: Aditya Pakki @ 2018-12-28 19:26 UTC (permalink / raw)
To: pakki001
Cc: kjlu, Dan Williams, Vinod Koul, Maxime Coquelin, Alexandre Torgue,
dmaengine, linux-stm32, linux-arm-kernel, linux-kernel
In stm32_mdma_probe, after reading the property "st,ahb-addr-masks", the
second call is not checked for failure. This time of check to time of use
case of "count" error is sent upstream.
Signed-off-by: Aditya Pakki <pakki001@umn.edu>
---
drivers/dma/stm32-mdma.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/stm32-mdma.c b/drivers/dma/stm32-mdma.c
index 390e4cae0e1a..485dea177704 100644
--- a/drivers/dma/stm32-mdma.c
+++ b/drivers/dma/stm32-mdma.c
@@ -1579,9 +1579,11 @@ static int stm32_mdma_probe(struct platform_device *pdev)
dmadev->nr_channels = nr_channels;
dmadev->nr_requests = nr_requests;
- device_property_read_u32_array(&pdev->dev, "st,ahb-addr-masks",
+ ret = device_property_read_u32_array(&pdev->dev, "st,ahb-addr-masks",
dmadev->ahb_addr_masks,
count);
+ if (ret)
+ return ret;
dmadev->nr_ahb_addr_masks = count;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
^ permalink raw reply related
* dmaengine: qcom_hidma: Check for driver register failure
From: Aditya Pakki @ 2018-12-28 20:11 UTC (permalink / raw)
To: pakki001
Cc: kjlu, Sinan Kaya, Andy Gross, David Brown, Vinod Koul,
Dan Williams, linux-arm-kernel, linux-arm-msm, dmaengine,
linux-kernel
While initializing the driver, the function platform_driver_register can
fail and return an error. Consistent with other invocations, this patch
returns the error upstream.
Signed-off-by: Aditya Pakki <pakki001@umn.edu>
---
drivers/dma/qcom/hidma_mgmt.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/dma/qcom/hidma_mgmt.c b/drivers/dma/qcom/hidma_mgmt.c
index d64edeb6771a..681de12f4c67 100644
--- a/drivers/dma/qcom/hidma_mgmt.c
+++ b/drivers/dma/qcom/hidma_mgmt.c
@@ -423,9 +423,8 @@ static int __init hidma_mgmt_init(void)
hidma_mgmt_of_populate_channels(child);
}
#endif
- platform_driver_register(&hidma_mgmt_driver);
+ return platform_driver_register(&hidma_mgmt_driver);
- return 0;
}
module_init(hidma_mgmt_init);
MODULE_LICENSE("GPL v2");
^ permalink raw reply related
* [v8,1/2] dmaengine: 8250_mtk_dma: add MediaTek uart DMA support
From: Long Cheng @ 2018-12-29 10:30 UTC (permalink / raw)
To: Vinod Koul, Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee,
Sean Wang, Nicolas Boichat
Cc: Matthias Brugger, Dan Williams, Greg Kroah-Hartman, Jiri Slaby,
Sean Wang, dmaengine, devicetree, linux-arm-kernel,
linux-mediatek, linux-kernel, linux-serial, srv_heupstream,
Yingjoe Chen, YT Shen, Zhenbao Liu, Long Cheng
In DMA engine framework, add 8250 uart dma to support MediaTek uart.
If MediaTek uart enabled(SERIAL_8250_MT6577), and want to improve
the performance, can enable the function.
Signed-off-by: Long Cheng <long.cheng@mediatek.com>
---
drivers/dma/mediatek/8250_mtk_dma.c | 694 +++++++++++++++++++++++++++++++++++
drivers/dma/mediatek/Kconfig | 11 +
drivers/dma/mediatek/Makefile | 1 +
3 files changed, 706 insertions(+)
create mode 100644 drivers/dma/mediatek/8250_mtk_dma.c
diff --git a/drivers/dma/mediatek/8250_mtk_dma.c b/drivers/dma/mediatek/8250_mtk_dma.c
new file mode 100644
index 0000000..c4090f2
--- /dev/null
+++ b/drivers/dma/mediatek/8250_mtk_dma.c
@@ -0,0 +1,694 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * MediaTek 8250 DMA driver.
+ *
+ * Copyright (c) 2018 MediaTek Inc.
+ * Author: Long Cheng <long.cheng@mediatek.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/list.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_dma.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/pm_runtime.h>
+#include <linux/iopoll.h>
+
+#include "../virt-dma.h"
+
+#define MTK_UART_APDMA_CHANNELS (CONFIG_SERIAL_8250_NR_UARTS * 2)
+
+#define VFF_EN_B BIT(0)
+#define VFF_STOP_B BIT(0)
+#define VFF_FLUSH_B BIT(0)
+#define VFF_4G_SUPPORT_B BIT(0)
+#define VFF_RX_INT_EN0_B BIT(0) /*rx valid size >= vff thre*/
+#define VFF_RX_INT_EN1_B BIT(1)
+#define VFF_TX_INT_EN_B BIT(0) /*tx left size >= vff thre*/
+#define VFF_WARM_RST_B BIT(0)
+#define VFF_RX_INT_FLAG_CLR_B (BIT(0) | BIT(1))
+#define VFF_TX_INT_FLAG_CLR_B 0
+#define VFF_STOP_CLR_B 0
+#define VFF_FLUSH_CLR_B 0
+#define VFF_INT_EN_CLR_B 0
+#define VFF_4G_SUPPORT_CLR_B 0
+
+/* interrupt trigger level for tx */
+#define VFF_TX_THRE(n) ((n) * 7 / 8)
+/* interrupt trigger level for rx */
+#define VFF_RX_THRE(n) ((n) * 3 / 4)
+
+#define MTK_UART_APDMA_RING_SIZE 0xffffU
+/* invert this bit when wrap ring head again*/
+#define MTK_UART_APDMA_RING_WRAP 0x10000U
+
+#define VFF_INT_FLAG 0x00
+#define VFF_INT_EN 0x04
+#define VFF_EN 0x08
+#define VFF_RST 0x0c
+#define VFF_STOP 0x10
+#define VFF_FLUSH 0x14
+#define VFF_ADDR 0x1c
+#define VFF_LEN 0x24
+#define VFF_THRE 0x28
+#define VFF_WPT 0x2c
+#define VFF_RPT 0x30
+/*TX: the buffer size HW can read. RX: the buffer size SW can read.*/
+#define VFF_VALID_SIZE 0x3c
+/*TX: the buffer size SW can write. RX: the buffer size HW can write.*/
+#define VFF_LEFT_SIZE 0x40
+#define VFF_DEBUG_STATUS 0x50
+#define VFF_4G_SUPPORT 0x54
+
+struct mtk_uart_apdmadev {
+ struct dma_device ddev;
+ struct clk *clk;
+ bool support_33bits;
+ unsigned int dma_irq[MTK_UART_APDMA_CHANNELS];
+};
+
+struct mtk_uart_apdma_desc {
+ struct virt_dma_desc vd;
+
+ unsigned int avail_len;
+};
+
+struct mtk_chan {
+ struct virt_dma_chan vc;
+ struct dma_slave_config cfg;
+ void __iomem *base;
+ struct mtk_uart_apdma_desc *desc;
+
+ bool requested;
+
+ unsigned int rx_status;
+};
+
+static inline struct mtk_uart_apdmadev *
+to_mtk_uart_apdma_dev(struct dma_device *d)
+{
+ return container_of(d, struct mtk_uart_apdmadev, ddev);
+}
+
+static inline struct mtk_chan *to_mtk_uart_apdma_chan(struct dma_chan *c)
+{
+ return container_of(c, struct mtk_chan, vc.chan);
+}
+
+static inline struct mtk_uart_apdma_desc *to_mtk_uart_apdma_desc
+ (struct dma_async_tx_descriptor *t)
+{
+ return container_of(t, struct mtk_uart_apdma_desc, vd.tx);
+}
+
+static void mtk_uart_apdma_chan_write(struct mtk_chan *c,
+ unsigned int reg, unsigned int val)
+{
+ writel(val, c->base + reg);
+}
+
+static unsigned int
+mtk_uart_apdma_chan_read(struct mtk_chan *c, unsigned int reg)
+{
+ return readl(c->base + reg);
+}
+
+static void mtk_uart_apdma_desc_free(struct virt_dma_desc *vd)
+{
+ struct dma_chan *chan = vd->tx.chan;
+ struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
+
+ kfree(c->desc);
+ c->desc = NULL;
+}
+
+static void mtk_uart_apdma_start_tx(struct mtk_chan *c)
+{
+ unsigned int txcount = c->desc->avail_len;
+ unsigned int len, send, left, wpt, wrap;
+
+ if (mtk_uart_apdma_chan_read(c, VFF_LEFT_SIZE) == 0U) {
+ mtk_uart_apdma_chan_write(c, VFF_INT_EN, VFF_TX_INT_EN_B);
+ } else {
+ len = mtk_uart_apdma_chan_read(c, VFF_LEN);
+
+ while (((left = mtk_uart_apdma_chan_read(c,
+ VFF_LEFT_SIZE)) > 0U)
+ && (c->desc->avail_len != 0U)) {
+ send = min_t(unsigned int, left, c->desc->avail_len);
+ wpt = mtk_uart_apdma_chan_read(c, VFF_WPT);
+ wrap = wpt & MTK_UART_APDMA_RING_WRAP ?
+ 0U : MTK_UART_APDMA_RING_WRAP;
+
+ if ((wpt & (len - 1U)) + send < len)
+ mtk_uart_apdma_chan_write(c,
+ VFF_WPT, wpt + send);
+ else
+ mtk_uart_apdma_chan_write(c, VFF_WPT,
+ ((wpt + send) & (len - 1U))
+ | wrap);
+
+ c->desc->avail_len -= send;
+ }
+
+ if (txcount != c->desc->avail_len) {
+ mtk_uart_apdma_chan_write(c,
+ VFF_INT_EN, VFF_TX_INT_EN_B);
+ if (mtk_uart_apdma_chan_read(c,
+ VFF_FLUSH) == 0U)
+ mtk_uart_apdma_chan_write(c,
+ VFF_FLUSH, VFF_FLUSH_B);
+ }
+ }
+}
+
+static void mtk_uart_apdma_start_rx(struct mtk_chan *c)
+{
+ struct mtk_uart_apdma_desc *d = c->desc;
+ unsigned int rx_len, wg, rg, count;
+
+ if (mtk_uart_apdma_chan_read(c, VFF_VALID_SIZE) == 0U)
+ return;
+
+ if (d && vchan_next_desc(&c->vc)) {
+ rx_len = mtk_uart_apdma_chan_read(c, VFF_LEN);
+ rg = mtk_uart_apdma_chan_read(c, VFF_RPT);
+ wg = mtk_uart_apdma_chan_read(c, VFF_WPT);
+ count = ((rg ^ wg) & MTK_UART_APDMA_RING_WRAP) ?
+ ((wg & MTK_UART_APDMA_RING_SIZE) +
+ rx_len - (rg & MTK_UART_APDMA_RING_SIZE)) :
+ ((wg & MTK_UART_APDMA_RING_SIZE) -
+ (rg & MTK_UART_APDMA_RING_SIZE));
+
+ c->rx_status = count;
+ mtk_uart_apdma_chan_write(c, VFF_RPT, wg);
+
+ list_del(&d->vd.node);
+ vchan_cookie_complete(&d->vd);
+ }
+}
+
+static irqreturn_t mtk_uart_apdma_irq_handler(int irq, void *dev_id)
+{
+ struct dma_chan *chan = (struct dma_chan *)dev_id;
+ struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
+ struct mtk_uart_apdma_desc *d;
+ unsigned long flags;
+
+ spin_lock_irqsave(&c->vc.lock, flags);
+ switch (c->cfg.direction) {
+ case DMA_DEV_TO_MEM:
+ mtk_uart_apdma_chan_write(c,
+ VFF_INT_FLAG, VFF_RX_INT_FLAG_CLR_B);
+ mtk_uart_apdma_start_rx(c);
+ break;
+ case DMA_MEM_TO_DEV:
+ d = c->desc;
+
+ mtk_uart_apdma_chan_write(c,
+ VFF_INT_FLAG, VFF_TX_INT_FLAG_CLR_B);
+
+ if (d->avail_len != 0U) {
+ mtk_uart_apdma_start_tx(c);
+ } else {
+ list_del(&d->vd.node);
+ vchan_cookie_complete(&d->vd);
+ }
+ break;
+ default:
+ break;
+ }
+ spin_unlock_irqrestore(&c->vc.lock, flags);
+
+ return IRQ_HANDLED;
+}
+
+static int mtk_uart_apdma_alloc_chan_resources(struct dma_chan *chan)
+{
+ struct mtk_uart_apdmadev *mtkd = to_mtk_uart_apdma_dev(chan->device);
+ struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
+ u32 status;
+ int ret = -EBUSY;
+
+ pm_runtime_get_sync(mtkd->ddev.dev);
+
+ mtk_uart_apdma_chan_write(c, VFF_ADDR, 0);
+ mtk_uart_apdma_chan_write(c, VFF_THRE, 0);
+ mtk_uart_apdma_chan_write(c, VFF_LEN, 0);
+ mtk_uart_apdma_chan_write(c, VFF_RST, VFF_WARM_RST_B);
+
+ ret = readx_poll_timeout(readl,
+ c->base + VFF_EN,
+ status, status == 0, 10, 100);
+ if (ret) {
+ dev_err(c->vc.chan.device->dev,
+ "dma reset: fail, timeout\n");
+ goto exit;
+ }
+
+ if (!c->requested) {
+ c->requested = true;
+ ret = request_irq(mtkd->dma_irq[chan->chan_id],
+ mtk_uart_apdma_irq_handler,
+ IRQF_TRIGGER_NONE,
+ KBUILD_MODNAME, chan);
+ if (ret < 0) {
+ dev_err(chan->device->dev, "Can't request dma IRQ\n");
+ return -EINVAL;
+ }
+ }
+
+ if (mtkd->support_33bits)
+ mtk_uart_apdma_chan_write(c,
+ VFF_4G_SUPPORT, VFF_4G_SUPPORT_CLR_B);
+
+exit:
+ return ret;
+}
+
+static void mtk_uart_apdma_free_chan_resources(struct dma_chan *chan)
+{
+ struct mtk_uart_apdmadev *mtkd = to_mtk_uart_apdma_dev(chan->device);
+ struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
+
+ if (c->requested) {
+ c->requested = false;
+ free_irq(mtkd->dma_irq[chan->chan_id], chan);
+ }
+
+ tasklet_kill(&c->vc.task);
+
+ vchan_free_chan_resources(&c->vc);
+
+ pm_runtime_put_sync(mtkd->ddev.dev);
+}
+
+static enum dma_status mtk_uart_apdma_tx_status(struct dma_chan *chan,
+ dma_cookie_t cookie,
+ struct dma_tx_state *txstate)
+{
+ struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
+ enum dma_status ret;
+ unsigned long flags;
+
+ if (!txstate)
+ return DMA_ERROR;
+
+ ret = dma_cookie_status(chan, cookie, txstate);
+ spin_lock_irqsave(&c->vc.lock, flags);
+ if (ret == DMA_IN_PROGRESS) {
+ c->rx_status = mtk_uart_apdma_chan_read(c, VFF_RPT)
+ & MTK_UART_APDMA_RING_SIZE;
+ dma_set_residue(txstate, c->rx_status);
+ } else if (ret == DMA_COMPLETE && c->cfg.direction == DMA_DEV_TO_MEM) {
+ dma_set_residue(txstate, c->rx_status);
+ } else {
+ dma_set_residue(txstate, 0);
+ }
+ spin_unlock_irqrestore(&c->vc.lock, flags);
+
+ return ret;
+}
+
+/*
+ * dmaengine_prep_slave_single will call the function. and sglen is 1.
+ * 8250 uart using one ring buffer, and deal with one sg.
+ */
+static struct dma_async_tx_descriptor *mtk_uart_apdma_prep_slave_sg
+ (struct dma_chan *chan, struct scatterlist *sgl,
+ unsigned int sglen, enum dma_transfer_direction dir,
+ unsigned long tx_flags, void *context)
+{
+ struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
+ struct mtk_uart_apdma_desc *d;
+
+ if ((dir != DMA_DEV_TO_MEM) &&
+ (dir != DMA_MEM_TO_DEV)) {
+ dev_err(chan->device->dev, "bad direction\n");
+ return NULL;
+ }
+
+ /* Now allocate and setup the descriptor */
+ d = kzalloc(sizeof(*d), GFP_ATOMIC);
+ if (!d)
+ return NULL;
+
+ /* sglen is 1 */
+ d->avail_len = sg_dma_len(sgl);
+
+ return vchan_tx_prep(&c->vc, &d->vd, tx_flags);
+}
+
+static void mtk_uart_apdma_issue_pending(struct dma_chan *chan)
+{
+ struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
+ struct virt_dma_desc *vd;
+ unsigned long flags;
+
+ spin_lock_irqsave(&c->vc.lock, flags);
+ if (c->cfg.direction == DMA_DEV_TO_MEM) {
+ if (vchan_issue_pending(&c->vc) && !c->desc) {
+ vd = vchan_next_desc(&c->vc);
+ c->desc = to_mtk_uart_apdma_desc(&vd->tx);
+ mtk_uart_apdma_start_rx(c);
+ }
+ } else if (c->cfg.direction == DMA_MEM_TO_DEV) {
+ if (vchan_issue_pending(&c->vc) && !c->desc) {
+ vd = vchan_next_desc(&c->vc);
+ c->desc = to_mtk_uart_apdma_desc(&vd->tx);
+ mtk_uart_apdma_start_tx(c);
+ }
+ }
+ spin_unlock_irqrestore(&c->vc.lock, flags);
+}
+
+static int mtk_uart_apdma_slave_config(struct dma_chan *chan,
+ struct dma_slave_config *cfg)
+{
+ struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
+ struct mtk_uart_apdmadev *mtkd =
+ to_mtk_uart_apdma_dev(c->vc.chan.device);
+
+ c->cfg = *cfg;
+
+ if (cfg->direction == DMA_DEV_TO_MEM) {
+ unsigned int rx_len = cfg->src_addr_width * 1024;
+
+ mtk_uart_apdma_chan_write(c, VFF_ADDR, cfg->src_addr);
+ mtk_uart_apdma_chan_write(c, VFF_LEN, rx_len);
+ mtk_uart_apdma_chan_write(c, VFF_THRE, VFF_RX_THRE(rx_len));
+ mtk_uart_apdma_chan_write(c,
+ VFF_INT_EN, VFF_RX_INT_EN0_B
+ | VFF_RX_INT_EN1_B);
+ mtk_uart_apdma_chan_write(c, VFF_RPT, 0);
+ mtk_uart_apdma_chan_write(c,
+ VFF_INT_FLAG, VFF_RX_INT_FLAG_CLR_B);
+ mtk_uart_apdma_chan_write(c, VFF_EN, VFF_EN_B);
+ } else if (cfg->direction == DMA_MEM_TO_DEV) {
+ unsigned int tx_len = cfg->dst_addr_width * 1024;
+
+ mtk_uart_apdma_chan_write(c, VFF_ADDR, cfg->dst_addr);
+ mtk_uart_apdma_chan_write(c, VFF_LEN, tx_len);
+ mtk_uart_apdma_chan_write(c, VFF_THRE, VFF_TX_THRE(tx_len));
+ mtk_uart_apdma_chan_write(c, VFF_WPT, 0);
+ mtk_uart_apdma_chan_write(c,
+ VFF_INT_FLAG, VFF_TX_INT_FLAG_CLR_B);
+ mtk_uart_apdma_chan_write(c, VFF_EN, VFF_EN_B);
+ }
+
+ if (mtkd->support_33bits)
+ mtk_uart_apdma_chan_write(c, VFF_4G_SUPPORT, VFF_4G_SUPPORT_B);
+
+ if (mtk_uart_apdma_chan_read(c, VFF_EN) != VFF_EN_B) {
+ dev_err(chan->device->dev,
+ "config dma dir[%d] fail\n", cfg->direction);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int mtk_uart_apdma_terminate_all(struct dma_chan *chan)
+{
+ struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
+ unsigned long flags;
+ u32 status;
+ int ret;
+
+ spin_lock_irqsave(&c->vc.lock, flags);
+
+ mtk_uart_apdma_chan_write(c, VFF_FLUSH, VFF_FLUSH_CLR_B);
+ /* Wait for flush */
+ ret = readx_poll_timeout(readl,
+ c->base + VFF_FLUSH,
+ status,
+ (status & VFF_FLUSH_B) != VFF_FLUSH_B,
+ 10, 100);
+ if (ret)
+ dev_err(c->vc.chan.device->dev,
+ "dma stop: polling FLUSH fail, DEBUG=0x%x\n",
+ mtk_uart_apdma_chan_read(c, VFF_DEBUG_STATUS));
+
+ /*set stop as 1 -> wait until en is 0 -> set stop as 0*/
+ mtk_uart_apdma_chan_write(c, VFF_STOP, VFF_STOP_B);
+ ret = readx_poll_timeout(readl,
+ c->base + VFF_EN,
+ status, status == 0, 10, 100);
+ if (ret)
+ dev_err(c->vc.chan.device->dev,
+ "dma stop: polling VFF_EN fail, DEBUG=0x%x\n",
+ mtk_uart_apdma_chan_read(c, VFF_DEBUG_STATUS));
+
+ mtk_uart_apdma_chan_write(c, VFF_STOP, VFF_STOP_CLR_B);
+ mtk_uart_apdma_chan_write(c, VFF_INT_EN, VFF_INT_EN_CLR_B);
+
+ switch (c->cfg.direction) {
+ case DMA_DEV_TO_MEM:
+ mtk_uart_apdma_chan_write(c,
+ VFF_INT_FLAG, VFF_RX_INT_FLAG_CLR_B);
+ break;
+ case DMA_MEM_TO_DEV:
+ mtk_uart_apdma_chan_write(c,
+ VFF_INT_FLAG, VFF_TX_INT_FLAG_CLR_B);
+ break;
+ default:
+ break;
+ }
+
+ spin_unlock_irqrestore(&c->vc.lock, flags);
+
+ return 0;
+}
+
+static int mtk_uart_apdma_device_pause(struct dma_chan *chan)
+{
+ /* just for check caps pass */
+ return 0;
+}
+
+static int mtk_uart_apdma_device_resume(struct dma_chan *chan)
+{
+ /* just for check caps pass */
+ return 0;
+}
+
+static void mtk_uart_apdma_free(struct mtk_uart_apdmadev *mtkd)
+{
+ while (list_empty(&mtkd->ddev.channels) == 0) {
+ struct mtk_chan *c = list_first_entry(&mtkd->ddev.channels,
+ struct mtk_chan, vc.chan.device_node);
+
+ list_del(&c->vc.chan.device_node);
+ tasklet_kill(&c->vc.task);
+ }
+}
+
+static const struct of_device_id mtk_uart_apdma_match[] = {
+ { .compatible = "mediatek,mt6577-uart-dma", },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, mtk_uart_apdma_match);
+
+static int mtk_uart_apdma_probe(struct platform_device *pdev)
+{
+ struct mtk_uart_apdmadev *mtkd;
+ struct resource *res;
+ struct mtk_chan *c;
+ unsigned int i;
+ int rc;
+
+ mtkd = devm_kzalloc(&pdev->dev, sizeof(*mtkd), GFP_KERNEL);
+ if (!mtkd)
+ return -ENOMEM;
+
+ mtkd->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(mtkd->clk)) {
+ dev_err(&pdev->dev, "No clock specified\n");
+ rc = PTR_ERR(mtkd->clk);
+ goto err_no_dma;
+ }
+
+ if (of_property_read_bool(pdev->dev.of_node, "dma-33bits")) {
+ dev_info(&pdev->dev, "Support dma 33bits\n");
+ mtkd->support_33bits = true;
+ }
+
+ rc = dma_set_mask_and_coherent(&pdev->dev,
+ DMA_BIT_MASK(32 | mtkd->support_33bits));
+ if (rc)
+ goto err_no_dma;
+
+ dma_cap_set(DMA_SLAVE, mtkd->ddev.cap_mask);
+ mtkd->ddev.device_alloc_chan_resources =
+ mtk_uart_apdma_alloc_chan_resources;
+ mtkd->ddev.device_free_chan_resources =
+ mtk_uart_apdma_free_chan_resources;
+ mtkd->ddev.device_tx_status = mtk_uart_apdma_tx_status;
+ mtkd->ddev.device_issue_pending = mtk_uart_apdma_issue_pending;
+ mtkd->ddev.device_prep_slave_sg = mtk_uart_apdma_prep_slave_sg;
+ mtkd->ddev.device_config = mtk_uart_apdma_slave_config;
+ mtkd->ddev.device_pause = mtk_uart_apdma_device_pause;
+ mtkd->ddev.device_resume = mtk_uart_apdma_device_resume;
+ mtkd->ddev.device_terminate_all = mtk_uart_apdma_terminate_all;
+ mtkd->ddev.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE);
+ mtkd->ddev.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE);
+ mtkd->ddev.directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
+ mtkd->ddev.residue_granularity = DMA_RESIDUE_GRANULARITY_SEGMENT;
+ mtkd->ddev.dev = &pdev->dev;
+ INIT_LIST_HEAD(&mtkd->ddev.channels);
+
+ for (i = 0; i < MTK_UART_APDMA_CHANNELS; i++) {
+ c = devm_kzalloc(mtkd->ddev.dev, sizeof(*c), GFP_KERNEL);
+ if (!c) {
+ rc = -ENODEV;
+ goto err_no_dma;
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, i);
+ if (!res) {
+ rc = -ENODEV;
+ goto err_no_dma;
+ }
+
+ c->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(c->base)) {
+ rc = PTR_ERR(c->base);
+ goto err_no_dma;
+ }
+ c->requested = false;
+ c->vc.desc_free = mtk_uart_apdma_desc_free;
+ vchan_init(&c->vc, &mtkd->ddev);
+
+ mtkd->dma_irq[i] = platform_get_irq(pdev, i);
+ if ((int)mtkd->dma_irq[i] < 0) {
+ dev_err(&pdev->dev, "failed to get IRQ[%d]\n", i);
+ rc = -EINVAL;
+ goto err_no_dma;
+ }
+ }
+
+ pm_runtime_enable(&pdev->dev);
+ pm_runtime_set_active(&pdev->dev);
+
+ rc = dma_async_device_register(&mtkd->ddev);
+ if (rc)
+ goto rpm_disable;
+
+ platform_set_drvdata(pdev, mtkd);
+
+ if (pdev->dev.of_node) {
+ /* Device-tree DMA controller registration */
+ rc = of_dma_controller_register(pdev->dev.of_node,
+ of_dma_xlate_by_chan_id,
+ mtkd);
+ if (rc)
+ goto dma_remove;
+ }
+
+ return rc;
+
+dma_remove:
+ dma_async_device_unregister(&mtkd->ddev);
+rpm_disable:
+ pm_runtime_disable(&pdev->dev);
+err_no_dma:
+ mtk_uart_apdma_free(mtkd);
+ return rc;
+}
+
+static int mtk_uart_apdma_remove(struct platform_device *pdev)
+{
+ struct mtk_uart_apdmadev *mtkd = platform_get_drvdata(pdev);
+
+ if (pdev->dev.of_node)
+ of_dma_controller_free(pdev->dev.of_node);
+
+ pm_runtime_disable(&pdev->dev);
+ pm_runtime_put_noidle(&pdev->dev);
+
+ dma_async_device_unregister(&mtkd->ddev);
+ mtk_uart_apdma_free(mtkd);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int mtk_uart_apdma_suspend(struct device *dev)
+{
+ struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
+
+ if (!pm_runtime_suspended(dev))
+ clk_disable_unprepare(mtkd->clk);
+
+ return 0;
+}
+
+static int mtk_uart_apdma_resume(struct device *dev)
+{
+ int ret;
+ struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
+
+ if (!pm_runtime_suspended(dev)) {
+ ret = clk_prepare_enable(mtkd->clk);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+#ifdef CONFIG_PM
+static int mtk_uart_apdma_runtime_suspend(struct device *dev)
+{
+ struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
+
+ clk_disable_unprepare(mtkd->clk);
+
+ return 0;
+}
+
+static int mtk_uart_apdma_runtime_resume(struct device *dev)
+{
+ int ret;
+ struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
+
+ ret = clk_prepare_enable(mtkd->clk);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+#endif /* CONFIG_PM */
+
+static const struct dev_pm_ops mtk_uart_apdma_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(mtk_uart_apdma_suspend, mtk_uart_apdma_resume)
+ SET_RUNTIME_PM_OPS(mtk_uart_apdma_runtime_suspend,
+ mtk_uart_apdma_runtime_resume, NULL)
+};
+
+static struct platform_driver mtk_uart_apdma_driver = {
+ .probe = mtk_uart_apdma_probe,
+ .remove = mtk_uart_apdma_remove,
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .pm = &mtk_uart_apdma_pm_ops,
+ .of_match_table = of_match_ptr(mtk_uart_apdma_match),
+ },
+};
+
+module_platform_driver(mtk_uart_apdma_driver);
+
+MODULE_DESCRIPTION("MediaTek UART APDMA Controller Driver");
+MODULE_AUTHOR("Long Cheng <long.cheng@mediatek.com>");
+MODULE_LICENSE("GPL v2");
+
diff --git a/drivers/dma/mediatek/Kconfig b/drivers/dma/mediatek/Kconfig
index 27bac0b..1a523c87 100644
--- a/drivers/dma/mediatek/Kconfig
+++ b/drivers/dma/mediatek/Kconfig
@@ -1,4 +1,15 @@
+config DMA_MTK_UART
+ tristate "MediaTek SoCs APDMA support for UART"
+ depends on OF && SERIAL_8250_MT6577
+ select DMA_ENGINE
+ select DMA_VIRTUAL_CHANNELS
+ help
+ Support for the UART DMA engine found on MediaTek MTK SoCs.
+ when SERIAL_8250_MT6577 is enabled, and if you want to use DMA,
+ you can enable the config. the DMA engine can only be used
+ with MediaTek SoCs.
+
config MTK_HSDMA
tristate "MediaTek High-Speed DMA controller support"
depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/dma/mediatek/Makefile b/drivers/dma/mediatek/Makefile
index 6e778f8..2f2efd9 100644
--- a/drivers/dma/mediatek/Makefile
+++ b/drivers/dma/mediatek/Makefile
@@ -1 +1,2 @@
+obj-$(CONFIG_DMA_MTK_UART) += 8250_mtk_dma.o
obj-$(CONFIG_MTK_HSDMA) += mtk-hsdma.o
^ permalink raw reply related
* [v8,2/2] arm: dts: mt2712: add uart APDMA to device tree
From: Long Cheng @ 2018-12-29 10:30 UTC (permalink / raw)
To: Vinod Koul, Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee,
Sean Wang, Nicolas Boichat
Cc: Matthias Brugger, Dan Williams, Greg Kroah-Hartman, Jiri Slaby,
Sean Wang, dmaengine, devicetree, linux-arm-kernel,
linux-mediatek, linux-kernel, linux-serial, srv_heupstream,
Yingjoe Chen, YT Shen, Zhenbao Liu, Long Cheng
1. add uart APDMA controller device node
2. add uart 0/1/2/3/4/5 DMA function
Signed-off-by: Long Cheng <long.cheng@mediatek.com>
---
arch/arm64/boot/dts/mediatek/mt2712e.dtsi | 50 +++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
index 976d92a..be1a22a 100644
--- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
@@ -300,6 +300,9 @@
interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_LOW>;
clocks = <&baud_clk>, <&sys_clk>;
clock-names = "baud", "bus";
+ dmas = <&apdma 10
+ &apdma 11>;
+ dma-names = "tx", "rx";
status = "disabled";
};
@@ -369,6 +372,38 @@
(GIC_CPU_MASK_RAW(0x13) | IRQ_TYPE_LEVEL_HIGH)>;
};
+ apdma: dma-controller@11000400 {
+ compatible = "mediatek,mt2712-uart-dma",
+ "mediatek,mt6577-uart-dma";
+ reg = <0 0x11000400 0 0x80>,
+ <0 0x11000480 0 0x80>,
+ <0 0x11000500 0 0x80>,
+ <0 0x11000580 0 0x80>,
+ <0 0x11000600 0 0x80>,
+ <0 0x11000680 0 0x80>,
+ <0 0x11000700 0 0x80>,
+ <0 0x11000780 0 0x80>,
+ <0 0x11000800 0 0x80>,
+ <0 0x11000880 0 0x80>,
+ <0 0x11000900 0 0x80>,
+ <0 0x11000980 0 0x80>;
+ interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 104 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 105 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 106 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 107 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 108 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 109 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 110 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 111 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 112 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 114 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&pericfg CLK_PERI_AP_DMA>;
+ clock-names = "apdma";
+ #dma-cells = <1>;
+ };
+
auxadc: adc@11001000 {
compatible = "mediatek,mt2712-auxadc";
reg = <0 0x11001000 0 0x1000>;
@@ -385,6 +420,9 @@
interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_LOW>;
clocks = <&baud_clk>, <&sys_clk>;
clock-names = "baud", "bus";
+ dmas = <&apdma 0
+ &apdma 1>;
+ dma-names = "tx", "rx";
status = "disabled";
};
@@ -395,6 +433,9 @@
interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_LOW>;
clocks = <&baud_clk>, <&sys_clk>;
clock-names = "baud", "bus";
+ dmas = <&apdma 2
+ &apdma 3>;
+ dma-names = "tx", "rx";
status = "disabled";
};
@@ -405,6 +446,9 @@
interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_LOW>;
clocks = <&baud_clk>, <&sys_clk>;
clock-names = "baud", "bus";
+ dmas = <&apdma 4
+ &apdma 5>;
+ dma-names = "tx", "rx";
status = "disabled";
};
@@ -415,6 +459,9 @@
interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_LOW>;
clocks = <&baud_clk>, <&sys_clk>;
clock-names = "baud", "bus";
+ dmas = <&apdma 6
+ &apdma 7>;
+ dma-names = "tx", "rx";
status = "disabled";
};
@@ -629,6 +676,9 @@
interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_LOW>;
clocks = <&baud_clk>, <&sys_clk>;
clock-names = "baud", "bus";
+ dmas = <&apdma 8
+ &apdma 9>;
+ dma-names = "tx", "rx";
status = "disabled";
};
^ permalink raw reply related
* dmaengine: qcom_hidma: Check for driver register failure
From: Sinan Kaya @ 2018-12-30 18:34 UTC (permalink / raw)
To: Aditya Pakki
Cc: kjlu, Andy Gross, David Brown, Vinod Koul, Dan Williams,
linux-arm-kernel, linux-arm-msm, dmaengine, open list
On Fri, Dec 28, 2018 at 11:11 PM Aditya Pakki <pakki001@umn.edu> wrote:
>
> While initializing the driver, the function platform_driver_register can
> fail and return an error. Consistent with other invocations, this patch
> returns the error upstream.
>
> Signed-off-by: Aditya Pakki <pakki001@umn.edu>
> ---
Acked-by: Sinan Kaya <okaya@kernel.org>
> drivers/dma/qcom/hidma_mgmt.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/dma/qcom/hidma_mgmt.c b/drivers/dma/qcom/hidma_mgmt.c
> index d64edeb6771a..681de12f4c67 100644
> --- a/drivers/dma/qcom/hidma_mgmt.c
> +++ b/drivers/dma/qcom/hidma_mgmt.c
> @@ -423,9 +423,8 @@ static int __init hidma_mgmt_init(void)
> hidma_mgmt_of_populate_channels(child);
> }
> #endif
> - platform_driver_register(&hidma_mgmt_driver);
> + return platform_driver_register(&hidma_mgmt_driver);
>
> - return 0;
> }
> module_init(hidma_mgmt_init);
> MODULE_LICENSE("GPL v2");
> --
> 2.17.1
>
^ permalink raw reply
* [v9,1/2] dmaengine: 8250_mtk_dma: add MediaTek uart DMA support
From: Long Cheng @ 2019-01-02 2:12 UTC (permalink / raw)
To: Vinod Koul, Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee,
Sean Wang, Nicolas Boichat
Cc: Matthias Brugger, Dan Williams, Greg Kroah-Hartman, Jiri Slaby,
Sean Wang, dmaengine, devicetree, linux-arm-kernel,
linux-mediatek, linux-kernel, linux-serial, srv_heupstream,
Yingjoe Chen, YT Shen, Zhenbao Liu, Long Cheng
In DMA engine framework, add 8250 uart dma to support MediaTek uart.
If MediaTek uart enabled(SERIAL_8250_MT6577), and want to improve
the performance, can enable the function.
Signed-off-by: Long Cheng <long.cheng@mediatek.com>
---
drivers/dma/mediatek/8250_mtk_dma.c | 652 +++++++++++++++++++++++++++++++++++
drivers/dma/mediatek/Kconfig | 11 +
drivers/dma/mediatek/Makefile | 1 +
3 files changed, 664 insertions(+)
create mode 100644 drivers/dma/mediatek/8250_mtk_dma.c
diff --git a/drivers/dma/mediatek/8250_mtk_dma.c b/drivers/dma/mediatek/8250_mtk_dma.c
new file mode 100644
index 0000000..dbf811e
--- /dev/null
+++ b/drivers/dma/mediatek/8250_mtk_dma.c
@@ -0,0 +1,652 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * MediaTek 8250 DMA driver.
+ *
+ * Copyright (c) 2018 MediaTek Inc.
+ * Author: Long Cheng <long.cheng@mediatek.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_dma.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#include "../virt-dma.h"
+
+#define MTK_UART_APDMA_CHANNELS (CONFIG_SERIAL_8250_NR_UARTS * 2)
+
+#define VFF_EN_B BIT(0)
+#define VFF_STOP_B BIT(0)
+#define VFF_FLUSH_B BIT(0)
+#define VFF_4G_SUPPORT_B BIT(0)
+#define VFF_RX_INT_EN0_B BIT(0) /*rx valid size >= vff thre*/
+#define VFF_RX_INT_EN1_B BIT(1)
+#define VFF_TX_INT_EN_B BIT(0) /*tx left size >= vff thre*/
+#define VFF_WARM_RST_B BIT(0)
+#define VFF_RX_INT_FLAG_CLR_B (BIT(0) | BIT(1))
+#define VFF_TX_INT_FLAG_CLR_B 0
+#define VFF_STOP_CLR_B 0
+#define VFF_INT_EN_CLR_B 0
+#define VFF_4G_SUPPORT_CLR_B 0
+
+/* interrupt trigger level for tx */
+#define VFF_TX_THRE(n) ((n) * 7 / 8)
+/* interrupt trigger level for rx */
+#define VFF_RX_THRE(n) ((n) * 3 / 4)
+
+#define VFF_RING_SIZE 0xffffU
+/* invert this bit when wrap ring head again*/
+#define VFF_RING_WRAP 0x10000U
+
+#define VFF_INT_FLAG 0x00
+#define VFF_INT_EN 0x04
+#define VFF_EN 0x08
+#define VFF_RST 0x0c
+#define VFF_STOP 0x10
+#define VFF_FLUSH 0x14
+#define VFF_ADDR 0x1c
+#define VFF_LEN 0x24
+#define VFF_THRE 0x28
+#define VFF_WPT 0x2c
+#define VFF_RPT 0x30
+/*TX: the buffer size HW can read. RX: the buffer size SW can read.*/
+#define VFF_VALID_SIZE 0x3c
+/*TX: the buffer size SW can write. RX: the buffer size HW can write.*/
+#define VFF_LEFT_SIZE 0x40
+#define VFF_DEBUG_STATUS 0x50
+#define VFF_4G_SUPPORT 0x54
+
+struct mtk_uart_apdmadev {
+ struct dma_device ddev;
+ struct clk *clk;
+ bool support_33bits;
+ unsigned int dma_irq[MTK_UART_APDMA_CHANNELS];
+};
+
+struct mtk_uart_apdma_desc {
+ struct virt_dma_desc vd;
+
+ unsigned int avail_len;
+};
+
+struct mtk_chan {
+ struct virt_dma_chan vc;
+ struct dma_slave_config cfg;
+ void __iomem *base;
+ struct mtk_uart_apdma_desc *desc;
+
+ bool requested;
+
+ unsigned int rx_status;
+};
+
+static inline struct mtk_uart_apdmadev *
+to_mtk_uart_apdma_dev(struct dma_device *d)
+{
+ return container_of(d, struct mtk_uart_apdmadev, ddev);
+}
+
+static inline struct mtk_chan *to_mtk_uart_apdma_chan(struct dma_chan *c)
+{
+ return container_of(c, struct mtk_chan, vc.chan);
+}
+
+static inline struct mtk_uart_apdma_desc *to_mtk_uart_apdma_desc
+ (struct dma_async_tx_descriptor *t)
+{
+ return container_of(t, struct mtk_uart_apdma_desc, vd.tx);
+}
+
+static void mtk_uart_apdma_write(struct mtk_chan *c,
+ unsigned int reg, unsigned int val)
+{
+ writel(val, c->base + reg);
+}
+
+static unsigned int mtk_uart_apdma_read(struct mtk_chan *c, unsigned int reg)
+{
+ return readl(c->base + reg);
+}
+
+static void mtk_uart_apdma_desc_free(struct virt_dma_desc *vd)
+{
+ struct dma_chan *chan = vd->tx.chan;
+ struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
+
+ kfree(c->desc);
+}
+
+static void mtk_uart_apdma_start_tx(struct mtk_chan *c)
+{
+ unsigned int len, send, left, wpt, d_wpt, tmp;
+ int ret;
+
+ left = mtk_uart_apdma_read(c, VFF_LEFT_SIZE);
+ if (!left) {
+ mtk_uart_apdma_write(c, VFF_INT_EN, VFF_TX_INT_EN_B);
+ return;
+ }
+
+ /* Wait 1sec for flush, can't sleep*/
+ ret = readx_poll_timeout(readl, c->base + VFF_FLUSH, tmp,
+ tmp != VFF_FLUSH_B, 0, 1000000);
+ if (ret)
+ dev_warn(c->vc.chan.device->dev, "tx: fail, debug=0x%x\n",
+ mtk_uart_apdma_read(c, VFF_DEBUG_STATUS));
+
+ send = min_t(unsigned int, left, c->desc->avail_len);
+ wpt = mtk_uart_apdma_read(c, VFF_WPT);
+ len = mtk_uart_apdma_read(c, VFF_LEN);
+
+ d_wpt = wpt + send;
+ if ((d_wpt & VFF_RING_SIZE) >= len) {
+ d_wpt = d_wpt - len;
+ d_wpt = d_wpt ^ VFF_RING_WRAP;
+ }
+ mtk_uart_apdma_write(c, VFF_WPT, d_wpt);
+
+ c->desc->avail_len -= send;
+
+ mtk_uart_apdma_write(c, VFF_INT_EN, VFF_TX_INT_EN_B);
+ if (mtk_uart_apdma_read(c, VFF_FLUSH) == 0U)
+ mtk_uart_apdma_write(c, VFF_FLUSH, VFF_FLUSH_B);
+}
+
+static void mtk_uart_apdma_start_rx(struct mtk_chan *c)
+{
+ struct mtk_uart_apdma_desc *d = c->desc;
+ unsigned int len, wg, rg, cnt;
+
+ if ((mtk_uart_apdma_read(c, VFF_VALID_SIZE) == 0U) ||
+ !d || !vchan_next_desc(&c->vc))
+ return;
+
+ len = mtk_uart_apdma_read(c, VFF_LEN);
+ rg = mtk_uart_apdma_read(c, VFF_RPT);
+ wg = mtk_uart_apdma_read(c, VFF_WPT);
+ if ((rg ^ wg) & VFF_RING_WRAP)
+ cnt = (wg & VFF_RING_SIZE) + len - (rg & VFF_RING_SIZE);
+ else
+ cnt = (wg & VFF_RING_SIZE) - (rg & VFF_RING_SIZE);
+
+ c->rx_status = cnt;
+ mtk_uart_apdma_write(c, VFF_RPT, wg);
+
+ list_del(&d->vd.node);
+ vchan_cookie_complete(&d->vd);
+}
+
+static irqreturn_t mtk_uart_apdma_irq_handler(int irq, void *dev_id)
+{
+ struct dma_chan *chan = (struct dma_chan *)dev_id;
+ struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
+ struct mtk_uart_apdma_desc *d;
+ unsigned long flags;
+
+ spin_lock_irqsave(&c->vc.lock, flags);
+ if (c->cfg.direction == DMA_DEV_TO_MEM) {
+ mtk_uart_apdma_write(c, VFF_INT_FLAG, VFF_RX_INT_FLAG_CLR_B);
+ mtk_uart_apdma_start_rx(c);
+ } else if (c->cfg.direction == DMA_MEM_TO_DEV) {
+ d = c->desc;
+
+ mtk_uart_apdma_write(c, VFF_INT_FLAG, VFF_TX_INT_FLAG_CLR_B);
+
+ if (d->avail_len != 0U) {
+ mtk_uart_apdma_start_tx(c);
+ } else {
+ list_del(&d->vd.node);
+ vchan_cookie_complete(&d->vd);
+ }
+ }
+ spin_unlock_irqrestore(&c->vc.lock, flags);
+
+ return IRQ_HANDLED;
+}
+
+static int mtk_uart_apdma_alloc_chan_resources(struct dma_chan *chan)
+{
+ struct mtk_uart_apdmadev *mtkd = to_mtk_uart_apdma_dev(chan->device);
+ struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
+ u32 tmp;
+ int ret;
+
+ pm_runtime_get_sync(mtkd->ddev.dev);
+
+ mtk_uart_apdma_write(c, VFF_ADDR, 0);
+ mtk_uart_apdma_write(c, VFF_THRE, 0);
+ mtk_uart_apdma_write(c, VFF_LEN, 0);
+ mtk_uart_apdma_write(c, VFF_RST, VFF_WARM_RST_B);
+
+ ret = readx_poll_timeout(readl, c->base + VFF_EN, tmp,
+ tmp == 0, 10, 100);
+ if (ret) {
+ dev_err(chan->device->dev, "dma reset: fail, timeout\n");
+ return ret;
+ }
+
+ if (!c->requested) {
+ c->requested = true;
+ ret = request_irq(mtkd->dma_irq[chan->chan_id],
+ mtk_uart_apdma_irq_handler, IRQF_TRIGGER_NONE,
+ KBUILD_MODNAME, chan);
+ if (ret < 0) {
+ dev_err(chan->device->dev, "Can't request dma IRQ\n");
+ return -EINVAL;
+ }
+ }
+
+ if (mtkd->support_33bits)
+ mtk_uart_apdma_write(c, VFF_4G_SUPPORT, VFF_4G_SUPPORT_CLR_B);
+
+ return ret;
+}
+
+static void mtk_uart_apdma_free_chan_resources(struct dma_chan *chan)
+{
+ struct mtk_uart_apdmadev *mtkd = to_mtk_uart_apdma_dev(chan->device);
+ struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
+
+ if (c->requested) {
+ c->requested = false;
+ free_irq(mtkd->dma_irq[chan->chan_id], chan);
+ }
+
+ tasklet_kill(&c->vc.task);
+
+ vchan_free_chan_resources(&c->vc);
+
+ pm_runtime_put_sync(mtkd->ddev.dev);
+}
+
+static enum dma_status mtk_uart_apdma_tx_status(struct dma_chan *chan,
+ dma_cookie_t cookie,
+ struct dma_tx_state *txstate)
+{
+ struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
+ enum dma_status ret;
+ unsigned long flags;
+
+ if (!txstate)
+ return DMA_ERROR;
+
+ ret = dma_cookie_status(chan, cookie, txstate);
+ spin_lock_irqsave(&c->vc.lock, flags);
+ if (ret == DMA_IN_PROGRESS) {
+ c->rx_status = mtk_uart_apdma_read(c, VFF_RPT) & VFF_RING_SIZE;
+ dma_set_residue(txstate, c->rx_status);
+ } else if (ret == DMA_COMPLETE && c->cfg.direction == DMA_DEV_TO_MEM) {
+ dma_set_residue(txstate, c->rx_status);
+ } else {
+ dma_set_residue(txstate, 0);
+ }
+ spin_unlock_irqrestore(&c->vc.lock, flags);
+
+ return ret;
+}
+
+/*
+ * dmaengine_prep_slave_single will call the function. and sglen is 1.
+ * 8250 uart using one ring buffer, and deal with one sg.
+ */
+static struct dma_async_tx_descriptor *mtk_uart_apdma_prep_slave_sg
+ (struct dma_chan *chan, struct scatterlist *sgl,
+ unsigned int sglen, enum dma_transfer_direction dir,
+ unsigned long tx_flags, void *context)
+{
+ struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
+ struct mtk_uart_apdma_desc *d;
+
+ if ((dir != DMA_DEV_TO_MEM) &&
+ (dir != DMA_MEM_TO_DEV)) {
+ dev_err(chan->device->dev, "bad direction\n");
+ return NULL;
+ }
+
+ /* Now allocate and setup the descriptor */
+ d = kzalloc(sizeof(*d), GFP_ATOMIC);
+ if (!d)
+ return NULL;
+
+ /* sglen is 1 */
+ d->avail_len = sg_dma_len(sgl);
+
+ return vchan_tx_prep(&c->vc, &d->vd, tx_flags);
+}
+
+static void mtk_uart_apdma_issue_pending(struct dma_chan *chan)
+{
+ struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
+ struct virt_dma_desc *vd;
+ unsigned long flags;
+
+ spin_lock_irqsave(&c->vc.lock, flags);
+ if (c->cfg.direction == DMA_DEV_TO_MEM) {
+ if (vchan_issue_pending(&c->vc)) {
+ vd = vchan_next_desc(&c->vc);
+ c->desc = to_mtk_uart_apdma_desc(&vd->tx);
+ mtk_uart_apdma_start_rx(c);
+ }
+ } else if (c->cfg.direction == DMA_MEM_TO_DEV) {
+ if (vchan_issue_pending(&c->vc)) {
+ vd = vchan_next_desc(&c->vc);
+ c->desc = to_mtk_uart_apdma_desc(&vd->tx);
+ mtk_uart_apdma_start_tx(c);
+ }
+ }
+ spin_unlock_irqrestore(&c->vc.lock, flags);
+}
+
+static int mtk_uart_apdma_slave_config(struct dma_chan *chan,
+ struct dma_slave_config *cfg)
+{
+ struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
+ struct mtk_uart_apdmadev *mtkd =
+ to_mtk_uart_apdma_dev(c->vc.chan.device);
+
+ c->cfg = *cfg;
+
+ if (cfg->direction == DMA_DEV_TO_MEM) {
+ unsigned int rx_len = cfg->src_addr_width * 1024;
+
+ mtk_uart_apdma_write(c, VFF_ADDR, cfg->src_addr);
+ mtk_uart_apdma_write(c, VFF_LEN, rx_len);
+ mtk_uart_apdma_write(c, VFF_THRE, VFF_RX_THRE(rx_len));
+ mtk_uart_apdma_write(c, VFF_INT_EN,
+ VFF_RX_INT_EN0_B | VFF_RX_INT_EN1_B);
+ mtk_uart_apdma_write(c, VFF_RPT, 0);
+ mtk_uart_apdma_write(c, VFF_INT_FLAG, VFF_RX_INT_FLAG_CLR_B);
+ mtk_uart_apdma_write(c, VFF_EN, VFF_EN_B);
+ } else if (cfg->direction == DMA_MEM_TO_DEV) {
+ unsigned int tx_len = cfg->dst_addr_width * 1024;
+
+ mtk_uart_apdma_write(c, VFF_ADDR, cfg->dst_addr);
+ mtk_uart_apdma_write(c, VFF_LEN, tx_len);
+ mtk_uart_apdma_write(c, VFF_THRE, VFF_TX_THRE(tx_len));
+ mtk_uart_apdma_write(c, VFF_WPT, 0);
+ mtk_uart_apdma_write(c, VFF_INT_FLAG, VFF_TX_INT_FLAG_CLR_B);
+ mtk_uart_apdma_write(c, VFF_EN, VFF_EN_B);
+ }
+
+ if (mtkd->support_33bits)
+ mtk_uart_apdma_write(c, VFF_4G_SUPPORT, VFF_4G_SUPPORT_B);
+
+ if (mtk_uart_apdma_read(c, VFF_EN) != VFF_EN_B) {
+ dev_err(chan->device->dev, "dir[%d] fail\n", cfg->direction);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int mtk_uart_apdma_terminate_all(struct dma_chan *chan)
+{
+ struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
+ unsigned long flags;
+ u32 tmp;
+ int ret;
+
+ spin_lock_irqsave(&c->vc.lock, flags);
+
+ mtk_uart_apdma_write(c, VFF_FLUSH, VFF_FLUSH_B);
+ /* Wait 1sec for flush, can't sleep*/
+ ret = readx_poll_timeout(readl, c->base + VFF_FLUSH, tmp,
+ tmp != VFF_FLUSH_B, 0, 1000000);
+ if (ret)
+ dev_err(c->vc.chan.device->dev, "flush: fail, debug=0x%x\n",
+ mtk_uart_apdma_read(c, VFF_DEBUG_STATUS));
+
+ /*set stop as 1 -> wait until en is 0 -> set stop as 0*/
+ mtk_uart_apdma_write(c, VFF_STOP, VFF_STOP_B);
+ ret = readx_poll_timeout(readl, c->base + VFF_EN, tmp,
+ tmp == 0, 10, 100);
+ if (ret)
+ dev_err(c->vc.chan.device->dev, "stop: fail, debug=0x%x\n",
+ mtk_uart_apdma_read(c, VFF_DEBUG_STATUS));
+
+ mtk_uart_apdma_write(c, VFF_STOP, VFF_STOP_CLR_B);
+ mtk_uart_apdma_write(c, VFF_INT_EN, VFF_INT_EN_CLR_B);
+
+ if (c->cfg.direction == DMA_DEV_TO_MEM)
+ mtk_uart_apdma_write(c, VFF_INT_FLAG, VFF_RX_INT_FLAG_CLR_B);
+ else if (c->cfg.direction == DMA_MEM_TO_DEV)
+ mtk_uart_apdma_write(c, VFF_INT_FLAG, VFF_TX_INT_FLAG_CLR_B);
+
+ spin_unlock_irqrestore(&c->vc.lock, flags);
+
+ return 0;
+}
+
+static int mtk_uart_apdma_device_pause(struct dma_chan *chan)
+{
+ /* just for check caps pass */
+ return 0;
+}
+
+static int mtk_uart_apdma_device_resume(struct dma_chan *chan)
+{
+ /* just for check caps pass */
+ return 0;
+}
+
+static void mtk_uart_apdma_free(struct mtk_uart_apdmadev *mtkd)
+{
+ while (list_empty(&mtkd->ddev.channels) == 0) {
+ struct mtk_chan *c = list_first_entry(&mtkd->ddev.channels,
+ struct mtk_chan, vc.chan.device_node);
+
+ list_del(&c->vc.chan.device_node);
+ tasklet_kill(&c->vc.task);
+ }
+}
+
+static const struct of_device_id mtk_uart_apdma_match[] = {
+ { .compatible = "mediatek,mt6577-uart-dma", },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, mtk_uart_apdma_match);
+
+static int mtk_uart_apdma_probe(struct platform_device *pdev)
+{
+ struct mtk_uart_apdmadev *mtkd;
+ struct resource *res;
+ struct mtk_chan *c;
+ unsigned int i;
+ int rc;
+
+ mtkd = devm_kzalloc(&pdev->dev, sizeof(*mtkd), GFP_KERNEL);
+ if (!mtkd)
+ return -ENOMEM;
+
+ mtkd->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(mtkd->clk)) {
+ dev_err(&pdev->dev, "No clock specified\n");
+ rc = PTR_ERR(mtkd->clk);
+ return rc;
+ }
+
+ if (of_property_read_bool(pdev->dev.of_node, "dma-33bits"))
+ mtkd->support_33bits = true;
+
+ rc = dma_set_mask_and_coherent(&pdev->dev,
+ DMA_BIT_MASK(32 | mtkd->support_33bits));
+ if (rc)
+ return rc;
+
+ dma_cap_set(DMA_SLAVE, mtkd->ddev.cap_mask);
+ mtkd->ddev.device_alloc_chan_resources =
+ mtk_uart_apdma_alloc_chan_resources;
+ mtkd->ddev.device_free_chan_resources =
+ mtk_uart_apdma_free_chan_resources;
+ mtkd->ddev.device_tx_status = mtk_uart_apdma_tx_status;
+ mtkd->ddev.device_issue_pending = mtk_uart_apdma_issue_pending;
+ mtkd->ddev.device_prep_slave_sg = mtk_uart_apdma_prep_slave_sg;
+ mtkd->ddev.device_config = mtk_uart_apdma_slave_config;
+ mtkd->ddev.device_pause = mtk_uart_apdma_device_pause;
+ mtkd->ddev.device_resume = mtk_uart_apdma_device_resume;
+ mtkd->ddev.device_terminate_all = mtk_uart_apdma_terminate_all;
+ mtkd->ddev.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE);
+ mtkd->ddev.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE);
+ mtkd->ddev.directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
+ mtkd->ddev.residue_granularity = DMA_RESIDUE_GRANULARITY_SEGMENT;
+ mtkd->ddev.dev = &pdev->dev;
+ INIT_LIST_HEAD(&mtkd->ddev.channels);
+
+ for (i = 0; i < MTK_UART_APDMA_CHANNELS; i++) {
+ c = devm_kzalloc(mtkd->ddev.dev, sizeof(*c), GFP_KERNEL);
+ if (!c) {
+ rc = -ENODEV;
+ goto err_no_dma;
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, i);
+ if (!res) {
+ rc = -ENODEV;
+ goto err_no_dma;
+ }
+
+ c->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(c->base)) {
+ rc = PTR_ERR(c->base);
+ goto err_no_dma;
+ }
+ c->requested = false;
+ c->vc.desc_free = mtk_uart_apdma_desc_free;
+ vchan_init(&c->vc, &mtkd->ddev);
+
+ mtkd->dma_irq[i] = platform_get_irq(pdev, i);
+ if ((int)mtkd->dma_irq[i] < 0) {
+ dev_err(&pdev->dev, "failed to get IRQ[%d]\n", i);
+ rc = -EINVAL;
+ goto err_no_dma;
+ }
+ }
+
+ pm_runtime_enable(&pdev->dev);
+ pm_runtime_set_active(&pdev->dev);
+
+ rc = dma_async_device_register(&mtkd->ddev);
+ if (rc)
+ goto rpm_disable;
+
+ platform_set_drvdata(pdev, mtkd);
+
+ if (pdev->dev.of_node) {
+ /* Device-tree DMA controller registration */
+ rc = of_dma_controller_register(pdev->dev.of_node,
+ of_dma_xlate_by_chan_id,
+ mtkd);
+ if (rc)
+ goto dma_remove;
+ }
+
+ return rc;
+
+dma_remove:
+ dma_async_device_unregister(&mtkd->ddev);
+rpm_disable:
+ pm_runtime_disable(&pdev->dev);
+err_no_dma:
+ mtk_uart_apdma_free(mtkd);
+ return rc;
+}
+
+static int mtk_uart_apdma_remove(struct platform_device *pdev)
+{
+ struct mtk_uart_apdmadev *mtkd = platform_get_drvdata(pdev);
+
+ if (pdev->dev.of_node)
+ of_dma_controller_free(pdev->dev.of_node);
+
+ pm_runtime_disable(&pdev->dev);
+ pm_runtime_put_noidle(&pdev->dev);
+
+ dma_async_device_unregister(&mtkd->ddev);
+ mtk_uart_apdma_free(mtkd);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int mtk_uart_apdma_suspend(struct device *dev)
+{
+ struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
+
+ if (!pm_runtime_suspended(dev))
+ clk_disable_unprepare(mtkd->clk);
+
+ return 0;
+}
+
+static int mtk_uart_apdma_resume(struct device *dev)
+{
+ int ret;
+ struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
+
+ if (!pm_runtime_suspended(dev)) {
+ ret = clk_prepare_enable(mtkd->clk);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+#ifdef CONFIG_PM
+static int mtk_uart_apdma_runtime_suspend(struct device *dev)
+{
+ struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
+
+ clk_disable_unprepare(mtkd->clk);
+
+ return 0;
+}
+
+static int mtk_uart_apdma_runtime_resume(struct device *dev)
+{
+ int ret;
+ struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
+
+ ret = clk_prepare_enable(mtkd->clk);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+#endif /* CONFIG_PM */
+
+static const struct dev_pm_ops mtk_uart_apdma_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(mtk_uart_apdma_suspend, mtk_uart_apdma_resume)
+ SET_RUNTIME_PM_OPS(mtk_uart_apdma_runtime_suspend,
+ mtk_uart_apdma_runtime_resume, NULL)
+};
+
+static struct platform_driver mtk_uart_apdma_driver = {
+ .probe = mtk_uart_apdma_probe,
+ .remove = mtk_uart_apdma_remove,
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .pm = &mtk_uart_apdma_pm_ops,
+ .of_match_table = of_match_ptr(mtk_uart_apdma_match),
+ },
+};
+
+module_platform_driver(mtk_uart_apdma_driver);
+
+MODULE_DESCRIPTION("MediaTek UART APDMA Controller Driver");
+MODULE_AUTHOR("Long Cheng <long.cheng@mediatek.com>");
+MODULE_LICENSE("GPL v2");
+
diff --git a/drivers/dma/mediatek/Kconfig b/drivers/dma/mediatek/Kconfig
index 27bac0b..1a523c87 100644
--- a/drivers/dma/mediatek/Kconfig
+++ b/drivers/dma/mediatek/Kconfig
@@ -1,4 +1,15 @@
+config DMA_MTK_UART
+ tristate "MediaTek SoCs APDMA support for UART"
+ depends on OF && SERIAL_8250_MT6577
+ select DMA_ENGINE
+ select DMA_VIRTUAL_CHANNELS
+ help
+ Support for the UART DMA engine found on MediaTek MTK SoCs.
+ when SERIAL_8250_MT6577 is enabled, and if you want to use DMA,
+ you can enable the config. the DMA engine can only be used
+ with MediaTek SoCs.
+
config MTK_HSDMA
tristate "MediaTek High-Speed DMA controller support"
depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/dma/mediatek/Makefile b/drivers/dma/mediatek/Makefile
index 6e778f8..2f2efd9 100644
--- a/drivers/dma/mediatek/Makefile
+++ b/drivers/dma/mediatek/Makefile
@@ -1 +1,2 @@
+obj-$(CONFIG_DMA_MTK_UART) += 8250_mtk_dma.o
obj-$(CONFIG_MTK_HSDMA) += mtk-hsdma.o
^ permalink raw reply related
* [v9,2/2] arm: dts: mt2712: add uart APDMA to device tree
From: Long Cheng @ 2019-01-02 2:12 UTC (permalink / raw)
To: Vinod Koul, Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee,
Sean Wang, Nicolas Boichat
Cc: Matthias Brugger, Dan Williams, Greg Kroah-Hartman, Jiri Slaby,
Sean Wang, dmaengine, devicetree, linux-arm-kernel,
linux-mediatek, linux-kernel, linux-serial, srv_heupstream,
Yingjoe Chen, YT Shen, Zhenbao Liu, Long Cheng
1. add uart APDMA controller device node
2. add uart 0/1/2/3/4/5 DMA function
Signed-off-by: Long Cheng <long.cheng@mediatek.com>
---
arch/arm64/boot/dts/mediatek/mt2712e.dtsi | 50 +++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
index 976d92a..be1a22a 100644
--- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
@@ -300,6 +300,9 @@
interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_LOW>;
clocks = <&baud_clk>, <&sys_clk>;
clock-names = "baud", "bus";
+ dmas = <&apdma 10
+ &apdma 11>;
+ dma-names = "tx", "rx";
status = "disabled";
};
@@ -369,6 +372,38 @@
(GIC_CPU_MASK_RAW(0x13) | IRQ_TYPE_LEVEL_HIGH)>;
};
+ apdma: dma-controller@11000400 {
+ compatible = "mediatek,mt2712-uart-dma",
+ "mediatek,mt6577-uart-dma";
+ reg = <0 0x11000400 0 0x80>,
+ <0 0x11000480 0 0x80>,
+ <0 0x11000500 0 0x80>,
+ <0 0x11000580 0 0x80>,
+ <0 0x11000600 0 0x80>,
+ <0 0x11000680 0 0x80>,
+ <0 0x11000700 0 0x80>,
+ <0 0x11000780 0 0x80>,
+ <0 0x11000800 0 0x80>,
+ <0 0x11000880 0 0x80>,
+ <0 0x11000900 0 0x80>,
+ <0 0x11000980 0 0x80>;
+ interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 104 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 105 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 106 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 107 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 108 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 109 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 110 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 111 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 112 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 114 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&pericfg CLK_PERI_AP_DMA>;
+ clock-names = "apdma";
+ #dma-cells = <1>;
+ };
+
auxadc: adc@11001000 {
compatible = "mediatek,mt2712-auxadc";
reg = <0 0x11001000 0 0x1000>;
@@ -385,6 +420,9 @@
interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_LOW>;
clocks = <&baud_clk>, <&sys_clk>;
clock-names = "baud", "bus";
+ dmas = <&apdma 0
+ &apdma 1>;
+ dma-names = "tx", "rx";
status = "disabled";
};
@@ -395,6 +433,9 @@
interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_LOW>;
clocks = <&baud_clk>, <&sys_clk>;
clock-names = "baud", "bus";
+ dmas = <&apdma 2
+ &apdma 3>;
+ dma-names = "tx", "rx";
status = "disabled";
};
@@ -405,6 +446,9 @@
interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_LOW>;
clocks = <&baud_clk>, <&sys_clk>;
clock-names = "baud", "bus";
+ dmas = <&apdma 4
+ &apdma 5>;
+ dma-names = "tx", "rx";
status = "disabled";
};
@@ -415,6 +459,9 @@
interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_LOW>;
clocks = <&baud_clk>, <&sys_clk>;
clock-names = "baud", "bus";
+ dmas = <&apdma 6
+ &apdma 7>;
+ dma-names = "tx", "rx";
status = "disabled";
};
@@ -629,6 +676,9 @@
interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_LOW>;
clocks = <&baud_clk>, <&sys_clk>;
clock-names = "baud", "bus";
+ dmas = <&apdma 8
+ &apdma 9>;
+ dma-names = "tx", "rx";
status = "disabled";
};
^ permalink raw reply related
* [12/20] dmaengine: dw: drop useless LIST_HEAD
From: Vinod Koul @ 2019-01-02 6:03 UTC (permalink / raw)
To: Julia Lawall
Cc: Viresh Kumar, kernel-janitors, Andy Shevchenko, Dan Williams,
dmaengine, linux-kernel
On 23-12-18, 09:57, Julia Lawall wrote:
> Drop LIST_HEAD where the variable it declares is never used.
>
> Commit ab703f818ac3 ("dmaengine: dw: lazy allocation of dma
> descriptors") removed the uses, but not the declaration.
>
> The semantic patch that fixes this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> identifier x;
> @@
> - LIST_HEAD(x);
> ... when != x
> // </smpl>
Applied, thanks
^ permalink raw reply
* [14/20] dmaengine: st_fdma: drop useless LIST_HEAD
From: Vinod Koul @ 2019-01-02 6:03 UTC (permalink / raw)
To: Julia Lawall
Cc: Patrice Chotard, kernel-janitors, Dan Williams, linux-arm-kernel,
dmaengine, linux-kernel
On 23-12-18, 09:57, Julia Lawall wrote:
> Drop LIST_HEAD where the variable it declares is never used.
>
> The declarations were introduced with the file, but the declared
> variables were not used.
>
> The semantic patch that fixes this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> identifier x;
> @@
> - LIST_HEAD(x);
> ... when != x
> // </smpl>
>
> Fixes: 6b4cd727eaf1 ("dmaengine: st_fdma: Add STMicroelectronics FDMA engine driver support")
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Applied, thanks
^ permalink raw reply
* [17/20] dmaengine: pl330: drop useless LIST_HEAD
From: Vinod Koul @ 2019-01-02 6:04 UTC (permalink / raw)
To: Julia Lawall; +Cc: Dan Williams, kernel-janitors, dmaengine, linux-kernel
On 23-12-18, 09:57, Julia Lawall wrote:
> Drop LIST_HEAD where the variable it declares is never used.
>
> The variable has not been used since the function was introduced
> in 740aa95703c5 ("dmaengine: pl330: Split device_control").
>
> The semantic patch that fixes this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> identifier x;
> @@
> - LIST_HEAD(x);
> ... when != x
> // </smpl>
Applied, thanks
^ permalink raw reply
* [18/20] dmaengine: sa11x0: drop useless LIST_HEAD
From: Vinod Koul @ 2019-01-02 6:04 UTC (permalink / raw)
To: Julia Lawall; +Cc: Dan Williams, kernel-janitors, dmaengine, linux-kernel
On 23-12-18, 09:57, Julia Lawall wrote:
> Drop LIST_HEAD where the variable it declares has never been
> used.
>
> The semantic patch that fixes this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> identifier x;
> @@
> - LIST_HEAD(x);
> ... when != x
> // </smpl>
Applied, thanks
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox