From: Jeremy Kerr <jk@codeconstruct.com.au> To: Ryan Chen <ryan_chen@aspeedtech.com>, Andrew Jeffery <andrew@aj.id.au>, Brendan Higgins <brendan.higgins@linux.dev>, Benjamin Herrenschmidt <benh@kernel.crashing.org>, Joel Stanley <joel@jms.id.au>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Philipp Zabel <p.zabel@pengutronix.de>, "linux-i2c@vger.kernel.org" <linux-i2c@vger.kernel.org>, "openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>, "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, "linux-aspeed@lists.ozlabs.org" <linux-aspeed@lists.ozlabs.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v6 1/2] dt-bindings: i2c: aspeed: support for AST2600-i2cv2 Date: Mon, 27 Feb 2023 13:40:12 +0800 [thread overview] Message-ID: <f4cb3efc9825efa582aa94bd03657b1319ff38fd.camel@codeconstruct.com.au> (raw) In-Reply-To: <SEZPR06MB52699858C92383E8E07D0832F2AF9@SEZPR06MB5269.apcprd06.prod.outlook.com> 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
WARNING: multiple messages have this Message-ID (diff)
From: Jeremy Kerr <jk@codeconstruct.com.au> To: Ryan Chen <ryan_chen@aspeedtech.com>, Andrew Jeffery <andrew@aj.id.au>, Brendan Higgins <brendan.higgins@linux.dev>, Benjamin Herrenschmidt <benh@kernel.crashing.org>, Joel Stanley <joel@jms.id.au>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Philipp Zabel <p.zabel@pengutronix.de>, "linux-i2c@vger.kernel.org" <linux-i2c@vger.kernel.org>, "openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>, "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, "linux-aspeed@lists.ozlabs.org" <linux-aspeed@lists.ozlabs.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v6 1/2] dt-bindings: i2c: aspeed: support for AST2600-i2cv2 Date: Mon, 27 Feb 2023 13:40:12 +0800 [thread overview] Message-ID: <f4cb3efc9825efa582aa94bd03657b1319ff38fd.camel@codeconstruct.com.au> (raw) In-Reply-To: <SEZPR06MB52699858C92383E8E07D0832F2AF9@SEZPR06MB5269.apcprd06.prod.outlook.com> 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-02-27 5:40 UTC|newest] Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-02-26 3:13 [PATCH v6 0/2] Add ASPEED AST2600 I2Cv2 controller driver Ryan Chen 2023-02-26 3:13 ` Ryan Chen 2023-02-26 3:13 ` [PATCH v6 1/2] dt-bindings: i2c: aspeed: support for AST2600-i2cv2 Ryan Chen 2023-02-26 3:13 ` Ryan Chen 2023-02-26 7:04 ` Jeremy Kerr 2023-02-26 7:04 ` Jeremy Kerr 2023-02-27 4:12 ` Ryan Chen 2023-02-27 4:12 ` Ryan Chen 2023-02-27 5:40 ` Jeremy Kerr [this message] 2023-02-27 5:40 ` Jeremy Kerr 2023-03-01 3:02 ` Ryan Chen 2023-03-01 3:02 ` Ryan Chen 2023-03-01 3:23 ` Jeremy Kerr 2023-03-01 3:23 ` Jeremy Kerr 2023-03-01 3:40 ` Ryan Chen 2023-03-01 3:40 ` Ryan Chen 2023-02-27 8:25 ` Krzysztof Kozlowski 2023-02-27 8:25 ` Krzysztof Kozlowski 2023-03-01 5:57 ` Ryan Chen 2023-03-01 5:57 ` Ryan Chen 2023-03-03 8:20 ` Krzysztof Kozlowski 2023-03-03 8:20 ` Krzysztof Kozlowski 2023-03-03 8:28 ` Ryan Chen 2023-03-03 8:28 ` Ryan Chen 2023-03-03 8:28 ` Ryan Chen 2023-03-03 8:50 ` Krzysztof Kozlowski 2023-03-03 8:50 ` Krzysztof Kozlowski 2023-03-03 8:55 ` Ryan Chen 2023-03-03 8:55 ` Ryan Chen 2023-03-03 8:55 ` Ryan Chen 2023-03-03 9:26 ` Krzysztof Kozlowski 2023-03-03 9:26 ` Krzysztof Kozlowski 2023-03-03 10:16 ` Ryan Chen 2023-03-03 10:16 ` Ryan Chen 2023-03-03 10:16 ` Ryan Chen 2023-03-03 10:41 ` Krzysztof Kozlowski 2023-03-03 10:41 ` Krzysztof Kozlowski 2023-03-04 1:33 ` Ryan Chen 2023-03-04 1:33 ` Ryan Chen 2023-03-04 1:33 ` Ryan Chen 2023-03-05 9:49 ` Krzysztof Kozlowski 2023-03-05 9:49 ` Krzysztof Kozlowski 2023-03-06 0:48 ` Ryan Chen 2023-03-06 0:48 ` Ryan Chen 2023-03-06 0:48 ` Ryan Chen 2023-03-07 8:11 ` Krzysztof Kozlowski 2023-03-07 8:11 ` Krzysztof Kozlowski 2023-03-07 10:09 ` Ryan Chen 2023-03-07 10:09 ` Ryan Chen 2023-03-07 10:09 ` Ryan Chen 2023-03-09 8:51 ` Krzysztof Kozlowski 2023-03-09 8:51 ` Krzysztof Kozlowski 2023-03-09 9:15 ` Ryan Chen 2023-03-09 9:15 ` Ryan Chen 2023-03-09 9:15 ` Ryan Chen 2023-03-12 12:33 ` Andi Shyti 2023-03-12 12:33 ` Andi Shyti 2023-03-12 12:33 ` Andi Shyti 2023-03-18 9:09 ` Andi Shyti 2023-03-18 9:09 ` Andi Shyti 2023-03-18 9:09 ` Andi Shyti 2023-03-19 2:05 ` Ryan Chen 2023-03-19 2:05 ` Ryan Chen 2023-03-19 2:05 ` Ryan Chen 2023-02-26 3:13 ` [PATCH v6 2/2] i2c: aspeed: support ast2600 i2cv new register mode driver Ryan Chen 2023-02-26 3:13 ` Ryan Chen
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=f4cb3efc9825efa582aa94bd03657b1319ff38fd.camel@codeconstruct.com.au \ --to=jk@codeconstruct.com.au \ --cc=andrew@aj.id.au \ --cc=benh@kernel.crashing.org \ --cc=brendan.higgins@linux.dev \ --cc=devicetree@vger.kernel.org \ --cc=joel@jms.id.au \ --cc=krzysztof.kozlowski+dt@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-aspeed@lists.ozlabs.org \ --cc=linux-i2c@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=openbmc@lists.ozlabs.org \ --cc=p.zabel@pengutronix.de \ --cc=robh+dt@kernel.org \ --cc=ryan_chen@aspeedtech.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.