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

[Ryan Chen]

Hello Krzysztof,
	Thanks the review.

> Subject: Re: [PATCH v25 1/4] dt-bindings: i2c: Split AST2600 binding into a new
> YAML
> 
> On Wed, Feb 25, 2026 at 05:19:38PM +0800, Ryan Chen wrote:
> > The AST2600 I2C controller introduces a completely new register layout
> > with separate controller and target register blocks, unlike the mixed
> > register layout used by AST2400/AST2500.
> >
> > To describe this properly, split out the AST2600 I2C binding into its
> > own YAML file. The compatible string remains unchanged.
> 
> But you made other changes in the binding. You must list them, because
> otherwise it sounds like you only SPLIT. It's not true. You actually changed the
> binding in at least two places, maybe more.

I will update with following.

Move AST2600 I2C binding from aspeed,i2c.yaml to a dedicated
aspeed,ast2600-i2c.yaml schema.

Besides the split, this also adjusts AST2600-specific constraints:
- require two reg regions (controller register block + buffer block)
- use clock-frequency for bus speed description
- require interrupts for AST2600 instances

No compatible strings are changed.

> 
> >
> > The example section is updated to reflect the actual AST2600 SoC
> > register layout and interrupt configuration (aspeed-g6.dtsi, lines
> > 885-897):
> >
> > - I2C bus and buffer register offsets
> >   - AST2600 I2C controller register base starts at 0x80, and the
> >     buffer region is located at 0xc00, per the AST2600 SoC register map.
> >
> > - Interrupt configuration
> >   - AST2600 I2C controllers are connected to the ARM GIC, not the legacy
> >     internal interrupt controller.
> 
> Example is irrelevant, don't mention it. We discuss here binding.

OK~ I will remove example in commit message/
> 
> >
> > Signed-off-by: Ryan Chen <ryan_chen@xxxxxxxxxxxxxx>
> > ---
> >  .../bindings/i2c/aspeed,ast2600-i2c.yaml           | 62
> ++++++++++++++++++++++
> >  .../devicetree/bindings/i2c/aspeed,i2c.yaml        |  3 +-
> >  2 files changed, 63 insertions(+), 2 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
> > b/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
> > new file mode 100644
> > index 000000000000..077be85137c9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
> > @@ -0,0 +1,62 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/i2c/aspeed,ast2600-i2c.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ASPEED I2C on the AST26XX SoCs
> 
> s/26XX/2600/ probably
Will update
title: ASPEED I2C on the AST2600 SoCs
> 
> > +
> > +maintainers:
> > +  - Ryan Chen <ryan_chen@xxxxxxxxxxxxxx>
> > +
> > +allOf:
> > +  - $ref: /schemas/i2c/i2c-controller.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - aspeed,ast2600-i2c-bus
> > +
> > +  reg:
> > +    items:
> > +      - description: controller registers
> > +      - description: controller buffer space
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-frequency:
> > +    description: Desired operating frequency of the I2C bus in Hz.
> > +    minimum: 500
> > +    maximum: 4000000
> > +    default: 100000
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +required:
> > +  - reg
> > +  - compatible
> > +  - clocks
> > +  - resets
> > +  - interrupts
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/aspeed-clock.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    i2c@80 {
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> 
> Please follow DTS coding style.

I will update with following.
    i2c@80 {
        compatible = "aspeed,ast2600-i2c-bus";
        reg = <0x80 0x80>, <0xc00 0x20>;
        #address-cells = <1>;
        #size-cells = <0>;
> 
> Best regards,
> Krzysztof