> -----Original Message----- > From: Vinod Koul [mailto:vinod.koul@xxxxxxxxxx] > Sent: 2018年5月30日 18:28 > To: Wen He <wen.he_1@xxxxxxx> > Cc: dmaengine@xxxxxxxxxxxxxxx; robh+dt@xxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; Leo Li <leoyang.li@xxxxxxx>; Jiafei Pan > <jiafei.pan@xxxxxxx>; Jiaheng Fan <jiaheng.fan@xxxxxxx> > Subject: Re: [v5 2/6] dmaengine: fsl-qdma: Add qDMA controller driver for > Layerscape SoCs > > 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.. > Got it. Thanks > > > > > > +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 > I tried that if I use only vchan lock then qdma will be can't work. Do you have a other good idea? Best Regards, Wen > -- > ~Vinod ?韬{.n?????%??檩??w?{.n???Ё?撖)?骅w* jg????????G??⒏ ⒎?:+v???????? ????＂??????
