Re: [PATCH 1/2] dt-bindings: interconnect: Add Qualcomm CCI dt-bindings

[Krzysztof Kozlowski <krzk@kernel.org>]

On 03/02/2023 04:45, Jun Nie wrote:
> Krzysztof Kozlowski <krzk@kernel.org> 于2023年2月2日周四 17:42写道：
>>
>> On 02/02/2023 10:29, Jun Nie wrote:
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    enum:
>>>>> +      - qcom,msm8939-cci
>>>>> +
>>>>> +  clocks:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  operating-points-v2: true
>>>>> +  opp-table:
>>>>> +    type: object
>>>>> +
>>>>> +required:
>>>>> +  - compatible
>>>>> +  - clocks
>>>>> +  - operating-points-v2
>>>>> +  - nvmem-cells
>>>>
>>>> ?? You cannot require properties which are not present.
>>>>
>>>>> +  - power-domains
>>>>
>>>> Same here.
>>>>
>>>
>>> So the properties should be added later, after cpr and fuse nodes are
>>> available in mainline, right?
>>
>> No, binding should be complete, so why would you skip some properties? I
>> don't see here dependency on other bindings. Unless I missed here
>> something and there is a dependency? But then what kind? Do you
>> reference other schema?
>>
>> Best regards,
>> Krzysztof
>>
> Sorry, it is a copy/modified error with overlooking some added properties.
> 
> +        cci_opp3: opp-cpr14-400000000 {
> +            opp-hz = /bits/ 64 <400000000>;
> +            opp-supported-hw = <0x1>;
> +            required-opps = <&cpr_opp14>;
> +        };
> +
> +        cci_opp4: opp-cpr15-400000000 {

This does not answer my concerns at all. Please include all relevant
properties in your bindings.

> Documentation/devicetree/bindings/interconnect/qcom,cci.example.dtb:
> opp-table-cci: Unevaluated properties are not allowed
> ('opp-cpr14-400000000', 'opp-cpr15-400000000' were unexpected)
> 
> Do you know how to fix this dts check warning?

Don't add properties which are not allowed or do not name properties in
a way it is not allowed.

> The cci_opp3 and cci_opp4 have the same frequency but with different
> requirements to

So you used the same node names... how do you expect such code to
compile? In current form bindings do not allow such setup. If you think
that OS should allow and support it, then change the bindings.


Best regards,
Krzysztof