From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeremy Kerr Date: Mon, 27 Feb 2023 13:40:12 +0800 Subject: [PATCH v6 1/2] dt-bindings: i2c: aspeed: support for AST2600-i2cv2 In-Reply-To: References: <20230226031321.3126756-1-ryan_chen@aspeedtech.com> <20230226031321.3126756-2-ryan_chen@aspeedtech.com> <8999ef4a57b035a81b086d8732d119638d46968c.camel@codeconstruct.com.au> Message-ID: List-Id: To: linux-aspeed@lists.ozlabs.org MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Hi Ryan, > Yes, I2C controller share the same dma engine. The original thought > can be enable in all i2c channel. But in AST2600 have ERRATA "I2C DMA > fails when DRAM bus is busy and it can not take DMA write data > immediately", So it means only 1 i2c bus can be enable for DMA mode. OK, this is a pretty important detail! I'd suggest putting it in the binding document. Anything in the cover letter will get lost after review. If there is documentation that would be useful for a DTS author, I'd suggest putting it in the binding. > It means only 1 bus channel can be enable DMA for use case. > That following example for board-specific selection. > It is description in cover-letter. > The following is board-specific design example. > Board A???????????????????????????????????????? Board B > -------------------------?????????????????????? ------------------------ > > i2c bus#1(master/slave)? <===fingerprint ===> i2c bus#x (master/slave)| > > i2c bus#2(master)-> tmp i2c device |??????? |?????????????????? | > > i2c bus#3(master)-> adc i2c device |??????? |?????????????????? | > -------------------------?????????????????????? ------------------------ > > - in bus#1 situation, you should use DMA mode. > Because bus#1 have trunk data needed for transfer, it can enable bus > dma mode to reduce cpu utilized. What is "trunk data" in this context? Is this just a statement about the amount of expected transfers? > - in bus#2/3 situation, you should use buffer/byte mode > bus#2/3 is small package transmit, it can enable buffer mode or byte > mode to reduce memory cache flush overhead. > Buffer mode is better, because byte mode have interrupt > overhead(interrupt per byte data transmit), > > -But if you more bus#4 that still have trunk data needed for transfer > (master/slave), > it also use buffer mode to transmit. Because bus#1 have been use for > DMA mode. So, it sounds like: - there's no point in using byte mode, as buffer mode provides equivalent functionality with fewer drawbacks (ie, less interrupt load) - this just leaves the dma and buffer modes - only one controller can use dma mode So: how about just a single boolean property to indicate "use DMA on this controller"? Something like aspeed,enable-dma? Or if DT binding experts can suggest something common that might be more suitable? Cheers, Jeremy From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 68A0EC64ED8 for ; Mon, 27 Feb 2023 05:40:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229619AbjB0FkT (ORCPT ); Mon, 27 Feb 2023 00:40:19 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35066 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229470AbjB0FkT (ORCPT ); Mon, 27 Feb 2023 00:40:19 -0500 Received: from codeconstruct.com.au (pi.codeconstruct.com.au [203.29.241.158]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E9FD8976F; Sun, 26 Feb 2023 21:40:16 -0800 (PST) Received: from pecola.lan (unknown [159.196.93.152]) by mail.codeconstruct.com.au (Postfix) with ESMTPSA id CB3652003E; Mon, 27 Feb 2023 13:40:12 +0800 (AWST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=codeconstruct.com.au; s=2022a; t=1677476415; bh=reKxI7sW0i4ltCDJTvKHcAaH9AmY4ryoZsjHKDcfMCY=; h=Subject:From:To:Date:In-Reply-To:References; b=UN5VOnm0rPJsYccuS4TGHKJv6cIomIM7hzAY5SFFKHUbKuT5deuBvy1uKT8zZDZk4 kOdLiWJQzZi9Kxu4H3henInU2gZGDWGanqbocR5aALKtRA2Yhp0CcRQZGzMOV7UQZj sZh00wNgJLA8ANcv0ZUo6MLthTqDu4JTXOZs+Q82JhQNOU3qxrPeEQdJhcmVGn78hA cSXILtC9ki4JOxi5PqGSOWZ1rDuJsdpE+di7PtbzJmUrx0esIMcC1GBXBZ8tzTMPFi bNEEh89jhf8roauQCKJLOu9yStst/pA8r8SlpSrEupvyVAoQ8oQLT1pG99kdP1f3e+ /1qskQMnr4x3w== Message-ID: Subject: Re: [PATCH v6 1/2] dt-bindings: i2c: aspeed: support for AST2600-i2cv2 From: Jeremy Kerr To: Ryan Chen , Andrew Jeffery , Brendan Higgins , Benjamin Herrenschmidt , Joel Stanley , Rob Herring , Krzysztof Kozlowski , Philipp Zabel , "linux-i2c@vger.kernel.org" , "openbmc@lists.ozlabs.org" , "devicetree@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-aspeed@lists.ozlabs.org" , "linux-kernel@vger.kernel.org" Date: Mon, 27 Feb 2023 13:40:12 +0800 In-Reply-To: References: <20230226031321.3126756-1-ryan_chen@aspeedtech.com> <20230226031321.3126756-2-ryan_chen@aspeedtech.com> <8999ef4a57b035a81b086d8732d119638d46968c.camel@codeconstruct.com.au> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.46.3-1 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-i2c@vger.kernel.org Hi Ryan, > Yes, I2C controller share the same dma engine. The original thought > can be enable in all i2c channel. But in AST2600 have ERRATA "I2C DMA > fails when DRAM bus is busy and it can not take DMA write data > immediately", So it means only 1 i2c bus can be enable for DMA mode. OK, this is a pretty important detail! I'd suggest putting it in the binding document. Anything in the cover letter will get lost after review. If there is documentation that would be useful for a DTS author, I'd suggest putting it in the binding. > It means only 1 bus channel can be enable DMA for use case. > That following example for board-specific selection. > It is description in cover-letter. > The following is board-specific design example. > Board A=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0 Board B > -------------------------=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0 ------------------------ > > i2c bus#1(master/slave)=C2=A0 <=3D=3D=3Dfingerprint =3D=3D=3D> i2c bus#= x (master/slave)| > > i2c bus#2(master)-> tmp i2c device |=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0 |=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 | > > i2c bus#3(master)-> adc i2c device |=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0 |=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 | > -------------------------=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0 ------------------------ >=20 > - in bus#1 situation, you should use DMA mode. > Because bus#1 have trunk data needed for transfer, it can enable bus > dma mode to reduce cpu utilized. What is "trunk data" in this context? Is this just a statement about the amount of expected transfers? > - in bus#2/3 situation, you should use buffer/byte mode > bus#2/3 is small package transmit, it can enable buffer mode or byte > mode to reduce memory cache flush overhead. > Buffer mode is better, because byte mode have interrupt > overhead(interrupt per byte data transmit), >=20 > -But if you more bus#4 that still have trunk data needed for transfer > (master/slave), > it also use buffer mode to transmit. Because bus#1 have been use for > DMA mode. So, it sounds like: - there's no point in using byte mode, as buffer mode provides equivalent functionality with fewer drawbacks (ie, less interrupt load) - this just leaves the dma and buffer modes - only one controller can use dma mode So: how about just a single boolean property to indicate "use DMA on this controller"? Something like aspeed,enable-dma? Or if DT binding experts can suggest something common that might be more suitable? Cheers, Jeremy From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 8BA72C64ED6 for ; Mon, 27 Feb 2023 05:41:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Date:To:From:Subject:Message-ID:Reply-To:Cc:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=NLCuzWVNeNJvPNvaewXq1xrz9f/hkAQs5XpV9D1KuuY=; b=N0clk6dXsfuRBp T78wsF8Ec1yvtufF1U0e+QE2/aGrHl78Eb3vdh5KCPensRpGu51lKCQ4wR6JZQdn8knu0UJw3k/L1 VMpPe9W1o0tsIbhYRZr/L9F/2yMjOidaBkRV+opDFBJS1LuiXskq1V1OLMsrYk/zDCFkk0Z7a9voW LBNfb74b+8iltyNx115+Dj4atUyTWdAFGnAuymY935881LYS04Bi1o+nftoNwu7Wmji7gwb/ijMs3 eUda0gYu2BC5YyYxl568msaF2j/i3TNJsyVKO0vlODXB7KYNfquZumJ6l94FStKcPJdhTtOvWi25K Lvg3MKbM8LWnY6G3LIMA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pWWFN-008WFn-R5; Mon, 27 Feb 2023 05:40:25 +0000 Received: from pi.codeconstruct.com.au ([203.29.241.158] helo=codeconstruct.com.au) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pWWFK-008WEt-Lt for linux-arm-kernel@lists.infradead.org; Mon, 27 Feb 2023 05:40:24 +0000 Received: from pecola.lan (unknown [159.196.93.152]) by mail.codeconstruct.com.au (Postfix) with ESMTPSA id CB3652003E; Mon, 27 Feb 2023 13:40:12 +0800 (AWST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=codeconstruct.com.au; s=2022a; t=1677476415; bh=reKxI7sW0i4ltCDJTvKHcAaH9AmY4ryoZsjHKDcfMCY=; h=Subject:From:To:Date:In-Reply-To:References; b=UN5VOnm0rPJsYccuS4TGHKJv6cIomIM7hzAY5SFFKHUbKuT5deuBvy1uKT8zZDZk4 kOdLiWJQzZi9Kxu4H3henInU2gZGDWGanqbocR5aALKtRA2Yhp0CcRQZGzMOV7UQZj sZh00wNgJLA8ANcv0ZUo6MLthTqDu4JTXOZs+Q82JhQNOU3qxrPeEQdJhcmVGn78hA cSXILtC9ki4JOxi5PqGSOWZ1rDuJsdpE+di7PtbzJmUrx0esIMcC1GBXBZ8tzTMPFi bNEEh89jhf8roauQCKJLOu9yStst/pA8r8SlpSrEupvyVAoQ8oQLT1pG99kdP1f3e+ /1qskQMnr4x3w== Message-ID: Subject: Re: [PATCH v6 1/2] dt-bindings: i2c: aspeed: support for AST2600-i2cv2 From: Jeremy Kerr To: Ryan Chen , Andrew Jeffery , Brendan Higgins , Benjamin Herrenschmidt , Joel Stanley , Rob Herring , Krzysztof Kozlowski , Philipp Zabel , "linux-i2c@vger.kernel.org" , "openbmc@lists.ozlabs.org" , "devicetree@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-aspeed@lists.ozlabs.org" , "linux-kernel@vger.kernel.org" Date: Mon, 27 Feb 2023 13:40:12 +0800 In-Reply-To: References: <20230226031321.3126756-1-ryan_chen@aspeedtech.com> <20230226031321.3126756-2-ryan_chen@aspeedtech.com> <8999ef4a57b035a81b086d8732d119638d46968c.camel@codeconstruct.com.au> User-Agent: Evolution 3.46.3-1 MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230226_214022_949633_45F2F18C X-CRM114-Status: GOOD ( 16.43 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org SGkgUnlhbiwKCj4gWWVzLCBJMkMgY29udHJvbGxlciBzaGFyZSB0aGUgc2FtZSBkbWEgZW5naW5l LiBUaGUgb3JpZ2luYWwgdGhvdWdodAo+IGNhbiBiZSBlbmFibGUgaW4gYWxsIGkyYyBjaGFubmVs LiBCdXQgaW4gQVNUMjYwMCBoYXZlIEVSUkFUQSAiSTJDIERNQQo+IGZhaWxzIHdoZW4gRFJBTSBi dXMgaXMgYnVzeSBhbmQgaXQgY2FuIG5vdCB0YWtlIERNQSB3cml0ZSBkYXRhCj4gaW1tZWRpYXRl bHkiLCBTbyBpdCBtZWFucyBvbmx5IDEgaTJjIGJ1cyBjYW4gYmUgZW5hYmxlIGZvciBETUEgbW9k ZS4KCk9LLCB0aGlzIGlzIGEgcHJldHR5IGltcG9ydGFudCBkZXRhaWwhIEknZCBzdWdnZXN0IHB1 dHRpbmcgaXQgaW4gdGhlCmJpbmRpbmcgZG9jdW1lbnQuCgpBbnl0aGluZyBpbiB0aGUgY292ZXIg bGV0dGVyIHdpbGwgZ2V0IGxvc3QgYWZ0ZXIgcmV2aWV3LiBJZiB0aGVyZSBpcwpkb2N1bWVudGF0 aW9uIHRoYXQgd291bGQgYmUgdXNlZnVsIGZvciBhIERUUyBhdXRob3IsIEknZCBzdWdnZXN0IHB1 dHRpbmcKaXQgaW4gdGhlIGJpbmRpbmcuCgo+IEl0IG1lYW5zIG9ubHkgMSBidXMgY2hhbm5lbCBj YW4gYmUgZW5hYmxlIERNQSBmb3IgdXNlIGNhc2UuCj4gVGhhdCBmb2xsb3dpbmcgZXhhbXBsZSBm b3IgYm9hcmQtc3BlY2lmaWMgc2VsZWN0aW9uLgo+IEl0IGlzIGRlc2NyaXB0aW9uIGluIGNvdmVy LWxldHRlci4KPiBUaGUgZm9sbG93aW5nIGlzIGJvYXJkLXNwZWNpZmljIGRlc2lnbiBleGFtcGxl Lgo+IEJvYXJkIEHCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoCBCb2FyZCBCCj4gLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLcKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg IC0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQo+ID4gaTJjIGJ1cyMxKG1hc3Rlci9zbGF2ZSnCoCA8 PT09ZmluZ2VycHJpbnQgPT09PiBpMmMgYnVzI3ggKG1hc3Rlci9zbGF2ZSl8Cj4gPiBpMmMgYnVz IzIobWFzdGVyKS0+IHRtcCBpMmMgZGV2aWNlIHzCoMKgwqDCoMKgwqDCoCB8wqDCoMKgwqDCoMKg wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgIHwKPiA+IGkyYyBidXMjMyhtYXN0ZXIpLT4gYWRjIGky YyBkZXZpY2UgfMKgwqDCoMKgwqDCoMKgIHzCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDC oMKgwqAgfAo+IC0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS3CoMKgwqDCoMKgwqDCoMKgwqDCoMKg wqDCoMKgwqDCoMKgwqDCoMKgwqDCoCAtLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0KPiAKPiAtIGlu IGJ1cyMxIHNpdHVhdGlvbiwgeW91IHNob3VsZCB1c2UgRE1BIG1vZGUuCj4gQmVjYXVzZSBidXMj MSBoYXZlIHRydW5rIGRhdGEgbmVlZGVkIGZvciB0cmFuc2ZlciwgaXQgY2FuIGVuYWJsZSBidXMK PiBkbWEgbW9kZSB0byByZWR1Y2UgY3B1IHV0aWxpemVkLgoKV2hhdCBpcyAidHJ1bmsgZGF0YSIg aW4gdGhpcyBjb250ZXh0PyBJcyB0aGlzIGp1c3QgYSBzdGF0ZW1lbnQgYWJvdXQgdGhlCmFtb3Vu dCBvZiBleHBlY3RlZCB0cmFuc2ZlcnM/Cgo+IC0gaW4gYnVzIzIvMyBzaXR1YXRpb24sIHlvdSBz aG91bGQgdXNlIGJ1ZmZlci9ieXRlIG1vZGUKPiBidXMjMi8zIGlzIHNtYWxsIHBhY2thZ2UgdHJh bnNtaXQsIGl0IGNhbiBlbmFibGUgYnVmZmVyIG1vZGUgb3IgYnl0ZQo+IG1vZGUgdG8gcmVkdWNl IG1lbW9yeSBjYWNoZSBmbHVzaCBvdmVyaGVhZC4KPiBCdWZmZXIgbW9kZSBpcyBiZXR0ZXIsIGJl Y2F1c2UgYnl0ZSBtb2RlIGhhdmUgaW50ZXJydXB0Cj4gb3ZlcmhlYWQoaW50ZXJydXB0IHBlciBi eXRlIGRhdGEgdHJhbnNtaXQpLAo+IAo+IC1CdXQgaWYgeW91IG1vcmUgYnVzIzQgdGhhdCBzdGls bCBoYXZlIHRydW5rIGRhdGEgbmVlZGVkIGZvciB0cmFuc2Zlcgo+IChtYXN0ZXIvc2xhdmUpLAo+ IGl0IGFsc28gdXNlIGJ1ZmZlciBtb2RlIHRvIHRyYW5zbWl0LiBCZWNhdXNlIGJ1cyMxIGhhdmUg YmVlbiB1c2UgZm9yCj4gRE1BIG1vZGUuCgpTbywgaXQgc291bmRzIGxpa2U6CgogLSB0aGVyZSdz IG5vIHBvaW50IGluIHVzaW5nIGJ5dGUgbW9kZSwgYXMgYnVmZmVyIG1vZGUgcHJvdmlkZXMKICAg ZXF1aXZhbGVudCBmdW5jdGlvbmFsaXR5IHdpdGggZmV3ZXIgZHJhd2JhY2tzIChpZSwgbGVzcyBp bnRlcnJ1cHQKICAgbG9hZCkKCiAtIHRoaXMganVzdCBsZWF2ZXMgdGhlIGRtYSBhbmQgYnVmZmVy IG1vZGVzCgogLSBvbmx5IG9uZSBjb250cm9sbGVyIGNhbiB1c2UgZG1hIG1vZGUKClNvOiBob3cg YWJvdXQganVzdCBhIHNpbmdsZSBib29sZWFuIHByb3BlcnR5IHRvIGluZGljYXRlICJ1c2UgRE1B IG9uCnRoaXMgY29udHJvbGxlciI/IFNvbWV0aGluZyBsaWtlIGFzcGVlZCxlbmFibGUtZG1hPyBP ciBpZiBEVCBiaW5kaW5nCmV4cGVydHMgY2FuIHN1Z2dlc3Qgc29tZXRoaW5nIGNvbW1vbiB0aGF0 IG1pZ2h0IGJlIG1vcmUgc3VpdGFibGU/CgpDaGVlcnMsCgoKSmVyZW15CgpfX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpsaW51eC1hcm0ta2VybmVsIG1haWxp bmcgbGlzdApsaW51eC1hcm0ta2VybmVsQGxpc3RzLmluZnJhZGVhZC5vcmcKaHR0cDovL2xpc3Rz LmluZnJhZGVhZC5vcmcvbWFpbG1hbi9saXN0aW5mby9saW51eC1hcm0ta2VybmVsCg==