Two documentations explain the property about realtek USB PHY drivers.
Realtek DHC (digital home center) RTD SoCs support DWC3 XHCI USB 2.0/3.0
controller. Added two drivers to drive the USB 2.0/3.0 PHY transceivers.
For USB 3.0 transceivers, a driver phy-rtk-usb3 is provided.
The driver phy-rtk-usb2 is used to support USB 2.0 transceivers.
Signed-off-by: Stanley Chang <[email protected]>
---
v1 to v2 change:
Add phy-cells for generic phy driver
---
.../bindings/phy/realtek,usb2phy.yaml | 255 ++++++++++++++++++
.../bindings/phy/realtek,usb3phy.yaml | 201 ++++++++++++++
2 files changed, 456 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
create mode 100644 Documentation/devicetree/bindings/phy/realtek,usb3phy.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..a2a69da0a163
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
@@ -0,0 +1,255 @@
+# 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 USB2 PHY
+
+maintainers:
+ - Stanley Chang <[email protected]>
+
+description: |
+ Realtek USB 2.0 PHY support the digital home center (DHC) RTD series SoCs.
+
+properties:
+ compatible:
+ enum:
+ - realtek,usb2phy
+ - realtek,rtd-usb2phy
+ - realtek,rtd1295-usb2phy
+ - realtek,rtd1395-usb2phy
+ - realtek,rtd1619-usb2phy
+ - realtek,rtd1319-usb2phy
+ - realtek,rtd1619b-usb2phy
+ - realtek,rtd1312c-usb2phy
+ - realtek,rtd1319d-usb2phy
+ - realtek,rtd1315e-usb2phy
+
+ reg:
+ items:
+ - description: PHY data registers
+ - description: PHY control registers
+
+ "#phy-cells":
+ const: 0
+
+ realtek,usb:
+ description: The phandler of realtek dwc3 node
+ $ref: /schemas/types.yaml#/definitions/phandle
+
+ realtek,mac:
+ description: The phandler of dwc3 node
+ $ref: /schemas/types.yaml#/definitions/phandle
+
+ realtek,usb_ctrl:
+ description: The phandler of usb power control node
+ $ref: /schemas/types.yaml#/definitions/phandle
+
+ realtek,port-index:
+ description: The index of USB 2.0 PHY
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ realtek,phyN:
+ description: The total amount of USB 2.0 PHY
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ phy0:
+ description: The child node of PHY for the parameter v1.
+ type: object
+ properties:
+ realtek,phy-data-page0-size:
+ description: PHY data page 0 size
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ realtek,phy-data-page0-addr:
+ description: PHY data page 0 address
+ $ref: /schemas/types.yaml#/definitions/uint8-array
+
+ realtek,phy-data-page0-A00:
+ description: PHY data page 0 value
+ $ref: /schemas/types.yaml#/definitions/uint8-array
+
+ realtek,phy-data-page1-size:
+ description: PHY data page 1 size
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ realtek,phy-data-page1-addr:
+ description: PHY data page 1 address
+ $ref: /schemas/types.yaml#/definitions/uint8-array
+
+ realtek,phy-data-page1-A00:
+ description: PHY data page 1 value
+ $ref: /schemas/types.yaml#/definitions/uint8-array
+
+ realtek,phy-data-page2-size:
+ description: PHY data page 2 size
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ realtek,phy-data-page2-addr:
+ description: PHY data page 2 address
+ $ref: /schemas/types.yaml#/definitions/uint8-array
+
+ realtek,phy-data-page2-A00:
+ description: PHY data page 2 value
+ $ref: /schemas/types.yaml#/definitions/uint8-array
+
+ realtek,do-toggle:
+ description: Do PHY parameter toggle when port status change
+ type: boolean
+
+ realtek,check-efuse:
+ description: Enable to fix PHY parameter from reading otp table
+ type: boolean
+
+ realtek,use-default-parameter:
+ description: Don't set parameter and use default value
+ type: boolean
+
+ realtek,is-double-sensitivity-mode:
+ description: Enable double sensitivity mode
+ type: boolean
+
+ realtek,ldo-page0-e4-compensate:
+ description: Adjust the PHY parameter for page 0 0xE4 for ldo mode
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ realtek,page0-e4-compensate:
+ description: Adjust the PHY parameter for page 0 0xE4
+ for efuse table v2
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ phy0_data:
+ description: The child node of PHY for parameter v2.
+ type: object
+ properties:
+ realtek,page0-size:
+ description: PHY data page 0 size
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ realtek,page0-data-A00:
+ description: PHY data page 0 address and value
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+
+ realtek,page1-size:
+ description: PHY data page 1 size
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ realtek,page1-data-A00:
+ description: PHY data page 1 address and value
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+
+ realtek,page2-size:
+ description: PHY data page 2 size
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ realtek,page2-data-A00:
+ description: PHY data page 2 address and value
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+
+ realtek,do-toggle:
+ description: Do PHY parameter toggle when port status change
+ type: boolean
+
+ realtek,do-toggle-driving:
+ description: Do PHY parameter toggle for driving when port
+ status change
+ type: boolean
+
+ realtek,check-efuse:
+ description: Enable to fix PHY parameter from reading otp table
+ type: boolean
+
+ realtek,use-default-parameter:
+ description: Don't set parameter and use default value
+ type: boolean
+
+ realtek,is-double-sensitivity-mode:
+ description: Enable double sensitivity mode
+ type: boolean
+
+ realtek,ldo-force-enable:
+ description: Force enable ldo mode
+ type: boolean
+
+ realtek,ldo-page0-e4-compensate:
+ description: Adjust the PHY parameter for page0 0xE4 for ldo mode
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ realtek,page0-e4-compensate:
+ description: Adjust the PHY parameter for page0 0xE4
+ for efuse table v2
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+required:
+ - compatible
+ - reg
+ - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ dwc3_u3drd_usb2phy: dwc3_u3drd_usb2phy@98013e14 {
+ compatible = "realtek,usb2phy";
+ reg = <0x98013e14 0x4>, <0x98058280 0x4>;
+ #phy-cells = <0>;
+ status = "okay";
+ realtek,phyN = <1>;
+
+ phy0 {
+ realtek,phy-data-page0-size = <16>;
+ realtek,phy-data-page0-addr = /bits/ 8
+ <0xE0 0xE1 0xE2 0xE3 0xE4 0xE5 0xE6 0xE7 0xF0 0xF1
+ 0xF2 0xF3 0xF4 0xF5 0xF6 0xF7>;
+ realtek,phy-data-page0-A00 = /bits/ 8
+ <0xE0 0x30 0x79 0x8D 0x6A 0x65 0x01 0x71 0xFC 0x8C
+ 0x00 0x11 0x9B 0x00 0x00 0x0A>;
+ realtek,phy-data-page0-B00 = /bits/ 8
+ <0x18 0x30 0x79 0x8D 0x6A 0x65 0x01 0x71 0xFC 0x8C
+ 0x00 0x11 0x9B 0x00 0x00 0x32>;
+ realtek,phy-data-page1-size = <8>;
+ realtek,phy-data-page1-addr = /bits/ 8
+ <0xE0 0xE1 0xE2 0xE3 0xE4 0xE5 0xE6 0xE7>;
+ realtek,phy-data-page1-A00 = /bits/ 8
+ <0x25 0xEF 0x60 0x44 0x00 0x0F 0x18 0xE3>;
+ realtek,phy-data-page2-size = <1>;
+ realtek,phy-data-page2-addr = /bits/ 8
+ <0xE0>;
+ realtek,phy-data-page2-A00 = /bits/ 8
+ <0x01>;
+ realtek,do-toggle;
+ realtek,check-efuse;
+ realtek,is-double-sensitivity-mode;
+ realtek,ldo-page0-e4-compensate = <(-2)>;
+ };
+ };
+
+ - |
+ usb_port0_usb2phy: usb_port0_usb2phy@13214 {
+ compatible = "realtek,usb2phy";
+ reg = <0x13214 0x4>, <0x28280 0x4>;
+ #phy-cells = <0>;
+ realtek,usb = <&usb_port0>;
+ realtek,mac = <&port0_dwc3>;
+ realtek,usb_ctrl = <&usb_ctrl>;
+
+ realtek,port-index = <0>;
+ realtek,phyN = <1>;
+ phy0_data {
+ realtek,page0-size = <16>;
+ realtek,page0-data-A00 = /* < addr data > */
+ <0xE0 0xA3>, <0xE4 0xB2>, <0xE5 0x4F>, <0xE6 0x42>;
+ realtek,page1-size = <8>;
+ realtek,page1-data-A00 = <0xE3 0x64>;
+ realtek,page2-size = <8>;
+ realtek,page2-data-A00 = <0xE7 0x45>;
+ realtek,do-toggle;
+ realtek.do-toggle-driving;
+ realtek,disconnect-driving-updated = <0x8>;
+ realtek,check-efuse;
+ realtek,is-double-sensitivity-mode;
+ realtek,ldo-force-enable;
+ };
+ };
diff --git a/Documentation/devicetree/bindings/phy/realtek,usb3phy.yaml b/Documentation/devicetree/bindings/phy/realtek,usb3phy.yaml
new file mode 100644
index 000000000000..2d2543acfb5d
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/realtek,usb3phy.yaml
@@ -0,0 +1,201 @@
+# 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,usb3phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Realtek DHC SoCs USB3 PHY
+
+maintainers:
+ - Stanley Chang <[email protected]>
+
+description: |
+ Realtek USB 3.0 PHY support the digital home center (DHC) RTD series SoCs.
+
+properties:
+ compatible:
+ enum:
+ - realtek,usb3phy
+ - realtek,rtd-usb3phy
+ - realtek,rtd1295-usb3phy
+ - realtek,rtd1619-usb3phy
+ - realtek,rtd1319-usb3phy
+ - realtek,rtd1619b-usb3phy
+ - realtek,rtd1319d-usb3phy
+
+ reg:
+ maxItems: 1
+
+ "#phy-cells":
+ const: 0
+
+ realtek,usb:
+ description: The phandler of realtek dwc3 node
+ $ref: /schemas/types.yaml#/definitions/phandle
+
+ realtek,mac:
+ description: The phandler of dwc3 node
+ $ref: /schemas/types.yaml#/definitions/phandle
+
+ realtek,usb_ctrl:
+ description: The phandler of usb power control node
+ $ref: /schemas/types.yaml#/definitions/phandle
+
+ realtek,port-index:
+ description: The index of USB 3.0 PHY
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ realtek,phyN:
+ description: The total amount of USB 3.0 PHY
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ phy0:
+ description: The child node of PHY for parameter v1.
+ type: object
+ properties:
+ realtek,phy-data-size:
+ description: PHY data size
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ realtek,phy-data-addr:
+ description: PHY data address
+ $ref: /schemas/types.yaml#/definitions/uint8-array
+
+ realtek,phy-data-A00:
+ description: PHY data value
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+
+ realtek,do-toggle:
+ description: Do PHY parameter toggle when port status change
+ type: boolean
+
+ realtek,do-toggle-once:
+ description: Do PHY parameter toggle only on PHY init
+ type: boolean
+
+ realtek,check-efuse:
+ description: Enable to fix PHY parameter from reading otp table
+ type: boolean
+
+ realtek,use-default-parameter:
+ description: Don't set parameter and use default value
+ type: boolean
+
+ realtek,check-rx-front-end-offset:
+ description: Enable to check rx front end offset
+ type: boolean
+
+ phy0_data:
+ description: The child node of PHY for parameter v2.
+ type: object
+ properties:
+ realtek,phy-data-size:
+ description: PHY data size
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ realtek,phy-data-A00:
+ description: PHY data address and value
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+
+ realtek,do-toggle:
+ description: Do PHY parameter toggle when port status change
+ type: boolean
+
+ realtek,do-toggle-once:
+ description: Do PHY parameter toggle only on phy init
+ type: boolean
+
+ realtek,check-efuse:
+ description: Enable to fix PHY parameter from reading otp table
+ type: boolean
+
+ realtek,use-default-parameter:
+ description: Don't set parameter and use default value
+ type: boolean
+
+ realtek,check-rx-front-end-offset:
+ description: Enable to check rx front end offset
+ type: boolean
+
+required:
+ - compatible
+ - reg
+ - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ dwc3_u3drd_usb3phy: dwc3_u3drd_usb3phy@98013e10 {
+ compatible = "realtek,usb3phy";
+ reg = <0x98013e10 0x4>;
+ #phy-cells = <0>;
+ status = "okay";
+ realtek,port-index = <0>; /* index in u3 port */
+ realtek,phyN = <1>;
+
+ phy0 {
+ realtek,phy-data-size = <0x30>;
+ realtek,phy-data-addr = /bits/ 8
+ <0x00 0x01 0x02 0x03 0x04 0x05 0x06
+ 0x07 0x08 0x09 0x0A 0x0B 0x0C 0x0D
+ 0x0E 0x0F 0x10 0x11 0x12 0x13 0x14
+ 0x15 0x16 0x17 0x18 0x19 0x1A 0x1B
+ 0x1C 0x1D 0x1E 0x1F 0x20 0x21 0x22
+ 0x23 0x24 0x25 0x26 0x27 0x28 0x29
+ 0x2A 0x2B 0x2C 0x2D 0x2E 0x2F>;
+ realtek,phy-data-A00 = /bits/ 16
+ <0x400C 0xAC86 0x6042 0x2771 0x72F5 0x2AD3 0x0003
+ 0x2E00 0x3591 0x925C 0xA608 0xA905 0xC000 0xEF1E
+ 0x2010 0x8D50 0x000C 0x4C10 0xFC00 0x0C81 0xDE01
+ 0x0000 0x0000 0x0000 0x0000 0x6000 0x0085 0x2014
+ 0xC900 0xA03F 0xC2E0 0x7E00 0x705A 0xF645 0x0013
+ 0xCB66 0x4770 0x126C 0x840A 0x01D6 0xF802 0xff04
+ 0x3040 0x8028 0xFFFF 0xFFFF 0x0000 0x8600>;
+ realtek,phy-data-B00 = /bits/ 16
+ <0x400C 0xAC86 0x6042 0x2771 0x72F5 0x2AD3 0x0003
+ 0x2E00 0x3591 0x924C 0xA608 0xB905 0xC000 0xEF1E
+ 0x2010 0x8D50 0x000C 0x4C10 0xFC00 0x0C81 0xDE01
+ 0x0000 0x0000 0x0000 0x0000 0x6000 0x0085 0x2014
+ 0xC900 0xA03F 0xC2E0 0x7E00 0x705A 0xF645 0x0013
+ 0xCB66 0x4770 0x126C 0x840A 0x01D6 0xF802 0xff04
+ 0x3040 0x8028 0xFFFF 0xFFFF 0x0000 0x8600>;
+ realtek,do-toggle;
+ };
+ };
+
+ - |
+ usb_port2_usb3phy: usb_port2_usb3phy@13e10 {
+ compatible = "realtek,usb3phy";
+ reg = <0x13e10 0x4>;
+ #phy-cells = <0>;
+ realtek,usb = <&usb_port2>;
+ realtek,mac = <&port2_dwc3>;
+ realtek,usb_ctrl = <&usb_ctrl>;
+
+ realtek,port-index = <0>; /* index in u3 port */
+ realtek,phyN = <1>;
+ phy0_data {
+ realtek,phy-data-size = <0x30>;
+ realtek,phy-data-A00 = /* <addr data> */
+ <0x01 0xAC8C>,
+ <0x06 0x0017>,
+ <0x09 0x724C>,
+ <0x0B 0xB90D>,
+ <0x0A 0xB610>,
+ <0x0D 0xEF2A>,
+ <0x0F 0x9050>,
+ <0x10 0x000C>,
+ <0x20 0x70FF>,
+ <0x21 0xCFAA>,
+ <0x22 0x0013>,
+ <0x23 0xDB66>,
+ <0x26 0x8609>,
+ <0x29 0xFF13>,
+ <0x2A 0x3070>;
+ realtek,do-toggle-once;
+ realtek,check_efuse;
+ };
+ };
+
--
2.34.1
Hey,
On Thu, May 25, 2023 at 10:26:04AM +0800, Stanley Chang wrote:
> +properties:
> + compatible:
> + enum:
> + - realtek,usb2phy
> + - realtek,rtd-usb2phy
> + - realtek,rtd1295-usb2phy
> + - realtek,rtd1395-usb2phy
> + - realtek,rtd1619-usb2phy
> + - realtek,rtd1319-usb2phy
> + - realtek,rtd1619b-usb2phy
> + - realtek,rtd1312c-usb2phy
> + - realtek,rtd1319d-usb2phy
> + - realtek,rtd1315e-usb2phy
> +properties:
> + compatible:
> + enum:
> + - realtek,usb3phy
> + - realtek,rtd-usb3phy
> + - realtek,rtd1295-usb3phy
> + - realtek,rtd1619-usb3phy
> + - realtek,rtd1319-usb3phy
> + - realtek,rtd1619b-usb3phy
> + - realtek,rtd1319d-usb3phy
Ignoring everything else, because I really want Krzysztof or Rob to
review this rather than me, but what's going on here with the
compatibles?
What hardware do "usbNphy" and "rtd-usbNphy" represent?
You have device-specific compatibles, which is great, but you also allow
only those two generic ones. I had a _brief_ look at the driver, and it
seems like there is no decision making done based on the compatibles,
only on the properties. Is that correct?
If it is, I would understand having "realtek,usb3phy" as a fallback
compatible for "realtek,rtd1619-usb3phy", but I do not get the current
setup.
Also, I really think this should be broken down into two patches, one
for each of USB 2 & 3.
Cheers,
Conor.
Hi Conor,
> > +properties:
> > + compatible:
> > + enum:
> > + - realtek,usb2phy
> > + - realtek,rtd-usb2phy
> > + - realtek,rtd1295-usb2phy
> > + - realtek,rtd1395-usb2phy
> > + - realtek,rtd1619-usb2phy
> > + - realtek,rtd1319-usb2phy
> > + - realtek,rtd1619b-usb2phy
> > + - realtek,rtd1312c-usb2phy
> > + - realtek,rtd1319d-usb2phy
> > + - realtek,rtd1315e-usb2phy
> > +properties:
> > + compatible:
> > + enum:
> > + - realtek,usb3phy
> > + - realtek,rtd-usb3phy
> > + - realtek,rtd1295-usb3phy
> > + - realtek,rtd1619-usb3phy
> > + - realtek,rtd1319-usb3phy
> > + - realtek,rtd1619b-usb3phy
> > + - realtek,rtd1319d-usb3phy
> Ignoring everything else, because I really want Krzysztof or Rob to
> review this rather than me, but what's going on here with the
> compatibles?
> What hardware do "usbNphy" and "rtd-usbNphy" represent?
>
> You have device-specific compatibles, which is great, but you also allow
> only those two generic ones. I had a _brief_ look at the driver, and it
> seems like there is no decision making done based on the compatibles,
> only on the properties. Is that correct?
> If it is, I would understand having "realtek,usb3phy" as a fallback
> compatible for "realtek,rtd1619-usb3phy", but I do not get the current
> setup.
This driver is compatible with all Realtek RTD SoCs without specifying different settings.
So use "realtek,usb3phy" as fallback compatible for all SoCs.
This is the compatible name we use.
Other compatible names simply indicate that the driver supports the SoCs.
The name "usbNphy" and "rtd-usbNphy" seem to be more generic for all RTD SoCs,
but they are not device-specific compatible.
Do you have a better suggestion?
>
> Also, I really think this should be broken down into two patches, one
> for each of USB 2 & 3.
Okay, I will split to two patches.
Thanks,
Stanley
On Tue, May 30, 2023 at 03:08:29AM +0000, Stanley Chang[昌育德] wrote:
> Hi Conor,
>
> > > +properties:
> > > + compatible:
> > > + enum:
> > > + - realtek,usb2phy
> > > + - realtek,rtd-usb2phy
> > > + - realtek,rtd1295-usb2phy
> > > + - realtek,rtd1395-usb2phy
> > > + - realtek,rtd1619-usb2phy
> > > + - realtek,rtd1319-usb2phy
> > > + - realtek,rtd1619b-usb2phy
> > > + - realtek,rtd1312c-usb2phy
> > > + - realtek,rtd1319d-usb2phy
> > > + - realtek,rtd1315e-usb2phy
>
> > > +properties:
> > > + compatible:
> > > + enum:
> > > + - realtek,usb3phy
> > > + - realtek,rtd-usb3phy
> > > + - realtek,rtd1295-usb3phy
> > > + - realtek,rtd1619-usb3phy
> > > + - realtek,rtd1319-usb3phy
> > > + - realtek,rtd1619b-usb3phy
> > > + - realtek,rtd1319d-usb3phy
>
> > Ignoring everything else, because I really want Krzysztof or Rob to
> > review this rather than me, but what's going on here with the
> > compatibles?
> > What hardware do "usbNphy" and "rtd-usbNphy" represent?
> >
> > You have device-specific compatibles, which is great, but you also allow
> > only those two generic ones. I had a _brief_ look at the driver, and it
> > seems like there is no decision making done based on the compatibles,
> > only on the properties. Is that correct?
> > If it is, I would understand having "realtek,usb3phy" as a fallback
> > compatible for "realtek,rtd1619-usb3phy", but I do not get the current
> > setup.
>
> This driver is compatible with all Realtek RTD SoCs without specifying different settings.
> So use "realtek,usb3phy" as fallback compatible for all SoCs.
> This is the compatible name we use.
> Other compatible names simply indicate that the driver supports the SoCs.
Then you should write the binding such that having fallback compatibles
is permitted. Try plugging
compatible = "realtek,rtd1295-usb2phy", "realtek,rtd-usb2phy", "realtek,usb2phy";
into your example below and see what happens.
> The name "usbNphy" and "rtd-usbNphy" seem to be more generic for all RTD SoCs,
> but they are not device-specific compatible.
> Do you have a better suggestion?
Write the binding so that having fallback compatibles in the DT actually
works, don't add the SoC-specific ones merely as indicators that those
SoCs are supported and don't permit "realtek,usbNphy" or
"realtek,rtd-usbNphy" in isolation ;)
Cheers,
Conor.
On 30/05/2023 04:53, Stanley Chang[昌育德] wrote:
>>> +properties:
>>> + compatible:
>>> + enum:
>>> + - realtek,usb3phy
>>> + - realtek,rtd-usb3phy
>>> + - realtek,rtd1295-usb3phy
>>> + - realtek,rtd1619-usb3phy
>>> + - realtek,rtd1319-usb3phy
>>> + - realtek,rtd1619b-usb3phy
>>> + - realtek,rtd1319d-usb3phy
>
>> Ignoring everything else, because I really want Krzysztof or Rob to
>> review this rather than me, but what's going on here with the
>> compatibles?
>> What hardware do "usbNphy" and "rtd-usbNphy" represent?
>>
>> You have device-specific compatibles, which is great, but you also allow
>> only those two generic ones. I had a _brief_ look at the driver, and it
>> seems like there is no decision making done based on the compatibles,
>> only on the properties. Is that correct?
>> If it is, I would understand having "realtek,usb3phy" as a fallback
>> compatible for "realtek,rtd1619-usb3phy", but I do not get the current
>> setup.
>
> This driver is compatible with all Realtek RTD SoCs without specifying different settings.
> So use "realtek,usb3phy" as fallback compatible for all SoCs.
Binding says something entirely else...
Best regards,
Krzysztof
On 25/05/2023 04:26, Stanley Chang wrote:
> Two documentations explain the property about realtek USB PHY drivers.
>
Thank you for your patch. There is something to discuss/improve.
Actually a lot... The bindings are not suitable for review.
> Realtek DHC (digital home center) RTD SoCs support DWC3 XHCI USB 2.0/3.0
> controller. Added two drivers to drive the USB 2.0/3.0 PHY transceivers.
> For USB 3.0 transceivers, a driver phy-rtk-usb3 is provided.
> The driver phy-rtk-usb2 is used to support USB 2.0 transceivers.
>
> Signed-off-by: Stanley Chang <[email protected]>
> ---
> v1 to v2 change:
> Add phy-cells for generic phy driver
> ---
> .../bindings/phy/realtek,usb2phy.yaml | 255 ++++++++++++++++++
> .../bindings/phy/realtek,usb3phy.yaml | 201 ++++++++++++++
> 2 files changed, 456 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
> create mode 100644 Documentation/devicetree/bindings/phy/realtek,usb3phy.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..a2a69da0a163
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
> @@ -0,0 +1,255 @@
> +# 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 USB2 PHY
> +
> +maintainers:
> + - Stanley Chang <[email protected]>
> +
> +description: |
Do not need '|' unless you need to preserve formatting.
> + Realtek USB 2.0 PHY support the digital home center (DHC) RTD series SoCs.
> +
> +properties:
> + compatible:
> + enum:
> + - realtek,usb2phy
As pointed out by Conor, this does not make sense.
> + - realtek,rtd-usb2phy
> + - realtek,rtd1295-usb2phy
> + - realtek,rtd1395-usb2phy
> + - realtek,rtd1619-usb2phy
> + - realtek,rtd1319-usb2phy
> + - realtek,rtd1619b-usb2phy
> + - realtek,rtd1312c-usb2phy
> + - realtek,rtd1319d-usb2phy
> + - realtek,rtd1315e-usb2phy
> +
> + reg:
> + items:
> + - description: PHY data registers
> + - description: PHY control registers
> +
> + "#phy-cells":
> + const: 0
> +
> + realtek,usb:
> + description: The phandler of realtek dwc3 node
"phandler"? Except obvious typo, drop "The phandler of" and describe
what is it for.
> + $ref: /schemas/types.yaml#/definitions/phandle
Anyway, it shouldn't be here. No, no.
> +
> + realtek,mac:
> + description: The phandler of dwc3 node
> + $ref: /schemas/types.yaml#/definitions/phandle
NAK.
> +
> + realtek,usb_ctrl:
> + description: The phandler of usb power control node
> + $ref: /schemas/types.yaml#/definitions/phandle
NAK for similar reasons (nothing here justifies it existence). Also, do
not use underscores in node names.
> +
> + realtek,port-index:
> + description: The index of USB 2.0 PHY
> + $ref: /schemas/types.yaml#/definitions/uint32
No. No reason for this. You have reg.
> +
> + realtek,phyN:
> + description: The total amount of USB 2.0 PHY
> + $ref: /schemas/types.yaml#/definitions/uint32
No. Compatible defines it.
> +
> + phy0:
> + description: The child node of PHY for the parameter v1.
??? Open other phy bindings and use them as example.
> + type: object
> + properties:
> + realtek,phy-data-page0-size:
> + description: PHY data page 0 size
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> + realtek,phy-data-page0-addr:
> + description: PHY data page 0 address
> + $ref: /schemas/types.yaml#/definitions/uint8-array
> +
> + realtek,phy-data-page0-A00:
> + description: PHY data page 0 value
> + $ref: /schemas/types.yaml#/definitions/uint8-array
> +
> + realtek,phy-data-page1-size:
> + description: PHY data page 1 size
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> + realtek,phy-data-page1-addr:
> + description: PHY data page 1 address
> + $ref: /schemas/types.yaml#/definitions/uint8-array
> +
> + realtek,phy-data-page1-A00:
> + description: PHY data page 1 value
> + $ref: /schemas/types.yaml#/definitions/uint8-array
> +
> + realtek,phy-data-page2-size:
> + description: PHY data page 2 size
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> + realtek,phy-data-page2-addr:
> + description: PHY data page 2 address
> + $ref: /schemas/types.yaml#/definitions/uint8-array
> +
> + realtek,phy-data-page2-A00:
> + description: PHY data page 2 value
> + $ref: /schemas/types.yaml#/definitions/uint8-array
> +
> + realtek,do-toggle:
> + description: Do PHY parameter toggle when port status change
> + type: boolean
> +
> + realtek,check-efuse:
> + description: Enable to fix PHY parameter from reading otp table
> + type: boolean
> +
> + realtek,use-default-parameter:
> + description: Don't set parameter and use default value
> + type: boolean
> +
> + realtek,is-double-sensitivity-mode:
> + description: Enable double sensitivity mode
> + type: boolean
> +
> + realtek,ldo-page0-e4-compensate:
> + description: Adjust the PHY parameter for page 0 0xE4 for ldo mode
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> + realtek,page0-e4-compensate:
> + description: Adjust the PHY parameter for page 0 0xE4
> + for efuse table v2
> + $ref: /schemas/types.yaml#/definitions/uint32
I don't understand what's all this for. Most of these descriptions do
not explain anything except duplicating name of property.
> +
> + phy0_data:
> + description: The child node of PHY for parameter v2.
Even more question marks. We are getting close to what the hell is this
binding?
> + type: object
> + properties:
> + realtek,page0-size:
> + description: PHY data page 0 size
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> + realtek,page0-data-A00:
> + description: PHY data page 0 address and value
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> +
> + realtek,page1-size:
> + description: PHY data page 1 size
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> + realtek,page1-data-A00:
> + description: PHY data page 1 address and value
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> +
> + realtek,page2-size:
> + description: PHY data page 2 size
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> + realtek,page2-data-A00:
> + description: PHY data page 2 address and value
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> +
> + realtek,do-toggle:
> + description: Do PHY parameter toggle when port status change
> + type: boolean
> +
> + realtek,do-toggle-driving:
> + description: Do PHY parameter toggle for driving when port
> + status change
> + type: boolean
> +
> + realtek,check-efuse:
> + description: Enable to fix PHY parameter from reading otp table
> + type: boolean
> +
> + realtek,use-default-parameter:
> + description: Don't set parameter and use default value
> + type: boolean
> +
> + realtek,is-double-sensitivity-mode:
> + description: Enable double sensitivity mode
> + type: boolean
> +
> + realtek,ldo-force-enable:
> + description: Force enable ldo mode
> + type: boolean
> +
> + realtek,ldo-page0-e4-compensate:
> + description: Adjust the PHY parameter for page0 0xE4 for ldo mode
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> + realtek,page0-e4-compensate:
> + description: Adjust the PHY parameter for page0 0xE4
> + for efuse table v2
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> +required:
> + - compatible
> + - reg
> + - "#phy-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + dwc3_u3drd_usb2phy: dwc3_u3drd_usb2phy@98013e14 {
You must be joking with the node name.
Node names should be generic. See also explanation and list of examples
in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
Make it proper DTS, not some downstream unacceptable code.
> + compatible = "realtek,usb2phy";
> + reg = <0x98013e14 0x4>, <0x98058280 0x4>;
> + #phy-cells = <0>;
> + status = "okay";
Drop.
> + realtek,phyN = <1>;
> +
> + phy0 {
Why not phy9999?
> + realtek,phy-data-page0-size = <16>;
> + realtek,phy-data-page0-addr = /bits/ 8
> + <0xE0 0xE1 0xE2 0xE3 0xE4 0xE5 0xE6 0xE7 0xF0 0xF1
lowercase hex
> + 0xF2 0xF3 0xF4 0xF5 0xF6 0xF7>;
> + realtek,phy-data-page0-A00 = /bits/ 8
> + <0xE0 0x30 0x79 0x8D 0x6A 0x65 0x01 0x71 0xFC 0x8C
> + 0x00 0x11 0x9B 0x00 0x00 0x0A>;
> + realtek,phy-data-page0-B00 = /bits/ 8
> + <0x18 0x30 0x79 0x8D 0x6A 0x65 0x01 0x71 0xFC 0x8C
> + 0x00 0x11 0x9B 0x00 0x00 0x32>;
> + realtek,phy-data-page1-size = <8>;
> + realtek,phy-data-page1-addr = /bits/ 8
> + <0xE0 0xE1 0xE2 0xE3 0xE4 0xE5 0xE6 0xE7>;
> + realtek,phy-data-page1-A00 = /bits/ 8
> + <0x25 0xEF 0x60 0x44 0x00 0x0F 0x18 0xE3>;
> + realtek,phy-data-page2-size = <1>;
> + realtek,phy-data-page2-addr = /bits/ 8
> + <0xE0>;
> + realtek,phy-data-page2-A00 = /bits/ 8
> + <0x01>;
> + realtek,do-toggle;
> + realtek,check-efuse;
> + realtek,is-double-sensitivity-mode;
> + realtek,ldo-page0-e4-compensate = <(-2)>;
> + };
> + };
> +
> + - |
> + usb_port0_usb2phy: usb_port0_usb2phy@13214 {
> + compatible = "realtek,usb2phy";
> + reg = <0x13214 0x4>, <0x28280 0x4>;
> + #phy-cells = <0>;
> + realtek,usb = <&usb_port0>;
> + realtek,mac = <&port0_dwc3>;
> + realtek,usb_ctrl = <&usb_ctrl>;
> +
> + realtek,port-index = <0>;
> + realtek,phyN = <1>;
> + phy0_data {
> + realtek,page0-size = <16>;
> + realtek,page0-data-A00 = /* < addr data > */
> + <0xE0 0xA3>, <0xE4 0xB2>, <0xE5 0x4F>, <0xE6 0x42>;
> + realtek,page1-size = <8>;
> + realtek,page1-data-A00 = <0xE3 0x64>;
> + realtek,page2-size = <8>;
> + realtek,page2-data-A00 = <0xE7 0x45>;
> + realtek,do-toggle;
> + realtek.do-toggle-driving;
> + realtek,disconnect-driving-updated = <0x8>;
> + realtek,check-efuse;
> + realtek,is-double-sensitivity-mode;
> + realtek,ldo-force-enable;
> + };
> + };
> diff --git a/Documentation/devicetree/bindings/phy/realtek,usb3phy.yaml b/Documentation/devicetree/bindings/phy/realtek,usb3phy.yaml
> new file mode 100644
> index 000000000000..2d2543acfb5d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/realtek,usb3phy.yaml
> @@ -0,0 +1,201 @@
> +# 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,usb3phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Realtek DHC SoCs USB3 PHY
> +
> +maintainers:
> + - Stanley Chang <[email protected]>
> +
> +description: |
> + Realtek USB 3.0 PHY support the digital home center (DHC) RTD series SoCs.
> +
> +properties:
> + compatible:
> + enum:
> + - realtek,usb3phy
> + - realtek,rtd-usb3phy
> + - realtek,rtd1295-usb3phy
> + - realtek,rtd1619-usb3phy
> + - realtek,rtd1319-usb3phy
> + - realtek,rtd1619b-usb3phy
> + - realtek,rtd1319d-usb3phy
Does not make sense...
> +
> + reg:
> + maxItems: 1
> +
> + "#phy-cells":
> + const: 0
One huge NAK for these bindings. It looks like copy-paste from
downstream stuff which should never be sent as is to upstream. I am
sorry for being harsh, but amount of questions, coding and naming
styles, incorrect choices is just too big to handle in one review.
Best regards,
Krzysztof
Hi Conor,
> > > You have device-specific compatibles, which is great, but you also allow
> > > only those two generic ones. I had a _brief_ look at the driver, and it
> > > seems like there is no decision making done based on the compatibles,
> > > only on the properties. Is that correct?
> > > If it is, I would understand having "realtek,usb3phy" as a fallback
> > > compatible for "realtek,rtd1619-usb3phy", but I do not get the current
> > > setup.
> >
> > This driver is compatible with all Realtek RTD SoCs without specifying different settings.
> > So use "realtek,usb3phy" as fallback compatible for all SoCs.
> > This is the compatible name we use.
> > Other compatible names simply indicate that the driver supports the SoCs.
>
> Then you should write the binding such that having fallback compatibles
> is permitted. Try plugging
> compatible = "realtek,rtd1295-usb2phy", "realtek,rtd-usb2phy", "realtek,usb2phy";
> into your example below and see what happens.
>
> > The name "usbNphy" and "rtd-usbNphy" seem to be more generic for all RTD SoCs,
> > but they are not device-specific compatible.
> > Do you have a better suggestion?
>
> Write the binding so that having fallback compatibles in the DT actually
> works, don't add the SoC-specific ones merely as indicators that those
> SoCs are supported and don't permit "realtek,usbNphy" or
> "realtek,rtd-usbNphy" in isolation ;)
>
As far as I understand what you mean.
I should follow other docs to define compatible.
Reference:
Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml
drivers/phy/mediatek/phy-mtk-xsphy.c
For example:
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
examples:
-
dwc3_u3drd_usb2phy: dwc3_u3drd_usb2phy@98013e14 {
compatible = "realtek,rtd1319-usb2phy", "realtek,usb2phy";
And use only "Realtek, usb2phy" in the driver.
static const struct of_device_id usbphy_rtk_dt_match[] = {
{ .compatible = "realtek,usb2phy", },
{},
};
Hi Krzysztof,
> Thank you for your patch. There is something to discuss/improve.
>
> Actually a lot... The bindings are not suitable for review.
Thanks for your patience in reviewing my patches.
Most of the properties are about the phy parameters.
Is the phy parameter data suitable to be placed in DTS?
I referenced other phy drivers.
These parameters should not be defined in dts.
I would move the parameters to the driver.
> > + realtek,usb:
> > + description: The phandler of realtek dwc3 node
>
> "phandler"? Except obvious typo, drop "The phandler of" and describe what is
> it for.
realtek,usb is a phandle of syscon used to control realtek dwc3 register.
> > + $ref: /schemas/types.yaml#/definitions/phandle
>
> Anyway, it shouldn't be here. No, no.
Can I use it for phandle of syscon?
> > + realtek,port-index:
> > + description: The index of USB 2.0 PHY
> > + $ref: /schemas/types.yaml#/definitions/uint32
>
> No. No reason for this. You have reg.
This index is used for phy parameters. I will move it to phy driver.
> > +
> > + realtek,phyN:
> > + description: The total amount of USB 2.0 PHY
> > + $ref: /schemas/types.yaml#/definitions/uint32
>
> No. Compatible defines it.
This amount is used for phy parameters. I will move it to phy driver.
> > +
> > + phy0:
> > + description: The child node of PHY for the parameter v1.
>
> ??? Open other phy bindings and use them as example.
phy0 Child node is defined to assign the phy parameter.
I will remove it.
> > + type: object
> > + properties:
> > + realtek,phy-data-page0-size:
> > + description: PHY data page 0 size
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > + realtek,phy-data-page0-addr:
> > + description: PHY data page 0 address
> > + $ref: /schemas/types.yaml#/definitions/uint8-array
> > +
> > + realtek,phy-data-page0-A00:
> > + description: PHY data page 0 value
> > + $ref: /schemas/types.yaml#/definitions/uint8-array
> > +
> > + realtek,phy-data-page1-size:
> > + description: PHY data page 1 size
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > + realtek,phy-data-page1-addr:
> > + description: PHY data page 1 address
> > + $ref: /schemas/types.yaml#/definitions/uint8-array
> > +
> > + realtek,phy-data-page1-A00:
> > + description: PHY data page 1 value
> > + $ref: /schemas/types.yaml#/definitions/uint8-array
> > +
> > + realtek,phy-data-page2-size:
> > + description: PHY data page 2 size
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > + realtek,phy-data-page2-addr:
> > + description: PHY data page 2 address
> > + $ref: /schemas/types.yaml#/definitions/uint8-array
> > +
> > + realtek,phy-data-page2-A00:
> > + description: PHY data page 2 value
> > + $ref: /schemas/types.yaml#/definitions/uint8-array
> > +
> > + realtek,do-toggle:
> > + description: Do PHY parameter toggle when port status change
> > + type: boolean
> > +
> > + realtek,check-efuse:
> > + description: Enable to fix PHY parameter from reading otp table
> > + type: boolean
> > +
> > + realtek,use-default-parameter:
> > + description: Don't set parameter and use default value
> > + type: boolean
> > +
> > + realtek,is-double-sensitivity-mode:
> > + description: Enable double sensitivity mode
> > + type: boolean
> > +
> > + realtek,ldo-page0-e4-compensate:
> > + description: Adjust the PHY parameter for page 0 0xE4 for ldo
> mode
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > + realtek,page0-e4-compensate:
> > + description: Adjust the PHY parameter for page 0 0xE4
> > + for efuse table v2
> > + $ref: /schemas/types.yaml#/definitions/uint32
>
> I don't understand what's all this for. Most of these descriptions do not explain
> anything except duplicating name of property.
I'll simplify these properties and remove the one about the phy parameter.
> One huge NAK for these bindings. It looks like copy-paste from downstream
> stuff which should never be sent as is to upstream. I am sorry for being harsh,
> but amount of questions, coding and naming styles, incorrect choices is just too
> big to handle in one review.
>
I will refactor this dts based on your comments.
Thanks,
Stanley
On 01/06/2023 12:49, Stanley Chang[昌育德] wrote:
> Hi Krzysztof,
>
>> Thank you for your patch. There is something to discuss/improve.
>>
>> Actually a lot... The bindings are not suitable for review.
>
> Thanks for your patience in reviewing my patches.
>
> Most of the properties are about the phy parameters.
> Is the phy parameter data suitable to be placed in DTS?
> I referenced other phy drivers.
> These parameters should not be defined in dts.
> I would move the parameters to the driver.
If these can be in the driver, why would ever they be in DTS in the
first place?
>
>>> + realtek,usb:
>>> + description: The phandler of realtek dwc3 node
>>
>> "phandler"? Except obvious typo, drop "The phandler of" and describe what is
>> it for.
>
> realtek,usb is a phandle of syscon used to control realtek dwc3 register.
Then no, phy should not control dwc3.
>
>>> + $ref: /schemas/types.yaml#/definitions/phandle
>>
>> Anyway, it shouldn't be here. No, no.
>
> Can I use it for phandle of syscon?
PHY getting phandle to block using this PHY? Looks wrong. Why would PHY
need to poke IP block register?
Best regards,
Krzysztof
On Thu, Jun 01, 2023 at 02:24:28AM +0000, Stanley Chang[昌育德] wrote:
> Hi Conor,
>
> > > > You have device-specific compatibles, which is great, but you also allow
> > > > only those two generic ones. I had a _brief_ look at the driver, and it
> > > > seems like there is no decision making done based on the compatibles,
> > > > only on the properties. Is that correct?
> > > > If it is, I would understand having "realtek,usb3phy" as a fallback
> > > > compatible for "realtek,rtd1619-usb3phy", but I do not get the current
> > > > setup.
> > >
> > > This driver is compatible with all Realtek RTD SoCs without specifying different settings.
> > > So use "realtek,usb3phy" as fallback compatible for all SoCs.
> > > This is the compatible name we use.
> > > Other compatible names simply indicate that the driver supports the SoCs.
> >
> > Then you should write the binding such that having fallback compatibles
> > is permitted. Try plugging
> > compatible = "realtek,rtd1295-usb2phy", "realtek,rtd-usb2phy", "realtek,usb2phy";
> > into your example below and see what happens.
> >
> > > The name "usbNphy" and "rtd-usbNphy" seem to be more generic for all RTD SoCs,
> > > but they are not device-specific compatible.
> > > Do you have a better suggestion?
> >
> > Write the binding so that having fallback compatibles in the DT actually
> > works, don't add the SoC-specific ones merely as indicators that those
> > SoCs are supported and don't permit "realtek,usbNphy" or
> > "realtek,rtd-usbNphy" in isolation ;)
> >
>
> As far as I understand what you mean.
> I should follow other docs to define compatible.
> Reference:
> Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml
> drivers/phy/mediatek/phy-mtk-xsphy.c
>
> For example:
>
> 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
>
> examples:
> -
> dwc3_u3drd_usb2phy: dwc3_u3drd_usb2phy@98013e14 {
> compatible = "realtek,rtd1319-usb2phy", "realtek,usb2phy";
>
> And use only "Realtek, usb2phy" in the driver.
> static const struct of_device_id usbphy_rtk_dt_match[] = {
> { .compatible = "realtek,usb2phy", },
> {},
> };
Yes, this would be a vast improvement, thanks.
Hi Krzysztof,
> >
> > Most of the properties are about the phy parameters.
> > Is the phy parameter data suitable to be placed in DTS?
> > I referenced other phy drivers.
> > These parameters should not be defined in dts.
> > I would move the parameters to the driver.
>
> If these can be in the driver, why would ever they be in DTS in the first place?
>
Our platforms have 3 xhci controllers which map to 3 different phy ports.
And the three phy ports use the same driver, but the parameters are different.
So I put the parameter settings in DTS, we have 3 usb-phy nodes representing 3 phy ports.
Also, some parameters have to be adjusted for different boards.
Therefore, it is more applicable in DTS than in driver.
> >>> + realtek,usb:
> >>> + description: The phandler of realtek dwc3 node
> >>
> >> "phandler"? Except obvious typo, drop "The phandler of" and describe
> >> what is it for.
> >
> > realtek,usb is a phandle of syscon used to control realtek dwc3 register.
>
> Then no, phy should not control dwc3.
OK I know it doesn't make sense.
We want to disable phy suspend from mac layer.
I will try other method.
> >
> >>> + $ref: /schemas/types.yaml#/definitions/phandle
> >>
> >> Anyway, it shouldn't be here. No, no.
> >
> > Can I use it for phandle of syscon?
>
> PHY getting phandle to block using this PHY? Looks wrong. Why would PHY
> need to poke IP block register?
>
OK. I know it doesn't make sense.
On 02/06/2023 05:20, Stanley Chang[昌育德] wrote:
> Hi Krzysztof,
>
>>>
>>> Most of the properties are about the phy parameters.
>>> Is the phy parameter data suitable to be placed in DTS?
>>> I referenced other phy drivers.
>>> These parameters should not be defined in dts.
>>> I would move the parameters to the driver.
>>
>> If these can be in the driver, why would ever they be in DTS in the first place?
>>
> Our platforms have 3 xhci controllers which map to 3 different phy ports.
You mean on the same SoC?
> And the three phy ports use the same driver, but the parameters are different.
> So I put the parameter settings in DTS, we have 3 usb-phy nodes representing 3 phy ports.
> Also, some parameters have to be adjusted for different boards.
> Therefore, it is more applicable in DTS than in driver.
Then it looks justified in DT, so please write proper descriptions for
proper properties. Underscores are not allowed in node names. No fake
nodes. Properties should usually describe physical/hardware effect not
the register value.
qcom,usb-snps-femto-v2.yaml is nice example. Few Mediatek bindings also
would work.
'
Best regards,
Krzysztof
Hi Krzysztof,
> >> If these can be in the driver, why would ever they be in DTS in the first
> place?
> >>
> > Our platforms have 3 xhci controllers which map to 3 different phy ports.
>
> You mean on the same SoC?
Yes, one SoC has three xhci controllers.
> > And the three phy ports use the same driver, but the parameters are
> different.
> > So I put the parameter settings in DTS, we have 3 usb-phy nodes representing
> 3 phy ports.
> > Also, some parameters have to be adjusted for different boards.
> > Therefore, it is more applicable in DTS than in driver.
>
> Then it looks justified in DT, so please write proper descriptions for proper
> properties. Underscores are not allowed in node names. No fake nodes.
> Properties should usually describe physical/hardware effect not the register
> value.
I will write more detail for properties.
> qcom,usb-snps-femto-v2.yaml is nice example. Few Mediatek bindings also
> would work.
> '
Thanks,
Stanley