On 29-05-18, 10:38, Wen He wrote: > > > > > + /* > > > > > + * Clear the command queue interrupt detect register for all > > queues. > > > > > + */ > > > > > + qdma_writel(fsl_qdma, 0xffffffff, block + FSL_QDMA_BCQIDR(0)); > > > > > > > > bunch of writes with 0xffffffff, can you explain why? Also helps to > > > > make a macro for this > > > > > > > > > > Maybe I missed that I should defined the value to the macro and add > > comment. > > > Right? > > > > that would help, but why are you writing 0xffffffff at all these places? > > This value is from the datasheet. > Should I write comments to here? Yes that would help explaining what this does.. > > > > > +static void fsl_qdma_issue_pending(struct dma_chan *chan) { > > > > > + struct fsl_qdma_chan *fsl_chan = to_fsl_qdma_chan(chan); > > > > > + struct fsl_qdma_queue *fsl_queue = fsl_chan->queue; > > > > > + unsigned long flags; > > > > > + > > > > > + spin_lock_irqsave(&fsl_queue->queue_lock, flags); > > > > > + spin_lock(&fsl_chan->vchan.lock); > > > > > + if (vchan_issue_pending(&fsl_chan->vchan)) > > > > > + fsl_qdma_enqueue_desc(fsl_chan); > > > > > + spin_unlock(&fsl_chan->vchan.lock); > > > > > + spin_unlock_irqrestore(&fsl_queue->queue_lock, flags); > > > > > > > > why do we need two locks, and since you are doing vchan why should > > > > you add your own lock on top > > > > > > > > > > Yes, we need two locks. > > > As you know, the QDMA support multiple virtualized blocks for multi-core > > support. > > > so we need to make sure that muliti-core access issues. > > > > but why cant you use vchan lock for all? > > > > We can't only use vchan lock for all. otherwise enqueue action will be interrupted. I think it is possible to use only vchan lock -- ~Vinod -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html