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: Revert "dmaengine: pl330: add DMA_PAUSE feature" From: Marek Szyprowski Message-Id: Date: Thu, 10 May 2018 10:31:53 +0200 To: Frank Mori Hess Cc: Vinod Koul , dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org, Dan Williams , r.baldyga@hackerion.com, Krzysztof Kozlowski , Bartlomiej Zolnierkiewicz , Linux Samsung SOC List-ID: SGkgRnJhbmssCgpPbiAyMDE4LTA1LTA5IDE5OjQ4LCBGcmFuayBNb3JpIEhlc3Mgd3JvdGU6Cj4g T24gV2VkLCBNYXkgOSwgMjAxOCBhdCA5OjE5IEFNLCBNYXJlayBTenlwcm93c2tpCj4gPG0uc3p5 cHJvd3NraUBzYW1zdW5nLmNvbT4gd3JvdGU6Cj4+IEkgdW5kZXJzdGFuZCB0aGF0IHBsMzMwIGRv ZXNuJ3Qgc3VwcG9ydCByZWFsIFBBVVNFLCBidXQgYXMgaXQgaGFzIGJlZW4KPj4gaW1wbGVtZW50 ZWQgc2luY2UgMjAxNSB0aGUgbGltaXRlZCBzdXBwb3J0IGlzIHBlcmZlY3RseSBwb3NzaWJsZSAt IGp1c3QKPj4gdG8gbGV0IHNlcmlhbCBkcml2ZXIgdG8gcmVhZCB0aGUgYW1vdW50IG9mIGRhdGEg dHJhbnNmZXJyZWQuCj4+Cj4+IFBsZWFzZSBub3RlIHRoYXQgRE1BIGVuZ2luZSBkb2N1bWVudGF0 aW9uIGNsZWFybHkgc3RhdGVzIHRoYXQgdGhlIGJlc3QKPj4gcmVzaWR1ZSBncmFudWxhcml0eSB0 aGUgZHJpdmVyIG1pZ2h0IGV4cGVjdCBpcyBCVVJTVCBncmFudWxhcml0eS4gVGhlcmUKPj4gaXMg bm8gd2F5IHRvIGdldCB0aGUgaW5mb3JtYXRpb24gYWJvdXQgdGhlIGRhdGEgd2hpY2ggc3RpbGwg c2l0cyBpbiB0aGUKPj4gRE1BIGVuZ2luZSBmaWZvIHdoZW4gdHJhbnNmZXIgaXMgcGF1c2VkL3Rl cm1pbmF0ZWQuIFRoaXMgbWVhbnMgdGhhdAo+PiB0aGUgc2VyaWFsIGRyaXZlciBzaG91bGQgc2V0 IG1heGJ1cnN0ID0gMSBpZiBpdCBpcyBpbnRlcmVzdGVkIGluCj4+IGdldHRpbmcgZXhhY3QgbnVt YmVyIG9mIGJ5dGVzIHJlY2VpdmVkL3NlbnQuIFdpdGggbWF4YnVyc3QgPSAxIHRoZXJlCj4+IGlz IG5vIHN1Y2ggdGhpbmcgYXMgZGF0YSBsb29zZSBpbiB0aGUgRE1BIGVuZ2luZSBmaWZvLgo+IFRo YXQgaXMgbm90IG15IHVuZGVyc3RhbmRpbmcgb2YgdGhlIGRtYWVuZ2luZSBwYXVzZSByZXF1aXJl bWVudHMsIGJ1dAo+IG9mIGNvdXJzZSBWaW5vZCBjYW4gc3BlYWsgYXV0aG9yaXRhdGl2ZWx5IG9u IHRoaXMuCgpCYXNpbmcgb24gdGhlIGNvbW1lbnRzIGluIGRtYV9yZXNpZHVlX2dyYW51bGFyaXR5 IGVudW0gZG9jdW1lbnRhdGlvbiAoc2VlCmluY2x1ZGUvbGludXgvZG1hZW5naW5lLmgpIHRoZXJl IGlzIG5vIGJldHRlciBncmFudWxhcml0eSBvZiByZXNpZHVlCnJlcG9ydGluZyB0aGFuIEJVUlNU IHVuaXRzLiBJZiBkcml2ZXIgbmVlZHMgYnl0ZSBhY2N1cmFjeSwgdGhlbiBpdCBzaG91bGQKdXNl IE1BWEJVUlNUPTEgYW5kIERNQV9TTEFWRV9CVVNXSURUSF8xX0JZVEUgY29uZmlndXJhdGlvbi4K Cj4gSSBhbHNvIGRvbid0IGFncmVlCj4gdGhhdCBkYXRhIGxvc3MgY2Fubm90IGhhcHBlbiBmb3Ig c2luZ2xlIHRyYW5zZmVycy4gIEl0IG9ubHkgYmVjb21lcwo+IGxlc3MgbGlrZWx5IHNpbmNlIHRo ZXJlIGFyZSBmZXdlciBieXRlcyBpbiB0aGUgZG1hIGNvbnRyb2xsZXIgZmlmbyBhbmQKPiB0aGV5 IHN0YXkgdGhlcmUgZm9yIGEgc2hvcnRlciBwZXJpb2Qgb2YgdGltZS4gIEJ1dCBldmVuIHNvLCB0 aGVyZSBpcwo+IG5vdGhpbmcgc3RvcHBpbmcgdGhlIERNQUtJTEwgZnJvbSBraWxsaW5nIHRoZSB0 cmFuc2ZlciB0aHJlYWQgYWZ0ZXIgaXQKPiBkb2VzIGEgc2luZ2xlIGJ5dGUgbG9hZCBidXQgYmVm b3JlIGl0IGRvZXMgdGhlIGFzc29jaWF0ZWQgc2luZ2xlIGJ5dGUKPiBzdG9yZSwgYXMgdGhleSBh cmUgcGVyZm9ybWVkIGJ5IHNlcGFyYXRlIGluc3RydWN0aW9ucy4gIEFzIGZhciBhcyB5b3VyCj4g c2VyaWFsIHBvcnQgZ29lcywgdGhlIGJ5dGUgaGFzIGJlZW4gcmVhZCBieSB0aGUgaG9zdCwgZXZl biB0aG91Z2ggaXQKPiBuZXZlciBtYWRlIGl0IHRvIG1lbW9yeSwgc28gaXQgaXMgZ29uZSBmb3Jl dmVyLiAgVGhlIGRtYS0zMzAgZG9lcyBoYXZlCj4gYSBsb2FkIGxvY2sgd2hpY2ggcHJldmVudHMg c29tZSBvcGVyYXRpb25zIGZyb20gaW50ZXJydXB0aW5nIGEKPiBsb2FkL3N0b3JlIGNvbWJpbmF0 aW9uLCBidXQgaW4gbXkgb2JzZXJ2YXRpb25zIERNQUtJTEwgZG9lcyBub3QKPiByZXNwZWN0IHRo ZSBsb2FkIGxvY2suCgpGb3IgdGhlIGxhc3QgMyB5ZWFycyBubyBvbmUgb2JzZXJ2ZWQgYW55IGlz c3VlIHdpdGggdGhlIGN1cnJlbnQgZGVzaWduCihzaW5nbGUgdHJhbnNmZXJzIHdpdGggRE1BX1NM QVZFX0JVU1dJRFRIXzFfQllURSkuIEJ5IHJlbW92aW5nIHRoaXMKZmVhdHVyZSB3ZSB3aWxsIGxv b3NlIGFiaWxpdHkgdG8gdXNlIERNQSBpbiB0aGUgc2VyaWFsIGRyaXZlcnMsIHdoYXQgaXMKbWFp bmx5IHVzZWZ1bCBmb3IgbG93LXBvd2VyIGJsdWV0b290aCBvcGVyYXRpb24gKHNlcmlhbCBjb25z b2xlIGlzIHJlYWxseQpuZWdsaWdpYmxlIGNhc2UpLgoKSSBhZ3JlZSB0aGF0IFBMMzMwIGRyaXZl ciBzaG91bGQgbm90IGFkdmVydGlzZSBQQVVTRSBzdXBwb3J0LCBiZWNhdXNlCkhXIGRvZXNuJ3Qg aGF2ZSBzdWNoIGFiaWxpdHksIGJ1dCBmb3Igbm93IHRoaXMgaXMgdGhlIG9ubHkgd2F5IHRvIGdl dAp0aGUgbnVtYmVyIG9mIGJ5dGVzIHRyYW5zZmVycmVkIGFmdGVyIHRlcm1pbmF0aW5nIGEgdHJh bnNmZXIuIE9uZSB3b3VsZApuZWVkIHRvIGNoYW5nZSBETUEgZW5naW5lIGZyYW1ld29yayBBUEkg dG8gZml4IHRoaXMuCgo+PiAzLiBTYW1zdW5nIGRyaXZlciBkb2Vzbid0IGNoZWNrIGlmIERNQSBl bmdpbmUgc3VwcG9ydHMgUEFVU0UgZmVhdHVyZSBhbmQKPj4gICAgICBwcm9wZXIgcmVzaWR1ZSBy ZXBvcnRpbmcgZ3JhbnVsYXJpdHksIHNvIHlvdXIgcmV2ZXJ0IHNpbXBseSBicmVha3MgaXRzCj4+ ICAgICAgb3BlcmF0aW9uLgo+IE9oLCBJIHdhcyB3cm9uZ2x5IGFzc3VtaW5nIHlvdSB3ZXJlIHRh bGtpbmcgYWJvdXQgYW4gODI1MCBiYXNlZCBzZXJpYWwgZHJpdmVyLgoKTm9wZSwgU2Ftc3VuZyBT b0NzIGhhdmUgYSBjdXN0b20gVUFSVCBpcCBibG9jay4KCj4+IEkndmUgY2hlY2tlZCBvdGhlciBk ZXZpY2UgZHJpdmVycywgd2hpY2ggdXNlIHBsMzMwIERNQSBvbiBTYW1zdW5nIFNvQ3MgYW5kCj4+ IGJlc2lkZXMgc2VyaWFsLCBub25lIG9mIHRoZW0gY29uZmlndXJlIG1heGJ1cnN0ID4gMS4gV2hl biBJIGZvcmNlZCBzdWNoCj4+IGNvbmZpZ3VyYXRpb24sIG5vbmUgd29ya2VkIGZpbmUuIEknbSBh IGJpdCBjb25mdXNlZCB3aGF0IGRvZXMgaXQgbWVhbi4KPj4gRWl0aGVyIG5vbmUgb2YgdGhlIFNh bXN1bmcgU29DIGludGVncmF0ZWQgcGVyaXBoZXJhbHMgc3VwcG9ydCByZWFsIGJ1cnN0Cj4+IERN QSB0cmFuc2ZlcnMsIG9yIHRoZSBQTDMzMCBpbiBTYW1zdW5nIFNvQ3MgYXJlIHNvbWVob3cgbGlt aXRlZCBvcgo+PiBkeXNmdW5jdGlvbmFsLiBUaGVyZSBpcyBhbHJlYWR5IGEgcXVpcmsgaW4gcGwz MzAgZm9yIGJyb2tlbiBGTFVTSFAsIGJ1dCBJCj4+IGhhdmUgbm8gaWRlYSBob3cgdG8gZGlhZ25v c2UgaWYgdGhpcyBpcyB0aGUgY2FzZSBvciB0aGUgcHJvYmxlbSBpcyBpbiB0aGUKPj4gU29DIHBl cmlwaGVyYWxzLiBJIGNhbiBsaXZlIHdpdGggbWF4YnVyc3Qgc2V0IHRvIDEgaW4gdGhvc2UgZHJp dmVycyBhcyB0aGUKPj4gcHJvcGVyIGZpeC4KPiBXaGVuIHRoZSBETUEtMzMwIGlzIGluc3RydWN0 ZWQgdG8gZG8gYSBwZXJpcGhlcmFsIGJ1cnN0IHRyYW5zZmVyLCBpdAo+IGlnbm9yZXMgc2luZ2xl IHRyYW5zZmVyIHJlcXVlc3RzIGZyb20gdGhlIHBlcmlwaGVyYWwuICBXaGVuIGl0IGlzCj4gaW5z dHJ1Y3RlZCB0byBkbyBhIHNpbmdsZSB0cmFuc2ZlciwgaXQgd2lsbCBkbyBhIHNpbmdsZSB0cmFu c2ZlciBpbgo+IHJlc3BvbnNlIHRvIGVpdGhlciBhIGJ1cnN0IHJlcXVlc3Qgb3IgYSBzaW5nbGUg cmVxdWVzdC4gIFNvIHVubGVzcyB0aGUKPiBwZXJpcGhlcmFsIGFjdHVhbGx5IHN1cHBvcnRzIGJ1 cnN0IHJlcXVlc3RzLCB0aGUgdHJhbnNmZXIgd2lsbCBqdXN0Cj4gd2FpdCBmb3JldmVyIGZvciBh IGJ1cnN0IHJlcXVlc3Qgd2hpY2ggbmV2ZXIgY29tZXMuCgpPa2F5LCBzbyBpZiBJIGdldCBicm9r ZW4gdHJhbnNmZXJzIHdoZW4gSSBmb3JjZWQgQlVSU1QgbW9kZSwgdGhlbiBJIGNhbgphc3N1bWUg dGhhdCBwZXJpcGhlcmFscyBkb2Vzbid0IHJlYWxseSBzdXBwb3J0IGl0LgoKQmVzdCByZWdhcmRz Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934740AbeEJIcJ (ORCPT ); Thu, 10 May 2018 04:32:09 -0400 Received: from mailout2.w1.samsung.com ([210.118.77.12]:57286 "EHLO mailout2.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934585AbeEJIcG (ORCPT ); Thu, 10 May 2018 04:32:06 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20180510083203euoutp02d9cd6843533b65cc91ff37395474292e~tO13_sKZj1343413434euoutp02D X-AuditID: cbfec7f4-713ff700000043e4-42-5af4037ee933 Subject: Re: Revert "dmaengine: pl330: add DMA_PAUSE feature" To: Frank Mori Hess Cc: Vinod Koul , dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org, Dan Williams , r.baldyga@hackerion.com, Krzysztof Kozlowski , Bartlomiej Zolnierkiewicz , Linux Samsung SOC From: Marek Szyprowski Message-ID: Date: Thu, 10 May 2018 10:31:53 +0200 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: Content-Transfer-Encoding: 7bit Content-Language: en-US X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrOKsWRmVeSWpSXmKPExsWy7djP87p1zF+iDA48VrLYOGM9q8X0qRcY LVZP/ctqMf/xXDaL8+c3sFtc3jWHzWLG+X1MFnf/fWK02HnnBLMDp8fOWXfZPY7sPMbmsXjP SyaPTas62Tz6tqxi9Pi8SS6ALYrLJiU1J7MstUjfLoEr4897iYKbChW/Oj+xNjAukepi5OSQ EDCRWHLqCWMXIxeHkMAKRomme7dYIZwvjBL/W3YzgVQJCXxmlLgyIaiLkQOs4/+XXIia5YwS n1evY4Jw3jNKfPm8jB2kQVjAVqJ9yyk2EFtEQE1iw/aDLCBFzAI7mCSW3/7CCJJgEzCU6Hrb BVbEK2AnMbO1FSzOIqAqsXbNdrC4qECMxLR515kgagQlTs58wgJicwoESqx6dwwsziwgL7H9 7RxmCFtc4taT+WAXSQicYpf4u+UBO8SjLhInHs2AsoUlXh3fAmXLSJye3MMCYddL9H0/AtXc wyixt2UqE0TCWuLw8YusIP8zC2hKrN+lDwkKW4mLj+QgTD6JG28FIU7gk5i0bTozRJhXoqNN CGKGmsSs4+vglh68cIl5AqPyLCSPzULyzCwkz8xCWLuAkWUVo3hqaXFuemqxUV5quV5xYm5x aV66XnJ+7iZGYHI6/e/4lx2Mu/4kHWIU4GBU4uH9wPM5Sog1say4MvcQowQHs5II749zQCHe lMTKqtSi/Pii0pzU4kOM0hwsSuK8cRp1UUIC6YklqdmpqQWpRTBZJg5OqQbG1F38HUvXfDxg EsQrsHezvvbLR7rWkcHrLrdsCrl59vHWexvnvhJrSOCIuMcSIbpaMPuCAufi164/ZiqI1H7P 3meW97BKRsZ2vdQVRdPp4X97vsdp7llnLD7/uUyRopVnsH+UdvyUxBMv7ovuPBJSNfFJpoz1 S/0LE//Ok/4qsva2ja/MIXdfJZbijERDLeai4kQA04WlzUoDAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrNIsWRmVeSWpSXmKPExsVy+t/xu7o1zF+iDFqOmFlsnLGe1WL61AuM Fqun/mW1mP94LpvF+fMb2C0u75rDZjHj/D4mi7v/PjFa7LxzgtmB02PnrLvsHkd2HmPzWLzn JZPHplWdbB59W1YxenzeJBfAFqVnU5RfWpKqkJFfXGKrFG1oYaRnaGmhZ2RiqWdobB5rZWSq pG9nk5Kak1mWWqRvl6CX8ee9RMFNhYpfnZ9YGxiXSHUxcnBICJhI/P+S28XIxSEksJRR4ueW p+xdjJxAcRmJk9MaWCFsYYk/17rYQGwhgbeMErefOoPYwgK2Eu1bToHFRQTUJDZsP8gCMohZ YAeTxObDs9ggpr5nkmh6+wSsik3AUKLrLcQkXgE7iZmtrYwgNouAqsTaNdvB4qICMRI/jnax QNQISpyc+QTM5hQIlFj17hgTiM0sYCYxb/NDZghbXmL72zlQtrjErSfzmSYwCs1C0j4LScss JC2zkLQsYGRZxSiSWlqcm55bbKRXnJhbXJqXrpecn7uJERiR24793LKDsetd8CFGAQ5GJR7e Dzyfo4RYE8uKK3MPMUpwMCuJ8P44BxTiTUmsrEotyo8vKs1JLT7EaAr03ERmKdHkfGCyyCuJ NzQ1NLewNDQ3Njc2s1AS5z1vUBklJJCeWJKanZpakFoE08fEwSnVwBhsXnzw9zzDdz955+XH mO6rCJhtoi3sIZ2mcTVn2rqbho275yY+yYq7tSLsveJ0YfWZOfxPlTTXum/sPsmsvff23HO8 Rq8T9JoPT9pz6R2T+2UT24fLGepOHGrN5jqiYdpTztEofv7FxD8Feh1X5u+yKt/8MbHYN3p3 /OLLBptn7Oi+b7ax1EiJpTgj0VCLuag4EQBuCGFP3gIAAA== X-CMS-MailID: 20180510083156eucas1p1455f4fb3fe9a157a446d8d08b972c6b1 X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-MTR: 20180510083156eucas1p1455f4fb3fe9a157a446d8d08b972c6b1 X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20180508090407eucas1p13713284c2d3f5aa5c66f8d136be683c1 X-RootMTR: 20180508090407eucas1p13713284c2d3f5aa5c66f8d136be683c1 References: <2484918.HKVQc3yJkt@bear> <53b13d76-16a1-0e0a-09e1-c917e5d49326@samsung.com> <182f50b9-55b6-c9ce-07fb-718a1d22e9c8@samsung.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Frank, On 2018-05-09 19:48, Frank Mori Hess wrote: > On Wed, May 9, 2018 at 9:19 AM, Marek Szyprowski > wrote: >> I understand that pl330 doesn't support real PAUSE, but as it has been >> implemented since 2015 the limited support is perfectly possible - just >> to let serial driver to read the amount of data transferred. >> >> Please note that DMA engine documentation clearly states that the best >> residue granularity the driver might expect is BURST granularity. There >> is no way to get the information about the data which still sits in the >> DMA engine fifo when transfer is paused/terminated. This means that >> the serial driver should set maxburst = 1 if it is interested in >> getting exact number of bytes received/sent. With maxburst = 1 there >> is no such thing as data loose in the DMA engine fifo. > That is not my understanding of the dmaengine pause requirements, but > of course Vinod can speak authoritatively on this. Basing on the comments in dma_residue_granularity enum documentation (see include/linux/dmaengine.h) there is no better granularity of residue reporting than BURST units. If driver needs byte accuracy, then it should use MAXBURST=1 and DMA_SLAVE_BUSWIDTH_1_BYTE configuration. > I also don't agree > that data loss cannot happen for single transfers. It only becomes > less likely since there are fewer bytes in the dma controller fifo and > they stay there for a shorter period of time. But even so, there is > nothing stopping the DMAKILL from killing the transfer thread after it > does a single byte load but before it does the associated single byte > store, as they are performed by separate instructions. As far as your > serial port goes, the byte has been read by the host, even though it > never made it to memory, so it is gone forever. The dma-330 does have > a load lock which prevents some operations from interrupting a > load/store combination, but in my observations DMAKILL does not > respect the load lock. For the last 3 years no one observed any issue with the current design (single transfers with DMA_SLAVE_BUSWIDTH_1_BYTE). By removing this feature we will loose ability to use DMA in the serial drivers, what is mainly useful for low-power bluetooth operation (serial console is really negligible case). I agree that PL330 driver should not advertise PAUSE support, because HW doesn't have such ability, but for now this is the only way to get the number of bytes transferred after terminating a transfer. One would need to change DMA engine framework API to fix this. >> 3. Samsung driver doesn't check if DMA engine supports PAUSE feature and >> proper residue reporting granularity, so your revert simply breaks its >> operation. > Oh, I was wrongly assuming you were talking about an 8250 based serial driver. Nope, Samsung SoCs have a custom UART ip block. >> I've checked other device drivers, which use pl330 DMA on Samsung SoCs and >> besides serial, none of them configure maxburst > 1. When I forced such >> configuration, none worked fine. I'm a bit confused what does it mean. >> Either none of the Samsung SoC integrated peripherals support real burst >> DMA transfers, or the PL330 in Samsung SoCs are somehow limited or >> dysfunctional. There is already a quirk in pl330 for broken FLUSHP, but I >> have no idea how to diagnose if this is the case or the problem is in the >> SoC peripherals. I can live with maxburst set to 1 in those drivers as the >> proper fix. > When the DMA-330 is instructed to do a peripheral burst transfer, it > ignores single transfer requests from the peripheral. When it is > instructed to do a single transfer, it will do a single transfer in > response to either a burst request or a single request. So unless the > peripheral actually supports burst requests, the transfer will just > wait forever for a burst request which never comes. Okay, so if I get broken transfers when I forced BURST mode, then I can assume that peripherals doesn't really support it. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland