public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: vinod.koul@intel•com (Vinod Koul)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v2 2/2] dmaengine: Add hisilicon k3 DMA engine driver
Date: Mon, 19 Aug 2013 11:05:11 +0530	[thread overview]
Message-ID: <20130819053511.GU32147@intel.com> (raw)
In-Reply-To: <CAMj5Bki0H+=0wQY39+jDw4nAEq5wEq3V3yri9xpNRxQ2GRwPbA@mail.gmail.com>

On Thu, Aug 15, 2013 at 01:54:28PM +0800, zhangfei gao wrote:
> On Tue, Aug 13, 2013 at 7:20 PM, Vinod Koul <vinod.koul@intel•com> wrote:
> > On Fri, Jun 28, 2013 at 08:39:13PM +0800, Zhangfei Gao wrote:
> >> Add dmaengine driver for hisilicon k3 platform based on virt_dma
> >>
> >> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro•org>
> >> Tested-by: Kai Yang <jean.yangkai@huawei•com>
> >> Acked-by: Arnd Bergmann <arnd@arndb•de>
> >> ---
> >
> >> +#define DRIVER_NAME          "k3-dma"
> >> +#define DMA_ALIGN            3
> >> +#define DMA_MAX_SIZE         0x1ffc
> >> +
> >> +#define INT_STAT             0x00
> >> +#define INT_TC1                      0x04
> >> +#define INT_ERR1             0x0c
> >> +#define INT_ERR2             0x10
> >> +#define INT_TC1_MASK         0x18
> >> +#define INT_ERR1_MASK                0x20
> >> +#define INT_ERR2_MASK                0x24
> >> +#define INT_TC1_RAW          0x600
> >> +#define INT_ERR1_RAW         0x608
> >> +#define INT_ERR2_RAW         0x610
> >> +#define CH_PRI                       0x688
> >> +#define CH_STAT                      0x690
> >> +#define CX_CUR_CNT           0x704
> >> +#define CX_LLI                       0x800
> >> +#define CX_CNT                       0x810
> >> +#define CX_SRC                       0x814
> >> +#define CX_DST                       0x818
> >> +#define CX_CONFIG            0x81c
> >> +#define AXI_CONFIG           0x820
> >> +#define DEF_AXI_CONFIG               0x201201
> >> +
> >> +#define CX_LLI_CHAIN_EN              0x2
> >> +#define CCFG_EN                      0x1
> >> +#define CCFG_MEM2PER         (0x1 << 2)
> >> +#define CCFG_PER2MEM         (0x2 << 2)
> >> +#define CCFG_SRCINCR         (0x1 << 31)
> >> +#define CCFG_DSTINCR         (0x1 << 30)
> > I see these are not namespace aptly and can collide..
> OK,
> How about change name to
> #define CX_CFG                 0x81c
since you are calling your driver K3_DMA it would be apt to namespace all of the
above with K3_INT_STAT to K3_CFG_EN and so on...

> 
> #define CX_CFG_EN              0x1
> #define CX_CFG_MEM2PER         (0x1 << 2)
> ~
> 
> >> +             switch (width) {
> >> +             case DMA_SLAVE_BUSWIDTH_1_BYTE:
> >> +                     val = 0;
> >> +                     break;
> >> +             case DMA_SLAVE_BUSWIDTH_2_BYTES:
> >> +                     val = 1;
> >> +                     break;
> >> +             case DMA_SLAVE_BUSWIDTH_4_BYTES:
> >> +                     val = 2;
> >> +                     break;
> >> +             case DMA_SLAVE_BUSWIDTH_8_BYTES:
> >> +                     val = 3;
> > DMA_SLAVE_BUSWIDTHS are 1, 2, 4, 8...
> > so if you can do val = ffs(width) as well?
> 
> Good idea, will use __ffs(width) here.
> 
> >
> >
> >> +     case DMA_PAUSE:
> >> +             dev_dbg(d->slave.dev, "vchan %p: pause\n", &c->vc);
> >> +             if (c->status == DMA_IN_PROGRESS) {
> >> +                     c->status = DMA_PAUSED;
> >> +                     if (p) {
> >> +                             k3_dma_pause_dma(p, false);
> >> +                     } else {
> >> +                             spin_lock(&d->lock);
> >> +                             list_del_init(&c->node);
> >> +                             spin_unlock(&d->lock);
> >> +                     }
> > why do we need the else part here?
> Since asynchronous mode is supported.
> Desc is submitted to list but may not get physical channel to run.
But when you pause you pause the running channel. You dont pause a descriptor.
So whatever you are trying to imply doesnt make sense to me.

~Vinod

  reply	other threads:[~2013-08-19  5:35 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-28 12:39 [PATCH v2 0/2] dmaengine: add k3dma Zhangfei Gao
2013-06-28 12:39 ` [PATCH v2 1/2] dmaengine: add interface of dma_get_slave_channel Zhangfei Gao
2013-06-28 14:32   ` Arnd Bergmann
2013-08-13 11:04   ` Vinod Koul
2013-06-28 12:39 ` [PATCH v2 2/2] dmaengine: Add hisilicon k3 DMA engine driver Zhangfei Gao
2013-08-13 11:20   ` Vinod Koul
2013-08-15  5:54     ` zhangfei gao
2013-08-19  5:35       ` Vinod Koul [this message]
2013-08-20  7:55         ` zhangfei gao
2013-08-20  8:27           ` Vinod Koul
2013-08-20  9:23             ` zhangfei
2013-08-20  8:50               ` Vinod Koul
2013-08-20 13:36                 ` zhangfei gao
2013-08-21  4:58                   ` Vinod Koul
2013-08-21  8:02                     ` zhangfei gao
2013-08-25  9:14                       ` Vinod Koul
2013-08-22  1:39                     ` zhangfei gao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130819053511.GU32147@intel.com \
    --to=vinod.koul@intel$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox