2023-06-29 06:16:30

by Stanley Chang[昌育德]

[permalink] [raw]
Subject: [PATCH v6 1/5] usb: phy: add usb phy notify port status API

In Realtek SoC, the parameter of usb phy is designed to can dynamic
tuning base on port status. Therefore, add a notify callback of phy
driver when usb port status change.

The Realtek phy driver is designed to dynamically adjust disconnection
level and calibrate phy parameters. When the device connected bit changes
and when the disconnected bit changes, do port status change notification:

Check if portstatus is USB_PORT_STAT_CONNECTION and portchange is
USB_PORT_STAT_C_CONNECTION.
1. The device is connected, the driver lowers the disconnection level and
calibrates the phy parameters.
2. The device disconnects, the driver increases the disconnect level and
calibrates the phy parameters.

When controller to notify connect that device is already ready. If we
adjust the disconnection level in notify_connect, the disconnect may have
been triggered at this stage. So we need to change that as early as
possible. Therefore, we add an api to notify phy the port status changes.

Signed-off-by: Stanley Chang <[email protected]>
---
v5 to v6 change:
No change
v4 to v5 change:
No change
v3 to v4 change:
Fix the warning for checkpatch with strict.
v2 to v3 change:
Add more comments about the reason for adding this api
v1 to v2 change:
No change
---
drivers/usb/core/hub.c | 13 +++++++++++++
include/linux/usb/phy.h | 13 +++++++++++++
2 files changed, 26 insertions(+)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 97a0f8faea6e..10f3364c3fc2 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -614,6 +614,19 @@ static int hub_ext_port_status(struct usb_hub *hub, int port1, int type,
ret = 0;
}
mutex_unlock(&hub->status_mutex);
+
+ if (!ret) {
+ struct usb_device *hdev = hub->hdev;
+
+ if (hdev && !hdev->parent) {
+ struct usb_hcd *hcd = bus_to_hcd(hdev->bus);
+
+ if (hcd->usb_phy)
+ usb_phy_notify_port_status(hcd->usb_phy,
+ port1 - 1, *status, *change);
+ }
+ }
+
return ret;
}

diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
index e4de6bc1f69b..b513749582d7 100644
--- a/include/linux/usb/phy.h
+++ b/include/linux/usb/phy.h
@@ -144,6 +144,10 @@ struct usb_phy {
*/
int (*set_wakeup)(struct usb_phy *x, bool enabled);

+ /* notify phy port status change */
+ int (*notify_port_status)(struct usb_phy *x, int port,
+ u16 portstatus, u16 portchange);
+
/* notify phy connect status change */
int (*notify_connect)(struct usb_phy *x,
enum usb_device_speed speed);
@@ -316,6 +320,15 @@ usb_phy_set_wakeup(struct usb_phy *x, bool enabled)
return 0;
}

+static inline int
+usb_phy_notify_port_status(struct usb_phy *x, int port, u16 portstatus, u16 portchange)
+{
+ if (x && x->notify_port_status)
+ return x->notify_port_status(x, port, portstatus, portchange);
+ else
+ return 0;
+}
+
static inline int
usb_phy_notify_connect(struct usb_phy *x, enum usb_device_speed speed)
{
--
2.34.1



2023-06-29 06:25:15

by Stanley Chang[昌育德]

[permalink] [raw]
Subject: [PATCH v6 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 <[email protected]>
---
v5 to v6 change:
Drop the redundant examples
Drop the label of example
v4 to v5 change:
1. Add more examples.
2. Remove the compatible realtek,usb2phy.
3. Revise the descriptor of the property.
4. Add the default of the property.
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 | 153 ++++++++++++++++++
1 file changed, 153 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..773663bf5c62
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
@@ -0,0 +1,153 @@
+# 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:
+ 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
+
+ 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.
+ type: boolean
+
+ realtek,driving-level:
+ 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
+
+ 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
+ default: 0
+ 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
+ default: 0
+ minimum: -8
+ maximum: 8
+
+required:
+ - compatible
+ - reg
+ - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ 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)>;
+ };
--
2.34.1


2023-06-29 17:01:29

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v6 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY

On Thu, Jun 29, 2023 at 01:45:12PM +0800, Stanley Chang wrote:
> Add the documentation explain the property about Realtek USB PHY driver.

In the subject, drop "the doc about the". It's redundant. And perhaps
add 'DHC RTD SoC' if this isn't for *all* Realtek SoCs.

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

driver? This is a binding for the h/w.

>
> Signed-off-by: Stanley Chang <[email protected]>
> ---
> v5 to v6 change:
> Drop the redundant examples
> Drop the label of example
> v4 to v5 change:
> 1. Add more examples.
> 2. Remove the compatible realtek,usb2phy.
> 3. Revise the descriptor of the property.
> 4. Add the default of the property.
> 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 | 153 ++++++++++++++++++
> 1 file changed, 153 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..773663bf5c62
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
> @@ -0,0 +1,153 @@
> +# 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:

You need '|' if formatting (line breaks) are important.

> + 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:
> + 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
> +
> + 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.

"invert" assumes I know what non-inverted means. I do not. Better to
state in terms of active high, low, falling edge, rising edge, etc.

> + type: boolean
> +
> + realtek,driving-level:
> + 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.

What are the units?

> + $ref: /schemas/types.yaml#/definitions/uint32
> + default: 8
> + minimum: 0
> + maximum: 31
> +
> + realtek,driving-compensate:

compensate what?

> + 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
> + default: 0
> + 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
> + default: 0
> + minimum: -8
> + maximum: 8
> +
> +required:
> + - compatible
> + - reg
> + - "#phy-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + 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)>;
> + };
> --
> 2.34.1
>

2023-06-30 07:47:40

by Stanley Chang[昌育德]

[permalink] [raw]
Subject: RE: [PATCH v6 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY

Hi Rob,

> On Thu, Jun 29, 2023 at 01:45:12PM +0800, Stanley Chang wrote:
> > Add the documentation explain the property about Realtek USB PHY driver.
>
> In the subject, drop "the doc about the". It's redundant. And perhaps add 'DHC
> RTD SoC' if this isn't for *all* Realtek SoCs.
>
> > Realtek DHC (digital home center) RTD SoCs support DWC3 XHCI USB
> > controller. Added the driver to drive the USB 2.0 PHY transceivers.
>
> driver? This is a binding for the h/w.

I mean, the driver is drivers/phy/realtek/phy-rtk-usb2.c
I will revise as
dt-bindings: phy: realtek: Add the Realtek DHC RTD 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 and uses phy-rtk-usb2 as driver for USB 2.0 PHY transceiver..

> > +$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:
>
> You need '|' if formatting (line breaks) are important.

I think I need it. I will add it.

> > + 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.
>
> "invert" assumes I know what non-inverted means. I do not. Better to state in
> terms of active high, low, falling edge, rising edge, etc.

Meaning, the clock must be reversed.
>
> > + type: boolean
> > +
> > + realtek,driving-level:
> > + 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.
>
> What are the units?

There is no unit. It is only a gain for adjusting the magnitude.

> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + default: 8
> > + minimum: 0
> > + maximum: 31
> > +
> > + realtek,driving-compensate:
>
> compensate what?

It is to compensate the driving level. In other word, to adjust the driving level.

Thanks,
Stanley

2023-06-30 18:47:03

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v6 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY

On Fri, Jun 30, 2023 at 07:33:45AM +0000, Stanley Chang[昌育德] wrote:
> Hi Rob,
>
> > On Thu, Jun 29, 2023 at 01:45:12PM +0800, Stanley Chang wrote:
> > > Add the documentation explain the property about Realtek USB PHY driver.
> >
> > In the subject, drop "the doc about the". It's redundant. And perhaps add 'DHC
> > RTD SoC' if this isn't for *all* Realtek SoCs.
> >
> > > Realtek DHC (digital home center) RTD SoCs support DWC3 XHCI USB
> > > controller. Added the driver to drive the USB 2.0 PHY transceivers.
> >
> > driver? This is a binding for the h/w.
>
> I mean, the driver is drivers/phy/realtek/phy-rtk-usb2.c
> I will revise as
> dt-bindings: phy: realtek: Add the Realtek DHC RTD 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 and uses phy-rtk-usb2 as driver for USB 2.0 PHY transceiver..

No, you should mention nothing to do with how a particular operating
system chooses to structure its code here. Bindings describe hardware,
and the commit message should reflect that.

>
> > > +$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:
> >
> > You need '|' if formatting (line breaks) are important.
>
> I think I need it. I will add it.
>
> > > + 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.
> >
> > "invert" assumes I know what non-inverted means. I do not. Better to state in
> > terms of active high, low, falling edge, rising edge, etc.
>
> Meaning, the clock must be reversed.

Maybe that means something to Rob, but "reversed" doesn't seem any more
meaningful than inverse. I agree that it should be described in terms of
"active high" etc, as they have well understood meanings.

> > > + type: boolean
> > > +
> > > + realtek,driving-level:
> > > + 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.
> >
> > What are the units?
>
> There is no unit. It is only a gain for adjusting the magnitude.

Gain has units too.

> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + default: 8
> > > + minimum: 0
> > > + maximum: 31
> > > +
> > > + realtek,driving-compensate:
> >
> > compensate what?
>
> It is to compensate the driving level.

Should it be called "driving-level-compensate" then?

> In other word, to adjust the driving level.

So, "realtek,driving-level" sets the gain and
"realtek,driving-compensate" adjusts the driving level.
By that logic, is this also a gain?

Also this property is only for the RTD1315e? That should be
described in/constrained by the binding itself, not in the text
description alone IMO.


Attachments:
(No filename) (3.26 kB)
signature.asc (235.00 B)
Download all attachments

2023-07-03 08:19:14

by Stanley Chang[昌育德]

[permalink] [raw]
Subject: RE: [PATCH v6 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY

Hi Conor,

> On Fri, Jun 30, 2023 at 07:33:45AM +0000, Stanley Chang[???|?w] wrote:
> > Hi Rob,
> >
> > > On Thu, Jun 29, 2023 at 01:45:12PM +0800, Stanley Chang wrote:
> > > > Add the documentation explain the property about Realtek USB PHY driver.
> > >
> > > In the subject, drop "the doc about the". It's redundant. And perhaps add 'DHC
> > > RTD SoC' if this isn't for *all* Realtek SoCs.
> > >
> > > > Realtek DHC (digital home center) RTD SoCs support DWC3 XHCI USB
> > > > controller. Added the driver to drive the USB 2.0 PHY transceivers.
> > >
> > > driver? This is a binding for the h/w.
> >
> > I mean, the driver is drivers/phy/realtek/phy-rtk-usb2.c
> > I will revise as
> > dt-bindings: phy: realtek: Add the Realtek DHC RTD 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 and uses phy-rtk-usb2 as driver for USB 2.0 PHY transceiver..
>
> No, you should mention nothing to do with how a particular operating
> system chooses to structure its code here. Bindings describe hardware,
> and the commit message should reflect that.

Okay. I should remove any related driver description.
Here, only describe the hardware about SoCs.
For example,
dt-bindings: phy: realtek: Add the Realtek DHC RTD SoC USB 2.0 PHY

Document the USB PHY bindings for Realtek SoCs.
Realtek DHC (digital home center) RTD SoCs support DWC3 XHCI USB
controller and using USB 2.0 PHY transceiver.

> >
> > > > +$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:
> > >
> > > You need '|' if formatting (line breaks) are important.
> >
> > I think I need it. I will add it.
> >
> > > > + 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.
> > >
> > > "invert" assumes I know what non-inverted means. I do not. Better to state in
> > > terms of active high, low, falling edge, rising edge, etc.
> >
> > Meaning, the clock must be reversed.
>
> Maybe that means something to Rob, but "reversed" doesn't seem any more
> meaningful than inverse. I agree that it should be described in terms of
> "active high" etc, as they have well understood meanings.

Here I describe a clock sequence is "inverted", not "high" or "low".
Add the property, then hstx-sync-clock sequence is being inverted.

> > > > + type: boolean
> > > > +
> > > > + realtek,driving-level:
> > > > + 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.
> > >
> > > What are the units?
> >
> > There is no unit. It is only a gain for adjusting the magnitude.
>
> Gain has units too.

The magnitude of High speed Dp/Dm output is voltage.

> > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > + default: 8
> > > > + minimum: 0
> > > > + maximum: 31
> > > > +
> > > > + realtek,driving-compensate:
> > >
> > > compensate what?
> >
> > It is to compensate the driving level.
>
> Should it be called "driving-level-compensate" then?
Yes, it can be named "driving-level-compensate".
I will rename it.

> > In other word, to adjust the driving level.
>
> So, "realtek,driving-level" sets the gain and
> "realtek,driving-compensate" adjusts the driving level.
> By that logic, is this also a gain?

Yes, the real driving level is
driving level (reading from otp) + driving-level-compensate.

> Also this property is only for the RTD1315e? That should be
> described in/constrained by the binding itself, not in the text
> description alone IMO.
>

Does your meaning is as follows?
if:
properties:
compatible:
contains:
enum:
- realtek,rtd1315e-usb2phy
then:
properties:
realtek,driving-level-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
default: 0
minimum: -8
maximum: 8

Thanks,
Stanley