Re: [PATCH v3 1/4] dt-bindings: clock: airoha: Add reset support to EN7581 clock binding

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

Il 27/06/24 11:47, Conor Dooley ha scritto:
> On Thu, Jun 27, 2024 at 11:33:47AM +0200, AngeloGioacchino Del Regno wrote:
>> Il 13/06/24 14:47, Lorenzo Bianconi ha scritto:
>>> Introduce reset capability to EN7581 device-tree clock binding
>>> documentation.
>>>
>>> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
>>> ---
>>>    .../bindings/clock/airoha,en7523-scu.yaml     | 25 ++++++-
>>>    .../dt-bindings/reset/airoha,en7581-reset.h   | 66 +++++++++++++++++++
>>>    2 files changed, 90 insertions(+), 1 deletion(-)
>>>    create mode 100644 include/dt-bindings/reset/airoha,en7581-reset.h
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
>>> index 3f4266637733..84353fd09428 100644
>>> --- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
>>> +++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
>>> @@ -35,7 +35,7 @@ properties:
>>>      reg:
>>>        minItems: 2
>>> -    maxItems: 3
>>> +    maxItems: 4
>>>      "#clock-cells":
>>>        description:
>>> @@ -43,6 +43,10 @@ properties:
>>>          clocks.
>>>        const: 1
>>> +  '#reset-cells':
>>> +    description: ID of the controller reset line
>>> +    const: 1
>>> +
>>>    required:
>>>      - compatible
>>>      - reg
>>> @@ -60,6 +64,8 @@ allOf:
>>>                - description: scu base address
>>>                - description: misc scu base address
>>> +        '#reset-cells': false
>>> +
>>>      - if:
>>>          properties:
>>>            compatible:
>>> @@ -70,6 +76,7 @@ allOf:
>>>              items:
>>>                - description: scu base address
>>>                - description: misc scu base address
>>> +            - description: reset base address
>>
>> Are you sure that the indentation is correct? :-)
>>
>> After fixing the indentation,
>>
>> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>>
>>>                - description: pb scu base address
> 
> The indentation actually looks okay when I apply this locally, but how is
> it backwards compatible to add this register in the middle of the list??

It's not, and this is actually something done on purpose - there is no DT using
this binding yet (here, nor uboot), and Lorenzo acknowledged the mistake before
it was too late...

At least this time, it wasn't a misattention :-P

Btw, as far as I know, the reset base address is in between misc scu and pb scu,
that's why it was put there in the middle.

Cheers!
Angelo