[EXT] Re: [PATCH v5 4/8] dt-bindings: usb: ci-hdrc-usb2: add restrictions for reg, interrupts, clock and clock-names properties

[Krzysztof Kozlowski]

On 02/02/2024 10:10, Xu Yang wrote:
> Hi Krzysztof,
> 
>>
>> On 31/01/2024 12:43, Xu Yang wrote:
>>> Change reg, interrupts, clock and clock-names as common properties and add
>>> restrictions on them for different compatibles.
>>>
>>> Signed-off-by: Xu Yang <xu.yang_2 at nxp.com>
>>>
>>> ---
>>> Changes in v4:
>>>  - new patch since v3's discussion
>>>  - split the reg, interrupts, clock and clock-names properties into
>>>    common part and device-specific
>>> Changes in v5:
>>>  - keep common property unchanged
>>>  - make if-then more readable
>>>  - remove non imx part
>>> ---
>>>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 118 ++++++++++++++++++
>>>  1 file changed, 118 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-
>> hdrc-usb2.yaml
>>> index 3b56e0edb1c6..6ad3582051b8 100644
>>> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
>>> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
>>> @@ -412,6 +412,124 @@ allOf:
>>>          samsung,picophy-pre-emp-curr-control: false
>>>          samsung,picophy-dc-vol-level-adjust: false
>>>
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          const: fsl,imx27-usb
>>> +    then:
>>> +      properties:
>>> +        clocks:
>>> +          minItems: 3
>>> +          maxItems: 3
>>> +        clock-names:
>>> +          minItems: 3
>>> +          maxItems: 3
>>> +          items:
>>> +            anyOf:
>>> +              - const: ipg
>>> +              - const: ahb
>>> +              - const: per
>>
>> This would be just: enum: [ipg, ahb, per], but in both cases I question
>> why the order should be flexible? Nothing in commit msg explains it.
> 
> The driver will get the clock by clock-name, then the order should not
> matter. However, these three clock-names should be present at the same
> time. I should use enum then.
> 
>>
>> Plus I will repeat myself from your v4. I don't think this is helping,
>> because the file will soon grow to umnanageable chunk. I prefer to fix
>> it at beginning, before we reach snps-schema level of complexities.
>>
>> Please define common schema, reference in this file and move IMX to own
>> file.
> 
> I'm not that familiar with dt-bindings architecture. If I define a common
> schema, then should I create imx, qcom, nvidia and other dt-binding files
> too? 

No, the rest you can leave here. Someone, maybe me, will move them some
time. The point is to move at least IMX to its own file.

Best regards,
Krzysztof