Devicetree
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: Billy Tsai <billy_tsai@aspeedtech.com>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>,
	 Frank Li <Frank.Li@nxp.com>, Rob Herring <robh@kernel.org>,
	 Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	 Joel Stanley <joel@jms.id.au>,
	Andrew Jeffery <andrew@codeconstruct.com.au>,
	 Philipp Zabel <p.zabel@pengutronix.de>,
	linux-i3c@lists.infradead.org, devicetree@vger.kernel.org,
	 linux-arm-kernel@lists.infradead.org,
	linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/8] dt-bindings: i3c: Document the AST2700 I3C controller
Date: Mon, 7 Sep 2026 08:28:38 +0200	[thread overview]
Message-ID: <20260907-imaginary-economic-oyster-57cdd6@quoll> (raw)
In-Reply-To: <20260901-b4-i3c-hci-ast2700-v1-1-19909e7cbd7e@aspeedtech.com>

On Tue, Sep 01, 2026 at 07:35:28PM +0800, Billy Tsai wrote:
> The AST2700 I3C controller implements the MIPI I3C HCI programming
> model, extended with an ASPEED in-house control block and a PHY
> programming window advertised through an HCI vendor extended
> capability. Follow the existing aspeed,ast2600-i3c precedent of
> describing SoC integrations of a common I3C IP in their own schema.

What precedent exactly?

Are you now bringing some LLM slop arguments here?

> 
> The controller takes a core clock for PHY timing and two reset-names
> lines: its own reset and a DMA engine reset shared by all I3C
> controllers on the SoC.
> 
> The PHY timings derived from the core clock and bus rate assume
> nominal bus loading. Boards with long traces, weak pull-ups, or
> capacitive loading need different ratios and setup periods, so
> document properties to override the push-pull and open-drain SCL
> periods, the SDA TX hold time, the tCAS/tCBP periods, and the SCL low
> period used for a repeated START or STOP.
> 
> Boards without external I3C pull-up resistors can use the PHY's
> built-in pull-ups instead. The achievable resistance depends on the
> voltage domain of the pins (high-voltage vs low-voltage), which can be
> determined from the pin group assigned to the bus. Reuse bias-pull-up
> (pincfg-node.yaml) for the strength in Ohms; it is declared on this
> node rather than a pin-config node since the pull-up switch is part of
> the I3C PHY, not the broader SoC pin control IPs.
> 
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> Assisted-by: Claude:claude-fable-5
> 
> ---
> We considered formally expressing bias-pull-up's dependency on a
> pinctrl state in the schema, but couldn't find a clean way to say
> "depends on whichever pinctrl-N is active" without hard-coding
> pinctrl-0 specifically, which would be wrong in general (a node's
> active pin state doesn't have to sit at index 0). We've added
> pinctrl-0 to the example to illustrate the typical usage instead,
> without a formal schema-level dependency.
> ---
>  .../bindings/i3c/aspeed,ast2700-i3c-hci.yaml       | 133 +++++++++++++++++++++
>  1 file changed, 133 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i3c/aspeed,ast2700-i3c-hci.yaml b/Documentation/devicetree/bindings/i3c/aspeed,ast2700-i3c-hci.yaml
> new file mode 100644
> index 000000000000..19ecb5b15038
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i3c/aspeed,ast2700-i3c-hci.yaml
> @@ -0,0 +1,133 @@
> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i3c/aspeed,ast2700-i3c-hci.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ASPEED AST2700 I3C controller
> +
> +maintainers:
> +  - Billy Tsai <billy_tsai@aspeedtech.com>
> +
> +description:
> +  The AST2700 I3C controller implements the MIPI I3C HCI programming model,
> +  extended with an ASPEED in-house control block and a PHY programming window
> +  that are advertised through an HCI vendor extended capability.
> +
> +properties:
> +  compatible:
> +    const: aspeed,ast2700-i3c-hci
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  resets:
> +    items:
> +      - description: Core controller reset.
> +      - description: DMA engine reset shared by all I3C controllers.
> +
> +  reset-names:
> +    items:
> +      - const: core
> +      - const: dma
> +
> +  bias-pull-up:
> +    $ref: /schemas/pinctrl/pincfg-node.yaml#/properties/bias-pull-up
> +    enum: [177, 231, 261, 317, 400, 470, 550, 600, 750]
> +    description:
> +      Enable the PHY-internal pull-ups on SCL and SDA and select their
> +      strength, in Ohms. High-voltage pins support 400, 470 or 600 Ohms,
> +      and low-voltage pins support 177, 231, 261, 317, 400, 550 or 750
> +      Ohms. When absent the internal pull-ups stay disabled and the bus
> +      relies on external pull-ups.
> +
> +  aspeed,pp-scl-high-ns:
> +    description:
> +      Push-pull SCL high period. Overrides the value derived from the
> +      i3c-scl-hz bus rate.
> +
> +  aspeed,pp-scl-low-ns:
> +    description:
> +      Push-pull SCL low period. Overrides the value derived from the
> +      i3c-scl-hz bus rate.
> +
> +  aspeed,od-scl-high-ns:
> +    default: 380
> +    description:
> +      Open-drain SCL high period.
> +
> +  aspeed,od-scl-low-ns:
> +    default: 620
> +    description:
> +      Open-drain SCL low period.
> +
> +  aspeed,sda-tx-hold-ns:
> +    default: 10
> +    description:
> +      SDA output hold time relative to the SCL edge.
> +
> +  aspeed,cas-ns:
> +    minimum: 39
> +    description:
> +      Clock-after-start (tCAS) period driven after a START condition.
> +      When absent it is derived from the SCL low period of the
> +      applicable bus context, clamped to the protocol minimum.
> +
> +  aspeed,cbp-ns:
> +    minimum: 20
> +    description:
> +      Clock-before-stop (tCBP) period driven before a STOP condition.
> +      When absent it is derived from the SCL low period of the
> +      applicable bus context, clamped to the protocol minimum.
> +
> +  aspeed,sr-p-scl-low-ns:
> +    default: 40
> +    description:
> +      SCL low period used while preparing a repeated START or STOP
> +      condition, kept separate from the push-pull SCL low period to
> +      satisfy the MIPI I3C clock stalling requirements.

Probably I3C maintainers should weight in here and help - should these
be made generic for all I3C controllers? I am surprised some "standard"
I3C controller needs to configure this as the single one. No other ever
needed...

Best regards,
Krzysztof


  parent reply	other threads:[~2026-09-07  6:28 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-01 11:35 [PATCH 0/8] i3c: mipi-i3c-hci: Add Aspeed AST2700 support Billy Tsai
2026-09-01 11:35 ` [PATCH 1/8] dt-bindings: i3c: Document the AST2700 I3C controller Billy Tsai
2026-09-01 20:41   ` Frank Li
2026-09-09  5:33     ` 回覆: " Billy Tsai
2026-09-07  6:28   ` Krzysztof Kozlowski [this message]
2026-09-09  5:35     ` Billy Tsai
2026-09-01 11:35 ` [PATCH 2/8] i3c: mipi-i3c-hci: Support address-indexed DAT slots Billy Tsai
2026-09-01 11:49   ` sashiko-bot
2026-09-01 20:47   ` Frank Li
2026-09-09  5:36     ` 回覆: " Billy Tsai
2026-09-01 11:35 ` [PATCH 3/8] i3c: mipi-i3c-hci: Add a quirk for 64-bit DMA addressing Billy Tsai
2026-09-01 11:55   ` sashiko-bot
2026-09-01 20:51   ` Frank Li
2026-09-09  5:37     ` 回覆: " Billy Tsai
2026-09-01 11:35 ` [PATCH 4/8] i3c: mipi-i3c-hci: Add a quirk to clear the TX start threshold Billy Tsai
2026-09-01 11:49   ` sashiko-bot
2026-09-01 20:58   ` Frank Li
2026-09-09  5:37     ` 回覆: " Billy Tsai
2026-09-01 11:35 ` [PATCH 5/8] i3c: mipi-i3c-hci: Add support for the AST2700 I3C controller Billy Tsai
2026-09-01 11:52   ` sashiko-bot
2026-09-01 21:18   ` Frank Li
2026-09-09  5:38     ` 回覆: " Billy Tsai
2026-09-01 11:35 ` [PATCH 6/8] i3c: mipi-i3c-hci: Program AST2700 IBI termination threshold Billy Tsai
2026-09-01 11:35 ` [PATCH 7/8] i3c: mipi-i3c-hci: Improve AST2700 PIO TX queue utilization Billy Tsai
2026-09-01 11:51   ` sashiko-bot
2026-09-01 11:35 ` [PATCH 8/8] i3c: mipi-i3c-hci: Support the AST2700 internal pull-ups Billy Tsai

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=20260907-imaginary-economic-oyster-57cdd6@quoll \
    --to=krzk@kernel.org \
    --cc=Frank.Li@nxp.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=andrew@codeconstruct.com.au \
    --cc=billy_tsai@aspeedtech.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=joel@jms.id.au \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-aspeed@lists.ozlabs.org \
    --cc=linux-i3c@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=p.zabel@pengutronix.de \
    --cc=robh@kernel.org \
    /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