devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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

  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).