Re: [PATCH V3 1/7] dt-bindings: usb: usbmisc-imx: convert to DT schema

[Peng Fan]

Hi Rob, Krzysztof,

On 2/28/2023 1:55 PM, Peng Fan wrote:
> > Subject: Re: [PATCH V3 1/7] dt-bindings: usb: usbmisc-imx: convert to DT
> > schema
...
> > > +
> > > +title: Freescale i.MX wrapper module for Chipidea USB2 controller
> > > +
> > > +maintainers:
> > > +  - Xu Yang <xu.yang_2@xxxxxxx>
> > > +  - Peng Fan <peng.fan@xxxxxxx>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    oneOf:
> > > +      - enum:
> > > +          - fsl,imx6q-usbmisc
> > > +          - fsl,imx7ulp-usbmisc
> > > +          - fsl,vf610-usbmisc
> > > +      - items:
> > > +          - enum:
> > > +              - fsl,imx6ul-usbmisc
> > > +              - fsl,imx6sx-usbmisc
> > > +              - fsl,imx7d-usbmisc
> > > +          - const: fsl,imx6q-usbmisc
> > > +      - items:
> > > +          - enum:
> > > +              - fsl,imx7ulp-usbmisc
> > > +              - fsl,imx8mm-usbmisc
> > > +          - const: fsl,imx7d-usbmisc
> >
> > So imx8mm is compatible with imx7d, and imx7d is compatible with imx6q,
> > but imx8mm is not compatible with imx6q? That doesn't really make sense.
> > Maybe all 3 compatibles makes sense, but only if s/w understanding only
> > one of the fallback compatibles would function without knowledge of the
> > newer h/w.
>
> I added the list according the current device tree in use. If using
> three compatibles,  that would involve device tree upate, and firmware
> update for SR-IR.
>
> My understanding is imx8mm is compatible with imx7d, imx7d is compatible
> with imx6q, then imx8mm is compatible with imx6q. it is just not put
> them under one item. Please correct if my understanding is wrong.
> >

Do you have any guidance here? I am not sure how to proceed on V4.

Thanks,
Peng

> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  '#index-cells':
> > > +    const: 1
> > > +    description: Cells used to describe usb controller index.
> >
> > Please mark this as deprecated. If it is always 1 cell, then there's no point.
>
> Fix in v4.
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - '#index-cells'
> >
> > And drop as required. That all can be a follow-up patch if you prefer or in
> > this patch is fine. Primarily, I don't want this pattern copied.
>
> Fix in v4.
>
> Thanks,
> Peng.
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    usbmisc@2184800 {
> > > +        #index-cells = <1>;
> > > +        compatible = "fsl,imx6q-usbmisc";
> > > +        reg = <0x02184800 0x200>;
> > > +    };
> > > +
> > > +...
> > > diff --git a/Documentation/devicetree/bindings/usb/usbmisc-imx.txt
> > > b/Documentation/devicetree/bindings/usb/usbmisc-imx.txt
> > > deleted file mode 100644
> > > index 29b8f65ff849..000000000000
> > > --- a/Documentation/devicetree/bindings/usb/usbmisc-imx.txt
> > > +++ /dev/null
> > > @@ -1,19 +0,0 @@
> > > -* Freescale i.MX non-core registers
> > > -
> > > -Required properties:
> > > -- #index-cells: Cells used to describe usb controller index. Should
> > > be <1>
> > > -- compatible: Should be one of below:
> > > -	"fsl,imx6q-usbmisc" for imx6q
> > > -	"fsl,vf610-usbmisc" for Vybrid vf610
> > > -	"fsl,imx6sx-usbmisc" for imx6sx
> > > -	"fsl,imx7d-usbmisc" for imx7d
> > > -	"fsl,imx7ulp-usbmisc" for imx7ulp
> > > -	"fsl,imx8mm-usbmisc" for imx8mm
> > > -- reg: Should contain registers location and length
> > > -
> > > -Examples:
> > > -usbmisc@2184800 {
> > > -	#index-cells = <1>;
> > > -	compatible = "fsl,imx6q-usbmisc";
> > > -	reg = <0x02184800 0x200>;
> > > -};
> > > --
> > > 2.37.1