* [PATCH v11 01/36] dt-bindings: phy: rockchip-usbdp: add improved ports scheme
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:02 ` [PATCH v11 02/36] phy: rockchip: usbdp: Update mode_change after error handling Sebastian Reichel
` (34 subsequent siblings)
35 siblings, 0 replies; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
Currently the Rockchip USBDP PHY is missing a documented port scheme.
Meanwhile upstream RK3588 DTS files are a bit messy and use different
port schemes. The upstream USBDP PHY Linux kernel driver does not yet
parse the ports at all and thus does not create any implicit ABI either.
But with the current mess it is not possible to properly support USB-C
DP AltMode. Thus this introduces a proper port scheme following roughly
the ports design of the Qualcomm QMP USB4-USB3-DP PHY controller binding
with a slight difference that there is an additional port for the
USB-C SBU port as the Rockchip USB-DP PHY also contains the SBU mux.
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
.../bindings/phy/phy-rockchip-usbdp.yaml | 24 ++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-usbdp.yaml b/Documentation/devicetree/bindings/phy/phy-rockchip-usbdp.yaml
index 8b7059d5b182..89efaf005a7b 100644
--- a/Documentation/devicetree/bindings/phy/phy-rockchip-usbdp.yaml
+++ b/Documentation/devicetree/bindings/phy/phy-rockchip-usbdp.yaml
@@ -110,10 +110,34 @@ properties:
port:
$ref: /schemas/graph.yaml#/properties/port
+ deprecated: true
description:
A port node to link the PHY to a TypeC controller for the purpose of
handling orientation switching.
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+ properties:
+ port@0:
+ $ref: /schemas/graph.yaml#/properties/port
+ description:
+ Output endpoint of the PHY for USB (or DP when configured into 4 lane
+ mode), which should point to the superspeed port of a USB connector.
+
+ port@1:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: Incoming endpoint from the USB controller
+
+ port@2:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: Incoming endpoint from the DisplayPort controller
+
+ port@3:
+ $ref: /schemas/graph.yaml#/properties/port
+ description:
+ Output endpoint of the PHY for DP Auxiliary, which should either point to
+ the SBU port of a USB-C connector or a DisplayPort connector input port.
+
required:
- compatible
- reg
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* [PATCH v11 02/36] phy: rockchip: usbdp: Update mode_change after error handling
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
2026-07-09 19:02 ` [PATCH v11 01/36] dt-bindings: phy: rockchip-usbdp: add improved ports scheme Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:20 ` sashiko-bot
2026-07-09 19:02 ` [PATCH v11 03/36] phy: rockchip: usbdp: Do not lose USB3 PHY status Sebastian Reichel
` (33 subsequent siblings)
35 siblings, 1 reply; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel, Sashiko
If rk_udphy_init() or rk_udphy_setup() fails, the reinit will not be
tried again. Fix this by only updating the variable after all potential
errors have been handled.
Note, that no errors have been seen on real hardware and failures would
most likely be fatal and require at least a full reboot as the function
already asserts the PHY reset lines. So this is more of a theoretical
issue.
Fixes: 2f70bbddeb45 ("phy: rockchip: add usbdp combo phy driver")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/linux-phy/20260612163835.8D5471F000E9@smtp.kernel.org/
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index fba35510d88c..a5ffdf7a6e76 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -1000,15 +1000,14 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
}
if (udphy->status == UDPHY_MODE_NONE) {
- udphy->mode_change = false;
ret = rk_udphy_setup(udphy);
if (ret)
return ret;
if (udphy->mode & UDPHY_MODE_USB)
rk_udphy_u3_port_disable(udphy, false);
- } else if (udphy->mode_change) {
udphy->mode_change = false;
+ } else if (udphy->mode_change) {
udphy->status = UDPHY_MODE_NONE;
if (udphy->mode == UDPHY_MODE_DP)
rk_udphy_u3_port_disable(udphy, true);
@@ -1017,6 +1016,7 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
ret = rk_udphy_setup(udphy);
if (ret)
return ret;
+ udphy->mode_change = false;
}
udphy->status |= mode;
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v11 02/36] phy: rockchip: usbdp: Update mode_change after error handling
2026-07-09 19:02 ` [PATCH v11 02/36] phy: rockchip: usbdp: Update mode_change after error handling Sebastian Reichel
@ 2026-07-09 19:20 ` sashiko-bot
0 siblings, 0 replies; 56+ messages in thread
From: sashiko-bot @ 2026-07-09 19:20 UTC (permalink / raw)
To: Sebastian Reichel
Cc: olteanv, linux-phy, devicetree, conor+dt, robh, neil.armstrong,
vkoul
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] When changing the PHY mode from DP-only to a state that includes USB (e.g., DP+USB), the U3 port remains permanently disabled, breaking USB 3.0 functionality.
- [High] The PHY is left permanently enabled and clocks leak due to flawed mode-checking in the power off path during Type-C role switches.
- [High] Re-initializing the PHY during a mode change while the USB controller is active disables PHY clocks without re-routing the DWC3 clock, leading to a fatal SError on register access.
--
--- Patch [2]: [PATCH v11 02/36] phy: rockchip: usbdp: Update mode_change after error handling ---
commit 1bc42e532da1064edae3f14df8f37ade40a0072d
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Update mode_change after error handling
If rk_udphy_init() or rk_udphy_setup() fails, the reinit will not be
tried again. Fix this by only updating the variable after all potential
errors have been handled.
Link: https://lore.kernel.org/linux-phy/20260612163835.8D5471F000E9@smtp.kernel.org/
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index fba35510d88ce..a5ffdf7a6e76b 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1000,15 +1000,14 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> }
>
> if (udphy->status == UDPHY_MODE_NONE) {
> - udphy->mode_change = false;
> ret = rk_udphy_setup(udphy);
> if (ret)
> return ret;
>
> if (udphy->mode & UDPHY_MODE_USB)
> rk_udphy_u3_port_disable(udphy, false);
> - } else if (udphy->mode_change) {
> udphy->mode_change = false;
> + } else if (udphy->mode_change) {
> udphy->status = UDPHY_MODE_NONE;
> if (udphy->mode == UDPHY_MODE_DP)
> rk_udphy_u3_port_disable(udphy, true);
[Severity: High]
This isn't a bug introduced by this patch, but does this lead to a fatal
SError when changing to a mode that includes USB?
If a mode change to DP+USB triggers rk_udphy_power_on(), the code skips calling
rk_udphy_u3_port_disable(udphy, true). It then unconditionally calls
rk_udphy_disable(udphy) below, which turns off the PHY clocks.
Because the USB3 port wasn't disabled to safely re-route the clock, and the
DWC3 driver isn't notified via phy_notify_reset(), the active DWC3 controller
could crash the system when it touches its registers.
>
> rk_udphy_disable(udphy);
> ret = rk_udphy_setup(udphy);
> if (ret)
> return ret;
> + udphy->mode_change = false;
> }
[Severity: High]
This isn't a bug introduced by this patch, but does this skip re-enabling the
U3 port when the new mode requires USB?
When changing the PHY mode from DP-only to a state that includes USB (e.g.,
DP+USB), the U3 port remains permanently disabled. The mode change block
fails to call rk_udphy_u3_port_disable(udphy, false) to re-enable it for the
newly added USB 3.0 mode, leaving USB functionality broken until a power cycle.
[Severity: High]
This isn't a bug introduced by this patch, but can the PHY be left
permanently enabled during Type-C role switches?
When the Type-C mux changes mode from DP+USB to DP-only, udphy->mode loses
the UDPHY_MODE_USB bit immediately.
Later, when the USB core calls rk_udphy_usb3_phy_exit():
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_usb3_phy_exit() {
...
/* DP only or high-speed */
if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
goto unlock;
rk_udphy_power_off(udphy, UDPHY_MODE_USB);
...
}
It sees the new mode and skips calling rk_udphy_power_off(). Even if it
were called, rk_udphy_power_off() also refuses to act if the requested
mode is absent from udphy->mode:
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_power_off() {
...
if (!(udphy->mode & mode)) {
dev_info(udphy->dev, "mode 0x%02x is not support\n", mode);
return;
}
...
}
Consequently, udphy->status incorrectly retains the USB bit, preventing
rk_udphy_disable() from ever powering down the PHY and leaking clocks.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=2
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v11 03/36] phy: rockchip: usbdp: Do not lose USB3 PHY status
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
2026-07-09 19:02 ` [PATCH v11 01/36] dt-bindings: phy: rockchip-usbdp: add improved ports scheme Sebastian Reichel
2026-07-09 19:02 ` [PATCH v11 02/36] phy: rockchip: usbdp: Update mode_change after error handling Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:19 ` sashiko-bot
2026-07-09 19:02 ` [PATCH v11 04/36] phy: rockchip: usbdp: Fix devm_clk_bulk_get_all check Sebastian Reichel
` (32 subsequent siblings)
35 siblings, 1 reply; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
By default (i.e. without manually enabling runtime PM) DWC3 requests the
USB3 PHY once and keeps it enabled all the time. When DisplayPort is
being requested later on, a mode change is needed. This re-initializes
the PHY. During re-initialization the status variable has incorrectly
been cleared, which means the tracking information for USB3 is lost.
This is not an immediate problem, since the DP side keeps the PHY
enabled. But once DP is toggled off, the whole PHY will be disabled.
This is a problem, because the USB side still needs it powered.
Fix things by not clearing the status flags.
Fixes: 2f70bbddeb45 ("phy: rockchip: add usbdp combo phy driver")
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index a5ffdf7a6e76..12dfdd470a78 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -1008,7 +1008,6 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
rk_udphy_u3_port_disable(udphy, false);
udphy->mode_change = false;
} else if (udphy->mode_change) {
- udphy->status = UDPHY_MODE_NONE;
if (udphy->mode == UDPHY_MODE_DP)
rk_udphy_u3_port_disable(udphy, true);
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v11 03/36] phy: rockchip: usbdp: Do not lose USB3 PHY status
2026-07-09 19:02 ` [PATCH v11 03/36] phy: rockchip: usbdp: Do not lose USB3 PHY status Sebastian Reichel
@ 2026-07-09 19:19 ` sashiko-bot
0 siblings, 0 replies; 56+ messages in thread
From: sashiko-bot @ 2026-07-09 19:19 UTC (permalink / raw)
To: Sebastian Reichel
Cc: olteanv, conor+dt, linux-phy, robh, devicetree, vkoul,
neil.armstrong
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Preserving the USB status bit during a mode change to 4-lane DP prevents the PHY from ever powering off, breaking system PM.
- [Medium] Removing the unconditional clear of `udphy->status` introduces a state desynchronization and unbalanced clock disable bug if `rk_udphy_setup()` fails during a mode change.
--
--- Patch [3]: [PATCH v11 03/36] phy: rockchip: usbdp: Do not lose USB3 PHY status ---
commit 183c8937bbbbcb2c04198e33a8cf6f50111a49a2
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Do not lose USB3 PHY status
By default DWC3 requests the USB3 PHY once and keeps it enabled all the time.
When DisplayPort is being requested later on, a mode change is needed which
re-initializes the PHY. During re-initialization the status variable has
incorrectly been cleared, which means the tracking information for USB3 is lost.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index a5ffdf7a6e76b..12dfdd470a782 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1008,7 +1008,6 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> rk_udphy_u3_port_disable(udphy, false);
> udphy->mode_change = false;
> } else if (udphy->mode_change) {
> - udphy->status = UDPHY_MODE_NONE;
[Severity: Medium]
If rk_udphy_setup() fails during a mode change, could this lead to an unbalanced
clock disable?
rk_udphy_power_on() calls rk_udphy_disable(), but if rk_udphy_setup() fails:
} else if (udphy->mode_change) {
if (udphy->mode == UDPHY_MODE_DP)
rk_udphy_u3_port_disable(udphy, true);
rk_udphy_disable(udphy);
ret = rk_udphy_setup(udphy);
if (ret)
return ret;
it returns early without clearing udphy->status since the unconditional
reset to UDPHY_MODE_NONE was removed.
Later, if rk_udphy_power_off() is called to unwind, it clears the old mode bit
from udphy->status, reaches UDPHY_MODE_NONE, and calls rk_udphy_disable() a
second time on already-disabled clocks.
[Severity: High]
Does preserving the USB status bit during a mode change to 4-lane DP prevent
the PHY from powering off?
When Type-C negotiates 4-lane DisplayPort, udphy->mode is updated exclusively
to UDPHY_MODE_DP.
Because the reset is removed here, rk_udphy_power_on() preserves the
UDPHY_MODE_USB bit in udphy->status when it updates the mode:
udphy->status |= mode;
When the USB subsystem suspends and calls rk_udphy_usb3_phy_exit(), it
performs an early bailout:
static int rk_udphy_usb3_phy_exit(struct phy *phy)
{
...
/* DP only or high-speed */
if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
goto unlock;
This leaves the USB bit permanently trapped in udphy->status.
Finally, when DP is powered off, udphy->status resolves to UDPHY_MODE_USB,
which skips the physical rk_udphy_disable() call and leaves the PHY running.
> if (udphy->mode == UDPHY_MODE_DP)
> rk_udphy_u3_port_disable(udphy, true);
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=3
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v11 04/36] phy: rockchip: usbdp: Fix devm_clk_bulk_get_all check
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (2 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 03/36] phy: rockchip: usbdp: Do not lose USB3 PHY status Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:02 ` [PATCH v11 05/36] phy: rockchip: usbdp: Handle missing clock-names DT property gracefully Sebastian Reichel
` (31 subsequent siblings)
35 siblings, 0 replies; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel, Sashiko
If devm_clk_bulk_get_all() returns -EPROBE_DEFER, it is replaced with
-ENODEV, permanently failing the driver probe instead of allowing it to
defer. Avoid masking the error code to fix the issue.
This effectively drops returning -ENODEV in case no clocks are being
described in DT. This special case will now be handled by the follow-up
check searching for "refclk" and exit with -EINVAL.
None of this will be hit in practice, since the driver is only used by
RK3588 and RK3576 - on these platforms the DT is validated to contain
the clocks and the clock driver is force probed early. Thus there is
no need to backport this.
Fixes: 2f70bbddeb45 ("phy: rockchip: add usbdp combo phy driver")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/linux-phy/20260612164107.C7DB21F000E9@smtp.kernel.org/
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 12dfdd470a78..2845a012eafc 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -427,8 +427,8 @@ static int rk_udphy_clk_init(struct rk_udphy *udphy, struct device *dev)
int i;
udphy->num_clks = devm_clk_bulk_get_all(dev, &udphy->clks);
- if (udphy->num_clks < 1)
- return -ENODEV;
+ if (udphy->num_clks < 0)
+ return udphy->num_clks;
/* used for configure phy reference clock frequency */
for (i = 0; i < udphy->num_clks; i++) {
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* [PATCH v11 05/36] phy: rockchip: usbdp: Handle missing clock-names DT property gracefully
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (3 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 04/36] phy: rockchip: usbdp: Fix devm_clk_bulk_get_all check Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:02 ` [PATCH v11 06/36] phy: rockchip: usbdp: Drop seamless DP takeover Sebastian Reichel
` (30 subsequent siblings)
35 siblings, 0 replies; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel, Sashiko
The rk_udphy_clk_init() function would currently try to do a strncmp for
a NULL pointer, if DT specifies 'clocks' property, but no 'clock-names'
property. Fix this by making sure the clock has an id string set.
Note that DT binding requires setting clock-names, so this is only a
problem when booting a non-compliant device tree.
Fixes: 2f70bbddeb45 ("phy: rockchip: add usbdp combo phy driver")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/message/20260619154349.071321F000E9%40smtp.kernel.org
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 2845a012eafc..3fc8222fcaec 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -432,6 +432,9 @@ static int rk_udphy_clk_init(struct rk_udphy *udphy, struct device *dev)
/* used for configure phy reference clock frequency */
for (i = 0; i < udphy->num_clks; i++) {
+ if (!udphy->clks[i].id)
+ continue;
+
if (!strncmp(udphy->clks[i].id, "refclk", 6)) {
udphy->refclk = udphy->clks[i].clk;
break;
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* [PATCH v11 06/36] phy: rockchip: usbdp: Drop seamless DP takeover
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (4 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 05/36] phy: rockchip: usbdp: Handle missing clock-names DT property gracefully Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:19 ` sashiko-bot
2026-07-09 19:02 ` [PATCH v11 07/36] phy: rockchip: usbdp: Keep clocks running on PHY re-init Sebastian Reichel
` (29 subsequent siblings)
35 siblings, 1 reply; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel, Sashiko
Right now the DRM drivers do not support seamless DP takeover and I'm
I'm not aware of any bootloader implementing this feature either.
In any case this feature would be limited to boards using the USBDP PHY
for a DP or eDP connection instead of the more commonly USB-C connector.
With USB-C's DP AltMode a seamless DP takeover requires handing over the
state of the TCPM state machine from the bootloader to the kernel. This
in turn requires a huge amount of work to keep the state machine
implementations synchronized. It's very unlikely we will see somebody
implementing that in the foreseeable future.
As the current code is obviously buggy and untested, let's simply drop
support for seamless DP takeover. It can be re-implemented cleanly once
somebody adds all missing bits.
Fixes: 2f70bbddeb45 ("phy: rockchip: add usbdp combo phy driver")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/linux-phy/20260612164107.C7DB21F000E9@smtp.kernel.org/
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 31 -------------------------------
1 file changed, 31 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 3fc8222fcaec..6cb9f6b4dbf6 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -452,11 +452,6 @@ static int rk_udphy_reset_assert_all(struct rk_udphy *udphy)
return reset_control_bulk_assert(udphy->num_rsts, udphy->rsts);
}
-static int rk_udphy_reset_deassert_all(struct rk_udphy *udphy)
-{
- return reset_control_bulk_deassert(udphy->num_rsts, udphy->rsts);
-}
-
static int rk_udphy_reset_deassert(struct rk_udphy *udphy, char *name)
{
struct reset_control_bulk_data *list = udphy->rsts;
@@ -924,28 +919,6 @@ static int rk_udphy_parse_lane_mux_data(struct rk_udphy *udphy)
return 0;
}
-static int rk_udphy_get_initial_status(struct rk_udphy *udphy)
-{
- int ret;
- u32 value;
-
- ret = clk_bulk_prepare_enable(udphy->num_clks, udphy->clks);
- if (ret) {
- dev_err(udphy->dev, "failed to enable clk\n");
- return ret;
- }
-
- rk_udphy_reset_deassert_all(udphy);
-
- regmap_read(udphy->pma_regmap, CMN_LANE_MUX_AND_EN_OFFSET, &value);
- if (FIELD_GET(CMN_DP_LANE_MUX_ALL, value) && FIELD_GET(CMN_DP_LANE_EN_ALL, value))
- udphy->status = UDPHY_MODE_DP;
- else
- rk_udphy_disable(udphy);
-
- return 0;
-}
-
static int rk_udphy_parse_dt(struct rk_udphy *udphy)
{
struct device *dev = udphy->dev;
@@ -1495,10 +1468,6 @@ static int rk_udphy_probe(struct platform_device *pdev)
if (ret)
return ret;
- ret = rk_udphy_get_initial_status(udphy);
- if (ret)
- return ret;
-
mutex_init(&udphy->mutex);
platform_set_drvdata(pdev, udphy);
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v11 06/36] phy: rockchip: usbdp: Drop seamless DP takeover
2026-07-09 19:02 ` [PATCH v11 06/36] phy: rockchip: usbdp: Drop seamless DP takeover Sebastian Reichel
@ 2026-07-09 19:19 ` sashiko-bot
0 siblings, 0 replies; 56+ messages in thread
From: sashiko-bot @ 2026-07-09 19:19 UTC (permalink / raw)
To: Sebastian Reichel
Cc: conor+dt, vkoul, neil.armstrong, olteanv, linux-phy, robh,
devicetree
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] If PHY setup fails during a mode change, the error path leaves the state machine inconsistent, leading to an unbalanced clock disable when the PHY is later powered off.
--
--- Patch [6]: [PATCH v11 06/36] phy: rockchip: usbdp: Drop seamless DP takeover ---
commit 6e8a746d34fb8b93e5acd40e0d0766f740b5b995
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Drop seamless DP takeover
This commit removes the initial status checking code from the Rockchip USBDP
combo PHY driver, dropping support for seamless DP takeover. The feature is
currently unsupported by DRM drivers and bootloaders, and would require
complex TCPM state synchronization for USB-C alternate modes. Removing it
cleans up unused code until it can be properly implemented.
Closes: https://lore.kernel.org/linux-phy/20260612164107.C7DB21F000E9@smtp.kernel.org/
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 3fc8222fcaec2..6cb9f6b4dbf6d 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -1495,10 +1468,6 @@ static int rk_udphy_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> - ret = rk_udphy_get_initial_status(udphy);
> - if (ret)
> - return ret;
> -
> mutex_init(&udphy->mutex);
> platform_set_drvdata(pdev, udphy);
>
[Severity: High]
This is a pre-existing issue, but I noticed a potential state inconsistency
in the PHY mode change error path.
In drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_power_on():
} else if (udphy->mode_change) {
if (udphy->mode == UDPHY_MODE_DP)
rk_udphy_u3_port_disable(udphy, true);
rk_udphy_disable(udphy);
ret = rk_udphy_setup(udphy);
if (ret)
return ret;
If rk_udphy_setup() fails (e.g. due to a PLL lock timeout), the function
returns an error directly without updating udphy->status.
Since the hardware has already been disabled by the preceding call to
rk_udphy_disable(), but udphy->status still reflects the previous active
state, could this lead to a double-disable of the bulk clocks when the
consumer later tears down the initial mode and calls phy_power_off()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=6
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v11 07/36] phy: rockchip: usbdp: Keep clocks running on PHY re-init
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (5 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 06/36] phy: rockchip: usbdp: Drop seamless DP takeover Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:18 ` sashiko-bot
2026-07-09 19:02 ` [PATCH v11 08/36] phy: rockchip: usbdp: Amend SSC modulation deviation Sebastian Reichel
` (28 subsequent siblings)
35 siblings, 1 reply; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
When a mode change is required rk_udphy_power_on() disables
the clocks and then calls rk_udphy_setup(), which then enables
all the clocks again before continuing with rk_udphy_init().
Considering that rk_udphy_init() does assert the reset lines,
re-enabling the clocks is just delaying things. Avoid it by
directly calling rk_udphy_init().
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 6cb9f6b4dbf6..e3f5a26c876a 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -987,8 +987,7 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
if (udphy->mode == UDPHY_MODE_DP)
rk_udphy_u3_port_disable(udphy, true);
- rk_udphy_disable(udphy);
- ret = rk_udphy_setup(udphy);
+ ret = rk_udphy_init(udphy);
if (ret)
return ret;
udphy->mode_change = false;
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v11 07/36] phy: rockchip: usbdp: Keep clocks running on PHY re-init
2026-07-09 19:02 ` [PATCH v11 07/36] phy: rockchip: usbdp: Keep clocks running on PHY re-init Sebastian Reichel
@ 2026-07-09 19:18 ` sashiko-bot
0 siblings, 0 replies; 56+ messages in thread
From: sashiko-bot @ 2026-07-09 19:18 UTC (permalink / raw)
To: Sebastian Reichel
Cc: olteanv, neil.armstrong, robh, vkoul, linux-phy, conor+dt,
devicetree
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] The USB3 (U3) port is not re-enabled when the PHY dynamically switches from DP-only mode to DP+USB mode, breaking USB3 functionality.
- [High] PHY clock and power leak due to early exit during power off based on current mode instead of active status.
- [High] The DisplayPort PHY init function fails to replay a cached HPD (Hot Plug Detect) event if the Type-C cable was connected before the DP controller probed.
--
--- Patch [7]: [PATCH v11 07/36] phy: rockchip: usbdp: Keep clocks running on PHY re-init ---
commit 79bb98238750965863b0b02f66603b673864fa91
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Keep clocks running on PHY re-init
This commit optimizes PHY re-initialization by avoiding the redundant disable
and re-enable of clocks. Instead of calling rk_udphy_disable() and
rk_udphy_setup(), it directly calls rk_udphy_init().
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 6cb9f6b4dbf6d..e3f5a26c876a5 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -987,8 +987,7 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> if (udphy->mode == UDPHY_MODE_DP)
> rk_udphy_u3_port_disable(udphy, true);
>
> - rk_udphy_disable(udphy);
> - ret = rk_udphy_setup(udphy);
> + ret = rk_udphy_init(udphy);
[Severity: High]
This isn't a bug introduced by this patch, but does this logic leave the USB3
port disabled when dynamically switching from DP-only back to DP+USB mode?
When the Type-C mux transitions from DP+USB to DP-only mode,
rk_udphy_power_on() executes rk_udphy_u3_port_disable(udphy, true) because
udphy->mode == UDPHY_MODE_DP.
If the mux later switches back to DP+USB mode (where
udphy->mode == UDPHY_MODE_DP_USB), rk_udphy_power_on() is called again with
udphy->mode_change == true, but there is no corresponding
rk_udphy_u3_port_disable(udphy, false) call here to re-enable the U3 port.
Consequently, the USB 3.0 port appears to remain disabled in the GRF registers
until a full PHY power cycle.
> if (ret)
> return ret;
> udphy->mode_change = false;
There are also two other pre-existing issues in this file that might need
attention.
[Severity: High]
This is a pre-existing issue, but does rk_udphy_usb3_phy_exit() permanently
leak PHY clock and power resources if the Type-C mode changes before the USB
controller exits?
If the mode changes (e.g., from DP+USB to DP-only) before the USB controller
calls phy_exit, the mode will be updated. When rk_udphy_usb3_phy_exit() is
subsequently called:
static int rk_udphy_usb3_phy_exit(struct phy *phy)
{
...
/* DP only or high-speed */
if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
goto unlock;
rk_udphy_power_off(udphy, UDPHY_MODE_USB);
...
}
Since udphy->mode was already updated, this condition evaluates to true,
causing an early return. The UDPHY_MODE_USB bit is never cleared from
udphy->status, permanently skipping rk_udphy_disable() and leaving clocks
running even when all devices detach.
The same logic flaw exists in rk_udphy_power_off():
static void rk_udphy_power_off(struct rk_udphy *udphy, u8 mode)
{
if (!(udphy->mode & mode)) {
dev_info(udphy->dev, "mode 0x%02x is not support\n", mode);
return;
}
...
}
Checking udphy->mode instead of the active udphy->status prevents the status
bit from being cleared.
[Severity: High]
This is a pre-existing issue, but does rk_udphy_dp_phy_init() silently miss
the initial connection event if the Type-C cable is connected before the
DisplayPort controller driver probes?
When a connection occurs, rk_udphy_dp_hpd_event_trigger() is called:
static void rk_udphy_dp_hpd_event_trigger(struct rk_udphy *udphy, bool hpd)
{
...
udphy->dp_sink_hpd_sel = true;
udphy->dp_sink_hpd_cfg = hpd;
if (!udphy->dp_in_use)
return;
rk_udphy_grfreg_write(udphy->vogrf, &cfg->vogrfcfg[udphy->id].hpd_trigger, hpd);
}
If the DP controller hasn't probed yet, the event is merely cached and not
written to the GRF register.
When the DP controller later probes and calls rk_udphy_dp_phy_init():
static int rk_udphy_dp_phy_init(struct phy *phy)
{
...
udphy->dp_in_use = true;
...
return 0;
}
The init function sets dp_in_use to true but never checks or applies the
cached HPD state (dp_sink_hpd_sel and dp_sink_hpd_cfg), leaving the display
blank until a physical replug.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=7
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v11 08/36] phy: rockchip: usbdp: Amend SSC modulation deviation
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (6 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 07/36] phy: rockchip: usbdp: Keep clocks running on PHY re-init Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:19 ` sashiko-bot
2026-07-09 19:02 ` [PATCH v11 09/36] phy: rockchip: usbdp: Fix LFPS detect threshold control Sebastian Reichel
` (27 subsequent siblings)
35 siblings, 1 reply; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
From: Frank Wang <frank.wang@rock-chips.com>
Move SSC modulation deviation into private config of clock
- 24M: 0x00d4[5:0] = 0x30
- 26M: 0x00d4[5:0] = 0x33
Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
[Taken over from rockchip's kernel tree; register 0x00d4 is not
described in the TRM]
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index e3f5a26c876a..cb6acadfbccf 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -350,7 +350,8 @@ static const struct reg_sequence rk_udphy_24m_refclk_cfg[] = {
{0x0a64, 0xa8}, {0x1a3c, 0xd0},
{0x1a44, 0xd0}, {0x1a48, 0x01},
{0x1a4c, 0x0d}, {0x1a54, 0xe0},
- {0x1a5c, 0xe0}, {0x1a64, 0xa8}
+ {0x1a5c, 0xe0}, {0x1a64, 0xa8},
+ {0x00d4, 0x30}
};
static const struct reg_sequence rk_udphy_26m_refclk_cfg[] = {
@@ -377,7 +378,7 @@ static const struct reg_sequence rk_udphy_26m_refclk_cfg[] = {
{0x0c30, 0x0e}, {0x0c48, 0x06},
{0x1c30, 0x0e}, {0x1c48, 0x06},
{0x028c, 0x18}, {0x0af0, 0x00},
- {0x1af0, 0x00}
+ {0x1af0, 0x00}, {0x00d4, 0x33}
};
static const struct reg_sequence rk_udphy_init_sequence[] = {
@@ -412,8 +413,7 @@ static const struct reg_sequence rk_udphy_init_sequence[] = {
{0x0070, 0x7d}, {0x0074, 0x68},
{0x0af4, 0x1a}, {0x1af4, 0x1a},
{0x0440, 0x3f}, {0x10d4, 0x08},
- {0x20d4, 0x08}, {0x00d4, 0x30},
- {0x0024, 0x6e},
+ {0x20d4, 0x08}, {0x0024, 0x6e}
};
static inline int rk_udphy_grfreg_write(struct regmap *base,
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v11 08/36] phy: rockchip: usbdp: Amend SSC modulation deviation
2026-07-09 19:02 ` [PATCH v11 08/36] phy: rockchip: usbdp: Amend SSC modulation deviation Sebastian Reichel
@ 2026-07-09 19:19 ` sashiko-bot
0 siblings, 0 replies; 56+ messages in thread
From: sashiko-bot @ 2026-07-09 19:19 UTC (permalink / raw)
To: Sebastian Reichel
Cc: conor+dt, olteanv, robh, devicetree, vkoul, neil.armstrong,
linux-phy
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The driver fails to update `udphy->mode` when exiting DisplayPort Alt Mode, leaving the PHY in a broken state for USB3.
- [High] The DisplayPort configure callback allows requesting more lanes than available, which corrupts the configuration of active USB lanes.
--
--- Patch [8]: [PATCH v11 08/36] phy: rockchip: usbdp: Amend SSC modulation deviation ---
commit 721fb4287b2e4de65248e4437ba81583781df56f
Author: Frank Wang <frank.wang@rock-chips.com>
phy: rockchip: usbdp: Amend SSC modulation deviation
This commit moves the SSC modulation deviation into the private config of
the clock for the 24M and 26M configurations.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index e3f5a26c876a5..cb6acadfbccfb 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -350,7 +350,8 @@ static const struct reg_sequence rk_udphy_24m_refclk_cfg[] = {
> {0x0a64, 0xa8}, {0x1a3c, 0xd0},
> {0x1a44, 0xd0}, {0x1a48, 0x01},
> {0x1a4c, 0x0d}, {0x1a54, 0xe0},
> - {0x1a5c, 0xe0}, {0x1a64, 0xa8}
> + {0x1a5c, 0xe0}, {0x1a64, 0xa8},
> + {0x00d4, 0x30}
> };
[ ... ]
[Severity: High]
This is a pre-existing issue, but I noticed that the driver might fail to
update udphy->mode when exiting DisplayPort Alt Mode.
In rk_udphy_typec_mux_set():
if (state->alt && state->alt->svid == USB_TYPEC_DP_SID) {
struct typec_displayport_data *data = state->data;
When the Type-C mux state changes to TYPEC_STATE_USB or TYPEC_STATE_SAFE,
state->alt is set to NULL. This causes the block above to be skipped,
bypassing the assignment of udphy->mode and udphy->mode_change.
Could this leave udphy->mode stuck in UDPHY_MODE_DP and prevent the USB3
port from being re-enabled?
[Severity: High]
This is a pre-existing issue, but does the DisplayPort configure callback
allow requesting more lanes than available?
In rk_udphy_dp_phy_verify_lanes():
/* valid lane count. */
udphy->lanes = dp->lanes;
The requested lane count is accepted and saved without validating it against
the actual number of available DP lanes.
If the DP controller requests 4 lanes while the PHY is in 2-lane DP + USB
mode, udphy->lanes is inappropriately set to 4.
Later in rk_udphy_dp_phy_configure():
rk_udphy_dp_set_voltage(udphy, udphy->bw, dp->voltage[i],
dp->pre[i], lane);
This out-of-bounds lane configuration is applied in a loop.
Could this corrupt the analog state of lanes that are actively being used by
the USB3 controller?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=8
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v11 09/36] phy: rockchip: usbdp: Fix LFPS detect threshold control
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (7 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 08/36] phy: rockchip: usbdp: Amend SSC modulation deviation Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:02 ` [PATCH v11 10/36] phy: rockchip: usbdp: Add missing mode_change update Sebastian Reichel
` (26 subsequent siblings)
35 siblings, 0 replies; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel, William Wu
From: William Wu <william.wu@rock-chips.com>
According to the LFPS Tx Low Power/LFPS Rx Detect Threshold [1],
the device under test(DUT) must not respond if LFPS below the
minimum LFPS Rx Detect Threshold 100mV. Test fail on Rockchip
platforms, because the default LFPS detect threshold is set to
65mV.
The USBDP PHY LFPS detect threshold voltage could be set to
30mV ~ 140mV, and since there could be 10-20% PVT variation,
we set LFPS detect threshold voltage to 110mV.
[1] https://compliance.usb.org/resources/LFPS_Rx_Tx_Low_Power_Compliance_Update_Rev5.pdf
Signed-off-by: William Wu <william.wu@rock-chips.com>
[Taken over from rockchip's kernel tree; the registers are not described
in the TRM]
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index cb6acadfbccf..982315535caf 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -413,7 +413,8 @@ static const struct reg_sequence rk_udphy_init_sequence[] = {
{0x0070, 0x7d}, {0x0074, 0x68},
{0x0af4, 0x1a}, {0x1af4, 0x1a},
{0x0440, 0x3f}, {0x10d4, 0x08},
- {0x20d4, 0x08}, {0x0024, 0x6e}
+ {0x20d4, 0x08}, {0x0024, 0x6e},
+ {0x09c0, 0x0a}, {0x19c0, 0x0a}
};
static inline int rk_udphy_grfreg_write(struct regmap *base,
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* [PATCH v11 10/36] phy: rockchip: usbdp: Add missing mode_change update
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (8 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 09/36] phy: rockchip: usbdp: Fix LFPS detect threshold control Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:19 ` sashiko-bot
2026-07-09 19:02 ` [PATCH v11 11/36] phy: rockchip: usbdp: Support single-lane DP Sebastian Reichel
` (25 subsequent siblings)
35 siblings, 1 reply; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
rk_udphy_set_typec_default_mapping() updates the available modes,
but does not set the mode_change as required. This results in
missing re-initialization and thus non-working DisplayPort.
Fix this issue by introducing a new helper to update the available
modes.
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 982315535caf..7540c897dde1 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -617,6 +617,15 @@ static void rk_udphy_dp_hpd_event_trigger(struct rk_udphy *udphy, bool hpd)
rk_udphy_grfreg_write(udphy->vogrf, &cfg->vogrfcfg[udphy->id].hpd_trigger, hpd);
}
+static void rk_udphy_mode_set(struct rk_udphy *udphy, u8 mode)
+{
+ if (udphy->mode == mode)
+ return;
+
+ udphy->mode_change = true;
+ udphy->mode = mode;
+}
+
static void rk_udphy_set_typec_default_mapping(struct rk_udphy *udphy)
{
if (udphy->flip) {
@@ -647,7 +656,7 @@ static void rk_udphy_set_typec_default_mapping(struct rk_udphy *udphy)
gpiod_set_value_cansleep(udphy->sbu2_dc_gpio, 1);
}
- udphy->mode = UDPHY_MODE_DP_USB;
+ rk_udphy_mode_set(udphy, UDPHY_MODE_DP_USB);
}
static int rk_udphy_orien_sw_set(struct typec_switch_dev *sw,
@@ -1361,10 +1370,7 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
usleep_range(750, 800);
rk_udphy_dp_hpd_event_trigger(udphy, true);
} else if (data->status & DP_STATUS_HPD_STATE) {
- if (udphy->mode != mode) {
- udphy->mode = mode;
- udphy->mode_change = true;
- }
+ rk_udphy_mode_set(udphy, mode);
rk_udphy_dp_hpd_event_trigger(udphy, true);
} else {
rk_udphy_dp_hpd_event_trigger(udphy, false);
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v11 10/36] phy: rockchip: usbdp: Add missing mode_change update
2026-07-09 19:02 ` [PATCH v11 10/36] phy: rockchip: usbdp: Add missing mode_change update Sebastian Reichel
@ 2026-07-09 19:19 ` sashiko-bot
0 siblings, 0 replies; 56+ messages in thread
From: sashiko-bot @ 2026-07-09 19:19 UTC (permalink / raw)
To: Sebastian Reichel
Cc: olteanv, robh, devicetree, conor+dt, vkoul, neil.armstrong,
linux-phy
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The newly introduced rk_udphy_mode_set() helper incorrectly returns early if the logical mode matches, failing to trigger PHY hardware reinitialization on Type-C cable orientation flips.
--
--- Patch [10]: [PATCH v11 10/36] phy: rockchip: usbdp: Add missing mode_change update ---
commit 3a5a910ac8d7d8bee622a7fda0b441c254a882fe
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Add missing mode_change update
This commit introduces a new helper rk_udphy_mode_set() to update available
modes and set the mode_change flag. It aims to fix an issue where missing
re-initialization caused DisplayPort to stop working.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 982315535caf7..7540c897dde13 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -617,6 +617,15 @@ static void rk_udphy_dp_hpd_event_trigger(struct rk_udphy *udphy, bool hpd)
> rk_udphy_grfreg_write(udphy->vogrf, &cfg->vogrfcfg[udphy->id].hpd_trigger, hpd);
> }
>
> +static void rk_udphy_mode_set(struct rk_udphy *udphy, u8 mode)
> +{
> + if (udphy->mode == mode)
> + return;
> +
> + udphy->mode_change = true;
> + udphy->mode = mode;
> +}
> +
> static void rk_udphy_set_typec_default_mapping(struct rk_udphy *udphy)
> {
> if (udphy->flip) {
> @@ -647,7 +656,7 @@ static void rk_udphy_set_typec_default_mapping(struct rk_udphy *udphy)
> gpiod_set_value_cansleep(udphy->sbu2_dc_gpio, 1);
> }
>
> - udphy->mode = UDPHY_MODE_DP_USB;
> + rk_udphy_mode_set(udphy, UDPHY_MODE_DP_USB);
[Severity: High]
Will this early return in rk_udphy_mode_set() prevent PHY re-initialization
during a Type-C cable orientation flip?
When the cable is rapidly flipped and replugged, the USB driver might keep the
PHY powered on temporarily due to runtime PM autosuspend, meaning udphy->status
can remain UDPHY_MODE_USB.
In this scenario, rk_udphy_set_typec_default_mapping() recalculates the new
reversed physical lane mappings and calls the new helper:
rk_udphy_orien_sw_set() ->
rk_udphy_set_typec_default_mapping() ->
// physical lane mappings are updated here
rk_udphy_mode_set(udphy, UDPHY_MODE_DP_USB);
Since udphy->mode is already UDPHY_MODE_DP_USB, the helper returns early
without setting mode_change to true.
Consequently, when DP power-on occurs in rk_udphy_power_on(), the hardware
initialization step is skipped because mode_change is false. The hardware's
lane mux registers are never updated with the new reversed mappings, which
causes DisplayPort to fail.
Does the mode_change flag need to be set during an orientation change even if
the logical mode remains the same?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=10
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v11 11/36] phy: rockchip: usbdp: Support single-lane DP
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (9 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 10/36] phy: rockchip: usbdp: Add missing mode_change update Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:19 ` sashiko-bot
2026-07-09 19:02 ` [PATCH v11 12/36] phy: rockchip: usbdp: Limit DP lane count to muxed lanes Sebastian Reichel
` (24 subsequent siblings)
35 siblings, 1 reply; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
From: Zhang Yubing <yubing.zhang@rock-chips.com>
Implement support for using just a single DisplayPort line.
Signed-off-by: Zhang Yubing <yubing.zhang@rock-chips.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 64 +++++++++++++------------------
1 file changed, 27 insertions(+), 37 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 7540c897dde1..c4d62b234ea8 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -193,6 +193,7 @@ struct rk_udphy {
int id;
bool dp_in_use;
+ int dp_lanes;
/* PHY const config */
const struct rk_udphy_cfg *cfgs;
@@ -535,6 +536,13 @@ static void rk_udphy_usb_bvalid_enable(struct rk_udphy *udphy, u8 enable)
* <0 1> dpln0 dpln1 usbrx usbtx
* <2 3> usbrx usbtx dpln0 dpln1
* ---------------------------------------------------------------------------
+ * if 1 lane for dp function, 2 lane for usb function, define rockchip,dp-lane-mux = <x>;
+ * sample as follow:
+ * ---------------------------------------------------------------------------
+ * B11-B10 A2-A3 A11-A10 B2-B3
+ * rockchip,dp-lane-mux ln0(tx/rx) ln1(tx) ln2(tx/rx) ln3(tx)
+ * <0> dpln0 \ usbrx usbtx
+ * ---------------------------------------------------------------------------
*/
static void rk_udphy_dplane_select(struct rk_udphy *udphy)
@@ -542,18 +550,18 @@ static void rk_udphy_dplane_select(struct rk_udphy *udphy)
const struct rk_udphy_cfg *cfg = udphy->cfgs;
u32 value = 0;
- switch (udphy->mode) {
- case UDPHY_MODE_DP:
- value |= 2 << udphy->dp_lane_sel[2] * 2;
+ switch (udphy->dp_lanes) {
+ case 4:
value |= 3 << udphy->dp_lane_sel[3] * 2;
+ value |= 2 << udphy->dp_lane_sel[2] * 2;
fallthrough;
- case UDPHY_MODE_DP_USB:
- value |= 0 << udphy->dp_lane_sel[0] * 2;
+ case 2:
value |= 1 << udphy->dp_lane_sel[1] * 2;
- break;
+ fallthrough;
- case UDPHY_MODE_USB:
+ case 1:
+ value |= 0 << udphy->dp_lane_sel[0] * 2;
break;
default:
@@ -566,28 +574,6 @@ static void rk_udphy_dplane_select(struct rk_udphy *udphy)
FIELD_PREP(DP_AUX_DOUT_SEL, udphy->dp_aux_dout_sel) | value);
}
-static int rk_udphy_dplane_get(struct rk_udphy *udphy)
-{
- int dp_lanes;
-
- switch (udphy->mode) {
- case UDPHY_MODE_DP:
- dp_lanes = 4;
- break;
-
- case UDPHY_MODE_DP_USB:
- dp_lanes = 2;
- break;
-
- case UDPHY_MODE_USB:
- default:
- dp_lanes = 0;
- break;
- }
-
- return dp_lanes;
-}
-
static void rk_udphy_dplane_enable(struct rk_udphy *udphy, int dp_lanes)
{
u32 val = 0;
@@ -657,6 +643,7 @@ static void rk_udphy_set_typec_default_mapping(struct rk_udphy *udphy)
}
rk_udphy_mode_set(udphy, UDPHY_MODE_DP_USB);
+ udphy->dp_lanes = 2;
}
static int rk_udphy_orien_sw_set(struct typec_switch_dev *sw,
@@ -895,7 +882,7 @@ static int rk_udphy_parse_lane_mux_data(struct rk_udphy *udphy)
return 0;
}
- if (num_lanes != 2 && num_lanes != 4)
+ if (num_lanes != 1 && num_lanes != 2 && num_lanes != 4)
return dev_err_probe(udphy->dev, -EINVAL,
"invalid number of lane mux\n");
@@ -921,9 +908,11 @@ static int rk_udphy_parse_lane_mux_data(struct rk_udphy *udphy)
}
udphy->mode = UDPHY_MODE_DP;
- if (num_lanes == 2) {
+ udphy->dp_lanes = num_lanes;
+ if (num_lanes == 1 || num_lanes == 2) {
udphy->mode |= UDPHY_MODE_USB;
- udphy->flip = (udphy->lane_mux_sel[0] == PHY_LANE_MUX_DP);
+ udphy->flip = (udphy->lane_mux_sel[0] == PHY_LANE_MUX_DP) ||
+ (udphy->lane_mux_sel[1] == PHY_LANE_MUX_DP);
}
return 0;
@@ -1050,18 +1039,17 @@ static int rk_udphy_dp_phy_exit(struct phy *phy)
static int rk_udphy_dp_phy_power_on(struct phy *phy)
{
struct rk_udphy *udphy = phy_get_drvdata(phy);
- int ret, dp_lanes;
+ int ret;
mutex_lock(&udphy->mutex);
- dp_lanes = rk_udphy_dplane_get(udphy);
- phy_set_bus_width(phy, dp_lanes);
+ phy_set_bus_width(phy, udphy->dp_lanes);
ret = rk_udphy_power_on(udphy, UDPHY_MODE_DP);
if (ret)
goto unlock;
- rk_udphy_dplane_enable(udphy, dp_lanes);
+ rk_udphy_dplane_enable(udphy, udphy->dp_lanes);
rk_udphy_dplane_select(udphy);
@@ -1341,6 +1329,7 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
udphy->lane_mux_sel[2] = PHY_LANE_MUX_DP;
udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
mode = UDPHY_MODE_DP;
+ udphy->dp_lanes = 4;
break;
case TYPEC_DP_STATE_D:
@@ -1357,6 +1346,7 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
}
mode = UDPHY_MODE_DP_USB;
+ udphy->dp_lanes = 2;
break;
}
@@ -1501,7 +1491,7 @@ static int rk_udphy_probe(struct platform_device *pdev)
ret = PTR_ERR(udphy->phy_dp);
return dev_err_probe(dev, ret, "failed to create DP phy\n");
}
- phy_set_bus_width(udphy->phy_dp, rk_udphy_dplane_get(udphy));
+ phy_set_bus_width(udphy->phy_dp, udphy->dp_lanes);
udphy->phy_dp->attrs.max_link_rate = 8100;
phy_set_drvdata(udphy->phy_dp, udphy);
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v11 11/36] phy: rockchip: usbdp: Support single-lane DP
2026-07-09 19:02 ` [PATCH v11 11/36] phy: rockchip: usbdp: Support single-lane DP Sebastian Reichel
@ 2026-07-09 19:19 ` sashiko-bot
0 siblings, 0 replies; 56+ messages in thread
From: sashiko-bot @ 2026-07-09 19:19 UTC (permalink / raw)
To: Sebastian Reichel
Cc: vkoul, neil.armstrong, conor+dt, linux-phy, robh, devicetree,
olteanv
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing bounds check for requested DP lanes allows out-of-bounds register corruption.
- [High] Data race and unprotected register access in `rk_udphy_dp_phy_configure()` due to missing mutex lock.
--
--- Patch [11]: [PATCH v11 11/36] phy: rockchip: usbdp: Support single-lane DP ---
commit 543954c74eaa703430fe27a5fd2cfce0594a6d78
Author: Zhang Yubing <yubing.zhang@rock-chips.com>
phy: rockchip: usbdp: Support single-lane DP
Implement support for using just a single DisplayPort line.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 7540c897dde13..c4d62b234ea80 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -193,6 +193,7 @@ struct rk_udphy {
> int id;
>
> bool dp_in_use;
> + int dp_lanes;
[Severity: High]
This isn't a bug introduced by this patch, but does the requested lane
count need to be bounded against the newly added udphy->dp_lanes?
If a DRM bridge calls phy_configure() with dp->lanes = 4 when
udphy->dp_lanes = 2, it seems rk_udphy_dp_phy_verify_lanes() will
accept it:
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_dp_phy_verify_lanes() {
switch (dp->lanes) {
case 1:
case 2:
case 4:
/* valid lane count. */
udphy->lanes = dp->lanes;
break;
...
}
Would this cause rk_udphy_dp_phy_configure() to iterate out-of-bounds of
the DP-assigned lanes and apply DP voltage/pre-emphasis settings to USB
PMA analog registers, potentially corrupting the USB3 PHY state?
[ ... ]
> @@ -1050,18 +1039,17 @@ static int rk_udphy_dp_phy_exit(struct phy *phy)
> static int rk_udphy_dp_phy_power_on(struct phy *phy)
> {
> struct rk_udphy *udphy = phy_get_drvdata(phy);
> - int ret, dp_lanes;
> + int ret;
>
> mutex_lock(&udphy->mutex);
[Severity: High]
This is a pre-existing issue, but while rk_udphy_dp_phy_power_on() acquires
udphy->mutex before modifying driver state, rk_udphy_dp_phy_configure()
appears to access shared state and hardware registers without this lock:
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_dp_phy_configure() {
struct rk_udphy *udphy = phy_get_drvdata(phy);
struct phy_configure_opts_dp *dp = &opts->dp;
...
/* No mutex_lock(&udphy->mutex) here */
...
}
Can a Type-C event (like a cable flip or alt mode change) concurrently
trigger rk_udphy_typec_mux_set() and race with rk_udphy_dp_phy_configure()?
Could this data race on udphy->lane_mux_sel and concurrent writes to the
pma_regmap lead to hardware corruption or link training failures?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=11
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v11 12/36] phy: rockchip: usbdp: Limit DP lane count to muxed lanes
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (10 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 11/36] phy: rockchip: usbdp: Support single-lane DP Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:21 ` sashiko-bot
2026-07-09 19:02 ` [PATCH v11 13/36] phy: rockchip: usbdp: Rename DP lane functions Sebastian Reichel
` (23 subsequent siblings)
35 siblings, 1 reply; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel, Sashiko
In theory the DP controller could request 4 lanes when the PHY is
restricted to 2 lanes as the other half is used by USB3.
With the current user (DW-DP) this cannot happen, but as the check is
cheap and users might change in the future protect things accordingly.
Not doing so would corrupt USB3 usage by the following code configuring
the voltages.
Fixes: 2f70bbddeb45 ("phy: rockchip: add usbdp combo phy driver")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/linux-phy/20260612165546.98E1F1F000E9@smtp.kernel.org/
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index c4d62b234ea8..b172ce6a722e 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -1099,6 +1099,9 @@ static int rk_udphy_dp_phy_verify_link_rate(struct rk_udphy *udphy,
static int rk_udphy_dp_phy_verify_lanes(struct rk_udphy *udphy,
struct phy_configure_opts_dp *dp)
{
+ if (dp->lanes > udphy->dp_lanes)
+ return -EINVAL;
+
switch (dp->lanes) {
case 1:
case 2:
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v11 12/36] phy: rockchip: usbdp: Limit DP lane count to muxed lanes
2026-07-09 19:02 ` [PATCH v11 12/36] phy: rockchip: usbdp: Limit DP lane count to muxed lanes Sebastian Reichel
@ 2026-07-09 19:21 ` sashiko-bot
0 siblings, 0 replies; 56+ messages in thread
From: sashiko-bot @ 2026-07-09 19:21 UTC (permalink / raw)
To: Sebastian Reichel
Cc: neil.armstrong, robh, conor+dt, devicetree, olteanv, linux-phy,
vkoul
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The DP lane bounds check and PHY configuration execute without holding the driver's mutex, creating a TOCTOU race condition with concurrent Type-C events.
- [High] The newly added lane count check is bypassed during standalone DP voltage updates, allowing the same USB3 register corruption the patch intended to fix.
--
--- Patch [12]: [PATCH v11 12/36] phy: rockchip: usbdp: Limit DP lane count to muxed lanes ---
commit 42fd02565f1c138c8faee25d5fd929ceb0f2e7e3
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
Subject: phy: rockchip: usbdp: Limit DP lane count to muxed lanes
This patch adds a check to limit the DisplayPort lane count to the muxed
lanes, preventing the DP controller from configuring 4 lanes when the PHY
is restricted to 2 lanes. This aims to protect USB3 usage from being
corrupted by incorrect voltage configurations.
Link: https://lore.kernel.org/linux-phy/20260612165546.98E1F1F000E9@smtp.kernel.org/
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index c4d62b234ea80..b172ce6a722ee 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1099,6 +1099,9 @@ static int rk_udphy_dp_phy_verify_link_rate(struct rk_udphy *udphy,
> static int rk_udphy_dp_phy_verify_lanes(struct rk_udphy *udphy,
> struct phy_configure_opts_dp *dp)
> {
> + if (dp->lanes > udphy->dp_lanes)
> + return -EINVAL;
> +
[Severity: High]
This is a pre-existing issue, but does this bounds check need to be
protected by a lock?
If a Type-C event (like a cable flip reducing DP lanes from 4 to 2)
occurs concurrently with a DP link training operation, could the bounds
check pass while udphy->dp_lanes is 4?
If a context switch allows the Type-C handler to reduce the lane count
and change udphy->lane_mux_sel while holding udphy->mutex, when this
thread resumes and proceeds to rk_udphy_dp_phy_configure() (which is
called without holding udphy->mutex), could it overwrite hardware
registers of physical lanes now assigned to USB3:
rk_udphy_dp_phy_configure() {
...
lane = udphy->dp_lane_sel[i];
...
regmap_update_bits(udphy->pma_regmap,
TRSV_ANA_TX_CLK_OFFSET_N(lane),
LN_ANA_TX_SER_TXCLK_INV,
FIELD_PREP(LN_ANA_TX_SER_TXCLK_INV,
udphy->lane_mux_sel[lane]));
...
}
[Severity: High]
This is also a pre-existing issue, but will this check be bypassed
during standalone DP voltage updates?
If a Type-C event asynchronously reduces udphy->dp_lanes (e.g., from
4 down to 2), and the DP controller subsequently issues a standalone
voltage update (where dp->set_voltages=1 and dp->set_lanes=0) prior to
a full link retrain:
rk_udphy_dp_phy_configure() {
...
if (dp->set_lanes) {
ret = rk_udphy_dp_phy_verify_lanes(udphy, dp);
if (ret)
return ret;
}
...
}
Would rk_udphy_dp_phy_verify_lanes() be bypassed entirely? This would
allow the configuration loop to iterate up to the stale cached
udphy->lanes limit:
rk_udphy_dp_phy_configure() {
...
for (i = 0; i < udphy->lanes; i++) {
...
and potentially cause the out-of-bounds writes to the USB3 hardware
registers that this patch intends to prevent.
> switch (dp->lanes) {
> case 1:
> case 2:
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=12
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v11 13/36] phy: rockchip: usbdp: Rename DP lane functions
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (11 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 12/36] phy: rockchip: usbdp: Limit DP lane count to muxed lanes Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:02 ` [PATCH v11 14/36] phy: rockchip: usbdp: Use FIELD_PREP_WM16_CONST Sebastian Reichel
` (22 subsequent siblings)
35 siblings, 0 replies; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
The common prefix for DisplayPort related functions is rk_udphy_dp_
(with a final _), so update the two DP lane functions to follow that
scheme.
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index b172ce6a722e..7fb864a1486c 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -545,7 +545,7 @@ static void rk_udphy_usb_bvalid_enable(struct rk_udphy *udphy, u8 enable)
* ---------------------------------------------------------------------------
*/
-static void rk_udphy_dplane_select(struct rk_udphy *udphy)
+static void rk_udphy_dp_lane_select(struct rk_udphy *udphy)
{
const struct rk_udphy_cfg *cfg = udphy->cfgs;
u32 value = 0;
@@ -574,7 +574,7 @@ static void rk_udphy_dplane_select(struct rk_udphy *udphy)
FIELD_PREP(DP_AUX_DOUT_SEL, udphy->dp_aux_dout_sel) | value);
}
-static void rk_udphy_dplane_enable(struct rk_udphy *udphy, int dp_lanes)
+static void rk_udphy_dp_lane_enable(struct rk_udphy *udphy, int dp_lanes)
{
u32 val = 0;
int i;
@@ -1049,9 +1049,9 @@ static int rk_udphy_dp_phy_power_on(struct phy *phy)
if (ret)
goto unlock;
- rk_udphy_dplane_enable(udphy, udphy->dp_lanes);
+ rk_udphy_dp_lane_enable(udphy, udphy->dp_lanes);
- rk_udphy_dplane_select(udphy);
+ rk_udphy_dp_lane_select(udphy);
unlock:
mutex_unlock(&udphy->mutex);
@@ -1069,7 +1069,7 @@ static int rk_udphy_dp_phy_power_off(struct phy *phy)
struct rk_udphy *udphy = phy_get_drvdata(phy);
mutex_lock(&udphy->mutex);
- rk_udphy_dplane_enable(udphy, 0);
+ rk_udphy_dp_lane_enable(udphy, 0);
rk_udphy_power_off(udphy, UDPHY_MODE_DP);
mutex_unlock(&udphy->mutex);
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* [PATCH v11 14/36] phy: rockchip: usbdp: Use FIELD_PREP_WM16_CONST
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (12 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 13/36] phy: rockchip: usbdp: Rename DP lane functions Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:02 ` [PATCH v11 15/36] phy: rockchip: usbdp: Cleanup DP lane selection function Sebastian Reichel
` (21 subsequent siblings)
35 siblings, 0 replies; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
Cleanup code by replacing open-coded version of FIELD_PREP_WM16_CONST
with the existing helper macro.
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 7fb864a1486c..05593e98c16d 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -12,6 +12,7 @@
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/gpio.h>
+#include <linux/hw_bitfield.h>
#include <linux/mfd/syscon.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
@@ -75,7 +76,6 @@
#define TRSV_LN2_MON_RX_CDR_DONE_OFFSET 0x1b84 /* trsv_reg06E1 */
#define TRSV_LN2_MON_RX_CDR_LOCK_DONE BIT(0)
-#define BIT_WRITEABLE_SHIFT 16
#define PHY_AUX_DP_DATA_POL_NORMAL 0
#define PHY_AUX_DP_DATA_POL_INVERT 1
#define PHY_LANE_MUX_USB 0
@@ -104,8 +104,8 @@ struct rk_udphy_grf_reg {
#define _RK_UDPHY_GEN_GRF_REG(offset, mask, disable, enable) \
{\
offset, \
- FIELD_PREP_CONST(mask, disable) | (mask << BIT_WRITEABLE_SHIFT), \
- FIELD_PREP_CONST(mask, enable) | (mask << BIT_WRITEABLE_SHIFT), \
+ FIELD_PREP_WM16_CONST(mask, disable), \
+ FIELD_PREP_WM16_CONST(mask, enable), \
}
#define RK_UDPHY_GEN_GRF_REG(offset, bitend, bitstart, disable, enable) \
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* [PATCH v11 15/36] phy: rockchip: usbdp: Cleanup DP lane selection function
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (13 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 14/36] phy: rockchip: usbdp: Use FIELD_PREP_WM16_CONST Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:02 ` [PATCH v11 16/36] phy: rockchip: usbdp: Register DP aux bridge Sebastian Reichel
` (20 subsequent siblings)
35 siblings, 0 replies; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
Use FIELD_PREP_WM16() helpers to simplify the DP lane selection
logic.
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 28 +++++++---------------------
1 file changed, 7 insertions(+), 21 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 05593e98c16d..eda3f7a1e267 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -548,30 +548,16 @@ static void rk_udphy_usb_bvalid_enable(struct rk_udphy *udphy, u8 enable)
static void rk_udphy_dp_lane_select(struct rk_udphy *udphy)
{
const struct rk_udphy_cfg *cfg = udphy->cfgs;
- u32 value = 0;
-
- switch (udphy->dp_lanes) {
- case 4:
- value |= 3 << udphy->dp_lane_sel[3] * 2;
- value |= 2 << udphy->dp_lane_sel[2] * 2;
- fallthrough;
-
- case 2:
- value |= 1 << udphy->dp_lane_sel[1] * 2;
- fallthrough;
+ u32 value = FIELD_PREP_WM16(DP_LANE_SEL_ALL, 0);
+ int i;
- case 1:
- value |= 0 << udphy->dp_lane_sel[0] * 2;
- break;
+ for (i = 0; i < udphy->dp_lanes; i++)
+ value |= field_prep(DP_LANE_SEL_N(udphy->dp_lane_sel[i]), i);
- default:
- break;
- }
+ value |= FIELD_PREP_WM16(DP_AUX_DIN_SEL, udphy->dp_aux_din_sel);
+ value |= FIELD_PREP_WM16(DP_AUX_DOUT_SEL, udphy->dp_aux_dout_sel);
- regmap_write(udphy->vogrf, cfg->vogrfcfg[udphy->id].dp_lane_reg,
- ((DP_AUX_DIN_SEL | DP_AUX_DOUT_SEL | DP_LANE_SEL_ALL) << 16) |
- FIELD_PREP(DP_AUX_DIN_SEL, udphy->dp_aux_din_sel) |
- FIELD_PREP(DP_AUX_DOUT_SEL, udphy->dp_aux_dout_sel) | value);
+ regmap_write(udphy->vogrf, cfg->vogrfcfg[udphy->id].dp_lane_reg, value);
}
static void rk_udphy_dp_lane_enable(struct rk_udphy *udphy, int dp_lanes)
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* [PATCH v11 16/36] phy: rockchip: usbdp: Register DP aux bridge
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (14 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 15/36] phy: rockchip: usbdp: Cleanup DP lane selection function Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:02 ` [PATCH v11 17/36] phy: rockchip: usbdp: Drop DP HPD handling Sebastian Reichel
` (19 subsequent siblings)
35 siblings, 0 replies; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
Add support to use USB-C connectors with the DP altmode helper code on
devicetree based platforms. To get this working there must be a DRM
bridge chain from the DisplayPort controller to the USB-C connector.
E.g. on Rockchip RK3576:
root@rk3576 # cat /sys/kernel/debug/dri/0/encoder-0/bridges
bridge[0]: dw_dp_bridge_funcs
refcount: 7
type: [10] DP
OF: /soc/dp@27e40000:rockchip,rk3576-dp
ops: [0x47] detect edid hpd
bridge[1]: drm_aux_bridge_funcs
refcount: 4
type: [0] Unknown
OF: /soc/phy@2b010000:rockchip,rk3576-usbdp-phy
ops: [0x0]
bridge[2]: drm_aux_hpd_bridge_funcs
refcount: 5
type: [10] DP
OF: /soc/i2c@2ac50000/typec-portc@22/connector:usb-c-connector
ops: [0x4] hpd
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/Kconfig | 2 ++
drivers/phy/rockchip/phy-rockchip-usbdp.c | 17 +++++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig
index 14698571b607..39759bb2fa1d 100644
--- a/drivers/phy/rockchip/Kconfig
+++ b/drivers/phy/rockchip/Kconfig
@@ -136,8 +136,10 @@ config PHY_ROCKCHIP_USBDP
tristate "Rockchip USBDP COMBO PHY Driver"
depends on ARCH_ROCKCHIP && OF
depends on TYPEC
+ depends on DRM || DRM=n
select GENERIC_PHY
select USB_COMMON
+ select DRM_AUX_BRIDGE if DRM_BRIDGE
help
Enable this to support the Rockchip USB3.0/DP combo PHY with
Samsung IP block. This is required for USB3 support on RK3588.
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index eda3f7a1e267..8ac6a83b0b2a 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -6,6 +6,7 @@
* Copyright (C) 2024 Collabora Ltd
*/
+#include <drm/bridge/aux-bridge.h>
#include <dt-bindings/phy/phy.h>
#include <linux/bitfield.h>
#include <linux/bits.h>
@@ -1414,6 +1415,7 @@ static int rk_udphy_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct phy_provider *phy_provider;
+ struct fwnode_handle *dp_aux_ep;
struct resource *res;
struct rk_udphy *udphy;
void __iomem *base;
@@ -1468,6 +1470,21 @@ static int rk_udphy_probe(struct platform_device *pdev)
return ret;
}
+ /*
+ * Only register the DRM bridge, if the DP aux channel is connected.
+ * Some boards use the USBDP PHY only for its USB3 capabilities. The
+ * aux bridge itself will be registered using port 0, endpoint 0, which
+ * is fine as that is the actual superspeed data connection shared by
+ * USB3 and DP based on the mux config.
+ */
+ dp_aux_ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 3, 0, 0);
+ if (dp_aux_ep) {
+ ret = drm_aux_bridge_register(dev);
+ fwnode_handle_put(dp_aux_ep);
+ if (ret)
+ return ret;
+ }
+
udphy->phy_u3 = devm_phy_create(dev, dev->of_node, &rk_udphy_usb3_phy_ops);
if (IS_ERR(udphy->phy_u3)) {
ret = PTR_ERR(udphy->phy_u3);
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* [PATCH v11 17/36] phy: rockchip: usbdp: Drop DP HPD handling
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (15 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 16/36] phy: rockchip: usbdp: Register DP aux bridge Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:23 ` sashiko-bot
2026-07-09 19:02 ` [PATCH v11 18/36] phy: rockchip: usbdp: Rename mode_change to phy_needs_reinit Sebastian Reichel
` (18 subsequent siblings)
35 siblings, 1 reply; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
Drop the HPD handling logic from the USBDP PHY. The registers involved
require the display controller power domain being enabled and thus the
HPD signal should be handled by the displayport controller itself.
Apart from that the HPD handling as it is done here is incorrect and
misses hotplug events happening after the USB-C connector (e.g. when
a USB-C to HDMI adapter is involved and the HDMI cable is replugged).
Proper USB-C DP HPD support requires some restructuring of the DP
controller driver, which will happen independent of this patch. The
mainline kernel does not yet support USB-C DP AltMode on RK3588 and
RK3576, so it is fine to drop this code without adding the counterpart
in the DRM in an atomic change.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 85 ++++---------------------------
1 file changed, 9 insertions(+), 76 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 8ac6a83b0b2a..24108816e3b9 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -128,7 +128,6 @@ struct rk_udphy_grf_cfg {
struct rk_udphy_vogrf_cfg {
/* vo-grf */
- struct rk_udphy_grf_reg hpd_trigger;
u32 dp_lane_reg;
};
@@ -186,14 +185,11 @@ struct rk_udphy {
u32 dp_lane_sel[4];
u32 dp_aux_dout_sel;
u32 dp_aux_din_sel;
- bool dp_sink_hpd_sel;
- bool dp_sink_hpd_cfg;
unsigned int link_rate;
unsigned int lanes;
u8 bw;
int id;
- bool dp_in_use;
int dp_lanes;
/* PHY const config */
@@ -577,19 +573,6 @@ static void rk_udphy_dp_lane_enable(struct rk_udphy *udphy, int dp_lanes)
CMN_DP_CMN_RSTN, FIELD_PREP(CMN_DP_CMN_RSTN, 0x0));
}
-static void rk_udphy_dp_hpd_event_trigger(struct rk_udphy *udphy, bool hpd)
-{
- const struct rk_udphy_cfg *cfg = udphy->cfgs;
-
- udphy->dp_sink_hpd_sel = true;
- udphy->dp_sink_hpd_cfg = hpd;
-
- if (!udphy->dp_in_use)
- return;
-
- rk_udphy_grfreg_write(udphy->vogrf, &cfg->vogrfcfg[udphy->id].hpd_trigger, hpd);
-}
-
static void rk_udphy_mode_set(struct rk_udphy *udphy, u8 mode)
{
if (udphy->mode == mode)
@@ -1000,29 +983,6 @@ static void rk_udphy_power_off(struct rk_udphy *udphy, u8 mode)
rk_udphy_disable(udphy);
}
-static int rk_udphy_dp_phy_init(struct phy *phy)
-{
- struct rk_udphy *udphy = phy_get_drvdata(phy);
-
- mutex_lock(&udphy->mutex);
-
- udphy->dp_in_use = true;
-
- mutex_unlock(&udphy->mutex);
-
- return 0;
-}
-
-static int rk_udphy_dp_phy_exit(struct phy *phy)
-{
- struct rk_udphy *udphy = phy_get_drvdata(phy);
-
- mutex_lock(&udphy->mutex);
- udphy->dp_in_use = false;
- mutex_unlock(&udphy->mutex);
- return 0;
-}
-
static int rk_udphy_dp_phy_power_on(struct phy *phy)
{
struct rk_udphy *udphy = phy_get_drvdata(phy);
@@ -1254,8 +1214,6 @@ static int rk_udphy_dp_phy_configure(struct phy *phy,
}
static const struct phy_ops rk_udphy_dp_phy_ops = {
- .init = rk_udphy_dp_phy_init,
- .exit = rk_udphy_dp_phy_exit,
.power_on = rk_udphy_dp_phy_power_on,
.power_off = rk_udphy_dp_phy_power_off,
.configure = rk_udphy_dp_phy_configure,
@@ -1309,6 +1267,14 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
struct rk_udphy *udphy = typec_mux_get_drvdata(mux);
u8 mode;
+ /*
+ * Ignore mux events not involving DP AltMode, because
+ * the mode field is being reused, e.g. state->mode == 4
+ * could be either TYPEC_MODE_USB4 or TYPEC_DP_STATE_C.
+ */
+ if (!state->alt || state->alt->svid != USB_TYPEC_DP_SID)
+ return 0;
+
mutex_lock(&udphy->mutex);
switch (state->mode) {
@@ -1340,22 +1306,7 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
break;
}
- if (state->alt && state->alt->svid == USB_TYPEC_DP_SID) {
- struct typec_displayport_data *data = state->data;
-
- if (!data) {
- rk_udphy_dp_hpd_event_trigger(udphy, false);
- } else if (data->status & DP_STATUS_IRQ_HPD) {
- rk_udphy_dp_hpd_event_trigger(udphy, false);
- usleep_range(750, 800);
- rk_udphy_dp_hpd_event_trigger(udphy, true);
- } else if (data->status & DP_STATUS_HPD_STATE) {
- rk_udphy_mode_set(udphy, mode);
- rk_udphy_dp_hpd_event_trigger(udphy, true);
- } else {
- rk_udphy_dp_hpd_event_trigger(udphy, false);
- }
- }
+ rk_udphy_mode_set(udphy, mode);
mutex_unlock(&udphy->mutex);
return 0;
@@ -1510,20 +1461,6 @@ static int rk_udphy_probe(struct platform_device *pdev)
return 0;
}
-static int __maybe_unused rk_udphy_resume(struct device *dev)
-{
- struct rk_udphy *udphy = dev_get_drvdata(dev);
-
- if (udphy->dp_sink_hpd_sel)
- rk_udphy_dp_hpd_event_trigger(udphy, udphy->dp_sink_hpd_cfg);
-
- return 0;
-}
-
-static const struct dev_pm_ops rk_udphy_pm_ops = {
- SET_LATE_SYSTEM_SLEEP_PM_OPS(NULL, rk_udphy_resume)
-};
-
static const char * const rk_udphy_rst_list[] = {
"init", "cmn", "lane", "pcs_apb", "pma_apb"
};
@@ -1547,7 +1484,6 @@ static const struct rk_udphy_cfg rk3576_udphy_cfgs = {
},
.vogrfcfg = {
{
- .hpd_trigger = RK_UDPHY_GEN_GRF_REG(0x0000, 11, 10, 1, 3),
.dp_lane_reg = 0x0000,
},
},
@@ -1588,11 +1524,9 @@ static const struct rk_udphy_cfg rk3588_udphy_cfgs = {
},
.vogrfcfg = {
{
- .hpd_trigger = RK_UDPHY_GEN_GRF_REG(0x0000, 11, 10, 1, 3),
.dp_lane_reg = 0x0000,
},
{
- .hpd_trigger = RK_UDPHY_GEN_GRF_REG(0x0008, 11, 10, 1, 3),
.dp_lane_reg = 0x0008,
},
},
@@ -1628,7 +1562,6 @@ static struct platform_driver rk_udphy_driver = {
.driver = {
.name = "rockchip-usbdp-phy",
.of_match_table = rk_udphy_dt_match,
- .pm = &rk_udphy_pm_ops,
},
};
module_platform_driver(rk_udphy_driver);
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v11 17/36] phy: rockchip: usbdp: Drop DP HPD handling
2026-07-09 19:02 ` [PATCH v11 17/36] phy: rockchip: usbdp: Drop DP HPD handling Sebastian Reichel
@ 2026-07-09 19:23 ` sashiko-bot
0 siblings, 0 replies; 56+ messages in thread
From: sashiko-bot @ 2026-07-09 19:23 UTC (permalink / raw)
To: Sebastian Reichel
Cc: conor+dt, linux-phy, olteanv, vkoul, neil.armstrong, devicetree,
robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The typec_mux_set callback explicitly ignores base Type-C states (TYPEC_STATE_SAFE and TYPEC_STATE_USB), preventing the PHY from reverting to standard USB mode when exiting DP AltMode.
--
--- Patch [17]: [PATCH v11 17/36] phy: rockchip: usbdp: Drop DP HPD handling ---
commit 4dd3353e7fa8e127b70281da8c2e85645bfbef65
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Drop DP HPD handling
This commit removes the HPD (Hot Plug Detect) handling logic from the
USBDP PHY driver, delegating it to the displayport controller. It also
simplifies the typec_mux_set callback by dropping AltMode specific event
handling and adding an early return for non-DP events.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 8ac6a83b0b2a9..24108816e3b95 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -1309,6 +1267,14 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
> struct rk_udphy *udphy = typec_mux_get_drvdata(mux);
> u8 mode;
>
> + /*
> + * Ignore mux events not involving DP AltMode, because
> + * the mode field is being reused, e.g. state->mode == 4
> + * could be either TYPEC_MODE_USB4 or TYPEC_DP_STATE_C.
> + */
> + if (!state->alt || state->alt->svid != USB_TYPEC_DP_SID)
> + return 0;
[Severity: High]
This isn't a bug introduced by this patch, but does this early return
prevent the PHY from reverting to standard USB mode when exiting DP AltMode?
When a user dynamically disables DP AltMode or the DP sink disconnects,
the Type-C core invokes typec_mux_set with state->mode = TYPEC_STATE_SAFE
and state->alt = NULL.
If this returns 0 immediately because state->alt is NULL, udphy->mode
remains stuck at UDPHY_MODE_DP. Later, rk_udphy_usb3_phy_init() will see
that the USB mode is not set (!(udphy->mode & UDPHY_MODE_USB)) and
will actively disable the USB3 port.
Should this condition be adjusted to only filter modal states, perhaps by
checking if state->mode >= TYPEC_STATE_MODAL before filtering based on
the AltMode SVID?
> +
> mutex_lock(&udphy->mutex);
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=17
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v11 18/36] phy: rockchip: usbdp: Rename mode_change to phy_needs_reinit
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (16 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 17/36] phy: rockchip: usbdp: Drop DP HPD handling Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:02 ` [PATCH v11 19/36] phy: rockchip: usbdp: Re-init the PHY on orientation change Sebastian Reichel
` (17 subsequent siblings)
35 siblings, 0 replies; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
Right now the mode_change property is set whenever the mode changes
between USB-only, DP-only and USB-DP. It is needed, because on any
mode change the PHY needs to be re-initialized. Apparently at least
DP also requires a re-init when the cable orientation is changed,
which is currently not being done (except when the orientation switch
also involves a mode change). Prepare for this by renaming mode_change
to phy_needs_reinit.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 24108816e3b9..e44d19c9d119 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -171,7 +171,7 @@ struct rk_udphy {
/* PHY status management */
bool flip;
- bool mode_change;
+ bool phy_needs_reinit;
u8 mode;
u8 status;
@@ -578,7 +578,7 @@ static void rk_udphy_mode_set(struct rk_udphy *udphy, u8 mode)
if (udphy->mode == mode)
return;
- udphy->mode_change = true;
+ udphy->phy_needs_reinit = true;
udphy->mode = mode;
}
@@ -951,15 +951,15 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
if (udphy->mode & UDPHY_MODE_USB)
rk_udphy_u3_port_disable(udphy, false);
- udphy->mode_change = false;
- } else if (udphy->mode_change) {
+ udphy->phy_needs_reinit = false;
+ } else if (udphy->phy_needs_reinit) {
if (udphy->mode == UDPHY_MODE_DP)
rk_udphy_u3_port_disable(udphy, true);
ret = rk_udphy_init(udphy);
if (ret)
return ret;
- udphy->mode_change = false;
+ udphy->phy_needs_reinit = false;
}
udphy->status |= mode;
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* [PATCH v11 19/36] phy: rockchip: usbdp: Re-init the PHY on orientation change
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (17 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 18/36] phy: rockchip: usbdp: Rename mode_change to phy_needs_reinit Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:26 ` sashiko-bot
2026-07-09 19:02 ` [PATCH v11 20/36] phy: rockchip: usbdp: Factor out lane_mux_sel setup Sebastian Reichel
` (16 subsequent siblings)
35 siblings, 1 reply; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
Changing the cable orientation reconfigures the lane muxing, which
requires re-initializing the PHY. Without this DP functionality
breaks, if the cable is re-plugged with swapped orientation.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index e44d19c9d119..4b454798c4ed 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -620,6 +620,7 @@ static int rk_udphy_orien_sw_set(struct typec_switch_dev *sw,
enum typec_orientation orien)
{
struct rk_udphy *udphy = typec_switch_get_drvdata(sw);
+ bool flipped = orien == TYPEC_ORIENTATION_REVERSE;
mutex_lock(&udphy->mutex);
@@ -631,7 +632,10 @@ static int rk_udphy_orien_sw_set(struct typec_switch_dev *sw,
goto unlock_ret;
}
- udphy->flip = orien == TYPEC_ORIENTATION_REVERSE;
+ if (udphy->flip != flipped)
+ udphy->phy_needs_reinit = true;
+
+ udphy->flip = flipped;
rk_udphy_set_typec_default_mapping(udphy);
rk_udphy_usb_bvalid_enable(udphy, true);
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v11 19/36] phy: rockchip: usbdp: Re-init the PHY on orientation change
2026-07-09 19:02 ` [PATCH v11 19/36] phy: rockchip: usbdp: Re-init the PHY on orientation change Sebastian Reichel
@ 2026-07-09 19:26 ` sashiko-bot
0 siblings, 0 replies; 56+ messages in thread
From: sashiko-bot @ 2026-07-09 19:26 UTC (permalink / raw)
To: Sebastian Reichel
Cc: devicetree, olteanv, robh, neil.armstrong, conor+dt, linux-phy,
vkoul
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The USB 3.0 PHY hardware is not re-initialized during an orientation switch for USB-only devices, causing USB 3.0 to silently fail.
--
--- Patch [19]: [PATCH v11 19/36] phy: rockchip: usbdp: Re-init the PHY on orientation change ---
commit 68cb52c178c7db7d63bbe3d9ed1bf373d3c040be
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Re-init the PHY on orientation change
This commit updates the Type-C orientation switch callback to track cable
orientation changes. It sets a flag to trigger PHY re-initialization when
the cable is flipped to fix DisplayPort functionality.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -620,6 +620,7 @@ static int rk_udphy_orien_sw_set(struct typec_switch_dev *sw,
> enum typec_orientation orien)
> {
> struct rk_udphy *udphy = typec_switch_get_drvdata(sw);
> + bool flipped = orien == TYPEC_ORIENTATION_REVERSE;
>
> mutex_lock(&udphy->mutex);
>
> @@ -631,7 +632,10 @@ static int rk_udphy_orien_sw_set(struct typec_switch_dev *sw,
> goto unlock_ret;
> }
>
> - udphy->flip = orien == TYPEC_ORIENTATION_REVERSE;
> + if (udphy->flip != flipped)
> + udphy->phy_needs_reinit = true;
[Severity: High]
This is a pre-existing issue, but does this actually fix the orientation swap
for fast replugs on USB-only devices?
If a user rapidly unplugs and replugs a Type-C cable upside down, the DWC3
controller's 5-second runtime PM autosuspend delay means the PHY active state
is not dynamically cycled via phy_init or phy_power_on.
Because rk_udphy_orien_sw_set() only sets a deferred phy_needs_reinit flag,
the hardware lane muxing will remain stuck in the old orientation if the PHY
is already active. This causes USB 3.0 connections to silently fail and fall
back to USB 2.0 speeds.
Is there a mechanism to force the hardware re-initialization immediately if
the PHY is currently active during this swap?
> +
> + udphy->flip = flipped;
> rk_udphy_set_typec_default_mapping(udphy);
> rk_udphy_usb_bvalid_enable(udphy, true);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=19
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v11 20/36] phy: rockchip: usbdp: Factor out lane_mux_sel setup
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (18 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 19/36] phy: rockchip: usbdp: Re-init the PHY on orientation change Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:02 ` [PATCH v11 21/36] phy: rockchip: usbdp: Properly handle TYPEC_STATE_SAFE and TYPEC_STATE_USB Sebastian Reichel
` (15 subsequent siblings)
35 siblings, 0 replies; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
Avoid describing the USB+DP lane_mux_sel logic twice by introducing
a helper function to reduce code duplication.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 81 +++++++++++++++----------------
1 file changed, 40 insertions(+), 41 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 4b454798c4ed..4e54020d1755 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -582,6 +582,42 @@ static void rk_udphy_mode_set(struct rk_udphy *udphy, u8 mode)
udphy->mode = mode;
}
+static void rk_udphy_set_typec_state(struct rk_udphy *udphy, unsigned long state)
+{
+ u8 mode;
+
+ switch (state) {
+ case TYPEC_DP_STATE_C:
+ case TYPEC_DP_STATE_E:
+ udphy->lane_mux_sel[0] = PHY_LANE_MUX_DP;
+ udphy->lane_mux_sel[1] = PHY_LANE_MUX_DP;
+ udphy->lane_mux_sel[2] = PHY_LANE_MUX_DP;
+ udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
+ mode = UDPHY_MODE_DP;
+ udphy->dp_lanes = 4;
+ break;
+
+ case TYPEC_DP_STATE_D:
+ default:
+ if (udphy->flip) {
+ udphy->lane_mux_sel[0] = PHY_LANE_MUX_DP;
+ udphy->lane_mux_sel[1] = PHY_LANE_MUX_DP;
+ udphy->lane_mux_sel[2] = PHY_LANE_MUX_USB;
+ udphy->lane_mux_sel[3] = PHY_LANE_MUX_USB;
+ } else {
+ udphy->lane_mux_sel[0] = PHY_LANE_MUX_USB;
+ udphy->lane_mux_sel[1] = PHY_LANE_MUX_USB;
+ udphy->lane_mux_sel[2] = PHY_LANE_MUX_DP;
+ udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
+ }
+ mode = UDPHY_MODE_DP_USB;
+ udphy->dp_lanes = 2;
+ break;
+ }
+
+ rk_udphy_mode_set(udphy, mode);
+}
+
static void rk_udphy_set_typec_default_mapping(struct rk_udphy *udphy)
{
if (udphy->flip) {
@@ -589,10 +625,6 @@ static void rk_udphy_set_typec_default_mapping(struct rk_udphy *udphy)
udphy->dp_lane_sel[1] = 1;
udphy->dp_lane_sel[2] = 3;
udphy->dp_lane_sel[3] = 2;
- udphy->lane_mux_sel[0] = PHY_LANE_MUX_DP;
- udphy->lane_mux_sel[1] = PHY_LANE_MUX_DP;
- udphy->lane_mux_sel[2] = PHY_LANE_MUX_USB;
- udphy->lane_mux_sel[3] = PHY_LANE_MUX_USB;
udphy->dp_aux_dout_sel = PHY_AUX_DP_DATA_POL_INVERT;
udphy->dp_aux_din_sel = PHY_AUX_DP_DATA_POL_INVERT;
gpiod_set_value_cansleep(udphy->sbu1_dc_gpio, 1);
@@ -602,18 +634,14 @@ static void rk_udphy_set_typec_default_mapping(struct rk_udphy *udphy)
udphy->dp_lane_sel[1] = 3;
udphy->dp_lane_sel[2] = 1;
udphy->dp_lane_sel[3] = 0;
- udphy->lane_mux_sel[0] = PHY_LANE_MUX_USB;
- udphy->lane_mux_sel[1] = PHY_LANE_MUX_USB;
- udphy->lane_mux_sel[2] = PHY_LANE_MUX_DP;
- udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
udphy->dp_aux_dout_sel = PHY_AUX_DP_DATA_POL_NORMAL;
udphy->dp_aux_din_sel = PHY_AUX_DP_DATA_POL_NORMAL;
gpiod_set_value_cansleep(udphy->sbu1_dc_gpio, 0);
gpiod_set_value_cansleep(udphy->sbu2_dc_gpio, 1);
}
- rk_udphy_mode_set(udphy, UDPHY_MODE_DP_USB);
- udphy->dp_lanes = 2;
+ /* default to USB3 + DP as 4 lane USB is not supported */
+ rk_udphy_set_typec_state(udphy, TYPEC_DP_STATE_D);
}
static int rk_udphy_orien_sw_set(struct typec_switch_dev *sw,
@@ -1269,7 +1297,6 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
struct typec_mux_state *state)
{
struct rk_udphy *udphy = typec_mux_get_drvdata(mux);
- u8 mode;
/*
* Ignore mux events not involving DP AltMode, because
@@ -1281,38 +1308,10 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
mutex_lock(&udphy->mutex);
- switch (state->mode) {
- case TYPEC_DP_STATE_C:
- case TYPEC_DP_STATE_E:
- udphy->lane_mux_sel[0] = PHY_LANE_MUX_DP;
- udphy->lane_mux_sel[1] = PHY_LANE_MUX_DP;
- udphy->lane_mux_sel[2] = PHY_LANE_MUX_DP;
- udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
- mode = UDPHY_MODE_DP;
- udphy->dp_lanes = 4;
- break;
-
- case TYPEC_DP_STATE_D:
- default:
- if (udphy->flip) {
- udphy->lane_mux_sel[0] = PHY_LANE_MUX_DP;
- udphy->lane_mux_sel[1] = PHY_LANE_MUX_DP;
- udphy->lane_mux_sel[2] = PHY_LANE_MUX_USB;
- udphy->lane_mux_sel[3] = PHY_LANE_MUX_USB;
- } else {
- udphy->lane_mux_sel[0] = PHY_LANE_MUX_USB;
- udphy->lane_mux_sel[1] = PHY_LANE_MUX_USB;
- udphy->lane_mux_sel[2] = PHY_LANE_MUX_DP;
- udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
- }
- mode = UDPHY_MODE_DP_USB;
- udphy->dp_lanes = 2;
- break;
- }
-
- rk_udphy_mode_set(udphy, mode);
+ rk_udphy_set_typec_state(udphy, state->mode);
mutex_unlock(&udphy->mutex);
+
return 0;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* [PATCH v11 21/36] phy: rockchip: usbdp: Properly handle TYPEC_STATE_SAFE and TYPEC_STATE_USB
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (19 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 20/36] phy: rockchip: usbdp: Factor out lane_mux_sel setup Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:27 ` sashiko-bot
2026-07-09 19:02 ` [PATCH v11 22/36] phy: rockchip: usbdp: Use guard functions for mutex Sebastian Reichel
` (14 subsequent siblings)
35 siblings, 1 reply; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel, Sashiko
Handle TYPEC_STATE_SAFE and TYPEC_STATE_USB Type-C state events,
so that the muxing is properly updated when exiting DP AltMode.
Fixes: 2f70bbddeb45 ("phy: rockchip: add usbdp combo phy driver")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/message/20260619155020.CC7361F000E9%40smtp.kernel.org
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 4e54020d1755..0399cbf96e19 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -1293,17 +1293,26 @@ static const struct phy_ops rk_udphy_usb3_phy_ops = {
.owner = THIS_MODULE,
};
+static bool rk_udphy_is_supported_mode(struct typec_mux_state *state)
+{
+ /* Handle Safe State and USB State */
+ if (state->mode < TYPEC_STATE_MODAL)
+ return true;
+
+ /* Handle DP AltMode */
+ if (state->alt && state->alt->svid == USB_TYPEC_DP_SID)
+ return true;
+
+ return false;
+}
+
static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
struct typec_mux_state *state)
{
struct rk_udphy *udphy = typec_mux_get_drvdata(mux);
- /*
- * Ignore mux events not involving DP AltMode, because
- * the mode field is being reused, e.g. state->mode == 4
- * could be either TYPEC_MODE_USB4 or TYPEC_DP_STATE_C.
- */
- if (!state->alt || state->alt->svid != USB_TYPEC_DP_SID)
+ /* Ignore mux events not involving USB or DP */
+ if (!rk_udphy_is_supported_mode(state))
return 0;
mutex_lock(&udphy->mutex);
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v11 21/36] phy: rockchip: usbdp: Properly handle TYPEC_STATE_SAFE and TYPEC_STATE_USB
2026-07-09 19:02 ` [PATCH v11 21/36] phy: rockchip: usbdp: Properly handle TYPEC_STATE_SAFE and TYPEC_STATE_USB Sebastian Reichel
@ 2026-07-09 19:27 ` sashiko-bot
0 siblings, 0 replies; 56+ messages in thread
From: sashiko-bot @ 2026-07-09 19:27 UTC (permalink / raw)
To: Sebastian Reichel
Cc: vkoul, olteanv, devicetree, linux-phy, robh, neil.armstrong,
conor+dt
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Exiting DP AltMode fails to physically restore USB lane muxing if the USB controller remains logically active.
- [High] `rk_udphy_usb3_phy_exit` leaks the USB PHY power state if the Type-C mux mode was changed to DP-only before the USB controller calls `phy_exit`.
--
--- Patch [21]: [PATCH v11 21/36] phy: rockchip: usbdp: Properly handle TYPEC_STATE_SAFE and TYPEC_STATE_USB ---
commit 8663a5e7459552707b3ef82190885d5f5b02aa6b
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Properly handle TYPEC_STATE_SAFE and TYPEC_STATE_USB
This commit adds support for TYPEC_STATE_SAFE and TYPEC_STATE_USB Type-C
state events to ensure muxing is properly updated when exiting DP AltMode.
It fixes an issue in the rockchip usbdp combo phy driver.
Fixes: 2f70bbddeb45 ("phy: rockchip: add usbdp combo phy driver")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/message/20260619155020.CC7361F000E9%40smtp.kernel.org
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 4e54020d17551..0399cbf96e195 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1293,17 +1293,26 @@ static const struct phy_ops rk_udphy_usb3_phy_ops = {
> .owner = THIS_MODULE,
> };
[Severity: High]
This is a pre-existing issue, but I am wondering if rk_udphy_usb3_phy_exit()
leaks the USB PHY power state when the Type-C mux mode is changed to DP-only
before the USB controller calls phy_exit()?
If the Type-C mux switches to 4-lane DP AltMode, udphy->mode becomes
UDPHY_MODE_DP. If the USB controller then suspends or is unbound, it calls
rk_udphy_usb3_phy_exit(), which returns early:
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_usb3_phy_exit() {
...
/* DP only or high-speed */
if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
goto unlock;
rk_udphy_power_off(udphy, UDPHY_MODE_USB);
...
}
This skips rk_udphy_power_off(), leaving UDPHY_MODE_USB set in udphy->status.
Since status never reaches UDPHY_MODE_NONE when DP later powers off,
rk_udphy_disable() is never called. Will the PHY clocks and resets remain
permanently active when they should be suspended?
>
> +static bool rk_udphy_is_supported_mode(struct typec_mux_state *state)
> +{
> + /* Handle Safe State and USB State */
> + if (state->mode < TYPEC_STATE_MODAL)
> + return true;
> +
> + /* Handle DP AltMode */
> + if (state->alt && state->alt->svid == USB_TYPEC_DP_SID)
> + return true;
> +
> + return false;
> +}
> +
> static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
> struct typec_mux_state *state)
> {
> struct rk_udphy *udphy = typec_mux_get_drvdata(mux);
>
> - /*
> - * Ignore mux events not involving DP AltMode, because
> - * the mode field is being reused, e.g. state->mode == 4
> - * could be either TYPEC_MODE_USB4 or TYPEC_DP_STATE_C.
> - */
> - if (!state->alt || state->alt->svid != USB_TYPEC_DP_SID)
> + /* Ignore mux events not involving USB or DP */
> + if (!rk_udphy_is_supported_mode(state))
> return 0;
>
> mutex_lock(&udphy->mutex);
[Severity: High]
This is a pre-existing issue, but can the PHY get permanently stuck in 4-lane
DP mode when exiting DP AltMode if the USB controller remains logically active?
When rk_udphy_typec_mux_set() handles the transition to TYPEC_STATE_SAFE
or TYPEC_STATE_USB, it updates the software mode and sets phy_needs_reinit
to true via rk_udphy_set_typec_state() later in this function:
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_typec_mux_set() {
...
rk_udphy_set_typec_state(udphy, state->mode);
...
}
However, the hardware muxing is only updated in rk_udphy_power_on():
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_power_on() {
...
} else if (udphy->phy_needs_reinit) {
if (udphy->mode == UDPHY_MODE_DP)
rk_udphy_u3_port_disable(udphy, true);
ret = rk_udphy_init(udphy);
...
}
If the USB controller (e.g. dwc3) is already active, it will not call
phy_power_on() again. This causes the hardware re-initialization to be missed,
leaving the PHY physically in 4-lane DP mode. Is it possible for USB 3.x
to become completely unresponsive in this state, even though the Type-C
subsystem correctly returned to USB mode?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=21
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v11 22/36] phy: rockchip: usbdp: Use guard functions for mutex
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (20 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 21/36] phy: rockchip: usbdp: Properly handle TYPEC_STATE_SAFE and TYPEC_STATE_USB Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:02 ` [PATCH v11 23/36] phy: rockchip: usbdp: Clear USB status on PHY exit Sebastian Reichel
` (13 subsequent siblings)
35 siblings, 0 replies; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
Convert the driver to use guard functions for mutex handling as
a small cleanup. There is a small functional change in the DP PHY
power up function, which no longer sleeps if the internal powerup
code returns an error. This is not a problem as the sleep is only
relevant for successful power-up.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 54 +++++++++++++------------------
1 file changed, 23 insertions(+), 31 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 0399cbf96e19..8c165bcab796 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -10,6 +10,7 @@
#include <dt-bindings/phy/phy.h>
#include <linux/bitfield.h>
#include <linux/bits.h>
+#include <linux/cleanup.h>
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/gpio.h>
@@ -650,14 +651,15 @@ static int rk_udphy_orien_sw_set(struct typec_switch_dev *sw,
struct rk_udphy *udphy = typec_switch_get_drvdata(sw);
bool flipped = orien == TYPEC_ORIENTATION_REVERSE;
- mutex_lock(&udphy->mutex);
+ guard(mutex)(&udphy->mutex);
if (orien == TYPEC_ORIENTATION_NONE) {
gpiod_set_value_cansleep(udphy->sbu1_dc_gpio, 0);
gpiod_set_value_cansleep(udphy->sbu2_dc_gpio, 0);
/* unattached */
rk_udphy_usb_bvalid_enable(udphy, false);
- goto unlock_ret;
+
+ return 0;
}
if (udphy->flip != flipped)
@@ -667,8 +669,6 @@ static int rk_udphy_orien_sw_set(struct typec_switch_dev *sw,
rk_udphy_set_typec_default_mapping(udphy);
rk_udphy_usb_bvalid_enable(udphy, true);
-unlock_ret:
- mutex_unlock(&udphy->mutex);
return 0;
}
@@ -1020,26 +1020,25 @@ static int rk_udphy_dp_phy_power_on(struct phy *phy)
struct rk_udphy *udphy = phy_get_drvdata(phy);
int ret;
- mutex_lock(&udphy->mutex);
+ scoped_guard(mutex, &udphy->mutex) {
+ phy_set_bus_width(phy, udphy->dp_lanes);
- phy_set_bus_width(phy, udphy->dp_lanes);
-
- ret = rk_udphy_power_on(udphy, UDPHY_MODE_DP);
- if (ret)
- goto unlock;
+ ret = rk_udphy_power_on(udphy, UDPHY_MODE_DP);
+ if (ret)
+ return ret;
- rk_udphy_dp_lane_enable(udphy, udphy->dp_lanes);
+ rk_udphy_dp_lane_enable(udphy, udphy->dp_lanes);
- rk_udphy_dp_lane_select(udphy);
+ rk_udphy_dp_lane_select(udphy);
+ }
-unlock:
- mutex_unlock(&udphy->mutex);
/*
* If data send by aux channel too fast after phy power on,
* the aux may be not ready which will cause aux error. Adding
* delay to avoid this issue.
*/
usleep_range(10000, 11000);
+
return ret;
}
@@ -1047,10 +1046,10 @@ static int rk_udphy_dp_phy_power_off(struct phy *phy)
{
struct rk_udphy *udphy = phy_get_drvdata(phy);
- mutex_lock(&udphy->mutex);
+ guard(mutex)(&udphy->mutex);
+
rk_udphy_dp_lane_enable(udphy, 0);
rk_udphy_power_off(udphy, UDPHY_MODE_DP);
- mutex_unlock(&udphy->mutex);
return 0;
}
@@ -1255,35 +1254,30 @@ static const struct phy_ops rk_udphy_dp_phy_ops = {
static int rk_udphy_usb3_phy_init(struct phy *phy)
{
struct rk_udphy *udphy = phy_get_drvdata(phy);
- int ret = 0;
- mutex_lock(&udphy->mutex);
+ guard(mutex)(&udphy->mutex);
+
/* DP only or high-speed, disable U3 port */
if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs) {
rk_udphy_u3_port_disable(udphy, true);
- goto unlock;
+ return 0;
}
- ret = rk_udphy_power_on(udphy, UDPHY_MODE_USB);
-
-unlock:
- mutex_unlock(&udphy->mutex);
- return ret;
+ return rk_udphy_power_on(udphy, UDPHY_MODE_USB);
}
static int rk_udphy_usb3_phy_exit(struct phy *phy)
{
struct rk_udphy *udphy = phy_get_drvdata(phy);
- mutex_lock(&udphy->mutex);
+ guard(mutex)(&udphy->mutex);
+
/* DP only or high-speed */
if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
- goto unlock;
+ return 0;
rk_udphy_power_off(udphy, UDPHY_MODE_USB);
-unlock:
- mutex_unlock(&udphy->mutex);
return 0;
}
@@ -1315,12 +1309,10 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
if (!rk_udphy_is_supported_mode(state))
return 0;
- mutex_lock(&udphy->mutex);
+ guard(mutex)(&udphy->mutex);
rk_udphy_set_typec_state(udphy, state->mode);
- mutex_unlock(&udphy->mutex);
-
return 0;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* [PATCH v11 23/36] phy: rockchip: usbdp: Clear USB status on PHY exit
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (21 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 22/36] phy: rockchip: usbdp: Use guard functions for mutex Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:28 ` sashiko-bot
2026-07-09 19:02 ` [PATCH v11 24/36] phy: rockchip: usbdp: Hold mutex in DP PHY configure Sebastian Reichel
` (12 subsequent siblings)
35 siblings, 1 reply; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel, Sashiko
Ensure the USB status flag is cleared when the USB3 PHY is
exited while the system is in DP-only mode. This can happen
if the USB3 controller device is unbound while a DP-only
adapter is plugged into the USB-C port.
Fixes: 2f70bbddeb45 ("phy: rockchip: add usbdp combo phy driver")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/message/20260625-rockchip-usbdp-cleanup-v7-24-38eb3cf654fd%40collabora.com
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 8c165bcab796..a742bde7155b 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -1273,8 +1273,10 @@ static int rk_udphy_usb3_phy_exit(struct phy *phy)
guard(mutex)(&udphy->mutex);
/* DP only or high-speed */
- if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
+ if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs) {
+ udphy->status &= ~UDPHY_MODE_USB;
return 0;
+ }
rk_udphy_power_off(udphy, UDPHY_MODE_USB);
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v11 23/36] phy: rockchip: usbdp: Clear USB status on PHY exit
2026-07-09 19:02 ` [PATCH v11 23/36] phy: rockchip: usbdp: Clear USB status on PHY exit Sebastian Reichel
@ 2026-07-09 19:28 ` sashiko-bot
0 siblings, 0 replies; 56+ messages in thread
From: sashiko-bot @ 2026-07-09 19:28 UTC (permalink / raw)
To: Sebastian Reichel
Cc: olteanv, linux-phy, neil.armstrong, conor+dt, robh, devicetree,
vkoul
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Clearing the `UDPHY_MODE_USB` flag from `udphy->status` on the early exit path can leave the hardware powered on and leak clock references if the overall PHY status drops to `UDPHY_MODE_NONE`.
--
--- Patch [23]: [PATCH v11 23/36] phy: rockchip: usbdp: Clear USB status on PHY exit ---
commit 7906fa37f6a1255421fda4caa815efbe878ca40d
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Clear USB status on PHY exit
This commit ensures the USB status flag is cleared when the USB3 PHY is
exited while the system is in DP-only mode to handle unbound controllers.
Link: https://sashiko.dev/#/message/20260625-rockchip-usbdp-cleanup-v7-24-38eb3cf654fd%40collabora.com
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1273,8 +1273,10 @@ static int rk_udphy_usb3_phy_exit(struct phy *phy)
> guard(mutex)(&udphy->mutex);
>
> /* DP only or high-speed */
> - if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
> + if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs) {
> + udphy->status &= ~UDPHY_MODE_USB;
> return 0;
> + }
[Severity: High]
Will this early return leak clock references if the overall PHY status drops
to 0 (UDPHY_MODE_NONE)?
If the USB3 PHY was previously initialized (enabling clocks via
rk_udphy_setup) and the mode switches to DP-only, udphy->status gets
cleared here.
Returning immediately skips rk_udphy_power_off() and its internal check
that calls rk_udphy_disable() when the status reaches 0.
Could this leave the hardware powered on and leak the clock references
obtained during initialization?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=23
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v11 24/36] phy: rockchip: usbdp: Hold mutex in DP PHY configure
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (22 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 23/36] phy: rockchip: usbdp: Clear USB status on PHY exit Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:02 ` [PATCH v11 25/36] phy: rockchip: usbdp: Add some extra debug messages Sebastian Reichel
` (11 subsequent siblings)
35 siblings, 0 replies; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel, Sashiko
rk_udphy_dp_phy_configure() accesses some variables from the struct
rk_udphy, which are updated independently from the USB-C framework.
The USB-C mux/orientation switch functions already hold a mutex to
ensure mutual exclusive access to the struct rk_udphy states, so
simply hold the same one in the DP PHY configuration function.
Reproducing problems due to this on real hardware would be really hard,
but could be possible when quickly re-connecting the USB-C connector.
Fixes: 2f70bbddeb45 ("phy: rockchip: add usbdp combo phy driver")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/linux-phy/20260612164627.23D391F000E9@smtp.kernel.org/
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index a742bde7155b..cd79c5da566a 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -1154,6 +1154,8 @@ static int rk_udphy_dp_phy_configure(struct phy *phy,
u32 i, val, lane;
int ret;
+ guard(mutex)(&udphy->mutex);
+
if (dp->set_rate) {
ret = rk_udphy_dp_phy_verify_link_rate(udphy, dp);
if (ret)
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* [PATCH v11 25/36] phy: rockchip: usbdp: Add some extra debug messages
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (23 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 24/36] phy: rockchip: usbdp: Hold mutex in DP PHY configure Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:02 ` [PATCH v11 26/36] phy: rockchip: usbdp: Avoid xHCI SErrors Sebastian Reichel
` (10 subsequent siblings)
35 siblings, 0 replies; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
It's useful to log PHY reinit to ease debugging issues around
USB-C hotplugging.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index cd79c5da566a..edee27933d89 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -24,6 +24,7 @@
#include <linux/property.h>
#include <linux/regmap.h>
#include <linux/reset.h>
+#include <linux/string_choices.h>
#include <linux/usb/ch9.h>
#include <linux/usb/typec_dp.h>
#include <linux/usb/typec_mux.h>
@@ -462,6 +463,8 @@ static int rk_udphy_reset_deassert(struct rk_udphy *udphy, char *name)
return reset_control_deassert(list[idx].rstc);
}
+ dev_err(udphy->dev, "failed to de-assert missing reset line: %s\n", name);
+
return -EINVAL;
}
@@ -488,6 +491,8 @@ static void rk_udphy_u3_port_disable(struct rk_udphy *udphy, u8 disable)
const struct rk_udphy_cfg *cfg = udphy->cfgs;
const struct rk_udphy_grf_reg *preg;
+ dev_dbg(udphy->dev, "USB3 port %s\n", str_on_off(!disable));
+
preg = udphy->id ? &cfg->grfcfg.usb3otg1_cfg : &cfg->grfcfg.usb3otg0_cfg;
rk_udphy_grfreg_write(udphy->usbgrf, preg, disable);
}
@@ -662,8 +667,10 @@ static int rk_udphy_orien_sw_set(struct typec_switch_dev *sw,
return 0;
}
- if (udphy->flip != flipped)
+ if (udphy->flip != flipped) {
+ dev_dbg(udphy->dev, "cable orientation changed, PHY re-init required.\n");
udphy->phy_needs_reinit = true;
+ }
udphy->flip = flipped;
rk_udphy_set_typec_default_mapping(udphy);
@@ -781,6 +788,11 @@ static int rk_udphy_init(struct rk_udphy *udphy)
const struct rk_udphy_cfg *cfg = udphy->cfgs;
int ret;
+ dev_dbg(udphy->dev, "reinit PHY with USB3=%s and DP=%s (%u lanes) flipped=%s\n",
+ str_on_off(udphy->mode & UDPHY_MODE_USB),
+ str_on_off(udphy->mode & UDPHY_MODE_DP),
+ udphy->dp_lanes, str_yes_no(udphy->flip));
+
rk_udphy_reset_assert_all(udphy);
usleep_range(10000, 11000);
@@ -851,6 +863,8 @@ static int rk_udphy_setup(struct rk_udphy *udphy)
{
int ret;
+ dev_dbg(udphy->dev, "enable PHY\n");
+
ret = clk_bulk_prepare_enable(udphy->num_clks, udphy->clks);
if (ret) {
dev_err(udphy->dev, "failed to enable clk\n");
@@ -869,6 +883,7 @@ static int rk_udphy_setup(struct rk_udphy *udphy)
static void rk_udphy_disable(struct rk_udphy *udphy)
{
+ dev_dbg(udphy->dev, "disable PHY\n");
clk_bulk_disable_unprepare(udphy->num_clks, udphy->clks);
rk_udphy_reset_assert_all(udphy);
}
@@ -1310,8 +1325,12 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
struct rk_udphy *udphy = typec_mux_get_drvdata(mux);
/* Ignore mux events not involving USB or DP */
- if (!rk_udphy_is_supported_mode(state))
+ if (!rk_udphy_is_supported_mode(state)) {
+ dev_dbg(udphy->dev, "ignore mux event with mode=%lu\n", state->mode);
return 0;
+ }
+
+ dev_dbg(udphy->dev, "new mode: %lu\n", state->mode);
guard(mutex)(&udphy->mutex);
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* [PATCH v11 26/36] phy: rockchip: usbdp: Avoid xHCI SErrors
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (24 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 25/36] phy: rockchip: usbdp: Add some extra debug messages Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:28 ` sashiko-bot
2026-07-09 19:02 ` [PATCH v11 27/36] phy: rockchip: usbdp: Handle rk_udphy_reset_deassert errors Sebastian Reichel
` (9 subsequent siblings)
35 siblings, 1 reply; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
The USBDP PHY provides the PIPE clock to the USB3 controller, which
means the PHY must be fully running when anything tries to access
the xHCI registers.
When switching between USB3-only, USB3 + DP and DP-only mode, the
PHY must be re-initialized resulting in a short period of the PHY
being disabled. If the DWC3 driver decides to access the xHCI at
this point the system will fail with an SError.
This patch avoids the problems by disabling the USB3 port before
re-initializing it. This does a couple of things:
- forces phystatus to 0 from GRF (not from PHY)
- switches PIPE clock source from PHY to UTMI (safe fallback clock)
- num_u3_port=0
The last part will be ignored, as DWC3 already probed, but the
clock re-routing will avoid the SError. There is a small delay
afterwards to make sure the mux happened. The datasheet gives
no hints how long it takes, so delay time is a guess.
Fixes: 2f70bbddeb45 ("phy: rockchip: add usbdp combo phy driver")
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index edee27933d89..9e77513643a3 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -1000,8 +1000,8 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
rk_udphy_u3_port_disable(udphy, false);
udphy->phy_needs_reinit = false;
} else if (udphy->phy_needs_reinit) {
- if (udphy->mode == UDPHY_MODE_DP)
- rk_udphy_u3_port_disable(udphy, true);
+ rk_udphy_u3_port_disable(udphy, true);
+ udelay(10);
ret = rk_udphy_init(udphy);
if (ret)
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v11 26/36] phy: rockchip: usbdp: Avoid xHCI SErrors
2026-07-09 19:02 ` [PATCH v11 26/36] phy: rockchip: usbdp: Avoid xHCI SErrors Sebastian Reichel
@ 2026-07-09 19:28 ` sashiko-bot
0 siblings, 0 replies; 56+ messages in thread
From: sashiko-bot @ 2026-07-09 19:28 UTC (permalink / raw)
To: Sebastian Reichel
Cc: olteanv, neil.armstrong, devicetree, vkoul, linux-phy, robh,
conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The USB3 port is unconditionally disabled before PHY re-initialization to avoid xHCI SErrors, but it is never re-enabled afterwards, permanently breaking USB3 functionality.
--
--- Patch [26]: [PATCH v11 26/36] phy: rockchip: usbdp: Avoid xHCI SErrors ---
commit 1f5e9aad179e51fb32f2117a4d8ff35248e145cc
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Avoid xHCI SErrors
The USBDP PHY provides the PIPE clock to the USB3 controller, which means
the PHY must be fully running when anything tries to access the xHCI
registers. This patch avoids SErrors during re-initialization by disabling
the USB3 port beforehand, forcing the clock source to a safe fallback.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index edee27933d890..9e77513643a33 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1000,8 +1000,8 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> rk_udphy_u3_port_disable(udphy, false);
> udphy->phy_needs_reinit = false;
> } else if (udphy->phy_needs_reinit) {
> - if (udphy->mode == UDPHY_MODE_DP)
> - rk_udphy_u3_port_disable(udphy, true);
> + rk_udphy_u3_port_disable(udphy, true);
> + udelay(10);
>
> ret = rk_udphy_init(udphy);
> if (ret)
[Severity: High]
Does this code permanently break the USB3 functionality?
We now unconditionally disable the USB3 port here before re-initializing the
PHY, but it appears there is no corresponding call to
rk_udphy_u3_port_disable(udphy, false) to re-enable it after rk_udphy_init()
completes.
Will this leave the DWC3 controller stuck on the UTMI fallback clock with
phystatus forced to 0, making the interface unusable after a mode switch?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=26
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v11 27/36] phy: rockchip: usbdp: Handle rk_udphy_reset_deassert errors
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (25 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 26/36] phy: rockchip: usbdp: Avoid xHCI SErrors Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:02 ` [PATCH v11 28/36] phy: rockchip: usbdp: Only enable USB3 when not in high-speed mode Sebastian Reichel
` (8 subsequent siblings)
35 siblings, 0 replies; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel, Sashiko
Handle rk_udphy_reset_deassert returning errors to avoid theoretical
(Rockchip reset controller driver does not return errors) SError.
Fixes: 2f70bbddeb45 ("phy: rockchip: add usbdp combo phy driver")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/message/20260626211151.2332F1F000E9%40smtp.kernel.org
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 9e77513643a3..c645fea67c4d 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -803,8 +803,12 @@ static int rk_udphy_init(struct rk_udphy *udphy)
/* Step 1: power on pma and deassert apb rstn */
rk_udphy_grfreg_write(udphy->udphygrf, &cfg->grfcfg.low_pwrn, true);
- rk_udphy_reset_deassert(udphy, "pma_apb");
- rk_udphy_reset_deassert(udphy, "pcs_apb");
+ ret = rk_udphy_reset_deassert(udphy, "pma_apb");
+ if (ret)
+ goto assert_resets;
+ ret = rk_udphy_reset_deassert(udphy, "pcs_apb");
+ if (ret)
+ goto assert_resets;
/* Step 2: set init sequence and phy refclk */
ret = regmap_multi_reg_write(udphy->pma_regmap, rk_udphy_init_sequence,
@@ -830,8 +834,11 @@ static int rk_udphy_init(struct rk_udphy *udphy)
FIELD_PREP(CMN_DP_LANE_EN_ALL, 0));
/* Step 4: deassert init rstn and wait for 200ns from datasheet */
- if (udphy->mode & UDPHY_MODE_USB)
- rk_udphy_reset_deassert(udphy, "init");
+ if (udphy->mode & UDPHY_MODE_USB) {
+ ret = rk_udphy_reset_deassert(udphy, "init");
+ if (ret)
+ goto assert_resets;
+ }
if (udphy->mode & UDPHY_MODE_DP) {
regmap_update_bits(udphy->pma_regmap, CMN_DP_RSTN_OFFSET,
@@ -843,8 +850,14 @@ static int rk_udphy_init(struct rk_udphy *udphy)
/* Step 5: deassert cmn/lane rstn */
if (udphy->mode & UDPHY_MODE_USB) {
- rk_udphy_reset_deassert(udphy, "cmn");
- rk_udphy_reset_deassert(udphy, "lane");
+ ret = rk_udphy_reset_deassert(udphy, "cmn");
+ if (ret)
+ goto assert_resets;
+
+ ret = rk_udphy_reset_deassert(udphy, "lane");
+ if (ret)
+ goto assert_resets;
+
}
/* Step 6: wait for lock done of pll */
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* [PATCH v11 28/36] phy: rockchip: usbdp: Only enable USB3 when not in high-speed mode
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (26 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 27/36] phy: rockchip: usbdp: Handle rk_udphy_reset_deassert errors Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:35 ` sashiko-bot
2026-07-09 19:02 ` [PATCH v11 29/36] phy: core: add notifier infrastructure Sebastian Reichel
` (7 subsequent siblings)
35 siblings, 1 reply; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel, Sashiko
Ensure that USB3 mode is not accidently enabled during PHY re-init
for systems that are configured as high-speed only via DT.
Fixes: 2f70bbddeb45 ("phy: rockchip: add usbdp combo phy driver")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/message/20260626212424.C215E1F000E9%40smtp.kernel.org
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index c645fea67c4d..694688ed387f 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -1009,7 +1009,7 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
if (ret)
return ret;
- if (udphy->mode & UDPHY_MODE_USB)
+ if (!udphy->hs && udphy->mode & UDPHY_MODE_USB)
rk_udphy_u3_port_disable(udphy, false);
udphy->phy_needs_reinit = false;
} else if (udphy->phy_needs_reinit) {
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v11 28/36] phy: rockchip: usbdp: Only enable USB3 when not in high-speed mode
2026-07-09 19:02 ` [PATCH v11 28/36] phy: rockchip: usbdp: Only enable USB3 when not in high-speed mode Sebastian Reichel
@ 2026-07-09 19:35 ` sashiko-bot
0 siblings, 0 replies; 56+ messages in thread
From: sashiko-bot @ 2026-07-09 19:35 UTC (permalink / raw)
To: Sebastian Reichel
Cc: linux-phy, vkoul, conor+dt, devicetree, neil.armstrong, olteanv,
robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The USB3 port is unconditionally disabled during PHY re-initialization but never re-enabled, permanently breaking USB3 functionality after a mode change.
--
--- Patch [28]: [PATCH v11 28/36] phy: rockchip: usbdp: Only enable USB3 when not in high-speed mode ---
commit f42d06c6d124fe98eab8964760884de8f170d304
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Only enable USB3 when not in high-speed mode
Ensure that USB3 mode is not accidently enabled during PHY re-init
for systems that are configured as high-speed only via DT.
Closes: https://sashiko.dev/#/message/20260626212424.C215E1F000E9%40smtp.kernel.org
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1009,7 +1009,7 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> if (ret)
> return ret;
>
> - if (udphy->mode & UDPHY_MODE_USB)
> + if (!udphy->hs && udphy->mode & UDPHY_MODE_USB)
> rk_udphy_u3_port_disable(udphy, false);
> udphy->phy_needs_reinit = false;
> } else if (udphy->phy_needs_reinit) {
> rk_udphy_u3_port_disable(udphy, true);
> udelay(10);
>
> ret = rk_udphy_init(udphy);
> if (ret)
> return ret;
> udphy->phy_needs_reinit = false;
> }
[Severity: High]
This is a pre-existing issue, but does this missing re-enablement permanently
break USB3 functionality after a mode change?
If the PHY is already active and a mode change occurs (such as plugging
in a DP monitor or a Type-C cable flip), udphy->phy_needs_reinit is set
to true.
When rk_udphy_power_on() is subsequently called, the U3 port is disabled
in this branch to avoid xHCI SErrors, but there is no corresponding call to
rk_udphy_u3_port_disable(udphy, false) to restore it after rk_udphy_init()
completes.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=28
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v11 29/36] phy: core: add notifier infrastructure
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (27 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 28/36] phy: rockchip: usbdp: Only enable USB3 when not in high-speed mode Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:31 ` sashiko-bot
2026-07-09 19:02 ` [PATCH v11 30/36] usb: dwc3: core: support PHY reset notifications Sebastian Reichel
` (6 subsequent siblings)
35 siblings, 1 reply; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
Some PHY devices with multiple ports (e.g. USB3 and DP) require a reset
if the configuration changes or cable orientation changes. This is a
problem, as the consumer device will run into undefined behavior.
With the new PHY notifier API introduced in this patch, the consumer
driver can hook into reset events coming from a PHY device to handle the
PHY going down gracefully.
Note that this uses -ENOSYS instead of the more sensible -ENOTSUP for
the stub functions when GENERIC_PHY is disabled to stay consistent with
the existing ones.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/phy-core.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/phy/phy.h | 40 +++++++++++++++++++++++++++++++++
2 files changed, 100 insertions(+)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 21aaf2f76e53..92c93d684ab0 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -542,6 +542,65 @@ int phy_notify_state(struct phy *phy, union phy_notify state)
}
EXPORT_SYMBOL_GPL(phy_notify_state);
+/**
+ * phy_register_notifier() - register a notifier for PHY events
+ * @phy: the phy returned by phy_get()
+ * @nb: notifier block to register
+ *
+ * Allows PHY consumers to receive notifications about PHY reset events.
+ * PHY providers can signal these events using phy_notify_reset().
+ *
+ * Returns: %0 if successful, a negative error code otherwise
+ */
+int phy_register_notifier(struct phy *phy, struct notifier_block *nb)
+{
+ if (!phy)
+ return 0;
+
+ return blocking_notifier_chain_register(&phy->notifier, nb);
+}
+EXPORT_SYMBOL_GPL(phy_register_notifier);
+
+/**
+ * phy_unregister_notifier() - unregister a notifier for PHY events
+ * @phy: the phy returned by phy_get()
+ * @nb: notifier block to unregister
+ *
+ * Returns: %0 if successful, a negative error code otherwise
+ */
+int phy_unregister_notifier(struct phy *phy, struct notifier_block *nb)
+{
+ if (!phy)
+ return 0;
+
+ return blocking_notifier_chain_unregister(&phy->notifier, nb);
+}
+EXPORT_SYMBOL_GPL(phy_unregister_notifier);
+
+/**
+ * phy_notify_reset() - notify consumers of a PHY reset event
+ * @phy: the phy that is being reset
+ * @event: the notification event (PRE_RESET or POST_RESET)
+ *
+ * Called by PHY providers to notify consumers that the PHY is about to
+ * be reset or has completed a reset. This allows consumers to quiesce
+ * hardware before the PHY becomes unavailable.
+ *
+ * Returns: %0 if successful or no notifiers registered, a negative error
+ * code if a notifier returns an error (for PRE_RESET only)
+ */
+int phy_notify_reset(struct phy *phy, enum phy_notification event)
+{
+ int ret;
+
+ if (!phy)
+ return 0;
+
+ ret = blocking_notifier_call_chain(&phy->notifier, event, phy);
+ return notifier_to_errno(ret);
+}
+EXPORT_SYMBOL_GPL(phy_notify_reset);
+
/**
* phy_configure() - Changes the phy parameters
* @phy: the phy returned by phy_get()
@@ -1018,6 +1077,7 @@ struct phy *phy_create(struct device *dev, struct device_node *node,
device_initialize(&phy->dev);
lockdep_register_key(&phy->lockdep_key);
mutex_init_with_key(&phy->mutex, &phy->lockdep_key);
+ BLOCKING_INIT_NOTIFIER_HEAD(&phy->notifier);
phy->dev.class = &phy_class;
phy->dev.parent = dev;
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index ea47975e288a..3779a4d0a02c 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -11,6 +11,7 @@
#define __DRIVERS_PHY_H
#include <linux/err.h>
+#include <linux/notifier.h>
#include <linux/of.h>
#include <linux/device.h>
#include <linux/pm_runtime.h>
@@ -53,6 +54,16 @@ enum phy_media {
PHY_MEDIA_DAC,
};
+/**
+ * enum phy_notification - PHY notification events
+ * @PHY_NOTIFY_PRE_RESET: PHY is about to be reset, consumers should quiesce
+ * @PHY_NOTIFY_POST_RESET: PHY reset is complete, consumers may resume
+ */
+enum phy_notification {
+ PHY_NOTIFY_PRE_RESET,
+ PHY_NOTIFY_POST_RESET,
+};
+
enum phy_ufs_state {
PHY_UFS_HIBERN8_ENTER,
PHY_UFS_HIBERN8_EXIT,
@@ -170,6 +181,7 @@ struct phy_attrs {
* @power_count: used to protect when the PHY is used by multiple consumers
* @attrs: used to specify PHY specific attributes
* @pwr: power regulator associated with the phy
+ * @notifier: notifier head for PHY reset events
* @debugfs: debugfs directory
*/
struct phy {
@@ -182,6 +194,7 @@ struct phy {
int power_count;
struct phy_attrs attrs;
struct regulator *pwr;
+ struct blocking_notifier_head notifier;
struct dentry *debugfs;
};
@@ -267,6 +280,9 @@ int phy_calibrate(struct phy *phy);
int phy_notify_connect(struct phy *phy, int port);
int phy_notify_disconnect(struct phy *phy, int port);
int phy_notify_state(struct phy *phy, union phy_notify state);
+int phy_register_notifier(struct phy *phy, struct notifier_block *nb);
+int phy_unregister_notifier(struct phy *phy, struct notifier_block *nb);
+int phy_notify_reset(struct phy *phy, enum phy_notification event);
static inline int phy_get_bus_width(struct phy *phy)
{
return phy->attrs.bus_width;
@@ -428,6 +444,30 @@ static inline int phy_notify_state(struct phy *phy, union phy_notify state)
return -ENOSYS;
}
+static inline int phy_register_notifier(struct phy *phy,
+ struct notifier_block *nb)
+{
+ if (!phy)
+ return 0;
+ return -ENOSYS;
+}
+
+static inline int phy_unregister_notifier(struct phy *phy,
+ struct notifier_block *nb)
+{
+ if (!phy)
+ return 0;
+ return -ENOSYS;
+}
+
+static inline int phy_notify_reset(struct phy *phy,
+ enum phy_notification event)
+{
+ if (!phy)
+ return 0;
+ return -ENOSYS;
+}
+
static inline int phy_configure(struct phy *phy,
union phy_configure_opts *opts)
{
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v11 29/36] phy: core: add notifier infrastructure
2026-07-09 19:02 ` [PATCH v11 29/36] phy: core: add notifier infrastructure Sebastian Reichel
@ 2026-07-09 19:31 ` sashiko-bot
0 siblings, 0 replies; 56+ messages in thread
From: sashiko-bot @ 2026-07-09 19:31 UTC (permalink / raw)
To: Sebastian Reichel
Cc: linux-phy, devicetree, robh, vkoul, neil.armstrong, olteanv,
conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Potential self-deadlock or AB-BA deadlock between `phy->mutex` and `phy->notifier.rwsem` when consumers handle reset notifications.
--
--- Patch [29]: [PATCH v11 29/36] phy: core: add notifier infrastructure ---
commit 205cc0767ab713005453a3401f261c9b0a0289ce
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: core: add notifier infrastructure
This commit introduces a new PHY notifier API to allow consumer drivers to
handle reset events coming from a PHY device. It adds functions to register
and unregister notifiers, and to trigger reset notifications.
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
[ ... ]
> +int phy_notify_reset(struct phy *phy, enum phy_notification event)
> +{
> + int ret;
> +
> + if (!phy)
> + return 0;
> +
> + ret = blocking_notifier_call_chain(&phy->notifier, event, phy);
[Severity: Medium]
Could this lead to a self-deadlock or an AB-BA deadlock between phy->mutex
and the notifier rwsem?
If a PHY provider calls phy_notify_reset() from within a PHY operation
callback that executes with phy->mutex held, the notifier chain takes a
read lock on phy->notifier.rwsem. If a consumer callback then attempts to
quiesce its hardware by calling a PHY framework API like phy_power_off(),
it will try to acquire phy->mutex again, resulting in a self-deadlock:
phy_set_mode()
mutex_lock(&phy->mutex);
provider_ops->set_mode()
phy_notify_reset()
blocking_notifier_call_chain() // takes rwsem read lock
consumer_callback()
phy_power_off()
mutex_lock(&phy->mutex); // self-deadlock
Conversely, if phy_notify_reset() is called outside of phy->mutex, but the
consumer calls back into the PHY framework, it establishes a lock dependency
from the notifier rwsem to phy->mutex. A concurrent PHY operation that holds
phy->mutex and registers a notifier could create a classic AB-BA deadlock if
a writer is pending on the rwsem.
Should the documentation warn consumers to defer PHY API calls to a
workqueue when handling these notifications, or should the lock hierarchy
be explicitly defined?
> + return notifier_to_errno(ret);
> +}
> +EXPORT_SYMBOL_GPL(phy_notify_reset);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=29
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v11 30/36] usb: dwc3: core: support PHY reset notifications
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (28 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 29/36] phy: core: add notifier infrastructure Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:02 ` [PATCH v11 31/36] phy: rockchip: usbdp: Add phy reset notification support Sebastian Reichel
` (5 subsequent siblings)
35 siblings, 0 replies; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
On recent Rockchip platforms (at least RK3588 & RK3576), DWC3 IP is used
with a USBDP PHY providing USB3 and DP. This PHY needs to be reset when
the mode changes, which may happen when plugging in different USB-C
devices.
If the USBDP PHY resets with the DWC3 IP running, its internal state
corrupts resulting in the USBDP PHY not being able to lock some PLL
clocks, which effectively renders USB3 unusable.
To fix the issue this adds handling for the new PHY framework reset
notifications, which will assert PHYSOFTRST before the actual PHY
is disabled and will deassert it once the PHY returns.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/usb/dwc3/core.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++
drivers/usb/dwc3/core.h | 18 +++++++++++
2 files changed, 98 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 517aa7f1486d..ae469183cf15 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -30,6 +30,7 @@
#include <linux/pinctrl/devinfo.h>
#include <linux/reset.h>
#include <linux/bitfield.h>
+#include <linux/phy/phy.h>
#include <linux/usb/ch9.h>
#include <linux/usb/gadget.h>
@@ -880,6 +881,82 @@ static int dwc3_phy_init(struct dwc3 *dwc)
return ret;
}
+static int dwc3_usb3_phy_notify(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct dwc3_phy_nb *pnb = container_of(nb, struct dwc3_phy_nb, nb);
+ struct dwc3 *dwc = pnb->dwc;
+ int port = pnb->port_index;
+ unsigned long flags;
+ u32 reg;
+ int ret;
+
+ switch (action) {
+ case PHY_NOTIFY_PRE_RESET:
+ ret = pm_runtime_resume_and_get(dwc->dev);
+ if (ret)
+ return notifier_from_errno(ret);
+
+ atomic_inc(&dwc->phy_reset_count);
+
+ /*
+ * Assert USB3 PHY soft reset within DWC3 before the external
+ * PHY resets. This disconnects the PIPE interface, preventing
+ * the DWC3 from interfering with PHY reinitialization and
+ * avoiding LCPLL lock failures.
+ */
+ spin_lock_irqsave(&dwc->lock, flags);
+ reg = dwc3_readl(dwc, DWC3_GUSB3PIPECTL(port));
+ reg |= DWC3_GUSB3PIPECTL_PHYSOFTRST;
+ dwc3_writel(dwc, DWC3_GUSB3PIPECTL(port), reg);
+ spin_unlock_irqrestore(&dwc->lock, flags);
+ break;
+
+ case PHY_NOTIFY_POST_RESET:
+ /*
+ * Deassert PHY soft reset to reconnect the PIPE interface
+ * after PHY reinitialization.
+ */
+ spin_lock_irqsave(&dwc->lock, flags);
+ reg = dwc3_readl(dwc, DWC3_GUSB3PIPECTL(port));
+ reg &= ~DWC3_GUSB3PIPECTL_PHYSOFTRST;
+ dwc3_writel(dwc, DWC3_GUSB3PIPECTL(port), reg);
+ spin_unlock_irqrestore(&dwc->lock, flags);
+
+ pm_runtime_put_autosuspend(dwc->dev);
+ atomic_dec(&dwc->phy_reset_count);
+ break;
+ }
+
+ return NOTIFY_OK;
+}
+
+static void dwc3_phy_register_notifiers(struct dwc3 *dwc)
+{
+ int i;
+
+ for (i = 0; i < dwc->num_usb3_ports; i++) {
+ dwc->usb3_phy_nb[i].nb.notifier_call = dwc3_usb3_phy_notify;
+ dwc->usb3_phy_nb[i].dwc = dwc;
+ dwc->usb3_phy_nb[i].port_index = i;
+ phy_register_notifier(dwc->usb3_generic_phy[i],
+ &dwc->usb3_phy_nb[i].nb);
+ }
+}
+
+static void dwc3_phy_unregister_notifiers(struct dwc3 *dwc)
+{
+ int i;
+
+ for (i = 0; i < dwc->num_usb3_ports; i++)
+ phy_unregister_notifier(dwc->usb3_generic_phy[i],
+ &dwc->usb3_phy_nb[i].nb);
+
+ for (i = atomic_read(&dwc->phy_reset_count); i > 0; i--)
+ pm_runtime_put_autosuspend(dwc->dev);
+ atomic_set(&dwc->phy_reset_count, 0);
+}
+
static void dwc3_phy_exit(struct dwc3 *dwc)
{
int i;
@@ -2341,6 +2418,7 @@ int dwc3_core_probe(const struct dwc3_probe_data *data)
dwc3_check_params(dwc);
dwc3_debugfs_init(dwc);
+ dwc3_phy_register_notifiers(dwc);
if (!data->skip_core_init_mode) {
ret = dwc3_core_init_mode(dwc);
@@ -2355,6 +2433,7 @@ int dwc3_core_probe(const struct dwc3_probe_data *data)
return 0;
err_exit_debugfs:
+ dwc3_phy_unregister_notifiers(dwc);
dwc3_debugfs_exit(dwc);
dwc3_event_buffers_cleanup(dwc);
dwc3_phy_power_off(dwc);
@@ -2412,6 +2491,7 @@ void dwc3_core_remove(struct dwc3 *dwc)
dwc3_core_exit_mode(dwc);
dwc3_debugfs_exit(dwc);
+ dwc3_phy_unregister_notifiers(dwc);
dwc3_core_exit(dwc);
dwc3_ulpi_exit(dwc);
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index e0dee9d28740..a7a2baf85015 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -11,6 +11,7 @@
#ifndef __DRIVERS_USB_DWC3_CORE_H
#define __DRIVERS_USB_DWC3_CORE_H
+#include <linux/atomic.h>
#include <linux/device.h>
#include <linux/spinlock.h>
#include <linux/mutex.h>
@@ -1002,6 +1003,20 @@ struct dwc3_glue_ops {
void (*pre_run_stop)(struct dwc3 *dwc, bool is_on);
};
+struct dwc3;
+
+/**
+ * struct dwc3_phy_nb - wrapper for PHY notifier block
+ * @nb: notifier block
+ * @dwc: back-pointer to the DWC3 controller
+ * @port_index: USB3 port index this notifier is registered for
+ */
+struct dwc3_phy_nb {
+ struct notifier_block nb;
+ struct dwc3 *dwc;
+ u8 port_index;
+};
+
/**
* struct dwc3 - representation of our controller
* @drd_work: workqueue used for role swapping
@@ -1065,6 +1080,7 @@ struct dwc3_glue_ops {
* @usb3_phy: pointer to USB3 PHY
* @usb2_generic_phy: pointer to array of USB2 PHYs
* @usb3_generic_phy: pointer to array of USB3 PHYs
+ * @usb3_phy_nb: notifier blocks for USB3 PHY reset events
* @num_usb2_ports: number of USB2 ports
* @num_usb3_ports: number of USB3 ports
* @phys_ready: flag to indicate that PHYs are ready
@@ -1229,6 +1245,8 @@ struct dwc3 {
struct phy *usb2_generic_phy[DWC3_USB2_MAX_PORTS];
struct phy *usb3_generic_phy[DWC3_USB3_MAX_PORTS];
+ struct dwc3_phy_nb usb3_phy_nb[DWC3_USB3_MAX_PORTS];
+ atomic_t phy_reset_count;
u8 num_usb2_ports;
u8 num_usb3_ports;
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* [PATCH v11 31/36] phy: rockchip: usbdp: Add phy reset notification support
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (29 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 30/36] usb: dwc3: core: support PHY reset notifications Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:32 ` sashiko-bot
2026-07-09 19:02 ` [PATCH v11 32/36] phy: rockchip: usbdp: Drop -EPROBE_DEFER hack Sebastian Reichel
` (4 subsequent siblings)
35 siblings, 1 reply; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
To resolve issues with running into permanent "cmn ana lcpll lock
timeout" errors after a few device replugs, add support for reset
notifications, which will be handled by the DWC3 driver to gracefully
handle the PHY being disabled. This avoids corrupting the controller's
internal state and the PIPE interface between the USB3 controller and
the PHY, thus fixing the issue.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 694688ed387f..5d151fff9eeb 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -1005,6 +1005,11 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
}
if (udphy->status == UDPHY_MODE_NONE) {
+ phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_PRE_RESET);
+
+ rk_udphy_u3_port_disable(udphy, true);
+ udelay(10);
+
ret = rk_udphy_setup(udphy);
if (ret)
return ret;
@@ -1012,13 +1017,22 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
if (!udphy->hs && udphy->mode & UDPHY_MODE_USB)
rk_udphy_u3_port_disable(udphy, false);
udphy->phy_needs_reinit = false;
+
+ phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_POST_RESET);
} else if (udphy->phy_needs_reinit) {
+ phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_PRE_RESET);
+
rk_udphy_u3_port_disable(udphy, true);
udelay(10);
ret = rk_udphy_init(udphy);
- if (ret)
+ if (ret) {
+ phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_POST_RESET);
return ret;
+ }
+
+ phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_POST_RESET);
+
udphy->phy_needs_reinit = false;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v11 31/36] phy: rockchip: usbdp: Add phy reset notification support
2026-07-09 19:02 ` [PATCH v11 31/36] phy: rockchip: usbdp: Add phy reset notification support Sebastian Reichel
@ 2026-07-09 19:32 ` sashiko-bot
0 siblings, 0 replies; 56+ messages in thread
From: sashiko-bot @ 2026-07-09 19:32 UTC (permalink / raw)
To: Sebastian Reichel
Cc: robh, conor+dt, vkoul, olteanv, devicetree, neil.armstrong,
linux-phy
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing PHY_NOTIFY_POST_RESET notification on the rk_udphy_setup() error path.
- [High] The return value of phy_notify_reset(..., PHY_NOTIFY_PRE_RESET) is completely ignored.
- [High] The USB3 port is permanently left disabled after a PHY re-initialization.
--
--- Patch [31]: [PATCH v11 31/36] phy: rockchip: usbdp: Add phy reset notification support ---
commit 8e70526891d152fa8ddbd23ac5f83716e6e5bc55
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Add phy reset notification support
This commit adds support for reset notifications to gracefully handle the
PHY being disabled. This avoids corrupting the controller's internal state
and fixes lock timeout errors after device replugs.
I noticed a few potential regressions related to error handling and hardware
enablement in this patch.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1005,6 +1005,11 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> }
>
> if (udphy->status == UDPHY_MODE_NONE) {
> + phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_PRE_RESET);
[Severity: High]
Should the return value of phy_notify_reset() be checked here? If the
consumer's pre-reset notifier returns an error code, proceeding with the
hardware reset could potentially trigger xHCI SErrors, and unconditionally
calling the post-reset notification later might unbalance the PM runtime
reference.
> +
> + rk_udphy_u3_port_disable(udphy, true);
> + udelay(10);
> +
> ret = rk_udphy_setup(udphy);
> if (ret)
> return ret;
[Severity: High]
Does this error path leak a PM runtime reference by leaving the DWC3
controller permanently in a pre-reset state? It appears the early return
skips balancing the prior PHY_NOTIFY_PRE_RESET notification with a
PHY_NOTIFY_POST_RESET call.
[ ... ]
> @@ -1012,13 +1017,22 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> if (!udphy->hs && udphy->mode & UDPHY_MODE_USB)
> rk_udphy_u3_port_disable(udphy, false);
> udphy->phy_needs_reinit = false;
> +
> + phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_POST_RESET);
> } else if (udphy->phy_needs_reinit) {
> + phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_PRE_RESET);
[Severity: High]
Similar to the check above, does this code need to handle errors returned
from phy_notify_reset() to avoid hardware issues or underflowing the
atomic reset count?
> +
> rk_udphy_u3_port_disable(udphy, true);
> udelay(10);
>
> ret = rk_udphy_init(udphy);
> - if (ret)
> + if (ret) {
> + phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_POST_RESET);
> return ret;
> + }
> +
> + phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_POST_RESET);
> +
> udphy->phy_needs_reinit = false;
> }
[Severity: High]
Is the USB3 port intentionally left disabled on this success path? Unlike
the cold power-on branch above it, there is no corresponding call to
rk_udphy_u3_port_disable(udphy, false) when rk_udphy_init() succeeds.
Could this permanently break USB3 connectivity after a PHY re-initialization
or mode change?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=31
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v11 32/36] phy: rockchip: usbdp: Drop -EPROBE_DEFER hack
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (30 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 31/36] phy: rockchip: usbdp: Add phy reset notification support Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:34 ` sashiko-bot
2026-07-09 19:02 ` [PATCH v11 33/36] phy: rockchip: usbdp: Rename mode to hw_mode Sebastian Reichel
` (3 subsequent siblings)
35 siblings, 1 reply; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
The hack to return -EPROBE_DEFER when the lcpll lock timeouts is no
longer needed. The driver now does a reset during its probe routine
marking everything as off and later on does a re-init, which avoids
the problem.
Apart from that rk_udphy_status_check() is called after the probe,
so it should not return -EPROBE_DEFER.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 5d151fff9eeb..3fb6eea2979b 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -750,17 +750,7 @@ static int rk_udphy_status_check(struct rk_udphy *udphy)
(val & CMN_ANA_LCPLL_LOCK_DONE), 200, 100000);
if (ret) {
dev_err(udphy->dev, "cmn ana lcpll lock timeout\n");
- /*
- * If earlier software (U-Boot) enabled USB once already
- * the PLL may have problems locking on the first try.
- * It will be successful on the second try, so for the
- * time being a -EPROBE_DEFER will solve the issue.
- *
- * This requires further investigation to understand the
- * root cause, especially considering that the driver is
- * asserting all reset lines at probe time.
- */
- return -EPROBE_DEFER;
+ return ret;
}
if (!udphy->flip) {
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v11 32/36] phy: rockchip: usbdp: Drop -EPROBE_DEFER hack
2026-07-09 19:02 ` [PATCH v11 32/36] phy: rockchip: usbdp: Drop -EPROBE_DEFER hack Sebastian Reichel
@ 2026-07-09 19:34 ` sashiko-bot
0 siblings, 0 replies; 56+ messages in thread
From: sashiko-bot @ 2026-07-09 19:34 UTC (permalink / raw)
To: Sebastian Reichel
Cc: devicetree, robh, neil.armstrong, conor+dt, vkoul, linux-phy,
olteanv
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing PHY_NOTIFY_POST_RESET notification on the rk_udphy_setup() error path permanently leaks the PM runtime usage counter and leaves the USB controller in soft reset.
--
--- Patch [32]: [PATCH v11 32/36] phy: rockchip: usbdp: Drop -EPROBE_DEFER hack ---
Note: The format of this review has been altered to a free-form summary due
to system recitation restrictions. Standard inline diff quoting has been
omitted.
commit 734b15714983185f791bb43091c9362d69abf8d8
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Drop -EPROBE_DEFER hack
This commit removes the -EPROBE_DEFER return from rk_udphy_status_check()
when the LCPLL lock times out. It notes that the driver now performs a reset
during probe and handles re-initialization later, making the deferral
unnecessary.
[Severity: High]
In drivers/phy/rockchip/phy-rockchip-usbdp.c, specifically within
rk_udphy_power_on():
When transitioning from UDPHY_MODE_NONE, the driver sends a
PHY_NOTIFY_PRE_RESET notification. Immediately following this, it calls
rk_udphy_setup(). If rk_udphy_setup() fails (for example, due to the LCPLL
lock timeout propagating up from rk_udphy_status_check or a clock enable
failure), the function returns the error directly.
Does this error path permanently leak the PM runtime usage counter and leave
the USB controller in a soft reset state?
Since PHY_NOTIFY_POST_RESET is never sent on this early return error path,
consumer drivers like DWC3 that asserted a soft reset and incremented their
PM runtime usage counter on PRE_RESET will never receive the matching
POST_RESET to recover.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=32
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v11 33/36] phy: rockchip: usbdp: Rename mode to hw_mode
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (31 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 32/36] phy: rockchip: usbdp: Drop -EPROBE_DEFER hack Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:35 ` sashiko-bot
2026-07-09 19:02 ` [PATCH v11 34/36] phy: rockchip: usbdp: Fix power state handling Sebastian Reichel
` (2 subsequent siblings)
35 siblings, 1 reply; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
Rename mode field to hw_mode to make clear that this is the modes
currently supported by the hardware, but not necessarily requested
by software. I.e. it is only set by either the USB-C state machine
or device-tree if the PHY is used in a fixed routing setup.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 46 +++++++++++++++----------------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 3fb6eea2979b..e2cd72643a7d 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -174,7 +174,7 @@ struct rk_udphy {
/* PHY status management */
bool flip;
bool phy_needs_reinit;
- u8 mode;
+ u8 hw_mode; /* modes currently supported by hardware */
u8 status;
/* utilized for USB */
@@ -579,18 +579,18 @@ static void rk_udphy_dp_lane_enable(struct rk_udphy *udphy, int dp_lanes)
CMN_DP_CMN_RSTN, FIELD_PREP(CMN_DP_CMN_RSTN, 0x0));
}
-static void rk_udphy_mode_set(struct rk_udphy *udphy, u8 mode)
+static void rk_udphy_mode_set(struct rk_udphy *udphy, u8 hw_mode)
{
- if (udphy->mode == mode)
+ if (udphy->hw_mode == hw_mode)
return;
udphy->phy_needs_reinit = true;
- udphy->mode = mode;
+ udphy->hw_mode = hw_mode;
}
static void rk_udphy_set_typec_state(struct rk_udphy *udphy, unsigned long state)
{
- u8 mode;
+ u8 hw_mode;
switch (state) {
case TYPEC_DP_STATE_C:
@@ -599,7 +599,7 @@ static void rk_udphy_set_typec_state(struct rk_udphy *udphy, unsigned long state
udphy->lane_mux_sel[1] = PHY_LANE_MUX_DP;
udphy->lane_mux_sel[2] = PHY_LANE_MUX_DP;
udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
- mode = UDPHY_MODE_DP;
+ hw_mode = UDPHY_MODE_DP;
udphy->dp_lanes = 4;
break;
@@ -616,12 +616,12 @@ static void rk_udphy_set_typec_state(struct rk_udphy *udphy, unsigned long state
udphy->lane_mux_sel[2] = PHY_LANE_MUX_DP;
udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
}
- mode = UDPHY_MODE_DP_USB;
+ hw_mode = UDPHY_MODE_DP_USB;
udphy->dp_lanes = 2;
break;
}
- rk_udphy_mode_set(udphy, mode);
+ rk_udphy_mode_set(udphy, hw_mode);
}
static void rk_udphy_set_typec_default_mapping(struct rk_udphy *udphy)
@@ -744,7 +744,7 @@ static int rk_udphy_status_check(struct rk_udphy *udphy)
int ret;
/* LCPLL check */
- if (udphy->mode & UDPHY_MODE_USB) {
+ if (udphy->hw_mode & UDPHY_MODE_USB) {
ret = regmap_read_poll_timeout(udphy->pma_regmap, CMN_ANA_LCPLL_DONE_OFFSET,
val, (val & CMN_ANA_LCPLL_AFC_DONE) &&
(val & CMN_ANA_LCPLL_LOCK_DONE), 200, 100000);
@@ -779,15 +779,15 @@ static int rk_udphy_init(struct rk_udphy *udphy)
int ret;
dev_dbg(udphy->dev, "reinit PHY with USB3=%s and DP=%s (%u lanes) flipped=%s\n",
- str_on_off(udphy->mode & UDPHY_MODE_USB),
- str_on_off(udphy->mode & UDPHY_MODE_DP),
+ str_on_off(udphy->hw_mode & UDPHY_MODE_USB),
+ str_on_off(udphy->hw_mode & UDPHY_MODE_DP),
udphy->dp_lanes, str_yes_no(udphy->flip));
rk_udphy_reset_assert_all(udphy);
usleep_range(10000, 11000);
/* enable rx lfps for usb */
- if (udphy->mode & UDPHY_MODE_USB)
+ if (udphy->hw_mode & UDPHY_MODE_USB)
rk_udphy_grfreg_write(udphy->udphygrf, &cfg->grfcfg.rx_lfps, true);
/* Step 1: power on pma and deassert apb rstn */
@@ -824,13 +824,13 @@ static int rk_udphy_init(struct rk_udphy *udphy)
FIELD_PREP(CMN_DP_LANE_EN_ALL, 0));
/* Step 4: deassert init rstn and wait for 200ns from datasheet */
- if (udphy->mode & UDPHY_MODE_USB) {
+ if (udphy->hw_mode & UDPHY_MODE_USB) {
ret = rk_udphy_reset_deassert(udphy, "init");
if (ret)
goto assert_resets;
}
- if (udphy->mode & UDPHY_MODE_DP) {
+ if (udphy->hw_mode & UDPHY_MODE_DP) {
regmap_update_bits(udphy->pma_regmap, CMN_DP_RSTN_OFFSET,
CMN_DP_INIT_RSTN,
FIELD_PREP(CMN_DP_INIT_RSTN, 0x1));
@@ -839,7 +839,7 @@ static int rk_udphy_init(struct rk_udphy *udphy)
udelay(1);
/* Step 5: deassert cmn/lane rstn */
- if (udphy->mode & UDPHY_MODE_USB) {
+ if (udphy->hw_mode & UDPHY_MODE_USB) {
ret = rk_udphy_reset_deassert(udphy, "cmn");
if (ret)
goto assert_resets;
@@ -898,7 +898,7 @@ static int rk_udphy_parse_lane_mux_data(struct rk_udphy *udphy)
num_lanes = device_property_count_u32(udphy->dev, "rockchip,dp-lane-mux");
if (num_lanes < 0) {
dev_dbg(udphy->dev, "no dp-lane-mux, following dp alt mode\n");
- udphy->mode = UDPHY_MODE_USB;
+ udphy->hw_mode = UDPHY_MODE_USB;
return 0;
}
@@ -927,10 +927,10 @@ static int rk_udphy_parse_lane_mux_data(struct rk_udphy *udphy)
}
}
- udphy->mode = UDPHY_MODE_DP;
+ udphy->hw_mode = UDPHY_MODE_DP;
udphy->dp_lanes = num_lanes;
if (num_lanes == 1 || num_lanes == 2) {
- udphy->mode |= UDPHY_MODE_USB;
+ udphy->hw_mode |= UDPHY_MODE_USB;
udphy->flip = (udphy->lane_mux_sel[0] == PHY_LANE_MUX_DP) ||
(udphy->lane_mux_sel[1] == PHY_LANE_MUX_DP);
}
@@ -989,7 +989,7 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
{
int ret;
- if (!(udphy->mode & mode)) {
+ if (!(udphy->hw_mode & mode)) {
dev_info(udphy->dev, "mode 0x%02x is not support\n", mode);
return 0;
}
@@ -1004,7 +1004,7 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
if (ret)
return ret;
- if (!udphy->hs && udphy->mode & UDPHY_MODE_USB)
+ if (!udphy->hs && udphy->hw_mode & UDPHY_MODE_USB)
rk_udphy_u3_port_disable(udphy, false);
udphy->phy_needs_reinit = false;
@@ -1033,7 +1033,7 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
static void rk_udphy_power_off(struct rk_udphy *udphy, u8 mode)
{
- if (!(udphy->mode & mode)) {
+ if (!(udphy->hw_mode & mode)) {
dev_info(udphy->dev, "mode 0x%02x is not support\n", mode);
return;
}
@@ -1292,7 +1292,7 @@ static int rk_udphy_usb3_phy_init(struct phy *phy)
guard(mutex)(&udphy->mutex);
/* DP only or high-speed, disable U3 port */
- if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs) {
+ if (!(udphy->hw_mode & UDPHY_MODE_USB) || udphy->hs) {
rk_udphy_u3_port_disable(udphy, true);
return 0;
}
@@ -1307,7 +1307,7 @@ static int rk_udphy_usb3_phy_exit(struct phy *phy)
guard(mutex)(&udphy->mutex);
/* DP only or high-speed */
- if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs) {
+ if (!(udphy->hw_mode & UDPHY_MODE_USB) || udphy->hs) {
udphy->status &= ~UDPHY_MODE_USB;
return 0;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v11 33/36] phy: rockchip: usbdp: Rename mode to hw_mode
2026-07-09 19:02 ` [PATCH v11 33/36] phy: rockchip: usbdp: Rename mode to hw_mode Sebastian Reichel
@ 2026-07-09 19:35 ` sashiko-bot
0 siblings, 0 replies; 56+ messages in thread
From: sashiko-bot @ 2026-07-09 19:35 UTC (permalink / raw)
To: Sebastian Reichel
Cc: conor+dt, vkoul, devicetree, linux-phy, neil.armstrong, olteanv,
robh
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] TYPEC_STATE_USB and TYPEC_STATE_SAFE mux events incorrectly configure the PHY for DisplayPort instead of USB-only.
- [High] rk_udphy_power_off and rk_udphy_usb3_phy_exit incorrectly use hw_mode to bypass power-off, permanently leaking the PHY state and preventing rk_udphy_disable() from being called.
--
--- Patch [33]: [PATCH v11 33/36] phy: rockchip: usbdp: Rename mode to hw_mode ---
commit 6f61047a0ab430913024fc15e4a26038f67189e4
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Rename mode to hw_mode
Rename mode field to hw_mode to make clear that this is the modes
currently supported by the hardware, but not necessarily requested
by software. It is only set by either the USB-C state machine
or device-tree if the PHY is used in a fixed routing setup.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 3fb6eea2979b9..e2cd72643a7d7 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -616,12 +616,12 @@ static void rk_udphy_set_typec_state(struct rk_udphy *udphy, unsigned long state
> udphy->lane_mux_sel[2] = PHY_LANE_MUX_DP;
> udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
> }
> - mode = UDPHY_MODE_DP_USB;
> + hw_mode = UDPHY_MODE_DP_USB;
[Severity: High]
This isn't a bug introduced by this patch, but since the switch statement lacks
explicit cases for TYPEC_STATE_USB and TYPEC_STATE_SAFE, do they fall through
to the default case here and incorrectly configure the PHY for DisplayPort
instead of USB-only?
If a standard USB device is connected or the port is in a safe/unattached state,
the Type-C subsystem fires a mux event with state->mode = TYPEC_STATE_USB (or
TYPEC_STATE_SAFE), and it seems they would erroneously be assigned
UDPHY_MODE_DP_USB with two DisplayPort lanes instead of just UDPHY_MODE_USB.
> udphy->dp_lanes = 2;
> break;
> }
[ ... ]
> @@ -1033,7 +1033,7 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
>
> static void rk_udphy_power_off(struct rk_udphy *udphy, u8 mode)
> {
> - if (!(udphy->mode & mode)) {
> + if (!(udphy->hw_mode & mode)) {
[Severity: High]
This is a pre-existing issue, but does using hw_mode here bypass the power-off
sequence and permanently leak the physical PHY state?
If a Type-C event changes the hardware mode to DP-only (hw_mode =
UDPHY_MODE_DP), the USB mode is dropped. If rk_udphy_power_off() is called
afterwards, this check will bypass physical shutdown since the requested mode
was already removed from hw_mode, preventing rk_udphy_disable() from being
called.
> dev_info(udphy->dev, "mode 0x%02x is not support\n", mode);
> return;
> }
[ ... ]
> @@ -1307,7 +1307,7 @@ static int rk_udphy_usb3_phy_exit(struct phy *phy)
> guard(mutex)(&udphy->mutex);
>
> /* DP only or high-speed */
> - if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs) {
> + if (!(udphy->hw_mode & UDPHY_MODE_USB) || udphy->hs) {
[Severity: High]
This is a pre-existing issue, but this appears to have the same flaw as
rk_udphy_power_off() above.
When the USB consumer subsequently calls phy_exit(), and hw_mode has already
dropped the USB mode, this returns early without delegating to
rk_udphy_power_off(). If the PHY status reached UDPHY_MODE_NONE,
rk_udphy_disable() is never called, and the PHY remains physically powered on
indefinitely.
> udphy->status &= ~UDPHY_MODE_USB;
> return 0;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=33
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v11 34/36] phy: rockchip: usbdp: Fix power state handling
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (32 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 33/36] phy: rockchip: usbdp: Rename mode to hw_mode Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:02 ` [PATCH v11 35/36] phy: rockchip: usbdp: Re-init PHY on mux change Sebastian Reichel
2026-07-09 19:02 ` [PATCH v11 36/36] phy: rockchip: usbdp: Add USB-C state without DP enabled Sebastian Reichel
35 siblings, 0 replies; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
Restructure power state handling by introducing sw_mode in addition
to the hw_mode field, so that the PHY knows about the currently
supported modes from the hardware perspective, the current modes
requested by software and the actual hardware status.
Now anything updating either the hardware or software state can simply
update the status field and call rk_udphy_update_power_state().
This makes it a lot more obvious what is going on and also fixes a few
potential resource leaks identified by Sashiko as a side-effect. For
example if USB3 is requested by software while the USB-C is in DP-only
mode, things are decently handled after this.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 144 ++++++++++++++++++------------
1 file changed, 87 insertions(+), 57 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index e2cd72643a7d..c5ad300620a6 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -173,9 +173,10 @@ struct rk_udphy {
/* PHY status management */
bool flip;
- bool phy_needs_reinit;
+ bool phy_needs_reinit; /* lane mux changed */
u8 hw_mode; /* modes currently supported by hardware */
- u8 status;
+ u8 sw_mode; /* modes currently requested */
+ u8 status; /* current PHY power state */
/* utilized for USB */
bool hs; /* flag for high-speed */
@@ -985,66 +986,86 @@ static int rk_udphy_parse_dt(struct rk_udphy *udphy)
return rk_udphy_reset_init(udphy, dev);
}
-static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
+static int rk_udphy_update_power_state(struct rk_udphy *udphy)
{
+ u8 target_mode;
int ret;
- if (!(udphy->hw_mode & mode)) {
- dev_info(udphy->dev, "mode 0x%02x is not support\n", mode);
+ /*
+ * Initialize PHY mode according to the hardware setup (either described
+ * in DT or negotiated via the Type-C controller) instead of requesting
+ * only the needed PHY side, because that would break the USB/DP data
+ * streams when the other PHY is being requested. This is not an issue
+ * during the Type-C negotiation as that happens during the hotplug phase
+ * and not during normal operation. Also disable everything if the
+ * software has not requested anything, as there shouldn't be any active
+ * data streams in that case.
+ */
+ target_mode = udphy->hw_mode;
+ if (udphy->sw_mode == UDPHY_MODE_NONE)
+ target_mode = UDPHY_MODE_NONE;
+
+ if (!udphy->phy_needs_reinit && udphy->status == target_mode)
return 0;
- }
- if (udphy->status == UDPHY_MODE_NONE) {
- phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_PRE_RESET);
+ /* Avoid to re-init disabled PHY */
+ if (udphy->status == target_mode && target_mode == UDPHY_MODE_NONE)
+ return 0;
- rk_udphy_u3_port_disable(udphy, true);
- udelay(10);
+ /*
+ * Inform DWC3 driver, that we are about to reset the PHY, so that it can
+ * assert its PIPE reset lines and avoid DWC3 getting into a buggy state.
+ * This is intentionally done for a PHY disable, since that also changes
+ * the clocks routed to the PHY.
+ */
+ ret = phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_PRE_RESET);
+ if (ret)
+ return ret;
+
+ /*
+ * Disable USB3 port, which among other things re-routes a DWC3 clock to
+ * avoid SErrors when the DWC3 registers are accessed while the PHY is
+ * disabled.
+ */
+ rk_udphy_u3_port_disable(udphy, true);
+ udelay(10);
+ if (udphy->status == UDPHY_MODE_NONE) {
+ /* Power up (incl. clocks) */
ret = rk_udphy_setup(udphy);
- if (ret)
+ if (ret) {
+ phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_POST_RESET);
return ret;
-
- if (!udphy->hs && udphy->hw_mode & UDPHY_MODE_USB)
- rk_udphy_u3_port_disable(udphy, false);
- udphy->phy_needs_reinit = false;
-
- phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_POST_RESET);
- } else if (udphy->phy_needs_reinit) {
- phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_PRE_RESET);
-
- rk_udphy_u3_port_disable(udphy, true);
- udelay(10);
-
+ }
+ } else if (target_mode == UDPHY_MODE_NONE) {
+ /* Power down (incl. clocks) */
+ rk_udphy_disable(udphy);
+ } else {
+ /* Mode change => re-init */
ret = rk_udphy_init(udphy);
if (ret) {
phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_POST_RESET);
return ret;
}
-
- phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_POST_RESET);
-
- udphy->phy_needs_reinit = false;
}
- udphy->status |= mode;
+ /* Ensure USB3 support is enabled when supported */
+ if (!udphy->hs && target_mode & UDPHY_MODE_USB)
+ rk_udphy_u3_port_disable(udphy, false);
- return 0;
-}
-
-static void rk_udphy_power_off(struct rk_udphy *udphy, u8 mode)
-{
- if (!(udphy->hw_mode & mode)) {
- dev_info(udphy->dev, "mode 0x%02x is not support\n", mode);
- return;
- }
-
- if (!udphy->status)
- return;
+ /*
+ * Inform DWC3, that we are done with the reset, so that it can deassert
+ * its PIPE reset line. This is sent in pair with a PRE_RESET allowing
+ * consumer driver to do paired resource requests (e.g. clocks) in their
+ * notification handlers. As we reroute the clocks, its also fine to
+ * send this after completely disabling the PHY.
+ */
+ phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_POST_RESET);
- udphy->status &= ~mode;
+ udphy->status = target_mode;
+ udphy->phy_needs_reinit = false;
- if (udphy->status == UDPHY_MODE_NONE)
- rk_udphy_disable(udphy);
+ return 0;
}
static int rk_udphy_dp_phy_power_on(struct phy *phy)
@@ -1053,11 +1074,15 @@ static int rk_udphy_dp_phy_power_on(struct phy *phy)
int ret;
scoped_guard(mutex, &udphy->mutex) {
+ udphy->sw_mode |= UDPHY_MODE_DP;
+
phy_set_bus_width(phy, udphy->dp_lanes);
- ret = rk_udphy_power_on(udphy, UDPHY_MODE_DP);
- if (ret)
+ ret = rk_udphy_update_power_state(udphy);
+ if (ret) {
+ udphy->sw_mode &= ~UDPHY_MODE_DP;
return ret;
+ }
rk_udphy_dp_lane_enable(udphy, udphy->dp_lanes);
@@ -1080,10 +1105,10 @@ static int rk_udphy_dp_phy_power_off(struct phy *phy)
guard(mutex)(&udphy->mutex);
- rk_udphy_dp_lane_enable(udphy, 0);
- rk_udphy_power_off(udphy, UDPHY_MODE_DP);
+ udphy->sw_mode &= ~UDPHY_MODE_DP;
- return 0;
+ rk_udphy_dp_lane_enable(udphy, 0);
+ return rk_udphy_update_power_state(udphy);
}
/*
@@ -1288,16 +1313,24 @@ static const struct phy_ops rk_udphy_dp_phy_ops = {
static int rk_udphy_usb3_phy_init(struct phy *phy)
{
struct rk_udphy *udphy = phy_get_drvdata(phy);
+ int ret;
guard(mutex)(&udphy->mutex);
- /* DP only or high-speed, disable U3 port */
- if (!(udphy->hw_mode & UDPHY_MODE_USB) || udphy->hs) {
+ if (udphy->hs) {
rk_udphy_u3_port_disable(udphy, true);
return 0;
}
- return rk_udphy_power_on(udphy, UDPHY_MODE_USB);
+ udphy->sw_mode |= UDPHY_MODE_USB;
+
+ ret = rk_udphy_update_power_state(udphy);
+ if (ret) {
+ udphy->sw_mode &= ~UDPHY_MODE_USB;
+ return ret;
+ }
+
+ return 0;
}
static int rk_udphy_usb3_phy_exit(struct phy *phy)
@@ -1306,15 +1339,12 @@ static int rk_udphy_usb3_phy_exit(struct phy *phy)
guard(mutex)(&udphy->mutex);
- /* DP only or high-speed */
- if (!(udphy->hw_mode & UDPHY_MODE_USB) || udphy->hs) {
- udphy->status &= ~UDPHY_MODE_USB;
+ if (udphy->hs)
return 0;
- }
- rk_udphy_power_off(udphy, UDPHY_MODE_USB);
+ udphy->sw_mode &= ~UDPHY_MODE_USB;
- return 0;
+ return rk_udphy_update_power_state(udphy);
}
static const struct phy_ops rk_udphy_usb3_phy_ops = {
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* [PATCH v11 35/36] phy: rockchip: usbdp: Re-init PHY on mux change
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (33 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 34/36] phy: rockchip: usbdp: Fix power state handling Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
2026-07-09 19:41 ` sashiko-bot
2026-07-09 19:02 ` [PATCH v11 36/36] phy: rockchip: usbdp: Add USB-C state without DP enabled Sebastian Reichel
35 siblings, 1 reply; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
Ensure that the right part of the PHY are powered up when the
mode changes. This ensures the PHY is re-initialized in the
following two scenarios, which are currently broken:
- cable orientation changes without DP being involved
- switching from DP-only into a mode with USB support
Fixes: 2f70bbddeb45 ("phy: rockchip: add usbdp combo phy driver")
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index c5ad300620a6..ecf0ed0139f6 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -1383,7 +1383,7 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
rk_udphy_set_typec_state(udphy, state->mode);
- return 0;
+ return rk_udphy_update_power_state(udphy);
}
static void rk_udphy_typec_mux_unregister(void *data)
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v11 35/36] phy: rockchip: usbdp: Re-init PHY on mux change
2026-07-09 19:02 ` [PATCH v11 35/36] phy: rockchip: usbdp: Re-init PHY on mux change Sebastian Reichel
@ 2026-07-09 19:41 ` sashiko-bot
0 siblings, 0 replies; 56+ messages in thread
From: sashiko-bot @ 2026-07-09 19:41 UTC (permalink / raw)
To: Sebastian Reichel
Cc: devicetree, neil.armstrong, conor+dt, olteanv, robh, vkoul,
linux-phy
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Deadlock / Recursive Lock
Pre-existing issues:
- [High] Silent DP PHY State Loss
--
--- Patch [35]: [PATCH v11 35/36] phy: rockchip: usbdp: Re-init PHY on mux change ---
commit 63e6102eaff345b4ebf6faefafd6aae88e7bb98e
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Re-init PHY on mux change
Ensure that the right part of the PHY are powered up when the mode changes.
This ensures the PHY is re-initialized when cable orientation changes without
DP being involved, and when switching from DP-only into a mode with USB
support.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index c5ad300620a65..ecf0ed0139f69 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1383,7 +1383,7 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
>
> rk_udphy_set_typec_state(udphy, state->mode);
>
> - return 0;
> + return rk_udphy_update_power_state(udphy);
[Severity: High]
Could this lead to a recursive locking deadlock if the DWC3 controller is
runtime-suspended?
Since rk_udphy_typec_mux_set() holds udphy->mutex using a guard, the
following chain seems to re-acquire the same mutex:
rk_udphy_typec_mux_set()
guard(mutex)(&udphy->mutex);
rk_udphy_update_power_state()
phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_PRE_RESET);
If the DWC3 driver handles this notification in device role:
dwc3_usb3_phy_notify()
pm_runtime_resume_and_get()
...
dwc3_resume_common()
dwc3_core_init_for_resume()
dwc3_phy_init()
phy_init()
The PHY init then attempts to acquire the lock again:
rk_udphy_usb3_phy_init()
guard(mutex)(&udphy->mutex);
Will this cause a system freeze when hotplugging or switching modes?
[Severity: High]
This isn't a bug introduced by this patch, but does invoking
rk_udphy_update_power_state() during a mux change cause the DP link to be
permanently lost?
When the power state updates, the PHY is re-initialized:
rk_udphy_update_power_state()
rk_udphy_init()
regmap_write(..., FIELD_PREP(CMN_DP_LANE_EN_ALL, 0));
This reset sequence clears the CMN_DP_LANE_EN_ALL bits and disables DP
transmission. Since rk_udphy_dp_lane_enable() does not appear to be called
afterward to restore the active DP configuration, does the DRM subsystem
lose the link without a full power cycle?
> }
>
> static void rk_udphy_typec_mux_unregister(void *data)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=35
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v11 36/36] phy: rockchip: usbdp: Add USB-C state without DP enabled
2026-07-09 19:02 [PATCH v11 00/36] phy: rockchip: usbdp: Clean up the mess Sebastian Reichel
` (34 preceding siblings ...)
2026-07-09 19:02 ` [PATCH v11 35/36] phy: rockchip: usbdp: Re-init PHY on mux change Sebastian Reichel
@ 2026-07-09 19:02 ` Sebastian Reichel
35 siblings, 0 replies; 56+ messages in thread
From: Sebastian Reichel @ 2026-07-09 19:02 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thinh Nguyen,
Greg Kroah-Hartman, Philipp Zabel
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, linux-usb, Sebastian Reichel
The driver currently only differs between 4 lanes DP mode or combined DP
+ USB3 mode. This makes sense from a lane routing point of view, as the
hardware only has 2 lanes of USB3.
But adding a separate state for USB-only helps with power management,
since we always power up all PHY parts according to the current hardware
setup to avoid data stream interruptions. Even if some lanes are muxed
to the DP controller there is no need to keep the DP side enabled if
something without DP AltMode is plugged into USB-C.
This potentially triggers some more USB reconnections during the PD
AltMode negotiation when switching from USB-only to combined USB+DP
mode. This should be fine, as the cable is freshly plugged at this
point.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 57 ++++++++++++++++++-------------
1 file changed, 33 insertions(+), 24 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index ecf0ed0139f6..bca096abb076 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -580,32 +580,14 @@ static void rk_udphy_dp_lane_enable(struct rk_udphy *udphy, int dp_lanes)
CMN_DP_CMN_RSTN, FIELD_PREP(CMN_DP_CMN_RSTN, 0x0));
}
-static void rk_udphy_mode_set(struct rk_udphy *udphy, u8 hw_mode)
+static void rk_udphy_set_lane_mux(struct rk_udphy *udphy)
{
- if (udphy->hw_mode == hw_mode)
- return;
-
- udphy->phy_needs_reinit = true;
- udphy->hw_mode = hw_mode;
-}
-
-static void rk_udphy_set_typec_state(struct rk_udphy *udphy, unsigned long state)
-{
- u8 hw_mode;
-
- switch (state) {
- case TYPEC_DP_STATE_C:
- case TYPEC_DP_STATE_E:
+ if (udphy->dp_lanes == 4) {
udphy->lane_mux_sel[0] = PHY_LANE_MUX_DP;
udphy->lane_mux_sel[1] = PHY_LANE_MUX_DP;
udphy->lane_mux_sel[2] = PHY_LANE_MUX_DP;
udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
- hw_mode = UDPHY_MODE_DP;
- udphy->dp_lanes = 4;
- break;
-
- case TYPEC_DP_STATE_D:
- default:
+ } else {
if (udphy->flip) {
udphy->lane_mux_sel[0] = PHY_LANE_MUX_DP;
udphy->lane_mux_sel[1] = PHY_LANE_MUX_DP;
@@ -617,12 +599,39 @@ static void rk_udphy_set_typec_state(struct rk_udphy *udphy, unsigned long state
udphy->lane_mux_sel[2] = PHY_LANE_MUX_DP;
udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
}
- hw_mode = UDPHY_MODE_DP_USB;
- udphy->dp_lanes = 2;
+ }
+}
+
+static void rk_udphy_mode_set(struct rk_udphy *udphy, u8 hw_mode, u8 dp_lanes)
+{
+ if (udphy->hw_mode == hw_mode && udphy->dp_lanes == dp_lanes)
+ return;
+
+ udphy->phy_needs_reinit = true;
+ udphy->hw_mode = hw_mode;
+ udphy->dp_lanes = dp_lanes;
+}
+
+static void rk_udphy_set_typec_state(struct rk_udphy *udphy, unsigned long state)
+{
+ switch (state) {
+ case TYPEC_DP_STATE_C:
+ case TYPEC_DP_STATE_E:
+ rk_udphy_mode_set(udphy, UDPHY_MODE_DP, 4);
+ break;
+
+ case TYPEC_DP_STATE_D:
+ rk_udphy_mode_set(udphy, UDPHY_MODE_DP_USB, 2);
+ break;
+
+ case TYPEC_STATE_SAFE:
+ case TYPEC_STATE_USB:
+ default:
+ rk_udphy_mode_set(udphy, UDPHY_MODE_USB, 0);
break;
}
- rk_udphy_mode_set(udphy, hw_mode);
+ rk_udphy_set_lane_mux(udphy);
}
static void rk_udphy_set_typec_default_mapping(struct rk_udphy *udphy)
--
2.53.0
^ permalink raw reply related [flat|nested] 56+ messages in thread