Re: [PATCH 2/2] dmaengine: mediatek-cqdma: remove redundant queue structure
On Thu, Jan 24, 2019 at 2:46 AM wrote:
>
> From: Shun-Chih Yu
>
> This patch introduces active_vdec to indicate the virtual descriptor
> under processing by the CQDMA dmaengine, and simplify the control logic
> by removing redundant queue structure, tasklets, and completion
> management.
>
> Also, wrong residue assignment in mtk_cqdma_tx_status and typos are
> fixed.
overall changes are good and let code become clear, but it's better to
do one thing at a patch instead of mixing all stuff together.
>
> Signed-off-by: Shun-Chih Yu
> ---
> drivers/dma/mediatek/mtk-cqdma.c | 399
> ++
> 1 file changed, 98 insertions(+), 301 deletions(-)
>
> diff --git a/drivers/dma/mediatek/mtk-cqdma.c
> b/drivers/dma/mediatek/mtk-cqdma.c
> index 131f397..387781b 100644
> --- a/drivers/dma/mediatek/mtk-cqdma.c
> +++ b/drivers/dma/mediatek/mtk-cqdma.c
> @@ -22,6 +22,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -47,7 +48,6 @@
> #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
>
> @@ -69,45 +69,32 @@
> * descriptor (CVD)
> * @vd:An instance for struct virt_dma_desc
> * @len: The total data size device wants to move
> - * @residue: The remaining data size device will 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: The lise_head struct to build link-list for VDs
> - * @parent:The pointer to the parent CVD
> */
> struct mtk_cqdma_vdesc {
> struct virt_dma_desc vd;
> size_t len;
> - size_t residue;
> dma_addr_t dest;
> dma_addr_t src;
> struct dma_chan *ch;
> -
> - struct list_head node;
> - struct mtk_cqdma_vdesc *parent;
> };
>
> /**
> * struct mtk_cqdma_pchan - The struct holding info describing physical
> * channel (PC)
> - * @queue: Queue for the PDs issued to this PC
> + * @active_vdesc: The pointer to the CVD which is under processing
> * @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
> - * @tasklet: Tasklet for this PC
> * @lock: Lock protect agaisting multiple VCs access PC
> */
> struct mtk_cqdma_pchan {
> - struct list_head queue;
> + struct mtk_cqdma_vdesc *active_vdesc;
> void __iomem *base;
> u32 irq;
> -
> refcount_t refcnt;
> -
> - struct tasklet_struct tasklet;
> -
> - /* lock to protect PC */
> spinlock_t lock;
> };
>
> @@ -116,14 +103,10 @@ struct mtk_cqdma_pchan {
> * 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;
> };
>
> /**
> @@ -168,7 +151,7 @@ static struct device *cqdma2dev(struct mtk_cqdma_device
> *cqdma)
>
> static u32 mtk_dma_read(struct mtk_cqdma_pchan *pc, u32 reg)
> {
> - return readl(pc->base + reg);
> + return readl_relaxed(pc->base + reg);
that can be moved to separate patch and explains why
> }
>
> static void mtk_dma_write(struct mtk_cqdma_pchan *pc, u32 reg, u32 val)
> @@ -202,22 +185,22 @@ 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)
> +static int mtk_cqdma_poll_engine_done(struct mtk_cqdma_pchan *pc)
> {
> u32 status = 0;
>
> - if (!atomic)
> + if (!in_task())
if (in_task())
> 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 &
[PATCH 2/2] dmaengine: mediatek-cqdma: remove redundant queue structure
From: Shun-Chih Yu
This patch introduces active_vdec to indicate the virtual descriptor
under processing by the CQDMA dmaengine, and simplify the control logic
by removing redundant queue structure, tasklets, and completion
management.
Also, wrong residue assignment in mtk_cqdma_tx_status and typos are
fixed.
Signed-off-by: Shun-Chih Yu
---
drivers/dma/mediatek/mtk-cqdma.c | 399 ++
1 file changed, 98 insertions(+), 301 deletions(-)
diff --git a/drivers/dma/mediatek/mtk-cqdma.c b/drivers/dma/mediatek/mtk-cqdma.c
index 131f397..387781b 100644
--- a/drivers/dma/mediatek/mtk-cqdma.c
+++ b/drivers/dma/mediatek/mtk-cqdma.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include
#include
@@ -47,7 +48,6 @@
#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
@@ -69,45 +69,32 @@
* descriptor (CVD)
* @vd:An instance for struct virt_dma_desc
* @len: The total data size device wants to move
- * @residue: The remaining data size device will 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: The lise_head struct to build link-list for VDs
- * @parent:The pointer to the parent CVD
*/
struct mtk_cqdma_vdesc {
struct virt_dma_desc vd;
size_t len;
- size_t residue;
dma_addr_t dest;
dma_addr_t src;
struct dma_chan *ch;
-
- struct list_head node;
- struct mtk_cqdma_vdesc *parent;
};
/**
* struct mtk_cqdma_pchan - The struct holding info describing physical
* channel (PC)
- * @queue: Queue for the PDs issued to this PC
+ * @active_vdesc: The pointer to the CVD which is under processing
* @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
- * @tasklet: Tasklet for this PC
* @lock: Lock protect agaisting multiple VCs access PC
*/
struct mtk_cqdma_pchan {
- struct list_head queue;
+ struct mtk_cqdma_vdesc *active_vdesc;
void __iomem *base;
u32 irq;
-
refcount_t refcnt;
-
- struct tasklet_struct tasklet;
-
- /* lock to protect PC */
spinlock_t lock;
};
@@ -116,14 +103,10 @@ struct mtk_cqdma_pchan {
* 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;
};
/**
@@ -168,7 +151,7 @@ static struct device *cqdma2dev(struct mtk_cqdma_device
*cqdma)
static u32 mtk_dma_read(struct mtk_cqdma_pchan *pc, u32 reg)
{
- return readl(pc->base + reg);
+ return readl_relaxed(pc->base + reg);
}
static void mtk_dma_write(struct mtk_cqdma_pchan *pc, u32 reg, u32 val)
@@ -202,22 +185,22 @@ 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)
+static int mtk_cqdma_poll_engine_done(struct mtk_cqdma_pchan *pc)
{
u32 status = 0;
- if (!atomic)
+ if (!in_task())
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);
+ else
+ return readl_poll_timeout_atomic(pc->base + MTK_CQDMA_EN,
+status,
+!(status & MTK_CQDMA_EN_BIT),
+MTK_CQDMA_USEC_POLL,
+
