On Wed, 2020-07-15 at 11:49 +0530, Vinod Koul wrote:

On 02-07-20, 15:06, EastL Lee wrote:

quoted

 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->residue;
-	}
-
-	dma_set_residue(txstate, bytes);

any reason why you want to remove setting residue?

Because Mediatek CQDMA HW can't support residue.

quoted

-static void mtk_cqdma_free_active_desc(struct dma_chan *c)
+static int mtk_cqdma_terminate_all(struct dma_chan *c)
 {
 	struct mtk_cqdma_vchan *cvc = to_cqdma_vchan(c);
-	bool sync_needed = false;
+	struct virt_dma_chan *vc = to_virt_chan(c);
 	unsigned long pc_flags;
 	unsigned long vc_flags;
+	LIST_HEAD(head);
+
+	/* wait for the VC to be inactive  */
+	if (!wait_for_completion_timeout(&cvc->cmp, msecs_to_jiffies(3000)))
+		return -EAGAIN;
 
 	/* 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;
-	}
+	/* get VDs from lists */
+	vchan_get_all_descriptors(vc, &head);
+
+	/* free all the VDs */
+	vchan_dma_desc_free_list(vc, &head);
 
 	spin_unlock_irqrestore(&cvc->vc.lock, vc_flags);
 	spin_unlock_irqrestore(&cvc->pc->lock, pc_flags);

Good cleanup, do you need both these locks?

Yes, because we will have multiple vc to pc, it will need both lock.