From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jonathan Bell Subject: Re: [PATCH v2] dmaengine: bcm2835: Add slave dma support Date: Fri, 8 May 2015 12:20:00 +0100 Message-ID: <554C9BE0.60205@raspberrypi.org> References: <1429355160-13657-1-git-send-email-noralf@tronnes.org> <554A69D5.90301@tronnes.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Cc: vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org, Linux Kernel Mailing List , linux-spi , linux-rpi-kernel , dmaengine-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org To: =?UTF-8?B?Tm9yYWxmIFRyw7hubmVz?= , Martin Sperl , Stephen Warren Return-path: In-Reply-To: <554A69D5.90301-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-rpi-kernel" Errors-To: linux-rpi-kernel-bounces+glkr-linux-rpi-kernel=m.gmane.org-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org List-Id: linux-spi.vger.kernel.org T24gMDYvMDUvMjAxNSAyMDoyMSwgTm9yYWxmIFRyw7hubmVzIHdyb3RlOgo+Cj4gRGVuIDAxLjA1 LjIwMTUgMjM6MDcsIHNrcmV2IE1hcnRpbiBTcGVybDoKPj4gVGVzdHMgd2l0aCB0aGUgaW5pdGlh bCAoYW5kIGluY29tcGxldGUpIHZlcnNpb24gb2YgdGhlIHNwaS1iY20yODM1IAo+PiBkcml2ZXIK Pj4gd2l0aCBETUEgdHJhbnNmZXIgc3VwcG9ydCBzaG93IHRoYXQgdGhlIGRtYS1lbmdpbmUgd29y a3MgYXMgZXhwZWN0ZWQgCj4+IHdpdGgKPj4gdGhpcyBwYXRjaC4KPj4KPj4gVGhlcmUgaXMgb25l IG9uZSBvYnNlcnZhdGlvbjoKPj4KPj4+IE9uIDE4LjA0LjIwMTUsIGF0IDEzOjA2LCBOb3JhbGYg VHLDuG5uZXMgPG5vcmFsZkB0cm9ubmVzLm9yZz4gd3JvdGU6Cj4+PiArc3RhdGljIHN0cnVjdCBk bWFfYXN5bmNfdHhfZGVzY3JpcHRvciAqCj4+PiArYmNtMjgzNV9kbWFfcHJlcF9zbGF2ZV9zZyhz dHJ1Y3QgZG1hX2NoYW4gKmNoYW4sCj4+PiArICAgICAgICAgICAgICBzdHJ1Y3Qgc2NhdHRlcmxp c3QgKnNnbCwKPj4+ICsgICAgICAgICAgICAgIHVuc2lnbmVkIGludCBzZ19sZW4sCj4+PiArICAg ICAgICAgICAgICBlbnVtIGRtYV90cmFuc2Zlcl9kaXJlY3Rpb24gZGlyZWN0aW9uLAo+Pj4gKyAg ICAgICAgICAgICAgdW5zaWduZWQgbG9uZyBmbGFncywgdm9pZCAqY29udGV4dCkKPj4+ICt7Cj4+ IC4uLgo+Pj4gKyAgICAgICAgICAgIC8qIEVuYWJsZSAqLwo+Pj4gKyAgICAgICAgICAgIGlmIChp ID09IHNnX2xlbiAtIDEgJiYgbGVuIC0gaiA8PSBtYXhfc2l6ZSkKPj4+ICsgICAgICAgICAgICAg ICAgY29udHJvbF9ibG9jay0+aW5mbyB8PSBCQ00yODM1X0RNQV9JTlRfRU47Cj4+IFRoZSBvYnNl cnZhdGlvbiBpcyB0aGF0IGFuIGludGVycnVwdCBpcyBhbHdheXMgdHJpZ2dlcmVkIC0gZXZlbiBp biAKPj4gdGhlIGNhc2UKPj4gd2hlcmUgZmxhZ3MgZG9lcyBOT1QgaGF2ZSBETUFfUFJFUF9JTlRF UlJVUFQgc2V0Lgo+PiBUaGlzIG1heSBub3QgYmUgbmVjZXNzYXJ5IGFuZCBhdm9pZCBpbnRlcnJ1 cHRzLgo+Pgo+PiBTbyBtYXliZSB0aGUgYWJvdmUgaWYgY2xhdXNlIHNob3VsZCBnZXQgZXh0ZW5k ZWQgYnk6Cj4+ICAgICAmJiAoZmxhZ3MgJiBETUFfUFJFUF9JTlRFUlJVUFQpCj4+IHRvIG9ubHkg dHJpZ2dlciBhbiBpbnRlcnJ1cHQgd2hlbiByZWFsbHkgcmVxdWVzdGVkLgo+Pgo+PiBJIGFtIG5v dCBzdXJlIGlmIHRoZXJlIGFyZSBhbnkgc2lkZS1lZmZlY3RzIGJlY2F1c2Ugb2YgdGhpcyBiZXNp ZGVzIAo+PiBoYXZpbmcgdGhlCj4+IHJlcXVpcmVtZW50IG9uIHRoZSBjbGllbnQgdG8gcnVuIGRt YWVuZ2luZV90ZXJtaW5hdGVfYWxsKCkgb24gdGhhdCAKPj4gc3BlY2lmaWMgZG1hCj4+IGNoYW5u ZWwgd2l0aG91dCBpbnRlcnJ1cHRzIHdoZW4gdGhlIHRyYW5zZmVyIGlzIGZpbmlzaGVkLgo+Pgo+ PiBJbiB0aGUgY2FzZSBvZiBTUEkgd2UgaGF2ZSBUWCBmZWVkIHRoZSBmaWZvIC0gd2hpY2ggZmlu aXNoZXMgZWFybHkgLSAKPj4gLCBidXQgd2UKPj4gb25seSBuZWVkIHRvIHRoZSBpbnRlcnJ1cHQg d2hlbiBSWCBmaW5pc2hlcyByZWFkaW5nIHRoZSBmaWZvLCB3aGljaCAKPj4gaW5kaWNhdGVzCj4+ IHRoYXQgdGhlIFNQSS10cmFuc2ZlciBpcyBmdWxseSBmaW5pc2hlZC4KPj4gU28gaGF2aW5nIGFu IGludGVycnVwdCBvbiBUWCBpcyBub3QgbmVjZXNzYXJ5IGZvciB0aGUgcHJvY2Vzcy4KPj4KPj4g VGhlIHNhbWUgb2JzZXJ2YXRpb25zIG1heSBhbHNvIGFwcGx5IHRvIGJjbTI4MzVfZG1hX3ByZXBf ZG1hX2N5Y2xpYyAKPj4gKHdoaWNoIGlzCj4+IG91dHNpZGUgb2YgdGhpcyBwYXRjaCBwcm92aWRl ZCBieSBOb3JhbGYpLgo+Cj4gSm9uYXRoYW4sIGNhbiB5b3UgY29tbWVudCBvbiB0aGlzPwo+Cj4K PiBOb3JhbGYuCj4KPgpJIGFncmVlIHRoYXQgdGhlIGludGVycnVwdCBnZW5lcmF0ZWQgd291bGQg YmUgc3B1cmlvdXMgLSBpbiB0aGUgY2FzZSAKd2hlcmUgaXQgaXMgbm90IHJlcXVpcmVkLgoKSG93 ZXZlciBpZiB5b3UgZG8gJiYgKGZsYWdzICYgRE1BX1BSRVBfSU5URVJSVVBUKSB0aGVuIGFsbCB1 c2VycyBvZiB0aGlzIApkcml2ZXIgbmVlZCB0byBleHBsaWNpdGx5IHNldCBpbnRlcnJ1cHQgZmxh Z3Mgd2hlbiBkb2luZyBhIApzY2F0dGVyLWdhdGhlciB0cmFuc2Zlci4gQXMgSSB1bmRlcnN0YW5k IGl0LCBjdXJyZW50bHkgdGhlIG9ubHkgdXBzdHJlYW0gCmNsaWVudCBvZiB0aGlzIGRyaXZlciBp cyB0aGUgSTJTIGRyaXZlciB3aGljaCBvbmx5IHVzZXMgY3ljbGljIGFueXdheS4KCk5vdCByZXF1 aXJpbmcgYW4gaW50ZXJydXB0IG9uIGNvbXBsZXRpb24gaXMgYSBiaXQgb2YgYW4gZWRnZSBjYXNl IC0gdGhlIApkZWZhdWx0IGFtb25nIG90aGVyIGRtYWVuZ2luZSBkcml2ZXJzIGFwcGVhcnMgdG8g YmUgdG8gZW5hYmxlIGludGVycnVwdHMgCnVuY29uZGl0aW9uYWxseS4KCgoKCl9fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmxpbnV4LXJwaS1rZXJuZWwgbWFp bGluZyBsaXN0CmxpbnV4LXJwaS1rZXJuZWxAbGlzdHMuaW5mcmFkZWFkLm9yZwpodHRwOi8vbGlz dHMuaW5mcmFkZWFkLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2xpbnV4LXJwaS1rZXJuZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753437AbbEHL0u (ORCPT ); Fri, 8 May 2015 07:26:50 -0400 Received: from mail1.bemta5.messagelabs.com ([195.245.231.152]:37143 "EHLO mail1.bemta5.messagelabs.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752840AbbEHL0n (ORCPT ); Fri, 8 May 2015 07:26:43 -0400 X-Greylist: delayed 388 seconds by postgrey-1.27 at vger.kernel.org; Fri, 08 May 2015 07:26:42 EDT X-Env-Sender: jonathan@raspberrypi.org X-Msg-Ref: server-4.tower-90.messagelabs.com!1431084005!1150277!1 X-Originating-IP: [217.28.140.9] X-StarScan-Received: X-StarScan-Version: 6.13.14; banners=-,-,- X-VirusChecked: Checked Message-ID: <554C9BE0.60205@raspberrypi.org> Date: Fri, 8 May 2015 12:20:00 +0100 From: Jonathan Bell User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: =?UTF-8?B?Tm9yYWxmIFRyw7hubmVz?= , Martin Sperl , Stephen Warren CC: , , Linux Kernel Mailing List , linux-rpi-kernel , , linux-spi Subject: Re: [PATCH v2] dmaengine: bcm2835: Add slave dma support References: <1429355160-13657-1-git-send-email-noralf@tronnes.org> <554A69D5.90301@tronnes.org> In-Reply-To: <554A69D5.90301@tronnes.org> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [86.129.220.154] X-ClientProxiedBy: THHSTE15FE06.hs20.net (192.168.251.46) To thhste15d1be5.hs20.net (192.168.251.26) X-EXCLAIMER-MD-CONFIG: 266e7a57-cddd-49fd-bdea-19bca6d40303 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/05/2015 20:21, Noralf Trønnes wrote: > > Den 01.05.2015 23:07, skrev Martin Sperl: >> Tests with the initial (and incomplete) version of the spi-bcm2835 >> driver >> with DMA transfer support show that the dma-engine works as expected >> with >> this patch. >> >> There is one one observation: >> >>> On 18.04.2015, at 13:06, Noralf Trønnes wrote: >>> +static struct dma_async_tx_descriptor * >>> +bcm2835_dma_prep_slave_sg(struct dma_chan *chan, >>> + struct scatterlist *sgl, >>> + unsigned int sg_len, >>> + enum dma_transfer_direction direction, >>> + unsigned long flags, void *context) >>> +{ >> ... >>> + /* Enable */ >>> + if (i == sg_len - 1 && len - j <= max_size) >>> + control_block->info |= BCM2835_DMA_INT_EN; >> The observation is that an interrupt is always triggered - even in >> the case >> where flags does NOT have DMA_PREP_INTERRUPT set. >> This may not be necessary and avoid interrupts. >> >> So maybe the above if clause should get extended by: >> && (flags & DMA_PREP_INTERRUPT) >> to only trigger an interrupt when really requested. >> >> I am not sure if there are any side-effects because of this besides >> having the >> requirement on the client to run dmaengine_terminate_all() on that >> specific dma >> channel without interrupts when the transfer is finished. >> >> In the case of SPI we have TX feed the fifo - which finishes early - >> , but we >> only need to the interrupt when RX finishes reading the fifo, which >> indicates >> that the SPI-transfer is fully finished. >> So having an interrupt on TX is not necessary for the process. >> >> The same observations may also apply to bcm2835_dma_prep_dma_cyclic >> (which is >> outside of this patch provided by Noralf). > > Jonathan, can you comment on this? > > > Noralf. > > I agree that the interrupt generated would be spurious - in the case where it is not required. However if you do && (flags & DMA_PREP_INTERRUPT) then all users of this driver need to explicitly set interrupt flags when doing a scatter-gather transfer. As I understand it, currently the only upstream client of this driver is the I2S driver which only uses cyclic anyway. Not requiring an interrupt on completion is a bit of an edge case - the default among other dmaengine drivers appears to be to enable interrupts unconditionally.