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

[Krzysztof Kozlowski <krzk@kernel.org>]

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