[v2,1/2] dt-bindings: phy: mxs-usb-phy: convert to DT schema format

Commit Message

Xu Yang June 8, 2023, 3:36 a.m. UTC

Convert the binding to DT schema format. Besides, this also add other
optional properties not contained in txt file.

Signed-off-by: Xu Yang <[e-mail]>

---
Changes in v2:
 - change filename to fsl,mxs-usbphy.yaml
 - add other optional properties
 - narrow fsl,anatop to imx6
 - use additionalProperties
---
 .../bindings/phy/fsl,mxs-usbphy.yaml          | 128 ++++++++++++++++++
 .../devicetree/bindings/phy/mxs-usb-phy.txt   |  33 -----
 2 files changed, 128 insertions(+), 33 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
 delete mode 100644 Documentation/devicetree/bindings/phy/mxs-usb-phy.txt

Comments

Rob Herring June 8, 2023, 4:26 a.m. UTC | #1

On Thu, 08 Jun 2023 11:36:41 +0800, Xu Yang wrote:
> Convert the binding to DT schema format. Besides, this also add other
> optional properties not contained in txt file.
> 
> Signed-off-by: Xu Yang <[e-mail]>
> 
> ---
> Changes in v2:
>  - change filename to fsl,mxs-usbphy.yaml
>  - add other optional properties
>  - narrow fsl,anatop to imx6
>  - use additionalProperties
> ---
>  .../bindings/phy/fsl,mxs-usbphy.yaml          | 128 ++++++++++++++++++
>  .../devicetree/bindings/phy/mxs-usb-phy.txt   |  33 -----
>  2 files changed, 128 insertions(+), 33 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
>  delete mode 100644 Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml:103:15: [warning] wrong indentation: expected 16 but found 14 (indentation)
./Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml:109:15: [warning] wrong indentation: expected 16 but found 14 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.example.dtb: usb-phy@20c9000: 'clocks' is a required property
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[e-mail]

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Stefan Wahren June 8, 2023, 7:56 a.m. UTC | #2

Hi Xu,

Am 08.06.23 um 05:36 schrieb Xu Yang:
> Convert the binding to DT schema format. Besides, this also add other
> optional properties not contained in txt file.
> 
> Signed-off-by: Xu Yang <[e-mail]>
> 
> ---
> Changes in v2:
>   - change filename to fsl,mxs-usbphy.yaml
>   - add other optional properties
>   - narrow fsl,anatop to imx6
>   - use additionalProperties
> ---
>   .../bindings/phy/fsl,mxs-usbphy.yaml          | 128 ++++++++++++++++++
>   .../devicetree/bindings/phy/mxs-usb-phy.txt   |  33 -----
>   2 files changed, 128 insertions(+), 33 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
>   delete mode 100644 Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> 
> diff --git a/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> new file mode 100644
> index 000000000000..1b6b19fdf491
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> @@ -0,0 +1,128 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/fsl,mxs-usbphy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale MXS USB Phy Device
> +
> +maintainers:
> +  - Xu Yang <[e-mail]>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - enum:
> +          - fsl,imx23-usbphy
> +          - fsl,vf610-usbphy
> +          - fsl,imx7ulp-usbphy

on the one side the fsl,imx7ulp-usbphy has an individual compatible

> +      - items:
> +          - enum:
> +              - fsl,imx28-usbphy
> +              - fsl,imx6ul-usbphy
> +              - fsl,imx6sl-usbphy
> +              - fsl,imx6sx-usbphy
> +              - fsl,imx6q-usbphy
> +          - const: fsl,imx23-usbphy
> +      - items:
> +          - const: fsl,imx6sll-usbphy
> +          - const: fsl,imx6ul-usbphy
> +          - const: fsl,imx23-usbphy
> +      - items:
> +          - const: fsl,imx7ulp-usbphy
> +          - const: fsl,imx6ul-usbphy

on the other side this should be compatible to imx6ul. So at least one 
definition seems to be unnecessary.

Looking at usb/phy/phy-mxs-usb.c suggests me that fsl,imx7ulp-usbphy is 
not directly compatible to fsl,imx6ul-usbphy, because the platform data 
is different. So maybe the using dts* files should be fixed instead?

> +      - items:
> +          - const: fsl,imx8dxl-usbphy
> +          - const: fsl,imx7ulp-usbphy
> +
> +  reg:
> +    minItems: 1
> +    maxItems: 2
> +
> +  interrupts:
> +    minItems: 1
> +    maxItems: 2
> +
> +  clocks:
> +    maxItems: 1
> +
> +  '#phy-cells':
> +    const: 0
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  fsl,anatop:
> +    description:
> +      phandle for anatop register, it is only for imx6 SoC series.
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +
> +  phy-3p0-supply:
> +    description:
> +      One of USB PHY's power supply. Can be used to keep a good signal
> +      quality.
> +
> +  fsl,tx-cal-45-dn-ohms:
> +    description:
> +      Resistance (in ohms) of switchable high-speed trimming resistor
> +      connected in parallel with the 45 ohm resistor that terminates
> +      the DN output signal.
> +    minimum: 35
> +    maximum: 54
> +    default: 45
> +
> +  fsl,tx-cal-45-dp-ohms:
> +    description:
> +      Resistance (in ohms) of switchable high-speed trimming resistor
> +      connected in parallel with the 45 ohm resistor that terminates
> +      the DP output signal.
> +    minimum: 35
> +    maximum: 54
> +    default: 45
> +
> +  fsl,tx-d-cal:
> +    description:
> +      Current trimming value (as a percentage) of the 17.78 mA TX
> +      reference current.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 79
> +    maximum: 119
> +    default: 100
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          oneOf:
> +            - enum:
> +              - fsl,imx6sl-usbphy
> +              - fsl,imx6sx-usbphy
> +              - fsl,imx6sll-usbphy
> +              - fsl,imx6q-usbphy
> +              - fsl,vf610-usbphy
> +            - items:
> +              - const: fsl,imx6ul-usbphy
> +              - const: fsl,imx23-usbphy
> +    then:
> +      required:
> +        - fsl,anatop
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    usbphy1: usb-phy@20c9000 {
> +        compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
> +        reg = <0x020c9000 0x1000>;
> +        interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
> +        fsl,anatop = <&anatop>;
> +    };
> +
> +...
> diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> deleted file mode 100644
> index 70c813b0755f..000000000000
> --- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> +++ /dev/null
> @@ -1,33 +0,0 @@
> -* Freescale MXS USB Phy Device
> -
> -Required properties:
> -- compatible: should contain:
> -	* "fsl,imx23-usbphy" for imx23 and imx28
> -	* "fsl,imx6q-usbphy" for imx6dq and imx6dl
> -	* "fsl,imx6sl-usbphy" for imx6sl
> -	* "fsl,vf610-usbphy" for Vybrid vf610
> -	* "fsl,imx6sx-usbphy" for imx6sx
> -	* "fsl,imx7ulp-usbphy" for imx7ulp
> -	* "fsl,imx8dxl-usbphy" for imx8dxl
> -  "fsl,imx23-usbphy" is still a fallback for other strings
> -- reg: Should contain registers location and length
> -- interrupts: Should contain phy interrupt
> -- fsl,anatop: phandle for anatop register, it is only for imx6 SoC series
> -
> -Optional properties:
> -- fsl,tx-cal-45-dn-ohms: Integer [35-54]. Resistance (in ohms) of switchable
> -  high-speed trimming resistor connected in parallel with the 45 ohm resistor
> -  that terminates the DN output signal. Default: 45
> -- fsl,tx-cal-45-dp-ohms: Integer [35-54]. Resistance (in ohms) of switchable
> -  high-speed trimming resistor connected in parallel with the 45 ohm resistor
> -  that terminates the DP output signal. Default: 45
> -- fsl,tx-d-cal: Integer [79-119]. Current trimming value (as a percentage) of
> -  the 17.78mA TX reference current. Default: 100
> -
> -Example:
> -usbphy1: usb-phy@20c9000 {
> -	compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
> -	reg = <0x020c9000 0x1000>;
> -	interrupts = <0 44 0x04>;
> -	fsl,anatop = <&anatop>;
> -};

Xu Yang June 8, 2023, 10:30 a.m. UTC | #3

Hi Stefan,

> -----Original Message-----
> From: Stefan Wahren <[e-mail]>
> Sent: Thursday, June 8, 2023 3:56 PM
> To: Xu Yang <[e-mail]>; [e-mail]; [e-mail]; [e-mail]
> Cc: [e-mail]; [e-mail]; [e-mail]; [e-mail]; dl-linux-imx <linux-
> [e-mail]>; [e-mail]; [e-mail]; [e-mail]; linux-
> [e-mail]; Jun Li <[e-mail]>
> Subject: [EXT] Re: [PATCH v2 1/2] dt-bindings: phy: mxs-usb-phy: convert to DT schema format
>
> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the
> message using the 'Report this email' button
>
>
> Hi Xu,
>
> Am 08.06.23 um 05:36 schrieb Xu Yang:
> > Convert the binding to DT schema format. Besides, this also add other
> > optional properties not contained in txt file.
> >
> > Signed-off-by: Xu Yang <[e-mail]>
> >
> > ---
> > Changes in v2:
> >   - change filename to fsl,mxs-usbphy.yaml
> >   - add other optional properties
> >   - narrow fsl,anatop to imx6
> >   - use additionalProperties
> > ---
> >   .../bindings/phy/fsl,mxs-usbphy.yaml          | 128 ++++++++++++++++++
> >   .../devicetree/bindings/phy/mxs-usb-phy.txt   |  33 -----
> >   2 files changed, 128 insertions(+), 33 deletions(-)
> >   create mode 100644 Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> >   delete mode 100644 Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> >
> > diff --git a/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> > new file mode 100644
> > index 000000000000..1b6b19fdf491
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> > @@ -0,0 +1,128 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id:
> http://devicetree.org/schemas/phy/fsl,mxs-
> usbphy.yaml%23&data=05%7C01%7Cxu.yang_2%40nxp.com%7C5df4d949f975469013b408db67f5d46c%7C686ea1d3bc2b4c
> 6fa92cd99c5c301635%7C0%7C0%7C638218077754788407%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjo
> iV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=8Sz8tK9GVQqE6ywVLpxPB8YDFQvygZvj6s1NjZk
> hbzU%3D&reserved=0
> > +$schema: http://devicetree.org/meta-
> schemas%2Fcore.yaml%23&data=05%7C01%7Cxu.yang_2%40nxp.com%7C5df4d949f975469013b408db67f5d46c%7C686ea1
> d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638218077754788407%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD
> AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=iaX16VnSJnvU%2F0tcnRgsdnTxMsD89
> 5r4WquGsCFt9Qo%3D&reserved=0
> > +
> > +title: Freescale MXS USB Phy Device
> > +
> > +maintainers:
> > +  - Xu Yang <[e-mail]>
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - enum:
> > +          - fsl,imx23-usbphy
> > +          - fsl,vf610-usbphy
> > +          - fsl,imx7ulp-usbphy
>
> on the one side the fsl,imx7ulp-usbphy has an individual compatible
>
> > +      - items:
> > +          - enum:
> > +              - fsl,imx28-usbphy
> > +              - fsl,imx6ul-usbphy
> > +              - fsl,imx6sl-usbphy
> > +              - fsl,imx6sx-usbphy
> > +              - fsl,imx6q-usbphy
> > +          - const: fsl,imx23-usbphy
> > +      - items:
> > +          - const: fsl,imx6sll-usbphy
> > +          - const: fsl,imx6ul-usbphy
> > +          - const: fsl,imx23-usbphy
> > +      - items:
> > +          - const: fsl,imx7ulp-usbphy
> > +          - const: fsl,imx6ul-usbphy
>
> on the other side this should be compatible to imx6ul. So at least one
> definition seems to be unnecessary.
>
> Looking at usb/phy/phy-mxs-usb.c suggests me that fsl,imx7ulp-usbphy is
> not directly compatible to fsl,imx6ul-usbphy, because the platform data
> is different. So maybe the using dts* files should be fixed instead?

The imx7ulp and imx6ul only has minor difference. In general, imx7ulp
is compatilbe with imx6ul. We don't need to modify both dts file and
this doc here. So the validation of existing dtb would not fail.

Thanks,
Xu  Yang

>
> > +      - items:
> > +          - const: fsl,imx8dxl-usbphy
> > +          - const: fsl,imx7ulp-usbphy
> > +
> > +  reg:
> > +    minItems: 1
> > +    maxItems: 2
> > +
> > +  interrupts:
> > +    minItems: 1
> > +    maxItems: 2
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  '#phy-cells':
> > +    const: 0
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  fsl,anatop:
> > +    description:
> > +      phandle for anatop register, it is only for imx6 SoC series.
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +
> > +  phy-3p0-supply:
> > +    description:
> > +      One of USB PHY's power supply. Can be used to keep a good signal
> > +      quality.
> > +
> > +  fsl,tx-cal-45-dn-ohms:
> > +    description:
> > +      Resistance (in ohms) of switchable high-speed trimming resistor
> > +      connected in parallel with the 45 ohm resistor that terminates
> > +      the DN output signal.
> > +    minimum: 35
> > +    maximum: 54
> > +    default: 45
> > +
> > +  fsl,tx-cal-45-dp-ohms:
> > +    description:
> > +      Resistance (in ohms) of switchable high-speed trimming resistor
> > +      connected in parallel with the 45 ohm resistor that terminates
> > +      the DP output signal.
> > +    minimum: 35
> > +    maximum: 54
> > +    default: 45
> > +
> > +  fsl,tx-d-cal:
> > +    description:
> > +      Current trimming value (as a percentage) of the 17.78 mA TX
> > +      reference current.
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    minimum: 79
> > +    maximum: 119
> > +    default: 100
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          oneOf:
> > +            - enum:
> > +              - fsl,imx6sl-usbphy
> > +              - fsl,imx6sx-usbphy
> > +              - fsl,imx6sll-usbphy
> > +              - fsl,imx6q-usbphy
> > +              - fsl,vf610-usbphy
> > +            - items:
> > +              - const: fsl,imx6ul-usbphy
> > +              - const: fsl,imx23-usbphy
> > +    then:
> > +      required:
> > +        - fsl,anatop
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    usbphy1: usb-phy@20c9000 {
> > +        compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
> > +        reg = <0x020c9000 0x1000>;
> > +        interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
> > +        fsl,anatop = <&anatop>;
> > +    };
> > +
> > +...
> > diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt b/Documentation/devicetree/bindings/phy/mxs-
> usb-phy.txt
> > deleted file mode 100644
> > index 70c813b0755f..000000000000
> > --- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> > +++ /dev/null
> > @@ -1,33 +0,0 @@
> > -* Freescale MXS USB Phy Device
> > -
> > -Required properties:
> > -- compatible: should contain:
> > -     * "fsl,imx23-usbphy" for imx23 and imx28
> > -     * "fsl,imx6q-usbphy" for imx6dq and imx6dl
> > -     * "fsl,imx6sl-usbphy" for imx6sl
> > -     * "fsl,vf610-usbphy" for Vybrid vf610
> > -     * "fsl,imx6sx-usbphy" for imx6sx
> > -     * "fsl,imx7ulp-usbphy" for imx7ulp
> > -     * "fsl,imx8dxl-usbphy" for imx8dxl
> > -  "fsl,imx23-usbphy" is still a fallback for other strings
> > -- reg: Should contain registers location and length
> > -- interrupts: Should contain phy interrupt
> > -- fsl,anatop: phandle for anatop register, it is only for imx6 SoC series
> > -
> > -Optional properties:
> > -- fsl,tx-cal-45-dn-ohms: Integer [35-54]. Resistance (in ohms) of switchable
> > -  high-speed trimming resistor connected in parallel with the 45 ohm resistor
> > -  that terminates the DN output signal. Default: 45
> > -- fsl,tx-cal-45-dp-ohms: Integer [35-54]. Resistance (in ohms) of switchable
> > -  high-speed trimming resistor connected in parallel with the 45 ohm resistor
> > -  that terminates the DP output signal. Default: 45
> > -- fsl,tx-d-cal: Integer [79-119]. Current trimming value (as a percentage) of
> > -  the 17.78mA TX reference current. Default: 100
> > -
> > -Example:
> > -usbphy1: usb-phy@20c9000 {
> > -     compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
> > -     reg = <0x020c9000 0x1000>;
> > -     interrupts = <0 44 0x04>;
> > -     fsl,anatop = <&anatop>;
> > -};

Stefan Wahren June 8, 2023, 11:38 a.m. UTC | #4

Hi Xu,

Am 08.06.23 um 12:30 schrieb Xu Yang:
> Hi Stefan,
> 
>> -----Original Message-----
>> From: Stefan Wahren <[e-mail]>
>> Sent: Thursday, June 8, 2023 3:56 PM
>> To: Xu Yang <[e-mail]>; [e-mail]; [e-mail]; [e-mail]
>> Cc: [e-mail]; [e-mail]; [e-mail]; [e-mail]; dl-linux-imx <linux-
>> [e-mail]>; [e-mail]; [e-mail]; [e-mail]; linux-
>> [e-mail]; Jun Li <[e-mail]>
>> Subject: [EXT] Re: [PATCH v2 1/2] dt-bindings: phy: mxs-usb-phy: convert to DT schema format
>>
>> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the
>> message using the 'Report this email' button
>>
>>
>> Hi Xu,
>>
>> Am 08.06.23 um 05:36 schrieb Xu Yang:
>>> Convert the binding to DT schema format. Besides, this also add other
>>> optional properties not contained in txt file.
>>>
>>> Signed-off-by: Xu Yang <[e-mail]>
>>>
>>> ---
>>> Changes in v2:
>>>    - change filename to fsl,mxs-usbphy.yaml
>>>    - add other optional properties
>>>    - narrow fsl,anatop to imx6
>>>    - use additionalProperties
>>> ---
>>>    .../bindings/phy/fsl,mxs-usbphy.yaml          | 128 ++++++++++++++++++
>>>    .../devicetree/bindings/phy/mxs-usb-phy.txt   |  33 -----
>>>    2 files changed, 128 insertions(+), 33 deletions(-)
>>>    create mode 100644 Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
>>>    delete mode 100644 Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
>> b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
>>> new file mode 100644
>>> index 000000000000..1b6b19fdf491
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
>>> @@ -0,0 +1,128 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id:
>> http://devicetree.org/schemas/phy/fsl,mxs-
>> usbphy.yaml%23&data=05%7C01%7Cxu.yang_2%40nxp.com%7C5df4d949f975469013b408db67f5d46c%7C686ea1d3bc2b4c
>> 6fa92cd99c5c301635%7C0%7C0%7C638218077754788407%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjo
>> iV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=8Sz8tK9GVQqE6ywVLpxPB8YDFQvygZvj6s1NjZk
>> hbzU%3D&reserved=0
>>> +$schema: http://devicetree.org/meta-
>> schemas%2Fcore.yaml%23&data=05%7C01%7Cxu.yang_2%40nxp.com%7C5df4d949f975469013b408db67f5d46c%7C686ea1
>> d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638218077754788407%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD
>> AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=iaX16VnSJnvU%2F0tcnRgsdnTxMsD89
>> 5r4WquGsCFt9Qo%3D&reserved=0
>>> +
>>> +title: Freescale MXS USB Phy Device
>>> +
>>> +maintainers:
>>> +  - Xu Yang <[e-mail]>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    oneOf:
>>> +      - enum:
>>> +          - fsl,imx23-usbphy
>>> +          - fsl,vf610-usbphy
>>> +          - fsl,imx7ulp-usbphy
>>
>> on the one side the fsl,imx7ulp-usbphy has an individual compatible
>>
>>> +      - items:
>>> +          - enum:
>>> +              - fsl,imx28-usbphy
>>> +              - fsl,imx6ul-usbphy
>>> +              - fsl,imx6sl-usbphy
>>> +              - fsl,imx6sx-usbphy
>>> +              - fsl,imx6q-usbphy
>>> +          - const: fsl,imx23-usbphy
>>> +      - items:
>>> +          - const: fsl,imx6sll-usbphy
>>> +          - const: fsl,imx6ul-usbphy
>>> +          - const: fsl,imx23-usbphy
>>> +      - items:
>>> +          - const: fsl,imx7ulp-usbphy
>>> +          - const: fsl,imx6ul-usbphy
>>
>> on the other side this should be compatible to imx6ul. So at least one
>> definition seems to be unnecessary.
>>
>> Looking at usb/phy/phy-mxs-usb.c suggests me that fsl,imx7ulp-usbphy is
>> not directly compatible to fsl,imx6ul-usbphy, because the platform data
>> is different. So maybe the using dts* files should be fixed instead?
> 
> The imx7ulp and imx6ul only has minor difference. In general, imx7ulp
> is compatilbe with imx6ul. We don't need to modify both dts file and
> this doc here. So the validation of existing dtb would not fail.

The fact that according to the schema imx6ul needs fsl,anatop and 
imx7ulp doesn't need it, let me think that the difference is not really 
minor.

Nevertheless the compatibles for imx7ulp-usbphy looks fishy to me, 
because there are two ways to describe imx7ulp-usbphy ( with and without 
fsl,imx6ul-usbphy ). From my understanding there should be only one way.

In case you are just concerned about validation issues in this series: 
it's acceptable to convert txt file to YAML and fix outstanding 
validation issues within the series via separate patch. In my opinion 
the goal is to get a proper DT schema and not just to avoid DT 
validation warnings.

Best regards

> 
> Thanks,
> Xu  Yang
> 
>>
>>> +      - items:
>>> +          - const: fsl,imx8dxl-usbphy
>>> +          - const: fsl,imx7ulp-usbphy
>>> +
>>> +  reg:
>>> +    minItems: 1
>>> +    maxItems: 2
>>> +
>>> +  interrupts:
>>> +    minItems: 1
>>> +    maxItems: 2
>>> +
>>> +  clocks:
>>> +    maxItems: 1
>>> +
>>> +  '#phy-cells':
>>> +    const: 0
>>> +
>>> +  power-domains:
>>> +    maxItems: 1
>>> +
>>> +  fsl,anatop:
>>> +    description:
>>> +      phandle for anatop register, it is only for imx6 SoC series.
>>> +    $ref: /schemas/types.yaml#/definitions/phandle
>>> +
>>> +  phy-3p0-supply:
>>> +    description:
>>> +      One of USB PHY's power supply. Can be used to keep a good signal
>>> +      quality.
>>> +
>>> +  fsl,tx-cal-45-dn-ohms:
>>> +    description:
>>> +      Resistance (in ohms) of switchable high-speed trimming resistor
>>> +      connected in parallel with the 45 ohm resistor that terminates
>>> +      the DN output signal.
>>> +    minimum: 35
>>> +    maximum: 54
>>> +    default: 45
>>> +
>>> +  fsl,tx-cal-45-dp-ohms:
>>> +    description:
>>> +      Resistance (in ohms) of switchable high-speed trimming resistor
>>> +      connected in parallel with the 45 ohm resistor that terminates
>>> +      the DP output signal.
>>> +    minimum: 35
>>> +    maximum: 54
>>> +    default: 45
>>> +
>>> +  fsl,tx-d-cal:
>>> +    description:
>>> +      Current trimming value (as a percentage) of the 17.78 mA TX
>>> +      reference current.
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    minimum: 79
>>> +    maximum: 119
>>> +    default: 100
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - clocks
>>> +
>>> +allOf:
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          oneOf:
>>> +            - enum:
>>> +              - fsl,imx6sl-usbphy
>>> +              - fsl,imx6sx-usbphy
>>> +              - fsl,imx6sll-usbphy
>>> +              - fsl,imx6q-usbphy
>>> +              - fsl,vf610-usbphy
>>> +            - items:
>>> +              - const: fsl,imx6ul-usbphy
>>> +              - const: fsl,imx23-usbphy
>>> +    then:
>>> +      required:
>>> +        - fsl,anatop
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +
>>> +    usbphy1: usb-phy@20c9000 {
>>> +        compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
>>> +        reg = <0x020c9000 0x1000>;
>>> +        interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
>>> +        fsl,anatop = <&anatop>;
>>> +    };
>>> +
>>> +...
>>> diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt b/Documentation/devicetree/bindings/phy/mxs-
>> usb-phy.txt
>>> deleted file mode 100644
>>> index 70c813b0755f..000000000000
>>> --- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>>> +++ /dev/null
>>> @@ -1,33 +0,0 @@
>>> -* Freescale MXS USB Phy Device
>>> -
>>> -Required properties:
>>> -- compatible: should contain:
>>> -     * "fsl,imx23-usbphy" for imx23 and imx28
>>> -     * "fsl,imx6q-usbphy" for imx6dq and imx6dl
>>> -     * "fsl,imx6sl-usbphy" for imx6sl
>>> -     * "fsl,vf610-usbphy" for Vybrid vf610
>>> -     * "fsl,imx6sx-usbphy" for imx6sx
>>> -     * "fsl,imx7ulp-usbphy" for imx7ulp
>>> -     * "fsl,imx8dxl-usbphy" for imx8dxl
>>> -  "fsl,imx23-usbphy" is still a fallback for other strings
>>> -- reg: Should contain registers location and length
>>> -- interrupts: Should contain phy interrupt
>>> -- fsl,anatop: phandle for anatop register, it is only for imx6 SoC series
>>> -
>>> -Optional properties:
>>> -- fsl,tx-cal-45-dn-ohms: Integer [35-54]. Resistance (in ohms) of switchable
>>> -  high-speed trimming resistor connected in parallel with the 45 ohm resistor
>>> -  that terminates the DN output signal. Default: 45
>>> -- fsl,tx-cal-45-dp-ohms: Integer [35-54]. Resistance (in ohms) of switchable
>>> -  high-speed trimming resistor connected in parallel with the 45 ohm resistor
>>> -  that terminates the DP output signal. Default: 45
>>> -- fsl,tx-d-cal: Integer [79-119]. Current trimming value (as a percentage) of
>>> -  the 17.78mA TX reference current. Default: 100
>>> -
>>> -Example:
>>> -usbphy1: usb-phy@20c9000 {
>>> -     compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
>>> -     reg = <0x020c9000 0x1000>;
>>> -     interrupts = <0 44 0x04>;
>>> -     fsl,anatop = <&anatop>;
>>> -};

Xu Yang June 9, 2023, 9:23 a.m. UTC | #5

Hi Stefan,

> -----Original Message-----
> From: Stefan Wahren <[e-mail]>
> Sent: Thursday, June 8, 2023 7:39 PM
> To: Xu Yang <[e-mail]>; [e-mail]; [e-mail]; [e-mail]
> Cc: [e-mail]; [e-mail]; [e-mail]; [e-mail]; dl-linux-imx <linux-
> [e-mail]>; [e-mail]; [e-mail]; [e-mail]; linux-
> [e-mail]; Jun Li <[e-mail]>
> Subject: Re: [EXT] Re: [PATCH v2 1/2] dt-bindings: phy: mxs-usb-phy: convert to DT schema format
>
> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the
> message using the 'Report this email' button
>
>
> Hi Xu,
>
> Am 08.06.23 um 12:30 schrieb Xu Yang:
> > Hi Stefan,
> >
> >> -----Original Message-----
> >> From: Stefan Wahren <[e-mail]>
> >> Sent: Thursday, June 8, 2023 3:56 PM
> >> To: Xu Yang <[e-mail]>; [e-mail]; [e-mail]; [e-mail]
> >> Cc: [e-mail]; [e-mail]; [e-mail]; [e-mail]; dl-linux-imx
> <linux-
> >> [e-mail]>; [e-mail]; [e-mail]; [e-mail];
> linux-
> >> [e-mail]; Jun Li <[e-mail]>
> >> Subject: [EXT] Re: [PATCH v2 1/2] dt-bindings: phy: mxs-usb-phy: convert to DT schema format
> >>
> >> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report
> the
> >> message using the 'Report this email' button
> >>
> >>
> >> Hi Xu,
> >>
> >> Am 08.06.23 um 05:36 schrieb Xu Yang:
> >>> Convert the binding to DT schema format. Besides, this also add other
> >>> optional properties not contained in txt file.
> >>>
> >>> Signed-off-by: Xu Yang <[e-mail]>
> >>>
> >>> ---
> >>> Changes in v2:
> >>>    - change filename to fsl,mxs-usbphy.yaml
> >>>    - add other optional properties
> >>>    - narrow fsl,anatop to imx6
> >>>    - use additionalProperties
> >>> ---
> >>>    .../bindings/phy/fsl,mxs-usbphy.yaml          | 128 ++++++++++++++++++
> >>>    .../devicetree/bindings/phy/mxs-usb-phy.txt   |  33 -----
> >>>    2 files changed, 128 insertions(+), 33 deletions(-)
> >>>    create mode 100644 Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> >>>    delete mode 100644 Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> >> b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> >>> new file mode 100644
> >>> index 000000000000..1b6b19fdf491
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> >>> @@ -0,0 +1,128 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +$id:
> >>
> http://devicetree.org/schemas/phy/fsl,mxs-
> &data=05%7C01%7Cxu.yang_2%40nxp.com%7C35ccff605dbd46ac9d8608db6814f13f%7C686ea1d3bc2b4c6fa92cd99c5c3016
> 35%7C0%7C0%7C638218211379372106%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6I
> k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=gSKnnBeB7BrscJNdHEPWgALwvaSlnZrCbW2lo1K8D0s%3D&reserv
> ed=0
> >>
> usbphy.yaml%23&data=05%7C01%7Cxu.yang_2%40nxp.com%7C5df4d949f975469013b408db67f5d46c%7C686ea1d3bc2b4c
> >>
> 6fa92cd99c5c301635%7C0%7C0%7C638218077754788407%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjo
> >>
> iV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=8Sz8tK9GVQqE6ywVLpxPB8YDFQvygZvj6s1NjZk
> >> hbzU%3D&reserved=0
> >>> +$schema: http://devicetree.org/meta-
> &data=05%7C01%7Cxu.yang_2%40nxp.com%7C35ccff605dbd46ac9d8608db6814f13f%7C686ea1d3bc2b4c6fa92cd99c5c3016
> 35%7C0%7C0%7C638218211379372106%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6I
> k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=D3MKKJpLHdfLhK9ubiqQYGZ7ORNDOYbA%2FJOQX%2B3DoAg%3
> D&reserved=0
> >>
> schemas%2Fcore.yaml%23&data=05%7C01%7Cxu.yang_2%40nxp.com%7C5df4d949f975469013b408db67f5d46c%7C686ea1
> >>
> d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638218077754788407%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD
> >>
> AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=iaX16VnSJnvU%2F0tcnRgsdnTxMsD89
> >> 5r4WquGsCFt9Qo%3D&reserved=0
> >>> +
> >>> +title: Freescale MXS USB Phy Device
> >>> +
> >>> +maintainers:
> >>> +  - Xu Yang <[e-mail]>
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    oneOf:
> >>> +      - enum:
> >>> +          - fsl,imx23-usbphy
> >>> +          - fsl,vf610-usbphy
> >>> +          - fsl,imx7ulp-usbphy
> >>
> >> on the one side the fsl,imx7ulp-usbphy has an individual compatible
> >>
> >>> +      - items:
> >>> +          - enum:
> >>> +              - fsl,imx28-usbphy
> >>> +              - fsl,imx6ul-usbphy
> >>> +              - fsl,imx6sl-usbphy
> >>> +              - fsl,imx6sx-usbphy
> >>> +              - fsl,imx6q-usbphy
> >>> +          - const: fsl,imx23-usbphy
> >>> +      - items:
> >>> +          - const: fsl,imx6sll-usbphy
> >>> +          - const: fsl,imx6ul-usbphy
> >>> +          - const: fsl,imx23-usbphy
> >>> +      - items:
> >>> +          - const: fsl,imx7ulp-usbphy
> >>> +          - const: fsl,imx6ul-usbphy
> >>
> >> on the other side this should be compatible to imx6ul. So at least one
> >> definition seems to be unnecessary.
> >>
> >> Looking at usb/phy/phy-mxs-usb.c suggests me that fsl,imx7ulp-usbphy is
> >> not directly compatible to fsl,imx6ul-usbphy, because the platform data
> >> is different. So maybe the using dts* files should be fixed instead?
> >
> > The imx7ulp and imx6ul only has minor difference. In general, imx7ulp
> > is compatilbe with imx6ul. We don't need to modify both dts file and
> > this doc here. So the validation of existing dtb would not fail.
>
> The fact that according to the schema imx6ul needs fsl,anatop and
> imx7ulp doesn't need it, let me think that the difference is not really
> minor.
>
> Nevertheless the compatibles for imx7ulp-usbphy looks fishy to me,
> because there are two ways to describe imx7ulp-usbphy ( with and without
> fsl,imx6ul-usbphy ). From my understanding there should be only one way.
>
> In case you are just concerned about validation issues in this series:
> it's acceptable to convert txt file to YAML and fix outstanding
> validation issues within the series via separate patch. In my opinion
> the goal is to get a proper DT schema and not just to avoid DT
> validation warnings.

I reviewed the dts files and driver again. It seems indeedly that
imx7ulp-usbphy is diverged from imx6ul-usbphy today. Therefore, to keep
this doc clear, "fsl,imx7ulp-usbphy", "fsl,imx6ul-usbphy" could be
removed. Thanks for your input. I will remove the second one and keep
the first one in V4.

Thanks,
Xu Yang

>
> Best regards
>
> >
> > Thanks,
> > Xu  Yang
> >
> >>
> >>> +      - items:
> >>> +          - const: fsl,imx8dxl-usbphy
> >>> +          - const: fsl,imx7ulp-usbphy
> >>> +
> >>> +  reg:
> >>> +    minItems: 1
> >>> +    maxItems: 2
> >>> +
> >>> +  interrupts:
> >>> +    minItems: 1
> >>> +    maxItems: 2
> >>> +
> >>> +  clocks:
> >>> +    maxItems: 1
> >>> +
> >>> +  '#phy-cells':
> >>> +    const: 0
> >>> +
> >>> +  power-domains:
> >>> +    maxItems: 1
> >>> +
> >>> +  fsl,anatop:
> >>> +    description:
> >>> +      phandle for anatop register, it is only for imx6 SoC series.
> >>> +    $ref: /schemas/types.yaml#/definitions/phandle
> >>> +
> >>> +  phy-3p0-supply:
> >>> +    description:
> >>> +      One of USB PHY's power supply. Can be used to keep a good signal
> >>> +      quality.
> >>> +
> >>> +  fsl,tx-cal-45-dn-ohms:
> >>> +    description:
> >>> +      Resistance (in ohms) of switchable high-speed trimming resistor
> >>> +      connected in parallel with the 45 ohm resistor that terminates
> >>> +      the DN output signal.
> >>> +    minimum: 35
> >>> +    maximum: 54
> >>> +    default: 45
> >>> +
> >>> +  fsl,tx-cal-45-dp-ohms:
> >>> +    description:
> >>> +      Resistance (in ohms) of switchable high-speed trimming resistor
> >>> +      connected in parallel with the 45 ohm resistor that terminates
> >>> +      the DP output signal.
> >>> +    minimum: 35
> >>> +    maximum: 54
> >>> +    default: 45
> >>> +
> >>> +  fsl,tx-d-cal:
> >>> +    description:
> >>> +      Current trimming value (as a percentage) of the 17.78 mA TX
> >>> +      reference current.
> >>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>> +    minimum: 79
> >>> +    maximum: 119
> >>> +    default: 100
> >>> +
> >>> +required:
> >>> +  - compatible
> >>> +  - reg
> >>> +  - clocks
> >>> +
> >>> +allOf:
> >>> +  - if:
> >>> +      properties:
> >>> +        compatible:
> >>> +          oneOf:
> >>> +            - enum:
> >>> +              - fsl,imx6sl-usbphy
> >>> +              - fsl,imx6sx-usbphy
> >>> +              - fsl,imx6sll-usbphy
> >>> +              - fsl,imx6q-usbphy
> >>> +              - fsl,vf610-usbphy
> >>> +            - items:
> >>> +              - const: fsl,imx6ul-usbphy
> >>> +              - const: fsl,imx23-usbphy
> >>> +    then:
> >>> +      required:
> >>> +        - fsl,anatop
> >>> +
> >>> +additionalProperties: false
> >>> +
> >>> +examples:
> >>> +  - |
> >>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> >>> +
> >>> +    usbphy1: usb-phy@20c9000 {
> >>> +        compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
> >>> +        reg = <0x020c9000 0x1000>;
> >>> +        interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
> >>> +        fsl,anatop = <&anatop>;
> >>> +    };
> >>> +
> >>> +...
> >>> diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> b/Documentation/devicetree/bindings/phy/mxs-
> >> usb-phy.txt
> >>> deleted file mode 100644
> >>> index 70c813b0755f..000000000000
> >>> --- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> >>> +++ /dev/null
> >>> @@ -1,33 +0,0 @@
> >>> -* Freescale MXS USB Phy Device
> >>> -
> >>> -Required properties:
> >>> -- compatible: should contain:
> >>> -     * "fsl,imx23-usbphy" for imx23 and imx28
> >>> -     * "fsl,imx6q-usbphy" for imx6dq and imx6dl
> >>> -     * "fsl,imx6sl-usbphy" for imx6sl
> >>> -     * "fsl,vf610-usbphy" for Vybrid vf610
> >>> -     * "fsl,imx6sx-usbphy" for imx6sx
> >>> -     * "fsl,imx7ulp-usbphy" for imx7ulp
> >>> -     * "fsl,imx8dxl-usbphy" for imx8dxl
> >>> -  "fsl,imx23-usbphy" is still a fallback for other strings
> >>> -- reg: Should contain registers location and length
> >>> -- interrupts: Should contain phy interrupt
> >>> -- fsl,anatop: phandle for anatop register, it is only for imx6 SoC series
> >>> -
> >>> -Optional properties:
> >>> -- fsl,tx-cal-45-dn-ohms: Integer [35-54]. Resistance (in ohms) of switchable
> >>> -  high-speed trimming resistor connected in parallel with the 45 ohm resistor
> >>> -  that terminates the DN output signal. Default: 45
> >>> -- fsl,tx-cal-45-dp-ohms: Integer [35-54]. Resistance (in ohms) of switchable
> >>> -  high-speed trimming resistor connected in parallel with the 45 ohm resistor
> >>> -  that terminates the DP output signal. Default: 45
> >>> -- fsl,tx-d-cal: Integer [79-119]. Current trimming value (as a percentage) of
> >>> -  the 17.78mA TX reference current. Default: 100
> >>> -
> >>> -Example:
> >>> -usbphy1: usb-phy@20c9000 {
> >>> -     compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
> >>> -     reg = <0x020c9000 0x1000>;
> >>> -     interrupts = <0 44 0x04>;
> >>> -     fsl,anatop = <&anatop>;
> >>> -};

Krzysztof Kozlowski June 9, 2023, 4:43 p.m. UTC | #6

On 08/06/2023 05:36, Xu Yang wrote:
> Convert the binding to DT schema format. Besides, this also add other
> optional properties not contained in txt file.

Why ones do you add?

> 
> Signed-off-by: Xu Yang <[e-mail]>
> 
> ---
> Changes in v2:
>  - change filename to fsl,mxs-usbphy.yaml
>  - add other optional properties
>  - narrow fsl,anatop to imx6
>  - use additionalProperties
> ---
>  .../bindings/phy/fsl,mxs-usbphy.yaml          | 128 ++++++++++++++++++
>  .../devicetree/bindings/phy/mxs-usb-phy.txt   |  33 -----
>  2 files changed, 128 insertions(+), 33 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
>  delete mode 100644 Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> 
> diff --git a/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> new file mode 100644
> index 000000000000..1b6b19fdf491
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> @@ -0,0 +1,128 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/fsl,mxs-usbphy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale MXS USB Phy Device
> +
> +maintainers:
> +  - Xu Yang <[e-mail]>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - enum:
> +          - fsl,imx23-usbphy
> +          - fsl,vf610-usbphy
> +          - fsl,imx7ulp-usbphy

Keep the list sorted.

> +      - items:
> +          - enum:
> +              - fsl,imx28-usbphy
> +              - fsl,imx6ul-usbphy
> +              - fsl,imx6sl-usbphy
> +              - fsl,imx6sx-usbphy
> +              - fsl,imx6q-usbphy
> +          - const: fsl,imx23-usbphy
> +      - items:
> +          - const: fsl,imx6sll-usbphy
> +          - const: fsl,imx6ul-usbphy
> +          - const: fsl,imx23-usbphy
> +      - items:
> +          - const: fsl,imx7ulp-usbphy
> +          - const: fsl,imx6ul-usbphy
> +      - items:
> +          - const: fsl,imx8dxl-usbphy
> +          - const: fsl,imx7ulp-usbphy
> +
> +  reg:
> +    minItems: 1
> +    maxItems: 2

You should list the items.

> +
> +  interrupts:
> +    minItems: 1
> +    maxItems: 2

You should list the items.

> +
> +  clocks:
> +    maxItems: 1
> +
> +  '#phy-cells':
> +    const: 0
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  fsl,anatop:
> +    description:
> +      phandle for anatop register, it is only for imx6 SoC series.
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +
> +  phy-3p0-supply:
> +    description:
> +      One of USB PHY's power supply. Can be used to keep a good signal
> +      quality.
> +
> +  fsl,tx-cal-45-dn-ohms:
> +    description:
> +      Resistance (in ohms) of switchable high-speed trimming resistor
> +      connected in parallel with the 45 ohm resistor that terminates
> +      the DN output signal.
> +    minimum: 35
> +    maximum: 54
> +    default: 45
> +
> +  fsl,tx-cal-45-dp-ohms:
> +    description:
> +      Resistance (in ohms) of switchable high-speed trimming resistor
> +      connected in parallel with the 45 ohm resistor that terminates
> +      the DP output signal.
> +    minimum: 35
> +    maximum: 54
> +    default: 45
> +
> +  fsl,tx-d-cal:
> +    description:
> +      Current trimming value (as a percentage) of the 17.78 mA TX
> +      reference current.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 79
> +    maximum: 119
> +    default: 100
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          oneOf:
> +            - enum:
> +              - fsl,imx6sl-usbphy

It does not look like you tested the bindings, at least after quick
look. Please run `make dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
Maybe you need to update your dtschema and yamllint.

> +              - fsl,imx6sx-usbphy
> +              - fsl,imx6sll-usbphy
> +              - fsl,imx6q-usbphy
> +              - fsl,vf610-usbphy
> +            - items:
> +              - const: fsl,imx6ul-usbphy
> +              - const: fsl,imx23-usbphy
> +    then:
> +      required:
> +        - fsl,anatop
> +
> +additionalProperties: false
> +


Best regards,
Krzysztof

Xu Yang June 13, 2023, 7:30 a.m. UTC | #7

Hi Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <[e-mail]>
> Sent: Saturday, June 10, 2023 12:44 AM
> To: Xu Yang <[e-mail]>; [e-mail]; [e-mail]; [e-mail]
> Cc: [e-mail]; [e-mail]; [e-mail]; [e-mail]; dl-linux-imx <linux-
> [e-mail]>; [e-mail]; [e-mail]; [e-mail]; linux-
> [e-mail]; Jun Li <[e-mail]>
> Subject: [EXT] Re: [PATCH v2 1/2] dt-bindings: phy: mxs-usb-phy: convert to DT schema format
>
> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the
> message using the 'Report this email' button
>
>
> On 08/06/2023 05:36, Xu Yang wrote:
> > Convert the binding to DT schema format. Besides, this also add other
> > optional properties not contained in txt file.
>
> Why ones do you add?

I add clock, power-domains and phy-3p0-supply to this doc. Because
the txt file should contain these properties but it not. I think the
txt doesn't get update in time.

>
> >
> > Signed-off-by: Xu Yang <[e-mail]>
> >
> > ---
> > Changes in v2:
> >  - change filename to fsl,mxs-usbphy.yaml
> >  - add other optional properties
> >  - narrow fsl,anatop to imx6
> >  - use additionalProperties
> > ---
> >  .../bindings/phy/fsl,mxs-usbphy.yaml          | 128 ++++++++++++++++++
> >  .../devicetree/bindings/phy/mxs-usb-phy.txt   |  33 -----
> >  2 files changed, 128 insertions(+), 33 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> >  delete mode 100644 Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> >
> > diff --git a/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> > new file mode 100644
> > index 000000000000..1b6b19fdf491
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
> > @@ -0,0 +1,128 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id:
> http://devicetree.org/schemas/phy/fsl,mxs-
> usbphy.yaml%23&data=05%7C01%7Cxu.yang_2%40nxp.com%7Ccdb34e27fd17419bbe3608db6908b13d%7C686ea1d3bc2b4
> c6fa92cd99c5c301635%7C0%7C0%7C638219258284008643%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj
> oiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=4pNCtSE9x4kENZsGf2nq%2F2S8uazubTjeqpSZ
> XyRRVOo%3D&reserved=0
> > +$schema: http://devicetree.org/meta-
> schemas%2Fcore.yaml%23&data=05%7C01%7Cxu.yang_2%40nxp.com%7Ccdb34e27fd17419bbe3608db6908b13d%7C686ea
> 1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638219258284008643%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM
> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=QjXo%2FzS6Ntl2RGBScVJfkdIONo6Sa
> psDkcDM5AY9cgQ%3D&reserved=0
> > +
> > +title: Freescale MXS USB Phy Device
> > +
> > +maintainers:
> > +  - Xu Yang <[e-mail]>
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - enum:
> > +          - fsl,imx23-usbphy
> > +          - fsl,vf610-usbphy
> > +          - fsl,imx7ulp-usbphy
>
> Keep the list sorted.

Okay.

>
> > +      - items:
> > +          - enum:
> > +              - fsl,imx28-usbphy
> > +              - fsl,imx6ul-usbphy
> > +              - fsl,imx6sl-usbphy
> > +              - fsl,imx6sx-usbphy
> > +              - fsl,imx6q-usbphy
> > +          - const: fsl,imx23-usbphy
> > +      - items:
> > +          - const: fsl,imx6sll-usbphy
> > +          - const: fsl,imx6ul-usbphy
> > +          - const: fsl,imx23-usbphy
> > +      - items:
> > +          - const: fsl,imx7ulp-usbphy
> > +          - const: fsl,imx6ul-usbphy
> > +      - items:
> > +          - const: fsl,imx8dxl-usbphy
> > +          - const: fsl,imx7ulp-usbphy
> > +
> > +  reg:
> > +    minItems: 1
> > +    maxItems: 2
>
> You should list the items.
>
> > +
> > +  interrupts:
> > +    minItems: 1
> > +    maxItems: 2
>
> You should list the items.

After my check, the maxItems value of reg and interrupts should be 1.
Will change it.

>
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  '#phy-cells':
> > +    const: 0
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  fsl,anatop:
> > +    description:
> > +      phandle for anatop register, it is only for imx6 SoC series.
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +
> > +  phy-3p0-supply:
> > +    description:
> > +      One of USB PHY's power supply. Can be used to keep a good signal
> > +      quality.
> > +
> > +  fsl,tx-cal-45-dn-ohms:
> > +    description:
> > +      Resistance (in ohms) of switchable high-speed trimming resistor
> > +      connected in parallel with the 45 ohm resistor that terminates
> > +      the DN output signal.
> > +    minimum: 35
> > +    maximum: 54
> > +    default: 45
> > +
> > +  fsl,tx-cal-45-dp-ohms:
> > +    description:
> > +      Resistance (in ohms) of switchable high-speed trimming resistor
> > +      connected in parallel with the 45 ohm resistor that terminates
> > +      the DP output signal.
> > +    minimum: 35
> > +    maximum: 54
> > +    default: 45
> > +
> > +  fsl,tx-d-cal:
> > +    description:
> > +      Current trimming value (as a percentage) of the 17.78 mA TX
> > +      reference current.
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    minimum: 79
> > +    maximum: 119
> > +    default: 100
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          oneOf:
> > +            - enum:
> > +              - fsl,imx6sl-usbphy
>
> It does not look like you tested the bindings, at least after quick
> look. Please run `make dt_binding_check` (see
> Documentation/devicetree/bindings/writing-schema.rst for instructions).
> Maybe you need to update your dtschema and yamllint.

Missed this in v2, but already checked bindings in v3. However, I have
checked schema using dt-doc-validate and dtbs using dt-validate tools.

>
> > +              - fsl,imx6sx-usbphy
> > +              - fsl,imx6sll-usbphy
> > +              - fsl,imx6q-usbphy
> > +              - fsl,vf610-usbphy
> > +            - items:
> > +              - const: fsl,imx6ul-usbphy
> > +              - const: fsl,imx23-usbphy
> > +    then:
> > +      required:
> > +        - fsl,anatop
> > +
> > +additionalProperties: false
> > +
>
>
> Best regards,
> Krzysztof

Thanks,
Xu Yang

Krzysztof Kozlowski June 13, 2023, 7:39 a.m. UTC | #8

On 13/06/2023 09:30, Xu Yang wrote:
> Hi Krzysztof,
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <[e-mail]>
>> Sent: Saturday, June 10, 2023 12:44 AM
>> To: Xu Yang <[e-mail]>; [e-mail]; [e-mail]; [e-mail]
>> Cc: [e-mail]; [e-mail]; [e-mail]; [e-mail]; dl-linux-imx <linux-
>> [e-mail]>; [e-mail]; [e-mail]; [e-mail]; linux-
>> [e-mail]; Jun Li <[e-mail]>
>> Subject: [EXT] Re: [PATCH v2 1/2] dt-bindings: phy: mxs-usb-phy: convert to DT schema format
>>
>> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the
>> message using the 'Report this email' button
>>
>>
>> On 08/06/2023 05:36, Xu Yang wrote:
>>> Convert the binding to DT schema format. Besides, this also add other
>>> optional properties not contained in txt file.
>>
>> Why ones do you add?
> 
> I add clock, power-domains and phy-3p0-supply to this doc. Because
> the txt file should contain these properties but it not. I think the
> txt doesn't get update in time.

Can you mention them in commit msg?

> 

>>> +  reg:
>>> +    minItems: 1
>>> +    maxItems: 2
>>
>> You should list the items.
>>
>>> +
>>> +  interrupts:
>>> +    minItems: 1
>>> +    maxItems: 2
>>
>> You should list the items.
> 
> After my check, the maxItems value of reg and interrupts should be 1.
> Will change it.

OK


Best regards,
Krzysztof

Xu Yang June 13, 2023, 7:41 a.m. UTC | #9

> -----Original Message-----
> From: Krzysztof Kozlowski <[e-mail]>
> Sent: Tuesday, June 13, 2023 3:40 PM
> To: Xu Yang <[e-mail]>; [e-mail]; [e-mail]; [e-mail]
> Cc: [e-mail]; [e-mail]; [e-mail]; [e-mail]; dl-linux-imx <linux-
> [e-mail]>; [e-mail]; [e-mail]; [e-mail]; linux-
> [e-mail]; Jun Li <[e-mail]>
> Subject: Re: [EXT] Re: [PATCH v2 1/2] dt-bindings: phy: mxs-usb-phy: convert to DT schema format
> 
> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the
> message using the 'Report this email' button
> 
> 
> On 13/06/2023 09:30, Xu Yang wrote:
> > Hi Krzysztof,
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <[e-mail]>
> >> Sent: Saturday, June 10, 2023 12:44 AM
> >> To: Xu Yang <[e-mail]>; [e-mail]; [e-mail]; [e-mail]
> >> Cc: [e-mail]; [e-mail]; [e-mail]; [e-mail]; dl-linux-imx
> <linux-
> >> [e-mail]>; [e-mail]; [e-mail]; [e-mail];
> linux-
> >> [e-mail]; Jun Li <[e-mail]>
> >> Subject: [EXT] Re: [PATCH v2 1/2] dt-bindings: phy: mxs-usb-phy: convert to DT schema format
> >>
> >> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report
> the
> >> message using the 'Report this email' button
> >>
> >>
> >> On 08/06/2023 05:36, Xu Yang wrote:
> >>> Convert the binding to DT schema format. Besides, this also add other
> >>> optional properties not contained in txt file.
> >>
> >> Why ones do you add?
> >
> > I add clock, power-domains and phy-3p0-supply to this doc. Because
> > the txt file should contain these properties but it not. I think the
> > txt doesn't get update in time.
> 
> Can you mention them in commit msg?

Sure.

Thanks,
Xu Yang

> 
> >
> 
> >>> +  reg:
> >>> +    minItems: 1
> >>> +    maxItems: 2
> >>
> >> You should list the items.
> >>
> >>> +
> >>> +  interrupts:
> >>> +    minItems: 1
> >>> +    maxItems: 2
> >>
> >> You should list the items.
> >
> > After my check, the maxItems value of reg and interrupts should be 1.
> > Will change it.
> 
> OK
> 
> 
> Best regards,
> Krzysztof

Patch

diff --git a/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
new file mode 100644
index 000000000000..1b6b19fdf491
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/fsl,mxs-usbphy.yaml
@@ -0,0 +1,128 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/fsl,mxs-usbphy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale MXS USB Phy Device
+
+maintainers:
+  - Xu Yang <[e-mail]>
+
+properties:
+  compatible:
+    oneOf:
+      - enum:
+          - fsl,imx23-usbphy
+          - fsl,vf610-usbphy
+          - fsl,imx7ulp-usbphy
+      - items:
+          - enum:
+              - fsl,imx28-usbphy
+              - fsl,imx6ul-usbphy
+              - fsl,imx6sl-usbphy
+              - fsl,imx6sx-usbphy
+              - fsl,imx6q-usbphy
+          - const: fsl,imx23-usbphy
+      - items:
+          - const: fsl,imx6sll-usbphy
+          - const: fsl,imx6ul-usbphy
+          - const: fsl,imx23-usbphy
+      - items:
+          - const: fsl,imx7ulp-usbphy
+          - const: fsl,imx6ul-usbphy
+      - items:
+          - const: fsl,imx8dxl-usbphy
+          - const: fsl,imx7ulp-usbphy
+
+  reg:
+    minItems: 1
+    maxItems: 2
+
+  interrupts:
+    minItems: 1
+    maxItems: 2
+
+  clocks:
+    maxItems: 1
+
+  '#phy-cells':
+    const: 0
+
+  power-domains:
+    maxItems: 1
+
+  fsl,anatop:
+    description:
+      phandle for anatop register, it is only for imx6 SoC series.
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  phy-3p0-supply:
+    description:
+      One of USB PHY's power supply. Can be used to keep a good signal
+      quality.
+
+  fsl,tx-cal-45-dn-ohms:
+    description:
+      Resistance (in ohms) of switchable high-speed trimming resistor
+      connected in parallel with the 45 ohm resistor that terminates
+      the DN output signal.
+    minimum: 35
+    maximum: 54
+    default: 45
+
+  fsl,tx-cal-45-dp-ohms:
+    description:
+      Resistance (in ohms) of switchable high-speed trimming resistor
+      connected in parallel with the 45 ohm resistor that terminates
+      the DP output signal.
+    minimum: 35
+    maximum: 54
+    default: 45
+
+  fsl,tx-d-cal:
+    description:
+      Current trimming value (as a percentage) of the 17.78 mA TX
+      reference current.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 79
+    maximum: 119
+    default: 100
+
+required:
+  - compatible
+  - reg
+  - clocks
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          oneOf:
+            - enum:
+              - fsl,imx6sl-usbphy
+              - fsl,imx6sx-usbphy
+              - fsl,imx6sll-usbphy
+              - fsl,imx6q-usbphy
+              - fsl,vf610-usbphy
+            - items:
+              - const: fsl,imx6ul-usbphy
+              - const: fsl,imx23-usbphy
+    then:
+      required:
+        - fsl,anatop
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    usbphy1: usb-phy@20c9000 {
+        compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
+        reg = <0x020c9000 0x1000>;
+        interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
+        fsl,anatop = <&anatop>;
+    };
+
+...
diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
deleted file mode 100644
index 70c813b0755f..000000000000
--- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
+++ /dev/null
@@ -1,33 +0,0 @@ 
-* Freescale MXS USB Phy Device
-
-Required properties:
-- compatible: should contain:
-	* "fsl,imx23-usbphy" for imx23 and imx28
-	* "fsl,imx6q-usbphy" for imx6dq and imx6dl
-	* "fsl,imx6sl-usbphy" for imx6sl
-	* "fsl,vf610-usbphy" for Vybrid vf610
-	* "fsl,imx6sx-usbphy" for imx6sx
-	* "fsl,imx7ulp-usbphy" for imx7ulp
-	* "fsl,imx8dxl-usbphy" for imx8dxl
-  "fsl,imx23-usbphy" is still a fallback for other strings
-- reg: Should contain registers location and length
-- interrupts: Should contain phy interrupt
-- fsl,anatop: phandle for anatop register, it is only for imx6 SoC series
-
-Optional properties:
-- fsl,tx-cal-45-dn-ohms: Integer [35-54]. Resistance (in ohms) of switchable
-  high-speed trimming resistor connected in parallel with the 45 ohm resistor
-  that terminates the DN output signal. Default: 45
-- fsl,tx-cal-45-dp-ohms: Integer [35-54]. Resistance (in ohms) of switchable
-  high-speed trimming resistor connected in parallel with the 45 ohm resistor
-  that terminates the DP output signal. Default: 45
-- fsl,tx-d-cal: Integer [79-119]. Current trimming value (as a percentage) of
-  the 17.78mA TX reference current. Default: 100
-
-Example:
-usbphy1: usb-phy@20c9000 {
-	compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
-	reg = <0x020c9000 0x1000>;
-	interrupts = <0 44 0x04>;
-	fsl,anatop = <&anatop>;
-};