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 <[email protected]>
---
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 | 213 ++++++++++++++++++
1 file changed, 213 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..69911e20a561
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
@@ -0,0 +1,213 @@
+# 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 <[email protected]>
+
+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,rtd1395-usb2phy
+ - realtek,rtd1619-usb2phy
+ - realtek,rtd1319-usb2phy
+ - realtek,rtd1619b-usb2phy
+ - realtek,rtd1312c-usb2phy
+ - realtek,rtd1319d-usb2phy
+ - realtek,rtd1315e-usb2phy
+ - const: realtek,usb2phy
+
+ reg:
+ items:
+ - description: PHY data registers
+ - description: PHY control registers
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ "#phy-cells":
+ const: 0
+
+ realtek,usb-ctrl:
+ description: The phandle of syscon used to control USB PHY power domain.
+ $ref: /schemas/types.yaml#/definitions/phandle
+
+patternProperties:
+ "^phy@[0-3]+$":
+ type: object
+ description:
+ Each sub-node is a PHY device for one XHCI controller.
+ For most Relatek SoCs, one XHCI controller only support one the USB 2.0
+ phy. For RTD1395 SoC, the one XHCI controller has two USB 2.0 PHYs.
+ properties:
+ realtek,page0-param:
+ description: PHY parameter at page 0. The data are the pair of the
+ offset and value.
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+
+ realtek,page1-param:
+ description: PHY parameter at page 1. The data are the pair of the
+ offset and value.
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+
+ realtek,page2-param:
+ description: PHY parameter at page 2. The data are the pair of the
+ offset and value. If the PHY support the page 2 parameter.
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+
+ realtek,support-page2-param:
+ description: Set this flag if PHY support page 2 parameter.
+ type: boolean
+
+ realtek,do-toggle:
+ description: Set this flag to enable PHY parameter toggle when port
+ status change.
+ type: boolean
+
+ realtek,do-toggle-driving:
+ description: Set this flag to enable PHY parameter toggle for adjust
+ the driving when port status change.
+ type: boolean
+
+ realtek,check-efuse:
+ description: Enable to update PHY parameter from reading otp table.
+ type: boolean
+
+ realtek,use-default-parameter:
+ description: Don't set parameter and use default value in hardware.
+ type: boolean
+
+ realtek,is-double-sensitivity-mode:
+ description: Set this flag to enable double sensitivity mode.
+ type: boolean
+
+ realtek,ldo-force-enable:
+ description: Set this flag to force enable ldo mode.
+ type: boolean
+
+ realtek,ldo-driving-compensate:
+ description: Set the value for adjust the PHY driving for ldo mode.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ realtek,driving-compensate:
+ description: Set the value for adjust the PHY driving for efuse
+ table v2.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+required:
+ - compatible
+ - reg
+ - "#address-cells"
+ - "#size-cells"
+ - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ usb_port0_usb2phy: usb-phy@13214 {
+ compatible = "realtek,rtd1319d-usb2phy", "realtek,usb2phy";
+ reg = <0x13214 0x4>, <0x28280 0x4>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ #phy-cells = <0>;
+ realtek,usb-ctrl = <&usb_ctrl>;
+
+ phy@0 {
+ reg = <0>;
+ realtek,page0-param =
+ <0xe0 0xa3>, <0xe4 0xb2>, <0xe5 0x4e>, <0xe6 0x42>;
+ realtek,page1-param = <0xe3 0x64>;
+ realtek,page2-param = <0xe7 0x45>;
+ realtek,support-page2-param;
+ realtek,do-toggle;
+ realtek.do-toggle-driving;
+ realtek,check-efuse;
+ realtek,is-double-sensitivity-mode;
+ realtek,ldo-force-enable;
+ };
+ };
+
+ - |
+ usb_port1_usb2phy: usb-phy@13c14 {
+ compatible = "realtek,rtd1395-usb2phy", "realtek,usb2phy";
+ reg = <0x132c4 0x4>, <0x31280 0x8>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ #phy-cells = <0>;
+ realtek,usb-ctrl = <&usb_ctrl>;
+
+ phy@0 {
+ reg = <0>;
+ realtek,page0-param =
+ <0xe0 0xa3>, <0xe4 0xb2>, <0xe5 0x4e>, <0xe6 0x42>;
+ realtek,page1-param = <0xe3 0x00>;
+ realtek,do-toggle;
+ realtek,check-efuse;
+ };
+ phy@1 {
+ reg = <1>;
+ realtek,page0-param =
+ <0xe0 0xe0>, <0xe4 0xb2>, <0xe5 0x4e>, <0xe6 0x42>;
+ realtek,page1-param = <0xe3 0x00>;
+ realtek,do-toggle;
+ realtek,check-efuse;
+ };
+ };
--
2.34.1
On 07/06/2023 08:24, 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 <[email protected]>
> ---
> 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 | 213 ++++++++++++++++++
> 1 file changed, 213 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..69911e20a561
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
> @@ -0,0 +1,213 @@
> +# 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 <[email protected]>
> +
> +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,rtd1395-usb2phy
> + - realtek,rtd1619-usb2phy
> + - realtek,rtd1319-usb2phy
> + - realtek,rtd1619b-usb2phy
> + - realtek,rtd1312c-usb2phy
> + - realtek,rtd1319d-usb2phy
> + - realtek,rtd1315e-usb2phy
Keep entries ordered alphabetically.
> + - const: realtek,usb2phy
> +
> + reg:
> + items:
> + - description: PHY data registers
> + - description: PHY control registers
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + "#phy-cells":
> + const: 0
> +
> + realtek,usb-ctrl:
> + description: The phandle of syscon used to control USB PHY power domain.
> + $ref: /schemas/types.yaml#/definitions/phandle
No, we have power-domains for this.
> +
> +patternProperties:
> + "^phy@[0-3]+$":
> + type: object
> + description:
> + Each sub-node is a PHY device for one XHCI controller.
I don't think it is true. You claim above that you have 0 as phy-cells,
means you have one phy. Here you say you can have up to 4 phys.
> + For most Relatek SoCs, one XHCI controller only support one the USB 2.0
> + phy. For RTD1395 SoC, the one XHCI controller has two USB 2.0 PHYs.
> + properties:
> + realtek,page0-param:
> + description: PHY parameter at page 0. The data are the pair of the
> + offset and value.
This needs to be specific. What the heck is "PHY parameter"?
> + $ref: /schemas/types.yaml#/definitions/uint32-array
Array? Then maxItems.
> +
> + realtek,page1-param:
> + description: PHY parameter at page 1. The data are the pair of the
> + offset and value.
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> +
> + realtek,page2-param:
> + description: PHY parameter at page 2. The data are the pair of the
> + offset and value. If the PHY support the page 2 parameter.
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> +
> + realtek,support-page2-param:
> + description: Set this flag if PHY support page 2 parameter.
Why this cannot be deducted from compatible?
> + type: boolean
> +
> + realtek,do-toggle:
> + description: Set this flag to enable PHY parameter toggle when port
> + status change.
Do not instruct OS what to do. Explain why this is a hardware
characteristic.
> + type: boolean
> +
> + realtek,do-toggle-driving:
> + description: Set this flag to enable PHY parameter toggle for adjust
> + the driving when port status change.
Do not instruct OS what to do. Explain why this is a hardware
characteristic.
> + type: boolean
> +
> + realtek,check-efuse:
> + description: Enable to update PHY parameter from reading otp table.
Do not instruct OS what to do. Explain why this is a hardware
characteristic.
> + type: boolean
> +
> + realtek,use-default-parameter:
> + description: Don't set parameter and use default value in hardware.
NAK, you are just making things up.
> + type: boolean
> +
> + realtek,is-double-sensitivity-mode:
> + description: Set this flag to enable double sensitivity mode.
All your descriptions copy the name of property. You basically say
nothing more. I already mentioned this before. Don't ignore the
feedback, but address it.
> + type: boolean
> +
> + realtek,ldo-force-enable:
> + description: Set this flag to force enable ldo mode.
Drop everywhere "Set this flag to", because it is redundant. Now compare
what is left with property name.
Property name: realtek,ldo-force-enable
Your description: "force enable ldo mode"
How is this helpful to anybody?
Best regards,
Krzysztof
Hi Krzysztof,
>
> On 07/06/2023 08:24, 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 <[email protected]>
> > ---
> > 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 | 213
> ++++++++++++++++++
> > 1 file changed, 213 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..69911e20a561
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
> > @@ -0,0 +1,213 @@
> > +# 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 <[email protected]>
> > +
> > +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,rtd1395-usb2phy
> > + - realtek,rtd1619-usb2phy
> > + - realtek,rtd1319-usb2phy
> > + - realtek,rtd1619b-usb2phy
> > + - realtek,rtd1312c-usb2phy
> > + - realtek,rtd1319d-usb2phy
> > + - realtek,rtd1315e-usb2phy
>
> Keep entries ordered alphabetically.
Okay.
> > + - const: realtek,usb2phy
> > +
> > + reg:
> > + items:
> > + - description: PHY data registers
> > + - description: PHY control registers
> > +
> > + "#address-cells":
> > + const: 1
> > +
> > + "#size-cells":
> > + const: 0
> > +
> > + "#phy-cells":
> > + const: 0
> > +
> > + realtek,usb-ctrl:
> > + description: The phandle of syscon used to control USB PHY power
> domain.
> > + $ref: /schemas/types.yaml#/definitions/phandle
>
> No, we have power-domains for this.
Maybe I use the word "control power domain" is not well, I just want to control the ldo of usb phy.
Revised:
The phandle of syscon used to control the ldo of USB PHY.
> > +
> > +patternProperties:
> > + "^phy@[0-3]+$":
> > + type: object
> > + description:
> > + Each sub-node is a PHY device for one XHCI controller.
>
> I don't think it is true. You claim above that you have 0 as phy-cells, means you
> have one phy. Here you say you can have up to 4 phys.
I mean the driver can support up to 4 phys.
For RTD1295 has only one phy.
For RTD1395 has two phys.
> > + For most Relatek SoCs, one XHCI controller only support one the USB
> 2.0
> > + phy. For RTD1395 SoC, the one XHCI controller has two USB 2.0
> PHYs.
> > + properties:
> > + realtek,page0-param:
> > + description: PHY parameter at page 0. The data are the pair of
> the
> > + offset and value.
>
> This needs to be specific. What the heck is "PHY parameter"?
>
> > + $ref: /schemas/types.yaml#/definitions/uint32-array
>
> Array? Then maxItems.
I have found other document.
It should be a uint32-matrix.
I will add the maxItems.
> > +
> > + realtek,page1-param:
> > + description: PHY parameter at page 1. The data are the pair of
> the
> > + offset and value.
> > + $ref: /schemas/types.yaml#/definitions/uint32-array
> > +
> > + realtek,page2-param:
> > + description: PHY parameter at page 2. The data are the pair of
> the
> > + offset and value. If the PHY support the page 2 parameter.
> > + $ref: /schemas/types.yaml#/definitions/uint32-array
> > +
> > + realtek,support-page2-param:
> > + description: Set this flag if PHY support page 2 parameter.
>
> Why this cannot be deducted from compatible?
It can identify by compatible.
>
> > + type: boolean
> > +
> > + realtek,do-toggle:
> > + description: Set this flag to enable PHY parameter toggle when
> port
> > + status change.
>
> Do not instruct OS what to do. Explain why this is a hardware characteristic.
In my original intention, we hope that this property can be used to control the phy driver do parameter toggle.
Is it a hardware characteristic? I don't think it's exactly a hardware feature.
Maybe it can be specified by the compatible.
> > + type: boolean
> > +
> > + realtek,do-toggle-driving:
> > + description: Set this flag to enable PHY parameter toggle for
> adjust
> > + the driving when port status change.
>
> Do not instruct OS what to do. Explain why this is a hardware characteristic.
>
>
> > + type: boolean
> > +
> > + realtek,check-efuse:
> > + description: Enable to update PHY parameter from reading otp
> table.
>
> Do not instruct OS what to do. Explain why this is a hardware characteristic.
Same above.
> > + type: boolean
> > +
> > + realtek,use-default-parameter:
> > + description: Don't set parameter and use default value in
> hardware.
>
> NAK, you are just making things up.
This is a software flow control.
I will remove it.
>
> > + type: boolean
> > +
> > + realtek,is-double-sensitivity-mode:
> > + description: Set this flag to enable double sensitivity mode.
>
> All your descriptions copy the name of property. You basically say nothing more.
> I already mentioned this before. Don't ignore the feedback, but address it.
I will improve this.
> > + type: boolean
> > +
> > + realtek,ldo-force-enable:
> > + description: Set this flag to force enable ldo mode.
>
> Drop everywhere "Set this flag to", because it is redundant. Now compare what
> is left with property name.
>
> Property name: realtek,ldo-force-enable
> Your description: "force enable ldo mode"
>
> How is this helpful to anybody?
This is a software flow control.
I will remove it.
Thanks,
Stanley
.
Hi Krzysztof,
> > + For most Relatek SoCs, one XHCI controller only support one the USB
> 2.0
> > + phy. For RTD1395 SoC, the one XHCI controller has two USB 2.0
> PHYs.
> > + properties:
> > + realtek,page0-param:
> > + description: PHY parameter at page 0. The data are the pair of
> the
> > + offset and value.
>
> This needs to be specific. What the heck is "PHY parameter"?
>
It contains more parameters
page0 has 16 parameters
page1 has 8 parameters
page2 has 8 parameters
It's tedious if we list them all.
And we only set the part that differs from the default.
It's hard to explain which parameters were changed because each platform is different.
Thanks,
Stanley
On 08/06/2023 09:24, Stanley Chang[昌育德] wrote:
>>> + realtek,usb-ctrl:
>>> + description: The phandle of syscon used to control USB PHY power
>> domain.
>>> + $ref: /schemas/types.yaml#/definitions/phandle
>>
>> No, we have power-domains for this.
>
> Maybe I use the word "control power domain" is not well, I just want to control the ldo of usb phy.
> Revised:
> The phandle of syscon used to control the ldo of USB PHY.
Isn't this still a power domain?
>
>>> +
>>> +patternProperties:
>>> + "^phy@[0-3]+$":
>>> + type: object
>>> + description:
>>> + Each sub-node is a PHY device for one XHCI controller.
>>
>> I don't think it is true. You claim above that you have 0 as phy-cells, means you
>> have one phy. Here you say you can have up to 4 phys.
>
> I mean the driver can support up to 4 phys.
What driver can or cannot do, does not matter. This is about hardware.
> For RTD1295 has only one phy.
> For RTD1395 has two phys.
Two phys? So how do you reference them when cells=0?
>
>>> + For most Relatek SoCs, one XHCI controller only support one the USB
>> 2.0
>>> + phy. For RTD1395 SoC, the one XHCI controller has two USB 2.0
>> PHYs.
>>> + properties:
>>> + realtek,page0-param:
>>> + description: PHY parameter at page 0. The data are the pair of
>> the
>>> + offset and value.
>>
>> This needs to be specific. What the heck is "PHY parameter"?
>>
>>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>>
>> Array? Then maxItems.
> I have found other document.
> It should be a uint32-matrix.
> I will add the maxItems.
Entire property should be dropped.
>
>>> +
>>> + realtek,page1-param:
>>> + description: PHY parameter at page 1. The data are the pair of
>> the
>>> + offset and value.
>>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>>> +
>>> + realtek,page2-param:
>>> + description: PHY parameter at page 2. The data are the pair of
>> the
>>> + offset and value. If the PHY support the page 2 parameter.
>>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>>> +
>>> + realtek,support-page2-param:
>>> + description: Set this flag if PHY support page 2 parameter.
>>
>> Why this cannot be deducted from compatible?
> It can identify by compatible.
So drop it.
>
>>
>>> + type: boolean
>>> +
>>> + realtek,do-toggle:
>>> + description: Set this flag to enable PHY parameter toggle when
>> port
>>> + status change.
>>
>> Do not instruct OS what to do. Explain why this is a hardware characteristic.
>
> In my original intention, we hope that this property can be used to control the phy driver do parameter toggle.
> Is it a hardware characteristic? I don't think it's exactly a hardware feature.
> Maybe it can be specified by the compatible.
Drop it.
>
>>> + type: boolean
>>> +
>>> + realtek,do-toggle-driving:
>>> + description: Set this flag to enable PHY parameter toggle for
>> adjust
>>> + the driving when port status change.
>>
>> Do not instruct OS what to do. Explain why this is a hardware characteristic.
>>
>>
>>> + type: boolean
>>> +
>>> + realtek,check-efuse:
>>> + description: Enable to update PHY parameter from reading otp
>> table.
>>
>> Do not instruct OS what to do. Explain why this is a hardware characteristic.
>
> Same above.
So drop all of these.
Best regards,
Krzysztof
On 08/06/2023 09:47, Stanley Chang[昌育德] wrote:
> Hi Krzysztof,
>
>
>>> + For most Relatek SoCs, one XHCI controller only support one the USB
>> 2.0
>>> + phy. For RTD1395 SoC, the one XHCI controller has two USB 2.0
>> PHYs.
>>> + properties:
>>> + realtek,page0-param:
>>> + description: PHY parameter at page 0. The data are the pair of
>> the
>>> + offset and value.
>>
>> This needs to be specific. What the heck is "PHY parameter"?
>>
> It contains more parameters
> page0 has 16 parameters
> page1 has 8 parameters
> page2 has 8 parameters
> It's tedious if we list them all.
Sure, if you prefer not to list them, then they should be removed from DT.
> And we only set the part that differs from the default.
> It's hard to explain which parameters were changed because each platform is different.
If this is phy tuning per board, you need to explain and justify them.
If this is per platform, then drop it - not even needed, because you
have compatible for this.
Best regards,
Krzysztof
> >>
> >> This needs to be specific. What the heck is "PHY parameter"?
> >>
> > It contains more parameters
> > page0 has 16 parameters
> > page1 has 8 parameters
> > page2 has 8 parameters
> > It's tedious if we list them all.
>
> Sure, if you prefer not to list them, then they should be removed from DT.
>
> > And we only set the part that differs from the default.
> > It's hard to explain which parameters were changed because each platform is
> different.
>
> If this is phy tuning per board, you need to explain and justify them.
> If this is per platform, then drop it - not even needed, because you have
> compatible for this.
>
Okay, I try to specify by the compatible.
Thanks,
Stanley
> > Maybe I use the word "control power domain" is not well, I just want to
> control the ldo of usb phy.
> > Revised:
> > The phandle of syscon used to control the ldo of USB PHY.
>
> Isn't this still a power domain?
I only control a register, it is not needed a driver of power domain.
> >
> >>> +
> >>> +patternProperties:
> >>> + "^phy@[0-3]+$":
> >>> + type: object
> >>> + description:
> >>> + Each sub-node is a PHY device for one XHCI controller.
> >>
> >> I don't think it is true. You claim above that you have 0 as
> >> phy-cells, means you have one phy. Here you say you can have up to 4 phys.
> >
> > I mean the driver can support up to 4 phys.
>
> What driver can or cannot do, does not matter. This is about hardware.
>
> > For RTD1295 has only one phy.
> > For RTD1395 has two phys.
>
> Two phys? So how do you reference them when cells=0?
About RTD1395 SoCs USB
XHCI controller#1 -- usb2phy -- phy#0
|- phy#1
One xhci controller map to one phy driver.
And one phy driver have two phys (phy@0 and phy@1).
Maybe the "phy" name is confusing.
This "phy" not mean a phy driver.
Would "port" be more appropriate?
For example,
Using phy@0 and phy@1:
usb_port1_usb2phy: usb-phy@13c14 {
compatible = "realtek,rtd1395-usb2phy", "realtek,usb2phy";
reg = <0x132c4 0x4>, <0x31280 0x8>;
#address-cells = <1>;
#size-cells = <0>;
#phy-cells = <0>;
realtek,usb-ctrl = <&usb_ctrl>;
phy@0 {
reg = <0>;
};
phy@1 {
reg = <1>;
};
};
Change: port@0 and port@1
usb_port1_usb2phy: usb-phy@13c14 {
compatible = "realtek,rtd1395-usb2phy", "realtek,usb2phy";
reg = <0x132c4 0x4>, <0x31280 0x8>;
#address-cells = <1>;
#size-cells = <0>;
#phy-cells = <0>;
realtek,usb-ctrl = <&usb_ctrl>;
prot@0 {
reg = <0>;
};
port@1 {
reg = <1>;
};
};
On 08/06/2023 10:21, Stanley Chang[昌育德] wrote:
>
>>> Maybe I use the word "control power domain" is not well, I just want to
>> control the ldo of usb phy.
>>> Revised:
>>> The phandle of syscon used to control the ldo of USB PHY.
>>
>> Isn't this still a power domain?
>
> I only control a register, it is not needed a driver of power domain.
Aren't many power domains just a registers? What about other drivers?
Don't you want in other driver control LDO of something else? And in
other something else?
>
>
>>>
>>>>> +
>>>>> +patternProperties:
>>>>> + "^phy@[0-3]+$":
>>>>> + type: object
>>>>> + description:
>>>>> + Each sub-node is a PHY device for one XHCI controller.
>>>>
>>>> I don't think it is true. You claim above that you have 0 as
>>>> phy-cells, means you have one phy. Here you say you can have up to 4 phys.
>>>
>>> I mean the driver can support up to 4 phys.
>>
>> What driver can or cannot do, does not matter. This is about hardware.
>>
>>> For RTD1295 has only one phy.
>>> For RTD1395 has two phys.
>>
>> Two phys? So how do you reference them when cells=0?
>
>
> About RTD1395 SoCs USB
> XHCI controller#1 -- usb2phy -- phy#0
> |- phy#1
> One xhci controller map to one phy driver.
> And one phy driver have two phys (phy@0 and phy@1).
>
> Maybe the "phy" name is confusing.
> This "phy" not mean a phy driver.
We do not talk about drivers, but DTS and hardware.
> Would "port" be more appropriate?
>
> For example,
> Using phy@0 and phy@1:
> usb_port1_usb2phy: usb-phy@13c14 {
> compatible = "realtek,rtd1395-usb2phy", "realtek,usb2phy";
> reg = <0x132c4 0x4>, <0x31280 0x8>;
> #address-cells = <1>;
> #size-cells = <0>;
> #phy-cells = <0>;
> realtek,usb-ctrl = <&usb_ctrl>;
>
> phy@0 {
> reg = <0>;
So such child is a NAK... you have nothing here. But it's unrelated topic.
> };
> phy@1 {
> reg = <1>;
> };
> };
>
> Change: port@0 and port@1
> usb_port1_usb2phy: usb-phy@13c14 {
> compatible = "realtek,rtd1395-usb2phy", "realtek,usb2phy";
> reg = <0x132c4 0x4>, <0x31280 0x8>;
> #address-cells = <1>;
> #size-cells = <0>;
> #phy-cells = <0>;
> realtek,usb-ctrl = <&usb_ctrl>;
>
> prot@0 {
> reg = <0>;
> };
> port@1 {
> reg = <1>;
> };
> };
This is not the answer. This is the provider. How do you reference it
from the consumer.
Upstream your entire DTS. It's frustrating to try to understand your DTS
from pieces of information you are sharing. Also very time consuming and
you are not the only one sending patches for review...
Best regards,
Krzysztof
> > I only control a register, it is not needed a driver of power domain.
>
> Aren't many power domains just a registers? What about other drivers?
> Don't you want in other driver control LDO of something else? And in other
> something else?
I will use power domain to instead this.
> > Would "port" be more appropriate?
> >
> > For example,
> > Using phy@0 and phy@1:
> > usb_port1_usb2phy: usb-phy@13c14 {
> > compatible = "realtek,rtd1395-usb2phy", "realtek,usb2phy";
> > reg = <0x132c4 0x4>, <0x31280 0x8>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> > #phy-cells = <0>;
> > realtek,usb-ctrl = <&usb_ctrl>;
> >
> > phy@0 {
> > reg = <0>;
>
> So such child is a NAK... you have nothing here. But it's unrelated topic.
Here is for simple, so some items ignore.
> > };
> > phy@1 {
> > reg = <1>;
> > };
> > };
> >
> > Change: port@0 and port@1
> > usb_port1_usb2phy: usb-phy@13c14 {
> > compatible = "realtek,rtd1395-usb2phy", "realtek,usb2phy";
> > reg = <0x132c4 0x4>, <0x31280 0x8>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> > #phy-cells = <0>;
> > realtek,usb-ctrl = <&usb_ctrl>;
> >
> > prot@0 {
> > reg = <0>;
> > };
> > port@1 {
> > reg = <1>;
> > };
> > };
>
> This is not the answer. This is the provider. How do you reference it from the
> consumer.
> Upstream your entire DTS. It's frustrating to try to understand your DTS from
> pieces of information you are sharing. Also very time consuming and you are
> not the only one sending patches for review...
Sorry to take up a lot of your time.
Apparently I don't know enough about dts.
I will reference more device tree document to understand the relating between DTS and hardware.
Thanks,
Stanley