Re: [PATCH 1/3] dt-bindings: clk: sprd: Add UMS9620 support

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 04/01/2024 10:09, Chunyan Zhang wrote:
> Hi Krzysztof,
> 
> On Thu, 4 Jan 2024 at 16:46, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 29/12/2023 09:51, Chunyan Zhang wrote:
>>> Add UMS9620's clock compatible strings and descriptions for clock
>>> and clock-names for UMS9620.
>>>
>>> Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
>>> ---
>>>  .../bindings/clock/sprd,ums512-clk.yaml       | 79 ++++++++++++++++---
>>>  1 file changed, 68 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/sprd,ums512-clk.yaml b/Documentation/devicetree/bindings/clock/sprd,ums512-clk.yaml
>>> index 43d2b6c31357..dcad41a1fea7 100644
>>> --- a/Documentation/devicetree/bindings/clock/sprd,ums512-clk.yaml
>>> +++ b/Documentation/devicetree/bindings/clock/sprd,ums512-clk.yaml
>>> @@ -30,25 +30,38 @@ properties:
>>>        - sprd,ums512-mm-clk
>>>        - sprd,ums512-mm-gate-clk
>>>        - sprd,ums512-apapb-gate
>>> +      - sprd,ums9620-pmu-gate
>>> +      - sprd,ums9620-g1-pll
>>> +      - sprd,ums9620-g5l-pll
>>> +      - sprd,ums9620-g5r-pll
>>> +      - sprd,ums9620-g8-pll
>>> +      - sprd,ums9620-g9-pll
>>> +      - sprd,ums9620-g10-pll
>>> +      - sprd,ums9620-apapb-gate
>>> +      - sprd,ums9620-ap-clk
>>> +      - sprd,ums9620-apahb-gate
>>> +      - sprd,ums9620-aon-gate
>>> +      - sprd,ums9620-aonapb-clk
>>> +      - sprd,ums9620-topdvfs-clk
>>> +      - sprd,ums9620-ipaapb-gate
>>> +      - sprd,ums9620-ipa-clk
>>> +      - sprd,ums9620-ipaglb-gate
>>> +      - sprd,ums9620-ipadispc-gate
>>> +      - sprd,ums9620-pcieapb-gate
>>> +      - sprd,ums9620-pcie-clk
>>> +      - sprd,ums9620-mm-gate
>>> +      - sprd,ums9620-mm-clk
>>> +      - sprd,ums9620-dpu-vsp-gate
>>> +      - sprd,ums9620-dpu-vsp-clk
>>>
>>>    "#clock-cells":
>>>      const: 1
>>>
>>>    clocks:
>>> -    minItems: 1
>>> -    maxItems: 4
>>
>> No, constraints stay here.
>>
>>>      description: |
>>>        The input parent clock(s) phandle for the clock, only list
>>>        fixed clocks which are declared in devicetree.
>>>
>>> -  clock-names:
>>
>> No, you cannot just drop properties. Widest constraints stay here.
>>
>>> -    minItems: 1
>>> -    items:
>>> -      - const: ext-26m
>>> -      - const: ext-32k
>>> -      - const: ext-4m
>>> -      - const: rco-100m
>>> -
>>>    reg:
>>>      maxItems: 1
>>>
>>> @@ -57,7 +70,43 @@ required:
>>>    - '#clock-cells'
>>>    - reg
>>>
>>> -additionalProperties: false
>>> +allOf:
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            pattern: "^sprd,ums512-.*"
>>> +
>>> +    then:
>>> +      properties:
>>> +        clocks:
>>> +          minItems: 1
>>> +          maxItems: 4
>>> +
>>> +        clock-names:
>>> +          minItems: 1
>>> +          items:
>>> +            - const: ext-26m
>>> +            - const: ext-32k
>>> +            - const: ext-4m
>>> +            - const: rco-100m
>>> +
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            pattern: "^sprd,ums9620-.*"
>>> +    then:
>>> +      properties:
>>> +        clocks:
>>> +          minItems: 1
>>> +          maxItems: 6
>>> +
>>> +        clock-names:
>>> +          minItems: 1
>>> +          maxItems: 6
>>
>> Missing names.
> 
> The names are fixed, but the order are not.

Order must be fixed.

> 
> For example:
> clk_a {
>     clocks = <&ext_26m>, <&ext_32k>;
>     clock-names = "ext-26m", "ext-32k";
> };
> 
> clk_b {
>     clocks = <&ext_26m>, <&ext_4m>;
>     clock-names = "ext-26m", "ext-4m";

And here the order is fixed...

> };
> 
> How to list the names for this kind of case? (Please forgive me for
> being lazy, didn't find a similar case after a fast search)

The same ums512 lists them or mentioned other bindings. You somehow cut
the quote...

Best regards,
Krzysztof