[EXT] Re: [PATCH v3 4/6] dt-bindings: usb: ci-hdrc-usb2: add compatible and clock-names restriction for imx93

[Krzysztof Kozlowski]

On 16/01/2024 08:49, Xu Yang wrote:
> Hi Krzysztof,
> 
>>
>> On 12/01/2024 12:17, Xu Yang wrote:
>>> The i.MX93 needs a wakup clock to work properly. This will add compatible
>>> and restriction for i.MX93 platform.
>>>
>>> Signed-off-by: Xu Yang <xu.yang_2 at nxp.com>
>>>
>>> ---
>>> Changes in v2:
>>>  - no changes
>>> Changes in v3:
>>>  - add clocks restriction
>>> ---
>>>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml    | 16 ++++++++++++++++
>>>  1 file changed, 16 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-
>> hdrc-usb2.yaml
>>> index b7e664f7395b..6e75099b6394 100644
>>> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
>>> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
>>> @@ -57,6 +57,7 @@ properties:
>>>            - enum:
>>>                - fsl,imx8mm-usb
>>>                - fsl,imx8mn-usb
>>> +              - fsl,imx93-usb
>>>            - const: fsl,imx7d-usb
>>>            - const: fsl,imx27-usb
>>>        - items:
>>> @@ -412,6 +413,21 @@ allOf:
>>>          samsung,picophy-pre-emp-curr-control: false
>>>          samsung,picophy-dc-vol-level-adjust: false
>>>
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: fsl,imx93-usb
>>> +    then:
>>> +      properties:
>>> +        clock-names:
>>> +          items:
>>> +            - const: usb_ctrl_root_clk
>>> +            - const: usb_wakeup_clk
>>> +        clocks:
>>> +          minItems: 2
>>> +          maxItems: 2
>>
>> Nothing improved regarding my comments. Why do you allow
>> any/unspecific/unconstrain interrupts and reg?
>>
>> You said:
>> "However, reset, reg and interrupts property is not special for imx93."
>> but what does it even mean? How they can be special or not special?
>>
>> My comments from previous version apply. If you do not want to work on
>> existing technical debt, BTW added by another NXP employee, then I will
>> NAK any new submissions.
> 
> You want me to adjust below properties to be more common properties
> and add device specific limitations, right?

Yes

> 
> ---
>   reg:
>     minItems: 1
>     maxItems: 2
> 
>   interrupts:
>     minItems: 1
>     maxItems: 2
> 
>   clocks:
>     minItems: 1
>     maxItems: 3
> 
>   clock-names:
>     minItems: 1
>     maxItems: 3
> ---
> 
> For most of the devices, property reg, interrupts, clocks and clock-names
> has 1 item. So these properties can set maxItems to 1. For special devices,
> I should list them standalone, is this your expectation?

Just like you constrain clocks for new variant, your variant should have
constrained reg, interrupts and whatever else is there variable. I don't
require fixing all the devices in this binding, but at least your new
one and preferably other NXP as well.


Best regards,
Krzysztof