* [PATCH v17 0/7] usb: dwc3: introduce Rockchip glue driver
@ 2026-10-16 18:43 ` Sebastian Reichel
0 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, Sebastian Reichel,
Krishna Kurapati
The recent Rockchip platform have a combined PHY, which does USB3 and
DisplayPort as well as muxing the lines in arbitrary order. Changing its
enabled functionality requires a PHY reset and as the PHY provides a
clock to the DWC3 controller, this can result in an SError. Apart from
that the PHY might not start up correctly when the DWC3 PIPE interface
is running. This series prepares a Rockchip specific DWC3 glue driver to
help out with these problems. To become useful more patches are needed
on the Rockchip USBDP PHY side, which are send separately.
This was split from the USBDP cleanup series. v17 has a dependency on
the patches that have been merged to phy's for-next branch. There are
some additional Rockchip USBDP phy fixes, which depend on this series
(which is effectively also a fix).
Changes in v17:
- Link to v16: https://patch.msgid.link/20260924-b4-rockchip-dwc3-rockchip-glue-v16-0-126a2e9133c3@collabora.com
- Add new patch adding just sw_mode, which was part of the "phy:
rockchip: usbdp: Fix power state handling" patch in USBDP cleanup
part 3 series; sw_mode will be used by the dwc3/usbdp reset quirk now
- drop generic PHY reset notifier API; I discussed this series at Linux
Plumbers Conference with Vinod Koul and he prefers a local driver
hack/quirk instead
- rework USBDP and DWC3 rockchip glue for the local driver quirk
using direct function calls
- Ensure that the reset handler in DWC3 is only called when the PHY is
initialized (and thus DWC3 is running)
- Also set vbus in the last patch when dwc3 is configured to device only
mode (in contrast to dual-role mode being in device mode).
Changes in v16:
- Link to v15 (sorry, I missed force setting the revision for this
part in the series split, so the series says v1 in the patch subject):
https://patch.msgid.link/20260915-b4-rockchip-dwc3-rockchip-glue-v1-0-763bb546824e@collabora.com
- Update first patch to also build the PM functions even when
PM is disabled
- Add new patch for defining an empty usb_udc_vbus_handler stub
when USB_GADGET is disabled
- Add dependency to Rockchip DWC3 glue, which disallows the
configuration with USB_DWC3_ROCKCHIP=y and USB_GADGET=m
- Collect tags
Changes in v15:
- Link to v14: https://patch.msgid.link/20260813-rockchip-usbdp-cleanup-v14-0-b5ad9c68fa11@collabora.com
- Avoid #ifdef for CONFIG_PM in the new glue driver (Thinh Nguyen)
- Do not add #if IS_ENABLED around the DT compatible blacklist in the
generic driver, using the glue driver should be mandatory as the
generic driver results in arbitrary system hangs. Thinh Nguyen suggested
updating Kconfig for this, but did not clarify what that means.
Current Kconfig works the same way as the other mandatory glue
drivers.
- Split series to simplify review
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
Sebastian Reichel (7):
phy: rockchip: usbdp: Track PHY power state
phy: rockchip: usbdp: Add reset hook support for dwc3
usb: dwc3: rockchip: introduce glue driver
usb: dwc3: core: add post PHY registration hook for platform glue
usb: dwc3: rockchip: add USBDP PHY reset quirk
usb: gadget: define stub for usb_udc_vbus_handler
usb: dwc3: rockchip: fix USB-C reconnect in gadget mode
drivers/phy/rockchip/phy-rockchip-usbdp.c | 79 ++++++++-
drivers/usb/dwc3/Kconfig | 13 ++
drivers/usb/dwc3/Makefile | 1 +
drivers/usb/dwc3/core.c | 17 +-
drivers/usb/dwc3/core.h | 9 +
drivers/usb/dwc3/dwc3-rockchip.c | 263 ++++++++++++++++++++++++++++++
drivers/usb/dwc3/trace.c | 3 +
include/linux/phy/rockchip-usbdp.h | 16 ++
include/linux/usb/gadget.h | 5 +
9 files changed, 401 insertions(+), 5 deletions(-)
---
base-commit: 8f56fdf3d8cb9f1dbdc93bf9ac816698aa6f70b7
change-id: 20260914-b4-rockchip-dwc3-rockchip-glue-979ea3735d43
Best regards,
--
Sebastian Reichel <sebastian.reichel@collabora.com>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH v17 0/7] usb: dwc3: introduce Rockchip glue driver
@ 2026-10-16 18:43 ` Sebastian Reichel
0 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, Sebastian Reichel,
Krishna Kurapati
The recent Rockchip platform have a combined PHY, which does USB3 and
DisplayPort as well as muxing the lines in arbitrary order. Changing its
enabled functionality requires a PHY reset and as the PHY provides a
clock to the DWC3 controller, this can result in an SError. Apart from
that the PHY might not start up correctly when the DWC3 PIPE interface
is running. This series prepares a Rockchip specific DWC3 glue driver to
help out with these problems. To become useful more patches are needed
on the Rockchip USBDP PHY side, which are send separately.
This was split from the USBDP cleanup series. v17 has a dependency on
the patches that have been merged to phy's for-next branch. There are
some additional Rockchip USBDP phy fixes, which depend on this series
(which is effectively also a fix).
Changes in v17:
- Link to v16: https://patch.msgid.link/20260924-b4-rockchip-dwc3-rockchip-glue-v16-0-126a2e9133c3@collabora.com
- Add new patch adding just sw_mode, which was part of the "phy:
rockchip: usbdp: Fix power state handling" patch in USBDP cleanup
part 3 series; sw_mode will be used by the dwc3/usbdp reset quirk now
- drop generic PHY reset notifier API; I discussed this series at Linux
Plumbers Conference with Vinod Koul and he prefers a local driver
hack/quirk instead
- rework USBDP and DWC3 rockchip glue for the local driver quirk
using direct function calls
- Ensure that the reset handler in DWC3 is only called when the PHY is
initialized (and thus DWC3 is running)
- Also set vbus in the last patch when dwc3 is configured to device only
mode (in contrast to dual-role mode being in device mode).
Changes in v16:
- Link to v15 (sorry, I missed force setting the revision for this
part in the series split, so the series says v1 in the patch subject):
https://patch.msgid.link/20260915-b4-rockchip-dwc3-rockchip-glue-v1-0-763bb546824e@collabora.com
- Update first patch to also build the PM functions even when
PM is disabled
- Add new patch for defining an empty usb_udc_vbus_handler stub
when USB_GADGET is disabled
- Add dependency to Rockchip DWC3 glue, which disallows the
configuration with USB_DWC3_ROCKCHIP=y and USB_GADGET=m
- Collect tags
Changes in v15:
- Link to v14: https://patch.msgid.link/20260813-rockchip-usbdp-cleanup-v14-0-b5ad9c68fa11@collabora.com
- Avoid #ifdef for CONFIG_PM in the new glue driver (Thinh Nguyen)
- Do not add #if IS_ENABLED around the DT compatible blacklist in the
generic driver, using the glue driver should be mandatory as the
generic driver results in arbitrary system hangs. Thinh Nguyen suggested
updating Kconfig for this, but did not clarify what that means.
Current Kconfig works the same way as the other mandatory glue
drivers.
- Split series to simplify review
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
Sebastian Reichel (7):
phy: rockchip: usbdp: Track PHY power state
phy: rockchip: usbdp: Add reset hook support for dwc3
usb: dwc3: rockchip: introduce glue driver
usb: dwc3: core: add post PHY registration hook for platform glue
usb: dwc3: rockchip: add USBDP PHY reset quirk
usb: gadget: define stub for usb_udc_vbus_handler
usb: dwc3: rockchip: fix USB-C reconnect in gadget mode
drivers/phy/rockchip/phy-rockchip-usbdp.c | 79 ++++++++-
drivers/usb/dwc3/Kconfig | 13 ++
drivers/usb/dwc3/Makefile | 1 +
drivers/usb/dwc3/core.c | 17 +-
drivers/usb/dwc3/core.h | 9 +
drivers/usb/dwc3/dwc3-rockchip.c | 263 ++++++++++++++++++++++++++++++
drivers/usb/dwc3/trace.c | 3 +
include/linux/phy/rockchip-usbdp.h | 16 ++
include/linux/usb/gadget.h | 5 +
9 files changed, 401 insertions(+), 5 deletions(-)
---
base-commit: 8f56fdf3d8cb9f1dbdc93bf9ac816698aa6f70b7
change-id: 20260914-b4-rockchip-dwc3-rockchip-glue-979ea3735d43
Best regards,
--
Sebastian Reichel <sebastian.reichel@collabora.com>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH v17 1/7] phy: rockchip: usbdp: Track PHY power state
2026-10-16 18:43 ` Sebastian Reichel
(?)
@ 2026-10-16 18:43 ` Sebastian Reichel
-1 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, Sebastian Reichel
Track PHY state requested by software for DP and USB3 individually
in preparation to rework and fix up power state handling.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index e748acdd2d31..db69dd24f73b 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -175,6 +175,7 @@ struct rk_udphy {
bool phy_needs_reinit;
u8 mode;
u8 status;
+ u8 sw_mode; /* modes currently requested */
/* utilized for USB */
bool hs; /* flag for high-speed */
@@ -1051,11 +1052,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)
+ if (ret) {
+ udphy->sw_mode &= ~UDPHY_MODE_DP;
return ret;
+ }
rk_udphy_dp_lane_enable(udphy, udphy->dp_lanes);
@@ -1078,6 +1083,8 @@ static int rk_udphy_dp_phy_power_off(struct phy *phy)
guard(mutex)(&udphy->mutex);
+ udphy->sw_mode &= ~UDPHY_MODE_DP;
+
rk_udphy_dp_lane_enable(udphy, 0);
rk_udphy_power_off(udphy, UDPHY_MODE_DP);
@@ -1286,16 +1293,25 @@ 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);
+ udphy->sw_mode |= UDPHY_MODE_USB;
+
/* DP only or high-speed, disable U3 port */
if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs) {
rk_udphy_u3_port_disable(udphy, true);
return 0;
}
- return rk_udphy_power_on(udphy, UDPHY_MODE_USB);
+ ret = rk_udphy_power_on(udphy, UDPHY_MODE_USB);
+ if (ret) {
+ udphy->sw_mode &= ~UDPHY_MODE_USB;
+ return ret;
+ }
+
+ return 0;
}
static int rk_udphy_usb3_phy_exit(struct phy *phy)
@@ -1308,6 +1324,8 @@ static int rk_udphy_usb3_phy_exit(struct phy *phy)
if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
return 0;
+ udphy->sw_mode &= ~UDPHY_MODE_USB;
+
rk_udphy_power_off(udphy, UDPHY_MODE_USB);
return 0;
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v17 1/7] phy: rockchip: usbdp: Track PHY power state
@ 2026-10-16 18:43 ` Sebastian Reichel
0 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, Sebastian Reichel
Track PHY state requested by software for DP and USB3 individually
in preparation to rework and fix up power state handling.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index e748acdd2d31..db69dd24f73b 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -175,6 +175,7 @@ struct rk_udphy {
bool phy_needs_reinit;
u8 mode;
u8 status;
+ u8 sw_mode; /* modes currently requested */
/* utilized for USB */
bool hs; /* flag for high-speed */
@@ -1051,11 +1052,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)
+ if (ret) {
+ udphy->sw_mode &= ~UDPHY_MODE_DP;
return ret;
+ }
rk_udphy_dp_lane_enable(udphy, udphy->dp_lanes);
@@ -1078,6 +1083,8 @@ static int rk_udphy_dp_phy_power_off(struct phy *phy)
guard(mutex)(&udphy->mutex);
+ udphy->sw_mode &= ~UDPHY_MODE_DP;
+
rk_udphy_dp_lane_enable(udphy, 0);
rk_udphy_power_off(udphy, UDPHY_MODE_DP);
@@ -1286,16 +1293,25 @@ 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);
+ udphy->sw_mode |= UDPHY_MODE_USB;
+
/* DP only or high-speed, disable U3 port */
if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs) {
rk_udphy_u3_port_disable(udphy, true);
return 0;
}
- return rk_udphy_power_on(udphy, UDPHY_MODE_USB);
+ ret = rk_udphy_power_on(udphy, UDPHY_MODE_USB);
+ if (ret) {
+ udphy->sw_mode &= ~UDPHY_MODE_USB;
+ return ret;
+ }
+
+ return 0;
}
static int rk_udphy_usb3_phy_exit(struct phy *phy)
@@ -1308,6 +1324,8 @@ static int rk_udphy_usb3_phy_exit(struct phy *phy)
if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
return 0;
+ udphy->sw_mode &= ~UDPHY_MODE_USB;
+
rk_udphy_power_off(udphy, UDPHY_MODE_USB);
return 0;
--
2.53.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v17 1/7] phy: rockchip: usbdp: Track PHY power state
@ 2026-10-16 18:43 ` Sebastian Reichel
0 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, Sebastian Reichel
Track PHY state requested by software for DP and USB3 individually
in preparation to rework and fix up power state handling.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index e748acdd2d31..db69dd24f73b 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -175,6 +175,7 @@ struct rk_udphy {
bool phy_needs_reinit;
u8 mode;
u8 status;
+ u8 sw_mode; /* modes currently requested */
/* utilized for USB */
bool hs; /* flag for high-speed */
@@ -1051,11 +1052,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)
+ if (ret) {
+ udphy->sw_mode &= ~UDPHY_MODE_DP;
return ret;
+ }
rk_udphy_dp_lane_enable(udphy, udphy->dp_lanes);
@@ -1078,6 +1083,8 @@ static int rk_udphy_dp_phy_power_off(struct phy *phy)
guard(mutex)(&udphy->mutex);
+ udphy->sw_mode &= ~UDPHY_MODE_DP;
+
rk_udphy_dp_lane_enable(udphy, 0);
rk_udphy_power_off(udphy, UDPHY_MODE_DP);
@@ -1286,16 +1293,25 @@ 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);
+ udphy->sw_mode |= UDPHY_MODE_USB;
+
/* DP only or high-speed, disable U3 port */
if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs) {
rk_udphy_u3_port_disable(udphy, true);
return 0;
}
- return rk_udphy_power_on(udphy, UDPHY_MODE_USB);
+ ret = rk_udphy_power_on(udphy, UDPHY_MODE_USB);
+ if (ret) {
+ udphy->sw_mode &= ~UDPHY_MODE_USB;
+ return ret;
+ }
+
+ return 0;
}
static int rk_udphy_usb3_phy_exit(struct phy *phy)
@@ -1308,6 +1324,8 @@ static int rk_udphy_usb3_phy_exit(struct phy *phy)
if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
return 0;
+ udphy->sw_mode &= ~UDPHY_MODE_USB;
+
rk_udphy_power_off(udphy, UDPHY_MODE_USB);
return 0;
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v17 1/7] phy: rockchip: usbdp: Track PHY power state
2026-10-16 18:43 ` Sebastian Reichel
@ 2026-10-16 18:50 ` sashiko-bot
-1 siblings, 0 replies; 35+ messages in thread
From: sashiko-bot @ 2026-10-16 18:50 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-usb, linux-phy, vkoul, neil.armstrong, olteanv
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] phy: state tracking leak in rk_udphy_usb3_phy_exit()
--
commit 71f779c7a4a8cedbd725b12e66fcbf3d92f884bf
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Track PHY power state
Track PHY state requested by software for DP and USB3 individually
in preparation to rework and fix up power state handling.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index e748acdd2d313..db69dd24f73b3 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -1286,16 +1293,25 @@ 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);
>
> + udphy->sw_mode |= UDPHY_MODE_USB;
> +
> /* DP only or high-speed, disable U3 port */
> if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs) {
> rk_udphy_u3_port_disable(udphy, true);
> return 0;
> }
>
> - return rk_udphy_power_on(udphy, UDPHY_MODE_USB);
> + ret = rk_udphy_power_on(udphy, UDPHY_MODE_USB);
> + if (ret) {
> + udphy->sw_mode &= ~UDPHY_MODE_USB;
> + return ret;
> + }
> +
> + return 0;
> }
>
> static int rk_udphy_usb3_phy_exit(struct phy *phy)
> @@ -1308,6 +1324,8 @@ static int rk_udphy_usb3_phy_exit(struct phy *phy)
> if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
> return 0;
>
> + udphy->sw_mode &= ~UDPHY_MODE_USB;
> +
[Severity: Medium]
In rk_udphy_usb3_phy_init(), UDPHY_MODE_USB is unconditionally added to
sw_mode before the early return for DP-only or high-speed cases.
However, in rk_udphy_usb3_phy_exit(), the corresponding early return happens
before clearing UDPHY_MODE_USB from sw_mode.
Will this cause the sw_mode state to leak? If so, could subsequent operations
like rk_udphy_power_on() called during DP PHY initialization incorrectly see
UDPHY_MODE_USB as requested, potentially executing the DWC3 reset handler
unnecessarily?
> rk_udphy_power_off(udphy, UDPHY_MODE_USB);
>
> return 0;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-b4-rockchip-dwc3-rockchip-glue-v17-0-876f96893e2f@collabora.com?part=1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v17 1/7] phy: rockchip: usbdp: Track PHY power state
@ 2026-10-16 18:50 ` sashiko-bot
0 siblings, 0 replies; 35+ messages in thread
From: sashiko-bot @ 2026-10-16 18:50 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-usb, linux-phy, vkoul, neil.armstrong, olteanv
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] phy: state tracking leak in rk_udphy_usb3_phy_exit()
--
commit 71f779c7a4a8cedbd725b12e66fcbf3d92f884bf
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Track PHY power state
Track PHY state requested by software for DP and USB3 individually
in preparation to rework and fix up power state handling.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index e748acdd2d313..db69dd24f73b3 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -1286,16 +1293,25 @@ 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);
>
> + udphy->sw_mode |= UDPHY_MODE_USB;
> +
> /* DP only or high-speed, disable U3 port */
> if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs) {
> rk_udphy_u3_port_disable(udphy, true);
> return 0;
> }
>
> - return rk_udphy_power_on(udphy, UDPHY_MODE_USB);
> + ret = rk_udphy_power_on(udphy, UDPHY_MODE_USB);
> + if (ret) {
> + udphy->sw_mode &= ~UDPHY_MODE_USB;
> + return ret;
> + }
> +
> + return 0;
> }
>
> static int rk_udphy_usb3_phy_exit(struct phy *phy)
> @@ -1308,6 +1324,8 @@ static int rk_udphy_usb3_phy_exit(struct phy *phy)
> if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
> return 0;
>
> + udphy->sw_mode &= ~UDPHY_MODE_USB;
> +
[Severity: Medium]
In rk_udphy_usb3_phy_init(), UDPHY_MODE_USB is unconditionally added to
sw_mode before the early return for DP-only or high-speed cases.
However, in rk_udphy_usb3_phy_exit(), the corresponding early return happens
before clearing UDPHY_MODE_USB from sw_mode.
Will this cause the sw_mode state to leak? If so, could subsequent operations
like rk_udphy_power_on() called during DP PHY initialization incorrectly see
UDPHY_MODE_USB as requested, potentially executing the DWC3 reset handler
unnecessarily?
> rk_udphy_power_off(udphy, UDPHY_MODE_USB);
>
> return 0;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-b4-rockchip-dwc3-rockchip-glue-v17-0-876f96893e2f@collabora.com?part=1
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v17 2/7] phy: rockchip: usbdp: Add reset hook support for dwc3
2026-10-16 18:43 ` Sebastian Reichel
(?)
@ 2026-10-16 18:43 ` Sebastian Reichel
-1 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, Sebastian Reichel
When TCPM (Type-C Port Manager) triggers a orientation or mux
change, the PHY needs to reset. This results in the PHY no longer
providing a clock to the USB controller. If the USB controller
registers are accessed without the clock, SErrors are generated.
This can be avoided by switching the clock provider for the USB
controller. But after switching the clock, the USB PHY sometimes does
not succeed to lock its "cmn ana lcpll" PLLs again. I suspect this is
due to a clock glitch and after some debugging figured out that this
can be avoided by disabling the USB controller's PIPE interface while
the PHY reset is ongoing.
Implement an API that allows the USBDP to actively request a reset
from DWC3 as a quirk.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 57 +++++++++++++++++++++++++++++--
include/linux/phy/rockchip-usbdp.h | 16 +++++++++
2 files changed, 71 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index db69dd24f73b..b396f286e785 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -19,6 +19,7 @@
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/phy/phy.h>
+#include <linux/phy/rockchip-usbdp.h>
#include <linux/platform_device.h>
#include <linux/property.h>
#include <linux/regmap.h>
@@ -179,6 +180,9 @@ struct rk_udphy {
/* utilized for USB */
bool hs; /* flag for high-speed */
+ int (*dwc3_reset_handler)(void *data, int port, bool pre);
+ void *dwc3_reset_data;
+ int dwc3_reset_port;
/* utilized for DP */
struct gpio_desc *sbu1_dc_gpio;
@@ -995,6 +999,43 @@ static int rk_udphy_parse_dt(struct rk_udphy *udphy)
return rk_udphy_reset_init(udphy, dev);
}
+static int rk_udphy_run_reset_handler(struct rk_udphy *udphy, bool pre)
+{
+ if (!udphy->dwc3_reset_handler)
+ return 0;
+
+ if (!(udphy->sw_mode & UDPHY_MODE_USB))
+ return 0;
+
+ return udphy->dwc3_reset_handler(udphy->dwc3_reset_data, udphy->dwc3_reset_port, pre);
+}
+
+void rk_udphy_unregister_reset_handler(struct phy *phy)
+{
+ struct rk_udphy *udphy = phy_get_drvdata(phy);
+
+ guard(mutex)(&udphy->mutex);
+
+ udphy->dwc3_reset_handler = NULL;
+ udphy->dwc3_reset_data = NULL;
+ udphy->dwc3_reset_port = 0;
+}
+EXPORT_SYMBOL(rk_udphy_unregister_reset_handler);
+
+void rk_udphy_register_reset_handler(struct phy *phy,
+ int (*reset_handler)(void *data, int port, bool pre),
+ void *data, int port)
+{
+ struct rk_udphy *udphy = phy_get_drvdata(phy);
+
+ guard(mutex)(&udphy->mutex);
+
+ udphy->dwc3_reset_handler = reset_handler;
+ udphy->dwc3_reset_data = data;
+ udphy->dwc3_reset_port = port;
+}
+EXPORT_SYMBOL(rk_udphy_register_reset_handler);
+
static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
{
int ret;
@@ -1005,24 +1046,36 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
}
if (udphy->status == UDPHY_MODE_NONE) {
+ rk_udphy_run_reset_handler(udphy, true);
+
ret = rk_udphy_setup(udphy);
- if (ret)
+ if (ret) {
+ rk_udphy_run_reset_handler(udphy, false);
return ret;
+ }
if (!udphy->hs && udphy->mode & UDPHY_MODE_USB)
rk_udphy_u3_port_disable(udphy, false);
udphy->phy_needs_reinit = false;
+
+ rk_udphy_run_reset_handler(udphy, false);
} else if (udphy->phy_needs_reinit) {
+ rk_udphy_run_reset_handler(udphy, true);
+
rk_udphy_u3_port_disable(udphy, true);
udelay(10);
ret = rk_udphy_init(udphy);
- if (ret)
+ if (ret) {
+ rk_udphy_run_reset_handler(udphy, false);
return ret;
+ }
if (!udphy->hs && udphy->mode & UDPHY_MODE_USB)
rk_udphy_u3_port_disable(udphy, false);
udphy->phy_needs_reinit = false;
+
+ rk_udphy_run_reset_handler(udphy, false);
}
udphy->status |= mode;
diff --git a/include/linux/phy/rockchip-usbdp.h b/include/linux/phy/rockchip-usbdp.h
new file mode 100644
index 000000000000..6923ac28303b
--- /dev/null
+++ b/include/linux/phy/rockchip-usbdp.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Rockchip USBDP Combo PHY with Samsung IP block driver
+ *
+ * Copyright (C) 2026 Collabora Ltd
+ */
+
+#ifndef __ROCKCHIP_USBDP
+#define __ROCKCHIP_USBDP
+
+void rk_udphy_unregister_reset_handler(struct phy *phy);
+void rk_udphy_register_reset_handler(struct phy *phy,
+ int (*reset_handler)(void *data, int port, bool pre),
+ void *data, int port);
+
+#endif
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v17 2/7] phy: rockchip: usbdp: Add reset hook support for dwc3
@ 2026-10-16 18:43 ` Sebastian Reichel
0 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, Sebastian Reichel
When TCPM (Type-C Port Manager) triggers a orientation or mux
change, the PHY needs to reset. This results in the PHY no longer
providing a clock to the USB controller. If the USB controller
registers are accessed without the clock, SErrors are generated.
This can be avoided by switching the clock provider for the USB
controller. But after switching the clock, the USB PHY sometimes does
not succeed to lock its "cmn ana lcpll" PLLs again. I suspect this is
due to a clock glitch and after some debugging figured out that this
can be avoided by disabling the USB controller's PIPE interface while
the PHY reset is ongoing.
Implement an API that allows the USBDP to actively request a reset
from DWC3 as a quirk.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 57 +++++++++++++++++++++++++++++--
include/linux/phy/rockchip-usbdp.h | 16 +++++++++
2 files changed, 71 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index db69dd24f73b..b396f286e785 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -19,6 +19,7 @@
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/phy/phy.h>
+#include <linux/phy/rockchip-usbdp.h>
#include <linux/platform_device.h>
#include <linux/property.h>
#include <linux/regmap.h>
@@ -179,6 +180,9 @@ struct rk_udphy {
/* utilized for USB */
bool hs; /* flag for high-speed */
+ int (*dwc3_reset_handler)(void *data, int port, bool pre);
+ void *dwc3_reset_data;
+ int dwc3_reset_port;
/* utilized for DP */
struct gpio_desc *sbu1_dc_gpio;
@@ -995,6 +999,43 @@ static int rk_udphy_parse_dt(struct rk_udphy *udphy)
return rk_udphy_reset_init(udphy, dev);
}
+static int rk_udphy_run_reset_handler(struct rk_udphy *udphy, bool pre)
+{
+ if (!udphy->dwc3_reset_handler)
+ return 0;
+
+ if (!(udphy->sw_mode & UDPHY_MODE_USB))
+ return 0;
+
+ return udphy->dwc3_reset_handler(udphy->dwc3_reset_data, udphy->dwc3_reset_port, pre);
+}
+
+void rk_udphy_unregister_reset_handler(struct phy *phy)
+{
+ struct rk_udphy *udphy = phy_get_drvdata(phy);
+
+ guard(mutex)(&udphy->mutex);
+
+ udphy->dwc3_reset_handler = NULL;
+ udphy->dwc3_reset_data = NULL;
+ udphy->dwc3_reset_port = 0;
+}
+EXPORT_SYMBOL(rk_udphy_unregister_reset_handler);
+
+void rk_udphy_register_reset_handler(struct phy *phy,
+ int (*reset_handler)(void *data, int port, bool pre),
+ void *data, int port)
+{
+ struct rk_udphy *udphy = phy_get_drvdata(phy);
+
+ guard(mutex)(&udphy->mutex);
+
+ udphy->dwc3_reset_handler = reset_handler;
+ udphy->dwc3_reset_data = data;
+ udphy->dwc3_reset_port = port;
+}
+EXPORT_SYMBOL(rk_udphy_register_reset_handler);
+
static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
{
int ret;
@@ -1005,24 +1046,36 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
}
if (udphy->status == UDPHY_MODE_NONE) {
+ rk_udphy_run_reset_handler(udphy, true);
+
ret = rk_udphy_setup(udphy);
- if (ret)
+ if (ret) {
+ rk_udphy_run_reset_handler(udphy, false);
return ret;
+ }
if (!udphy->hs && udphy->mode & UDPHY_MODE_USB)
rk_udphy_u3_port_disable(udphy, false);
udphy->phy_needs_reinit = false;
+
+ rk_udphy_run_reset_handler(udphy, false);
} else if (udphy->phy_needs_reinit) {
+ rk_udphy_run_reset_handler(udphy, true);
+
rk_udphy_u3_port_disable(udphy, true);
udelay(10);
ret = rk_udphy_init(udphy);
- if (ret)
+ if (ret) {
+ rk_udphy_run_reset_handler(udphy, false);
return ret;
+ }
if (!udphy->hs && udphy->mode & UDPHY_MODE_USB)
rk_udphy_u3_port_disable(udphy, false);
udphy->phy_needs_reinit = false;
+
+ rk_udphy_run_reset_handler(udphy, false);
}
udphy->status |= mode;
diff --git a/include/linux/phy/rockchip-usbdp.h b/include/linux/phy/rockchip-usbdp.h
new file mode 100644
index 000000000000..6923ac28303b
--- /dev/null
+++ b/include/linux/phy/rockchip-usbdp.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Rockchip USBDP Combo PHY with Samsung IP block driver
+ *
+ * Copyright (C) 2026 Collabora Ltd
+ */
+
+#ifndef __ROCKCHIP_USBDP
+#define __ROCKCHIP_USBDP
+
+void rk_udphy_unregister_reset_handler(struct phy *phy);
+void rk_udphy_register_reset_handler(struct phy *phy,
+ int (*reset_handler)(void *data, int port, bool pre),
+ void *data, int port);
+
+#endif
--
2.53.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v17 2/7] phy: rockchip: usbdp: Add reset hook support for dwc3
@ 2026-10-16 18:43 ` Sebastian Reichel
0 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, Sebastian Reichel
When TCPM (Type-C Port Manager) triggers a orientation or mux
change, the PHY needs to reset. This results in the PHY no longer
providing a clock to the USB controller. If the USB controller
registers are accessed without the clock, SErrors are generated.
This can be avoided by switching the clock provider for the USB
controller. But after switching the clock, the USB PHY sometimes does
not succeed to lock its "cmn ana lcpll" PLLs again. I suspect this is
due to a clock glitch and after some debugging figured out that this
can be avoided by disabling the USB controller's PIPE interface while
the PHY reset is ongoing.
Implement an API that allows the USBDP to actively request a reset
from DWC3 as a quirk.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 57 +++++++++++++++++++++++++++++--
include/linux/phy/rockchip-usbdp.h | 16 +++++++++
2 files changed, 71 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index db69dd24f73b..b396f286e785 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -19,6 +19,7 @@
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/phy/phy.h>
+#include <linux/phy/rockchip-usbdp.h>
#include <linux/platform_device.h>
#include <linux/property.h>
#include <linux/regmap.h>
@@ -179,6 +180,9 @@ struct rk_udphy {
/* utilized for USB */
bool hs; /* flag for high-speed */
+ int (*dwc3_reset_handler)(void *data, int port, bool pre);
+ void *dwc3_reset_data;
+ int dwc3_reset_port;
/* utilized for DP */
struct gpio_desc *sbu1_dc_gpio;
@@ -995,6 +999,43 @@ static int rk_udphy_parse_dt(struct rk_udphy *udphy)
return rk_udphy_reset_init(udphy, dev);
}
+static int rk_udphy_run_reset_handler(struct rk_udphy *udphy, bool pre)
+{
+ if (!udphy->dwc3_reset_handler)
+ return 0;
+
+ if (!(udphy->sw_mode & UDPHY_MODE_USB))
+ return 0;
+
+ return udphy->dwc3_reset_handler(udphy->dwc3_reset_data, udphy->dwc3_reset_port, pre);
+}
+
+void rk_udphy_unregister_reset_handler(struct phy *phy)
+{
+ struct rk_udphy *udphy = phy_get_drvdata(phy);
+
+ guard(mutex)(&udphy->mutex);
+
+ udphy->dwc3_reset_handler = NULL;
+ udphy->dwc3_reset_data = NULL;
+ udphy->dwc3_reset_port = 0;
+}
+EXPORT_SYMBOL(rk_udphy_unregister_reset_handler);
+
+void rk_udphy_register_reset_handler(struct phy *phy,
+ int (*reset_handler)(void *data, int port, bool pre),
+ void *data, int port)
+{
+ struct rk_udphy *udphy = phy_get_drvdata(phy);
+
+ guard(mutex)(&udphy->mutex);
+
+ udphy->dwc3_reset_handler = reset_handler;
+ udphy->dwc3_reset_data = data;
+ udphy->dwc3_reset_port = port;
+}
+EXPORT_SYMBOL(rk_udphy_register_reset_handler);
+
static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
{
int ret;
@@ -1005,24 +1046,36 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
}
if (udphy->status == UDPHY_MODE_NONE) {
+ rk_udphy_run_reset_handler(udphy, true);
+
ret = rk_udphy_setup(udphy);
- if (ret)
+ if (ret) {
+ rk_udphy_run_reset_handler(udphy, false);
return ret;
+ }
if (!udphy->hs && udphy->mode & UDPHY_MODE_USB)
rk_udphy_u3_port_disable(udphy, false);
udphy->phy_needs_reinit = false;
+
+ rk_udphy_run_reset_handler(udphy, false);
} else if (udphy->phy_needs_reinit) {
+ rk_udphy_run_reset_handler(udphy, true);
+
rk_udphy_u3_port_disable(udphy, true);
udelay(10);
ret = rk_udphy_init(udphy);
- if (ret)
+ if (ret) {
+ rk_udphy_run_reset_handler(udphy, false);
return ret;
+ }
if (!udphy->hs && udphy->mode & UDPHY_MODE_USB)
rk_udphy_u3_port_disable(udphy, false);
udphy->phy_needs_reinit = false;
+
+ rk_udphy_run_reset_handler(udphy, false);
}
udphy->status |= mode;
diff --git a/include/linux/phy/rockchip-usbdp.h b/include/linux/phy/rockchip-usbdp.h
new file mode 100644
index 000000000000..6923ac28303b
--- /dev/null
+++ b/include/linux/phy/rockchip-usbdp.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Rockchip USBDP Combo PHY with Samsung IP block driver
+ *
+ * Copyright (C) 2026 Collabora Ltd
+ */
+
+#ifndef __ROCKCHIP_USBDP
+#define __ROCKCHIP_USBDP
+
+void rk_udphy_unregister_reset_handler(struct phy *phy);
+void rk_udphy_register_reset_handler(struct phy *phy,
+ int (*reset_handler)(void *data, int port, bool pre),
+ void *data, int port);
+
+#endif
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v17 2/7] phy: rockchip: usbdp: Add reset hook support for dwc3
2026-10-16 18:43 ` Sebastian Reichel
@ 2026-10-16 18:54 ` sashiko-bot
-1 siblings, 0 replies; 35+ messages in thread
From: sashiko-bot @ 2026-10-16 18:54 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-usb, linux-phy, vkoul, neil.armstrong, olteanv
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] phy: rockchip: missing NULL check in reset APIs causes panic
- [High] phy: rockchip: usbdp: sw_mode leak causes SError on DWC3 remove
- [Medium] phy: rockchip: usbdp: Ignored return value of dwc3 pre-reset handler allows unsafe PHY reset
- [Low] build: missing dummy stubs for rk_udphy_register_reset_handler
--
commit c002b32dadef4ddd580b443a7afb2debf23073cc
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Add reset hook support for dwc3
When TCPM (Type-C Port Manager) triggers a orientation or mux
change, the PHY needs to reset. This results in the PHY no longer
providing a clock to the USB controller. If the USB controller
registers are accessed without the clock, SErrors are generated.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index db69dd24f73b3..b396f286e7852 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -995,6 +999,43 @@ static int rk_udphy_parse_dt(struct rk_udphy *udphy)
> return rk_udphy_reset_init(udphy, dev);
> }
>
> +static int rk_udphy_run_reset_handler(struct rk_udphy *udphy, bool pre)
> +{
> + if (!udphy->dwc3_reset_handler)
> + return 0;
> +
> + if (!(udphy->sw_mode & UDPHY_MODE_USB))
> + return 0;
> +
> + return udphy->dwc3_reset_handler(udphy->dwc3_reset_data, udphy->dwc3_reset_port, pre);
> +}
[Severity: High]
Can the UDPHY_MODE_USB flag leak in udphy->sw_mode when rk_udphy_usb3_phy_exit()
returns early (e.g. in high-speed or DP-only mode)?
If rk_udphy_usb3_phy_exit() 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)
return 0;
udphy->sw_mode &= ~UDPHY_MODE_USB;
...
}
It bypasses clearing UDPHY_MODE_USB, leaving it persistently set even after the
USB PHY is logically closed. Then, during DWC3 driver removal, if a concurrent
Type-C mux event triggers a DP PHY reinitialization, the leaked sw_mode flag
would cause rk_udphy_run_reset_handler() to erroneously execute the reset
handler. Because pm_runtime_disable() was called, pm_runtime_get_if_active()
returns -EINVAL, bypassing the PM suspension check, allowing the function to
access DWC3 registers without a clock and generating an SError.
> +
> +void rk_udphy_unregister_reset_handler(struct phy *phy)
> +{
> + struct rk_udphy *udphy = phy_get_drvdata(phy);
> +
[Severity: High]
Does this exported API need to validate that the phy pointer is not NULL or
ERR_PTR before dereferencing it?
If a consumer driver passes a NULL phy (for example, if the USB3 PHY is
optional or absent and the USB core set dwc->usb3_generic_phy[i] to NULL),
phy_get_drvdata(phy) will cause an immediate kernel panic. Follow-up patches
in dwc3-rockchip manually check dev_driver_string(phy->dev.parent), which
also causes a panic if phy is NULL. Adding a NULL check here and in
rk_udphy_register_reset_handler() would prevent these panics and relieve
consumers from performing fragile type checking.
> + guard(mutex)(&udphy->mutex);
> +
> + udphy->dwc3_reset_handler = NULL;
> + udphy->dwc3_reset_data = NULL;
> + udphy->dwc3_reset_port = 0;
> +}
> +EXPORT_SYMBOL(rk_udphy_unregister_reset_handler);
> +
> +void rk_udphy_register_reset_handler(struct phy *phy,
> + int (*reset_handler)(void *data, int port, bool pre),
> + void *data, int port)
> +{
> + struct rk_udphy *udphy = phy_get_drvdata(phy);
> +
[Severity: High]
Similar to rk_udphy_unregister_reset_handler(), should we check if phy is NULL
or ERR_PTR before dereferencing it here?
> + guard(mutex)(&udphy->mutex);
> +
> + udphy->dwc3_reset_handler = reset_handler;
> + udphy->dwc3_reset_data = data;
> + udphy->dwc3_reset_port = port;
> +}
> +EXPORT_SYMBOL(rk_udphy_register_reset_handler);
> +
> static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> {
> int ret;
> @@ -1005,24 +1046,36 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> }
>
> if (udphy->status == UDPHY_MODE_NONE) {
> + rk_udphy_run_reset_handler(udphy, true);
> +
[Severity: Medium]
Should we check the return value of rk_udphy_run_reset_handler(udphy, true)
here and abort the power-on sequence on failure?
If the DWC3 controller fails to quiesce due to hardware state or timeouts,
the handler will propagate the error code. Ignoring this error and
proceeding unconditionally to rk_udphy_setup() drops the PHY clock while
the DWC3 PIPE interface might still be active, directly violating the safety
invariant this patch was introduced to enforce and leading to SErrors.
> ret = rk_udphy_setup(udphy);
> - if (ret)
> + if (ret) {
> + rk_udphy_run_reset_handler(udphy, false);
> return ret;
> + }
>
> if (!udphy->hs && udphy->mode & UDPHY_MODE_USB)
> rk_udphy_u3_port_disable(udphy, false);
> udphy->phy_needs_reinit = false;
> +
> + rk_udphy_run_reset_handler(udphy, false);
> } else if (udphy->phy_needs_reinit) {
> + rk_udphy_run_reset_handler(udphy, true);
> +
[Severity: Medium]
Same question here during the reinitialization sequence. Does this need to
handle the return value to prevent proceeding with PHY reset when the DWC3 PIPE
interface has not successfully been disabled?
> rk_udphy_u3_port_disable(udphy, true);
> udelay(10);
>
> ret = rk_udphy_init(udphy);
> - if (ret)
> + if (ret) {
> + rk_udphy_run_reset_handler(udphy, false);
> return ret;
> + }
>
> if (!udphy->hs && udphy->mode & UDPHY_MODE_USB)
> rk_udphy_u3_port_disable(udphy, false);
> udphy->phy_needs_reinit = false;
> +
> + rk_udphy_run_reset_handler(udphy, false);
> }
>
> udphy->status |= mode;
> diff --git a/include/linux/phy/rockchip-usbdp.h b/include/linux/phy/rockchip-usbdp.h
> new file mode 100644
> --- /dev/null
> +++ b/include/linux/phy/rockchip-usbdp.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Rockchip USBDP Combo PHY with Samsung IP block driver
> + *
> + * Copyright (C) 2026 Collabora Ltd
> + */
> +
> +#ifndef __ROCKCHIP_USBDP
> +#define __ROCKCHIP_USBDP
> +
> +void rk_udphy_unregister_reset_handler(struct phy *phy);
> +void rk_udphy_register_reset_handler(struct phy *phy,
> + int (*reset_handler)(void *data, int port, bool pre),
> + void *data, int port);
> +
[Severity: Low]
Should this header provide #if IS_ENABLED(CONFIG_PHY_ROCKCHIP_USBDP) guards
and static inline dummy fallbacks for these functions?
If CONFIG_USB_DWC3_ROCKCHIP is enabled but CONFIG_PHY_ROCKCHIP_USBDP is disabled,
any caller invoking these APIs directly will cause the kernel build to fail at the
linking stage with undefined reference errors.
> +#endif
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-b4-rockchip-dwc3-rockchip-glue-v17-0-876f96893e2f@collabora.com?part=2
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v17 2/7] phy: rockchip: usbdp: Add reset hook support for dwc3
@ 2026-10-16 18:54 ` sashiko-bot
0 siblings, 0 replies; 35+ messages in thread
From: sashiko-bot @ 2026-10-16 18:54 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-usb, linux-phy, vkoul, neil.armstrong, olteanv
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] phy: rockchip: missing NULL check in reset APIs causes panic
- [High] phy: rockchip: usbdp: sw_mode leak causes SError on DWC3 remove
- [Medium] phy: rockchip: usbdp: Ignored return value of dwc3 pre-reset handler allows unsafe PHY reset
- [Low] build: missing dummy stubs for rk_udphy_register_reset_handler
--
commit c002b32dadef4ddd580b443a7afb2debf23073cc
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Add reset hook support for dwc3
When TCPM (Type-C Port Manager) triggers a orientation or mux
change, the PHY needs to reset. This results in the PHY no longer
providing a clock to the USB controller. If the USB controller
registers are accessed without the clock, SErrors are generated.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index db69dd24f73b3..b396f286e7852 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -995,6 +999,43 @@ static int rk_udphy_parse_dt(struct rk_udphy *udphy)
> return rk_udphy_reset_init(udphy, dev);
> }
>
> +static int rk_udphy_run_reset_handler(struct rk_udphy *udphy, bool pre)
> +{
> + if (!udphy->dwc3_reset_handler)
> + return 0;
> +
> + if (!(udphy->sw_mode & UDPHY_MODE_USB))
> + return 0;
> +
> + return udphy->dwc3_reset_handler(udphy->dwc3_reset_data, udphy->dwc3_reset_port, pre);
> +}
[Severity: High]
Can the UDPHY_MODE_USB flag leak in udphy->sw_mode when rk_udphy_usb3_phy_exit()
returns early (e.g. in high-speed or DP-only mode)?
If rk_udphy_usb3_phy_exit() 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)
return 0;
udphy->sw_mode &= ~UDPHY_MODE_USB;
...
}
It bypasses clearing UDPHY_MODE_USB, leaving it persistently set even after the
USB PHY is logically closed. Then, during DWC3 driver removal, if a concurrent
Type-C mux event triggers a DP PHY reinitialization, the leaked sw_mode flag
would cause rk_udphy_run_reset_handler() to erroneously execute the reset
handler. Because pm_runtime_disable() was called, pm_runtime_get_if_active()
returns -EINVAL, bypassing the PM suspension check, allowing the function to
access DWC3 registers without a clock and generating an SError.
> +
> +void rk_udphy_unregister_reset_handler(struct phy *phy)
> +{
> + struct rk_udphy *udphy = phy_get_drvdata(phy);
> +
[Severity: High]
Does this exported API need to validate that the phy pointer is not NULL or
ERR_PTR before dereferencing it?
If a consumer driver passes a NULL phy (for example, if the USB3 PHY is
optional or absent and the USB core set dwc->usb3_generic_phy[i] to NULL),
phy_get_drvdata(phy) will cause an immediate kernel panic. Follow-up patches
in dwc3-rockchip manually check dev_driver_string(phy->dev.parent), which
also causes a panic if phy is NULL. Adding a NULL check here and in
rk_udphy_register_reset_handler() would prevent these panics and relieve
consumers from performing fragile type checking.
> + guard(mutex)(&udphy->mutex);
> +
> + udphy->dwc3_reset_handler = NULL;
> + udphy->dwc3_reset_data = NULL;
> + udphy->dwc3_reset_port = 0;
> +}
> +EXPORT_SYMBOL(rk_udphy_unregister_reset_handler);
> +
> +void rk_udphy_register_reset_handler(struct phy *phy,
> + int (*reset_handler)(void *data, int port, bool pre),
> + void *data, int port)
> +{
> + struct rk_udphy *udphy = phy_get_drvdata(phy);
> +
[Severity: High]
Similar to rk_udphy_unregister_reset_handler(), should we check if phy is NULL
or ERR_PTR before dereferencing it here?
> + guard(mutex)(&udphy->mutex);
> +
> + udphy->dwc3_reset_handler = reset_handler;
> + udphy->dwc3_reset_data = data;
> + udphy->dwc3_reset_port = port;
> +}
> +EXPORT_SYMBOL(rk_udphy_register_reset_handler);
> +
> static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> {
> int ret;
> @@ -1005,24 +1046,36 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> }
>
> if (udphy->status == UDPHY_MODE_NONE) {
> + rk_udphy_run_reset_handler(udphy, true);
> +
[Severity: Medium]
Should we check the return value of rk_udphy_run_reset_handler(udphy, true)
here and abort the power-on sequence on failure?
If the DWC3 controller fails to quiesce due to hardware state or timeouts,
the handler will propagate the error code. Ignoring this error and
proceeding unconditionally to rk_udphy_setup() drops the PHY clock while
the DWC3 PIPE interface might still be active, directly violating the safety
invariant this patch was introduced to enforce and leading to SErrors.
> ret = rk_udphy_setup(udphy);
> - if (ret)
> + if (ret) {
> + rk_udphy_run_reset_handler(udphy, false);
> return ret;
> + }
>
> if (!udphy->hs && udphy->mode & UDPHY_MODE_USB)
> rk_udphy_u3_port_disable(udphy, false);
> udphy->phy_needs_reinit = false;
> +
> + rk_udphy_run_reset_handler(udphy, false);
> } else if (udphy->phy_needs_reinit) {
> + rk_udphy_run_reset_handler(udphy, true);
> +
[Severity: Medium]
Same question here during the reinitialization sequence. Does this need to
handle the return value to prevent proceeding with PHY reset when the DWC3 PIPE
interface has not successfully been disabled?
> rk_udphy_u3_port_disable(udphy, true);
> udelay(10);
>
> ret = rk_udphy_init(udphy);
> - if (ret)
> + if (ret) {
> + rk_udphy_run_reset_handler(udphy, false);
> return ret;
> + }
>
> if (!udphy->hs && udphy->mode & UDPHY_MODE_USB)
> rk_udphy_u3_port_disable(udphy, false);
> udphy->phy_needs_reinit = false;
> +
> + rk_udphy_run_reset_handler(udphy, false);
> }
>
> udphy->status |= mode;
> diff --git a/include/linux/phy/rockchip-usbdp.h b/include/linux/phy/rockchip-usbdp.h
> new file mode 100644
> --- /dev/null
> +++ b/include/linux/phy/rockchip-usbdp.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Rockchip USBDP Combo PHY with Samsung IP block driver
> + *
> + * Copyright (C) 2026 Collabora Ltd
> + */
> +
> +#ifndef __ROCKCHIP_USBDP
> +#define __ROCKCHIP_USBDP
> +
> +void rk_udphy_unregister_reset_handler(struct phy *phy);
> +void rk_udphy_register_reset_handler(struct phy *phy,
> + int (*reset_handler)(void *data, int port, bool pre),
> + void *data, int port);
> +
[Severity: Low]
Should this header provide #if IS_ENABLED(CONFIG_PHY_ROCKCHIP_USBDP) guards
and static inline dummy fallbacks for these functions?
If CONFIG_USB_DWC3_ROCKCHIP is enabled but CONFIG_PHY_ROCKCHIP_USBDP is disabled,
any caller invoking these APIs directly will cause the kernel build to fail at the
linking stage with undefined reference errors.
> +#endif
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-b4-rockchip-dwc3-rockchip-glue-v17-0-876f96893e2f@collabora.com?part=2
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v17 3/7] usb: dwc3: rockchip: introduce glue driver
2026-10-16 18:43 ` Sebastian Reichel
(?)
@ 2026-10-16 18:43 ` Sebastian Reichel
-1 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, Sebastian Reichel
Introduce Rockchip specific glue code for the Synopsys DWC3 USB driver.
For now this handles things identical to the default glue.
After this change the glue driver is mandatory for Rockchip RK3588 and
RK3576; it does not make sense to fallback to the core driver as the
whole purpose of the glue driver is fixing race conditions resulting in
SErrors and non-functional USB.
Tested-by: Igor Paunovic <royalnet026@gmail.com> # Orange Pi 5 Plus
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/usb/dwc3/Kconfig | 11 ++++
drivers/usb/dwc3/Makefile | 1 +
drivers/usb/dwc3/core.c | 15 ++++++
drivers/usb/dwc3/dwc3-rockchip.c | 106 +++++++++++++++++++++++++++++++++++++++
4 files changed, 133 insertions(+)
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 18169727a413..3c120ea9746d 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -190,6 +190,17 @@ config USB_DWC3_OCTEON
Only the host mode is currently supported.
Say 'Y' or 'M' here if you have one such device.
+config USB_DWC3_ROCKCHIP
+ tristate "Rockchip DWC3 Platform Driver"
+ depends on ARCH_ROCKCHIP || COMPILE_TEST
+ depends on OF
+ default USB_DWC3
+ help
+ Rockchip SoCs with DesignWare Core USB3 IP inside,
+ and IP Core configured for USB 2.0 and USB 3.0 in host
+ or dual-role mode.
+ Say 'Y' or 'M' if you have such device.
+
config USB_DWC3_RTK
tristate "Realtek DWC3 Platform Driver"
depends on OF && ARCH_REALTEK
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index f37971197203..444e7e7f34b2 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -58,6 +58,7 @@ obj-$(CONFIG_USB_DWC3_IMX8MP) += dwc3-imx8mp.o
obj-$(CONFIG_USB_DWC3_IMX) += dwc3-imx.o
obj-$(CONFIG_USB_DWC3_XILINX) += dwc3-xilinx.o
obj-$(CONFIG_USB_DWC3_OCTEON) += dwc3-octeon.o
+obj-$(CONFIG_USB_DWC3_ROCKCHIP) += dwc3-rockchip.o
obj-$(CONFIG_USB_DWC3_RTK) += dwc3-rtk.o
obj-$(CONFIG_USB_DWC3_GENERIC_PLAT) += dwc3-generic-plat.o
obj-$(CONFIG_USB_DWC3_GOOGLE) += dwc3-google.o
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index fd5c2cd36c59..4b7132887a03 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -2448,11 +2448,26 @@ int dwc3_core_probe(const struct dwc3_probe_data *data)
}
EXPORT_SYMBOL_GPL(dwc3_core_probe);
+/*
+ * List of compatibles, which have "synopsys,dwc3" as a fallback
+ * compatible, but have a vendor specific glue driver that should
+ * be used instead of this one.
+ */
+static const char *const dwc3_compatible_blocklist[] = {
+ "rockchip,rk3588-dwc3",
+ "rockchip,rk3576-dwc3",
+};
+
static int dwc3_probe(struct platform_device *pdev)
{
struct dwc3_probe_data probe_data = {};
struct resource *res;
struct dwc3 *dwc;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(dwc3_compatible_blocklist); i++)
+ if (device_is_compatible(&pdev->dev, dwc3_compatible_blocklist[i]))
+ return -ENODEV;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c
new file mode 100644
index 000000000000..4129aefd6a8b
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-rockchip.c
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026, Collabora Ltd. */
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include "glue.h"
+
+struct dwc3_rockchip {
+ struct dwc3 dwc;
+};
+
+static int dwc3_rockchip_probe(struct platform_device *pdev)
+{
+ struct dwc3_probe_data probe_data = {};
+ struct resource *res;
+ struct dwc3_rockchip *dwc_rk;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(&pdev->dev, "missing memory resource\n");
+ return -ENODEV;
+ }
+
+ dwc_rk = devm_kzalloc(&pdev->dev, sizeof(*dwc_rk), GFP_KERNEL);
+ if (!dwc_rk)
+ return -ENOMEM;
+
+ dwc_rk->dwc.dev = &pdev->dev;
+ dwc_rk->dwc.glue_ops = NULL;
+
+ probe_data.dwc = &dwc_rk->dwc;
+ probe_data.res = res;
+ probe_data.properties = DWC3_DEFAULT_PROPERTIES;
+
+ return dwc3_core_probe(&probe_data);
+}
+
+static void dwc3_rockchip_remove(struct platform_device *pdev)
+{
+ dwc3_core_remove(platform_get_drvdata(pdev));
+}
+
+static int dwc3_rockchip_runtime_suspend(struct device *dev)
+{
+ return dwc3_runtime_suspend(dev_get_drvdata(dev));
+}
+
+static int dwc3_rockchip_runtime_resume(struct device *dev)
+{
+ return dwc3_runtime_resume(dev_get_drvdata(dev));
+}
+
+static int dwc3_rockchip_runtime_idle(struct device *dev)
+{
+ return dwc3_runtime_idle(dev_get_drvdata(dev));
+}
+
+static int dwc3_rockchip_suspend(struct device *dev)
+{
+ return dwc3_pm_suspend(dev_get_drvdata(dev));
+}
+
+static int dwc3_rockchip_resume(struct device *dev)
+{
+ return dwc3_pm_resume(dev_get_drvdata(dev));
+}
+
+static void dwc3_rockchip_complete(struct device *dev)
+{
+ dwc3_pm_complete(dev_get_drvdata(dev));
+}
+
+static int dwc3_rockchip_prepare(struct device *dev)
+{
+ return dwc3_pm_prepare(dev_get_drvdata(dev));
+}
+
+static const struct dev_pm_ops dwc3_rockchip_dev_pm_ops = {
+ SYSTEM_SLEEP_PM_OPS(dwc3_rockchip_suspend, dwc3_rockchip_resume)
+ RUNTIME_PM_OPS(dwc3_rockchip_runtime_suspend, dwc3_rockchip_runtime_resume,
+ dwc3_rockchip_runtime_idle)
+ .complete = pm_sleep_ptr(dwc3_rockchip_complete),
+ .prepare = pm_sleep_ptr(dwc3_rockchip_prepare),
+};
+
+static const struct of_device_id dwc3_rockchip_of_match[] = {
+ { .compatible = "rockchip,rk3588-dwc3" },
+ { .compatible = "rockchip,rk3576-dwc3" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, dwc3_rockchip_of_match);
+
+static struct platform_driver dwc3_rockchip_driver = {
+ .probe = dwc3_rockchip_probe,
+ .remove = dwc3_rockchip_remove,
+ .driver = {
+ .name = "dwc3-rockchip",
+ .pm = pm_ptr(&dwc3_rockchip_dev_pm_ops),
+ .of_match_table = dwc3_rockchip_of_match,
+ },
+};
+
+module_platform_driver(dwc3_rockchip_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("DesignWare DWC3 Rockchip Glue Driver");
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v17 3/7] usb: dwc3: rockchip: introduce glue driver
@ 2026-10-16 18:43 ` Sebastian Reichel
0 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, Sebastian Reichel
Introduce Rockchip specific glue code for the Synopsys DWC3 USB driver.
For now this handles things identical to the default glue.
After this change the glue driver is mandatory for Rockchip RK3588 and
RK3576; it does not make sense to fallback to the core driver as the
whole purpose of the glue driver is fixing race conditions resulting in
SErrors and non-functional USB.
Tested-by: Igor Paunovic <royalnet026@gmail.com> # Orange Pi 5 Plus
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/usb/dwc3/Kconfig | 11 ++++
drivers/usb/dwc3/Makefile | 1 +
drivers/usb/dwc3/core.c | 15 ++++++
drivers/usb/dwc3/dwc3-rockchip.c | 106 +++++++++++++++++++++++++++++++++++++++
4 files changed, 133 insertions(+)
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 18169727a413..3c120ea9746d 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -190,6 +190,17 @@ config USB_DWC3_OCTEON
Only the host mode is currently supported.
Say 'Y' or 'M' here if you have one such device.
+config USB_DWC3_ROCKCHIP
+ tristate "Rockchip DWC3 Platform Driver"
+ depends on ARCH_ROCKCHIP || COMPILE_TEST
+ depends on OF
+ default USB_DWC3
+ help
+ Rockchip SoCs with DesignWare Core USB3 IP inside,
+ and IP Core configured for USB 2.0 and USB 3.0 in host
+ or dual-role mode.
+ Say 'Y' or 'M' if you have such device.
+
config USB_DWC3_RTK
tristate "Realtek DWC3 Platform Driver"
depends on OF && ARCH_REALTEK
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index f37971197203..444e7e7f34b2 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -58,6 +58,7 @@ obj-$(CONFIG_USB_DWC3_IMX8MP) += dwc3-imx8mp.o
obj-$(CONFIG_USB_DWC3_IMX) += dwc3-imx.o
obj-$(CONFIG_USB_DWC3_XILINX) += dwc3-xilinx.o
obj-$(CONFIG_USB_DWC3_OCTEON) += dwc3-octeon.o
+obj-$(CONFIG_USB_DWC3_ROCKCHIP) += dwc3-rockchip.o
obj-$(CONFIG_USB_DWC3_RTK) += dwc3-rtk.o
obj-$(CONFIG_USB_DWC3_GENERIC_PLAT) += dwc3-generic-plat.o
obj-$(CONFIG_USB_DWC3_GOOGLE) += dwc3-google.o
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index fd5c2cd36c59..4b7132887a03 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -2448,11 +2448,26 @@ int dwc3_core_probe(const struct dwc3_probe_data *data)
}
EXPORT_SYMBOL_GPL(dwc3_core_probe);
+/*
+ * List of compatibles, which have "synopsys,dwc3" as a fallback
+ * compatible, but have a vendor specific glue driver that should
+ * be used instead of this one.
+ */
+static const char *const dwc3_compatible_blocklist[] = {
+ "rockchip,rk3588-dwc3",
+ "rockchip,rk3576-dwc3",
+};
+
static int dwc3_probe(struct platform_device *pdev)
{
struct dwc3_probe_data probe_data = {};
struct resource *res;
struct dwc3 *dwc;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(dwc3_compatible_blocklist); i++)
+ if (device_is_compatible(&pdev->dev, dwc3_compatible_blocklist[i]))
+ return -ENODEV;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c
new file mode 100644
index 000000000000..4129aefd6a8b
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-rockchip.c
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026, Collabora Ltd. */
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include "glue.h"
+
+struct dwc3_rockchip {
+ struct dwc3 dwc;
+};
+
+static int dwc3_rockchip_probe(struct platform_device *pdev)
+{
+ struct dwc3_probe_data probe_data = {};
+ struct resource *res;
+ struct dwc3_rockchip *dwc_rk;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(&pdev->dev, "missing memory resource\n");
+ return -ENODEV;
+ }
+
+ dwc_rk = devm_kzalloc(&pdev->dev, sizeof(*dwc_rk), GFP_KERNEL);
+ if (!dwc_rk)
+ return -ENOMEM;
+
+ dwc_rk->dwc.dev = &pdev->dev;
+ dwc_rk->dwc.glue_ops = NULL;
+
+ probe_data.dwc = &dwc_rk->dwc;
+ probe_data.res = res;
+ probe_data.properties = DWC3_DEFAULT_PROPERTIES;
+
+ return dwc3_core_probe(&probe_data);
+}
+
+static void dwc3_rockchip_remove(struct platform_device *pdev)
+{
+ dwc3_core_remove(platform_get_drvdata(pdev));
+}
+
+static int dwc3_rockchip_runtime_suspend(struct device *dev)
+{
+ return dwc3_runtime_suspend(dev_get_drvdata(dev));
+}
+
+static int dwc3_rockchip_runtime_resume(struct device *dev)
+{
+ return dwc3_runtime_resume(dev_get_drvdata(dev));
+}
+
+static int dwc3_rockchip_runtime_idle(struct device *dev)
+{
+ return dwc3_runtime_idle(dev_get_drvdata(dev));
+}
+
+static int dwc3_rockchip_suspend(struct device *dev)
+{
+ return dwc3_pm_suspend(dev_get_drvdata(dev));
+}
+
+static int dwc3_rockchip_resume(struct device *dev)
+{
+ return dwc3_pm_resume(dev_get_drvdata(dev));
+}
+
+static void dwc3_rockchip_complete(struct device *dev)
+{
+ dwc3_pm_complete(dev_get_drvdata(dev));
+}
+
+static int dwc3_rockchip_prepare(struct device *dev)
+{
+ return dwc3_pm_prepare(dev_get_drvdata(dev));
+}
+
+static const struct dev_pm_ops dwc3_rockchip_dev_pm_ops = {
+ SYSTEM_SLEEP_PM_OPS(dwc3_rockchip_suspend, dwc3_rockchip_resume)
+ RUNTIME_PM_OPS(dwc3_rockchip_runtime_suspend, dwc3_rockchip_runtime_resume,
+ dwc3_rockchip_runtime_idle)
+ .complete = pm_sleep_ptr(dwc3_rockchip_complete),
+ .prepare = pm_sleep_ptr(dwc3_rockchip_prepare),
+};
+
+static const struct of_device_id dwc3_rockchip_of_match[] = {
+ { .compatible = "rockchip,rk3588-dwc3" },
+ { .compatible = "rockchip,rk3576-dwc3" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, dwc3_rockchip_of_match);
+
+static struct platform_driver dwc3_rockchip_driver = {
+ .probe = dwc3_rockchip_probe,
+ .remove = dwc3_rockchip_remove,
+ .driver = {
+ .name = "dwc3-rockchip",
+ .pm = pm_ptr(&dwc3_rockchip_dev_pm_ops),
+ .of_match_table = dwc3_rockchip_of_match,
+ },
+};
+
+module_platform_driver(dwc3_rockchip_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("DesignWare DWC3 Rockchip Glue Driver");
--
2.53.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v17 3/7] usb: dwc3: rockchip: introduce glue driver
@ 2026-10-16 18:43 ` Sebastian Reichel
0 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, Sebastian Reichel
Introduce Rockchip specific glue code for the Synopsys DWC3 USB driver.
For now this handles things identical to the default glue.
After this change the glue driver is mandatory for Rockchip RK3588 and
RK3576; it does not make sense to fallback to the core driver as the
whole purpose of the glue driver is fixing race conditions resulting in
SErrors and non-functional USB.
Tested-by: Igor Paunovic <royalnet026@gmail.com> # Orange Pi 5 Plus
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/usb/dwc3/Kconfig | 11 ++++
drivers/usb/dwc3/Makefile | 1 +
drivers/usb/dwc3/core.c | 15 ++++++
drivers/usb/dwc3/dwc3-rockchip.c | 106 +++++++++++++++++++++++++++++++++++++++
4 files changed, 133 insertions(+)
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 18169727a413..3c120ea9746d 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -190,6 +190,17 @@ config USB_DWC3_OCTEON
Only the host mode is currently supported.
Say 'Y' or 'M' here if you have one such device.
+config USB_DWC3_ROCKCHIP
+ tristate "Rockchip DWC3 Platform Driver"
+ depends on ARCH_ROCKCHIP || COMPILE_TEST
+ depends on OF
+ default USB_DWC3
+ help
+ Rockchip SoCs with DesignWare Core USB3 IP inside,
+ and IP Core configured for USB 2.0 and USB 3.0 in host
+ or dual-role mode.
+ Say 'Y' or 'M' if you have such device.
+
config USB_DWC3_RTK
tristate "Realtek DWC3 Platform Driver"
depends on OF && ARCH_REALTEK
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index f37971197203..444e7e7f34b2 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -58,6 +58,7 @@ obj-$(CONFIG_USB_DWC3_IMX8MP) += dwc3-imx8mp.o
obj-$(CONFIG_USB_DWC3_IMX) += dwc3-imx.o
obj-$(CONFIG_USB_DWC3_XILINX) += dwc3-xilinx.o
obj-$(CONFIG_USB_DWC3_OCTEON) += dwc3-octeon.o
+obj-$(CONFIG_USB_DWC3_ROCKCHIP) += dwc3-rockchip.o
obj-$(CONFIG_USB_DWC3_RTK) += dwc3-rtk.o
obj-$(CONFIG_USB_DWC3_GENERIC_PLAT) += dwc3-generic-plat.o
obj-$(CONFIG_USB_DWC3_GOOGLE) += dwc3-google.o
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index fd5c2cd36c59..4b7132887a03 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -2448,11 +2448,26 @@ int dwc3_core_probe(const struct dwc3_probe_data *data)
}
EXPORT_SYMBOL_GPL(dwc3_core_probe);
+/*
+ * List of compatibles, which have "synopsys,dwc3" as a fallback
+ * compatible, but have a vendor specific glue driver that should
+ * be used instead of this one.
+ */
+static const char *const dwc3_compatible_blocklist[] = {
+ "rockchip,rk3588-dwc3",
+ "rockchip,rk3576-dwc3",
+};
+
static int dwc3_probe(struct platform_device *pdev)
{
struct dwc3_probe_data probe_data = {};
struct resource *res;
struct dwc3 *dwc;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(dwc3_compatible_blocklist); i++)
+ if (device_is_compatible(&pdev->dev, dwc3_compatible_blocklist[i]))
+ return -ENODEV;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c
new file mode 100644
index 000000000000..4129aefd6a8b
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-rockchip.c
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026, Collabora Ltd. */
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include "glue.h"
+
+struct dwc3_rockchip {
+ struct dwc3 dwc;
+};
+
+static int dwc3_rockchip_probe(struct platform_device *pdev)
+{
+ struct dwc3_probe_data probe_data = {};
+ struct resource *res;
+ struct dwc3_rockchip *dwc_rk;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(&pdev->dev, "missing memory resource\n");
+ return -ENODEV;
+ }
+
+ dwc_rk = devm_kzalloc(&pdev->dev, sizeof(*dwc_rk), GFP_KERNEL);
+ if (!dwc_rk)
+ return -ENOMEM;
+
+ dwc_rk->dwc.dev = &pdev->dev;
+ dwc_rk->dwc.glue_ops = NULL;
+
+ probe_data.dwc = &dwc_rk->dwc;
+ probe_data.res = res;
+ probe_data.properties = DWC3_DEFAULT_PROPERTIES;
+
+ return dwc3_core_probe(&probe_data);
+}
+
+static void dwc3_rockchip_remove(struct platform_device *pdev)
+{
+ dwc3_core_remove(platform_get_drvdata(pdev));
+}
+
+static int dwc3_rockchip_runtime_suspend(struct device *dev)
+{
+ return dwc3_runtime_suspend(dev_get_drvdata(dev));
+}
+
+static int dwc3_rockchip_runtime_resume(struct device *dev)
+{
+ return dwc3_runtime_resume(dev_get_drvdata(dev));
+}
+
+static int dwc3_rockchip_runtime_idle(struct device *dev)
+{
+ return dwc3_runtime_idle(dev_get_drvdata(dev));
+}
+
+static int dwc3_rockchip_suspend(struct device *dev)
+{
+ return dwc3_pm_suspend(dev_get_drvdata(dev));
+}
+
+static int dwc3_rockchip_resume(struct device *dev)
+{
+ return dwc3_pm_resume(dev_get_drvdata(dev));
+}
+
+static void dwc3_rockchip_complete(struct device *dev)
+{
+ dwc3_pm_complete(dev_get_drvdata(dev));
+}
+
+static int dwc3_rockchip_prepare(struct device *dev)
+{
+ return dwc3_pm_prepare(dev_get_drvdata(dev));
+}
+
+static const struct dev_pm_ops dwc3_rockchip_dev_pm_ops = {
+ SYSTEM_SLEEP_PM_OPS(dwc3_rockchip_suspend, dwc3_rockchip_resume)
+ RUNTIME_PM_OPS(dwc3_rockchip_runtime_suspend, dwc3_rockchip_runtime_resume,
+ dwc3_rockchip_runtime_idle)
+ .complete = pm_sleep_ptr(dwc3_rockchip_complete),
+ .prepare = pm_sleep_ptr(dwc3_rockchip_prepare),
+};
+
+static const struct of_device_id dwc3_rockchip_of_match[] = {
+ { .compatible = "rockchip,rk3588-dwc3" },
+ { .compatible = "rockchip,rk3576-dwc3" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, dwc3_rockchip_of_match);
+
+static struct platform_driver dwc3_rockchip_driver = {
+ .probe = dwc3_rockchip_probe,
+ .remove = dwc3_rockchip_remove,
+ .driver = {
+ .name = "dwc3-rockchip",
+ .pm = pm_ptr(&dwc3_rockchip_dev_pm_ops),
+ .of_match_table = dwc3_rockchip_of_match,
+ },
+};
+
+module_platform_driver(dwc3_rockchip_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("DesignWare DWC3 Rockchip Glue Driver");
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v17 3/7] usb: dwc3: rockchip: introduce glue driver
2026-10-16 18:43 ` Sebastian Reichel
(?)
(?)
@ 2026-10-16 18:54 ` sashiko-bot
-1 siblings, 0 replies; 35+ messages in thread
From: sashiko-bot @ 2026-10-16 18:54 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-usb
> Introduce Rockchip specific glue code for the Synopsys DWC3 USB driver.
> For now this handles things identical to the default glue.
>
> After this change the glue driver is mandatory for Rockchip RK3588 and
> RK3576; it does not make sense to fallback to the core driver as the
> whole purpose of the glue driver is fixing race conditions resulting in
> SErrors and non-functional USB.
>
> Tested-by: Igor Paunovic <royalnet026@gmail.com> # Orange Pi 5 Plus
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-b4-rockchip-dwc3-rockchip-glue-v17-0-876f96893e2f@collabora.com?part=3
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v17 4/7] usb: dwc3: core: add post PHY registration hook for platform glue
2026-10-16 18:43 ` Sebastian Reichel
(?)
@ 2026-10-16 18:43 ` Sebastian Reichel
-1 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, Sebastian Reichel,
Krishna Kurapati
Add support for custom PHY handling steps in platform glue code
by adding a post registration hook. This will be used for handling
PHY reset notifications on the Rockchip platform.
Tested-by: Igor Paunovic <royalnet026@gmail.com> # Orange Pi 5 Plus
Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Reviewed-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/usb/dwc3/core.c | 2 +-
drivers/usb/dwc3/core.h | 9 +++++++++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 4b7132887a03..daf5aec07fbf 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1613,7 +1613,7 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
}
}
- return 0;
+ return dwc3_post_phy_registration(dwc);
}
static int dwc3_core_init_mode(struct dwc3 *dwc)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 608daeb7ef10..e9d8df0a298e 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -995,10 +995,12 @@ struct dwc3_scratchpad_array {
* need to be passed on to glue layer
* @pre_set_role: Notify glue of role switch notifications
* @pre_run_stop: Notify run stop enable/disable information to glue
+ * @post_phy_registration: Called directly after PHY got registered
*/
struct dwc3_glue_ops {
void (*pre_set_role)(struct dwc3 *dwc, enum usb_role role);
void (*pre_run_stop)(struct dwc3 *dwc, bool is_on);
+ int (*post_phy_registration)(struct dwc3 *dwc);
};
/**
@@ -1655,6 +1657,13 @@ static inline void dwc3_pre_run_stop(struct dwc3 *dwc, bool is_on)
dwc->glue_ops->pre_run_stop(dwc, is_on);
}
+static inline int dwc3_post_phy_registration(struct dwc3 *dwc)
+{
+ if (dwc->glue_ops && dwc->glue_ops->post_phy_registration)
+ return dwc->glue_ops->post_phy_registration(dwc);
+ return 0;
+}
+
#if IS_ENABLED(CONFIG_USB_DWC3_HOST) || IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE)
int dwc3_host_init(struct dwc3 *dwc);
void dwc3_host_exit(struct dwc3 *dwc);
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v17 4/7] usb: dwc3: core: add post PHY registration hook for platform glue
@ 2026-10-16 18:43 ` Sebastian Reichel
0 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, Sebastian Reichel,
Krishna Kurapati
Add support for custom PHY handling steps in platform glue code
by adding a post registration hook. This will be used for handling
PHY reset notifications on the Rockchip platform.
Tested-by: Igor Paunovic <royalnet026@gmail.com> # Orange Pi 5 Plus
Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Reviewed-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/usb/dwc3/core.c | 2 +-
drivers/usb/dwc3/core.h | 9 +++++++++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 4b7132887a03..daf5aec07fbf 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1613,7 +1613,7 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
}
}
- return 0;
+ return dwc3_post_phy_registration(dwc);
}
static int dwc3_core_init_mode(struct dwc3 *dwc)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 608daeb7ef10..e9d8df0a298e 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -995,10 +995,12 @@ struct dwc3_scratchpad_array {
* need to be passed on to glue layer
* @pre_set_role: Notify glue of role switch notifications
* @pre_run_stop: Notify run stop enable/disable information to glue
+ * @post_phy_registration: Called directly after PHY got registered
*/
struct dwc3_glue_ops {
void (*pre_set_role)(struct dwc3 *dwc, enum usb_role role);
void (*pre_run_stop)(struct dwc3 *dwc, bool is_on);
+ int (*post_phy_registration)(struct dwc3 *dwc);
};
/**
@@ -1655,6 +1657,13 @@ static inline void dwc3_pre_run_stop(struct dwc3 *dwc, bool is_on)
dwc->glue_ops->pre_run_stop(dwc, is_on);
}
+static inline int dwc3_post_phy_registration(struct dwc3 *dwc)
+{
+ if (dwc->glue_ops && dwc->glue_ops->post_phy_registration)
+ return dwc->glue_ops->post_phy_registration(dwc);
+ return 0;
+}
+
#if IS_ENABLED(CONFIG_USB_DWC3_HOST) || IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE)
int dwc3_host_init(struct dwc3 *dwc);
void dwc3_host_exit(struct dwc3 *dwc);
--
2.53.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v17 4/7] usb: dwc3: core: add post PHY registration hook for platform glue
@ 2026-10-16 18:43 ` Sebastian Reichel
0 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, Sebastian Reichel,
Krishna Kurapati
Add support for custom PHY handling steps in platform glue code
by adding a post registration hook. This will be used for handling
PHY reset notifications on the Rockchip platform.
Tested-by: Igor Paunovic <royalnet026@gmail.com> # Orange Pi 5 Plus
Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Reviewed-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/usb/dwc3/core.c | 2 +-
drivers/usb/dwc3/core.h | 9 +++++++++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 4b7132887a03..daf5aec07fbf 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1613,7 +1613,7 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
}
}
- return 0;
+ return dwc3_post_phy_registration(dwc);
}
static int dwc3_core_init_mode(struct dwc3 *dwc)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 608daeb7ef10..e9d8df0a298e 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -995,10 +995,12 @@ struct dwc3_scratchpad_array {
* need to be passed on to glue layer
* @pre_set_role: Notify glue of role switch notifications
* @pre_run_stop: Notify run stop enable/disable information to glue
+ * @post_phy_registration: Called directly after PHY got registered
*/
struct dwc3_glue_ops {
void (*pre_set_role)(struct dwc3 *dwc, enum usb_role role);
void (*pre_run_stop)(struct dwc3 *dwc, bool is_on);
+ int (*post_phy_registration)(struct dwc3 *dwc);
};
/**
@@ -1655,6 +1657,13 @@ static inline void dwc3_pre_run_stop(struct dwc3 *dwc, bool is_on)
dwc->glue_ops->pre_run_stop(dwc, is_on);
}
+static inline int dwc3_post_phy_registration(struct dwc3 *dwc)
+{
+ if (dwc->glue_ops && dwc->glue_ops->post_phy_registration)
+ return dwc->glue_ops->post_phy_registration(dwc);
+ return 0;
+}
+
#if IS_ENABLED(CONFIG_USB_DWC3_HOST) || IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE)
int dwc3_host_init(struct dwc3 *dwc);
void dwc3_host_exit(struct dwc3 *dwc);
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v17 4/7] usb: dwc3: core: add post PHY registration hook for platform glue
2026-10-16 18:43 ` Sebastian Reichel
(?)
(?)
@ 2026-10-16 18:51 ` sashiko-bot
-1 siblings, 0 replies; 35+ messages in thread
From: sashiko-bot @ 2026-10-16 18:51 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-usb
> Add support for custom PHY handling steps in platform glue code
> by adding a post registration hook. This will be used for handling
> PHY reset notifications on the Rockchip platform.
>
> Tested-by: Igor Paunovic <royalnet026@gmail.com> # Orange Pi 5 Plus
> Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> Reviewed-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-b4-rockchip-dwc3-rockchip-glue-v17-0-876f96893e2f@collabora.com?part=4
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v17 5/7] usb: dwc3: rockchip: add USBDP PHY reset quirk
2026-10-16 18:43 ` Sebastian Reichel
(?)
@ 2026-10-16 18:43 ` Sebastian Reichel
-1 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, 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 a Rockchip USBDP specific quirk, which will
assert PHYSOFTRST before the actual PHY is disabled and will deassert it
once the PHY returns.
Note, that the same platforms also use DWC3 with a different PHY
(rockchip-naneng-combphy), so the PHY calls are protected by a
driver name check.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/usb/dwc3/Kconfig | 1 +
drivers/usb/dwc3/dwc3-rockchip.c | 135 ++++++++++++++++++++++++++++++++++++++-
drivers/usb/dwc3/trace.c | 3 +
3 files changed, 138 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 3c120ea9746d..c86c1ac293bf 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -193,6 +193,7 @@ config USB_DWC3_OCTEON
config USB_DWC3_ROCKCHIP
tristate "Rockchip DWC3 Platform Driver"
depends on ARCH_ROCKCHIP || COMPILE_TEST
+ depends on PHY_ROCKCHIP_USBDP || !PHY_ROCKCHIP_USBDP
depends on OF
default USB_DWC3
help
diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c
index 4129aefd6a8b..1f32150caa09 100644
--- a/drivers/usb/dwc3/dwc3-rockchip.c
+++ b/drivers/usb/dwc3/dwc3-rockchip.c
@@ -2,11 +2,144 @@
/* Copyright (c) 2026, Collabora Ltd. */
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/phy/phy.h>
+#include <linux/phy/rockchip-usbdp.h>
#include <linux/pm_runtime.h>
#include "glue.h"
+#include "io.h"
+
+struct dwc3_rockchip;
+
+/**
+ * struct dwc3_rk_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_rk_phy_nb {
+ struct notifier_block nb;
+ struct dwc3_rockchip *dwc_rk;
+ u8 port_index;
+};
struct dwc3_rockchip {
struct dwc3 dwc;
+ u8 phy_reset_active;
+};
+
+static int dwc3_rockchip_phy_pre_reset(struct dwc3_rockchip *dwc_rk, int port)
+{
+ struct dwc3 *dwc = &dwc_rk->dwc;
+ unsigned long flags;
+ u32 reg;
+ int ret;
+
+ /*
+ * If already suspended, the resume path will reinit GUSB3PIPECTL
+ * via dwc3_core_init(). A forced resume is not possible as that
+ * would call phy_init() resulting in a deadlock. Due to the
+ * phy_init() in the resume path there is also no need to block
+ * async RPM resume on our side, since the PHY synchronizes it
+ * for us.
+ *
+ * pm_runtime_get_if_active() returns 0 when suspended (skip),
+ * 1 when active (ref held), or -EINVAL when PM is disabled
+ * (device always active). In the -EINVAL case PM ref counting
+ * is a no-op, so the unconditional put in POST_RESET is safe.
+ */
+ ret = pm_runtime_get_if_active(dwc->dev);
+ if (!ret)
+ return ret;
+
+ /*
+ * 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);
+ dwc_rk->phy_reset_active |= BIT(port);
+ reg = dwc3_readl(dwc, DWC3_GUSB3PIPECTL(port));
+ reg |= DWC3_GUSB3PIPECTL_PHYSOFTRST;
+ dwc3_writel(dwc, DWC3_GUSB3PIPECTL(port), reg);
+ spin_unlock_irqrestore(&dwc->lock, flags);
+
+ return 0;
+}
+
+static int dwc3_rockchip_phy_post_reset(struct dwc3_rockchip *dwc_rk, int port)
+{
+ struct dwc3 *dwc = &dwc_rk->dwc;
+ unsigned long flags;
+ u32 reg;
+
+ spin_lock_irqsave(&dwc->lock, flags);
+ if (!(dwc_rk->phy_reset_active & BIT(port))) {
+ spin_unlock_irqrestore(&dwc->lock, flags);
+ return 0;
+ }
+
+ dwc_rk->phy_reset_active &= ~BIT(port);
+
+ /* Restore normal operation */
+ 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);
+
+ return 0;
+}
+
+static int dwc3_rockchip_phy_reset(void *data, int port, bool pre)
+{
+ struct dwc3_rockchip *dwc_rk = data;
+
+ if (pre)
+ return dwc3_rockchip_phy_pre_reset(dwc_rk, port);
+ else
+ return dwc3_rockchip_phy_post_reset(dwc_rk, port);
+}
+
+static void dwc3_rk_phy_unregister_notifiers(void *data)
+{
+ struct dwc3_rockchip *dwc_rk = data;
+ struct dwc3 *dwc = &dwc_rk->dwc;
+ int i;
+
+ for (i = 0; i < dwc->num_usb3_ports; i++) {
+ struct phy *phy = dwc->usb3_generic_phy[i];
+
+ if (!strcmp(dev_driver_string(phy->dev.parent), "rockchip-usbdp-phy"))
+ rk_udphy_unregister_reset_handler(dwc->usb3_generic_phy[i]);
+ }
+
+ /* Release any PM references from in-flight resets */
+ for (i = 0; i < dwc->num_usb3_ports; i++) {
+ if (dwc_rk->phy_reset_active & BIT(i))
+ pm_runtime_put_autosuspend(dwc->dev);
+ }
+ dwc_rk->phy_reset_active = 0;
+}
+
+static int dwc3_rk_phy_register_reset_handler(struct dwc3 *dwc)
+{
+ struct dwc3_rockchip *dwc_rk = container_of(dwc, struct dwc3_rockchip, dwc);
+ int i;
+
+ for (i = 0; i < dwc->num_usb3_ports; i++) {
+ struct phy *phy = dwc->usb3_generic_phy[i];
+
+ if (!strcmp(dev_driver_string(phy->dev.parent), "rockchip-usbdp-phy"))
+ rk_udphy_register_reset_handler(phy, dwc3_rockchip_phy_reset, dwc_rk, i);
+ }
+
+ return devm_add_action_or_reset(dwc->dev, dwc3_rk_phy_unregister_notifiers, dwc_rk);
+}
+
+static struct dwc3_glue_ops dwc3_rockchip_glue_ops = {
+ .post_phy_registration = dwc3_rk_phy_register_reset_handler,
};
static int dwc3_rockchip_probe(struct platform_device *pdev)
@@ -26,7 +159,7 @@ static int dwc3_rockchip_probe(struct platform_device *pdev)
return -ENOMEM;
dwc_rk->dwc.dev = &pdev->dev;
- dwc_rk->dwc.glue_ops = NULL;
+ dwc_rk->dwc.glue_ops = &dwc3_rockchip_glue_ops;
probe_data.dwc = &dwc_rk->dwc;
probe_data.res = res;
diff --git a/drivers/usb/dwc3/trace.c b/drivers/usb/dwc3/trace.c
index 088995885678..8c4e2a7b142e 100644
--- a/drivers/usb/dwc3/trace.c
+++ b/drivers/usb/dwc3/trace.c
@@ -9,3 +9,6 @@
#define CREATE_TRACE_POINTS
#include "trace.h"
+
+EXPORT_TRACEPOINT_SYMBOL_GPL(dwc3_readl);
+EXPORT_TRACEPOINT_SYMBOL_GPL(dwc3_writel);
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v17 5/7] usb: dwc3: rockchip: add USBDP PHY reset quirk
@ 2026-10-16 18:43 ` Sebastian Reichel
0 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, 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 a Rockchip USBDP specific quirk, which will
assert PHYSOFTRST before the actual PHY is disabled and will deassert it
once the PHY returns.
Note, that the same platforms also use DWC3 with a different PHY
(rockchip-naneng-combphy), so the PHY calls are protected by a
driver name check.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/usb/dwc3/Kconfig | 1 +
drivers/usb/dwc3/dwc3-rockchip.c | 135 ++++++++++++++++++++++++++++++++++++++-
drivers/usb/dwc3/trace.c | 3 +
3 files changed, 138 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 3c120ea9746d..c86c1ac293bf 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -193,6 +193,7 @@ config USB_DWC3_OCTEON
config USB_DWC3_ROCKCHIP
tristate "Rockchip DWC3 Platform Driver"
depends on ARCH_ROCKCHIP || COMPILE_TEST
+ depends on PHY_ROCKCHIP_USBDP || !PHY_ROCKCHIP_USBDP
depends on OF
default USB_DWC3
help
diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c
index 4129aefd6a8b..1f32150caa09 100644
--- a/drivers/usb/dwc3/dwc3-rockchip.c
+++ b/drivers/usb/dwc3/dwc3-rockchip.c
@@ -2,11 +2,144 @@
/* Copyright (c) 2026, Collabora Ltd. */
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/phy/phy.h>
+#include <linux/phy/rockchip-usbdp.h>
#include <linux/pm_runtime.h>
#include "glue.h"
+#include "io.h"
+
+struct dwc3_rockchip;
+
+/**
+ * struct dwc3_rk_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_rk_phy_nb {
+ struct notifier_block nb;
+ struct dwc3_rockchip *dwc_rk;
+ u8 port_index;
+};
struct dwc3_rockchip {
struct dwc3 dwc;
+ u8 phy_reset_active;
+};
+
+static int dwc3_rockchip_phy_pre_reset(struct dwc3_rockchip *dwc_rk, int port)
+{
+ struct dwc3 *dwc = &dwc_rk->dwc;
+ unsigned long flags;
+ u32 reg;
+ int ret;
+
+ /*
+ * If already suspended, the resume path will reinit GUSB3PIPECTL
+ * via dwc3_core_init(). A forced resume is not possible as that
+ * would call phy_init() resulting in a deadlock. Due to the
+ * phy_init() in the resume path there is also no need to block
+ * async RPM resume on our side, since the PHY synchronizes it
+ * for us.
+ *
+ * pm_runtime_get_if_active() returns 0 when suspended (skip),
+ * 1 when active (ref held), or -EINVAL when PM is disabled
+ * (device always active). In the -EINVAL case PM ref counting
+ * is a no-op, so the unconditional put in POST_RESET is safe.
+ */
+ ret = pm_runtime_get_if_active(dwc->dev);
+ if (!ret)
+ return ret;
+
+ /*
+ * 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);
+ dwc_rk->phy_reset_active |= BIT(port);
+ reg = dwc3_readl(dwc, DWC3_GUSB3PIPECTL(port));
+ reg |= DWC3_GUSB3PIPECTL_PHYSOFTRST;
+ dwc3_writel(dwc, DWC3_GUSB3PIPECTL(port), reg);
+ spin_unlock_irqrestore(&dwc->lock, flags);
+
+ return 0;
+}
+
+static int dwc3_rockchip_phy_post_reset(struct dwc3_rockchip *dwc_rk, int port)
+{
+ struct dwc3 *dwc = &dwc_rk->dwc;
+ unsigned long flags;
+ u32 reg;
+
+ spin_lock_irqsave(&dwc->lock, flags);
+ if (!(dwc_rk->phy_reset_active & BIT(port))) {
+ spin_unlock_irqrestore(&dwc->lock, flags);
+ return 0;
+ }
+
+ dwc_rk->phy_reset_active &= ~BIT(port);
+
+ /* Restore normal operation */
+ 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);
+
+ return 0;
+}
+
+static int dwc3_rockchip_phy_reset(void *data, int port, bool pre)
+{
+ struct dwc3_rockchip *dwc_rk = data;
+
+ if (pre)
+ return dwc3_rockchip_phy_pre_reset(dwc_rk, port);
+ else
+ return dwc3_rockchip_phy_post_reset(dwc_rk, port);
+}
+
+static void dwc3_rk_phy_unregister_notifiers(void *data)
+{
+ struct dwc3_rockchip *dwc_rk = data;
+ struct dwc3 *dwc = &dwc_rk->dwc;
+ int i;
+
+ for (i = 0; i < dwc->num_usb3_ports; i++) {
+ struct phy *phy = dwc->usb3_generic_phy[i];
+
+ if (!strcmp(dev_driver_string(phy->dev.parent), "rockchip-usbdp-phy"))
+ rk_udphy_unregister_reset_handler(dwc->usb3_generic_phy[i]);
+ }
+
+ /* Release any PM references from in-flight resets */
+ for (i = 0; i < dwc->num_usb3_ports; i++) {
+ if (dwc_rk->phy_reset_active & BIT(i))
+ pm_runtime_put_autosuspend(dwc->dev);
+ }
+ dwc_rk->phy_reset_active = 0;
+}
+
+static int dwc3_rk_phy_register_reset_handler(struct dwc3 *dwc)
+{
+ struct dwc3_rockchip *dwc_rk = container_of(dwc, struct dwc3_rockchip, dwc);
+ int i;
+
+ for (i = 0; i < dwc->num_usb3_ports; i++) {
+ struct phy *phy = dwc->usb3_generic_phy[i];
+
+ if (!strcmp(dev_driver_string(phy->dev.parent), "rockchip-usbdp-phy"))
+ rk_udphy_register_reset_handler(phy, dwc3_rockchip_phy_reset, dwc_rk, i);
+ }
+
+ return devm_add_action_or_reset(dwc->dev, dwc3_rk_phy_unregister_notifiers, dwc_rk);
+}
+
+static struct dwc3_glue_ops dwc3_rockchip_glue_ops = {
+ .post_phy_registration = dwc3_rk_phy_register_reset_handler,
};
static int dwc3_rockchip_probe(struct platform_device *pdev)
@@ -26,7 +159,7 @@ static int dwc3_rockchip_probe(struct platform_device *pdev)
return -ENOMEM;
dwc_rk->dwc.dev = &pdev->dev;
- dwc_rk->dwc.glue_ops = NULL;
+ dwc_rk->dwc.glue_ops = &dwc3_rockchip_glue_ops;
probe_data.dwc = &dwc_rk->dwc;
probe_data.res = res;
diff --git a/drivers/usb/dwc3/trace.c b/drivers/usb/dwc3/trace.c
index 088995885678..8c4e2a7b142e 100644
--- a/drivers/usb/dwc3/trace.c
+++ b/drivers/usb/dwc3/trace.c
@@ -9,3 +9,6 @@
#define CREATE_TRACE_POINTS
#include "trace.h"
+
+EXPORT_TRACEPOINT_SYMBOL_GPL(dwc3_readl);
+EXPORT_TRACEPOINT_SYMBOL_GPL(dwc3_writel);
--
2.53.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v17 5/7] usb: dwc3: rockchip: add USBDP PHY reset quirk
@ 2026-10-16 18:43 ` Sebastian Reichel
0 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, 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 a Rockchip USBDP specific quirk, which will
assert PHYSOFTRST before the actual PHY is disabled and will deassert it
once the PHY returns.
Note, that the same platforms also use DWC3 with a different PHY
(rockchip-naneng-combphy), so the PHY calls are protected by a
driver name check.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/usb/dwc3/Kconfig | 1 +
drivers/usb/dwc3/dwc3-rockchip.c | 135 ++++++++++++++++++++++++++++++++++++++-
drivers/usb/dwc3/trace.c | 3 +
3 files changed, 138 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 3c120ea9746d..c86c1ac293bf 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -193,6 +193,7 @@ config USB_DWC3_OCTEON
config USB_DWC3_ROCKCHIP
tristate "Rockchip DWC3 Platform Driver"
depends on ARCH_ROCKCHIP || COMPILE_TEST
+ depends on PHY_ROCKCHIP_USBDP || !PHY_ROCKCHIP_USBDP
depends on OF
default USB_DWC3
help
diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c
index 4129aefd6a8b..1f32150caa09 100644
--- a/drivers/usb/dwc3/dwc3-rockchip.c
+++ b/drivers/usb/dwc3/dwc3-rockchip.c
@@ -2,11 +2,144 @@
/* Copyright (c) 2026, Collabora Ltd. */
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/phy/phy.h>
+#include <linux/phy/rockchip-usbdp.h>
#include <linux/pm_runtime.h>
#include "glue.h"
+#include "io.h"
+
+struct dwc3_rockchip;
+
+/**
+ * struct dwc3_rk_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_rk_phy_nb {
+ struct notifier_block nb;
+ struct dwc3_rockchip *dwc_rk;
+ u8 port_index;
+};
struct dwc3_rockchip {
struct dwc3 dwc;
+ u8 phy_reset_active;
+};
+
+static int dwc3_rockchip_phy_pre_reset(struct dwc3_rockchip *dwc_rk, int port)
+{
+ struct dwc3 *dwc = &dwc_rk->dwc;
+ unsigned long flags;
+ u32 reg;
+ int ret;
+
+ /*
+ * If already suspended, the resume path will reinit GUSB3PIPECTL
+ * via dwc3_core_init(). A forced resume is not possible as that
+ * would call phy_init() resulting in a deadlock. Due to the
+ * phy_init() in the resume path there is also no need to block
+ * async RPM resume on our side, since the PHY synchronizes it
+ * for us.
+ *
+ * pm_runtime_get_if_active() returns 0 when suspended (skip),
+ * 1 when active (ref held), or -EINVAL when PM is disabled
+ * (device always active). In the -EINVAL case PM ref counting
+ * is a no-op, so the unconditional put in POST_RESET is safe.
+ */
+ ret = pm_runtime_get_if_active(dwc->dev);
+ if (!ret)
+ return ret;
+
+ /*
+ * 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);
+ dwc_rk->phy_reset_active |= BIT(port);
+ reg = dwc3_readl(dwc, DWC3_GUSB3PIPECTL(port));
+ reg |= DWC3_GUSB3PIPECTL_PHYSOFTRST;
+ dwc3_writel(dwc, DWC3_GUSB3PIPECTL(port), reg);
+ spin_unlock_irqrestore(&dwc->lock, flags);
+
+ return 0;
+}
+
+static int dwc3_rockchip_phy_post_reset(struct dwc3_rockchip *dwc_rk, int port)
+{
+ struct dwc3 *dwc = &dwc_rk->dwc;
+ unsigned long flags;
+ u32 reg;
+
+ spin_lock_irqsave(&dwc->lock, flags);
+ if (!(dwc_rk->phy_reset_active & BIT(port))) {
+ spin_unlock_irqrestore(&dwc->lock, flags);
+ return 0;
+ }
+
+ dwc_rk->phy_reset_active &= ~BIT(port);
+
+ /* Restore normal operation */
+ 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);
+
+ return 0;
+}
+
+static int dwc3_rockchip_phy_reset(void *data, int port, bool pre)
+{
+ struct dwc3_rockchip *dwc_rk = data;
+
+ if (pre)
+ return dwc3_rockchip_phy_pre_reset(dwc_rk, port);
+ else
+ return dwc3_rockchip_phy_post_reset(dwc_rk, port);
+}
+
+static void dwc3_rk_phy_unregister_notifiers(void *data)
+{
+ struct dwc3_rockchip *dwc_rk = data;
+ struct dwc3 *dwc = &dwc_rk->dwc;
+ int i;
+
+ for (i = 0; i < dwc->num_usb3_ports; i++) {
+ struct phy *phy = dwc->usb3_generic_phy[i];
+
+ if (!strcmp(dev_driver_string(phy->dev.parent), "rockchip-usbdp-phy"))
+ rk_udphy_unregister_reset_handler(dwc->usb3_generic_phy[i]);
+ }
+
+ /* Release any PM references from in-flight resets */
+ for (i = 0; i < dwc->num_usb3_ports; i++) {
+ if (dwc_rk->phy_reset_active & BIT(i))
+ pm_runtime_put_autosuspend(dwc->dev);
+ }
+ dwc_rk->phy_reset_active = 0;
+}
+
+static int dwc3_rk_phy_register_reset_handler(struct dwc3 *dwc)
+{
+ struct dwc3_rockchip *dwc_rk = container_of(dwc, struct dwc3_rockchip, dwc);
+ int i;
+
+ for (i = 0; i < dwc->num_usb3_ports; i++) {
+ struct phy *phy = dwc->usb3_generic_phy[i];
+
+ if (!strcmp(dev_driver_string(phy->dev.parent), "rockchip-usbdp-phy"))
+ rk_udphy_register_reset_handler(phy, dwc3_rockchip_phy_reset, dwc_rk, i);
+ }
+
+ return devm_add_action_or_reset(dwc->dev, dwc3_rk_phy_unregister_notifiers, dwc_rk);
+}
+
+static struct dwc3_glue_ops dwc3_rockchip_glue_ops = {
+ .post_phy_registration = dwc3_rk_phy_register_reset_handler,
};
static int dwc3_rockchip_probe(struct platform_device *pdev)
@@ -26,7 +159,7 @@ static int dwc3_rockchip_probe(struct platform_device *pdev)
return -ENOMEM;
dwc_rk->dwc.dev = &pdev->dev;
- dwc_rk->dwc.glue_ops = NULL;
+ dwc_rk->dwc.glue_ops = &dwc3_rockchip_glue_ops;
probe_data.dwc = &dwc_rk->dwc;
probe_data.res = res;
diff --git a/drivers/usb/dwc3/trace.c b/drivers/usb/dwc3/trace.c
index 088995885678..8c4e2a7b142e 100644
--- a/drivers/usb/dwc3/trace.c
+++ b/drivers/usb/dwc3/trace.c
@@ -9,3 +9,6 @@
#define CREATE_TRACE_POINTS
#include "trace.h"
+
+EXPORT_TRACEPOINT_SYMBOL_GPL(dwc3_readl);
+EXPORT_TRACEPOINT_SYMBOL_GPL(dwc3_writel);
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v17 5/7] usb: dwc3: rockchip: add USBDP PHY reset quirk
2026-10-16 18:43 ` Sebastian Reichel
@ 2026-10-16 18:55 ` sashiko-bot
-1 siblings, 0 replies; 35+ messages in thread
From: sashiko-bot @ 2026-10-16 18:55 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-usb, linux-phy, vkoul, neil.armstrong, olteanv
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] usb: dwc3: NULL pointer dereference on missing generic PHY
- [High] usb: dwc3: rockchip: Runtime PM usage count underflow on PHY reset
- [High] build: missing stubs for rk_udphy_*_reset_handler causes linker error
- [Low] usb: dwc3: unused struct dwc3_rk_phy_nb
--
commit 91514aa6803f514c30a280950070ac9faec72092
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
usb: dwc3: rockchip: add USBDP PHY reset quirk
This patch introduces a Rockchip USBDP-specific quirk to assert PHYSOFTRST
before disabling the external PHY and deassert it once the PHY returns. This
prevents the DWC3 IP from corrupting the PHY's internal state when the mode
changes on RK3588 and RK3576 platforms.
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 3c120ea9746d9..c86c1ac293bfa 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -193,6 +193,7 @@ config USB_DWC3_OCTEON
> config USB_DWC3_ROCKCHIP
> tristate "Rockchip DWC3 Platform Driver"
> depends on ARCH_ROCKCHIP || COMPILE_TEST
> + depends on PHY_ROCKCHIP_USBDP || !PHY_ROCKCHIP_USBDP
[Severity: High]
If CONFIG_PHY_ROCKCHIP_USBDP is disabled, `!PHY_ROCKCHIP_USBDP` evaluates to y,
which permits USB_DWC3_ROCKCHIP to be built. Since
include/linux/phy/rockchip-usbdp.h does not provide static inline dummy stubs
for the rk_udphy reset handlers, will this cause a linker error due to missing
symbols?
> depends on OF
> default USB_DWC3
> help
> diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c
> index 4129aefd6a8b0..1f32150caa090 100644
> --- a/drivers/usb/dwc3/dwc3-rockchip.c
> +++ b/drivers/usb/dwc3/dwc3-rockchip.c
> @@ -2,11 +2,144 @@
> /* Copyright (c) 2026, Collabora Ltd. */
> #include <linux/module.h>
> #include <linux/platform_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/phy/rockchip-usbdp.h>
> #include <linux/pm_runtime.h>
> #include "glue.h"
> +#include "io.h"
> +
> +struct dwc3_rockchip;
> +
> +/**
> + * struct dwc3_rk_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_rk_phy_nb {
> + struct notifier_block nb;
> + struct dwc3_rockchip *dwc_rk;
> + u8 port_index;
> +};
[Severity: Low]
Is this struct dwc3_rk_phy_nb declaration still needed? It appears to be
unused anywhere in the driver, possibly left over from an earlier version
that used notifier blocks.
> +
> struct dwc3_rockchip {
> struct dwc3 dwc;
> + u8 phy_reset_active;
> +};
> +
> +static int dwc3_rockchip_phy_pre_reset(struct dwc3_rockchip *dwc_rk, int port)
> +{
> + struct dwc3 *dwc = &dwc_rk->dwc;
> + unsigned long flags;
> + u32 reg;
> + int ret;
> +
> + /*
> + * If already suspended, the resume path will reinit GUSB3PIPECTL
> + * via dwc3_core_init(). A forced resume is not possible as that
> + * would call phy_init() resulting in a deadlock. Due to the
> + * phy_init() in the resume path there is also no need to block
> + * async RPM resume on our side, since the PHY synchronizes it
> + * for us.
> + *
> + * pm_runtime_get_if_active() returns 0 when suspended (skip),
> + * 1 when active (ref held), or -EINVAL when PM is disabled
> + * (device always active). In the -EINVAL case PM ref counting
> + * is a no-op, so the unconditional put in POST_RESET is safe.
> + */
> + ret = pm_runtime_get_if_active(dwc->dev);
> + if (!ret)
> + return ret;
[Severity: High]
If runtime PM is disabled for the device (e.g., during suspend or removal),
pm_runtime_get_if_active() returns -EINVAL and does not increment the usage
count.
Because `if (!ret)` evaluates to false for -EINVAL, the driver proceeds with
the reset. However, the post-reset and unregister paths unconditionally drop
the usage count via pm_runtime_put_autosuspend(), which could lead to a
mismatched decrement and trigger a "Runtime PM usage count underflow!" warning.
[ ... ]
> +static int dwc3_rockchip_phy_post_reset(struct dwc3_rockchip *dwc_rk, int port)
> +{
> + struct dwc3 *dwc = &dwc_rk->dwc;
> + unsigned long flags;
> + u32 reg;
> +
> + spin_lock_irqsave(&dwc->lock, flags);
> + if (!(dwc_rk->phy_reset_active & BIT(port))) {
> + spin_unlock_irqrestore(&dwc->lock, flags);
> + return 0;
> + }
> +
> + dwc_rk->phy_reset_active &= ~BIT(port);
> +
> + /* Restore normal operation */
> + 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);
[Severity: High]
Does this unconditionally drop the PM usage count even if the initial
pm_runtime_get_if_active() call returned -EINVAL without incrementing it?
[ ... ]
> +static void dwc3_rk_phy_unregister_notifiers(void *data)
> +{
> + struct dwc3_rockchip *dwc_rk = data;
> + struct dwc3 *dwc = &dwc_rk->dwc;
> + int i;
> +
> + for (i = 0; i < dwc->num_usb3_ports; i++) {
> + struct phy *phy = dwc->usb3_generic_phy[i];
> +
> + if (!strcmp(dev_driver_string(phy->dev.parent), "rockchip-usbdp-phy"))
[Severity: High]
Is it possible for `phy` to be NULL here?
If a USB3 port is not configured with a PHY in the device tree, the DWC3
core driver leaves `dwc->usb3_generic_phy[i]` as NULL. Evaluating
`phy->dev.parent` without a valid pointer check would cause an unconditional
NULL pointer dereference during unbind or teardown.
> + rk_udphy_unregister_reset_handler(dwc->usb3_generic_phy[i]);
> + }
> +
> + /* Release any PM references from in-flight resets */
> + for (i = 0; i < dwc->num_usb3_ports; i++) {
> + if (dwc_rk->phy_reset_active & BIT(i))
> + pm_runtime_put_autosuspend(dwc->dev);
> + }
> + dwc_rk->phy_reset_active = 0;
> +}
> +
> +static int dwc3_rk_phy_register_reset_handler(struct dwc3 *dwc)
> +{
> + struct dwc3_rockchip *dwc_rk = container_of(dwc, struct dwc3_rockchip, dwc);
> + int i;
> +
> + for (i = 0; i < dwc->num_usb3_ports; i++) {
> + struct phy *phy = dwc->usb3_generic_phy[i];
> +
> + if (!strcmp(dev_driver_string(phy->dev.parent), "rockchip-usbdp-phy"))
[Severity: High]
Could this also dereference a NULL pointer during probe if a USB3 port lacks
a defined PHY?
> + rk_udphy_register_reset_handler(phy, dwc3_rockchip_phy_reset, dwc_rk, i);
[Severity: High]
Does this direct call to the PHY driver API require an IS_ENABLED() check?
Since the Kconfig permits USB_DWC3_ROCKCHIP to be built even when
PHY_ROCKCHIP_USBDP is disabled, this could cause a build failure during
linking if the PHY driver lacks dummy stubs for these functions.
> + }
> +
> + return devm_add_action_or_reset(dwc->dev, dwc3_rk_phy_unregister_notifiers, dwc_rk);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-b4-rockchip-dwc3-rockchip-glue-v17-0-876f96893e2f@collabora.com?part=5
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v17 5/7] usb: dwc3: rockchip: add USBDP PHY reset quirk
@ 2026-10-16 18:55 ` sashiko-bot
0 siblings, 0 replies; 35+ messages in thread
From: sashiko-bot @ 2026-10-16 18:55 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-usb, linux-phy, vkoul, neil.armstrong, olteanv
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] usb: dwc3: NULL pointer dereference on missing generic PHY
- [High] usb: dwc3: rockchip: Runtime PM usage count underflow on PHY reset
- [High] build: missing stubs for rk_udphy_*_reset_handler causes linker error
- [Low] usb: dwc3: unused struct dwc3_rk_phy_nb
--
commit 91514aa6803f514c30a280950070ac9faec72092
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
usb: dwc3: rockchip: add USBDP PHY reset quirk
This patch introduces a Rockchip USBDP-specific quirk to assert PHYSOFTRST
before disabling the external PHY and deassert it once the PHY returns. This
prevents the DWC3 IP from corrupting the PHY's internal state when the mode
changes on RK3588 and RK3576 platforms.
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 3c120ea9746d9..c86c1ac293bfa 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -193,6 +193,7 @@ config USB_DWC3_OCTEON
> config USB_DWC3_ROCKCHIP
> tristate "Rockchip DWC3 Platform Driver"
> depends on ARCH_ROCKCHIP || COMPILE_TEST
> + depends on PHY_ROCKCHIP_USBDP || !PHY_ROCKCHIP_USBDP
[Severity: High]
If CONFIG_PHY_ROCKCHIP_USBDP is disabled, `!PHY_ROCKCHIP_USBDP` evaluates to y,
which permits USB_DWC3_ROCKCHIP to be built. Since
include/linux/phy/rockchip-usbdp.h does not provide static inline dummy stubs
for the rk_udphy reset handlers, will this cause a linker error due to missing
symbols?
> depends on OF
> default USB_DWC3
> help
> diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c
> index 4129aefd6a8b0..1f32150caa090 100644
> --- a/drivers/usb/dwc3/dwc3-rockchip.c
> +++ b/drivers/usb/dwc3/dwc3-rockchip.c
> @@ -2,11 +2,144 @@
> /* Copyright (c) 2026, Collabora Ltd. */
> #include <linux/module.h>
> #include <linux/platform_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/phy/rockchip-usbdp.h>
> #include <linux/pm_runtime.h>
> #include "glue.h"
> +#include "io.h"
> +
> +struct dwc3_rockchip;
> +
> +/**
> + * struct dwc3_rk_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_rk_phy_nb {
> + struct notifier_block nb;
> + struct dwc3_rockchip *dwc_rk;
> + u8 port_index;
> +};
[Severity: Low]
Is this struct dwc3_rk_phy_nb declaration still needed? It appears to be
unused anywhere in the driver, possibly left over from an earlier version
that used notifier blocks.
> +
> struct dwc3_rockchip {
> struct dwc3 dwc;
> + u8 phy_reset_active;
> +};
> +
> +static int dwc3_rockchip_phy_pre_reset(struct dwc3_rockchip *dwc_rk, int port)
> +{
> + struct dwc3 *dwc = &dwc_rk->dwc;
> + unsigned long flags;
> + u32 reg;
> + int ret;
> +
> + /*
> + * If already suspended, the resume path will reinit GUSB3PIPECTL
> + * via dwc3_core_init(). A forced resume is not possible as that
> + * would call phy_init() resulting in a deadlock. Due to the
> + * phy_init() in the resume path there is also no need to block
> + * async RPM resume on our side, since the PHY synchronizes it
> + * for us.
> + *
> + * pm_runtime_get_if_active() returns 0 when suspended (skip),
> + * 1 when active (ref held), or -EINVAL when PM is disabled
> + * (device always active). In the -EINVAL case PM ref counting
> + * is a no-op, so the unconditional put in POST_RESET is safe.
> + */
> + ret = pm_runtime_get_if_active(dwc->dev);
> + if (!ret)
> + return ret;
[Severity: High]
If runtime PM is disabled for the device (e.g., during suspend or removal),
pm_runtime_get_if_active() returns -EINVAL and does not increment the usage
count.
Because `if (!ret)` evaluates to false for -EINVAL, the driver proceeds with
the reset. However, the post-reset and unregister paths unconditionally drop
the usage count via pm_runtime_put_autosuspend(), which could lead to a
mismatched decrement and trigger a "Runtime PM usage count underflow!" warning.
[ ... ]
> +static int dwc3_rockchip_phy_post_reset(struct dwc3_rockchip *dwc_rk, int port)
> +{
> + struct dwc3 *dwc = &dwc_rk->dwc;
> + unsigned long flags;
> + u32 reg;
> +
> + spin_lock_irqsave(&dwc->lock, flags);
> + if (!(dwc_rk->phy_reset_active & BIT(port))) {
> + spin_unlock_irqrestore(&dwc->lock, flags);
> + return 0;
> + }
> +
> + dwc_rk->phy_reset_active &= ~BIT(port);
> +
> + /* Restore normal operation */
> + 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);
[Severity: High]
Does this unconditionally drop the PM usage count even if the initial
pm_runtime_get_if_active() call returned -EINVAL without incrementing it?
[ ... ]
> +static void dwc3_rk_phy_unregister_notifiers(void *data)
> +{
> + struct dwc3_rockchip *dwc_rk = data;
> + struct dwc3 *dwc = &dwc_rk->dwc;
> + int i;
> +
> + for (i = 0; i < dwc->num_usb3_ports; i++) {
> + struct phy *phy = dwc->usb3_generic_phy[i];
> +
> + if (!strcmp(dev_driver_string(phy->dev.parent), "rockchip-usbdp-phy"))
[Severity: High]
Is it possible for `phy` to be NULL here?
If a USB3 port is not configured with a PHY in the device tree, the DWC3
core driver leaves `dwc->usb3_generic_phy[i]` as NULL. Evaluating
`phy->dev.parent` without a valid pointer check would cause an unconditional
NULL pointer dereference during unbind or teardown.
> + rk_udphy_unregister_reset_handler(dwc->usb3_generic_phy[i]);
> + }
> +
> + /* Release any PM references from in-flight resets */
> + for (i = 0; i < dwc->num_usb3_ports; i++) {
> + if (dwc_rk->phy_reset_active & BIT(i))
> + pm_runtime_put_autosuspend(dwc->dev);
> + }
> + dwc_rk->phy_reset_active = 0;
> +}
> +
> +static int dwc3_rk_phy_register_reset_handler(struct dwc3 *dwc)
> +{
> + struct dwc3_rockchip *dwc_rk = container_of(dwc, struct dwc3_rockchip, dwc);
> + int i;
> +
> + for (i = 0; i < dwc->num_usb3_ports; i++) {
> + struct phy *phy = dwc->usb3_generic_phy[i];
> +
> + if (!strcmp(dev_driver_string(phy->dev.parent), "rockchip-usbdp-phy"))
[Severity: High]
Could this also dereference a NULL pointer during probe if a USB3 port lacks
a defined PHY?
> + rk_udphy_register_reset_handler(phy, dwc3_rockchip_phy_reset, dwc_rk, i);
[Severity: High]
Does this direct call to the PHY driver API require an IS_ENABLED() check?
Since the Kconfig permits USB_DWC3_ROCKCHIP to be built even when
PHY_ROCKCHIP_USBDP is disabled, this could cause a build failure during
linking if the PHY driver lacks dummy stubs for these functions.
> + }
> +
> + return devm_add_action_or_reset(dwc->dev, dwc3_rk_phy_unregister_notifiers, dwc_rk);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-b4-rockchip-dwc3-rockchip-glue-v17-0-876f96893e2f@collabora.com?part=5
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v17 6/7] usb: gadget: define stub for usb_udc_vbus_handler
2026-10-16 18:43 ` Sebastian Reichel
(?)
@ 2026-10-16 18:43 ` Sebastian Reichel
-1 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, Sebastian Reichel
Platform drivers with optional gadget mode support may want to
call usb_udc_vbus_handler to properly handle a cable being
re-plugged.
Ensure these drivers can call the function without a hard dependency on
CONFIG_USB_GADGET or having the ifdef logic in the driver.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
include/linux/usb/gadget.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 8285b19a25e0..551e71b6eda0 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -979,7 +979,12 @@ extern int usb_gadget_ep_match_desc(struct usb_gadget *gadget,
/*-------------------------------------------------------------------------*/
/* utility to update vbus status for udc core, it may be scheduled */
+#if IS_ENABLED(CONFIG_USB_GADGET)
extern void usb_udc_vbus_handler(struct usb_gadget *gadget, bool status);
+#else
+static inline void usb_udc_vbus_handler(struct usb_gadget *gadget, bool status)
+{}
+#endif
/*-------------------------------------------------------------------------*/
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v17 6/7] usb: gadget: define stub for usb_udc_vbus_handler
@ 2026-10-16 18:43 ` Sebastian Reichel
0 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, Sebastian Reichel
Platform drivers with optional gadget mode support may want to
call usb_udc_vbus_handler to properly handle a cable being
re-plugged.
Ensure these drivers can call the function without a hard dependency on
CONFIG_USB_GADGET or having the ifdef logic in the driver.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
include/linux/usb/gadget.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 8285b19a25e0..551e71b6eda0 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -979,7 +979,12 @@ extern int usb_gadget_ep_match_desc(struct usb_gadget *gadget,
/*-------------------------------------------------------------------------*/
/* utility to update vbus status for udc core, it may be scheduled */
+#if IS_ENABLED(CONFIG_USB_GADGET)
extern void usb_udc_vbus_handler(struct usb_gadget *gadget, bool status);
+#else
+static inline void usb_udc_vbus_handler(struct usb_gadget *gadget, bool status)
+{}
+#endif
/*-------------------------------------------------------------------------*/
--
2.53.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v17 6/7] usb: gadget: define stub for usb_udc_vbus_handler
@ 2026-10-16 18:43 ` Sebastian Reichel
0 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, Sebastian Reichel
Platform drivers with optional gadget mode support may want to
call usb_udc_vbus_handler to properly handle a cable being
re-plugged.
Ensure these drivers can call the function without a hard dependency on
CONFIG_USB_GADGET or having the ifdef logic in the driver.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
include/linux/usb/gadget.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 8285b19a25e0..551e71b6eda0 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -979,7 +979,12 @@ extern int usb_gadget_ep_match_desc(struct usb_gadget *gadget,
/*-------------------------------------------------------------------------*/
/* utility to update vbus status for udc core, it may be scheduled */
+#if IS_ENABLED(CONFIG_USB_GADGET)
extern void usb_udc_vbus_handler(struct usb_gadget *gadget, bool status);
+#else
+static inline void usb_udc_vbus_handler(struct usb_gadget *gadget, bool status)
+{}
+#endif
/*-------------------------------------------------------------------------*/
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v17 6/7] usb: gadget: define stub for usb_udc_vbus_handler
2026-10-16 18:43 ` Sebastian Reichel
(?)
(?)
@ 2026-10-16 18:53 ` sashiko-bot
-1 siblings, 0 replies; 35+ messages in thread
From: sashiko-bot @ 2026-10-16 18:53 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-usb
> Platform drivers with optional gadget mode support may want to
> call usb_udc_vbus_handler to properly handle a cable being
> re-plugged.
>
> Ensure these drivers can call the function without a hard dependency on
> CONFIG_USB_GADGET or having the ifdef logic in the driver.
>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-b4-rockchip-dwc3-rockchip-glue-v17-0-876f96893e2f@collabora.com?part=6
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v17 7/7] usb: dwc3: rockchip: fix USB-C reconnect in gadget mode
2026-10-16 18:43 ` Sebastian Reichel
(?)
@ 2026-10-16 18:43 ` Sebastian Reichel
-1 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, Sebastian Reichel
When USB-C is configured in gadget mode and the cable is unplugged
the USB controller is suspended. After plugging in the cable again,
the USB controller stays suspended and thus the port status remains
not-attached.
Fix this by informing the USB core about VBUS updates when the PHY
mode changes.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/usb/dwc3/Kconfig | 1 +
drivers/usb/dwc3/dwc3-rockchip.c | 24 ++++++++++++++++++++++++
2 files changed, 25 insertions(+)
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index c86c1ac293bf..4cbc949c6abd 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -194,6 +194,7 @@ config USB_DWC3_ROCKCHIP
tristate "Rockchip DWC3 Platform Driver"
depends on ARCH_ROCKCHIP || COMPILE_TEST
depends on PHY_ROCKCHIP_USBDP || !PHY_ROCKCHIP_USBDP
+ depends on USB_GADGET || !USB_GADGET
depends on OF
default USB_DWC3
help
diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c
index 1f32150caa09..5cb5feec8bd4 100644
--- a/drivers/usb/dwc3/dwc3-rockchip.c
+++ b/drivers/usb/dwc3/dwc3-rockchip.c
@@ -25,8 +25,17 @@ struct dwc3_rk_phy_nb {
struct dwc3_rockchip {
struct dwc3 dwc;
u8 phy_reset_active;
+ enum usb_role role;
};
+static void dwc3_rockchip_vbus_handler(struct dwc3 *dwc, bool present)
+{
+ if (!dwc->gadget || !dwc->gadget_driver)
+ return;
+
+ usb_udc_vbus_handler(dwc->gadget, present);
+}
+
static int dwc3_rockchip_phy_pre_reset(struct dwc3_rockchip *dwc_rk, int port)
{
struct dwc3 *dwc = &dwc_rk->dwc;
@@ -51,6 +60,8 @@ static int dwc3_rockchip_phy_pre_reset(struct dwc3_rockchip *dwc_rk, int port)
if (!ret)
return ret;
+ dwc3_rockchip_vbus_handler(dwc, false);
+
/*
* Assert USB3 PHY soft reset within DWC3 before the external
* PHY resets. This disconnects the PIPE interface, preventing
@@ -70,6 +81,8 @@ static int dwc3_rockchip_phy_pre_reset(struct dwc3_rockchip *dwc_rk, int port)
static int dwc3_rockchip_phy_post_reset(struct dwc3_rockchip *dwc_rk, int port)
{
struct dwc3 *dwc = &dwc_rk->dwc;
+ bool gadget_mode = (dwc->current_dr_role == DWC3_GCTL_PRTCAP_DEVICE) ||
+ (dwc_rk->role == USB_ROLE_DEVICE);
unsigned long flags;
u32 reg;
@@ -87,6 +100,8 @@ static int dwc3_rockchip_phy_post_reset(struct dwc3_rockchip *dwc_rk, int port)
dwc3_writel(dwc, DWC3_GUSB3PIPECTL(port), reg);
spin_unlock_irqrestore(&dwc->lock, flags);
+ dwc3_rockchip_vbus_handler(dwc, gadget_mode);
+
pm_runtime_put_autosuspend(dwc->dev);
return 0;
@@ -138,7 +153,16 @@ static int dwc3_rk_phy_register_reset_handler(struct dwc3 *dwc)
return devm_add_action_or_reset(dwc->dev, dwc3_rk_phy_unregister_notifiers, dwc_rk);
}
+static void dwc3_rockchip_set_role(struct dwc3 *dwc, enum usb_role role)
+{
+ struct dwc3_rockchip *dwc_rk = container_of(dwc, struct dwc3_rockchip, dwc);
+
+ dwc_rk->role = role;
+ dwc3_rockchip_vbus_handler(dwc, role == USB_ROLE_DEVICE);
+}
+
static struct dwc3_glue_ops dwc3_rockchip_glue_ops = {
+ .pre_set_role = dwc3_rockchip_set_role,
.post_phy_registration = dwc3_rk_phy_register_reset_handler,
};
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v17 7/7] usb: dwc3: rockchip: fix USB-C reconnect in gadget mode
@ 2026-10-16 18:43 ` Sebastian Reichel
0 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, Sebastian Reichel
When USB-C is configured in gadget mode and the cable is unplugged
the USB controller is suspended. After plugging in the cable again,
the USB controller stays suspended and thus the port status remains
not-attached.
Fix this by informing the USB core about VBUS updates when the PHY
mode changes.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/usb/dwc3/Kconfig | 1 +
drivers/usb/dwc3/dwc3-rockchip.c | 24 ++++++++++++++++++++++++
2 files changed, 25 insertions(+)
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index c86c1ac293bf..4cbc949c6abd 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -194,6 +194,7 @@ config USB_DWC3_ROCKCHIP
tristate "Rockchip DWC3 Platform Driver"
depends on ARCH_ROCKCHIP || COMPILE_TEST
depends on PHY_ROCKCHIP_USBDP || !PHY_ROCKCHIP_USBDP
+ depends on USB_GADGET || !USB_GADGET
depends on OF
default USB_DWC3
help
diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c
index 1f32150caa09..5cb5feec8bd4 100644
--- a/drivers/usb/dwc3/dwc3-rockchip.c
+++ b/drivers/usb/dwc3/dwc3-rockchip.c
@@ -25,8 +25,17 @@ struct dwc3_rk_phy_nb {
struct dwc3_rockchip {
struct dwc3 dwc;
u8 phy_reset_active;
+ enum usb_role role;
};
+static void dwc3_rockchip_vbus_handler(struct dwc3 *dwc, bool present)
+{
+ if (!dwc->gadget || !dwc->gadget_driver)
+ return;
+
+ usb_udc_vbus_handler(dwc->gadget, present);
+}
+
static int dwc3_rockchip_phy_pre_reset(struct dwc3_rockchip *dwc_rk, int port)
{
struct dwc3 *dwc = &dwc_rk->dwc;
@@ -51,6 +60,8 @@ static int dwc3_rockchip_phy_pre_reset(struct dwc3_rockchip *dwc_rk, int port)
if (!ret)
return ret;
+ dwc3_rockchip_vbus_handler(dwc, false);
+
/*
* Assert USB3 PHY soft reset within DWC3 before the external
* PHY resets. This disconnects the PIPE interface, preventing
@@ -70,6 +81,8 @@ static int dwc3_rockchip_phy_pre_reset(struct dwc3_rockchip *dwc_rk, int port)
static int dwc3_rockchip_phy_post_reset(struct dwc3_rockchip *dwc_rk, int port)
{
struct dwc3 *dwc = &dwc_rk->dwc;
+ bool gadget_mode = (dwc->current_dr_role == DWC3_GCTL_PRTCAP_DEVICE) ||
+ (dwc_rk->role == USB_ROLE_DEVICE);
unsigned long flags;
u32 reg;
@@ -87,6 +100,8 @@ static int dwc3_rockchip_phy_post_reset(struct dwc3_rockchip *dwc_rk, int port)
dwc3_writel(dwc, DWC3_GUSB3PIPECTL(port), reg);
spin_unlock_irqrestore(&dwc->lock, flags);
+ dwc3_rockchip_vbus_handler(dwc, gadget_mode);
+
pm_runtime_put_autosuspend(dwc->dev);
return 0;
@@ -138,7 +153,16 @@ static int dwc3_rk_phy_register_reset_handler(struct dwc3 *dwc)
return devm_add_action_or_reset(dwc->dev, dwc3_rk_phy_unregister_notifiers, dwc_rk);
}
+static void dwc3_rockchip_set_role(struct dwc3 *dwc, enum usb_role role)
+{
+ struct dwc3_rockchip *dwc_rk = container_of(dwc, struct dwc3_rockchip, dwc);
+
+ dwc_rk->role = role;
+ dwc3_rockchip_vbus_handler(dwc, role == USB_ROLE_DEVICE);
+}
+
static struct dwc3_glue_ops dwc3_rockchip_glue_ops = {
+ .pre_set_role = dwc3_rockchip_set_role,
.post_phy_registration = dwc3_rk_phy_register_reset_handler,
};
--
2.53.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v17 7/7] usb: dwc3: rockchip: fix USB-C reconnect in gadget mode
@ 2026-10-16 18:43 ` Sebastian Reichel
0 siblings, 0 replies; 35+ messages in thread
From: Sebastian Reichel @ 2026-10-16 18:43 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Heiko Stuebner, Vinod Koul,
Neil Armstrong, Manivannan Sadhasivam
Cc: Igor Paunovic, linux-kernel, linux-usb, linux-arm-kernel,
linux-rockchip, linux-phy, kernel, Sebastian Reichel
When USB-C is configured in gadget mode and the cable is unplugged
the USB controller is suspended. After plugging in the cable again,
the USB controller stays suspended and thus the port status remains
not-attached.
Fix this by informing the USB core about VBUS updates when the PHY
mode changes.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/usb/dwc3/Kconfig | 1 +
drivers/usb/dwc3/dwc3-rockchip.c | 24 ++++++++++++++++++++++++
2 files changed, 25 insertions(+)
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index c86c1ac293bf..4cbc949c6abd 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -194,6 +194,7 @@ config USB_DWC3_ROCKCHIP
tristate "Rockchip DWC3 Platform Driver"
depends on ARCH_ROCKCHIP || COMPILE_TEST
depends on PHY_ROCKCHIP_USBDP || !PHY_ROCKCHIP_USBDP
+ depends on USB_GADGET || !USB_GADGET
depends on OF
default USB_DWC3
help
diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c
index 1f32150caa09..5cb5feec8bd4 100644
--- a/drivers/usb/dwc3/dwc3-rockchip.c
+++ b/drivers/usb/dwc3/dwc3-rockchip.c
@@ -25,8 +25,17 @@ struct dwc3_rk_phy_nb {
struct dwc3_rockchip {
struct dwc3 dwc;
u8 phy_reset_active;
+ enum usb_role role;
};
+static void dwc3_rockchip_vbus_handler(struct dwc3 *dwc, bool present)
+{
+ if (!dwc->gadget || !dwc->gadget_driver)
+ return;
+
+ usb_udc_vbus_handler(dwc->gadget, present);
+}
+
static int dwc3_rockchip_phy_pre_reset(struct dwc3_rockchip *dwc_rk, int port)
{
struct dwc3 *dwc = &dwc_rk->dwc;
@@ -51,6 +60,8 @@ static int dwc3_rockchip_phy_pre_reset(struct dwc3_rockchip *dwc_rk, int port)
if (!ret)
return ret;
+ dwc3_rockchip_vbus_handler(dwc, false);
+
/*
* Assert USB3 PHY soft reset within DWC3 before the external
* PHY resets. This disconnects the PIPE interface, preventing
@@ -70,6 +81,8 @@ static int dwc3_rockchip_phy_pre_reset(struct dwc3_rockchip *dwc_rk, int port)
static int dwc3_rockchip_phy_post_reset(struct dwc3_rockchip *dwc_rk, int port)
{
struct dwc3 *dwc = &dwc_rk->dwc;
+ bool gadget_mode = (dwc->current_dr_role == DWC3_GCTL_PRTCAP_DEVICE) ||
+ (dwc_rk->role == USB_ROLE_DEVICE);
unsigned long flags;
u32 reg;
@@ -87,6 +100,8 @@ static int dwc3_rockchip_phy_post_reset(struct dwc3_rockchip *dwc_rk, int port)
dwc3_writel(dwc, DWC3_GUSB3PIPECTL(port), reg);
spin_unlock_irqrestore(&dwc->lock, flags);
+ dwc3_rockchip_vbus_handler(dwc, gadget_mode);
+
pm_runtime_put_autosuspend(dwc->dev);
return 0;
@@ -138,7 +153,16 @@ static int dwc3_rk_phy_register_reset_handler(struct dwc3 *dwc)
return devm_add_action_or_reset(dwc->dev, dwc3_rk_phy_unregister_notifiers, dwc_rk);
}
+static void dwc3_rockchip_set_role(struct dwc3 *dwc, enum usb_role role)
+{
+ struct dwc3_rockchip *dwc_rk = container_of(dwc, struct dwc3_rockchip, dwc);
+
+ dwc_rk->role = role;
+ dwc3_rockchip_vbus_handler(dwc, role == USB_ROLE_DEVICE);
+}
+
static struct dwc3_glue_ops dwc3_rockchip_glue_ops = {
+ .pre_set_role = dwc3_rockchip_set_role,
.post_phy_registration = dwc3_rk_phy_register_reset_handler,
};
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v17 7/7] usb: dwc3: rockchip: fix USB-C reconnect in gadget mode
2026-10-16 18:43 ` Sebastian Reichel
@ 2026-10-16 18:56 ` sashiko-bot
-1 siblings, 0 replies; 35+ messages in thread
From: sashiko-bot @ 2026-10-16 18:56 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-usb, linux-phy, vkoul, neil.armstrong, olteanv
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] usb: dwc3: rockchip: UAF in vbus handler during gadget teardown
- [High] usb: dwc3: rockchip: unbalanced pm_runtime_put when PM disabled
- [High] usb: dwc3: hardware abort due to late PHY callback unregistration
--
commit d44e5645f925209462f1bf666f95eb79ad1d50f1
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
usb: dwc3: rockchip: fix USB-C reconnect in gadget mode
When USB-C is configured in gadget mode and the cable is unplugged
the USB controller is suspended. After plugging in the cable again,
the USB controller stays suspended and thus the port status remains
not-attached.
This commit fixes this by informing the USB core about VBUS updates when
the PHY mode changes.
> diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c
> index 1f32150caa090..5cb5feec8bd4b 100644
> --- a/drivers/usb/dwc3/dwc3-rockchip.c
> +++ b/drivers/usb/dwc3/dwc3-rockchip.c
[ ... ]
> @@ -25,8 +25,17 @@ struct dwc3_rk_phy_nb {
> struct dwc3_rockchip {
> struct dwc3 dwc;
> u8 phy_reset_active;
> + enum usb_role role;
> };
>
> +static void dwc3_rockchip_vbus_handler(struct dwc3 *dwc, bool present)
> +{
> + if (!dwc->gadget || !dwc->gadget_driver)
> + return;
> +
> + usb_udc_vbus_handler(dwc->gadget, present);
> +}
[Severity: High]
Does this lockless check of dwc->gadget allow a TOCTOU race?
The handler is invoked by dwc3_rockchip_phy_pre_reset(), which is a PHY
reset callback that can run concurrently (for instance, from DP hotplug
events in the rockchip-usbdp driver via rk_udphy_dp_phy_power_on).
If the PHY reset runs concurrently with dwc3_core_exit_mode(), could the
lockless check succeed just before dwc3_gadget_stop() clears
dwc->gadget_driver and dwc3_gadget_exit() frees dwc->gadget?
Would this cause the reset handler to pass a freed dwc->gadget to
usb_udc_vbus_handler(), resulting in a use-after-free?
Would serializing the check and call within spin_lock_irqsave(&dwc->lock,
flags) prevent this race safely?
> +
> static int dwc3_rockchip_phy_pre_reset(struct dwc3_rockchip *dwc_rk, int port)
> {
> struct dwc3 *dwc = &dwc_rk->dwc;
[ ... ]
> @@ -51,6 +60,8 @@ static int dwc3_rockchip_phy_pre_reset(struct dwc3_rockchip *dwc_rk, int port)
> if (!ret)
> return ret;
[Severity: High]
When runtime PM is disabled, pm_runtime_get_if_active(dwc->dev) returns
-EINVAL and does not increment usage_count. Since the `if (!ret)` check
evaluates to false for -EINVAL, will execution continue and lead to an
unbalanced pm_runtime_put_autosuspend() later?
Additionally, does this PM check expose the driver to an asynchronous
external abort (kernel panic) due to late PHY callback unregistration?
During dwc3_core_remove(), clocks are disabled via dwc3_core_exit() while
the PM state remains RPM_ACTIVE until pm_runtime_disable() is called.
Because the PHY callback is unregistered via devres (which runs after
remove completes), could a concurrent DP PHY power-on execute this
pre-reset function and access unclocked hardware registers?
>
> + dwc3_rockchip_vbus_handler(dwc, false);
> +
> /*
> * Assert USB3 PHY soft reset within DWC3 before the external
> * PHY resets. This disconnects the PIPE interface, preventing
[ ... ]
> @@ -87,6 +100,8 @@ static int dwc3_rockchip_phy_post_reset(struct dwc3_rockchip *dwc_rk, int port)
> dwc3_writel(dwc, DWC3_GUSB3PIPECTL(port), reg);
> spin_unlock_irqrestore(&dwc->lock, flags);
>
> + dwc3_rockchip_vbus_handler(dwc, gadget_mode);
> +
> pm_runtime_put_autosuspend(dwc->dev);
[Severity: High]
Since this pm_runtime_put_autosuspend() is executed unconditionally, will
it decrement the usage_count even when pm_runtime_get_if_active()
previously returned -EINVAL?
Could this falsely drop the usage count from > 0 to 0 (stealing a reference
from another subsystem component) and lead to the device being runtime
suspended while actively in use?
>
> return 0;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-b4-rockchip-dwc3-rockchip-glue-v17-0-876f96893e2f@collabora.com?part=7
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v17 7/7] usb: dwc3: rockchip: fix USB-C reconnect in gadget mode
@ 2026-10-16 18:56 ` sashiko-bot
0 siblings, 0 replies; 35+ messages in thread
From: sashiko-bot @ 2026-10-16 18:56 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-usb, linux-phy, vkoul, neil.armstrong, olteanv
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] usb: dwc3: rockchip: UAF in vbus handler during gadget teardown
- [High] usb: dwc3: rockchip: unbalanced pm_runtime_put when PM disabled
- [High] usb: dwc3: hardware abort due to late PHY callback unregistration
--
commit d44e5645f925209462f1bf666f95eb79ad1d50f1
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
usb: dwc3: rockchip: fix USB-C reconnect in gadget mode
When USB-C is configured in gadget mode and the cable is unplugged
the USB controller is suspended. After plugging in the cable again,
the USB controller stays suspended and thus the port status remains
not-attached.
This commit fixes this by informing the USB core about VBUS updates when
the PHY mode changes.
> diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c
> index 1f32150caa090..5cb5feec8bd4b 100644
> --- a/drivers/usb/dwc3/dwc3-rockchip.c
> +++ b/drivers/usb/dwc3/dwc3-rockchip.c
[ ... ]
> @@ -25,8 +25,17 @@ struct dwc3_rk_phy_nb {
> struct dwc3_rockchip {
> struct dwc3 dwc;
> u8 phy_reset_active;
> + enum usb_role role;
> };
>
> +static void dwc3_rockchip_vbus_handler(struct dwc3 *dwc, bool present)
> +{
> + if (!dwc->gadget || !dwc->gadget_driver)
> + return;
> +
> + usb_udc_vbus_handler(dwc->gadget, present);
> +}
[Severity: High]
Does this lockless check of dwc->gadget allow a TOCTOU race?
The handler is invoked by dwc3_rockchip_phy_pre_reset(), which is a PHY
reset callback that can run concurrently (for instance, from DP hotplug
events in the rockchip-usbdp driver via rk_udphy_dp_phy_power_on).
If the PHY reset runs concurrently with dwc3_core_exit_mode(), could the
lockless check succeed just before dwc3_gadget_stop() clears
dwc->gadget_driver and dwc3_gadget_exit() frees dwc->gadget?
Would this cause the reset handler to pass a freed dwc->gadget to
usb_udc_vbus_handler(), resulting in a use-after-free?
Would serializing the check and call within spin_lock_irqsave(&dwc->lock,
flags) prevent this race safely?
> +
> static int dwc3_rockchip_phy_pre_reset(struct dwc3_rockchip *dwc_rk, int port)
> {
> struct dwc3 *dwc = &dwc_rk->dwc;
[ ... ]
> @@ -51,6 +60,8 @@ static int dwc3_rockchip_phy_pre_reset(struct dwc3_rockchip *dwc_rk, int port)
> if (!ret)
> return ret;
[Severity: High]
When runtime PM is disabled, pm_runtime_get_if_active(dwc->dev) returns
-EINVAL and does not increment usage_count. Since the `if (!ret)` check
evaluates to false for -EINVAL, will execution continue and lead to an
unbalanced pm_runtime_put_autosuspend() later?
Additionally, does this PM check expose the driver to an asynchronous
external abort (kernel panic) due to late PHY callback unregistration?
During dwc3_core_remove(), clocks are disabled via dwc3_core_exit() while
the PM state remains RPM_ACTIVE until pm_runtime_disable() is called.
Because the PHY callback is unregistered via devres (which runs after
remove completes), could a concurrent DP PHY power-on execute this
pre-reset function and access unclocked hardware registers?
>
> + dwc3_rockchip_vbus_handler(dwc, false);
> +
> /*
> * Assert USB3 PHY soft reset within DWC3 before the external
> * PHY resets. This disconnects the PIPE interface, preventing
[ ... ]
> @@ -87,6 +100,8 @@ static int dwc3_rockchip_phy_post_reset(struct dwc3_rockchip *dwc_rk, int port)
> dwc3_writel(dwc, DWC3_GUSB3PIPECTL(port), reg);
> spin_unlock_irqrestore(&dwc->lock, flags);
>
> + dwc3_rockchip_vbus_handler(dwc, gadget_mode);
> +
> pm_runtime_put_autosuspend(dwc->dev);
[Severity: High]
Since this pm_runtime_put_autosuspend() is executed unconditionally, will
it decrement the usage_count even when pm_runtime_get_if_active()
previously returned -EINVAL?
Could this falsely drop the usage count from > 0 to 0 (stealing a reference
from another subsystem component) and lead to the device being runtime
suspended while actively in use?
>
> return 0;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-b4-rockchip-dwc3-rockchip-glue-v17-0-876f96893e2f@collabora.com?part=7
^ permalink raw reply [flat|nested] 35+ messages in thread