1. Patchew
  2. linux
  3. [PATCH v21 0/4] Add ASPEED AST2600 I2C controller driver
  4. View patch

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

Ryan Chen posted 4 patches 3 months, 1 week ago
There is a newer version of this series
[PATCH v21 1/4] dt-bindings: i2c: Split AST2600 binding into a new YAML
Posted by Ryan Chen 3 months, 1 week ago 
The AST2600 I2C controller introduces a completely new register
map and Separate control/target register sets, unlike the mixed
layout used in AST2400/AST2500.

In addition, at new AST2600 configuration registers and transfer
modes require new DT properties, which are incompatible with
existing bindings. Therefore, this patch creates a dedicated
binding file for AST2600 to properly describe these new
hardware capabilities.

A subsequent change will modify this new binding to properly
describe the AST2600 hardware.

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
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
+    items:
+      - description: address offset and range of bus
+      - description: address offset and range of bus buffer
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+    description:
+      root clock of bus, should reference the APB
+      clock in the second cell
+
+  resets:
+    maxItems: 1
+
+  bus-frequency:
+    minimum: 500
+    maximum: 4000000
+    default: 100000
+    description: frequency of the bus clock in Hz defaults to 100 kHz when not
+      specified
+
+required:
+  - reg
+  - compatible
+  - clocks
+  - resets
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/aspeed-clock.h>
+    i2c@40 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      compatible = "aspeed,ast2600-i2c-bus";
+      reg = <0x40 0x40>;
+      clocks = <&syscon ASPEED_CLK_APB>;
+      resets = <&syscon ASPEED_RESET_I2C>;
+      bus-frequency = <100000>;
+      interrupts = <0>;
+      interrupt-parent = <&i2c_ic>;
+    };
-- 
2.34.1
Re: [PATCH v21 1/4] dt-bindings: i2c: Split AST2600 binding into a new YAML
Posted by Krzysztof Kozlowski 3 months, 1 week ago 
On 27/10/2025 07:12, Ryan Chen wrote:
> The AST2600 I2C controller introduces a completely new register
> map and Separate control/target register sets, unlike the mixed
> layout used in AST2400/AST2500.
> 
> In addition, at new AST2600 configuration registers and transfer
> modes require new DT properties, which are incompatible with
> existing bindings. Therefore, this patch creates a dedicated
> binding file for AST2600 to properly describe these new
> hardware capabilities.
> 
> A subsequent change will modify this new binding to properly
> describe the AST2600 hardware.
> 
> 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
> 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

<form letter>
This is a friendly reminder during the review process.

It seems my or other reviewer's previous comments were not fully
addressed. Maybe the feedback got lost between the quotes, maybe you
just forgot to apply it. Please go back to the previous discussion and
either implement all requested changes or keep discussing them.

Thank you.
</form letter>

> +    items:
> +      - description: address offset and range of bus
> +      - description: address offset and range of bus buffer
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1

Nothing improved

> +    description:
> +      root clock of bus, should reference the APB
> +      clock in the second cell

Really, you just ignored all the comments.

No wonder this is v21 and there will be more. Please re-think what is
going wrong in this process that you need 22 or more revisions.

Best regards,
Krzysztof
Re: [PATCH v21 1/4] dt-bindings: i2c: Split AST2600 binding into a new YAML
Posted by Jeremy Kerr 3 months, 1 week ago 
Hi Kyzysztof,

> > +++ 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
> 
> <form letter>
> This is a friendly reminder during the review process.
> 
> It seems my or other reviewer's previous comments were not fully
> addressed. Maybe the feedback got lost between the quotes, maybe you
> just forgot to apply it. Please go back to the previous discussion
> and
> either implement all requested changes or keep discussing them.
> 
> Thank you.
> </form letter>
> 
> > +    items:
> > +      - description: address offset and range of bus
> > +      - description: address offset and range of bus buffer
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> 
> Nothing improved

That was mostly the point - this first patch just splits out the 2600
definitions to the new file, with zero change.

That means the *actual* changes to the binding are visible via the diff
in 2/4, and not hidden by the copy.

This was mentioned on v20, and you replied saying it was irrelevant to
the separate discussion around the rationale for the change, but didn't
object to the split-patches approach.

If your preference is to *not* do this via a verbatim copy as an initial
step (and essentially squash with 2/4), that's also fine, but I'm sure
that knowing your preference would help Ryan out here.

Cheers,


Jeremy
Re: [PATCH v21 1/4] dt-bindings: i2c: Split AST2600 binding into a new YAML
Posted by Krzysztof Kozlowski 3 months ago 
On 30/10/2025 07:04, Jeremy Kerr wrote:
> Hi Kyzysztof,
> 
>>> +++ 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
>>
>> <form letter>
>> This is a friendly reminder during the review process.
>>
>> It seems my or other reviewer's previous comments were not fully
>> addressed. Maybe the feedback got lost between the quotes, maybe you
>> just forgot to apply it. Please go back to the previous discussion
>> and
>> either implement all requested changes or keep discussing them.
>>
>> Thank you.
>> </form letter>
>>
>>> +    items:
>>> +      - description: address offset and range of bus
>>> +      - description: address offset and range of bus buffer
>>> +
>>> +  interrupts:
>>> +    maxItems: 1
>>> +
>>> +  clocks:
>>> +    maxItems: 1
>>
>> Nothing improved
> 
> That was mostly the point - this first patch just splits out the 2600
> definitions to the new file, with zero change.
> 
> That means the *actual* changes to the binding are visible via the diff
> in 2/4, and not hidden by the copy.
> 
> This was mentioned on v20, and you replied saying it was irrelevant to
> the separate discussion around the rationale for the change, but didn't
> object to the split-patches approach.
> 
> If your preference is to *not* do this via a verbatim copy as an initial
> step (and essentially squash with 2/4), that's also fine, but I'm sure
> that knowing your preference would help Ryan out here.


The next patch did not correct issues copied from old binding, so above
arguments are not applicable.

I did not ask to merge the patches. I asked not to create WRONG schema
when copying to the new file. This split should not be a verbatim copy,
because we do not create intentionally buggy code which we are going to
fix immediately. Also it does not make sense to make verbatim copy of
ast2500 stuff, since new file is ONLY ast2600.

That copy should include all necessary changes needed to make new
binding correct. I already pointed out this and this was not fixed -
neither here nor in a following commit (which I would still ask to squash).

Best regards,
Krzysztof

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.