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: Wed, 01 Mar 2023 11:23:33 +0800 [thread overview] Message-ID: <c18b064b6b01bd547b2f03006dbf4bb6fdf9b91d.camel@codeconstruct.com.au> (raw) In-Reply-To: <SEZPR06MB52695281E21B27DB57A7B2FFF2AD9@SEZPR06MB5269.apcprd06.prod.outlook.com> Hi Ryan, > Sorry, Do you mean add in description like following?? > aspeed,xfer-mode: > description: | > I2C bus transfer mode selection. > ERRATA "I2C DMA fails when DRAM bus is busy and it can not > take DMA write data > Immediately", only 1 i2c bus can be enable for DMA mode. > - "byte": I2C bus byte transfer mode. > - "buffered": I2C bus buffer register transfer mode. > - "dma": I2C bus dma transfer mode (default) I would suggest putting some background about the transfer mode as a top-level description in the binding. There has been a lot of discussion here on why the binding specifies the transfer mode; it would be useful (for future readers) to have a bit of context on what modes they should use. Perhaps something like: description: | [general binding description] ASPEED ast2600 platforms have a number of i2c controllers, and share a single DMA engine between the set. DTSes can specify the mode of data transfer to/from the device - either DMA or programmed I/O - but hardware limitations may require a DTS to manually allocate which controller can use DMA mode; the enable-dma property allows control of this. In cases where one the hardware design results in a specific controller handling a larger amount of data, a DTS would likely allocate DMA mode for that one controller. - adjusted for whatever property interface we settle on here, of course. > > 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? > > If so, just leave enable-dma and only support for buffer mode and dma > mode, am I right? Yes, from what you have said so far, I think just a single switch between DMA / not-DMA is all you need here (unless there is any time that byte mode is preferable?) If there is already an existing DT convention for indicating/enabling DMA capability, I would suggest using that. Otherwise, just a boolean flag with a sensible name would seem to work fine. The DT experts probably have a good idea of what works best here :) 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: Wed, 01 Mar 2023 11:23:33 +0800 [thread overview] Message-ID: <c18b064b6b01bd547b2f03006dbf4bb6fdf9b91d.camel@codeconstruct.com.au> (raw) In-Reply-To: <SEZPR06MB52695281E21B27DB57A7B2FFF2AD9@SEZPR06MB5269.apcprd06.prod.outlook.com> Hi Ryan, > Sorry, Do you mean add in description like following?? > aspeed,xfer-mode: > description: | > I2C bus transfer mode selection. > ERRATA "I2C DMA fails when DRAM bus is busy and it can not > take DMA write data > Immediately", only 1 i2c bus can be enable for DMA mode. > - "byte": I2C bus byte transfer mode. > - "buffered": I2C bus buffer register transfer mode. > - "dma": I2C bus dma transfer mode (default) I would suggest putting some background about the transfer mode as a top-level description in the binding. There has been a lot of discussion here on why the binding specifies the transfer mode; it would be useful (for future readers) to have a bit of context on what modes they should use. Perhaps something like: description: | [general binding description] ASPEED ast2600 platforms have a number of i2c controllers, and share a single DMA engine between the set. DTSes can specify the mode of data transfer to/from the device - either DMA or programmed I/O - but hardware limitations may require a DTS to manually allocate which controller can use DMA mode; the enable-dma property allows control of this. In cases where one the hardware design results in a specific controller handling a larger amount of data, a DTS would likely allocate DMA mode for that one controller. - adjusted for whatever property interface we settle on here, of course. > > 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? > > If so, just leave enable-dma and only support for buffer mode and dma > mode, am I right? Yes, from what you have said so far, I think just a single switch between DMA / not-DMA is all you need here (unless there is any time that byte mode is preferable?) If there is already an existing DT convention for indicating/enabling DMA capability, I would suggest using that. Otherwise, just a boolean flag with a sensible name would seem to work fine. The DT experts probably have a good idea of what works best here :) 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-03-01 3:23 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 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 [this message] 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=c18b064b6b01bd547b2f03006dbf4bb6fdf9b91d.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.