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
next prev parent reply other threads:[~2023-02-27 5:40 UTC|newest]
Thread overview: 28+ 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 ` [PATCH v6 1/2] dt-bindings: i2c: aspeed: support for AST2600-i2cv2 Ryan Chen
2023-02-26 7:04 ` Jeremy Kerr
2023-02-27 4:12 ` Ryan Chen
2023-02-27 5:40 ` Jeremy Kerr [this message]
2023-03-01 3:02 ` Ryan Chen
2023-03-01 3:23 ` Jeremy Kerr
2023-03-01 3:40 ` Ryan Chen
2023-02-27 8:25 ` Krzysztof Kozlowski
2023-03-01 5:57 ` Ryan Chen
2023-03-03 8:20 ` Krzysztof Kozlowski
2023-03-03 8:28 ` Ryan Chen
2023-03-03 8:50 ` Krzysztof Kozlowski
2023-03-03 8:55 ` Ryan Chen
2023-03-03 9:26 ` Krzysztof Kozlowski
2023-03-03 10:16 ` Ryan Chen
2023-03-03 10:41 ` Krzysztof Kozlowski
2023-03-04 1:33 ` Ryan Chen
2023-03-05 9:49 ` Krzysztof Kozlowski
2023-03-06 0:48 ` Ryan Chen
2023-03-07 8:11 ` Krzysztof Kozlowski
2023-03-07 10:09 ` Ryan Chen
2023-03-09 8:51 ` Krzysztof Kozlowski
2023-03-09 9:15 ` Ryan Chen
2023-03-12 12:33 ` Andi Shyti
2023-03-18 9:09 ` Andi Shyti
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
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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).