RE: [PATCH v20 1/4] dt-bindings: i2c: Split AST2600 binding into a new YAML

Ryan Chen
2025-11-13T09:35:08Z

> Subject: RE: [PATCH v20 1/4] dt-bindings: i2c: Split AST2600 binding into a new
> YAML
>
> > Subject: Re: [PATCH v20 1/4] dt-bindings: i2c: Split AST2600 binding
> > into a new YAML
> >
> > On 29/10/2025 09:29, Ryan Chen wrote:
> > >> Subject: Re: [PATCH v20 1/4] dt-bindings: i2c: Split AST2600
> > >> binding into a new YAML
> > >>
> > >> On 21/10/2025 03:35, Ryan Chen wrote:
> > >>> The AST2600 I2C controller is a new hardware design compared to
> > >>> the I2C controllers in previous ASPEED SoCs (e.g., AST2400, AST2500).
> > >>>
> > >>> It introduces new features such as:
> > >>> - A redesigned register layout
> > >>> - Separation between controller and target mode registers
> > >>> - Transfer mode selection (byte, buffer, DMA)
> > >>> - Support for a shared global register block for configuration
> > >>>
> > >>> Due to these fundamental differences, maintaining a separate
> > >>> devicetree binding file for AST2600 helps to clearly distinguish
> > >>> the hardware capabilities and configuration options from the older
> > >>> controllers.
> > >>>
> > >>> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> > >>> ---
> > >>> .../devicetree/bindings/i2c/aspeed,i2c.yaml | 3 +-
> > >>> .../devicetree/bindings/i2c/ast2600-i2c.yaml | 66
> > >>> +++++++++++++++++++
> > >>> 2 files changed, 67 insertions(+), 2 deletions(-) create mode
> > >>> 100644 Documentation/devicetree/bindings/i2c/ast2600-i2c.yaml
> > >>>
> > >>> diff --git a/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
> > >>> b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
> > >>> index 5b9bd2feda3b..d4e4f412feba 100644
> > >>> --- a/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
> > >>> +++ b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
> > >>> @@ -4,7 +4,7 @@
> > >>> $id: http://devicetree.org/schemas/i2c/aspeed,i2c.yaml#
> > >>> $schema: http://devicetree.org/meta-schemas/core.yaml#
> > >>>
> > >>> -title: ASPEED I2C on the AST24XX, AST25XX, and AST26XX SoCs
> > >>> +title: ASPEED I2C on the AST24XX, AST25XX SoCs
> > >>>
> > >>> maintainers:
> > >>> - Rayn Chen <rayn_chen@aspeedtech.com> @@ -17,7 +17,6 @@
> > >>> properties:
> > >>> enum:
> > >>> - aspeed,ast2400-i2c-bus
> > >>> - aspeed,ast2500-i2c-bus
> > >>> - - aspeed,ast2600-i2c-bus
> > >>>
> > >>> reg:
> > >>> minItems: 1
> > >>> diff --git
> > >>> a/Documentation/devicetree/bindings/i2c/ast2600-i2c.yaml
> > >>> b/Documentation/devicetree/bindings/i2c/ast2600-i2c.yaml
> > >>
> > >> Why completely breaking naming? Please follow writing bindings
> carefully.
> > >
> > > Will update
> > > $id: "http://devicetree.org/schemas/i2c/aspeed,ast2600-i2c.yaml#"
> > >>
> > >>> new file mode 100644
> > >>> index 000000000000..6ddcec5decdc
> > >>> --- /dev/null
> > >>> +++ b/Documentation/devicetree/bindings/i2c/ast2600-i2c.yaml
> > >>> @@ -0,0 +1,66 @@
> > >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML
> > >>> +1.2
> > >>> +---
> > >>> +$id: http://devicetree.org/schemas/i2c/ast2600-i2c.yaml#
> > >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > >>> +
> > >>> +title: ASPEED I2C on the AST26XX SoCs
> > >>> +
> > >>> +maintainers:
> > >>> + - Ryan Chen <ryan_chen@aspeedtech.com>
> > >>> +
> > >>> +allOf:
> > >>> + - $ref: /schemas/i2c/i2c-controller.yaml#
> > >>> +
> > >>> +properties:
> > >>> + compatible:
> > >>> + enum:
> > >>> + - aspeed,ast2600-i2c-bus
> > >>> +
> > >>> + reg:
> > >>> + minItems: 1
> > >>
> > >> Why?
> > >
> > > Will update as following.
> > >
> > > reg:
> > > minItems: 1
> > > maxItems: 2
> >
> >
> > No. You changed nothing. Instead explain why this is flexible.
> >
> > See writing bindings.
>
> Sorry, I still not understand your point. Do you mean need to explain why reg is
> flexible 1 -> 2?
> If yes, I will update to following.
>
> reg:
> minItems: 1
> maxItems: 2
> description:
> The first region covers the controller registers.
> The optional second region covers the controller's buffer space.

After check the
https://docs.kernel.org/devicetree/bindings/writing-schema.html#annotated-example-schema
I think I should update with following, am I correct ?

reg:
items:
- description: The first region covers the controller registers.
- description: The optional second region covers the controller's buffer space.

What you question about
" Please explain me how one, same SoC has optional IO address space? I asked to explain WHY this is flexible"
The AST2600 i2c controller have three io,buffer,dma mode.
The AST2600 have buffer register for buffer transfer. That is 2nd reg offset.
If dtsi not descript it, the driver will go back to io mode transfer. Flexible implement is in driver.

"

>
>
> >
> >
> > ...
> >
> >
> > >>> + bus-frequency:
> > >>> + minimum: 500
> > >>> + maximum: 4000000
> > >>> + default: 100000
> > >>> + description: frequency of the bus clock in Hz defaults to 100
> > >>> + kHz when
> > >> not
> > >>> + specified
> > >>
> > >> Don't repeat constraints in free form text.
> > >
> > > Will update
> > > clock-frequency:
> > > description: Desired I2C bus frequency in Hz
> > > default: 100000
> >
> > Heh? You are making random set of changes like did not really read the
> > feedback. I not to repeat something. What is repeated? Constraints.
> > Where are the repeated "in free form text". What did you do? Dropped
> > constraints.
>
> Sorry, I misunderstood your comment,
> I think you are point out my description.
>
> I will update with following.
>
> clock-frequency:
> description: Desired operating frequency of the I2C bus in Hz.
> minimum: 500
> maximum: 4000000
> default: 100000
>
> >
> > I don't know what to say.
> >
> > Best regards,
> > Krzysztof