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

[Xu Yang]

Hi Krzysztof,

> 
> 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.

Okay. I will do this and test it.

Thanks,
Xu Yang