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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxx>
> >
> > ---
> > 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? 

> 
> In any case, please provide short comment in the code why you have such
> duplicated if:then:

Okay.

> 
> Best regards,
> Krzysztof





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux