[v4,4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY

Add the documentation explain the property about Realtek USB PHY driver.

Realtek DHC (digital home center) RTD SoCs support DWC3 XHCI USB
controller. Added the driver to drive the USB 2.0 PHY transceivers.

Signed-off-by: Stanley Chang <stanley_chang@realtek.com>
---
v3 to v4 change:
    1. Remove the parameter and non hardware properties from dts.
    2. Using the compatible data included the config and parameter
       in driver.
v2 to v3 change:
    1. Broken down into two patches, one for each of USB 2 & 3.
    2. Add more description about Realtek RTD SoCs architecture.
    3. Removed parameter v1 support for simplification.
    4. Revised the compatible name for fallback compatible.
    5. Remove some properties that can be set in the driver.
v1 to v2 change:
    Add phy-cells for generic phy driver
---
 .../bindings/phy/realtek,usb2phy.yaml         | 145 ++++++++++++++++++
 1 file changed, 145 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml

On 14/06/2023 11:28, Stanley Chang wrote:
> Add the documentation explain the property about Realtek USB PHY driver.
> 
> Realtek DHC (digital home center) RTD SoCs support DWC3 XHCI USB
> controller. Added the driver to drive the USB 2.0 PHY transceivers.
> 
> Signed-off-by: Stanley Chang <stanley_chang@realtek.com>
> ---
> v3 to v4 change:
>     1. Remove the parameter and non hardware properties from dts.
>     2. Using the compatible data included the config and parameter
>        in driver.
> v2 to v3 change:
>     1. Broken down into two patches, one for each of USB 2 & 3.
>     2. Add more description about Realtek RTD SoCs architecture.
>     3. Removed parameter v1 support for simplification.
>     4. Revised the compatible name for fallback compatible.
>     5. Remove some properties that can be set in the driver.
> v1 to v2 change:
>     Add phy-cells for generic phy driver
> ---
>  .../bindings/phy/realtek,usb2phy.yaml         | 145 ++++++++++++++++++
>  1 file changed, 145 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml b/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
> new file mode 100644
> index 000000000000..cfd77143475c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
> @@ -0,0 +1,145 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2023 Realtek Semiconductor Corporation
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/realtek,usb2phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Realtek DHC SoCs USB 2.0 PHY
> +
> +maintainers:
> +  - Stanley Chang <stanley_chang@realtek.com>
> +
> +description:
> +  Realtek USB 2.0 PHY support the digital home center (DHC) RTD series SoCs.
> +  The USB 2.0 PHY driver is designed to support the XHCI controller. The SoCs
> +  support multiple XHCI controllers. One PHY device node maps to one XHCI
> +  controller.
> +
> +  RTD1295/RTD1619 SoCs USB
> +  The USB architecture includes three XHCI controllers.
> +  Each XHCI maps to one USB 2.0 PHY and map one USB 3.0 PHY on some
> +  controllers.
> +  XHCI controller#0 -- usb2phy -- phy#0
> +                    |- usb3phy -- phy#0
> +  XHCI controller#1 -- usb2phy -- phy#0
> +  XHCI controller#2 -- usb2phy -- phy#0
> +                    |- usb3phy -- phy#0
> +
> +  RTD1395 SoCs USB
> +  The USB architecture includes two XHCI controllers.
> +  The controller#0 has one USB 2.0 PHY. The controller#1 includes two USB 2.0
> +  PHY.
> +  XHCI controller#0 -- usb2phy -- phy#0
> +  XHCI controller#1 -- usb2phy -- phy#0
> +                               |- phy#1
> +
> +  RTD1319/RTD1619b SoCs USB
> +  The USB architecture includes three XHCI controllers.
> +  Each XHCI maps to one USB 2.0 PHY and map one USB 3.0 PHY on controllers#2.
> +  XHCI controller#0 -- usb2phy -- phy#0
> +  XHCI controller#1 -- usb2phy -- phy#0
> +  XHCI controller#2 -- usb2phy -- phy#0
> +                    |- usb3phy -- phy#0
> +
> +  RTD1319d SoCs USB
> +  The USB architecture includes three XHCI controllers.
> +  Each xhci maps to one USB 2.0 PHY and map one USB 3.0 PHY on controllers#0.
> +  XHCI controller#0 -- usb2phy -- phy#0
> +                    |- usb3phy -- phy#0
> +  XHCI controller#1 -- usb2phy -- phy#0
> +  XHCI controller#2 -- usb2phy -- phy#0
> +
> +  RTD1312c/RTD1315e SoCs USB
> +  The USB architecture includes three XHCI controllers.
> +  Each XHCI maps to one USB 2.0 PHY.
> +  XHCI controller#0 -- usb2phy -- phy#0
> +  XHCI controller#1 -- usb2phy -- phy#0
> +  XHCI controller#2 -- usb2phy -- phy#0
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - realtek,rtd1295-usb2phy
> +          - realtek,rtd1312c-usb2phy
> +          - realtek,rtd1315e-usb2phy
> +          - realtek,rtd1319-usb2phy
> +          - realtek,rtd1319d-usb2phy
> +          - realtek,rtd1395-usb2phy
> +          - realtek,rtd1395-usb2phy-2port
> +          - realtek,rtd1619-usb2phy
> +          - realtek,rtd1619b-usb2phy
> +      - const: realtek,usb2phy

That's not what your driver is saying... This patchset has random set of
changes.

I suggest to drop "realtek,usb2phy".

> +
> +  reg:
> +    items:
> +      - description: PHY data registers
> +      - description: PHY control registers
> +
> +  "#phy-cells":
> +    const: 0
> +
> +  nvmem-cells:
> +    maxItems: 2
> +    description:
> +      Phandles to nvmem cell that contains the trimming data.
> +      If unspecified, default value is used.
> +
> +  nvmem-cell-names:
> +    items:
> +      - const: usb-dc-cal
> +      - const: usb-dc-dis
> +    description:
> +      The following names, which correspond to each nvmem-cells.
> +      usb-dc-cal is the driving level for each phy specified via efuse.
> +      usb-dc-dis is the disconnection level for each phy specified via efuse.
> +
> +  realtek,inverse-hstx-sync-clock:
> +    description:
> +      For one of the phys of RTD1619b SoC, the synchronous clock of the
> +      high-speed tx must be inverted. So this property is used to set the
> +      inverted clock.

Drop last sentence, it is redundant.

> +    type: boolean
> +
> +  realtek,driving-level:
> +    description:
> +      Each board or port may have a different driving capability. This
> +      will adjust the driving level value if the value is not the default.

I don't understand it. What is "driving capability" and if each port can
have it different, why do you need property for this?

You mention some default - why it is not expressed as "default: xx"?

What do the values mean?

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 31
> +
> +  realtek,driving-compensate:
> +    description:
> +      For RTD1315e SoC, the driving level can be adjusted by reading the
> +      efuse table. Therefore, this property provides drive compensation for
> +      different boards with different drive capabilities.

if driving level can be read from nvmem, why do you have
realtek,driving-level in the first place? Don't you miss here some allOf
making this constrained per variant?

"Therefore" means "for that reason" or "as a consequence". How is this
property a consequence of reading driving level from efuse? Is it then
mutually exclusive with "realtek,driving-level"? But your schema does
not express it.

> +    $ref: /schemas/types.yaml#/definitions/int32
> +    minimum: -8
> +    maximum: 8
> +
> +  realtek,disconnection-compensate:
> +    description:
> +      This adjusts the disconnection level compensation for the different
> +      boards with different disconnection level.
> +    $ref: /schemas/types.yaml#/definitions/int32
> +    minimum: -8
> +    maximum: 8
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#phy-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    usb_port0_usb2phy: usb-phy@13214 {
> +        compatible = "realtek,rtd1319d-usb2phy", "realtek,usb2phy";
> +        reg = <0x13214 0x4>, <0x28280 0x4>;
> +        #phy-cells = <0>;
> +
> +        realtek,driving-level = <0xe>;

Why this example is so empty? You have at least 6 more properties which
should be shown here.

Best regards,
Krzysztof

Hi Krzysztof,

> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - realtek,rtd1295-usb2phy
> > +          - realtek,rtd1312c-usb2phy
> > +          - realtek,rtd1315e-usb2phy
> > +          - realtek,rtd1319-usb2phy
> > +          - realtek,rtd1319d-usb2phy
> > +          - realtek,rtd1395-usb2phy
> > +          - realtek,rtd1395-usb2phy-2port
> > +          - realtek,rtd1619-usb2phy
> > +          - realtek,rtd1619b-usb2phy
> > +      - const: realtek,usb2phy
> 
> That's not what your driver is saying... This patchset has random set of
> changes.
> 
> I suggest to drop "realtek,usb2phy".

Okay, I will remove it.

> > +
> > +  reg:
> > +    items:
> > +      - description: PHY data registers
> > +      - description: PHY control registers
> > +
> > +  "#phy-cells":
> > +    const: 0
> > +
> > +  nvmem-cells:
> > +    maxItems: 2
> > +    description:
> > +      Phandles to nvmem cell that contains the trimming data.
> > +      If unspecified, default value is used.
> > +
> > +  nvmem-cell-names:
> > +    items:
> > +      - const: usb-dc-cal
> > +      - const: usb-dc-dis
> > +    description:
> > +      The following names, which correspond to each nvmem-cells.
> > +      usb-dc-cal is the driving level for each phy specified via efuse.
> > +      usb-dc-dis is the disconnection level for each phy specified via efuse.
> > +
> > +  realtek,inverse-hstx-sync-clock:
> > +    description:
> > +      For one of the phys of RTD1619b SoC, the synchronous clock of the
> > +      high-speed tx must be inverted. So this property is used to set the
> > +      inverted clock.
> 
> Drop last sentence, it is redundant.

Okay

> > +    type: boolean
> > +
> > +  realtek,driving-level:
> > +    description:
> > +      Each board or port may have a different driving capability. This
> > +      will adjust the driving level value if the value is not the default.
> 
> I don't understand it. What is "driving capability" and if each port can have it
> different, why do you need property for this?

Sorry. I didn't express myself clearly.
"Driving capability" refers to the magnitude of the Dp/Dm output swing.
For a different board or port, the original magnitude maybe not meet the specification. 
In this situation we can adjust the value to meet the specification.

> You mention some default - why it is not expressed as "default: xx"?

The default is mean hardware default value.
I can add the default value.

> What do the values mean?

The description can be modified to:
    description:
      Control the magnitude of High speed Dp/Dm output swing.
      For a different board or port, the original magnitude maybe not meet
      the specification. In this situation we can adjust the value to meet
      the specification.
    $ref: /schemas/types.yaml#/definitions/uint32
    default: 8
    minimum: 0
    maximum: 31

> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    minimum: 0
> > +    maximum: 31
> > +
> > +  realtek,driving-compensate:
> > +    description:
> > +      For RTD1315e SoC, the driving level can be adjusted by reading the
> > +      efuse table. Therefore, this property provides drive compensation
> for
> > +      different boards with different drive capabilities.
> 
> if driving level can be read from nvmem, why do you have realtek,driving-level
> in the first place? Don't you miss here some allOf making this constrained per
> variant?
> 
> "Therefore" means "for that reason" or "as a consequence". How is this
> property a consequence of reading driving level from efuse? Is it then mutually
> exclusive with "realtek,driving-level"? But your schema does not express it.

No, it's not that complicated.
In our case, not all ICs have programming efuse.
The value "realtek,deiving-level" is for non-programmed efuse ICs.

In the programmed efuse IC, we use the value of efuse to instead of "realtek,driving-level". 
If the magnitude of High speed Dp/Dm output swing still not meet the specification,
then we can set the value "driving-compensate" to meet the specification.

The description can be modified to:
  realtek,driving-compensate:
    description:
      For RTD1315e SoC, the driving level can be adjusted by reading the
      efuse table. This property provides drive compensation.
      If the magnitude of High speed Dp/Dm output swing still not meet the
      specification, then we can set this value to meet the specification.
    $ref: /schemas/types.yaml#/definitions/int32
    minimum: -8
    maximum: 8

> > +    $ref: /schemas/types.yaml#/definitions/int32
> > +    minimum: -8
> > +    maximum: 8
> > +
> > +  realtek,disconnection-compensate:
> > +    description:
> > +      This adjusts the disconnection level compensation for the different
> > +      boards with different disconnection level.
> > +    $ref: /schemas/types.yaml#/definitions/int32
> > +    minimum: -8
> > +    maximum: 8
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - "#phy-cells"
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    usb_port0_usb2phy: usb-phy@13214 {
> > +        compatible = "realtek,rtd1319d-usb2phy", "realtek,usb2phy";
> > +        reg = <0x13214 0x4>, <0x28280 0x4>;
> > +        #phy-cells = <0>;
> > +
> > +        realtek,driving-level = <0xe>;
> 
> Why this example is so empty? You have at least 6 more properties which
> should be shown here.

I will add more examples in here.
examples:
  - |
    usb_port0_usb2phy: usb-phy@13214 {
        compatible = "realtek,rtd1319d-usb2phy";
        reg = <0x13214 0x4>, <0x28280 0x4>;
        #phy-cells = <0>;
        nvmem-cells = <&otp_usb_port0_dc_cal>, <&otp_usb_port0_dc_dis>;
        nvmem-cell-names = "usb-dc-cal", "usb-dc-dis";

        realtek,driving-level = <0xe>;
    };

  - |
    port0_usb2phy: usb-phy@13214 {
        compatible = "realtek,rtd1619b-usb2phy";
        reg = <0x13214 0x4>, <0x28280 0x4>;
        #phy-cells = <0>;
        nvmem-cells = <&otp_usb_port0_dc_cal>, <&otp_usb_port0_dc_dis>;
        nvmem-cell-names = "usb-dc-cal", "usb-dc-dis";

        realtek,inverse-hstx-sync-clock;
        realtek,driving-level = <0xa>;
        realtek,driving-compensate = <(-1)>;
        realtek,disconnection-compensate = <(-1)>;
    };

    port1_usb2phy: usb-phy@13c14 {
        compatible = "realtek,rtd1619b-usb2phy";
        reg = <0x13c14 0x4>, <0x31280 0x4>;
        #phy-cells = <0>;
        nvmem-cells = <&otp_usb_port1_dc_cal>, <&otp_usb_port1_dc_dis>;
        nvmem-cell-names = "usb-dc-cal", "usb-dc-dis";

        realtek,driving-level = <8>;
        realtek,driving-compensate = <1>;
        realtek,disconnection-compensate = <0>;
    };

Thanks,
Stanley