From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [v5,2/6] dmaengine: fsl-qdma: Add qDMA controller driver for Layerscape SoCs From: Vinod Koul Message-Id: <20180530102744.GE16230@vkoul-mobl> Date: Wed, 30 May 2018 15:57:44 +0530 To: Wen He Cc: "dmaengine@vger.kernel.org" , "robh+dt@kernel.org" , "devicetree@vger.kernel.org" , Leo Li , Jiafei Pan , Jiaheng Fan List-ID: T24gMjktMDUtMTgsIDEwOjM4LCBXZW4gSGUgd3JvdGU6Cgo+ID4gPiA+ID4gKwkvKgo+ID4gPiA+ ID4gKwkgKiBDbGVhciB0aGUgY29tbWFuZCBxdWV1ZSBpbnRlcnJ1cHQgZGV0ZWN0IHJlZ2lzdGVy IGZvciBhbGwKPiA+IHF1ZXVlcy4KPiA+ID4gPiA+ICsJICovCj4gPiA+ID4gPiArCXFkbWFfd3Jp dGVsKGZzbF9xZG1hLCAweGZmZmZmZmZmLCBibG9jayArIEZTTF9RRE1BX0JDUUlEUigwKSk7Cj4g PiA+ID4KPiA+ID4gPiBidW5jaCBvZiB3cml0ZXMgd2l0aCAweGZmZmZmZmZmLCBjYW4geW91IGV4 cGxhaW4gd2h5PyBBbHNvIGhlbHBzIHRvCj4gPiA+ID4gbWFrZSBhIG1hY3JvIGZvciB0aGlzCj4g PiA+ID4KPiA+ID4KPiA+ID4gTWF5YmUgSSBtaXNzZWQgdGhhdCBJIHNob3VsZCBkZWZpbmVkIHRo ZSB2YWx1ZSB0byB0aGUgbWFjcm8gYW5kIGFkZAo+ID4gY29tbWVudC4KPiA+ID4gUmlnaHQ/Cj4g PiAKPiA+IHRoYXQgd291bGQgaGVscCwgYnV0IHdoeSBhcmUgeW91IHdyaXRpbmcgMHhmZmZmZmZm ZiBhdCBhbGwgdGhlc2UgcGxhY2VzPwo+IAo+IFRoaXMgdmFsdWUgaXMgZnJvbSB0aGUgZGF0YXNo ZWV0Lgo+IFNob3VsZCBJIHdyaXRlIGNvbW1lbnRzIHRvIGhlcmU/CgpZZXMgdGhhdCB3b3VsZCBo ZWxwIGV4cGxhaW5pbmcgd2hhdCB0aGlzIGRvZXMuLgoKPiA+ID4gPiA+ICtzdGF0aWMgdm9pZCBm c2xfcWRtYV9pc3N1ZV9wZW5kaW5nKHN0cnVjdCBkbWFfY2hhbiAqY2hhbikgewo+ID4gPiA+ID4g KwlzdHJ1Y3QgZnNsX3FkbWFfY2hhbiAqZnNsX2NoYW4gPSB0b19mc2xfcWRtYV9jaGFuKGNoYW4p Owo+ID4gPiA+ID4gKwlzdHJ1Y3QgZnNsX3FkbWFfcXVldWUgKmZzbF9xdWV1ZSA9IGZzbF9jaGFu LT5xdWV1ZTsKPiA+ID4gPiA+ICsJdW5zaWduZWQgbG9uZyBmbGFnczsKPiA+ID4gPiA+ICsKPiA+ ID4gPiA+ICsJc3Bpbl9sb2NrX2lycXNhdmUoJmZzbF9xdWV1ZS0+cXVldWVfbG9jaywgZmxhZ3Mp Owo+ID4gPiA+ID4gKwlzcGluX2xvY2soJmZzbF9jaGFuLT52Y2hhbi5sb2NrKTsKPiA+ID4gPiA+ ICsJaWYgKHZjaGFuX2lzc3VlX3BlbmRpbmcoJmZzbF9jaGFuLT52Y2hhbikpCj4gPiA+ID4gPiAr CQlmc2xfcWRtYV9lbnF1ZXVlX2Rlc2MoZnNsX2NoYW4pOwo+ID4gPiA+ID4gKwlzcGluX3VubG9j aygmZnNsX2NoYW4tPnZjaGFuLmxvY2spOwo+ID4gPiA+ID4gKwlzcGluX3VubG9ja19pcnFyZXN0 b3JlKCZmc2xfcXVldWUtPnF1ZXVlX2xvY2ssIGZsYWdzKTsKPiA+ID4gPgo+ID4gPiA+IHdoeSBk byB3ZSBuZWVkIHR3byBsb2NrcywgYW5kIHNpbmNlIHlvdSBhcmUgZG9pbmcgdmNoYW4gd2h5IHNo b3VsZAo+ID4gPiA+IHlvdSBhZGQgeW91ciBvd24gbG9jayBvbiB0b3AKPiA+ID4gPgo+ID4gPgo+ ID4gPiBZZXMsIHdlIG5lZWQgdHdvIGxvY2tzLgo+ID4gPiBBcyB5b3Uga25vdywgdGhlIFFETUEg c3VwcG9ydCBtdWx0aXBsZSB2aXJ0dWFsaXplZCBibG9ja3MgZm9yIG11bHRpLWNvcmUKPiA+IHN1 cHBvcnQuCj4gPiA+IHNvIHdlIG5lZWQgdG8gbWFrZSBzdXJlIHRoYXQgbXVsaXRpLWNvcmUgYWNj ZXNzIGlzc3Vlcy4KPiA+IAo+ID4gYnV0IHdoeSBjYW50IHlvdSB1c2UgdmNoYW4gbG9jayBmb3Ig YWxsPwo+ID4gCj4gCj4gV2UgY2FuJ3Qgb25seSB1c2UgdmNoYW4gbG9jayBmb3IgYWxsLiBvdGhl cndpc2UgZW5xdWV1ZSBhY3Rpb24gd2lsbCBiZSBpbnRlcnJ1cHRlZC4KCkkgdGhpbmsgaXQgaXMg cG9zc2libGUgdG8gdXNlIG9ubHkgdmNoYW4gbG9jawo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Wed, 30 May 2018 15:57:44 +0530 From: Vinod Koul Subject: Re: [v5 2/6] dmaengine: fsl-qdma: Add qDMA controller driver for Layerscape SoCs Message-ID: <20180530102744.GE16230@vkoul-mobl> References: <20180525111920.24498-1-wen.he_1@nxp.com> <20180525111920.24498-2-wen.he_1@nxp.com> <20180529070724.GE5666@vkoul-mobl> <20180529101954.GJ5666@vkoul-mobl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: To: Wen He Cc: "dmaengine@vger.kernel.org" , "robh+dt@kernel.org" , "devicetree@vger.kernel.org" , Leo Li , Jiafei Pan , Jiaheng Fan List-ID: 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