RE: [v5 2/6] dmaengine: fsl-qdma: Add qDMA controller driver for Layerscape SoCs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----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????z谵{???塄}?财??j:+v??????2??璀??摺?囤??z夸z罐?+?????w棹f




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux