dmaengine: fsl-qdma: Add qDMA controller driver for Layerscape SoCs
PATCH v5 2/6 dmaengine: fsl-qdma: Add qDMA controller driver for Layerscape SoCs
Quoted 10 lines · Wen He
Quoted 10 lines
Quoted 2 lines
Quoted 2 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ /* + * Clear the command queue interrupt detect register for allqueues.
Quoted 7 lines
Quoted 6 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ */ + 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
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
Maybe I missed that I should defined the value to the macro and add
comment.
Quoted 1 line
Right?
that would help, but why are you writing
0xffffffffat all these places?This value is from the datasheet.
Should I write comments to here?
Yes that would help explaining what this does..
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
Quoted 6 lines
Quoted 6 lines
(quoted diff) +6
-0+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);Quoted 9 lines
Review
Quoted 5 lines
(quoted diff) +5
-0+ 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
Quoted 7 lines
Quoted 5 lines
support.
Quoted 1 line
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