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

[Krzysztof Kozlowski <krzk@kernel.org>]

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.

> 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?

> +    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

That's not even correct.Either root clock or APB clock, you cannot have
both. Unless "root clock" is not "clock" but then it is just confusing.
Drop description and fix the constraints.

> +
> +  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

Don't repeat constraints in free form text.

> +
> +required:
> +  - reg
> +  - compatible



Best regards,
Krzysztof