* [PATCH v8 27/29] phy: rockchip: usbdp: Add some extra debug messages
From: Sebastian Reichel @ 2026-06-26 20:55 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, Sebastian Reichel
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com>
It's useful to log PHY reinit to ease debugging issues around
USB-C hotplugging.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 2681610d10d5..dc166392ba19 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -24,6 +24,7 @@
#include <linux/property.h>
#include <linux/regmap.h>
#include <linux/reset.h>
+#include <linux/string_choices.h>
#include <linux/usb/ch9.h>
#include <linux/usb/typec_dp.h>
#include <linux/usb/typec_mux.h>
@@ -469,6 +470,8 @@ static int rk_udphy_reset_deassert(struct rk_udphy *udphy, char *name)
return reset_control_deassert(list[idx].rstc);
}
+ dev_err(udphy->dev, "failed to de-assert missing reset line: %s\n", name);
+
return -EINVAL;
}
@@ -495,6 +498,8 @@ static void rk_udphy_u3_port_disable(struct rk_udphy *udphy, u8 disable)
const struct rk_udphy_cfg *cfg = udphy->cfgs;
const struct rk_udphy_grf_reg *preg;
+ dev_dbg(udphy->dev, "USB3 port %s\n", str_on_off(!disable));
+
preg = udphy->id ? &cfg->grfcfg.usb3otg1_cfg : &cfg->grfcfg.usb3otg0_cfg;
rk_udphy_grfreg_write(udphy->usbgrf, preg, disable);
}
@@ -788,6 +793,11 @@ static int rk_udphy_init(struct rk_udphy *udphy)
const struct rk_udphy_cfg *cfg = udphy->cfgs;
int ret;
+ dev_dbg(udphy->dev, "(re-)init PHY with USB=%s and DP=%s (%u lanes) flipped=%s\n",
+ str_enabled_disabled(udphy->mode & UDPHY_MODE_USB),
+ str_enabled_disabled(udphy->mode & UDPHY_MODE_DP),
+ udphy->dp_lanes, str_yes_no(udphy->flip));
+
rk_udphy_reset_assert_all(udphy);
usleep_range(10000, 11000);
@@ -858,6 +868,8 @@ static int rk_udphy_setup(struct rk_udphy *udphy)
{
int ret;
+ dev_dbg(udphy->dev, "enable PHY\n");
+
ret = clk_bulk_prepare_enable(udphy->num_clks, udphy->clks);
if (ret) {
dev_err(udphy->dev, "failed to enable clk\n");
@@ -876,6 +888,7 @@ static int rk_udphy_setup(struct rk_udphy *udphy)
static void rk_udphy_disable(struct rk_udphy *udphy)
{
+ dev_dbg(udphy->dev, "disable PHY\n");
clk_bulk_disable_unprepare(udphy->num_clks, udphy->clks);
rk_udphy_reset_assert_all(udphy);
}
@@ -1359,8 +1372,10 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
struct rk_udphy *udphy = typec_mux_get_drvdata(mux);
/* Ignore mux events not involving USB or DP */
- if (!rk_udphy_is_supported_mode(state))
+ if (!rk_udphy_is_supported_mode(state)) {
+ dev_dbg(udphy->dev, "ignore mux event with mode=%lu\n", state->mode);
return 0;
+ }
guard(mutex)(&udphy->mutex);
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v8 17/29] phy: rockchip: usbdp: Register DP aux bridge
From: Sebastian Reichel @ 2026-06-26 20:55 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, Sebastian Reichel
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com>
Add support to use USB-C connectors with the DP altmode helper code on
devicetree based platforms. To get this working there must be a DRM
bridge chain from the DisplayPort controller to the USB-C connector.
E.g. on Rockchip RK3576:
root@rk3576 # cat /sys/kernel/debug/dri/0/encoder-0/bridges
bridge[0]: dw_dp_bridge_funcs
refcount: 7
type: [10] DP
OF: /soc/dp@27e40000:rockchip,rk3576-dp
ops: [0x47] detect edid hpd
bridge[1]: drm_aux_bridge_funcs
refcount: 4
type: [0] Unknown
OF: /soc/phy@2b010000:rockchip,rk3576-usbdp-phy
ops: [0x0]
bridge[2]: drm_aux_hpd_bridge_funcs
refcount: 5
type: [10] DP
OF: /soc/i2c@2ac50000/typec-portc@22/connector:usb-c-connector
ops: [0x4] hpd
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/Kconfig | 2 ++
drivers/phy/rockchip/phy-rockchip-usbdp.c | 14 ++++++++++++++
2 files changed, 16 insertions(+)
diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig
index 14698571b607..39759bb2fa1d 100644
--- a/drivers/phy/rockchip/Kconfig
+++ b/drivers/phy/rockchip/Kconfig
@@ -136,8 +136,10 @@ config PHY_ROCKCHIP_USBDP
tristate "Rockchip USBDP COMBO PHY Driver"
depends on ARCH_ROCKCHIP && OF
depends on TYPEC
+ depends on DRM || DRM=n
select GENERIC_PHY
select USB_COMMON
+ select DRM_AUX_BRIDGE if DRM_BRIDGE
help
Enable this to support the Rockchip USB3.0/DP combo PHY with
Samsung IP block. This is required for USB3 support on RK3588.
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index e243d92483e0..a204699619b8 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -6,6 +6,7 @@
* Copyright (C) 2024 Collabora Ltd
*/
+#include <drm/bridge/aux-bridge.h>
#include <dt-bindings/phy/phy.h>
#include <linux/bitfield.h>
#include <linux/bits.h>
@@ -1447,6 +1448,7 @@ static int rk_udphy_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct phy_provider *phy_provider;
+ struct fwnode_handle *dp_aux_ep;
struct resource *res;
struct rk_udphy *udphy;
void __iomem *base;
@@ -1505,6 +1507,18 @@ static int rk_udphy_probe(struct platform_device *pdev)
return ret;
}
+ /*
+ * Only register the DRM bridge, if the DP aux channel is connected.
+ * Some boards use the USBDP PHY only for its USB3 capabilities.
+ */
+ dp_aux_ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 3, 0, 0);
+ if (dp_aux_ep) {
+ ret = drm_aux_bridge_register(dev);
+ fwnode_handle_put(dp_aux_ep);
+ if (ret)
+ return ret;
+ }
+
udphy->phy_u3 = devm_phy_create(dev, dev->of_node, &rk_udphy_usb3_phy_ops);
if (IS_ERR(udphy->phy_u3)) {
ret = PTR_ERR(udphy->phy_u3);
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v8 18/29] phy: rockchip: usbdp: Drop DP HPD handling
From: Sebastian Reichel @ 2026-06-26 20:55 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, Sebastian Reichel
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com>
Drop the HPD handling logic from the USBDP PHY. The registers involved
require the display controller power domain being enabled and thus the
HPD signal should be handled by the displayport controller itself.
Apart from that the HPD handling as it is done here is incorrect and
misses hotplug events happening after the USB-C connector (e.g. when
a USB-C to HDMI adapter is involved and the HDMI cable is replugged).
Proper USB-C DP HPD support requires some restructuring of the DP
controller driver, which will happen independent of this patch. The
mainline kernel does not yet support USB-C DP AltMode on RK3588 and
RK3576, so it is fine to drop this code without adding the counterpart
in the DRM in an atomic change.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 81 ++++---------------------------
1 file changed, 9 insertions(+), 72 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index a204699619b8..731e487db57e 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -186,14 +186,11 @@ struct rk_udphy {
u32 dp_lane_sel[4];
u32 dp_aux_dout_sel;
u32 dp_aux_din_sel;
- bool dp_sink_hpd_sel;
- bool dp_sink_hpd_cfg;
unsigned int link_rate;
unsigned int lanes;
u8 bw;
int id;
- bool dp_in_use;
int dp_lanes;
/* PHY const config */
@@ -582,19 +579,6 @@ static void rk_udphy_dp_lane_enable(struct rk_udphy *udphy, int dp_lanes)
CMN_DP_CMN_RSTN, FIELD_PREP(CMN_DP_CMN_RSTN, 0x0));
}
-static void rk_udphy_dp_hpd_event_trigger(struct rk_udphy *udphy, bool hpd)
-{
- const struct rk_udphy_cfg *cfg = udphy->cfgs;
-
- udphy->dp_sink_hpd_sel = true;
- udphy->dp_sink_hpd_cfg = hpd;
-
- if (!udphy->dp_in_use)
- return;
-
- rk_udphy_grfreg_write(udphy->vogrf, &cfg->vogrfcfg[udphy->id].hpd_trigger, hpd);
-}
-
static void rk_udphy_mode_set(struct rk_udphy *udphy, u8 mode)
{
if (udphy->mode == mode)
@@ -1033,29 +1017,6 @@ static void rk_udphy_power_off(struct rk_udphy *udphy, u8 mode)
rk_udphy_disable(udphy);
}
-static int rk_udphy_dp_phy_init(struct phy *phy)
-{
- struct rk_udphy *udphy = phy_get_drvdata(phy);
-
- mutex_lock(&udphy->mutex);
-
- udphy->dp_in_use = true;
-
- mutex_unlock(&udphy->mutex);
-
- return 0;
-}
-
-static int rk_udphy_dp_phy_exit(struct phy *phy)
-{
- struct rk_udphy *udphy = phy_get_drvdata(phy);
-
- mutex_lock(&udphy->mutex);
- udphy->dp_in_use = false;
- mutex_unlock(&udphy->mutex);
- return 0;
-}
-
static int rk_udphy_dp_phy_power_on(struct phy *phy)
{
struct rk_udphy *udphy = phy_get_drvdata(phy);
@@ -1287,8 +1248,6 @@ static int rk_udphy_dp_phy_configure(struct phy *phy,
}
static const struct phy_ops rk_udphy_dp_phy_ops = {
- .init = rk_udphy_dp_phy_init,
- .exit = rk_udphy_dp_phy_exit,
.power_on = rk_udphy_dp_phy_power_on,
.power_off = rk_udphy_dp_phy_power_off,
.configure = rk_udphy_dp_phy_configure,
@@ -1342,6 +1301,14 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
struct rk_udphy *udphy = typec_mux_get_drvdata(mux);
u8 mode;
+ /*
+ * Ignore mux events not involving DP AltMode, because
+ * the mode field is being reused, e.g. state->mode == 4
+ * could be either TYPEC_MODE_USB4 or TYPEC_DP_STATE_C.
+ */
+ if (!state->alt || state->alt->svid != USB_TYPEC_DP_SID)
+ return 0;
+
mutex_lock(&udphy->mutex);
switch (state->mode) {
@@ -1373,22 +1340,7 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
break;
}
- if (state->alt && state->alt->svid == USB_TYPEC_DP_SID) {
- struct typec_displayport_data *data = state->data;
-
- if (!data) {
- rk_udphy_dp_hpd_event_trigger(udphy, false);
- } else if (data->status & DP_STATUS_IRQ_HPD) {
- rk_udphy_dp_hpd_event_trigger(udphy, false);
- usleep_range(750, 800);
- rk_udphy_dp_hpd_event_trigger(udphy, true);
- } else if (data->status & DP_STATUS_HPD_STATE) {
- rk_udphy_mode_set(udphy, mode);
- rk_udphy_dp_hpd_event_trigger(udphy, true);
- } else {
- rk_udphy_dp_hpd_event_trigger(udphy, false);
- }
- }
+ rk_udphy_mode_set(udphy, mode);
mutex_unlock(&udphy->mutex);
return 0;
@@ -1544,20 +1496,6 @@ static int rk_udphy_probe(struct platform_device *pdev)
return 0;
}
-static int __maybe_unused rk_udphy_resume(struct device *dev)
-{
- struct rk_udphy *udphy = dev_get_drvdata(dev);
-
- if (udphy->dp_sink_hpd_sel)
- rk_udphy_dp_hpd_event_trigger(udphy, udphy->dp_sink_hpd_cfg);
-
- return 0;
-}
-
-static const struct dev_pm_ops rk_udphy_pm_ops = {
- SET_LATE_SYSTEM_SLEEP_PM_OPS(NULL, rk_udphy_resume)
-};
-
static const char * const rk_udphy_rst_list[] = {
"init", "cmn", "lane", "pcs_apb", "pma_apb"
};
@@ -1662,7 +1600,6 @@ static struct platform_driver rk_udphy_driver = {
.driver = {
.name = "rockchip-usbdp-phy",
.of_match_table = rk_udphy_dt_match,
- .pm = &rk_udphy_pm_ops,
},
};
module_platform_driver(rk_udphy_driver);
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v8 24/29] phy: rockchip: usbdp: Support going from DP-only mode to USB mode
From: Sebastian Reichel @ 2026-06-26 20:55 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, Sebastian Reichel
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com>
When a USB-C adapter, which maps all Superspeed lanes to DP is plugged
in, the USB support is disabled in the PHY. When the adapter is
unplugged and a different adapter with USB functionality is plugged in
afterwards, USB functionality is not restored as the USB controller
keeps the PHY enabled for the entire time.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 837a4cb3e4b6..4566822d70c4 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -179,6 +179,7 @@ struct rk_udphy {
/* utilized for USB */
bool hs; /* flag for high-speed */
+ bool usb_in_use;
/* utilized for DP */
struct gpio_desc *sbu1_dc_gpio;
@@ -1025,6 +1026,10 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
ret = rk_udphy_init(udphy);
if (ret)
return ret;
+
+ if (udphy->mode & UDPHY_MODE_USB)
+ rk_udphy_u3_port_disable(udphy, false);
+
udphy->phy_needs_reinit = false;
}
@@ -1288,16 +1293,24 @@ static const struct phy_ops rk_udphy_dp_phy_ops = {
static int rk_udphy_usb3_phy_init(struct phy *phy)
{
struct rk_udphy *udphy = phy_get_drvdata(phy);
+ int ret;
guard(mutex)(&udphy->mutex);
/* DP only or high-speed, disable U3 port */
if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs) {
rk_udphy_u3_port_disable(udphy, true);
+ udphy->usb_in_use = true;
return 0;
}
- return rk_udphy_power_on(udphy, UDPHY_MODE_USB);
+ ret = rk_udphy_power_on(udphy, UDPHY_MODE_USB);
+ if (ret)
+ return ret;
+
+ udphy->usb_in_use = true;
+
+ return 0;
}
static int rk_udphy_usb3_phy_exit(struct phy *phy)
@@ -1306,6 +1319,8 @@ static int rk_udphy_usb3_phy_exit(struct phy *phy)
guard(mutex)(&udphy->mutex);
+ udphy->usb_in_use = false;
+
/* DP only or high-speed */
if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
return 0;
@@ -1347,6 +1362,17 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
rk_udphy_set_typec_state(udphy, state->mode);
+ /*
+ * If the new mode includes USB, but it has not yet been powered
+ * (because the previous mode was DP-only) and the USB PHY was
+ * already initialized by the USB controller, we need to power on
+ * the USB side now since no subsequent phy_init call will come
+ * from the controller.
+ */
+ if ((udphy->mode & UDPHY_MODE_USB) && !(udphy->status & UDPHY_MODE_USB) &&
+ udphy->usb_in_use && !udphy->hs)
+ return rk_udphy_power_on(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
* [PATCH v8 28/29] phy: rockchip: usbdp: Avoid xHCI SErrors
From: Sebastian Reichel @ 2026-06-26 20:55 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, Sebastian Reichel
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com>
The USBDP PHY provides the PIPE clock to the USB3 controller, which
means the PHY must be fully running when anything tries to access
the xHCI registers.
When switching between USB3-only, USB3 + DP and DP-only mode, the
PHY must be re-initialized resulting in a short period of the PHY
being disabled. If the DWC3 driver decides to access the xHCI at
this point the system will fail with an SError.
This patch avoids the problems by disabling the USB3 port before
re-initializing it. This does a couple of things:
- forces phystatus to 0 from GRF (not from PHY)
- switches PIPE clock source from PHY to UTMI (safe fallback clock)
- num_u3_port=0
The last part will be ignored, as DWC3 already probed, but the
clock re-routing will avoid the SError. There is a small delay
afterwards to make sure the mux happened. The datasheet gives
no hints how long it takes, so delay time is a guess.
Fixes: 2f70bbddeb45 ("phy: rockchip: add usbdp combo phy driver")
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index dc166392ba19..7c8b9eaaf352 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -1033,8 +1033,8 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
rk_udphy_u3_port_disable(udphy, false);
udphy->phy_needs_reinit = false;
} else if (udphy->phy_needs_reinit) {
- if (udphy->mode == UDPHY_MODE_DP)
- rk_udphy_u3_port_disable(udphy, true);
+ rk_udphy_u3_port_disable(udphy, true);
+ udelay(10);
ret = rk_udphy_init(udphy);
if (ret)
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v8 19/29] phy: rockchip: usbdp: Rename mode_change to phy_needs_reinit
From: Sebastian Reichel @ 2026-06-26 20:55 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, Sebastian Reichel
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com>
Right now the mode_change property is set whenever the mode changes
between USB-only, DP-only and USB-DP. It is needed, because on any
mode change the PHY needs to be re-initialized. Apparently at least
DP also requires a re-init when the cable orientation is changed,
which is currently not being done (except when the orientation switch
also involves a mode change). Prepare for this by renaming mode_change
to phy_needs_reinit.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 731e487db57e..1bb22fc18c9f 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -172,7 +172,7 @@ struct rk_udphy {
/* PHY status management */
bool flip;
- bool mode_change;
+ bool phy_needs_reinit;
u8 mode;
u8 status;
@@ -584,7 +584,7 @@ static void rk_udphy_mode_set(struct rk_udphy *udphy, u8 mode)
if (udphy->mode == mode)
return;
- udphy->mode_change = true;
+ udphy->phy_needs_reinit = true;
udphy->mode = mode;
}
@@ -985,15 +985,15 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
if (udphy->mode & UDPHY_MODE_USB)
rk_udphy_u3_port_disable(udphy, false);
- udphy->mode_change = false;
- } else if (udphy->mode_change) {
+ udphy->phy_needs_reinit = false;
+ } else if (udphy->phy_needs_reinit) {
if (udphy->mode == UDPHY_MODE_DP)
rk_udphy_u3_port_disable(udphy, true);
ret = rk_udphy_init(udphy);
if (ret)
return ret;
- udphy->mode_change = false;
+ udphy->phy_needs_reinit = false;
}
udphy->status |= mode;
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v8 16/29] phy: rockchip: usbdp: Cleanup DP lane selection function
From: Sebastian Reichel @ 2026-06-26 20:55 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, Sebastian Reichel
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com>
Use FIELD_PREP_WM16() helpers to simplify the DP lane selection
logic.
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 28 +++++++---------------------
1 file changed, 7 insertions(+), 21 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index d8978cd22707..e243d92483e0 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -553,30 +553,16 @@ static void rk_udphy_usb_bvalid_enable(struct rk_udphy *udphy, u8 enable)
static void rk_udphy_dp_lane_select(struct rk_udphy *udphy)
{
const struct rk_udphy_cfg *cfg = udphy->cfgs;
- u32 value = 0;
-
- switch (udphy->dp_lanes) {
- case 4:
- value |= 3 << udphy->dp_lane_sel[3] * 2;
- value |= 2 << udphy->dp_lane_sel[2] * 2;
- fallthrough;
-
- case 2:
- value |= 1 << udphy->dp_lane_sel[1] * 2;
- fallthrough;
+ u32 value = FIELD_PREP_WM16(DP_LANE_SEL_ALL, 0);
+ int i;
- case 1:
- value |= 0 << udphy->dp_lane_sel[0] * 2;
- break;
+ for (i = 0; i < udphy->dp_lanes; i++)
+ value |= field_prep(DP_LANE_SEL_N(udphy->dp_lane_sel[i]), i);
- default:
- break;
- }
+ value |= FIELD_PREP_WM16(DP_AUX_DIN_SEL, udphy->dp_aux_din_sel);
+ value |= FIELD_PREP_WM16(DP_AUX_DOUT_SEL, udphy->dp_aux_dout_sel);
- regmap_write(udphy->vogrf, cfg->vogrfcfg[udphy->id].dp_lane_reg,
- ((DP_AUX_DIN_SEL | DP_AUX_DOUT_SEL | DP_LANE_SEL_ALL) << 16) |
- FIELD_PREP(DP_AUX_DIN_SEL, udphy->dp_aux_din_sel) |
- FIELD_PREP(DP_AUX_DOUT_SEL, udphy->dp_aux_dout_sel) | value);
+ regmap_write(udphy->vogrf, cfg->vogrfcfg[udphy->id].dp_lane_reg, value);
}
static void rk_udphy_dp_lane_enable(struct rk_udphy *udphy, int dp_lanes)
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v8 22/29] phy: rockchip: usbdp: Properly handle TYPEC_STATE_SAFE and TYPEC_STATE_USB
From: Sebastian Reichel @ 2026-06-26 20:55 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, Sebastian Reichel, Sashiko
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com>
Handle TYPEC_STATE_SAFE and TYPEC_STATE_USB Type-C state events,
so that the muxing is properly updated when exiting DP AltMode.
Fixes: 2f70bbddeb45 ("phy: rockchip: add usbdp combo phy driver")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/message/20260619155020.CC7361F000E9%40smtp.kernel.org
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 7c3b0d45d938..63fb41ad0199 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -1327,17 +1327,26 @@ static const struct phy_ops rk_udphy_usb3_phy_ops = {
.owner = THIS_MODULE,
};
+static bool rk_udphy_is_supported_mode(struct typec_mux_state *state)
+{
+ /* Handle Safe State and USB State */
+ if (state->mode < TYPEC_STATE_MODAL)
+ return true;
+
+ /* Handle DP AltMode */
+ if (state->alt && state->alt->svid == USB_TYPEC_DP_SID)
+ return true;
+
+ return false;
+}
+
static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
struct typec_mux_state *state)
{
struct rk_udphy *udphy = typec_mux_get_drvdata(mux);
- /*
- * Ignore mux events not involving DP AltMode, because
- * the mode field is being reused, e.g. state->mode == 4
- * could be either TYPEC_MODE_USB4 or TYPEC_DP_STATE_C.
- */
- if (!state->alt || state->alt->svid != USB_TYPEC_DP_SID)
+ /* Ignore mux events not involving USB or DP */
+ if (!rk_udphy_is_supported_mode(state))
return 0;
mutex_lock(&udphy->mutex);
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v8 25/29] phy: rockchip: usbdp: Clear USB status on PHY exit
From: Sebastian Reichel @ 2026-06-26 20:55 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Frank Wang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Andy Yan, Dmitry Baryshkov, Yubing Zhang, Alexey Charkov,
linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
devicetree, Sebastian Reichel, Sashiko
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com>
Ensure the USB status flag is cleared when the USB3 PHY is
exited while the system is in DP-only mode. This can happen
if the USB3 controller device is unbound while a DP-only
adapter is plugged into the USB-C port.
Fixes: 2f70bbddeb45 ("phy: rockchip: add usbdp combo phy driver")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/message/20260625-rockchip-usbdp-cleanup-v7-24-38eb3cf654fd%40collabora.com
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 4566822d70c4..15c18c6e3093 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -1322,8 +1322,10 @@ static int rk_udphy_usb3_phy_exit(struct phy *phy)
udphy->usb_in_use = false;
/* DP only or high-speed */
- if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
+ if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs) {
+ udphy->status &= ~UDPHY_MODE_USB;
return 0;
+ }
rk_udphy_power_off(udphy, UDPHY_MODE_USB);
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* Re: [PATCH v8 07/29] phy: rockchip: usbdp: Handle rk_udphy_reset_deassert_all errors in init check
From: sashiko-bot @ 2026-06-26 21:11 UTC (permalink / raw)
To: Sebastian Reichel
Cc: vkoul, neil.armstrong, conor+dt, olteanv, robh, linux-phy,
devicetree
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-7-47f682987895@collabora.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [Medium] The return value of `regmap_read` is ignored, leading to the use of an uninitialized stack variable.
- [Medium] The patch fails to address identical reset deassertion vulnerabilities in `rk_udphy_init`, leaving the driver susceptible to the exact same SError it claims to fix.
--
commit af7a83cb1b1746054cd095d5743210c65397d6b1
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Handle rk_udphy_reset_deassert_all errors in init check
This commit fixes a theoretical SError issue by ensuring that the return
value of rk_udphy_reset_deassert_all is checked. It bails out early if
deasserting the reset lines fails before attempting further register access.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/message/20260619154903.2225A1F000E9%40smtp.kernel.org
> Fix theoretical issue, that failure to deassert the reset lines would
> result in an SError in follow-up register access. Note, that this cannot
> happen with current code, since the Rockchip reset driver never returns
> any errors for (de)asserting reset lines.
[Severity: Medium]
This is a pre-existing issue, but does this same vulnerability still exist in
rk_udphy_init() where reset deassertions are also unchecked?
For example:
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_init() {
...
rk_udphy_reset_deassert(udphy, "pma_apb");
rk_udphy_reset_deassert(udphy, "pcs_apb");
/* Step 2: set init sequence and phy refclk */
ret = regmap_multi_reg_write(udphy->pma_regmap, ...
...
}
If rk_udphy_reset_deassert() fails here, wouldn't the follow-up
regmap_multi_reg_write() trigger the exact same SError?
Should the reset deassertions in rk_udphy_init() also be checked?
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index f7ff55f03b7cf..b8de0a47a6165 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -936,7 +936,9 @@ static int rk_udphy_get_initial_status(struct rk_udphy *udphy)
> return ret;
> }
>
> - rk_udphy_reset_deassert_all(udphy);
> + ret = rk_udphy_reset_deassert_all(udphy);
> + if (ret)
> + goto exit;
>
> regmap_read(udphy->pma_regmap, CMN_LANE_MUX_AND_EN_OFFSET, &value);
> if (FIELD_GET(CMN_DP_LANE_MUX_ALL, value) && FIELD_GET(CMN_DP_LANE_EN_ALL, value)) {
[Severity: Medium]
This is a pre-existing issue, but does this code evaluate an uninitialized
variable if the read fails?
If regmap_read() fails and returns an error, value is not initialized, and
the subsequent FIELD_GET() checks will operate on uninitialized stack memory.
Should the return value of regmap_read() be checked here?
> dev_info(udphy->dev, "Started with DP PHY pre-enabled; seamless takeover unsupported\n");
> rk_udphy_grfreg_write(udphy->vogrf, &cfg->vogrfcfg[udphy->id].hpd_trigger, false);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=7
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 05/29] phy: rockchip: usbdp: Handle missing clock-names DT property gracefully
From: sashiko-bot @ 2026-06-26 21:12 UTC (permalink / raw)
To: Sebastian Reichel
Cc: neil.armstrong, linux-phy, conor+dt, devicetree, vkoul, olteanv,
robh
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-5-47f682987895@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Incomplete fix: NULL pointer dereference on missing `clock-names` device tree property remains unpatched in sibling rockchip PHY drivers.
--
commit 76423f2a5c05c96f96a6a0ac328646bd389ab49c
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Handle missing clock-names DT property gracefully
This commit fixes a NULL pointer dereference in the rockchip usbdp PHY driver
that occurs when the device tree specifies clocks but omits clock-names. It
adds a check to ensure the clock ID string is set before passing it to
strncmp().
Closes: https://sashiko.dev/#/message/20260619154349.071321F000E9%40smtp.kernel.org
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 2845a012eafcc..3fc8222fcaec2 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -432,6 +432,9 @@ static int rk_udphy_clk_init(struct rk_udphy *udphy, struct device *dev)
>
> /* used for configure phy reference clock frequency */
> for (i = 0; i < udphy->num_clks; i++) {
> + if (!udphy->clks[i].id)
> + continue;
> +
[Severity: High]
This is a pre-existing issue, but does this same missing clock-names
vulnerability need to be addressed in sibling rockchip PHY drivers?
Looking at phy-rockchip-inno-usb2.c in rockchip_usb2phy_clk480m_register(),
the same pattern appears:
for (i = 0; i < rphy->num_clks; i++) {
if (!strncmp(rphy->clks[i].id, "phyclk", 6)) {
...
Similarly, in phy-rockchip-naneng-combphy.c in rockchip_combphy_parse_dt():
for (i = 0; i < priv->num_clks; i++) {
if (!strncmp(priv->clks[i].id, "ref", 3)) {
...
If the device tree omits clock-names for these drivers, won't
devm_clk_bulk_get_all() leave the id field as NULL and cause a NULL pointer
dereference during strncmp() just like it did here?
> if (!strncmp(udphy->clks[i].id, "refclk", 6)) {
> udphy->refclk = udphy->clks[i].clk;
> break;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=5
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 01/29] dt-bindings: phy: rockchip-usbdp: add improved ports scheme
From: sashiko-bot @ 2026-06-26 21:13 UTC (permalink / raw)
To: Sebastian Reichel
Cc: neil.armstrong, devicetree, olteanv, linux-phy, conor+dt, vkoul,
robh
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-1-47f682987895@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing mutual exclusion between `port` and `ports` in device tree schema
--
commit bef609fa8f96018c59387544e50ed1e5818faa7a
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
dt-bindings: phy: rockchip-usbdp: add improved ports scheme
This commit introduces a documented port scheme for the Rockchip USBDP PHY,
modeled after the Qualcomm QMP USB4-USB3-DP PHY controller binding. It adds
a top-level ports node with specific endpoints for USB, DisplayPort, and
SBU connections to properly support USB-C DP AltMode.
> diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-usbdp.yaml b/Documentation/devicetree/bindings/phy/phy-rockchip-usbdp.yaml
> --- a/Documentation/devicetree/bindings/phy/phy-rockchip-usbdp.yaml
> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-usbdp.yaml
> @@ -114,6 +114,29 @@ properties:
> A port node to link the PHY to a TypeC controller for the purpose of
> handling orientation switching.
>
> + ports:
> + $ref: /schemas/graph.yaml#/properties/ports
[Severity: Medium]
Should there be a mutual exclusion constraint between the newly added
ports property and the pre-existing port property?
If a device tree node defines both a top-level port and a ports node
simultaneously, it currently passes schema validation. However, standard
OF graph parsing functions like of_graph_get_next_endpoint() will silently
ignore the top-level port if a ports node is present, creating a shadowing
conflict.
Could this be prevented by adding a oneOf mutual exclusion constraint?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 17/29] phy: rockchip: usbdp: Register DP aux bridge
From: sashiko-bot @ 2026-06-26 21:14 UTC (permalink / raw)
To: Sebastian Reichel
Cc: vkoul, linux-phy, devicetree, conor+dt, robh, olteanv,
neil.armstrong
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-17-47f682987895@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm_aux_bridge_register() is incompatible with the rockchip-usbdp PHY topology, breaking the DP bridge chain when USB and DP ports are split.
--
commit 70d2ba94c5da680bd420eb74b5cffb26d6fad15d
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Register DP aux bridge
This patch adds support to register a DRM auxiliary bridge if the DP
aux channel is connected. This enables DP altmode helper code on
devicetree based platforms.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index e243d92483e09..a204699619b80 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -1505,6 +1507,18 @@ static int rk_udphy_probe(struct platform_device *pdev)
> return ret;
> }
>
> + /*
> + * Only register the DRM bridge, if the DP aux channel is connected.
> + * Some boards use the USBDP PHY only for its USB3 capabilities.
> + */
> + dp_aux_ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 3, 0, 0);
> + if (dp_aux_ep) {
> + ret = drm_aux_bridge_register(dev);
[Severity: High]
Since the PHY detects the DP connector on port 3, is the call to
drm_aux_bridge_register() compatible with this topology?
Looking at how the auxiliary bridge driver probes in
drivers/gpu/drm/bridge/aux-bridge.c, it hardcodes the downstream bridge
lookup to port 0:
drm_aux_bridge_probe()
data->next_bridge = devm_drm_of_get_bridge(..., 0, 0);
If port 0 (the USB output) is unconnected or connected to a non-DRM
component, will devm_drm_of_get_bridge() return an error like -ENODEV?
This looks like it would cause the aux bridge probe to fail, breaking the
DP chain and leaving the DP controller waiting indefinitely with
-EPROBE_DEFER, unless both port 0 and port 3 happen to point to the exact
same connector node.
> + fwnode_handle_put(dp_aux_ep);
> + if (ret)
> + return ret;
> + }
> +
> udphy->phy_u3 = devm_phy_create(dev, dev->of_node, &rk_udphy_usb3_phy_ops);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=17
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 03/29] phy: rockchip: usbdp: Do not lose USB3 PHY status
From: sashiko-bot @ 2026-06-26 21:16 UTC (permalink / raw)
To: Sebastian Reichel
Cc: robh, conor+dt, linux-phy, devicetree, vkoul, neil.armstrong,
olteanv
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-3-47f682987895@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] USB3 reverse orientation changes are never programmed into the PHY hardware because `rk_udphy_orien_sw_set` updates the software structs but fails to trigger a hardware re-initialization.
--
commit 2e0448207186d0a4562665690cfdad4abd051059
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Do not lose USB3 PHY status
This commit fixes an issue where the PHY status is lost during a DisplayPort
mode change. It ensures that the tracking information for USB3 is preserved
so the PHY isn't inadvertently disabled when DP is toggled off.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index a5ffdf7a6e76b..12dfdd470a782 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1008,7 +1008,6 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> rk_udphy_u3_port_disable(udphy, false);
> udphy->mode_change = false;
> } else if (udphy->mode_change) {
> - udphy->status = UDPHY_MODE_NONE;
[Severity: High]
This isn't a bug introduced by this patch, but does keeping the USB mode bit
in the status flag prevent USB3 reverse orientation changes from being
programmed into the PHY hardware?
When a USB-C cable is flipped, the orientation switch handler
rk_udphy_orien_sw_set() updates the software state and requests a re-init:
udphy->phy_needs_reinit = true;
udphy->flip = flipped;
rk_udphy_set_typec_default_mapping(udphy);
Later, rk_udphy_typec_mux_set() evaluates whether to apply this to the
hardware. However, it only calls rk_udphy_power_on() if the USB mode is not
already set in the status flag:
if (... && !(udphy->status & UDPHY_MODE_USB) && ...)
return rk_udphy_power_on(udphy, UDPHY_MODE_USB);
Since DWC3 keeps the PHY enabled permanently and the status flag retains
the USB mode, it appears rk_udphy_power_on() is never invoked to apply the
new lane mapping.
Could this cause the USB3 port to silently fail when plugged in reverse
orientation, since the hardware is never reconfigured?
> if (udphy->mode == UDPHY_MODE_DP)
> rk_udphy_u3_port_disable(udphy, true);
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=3
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 06/29] phy: rockchip: usbdp: Drop seamless DP takeover
From: sashiko-bot @ 2026-06-26 21:16 UTC (permalink / raw)
To: Sebastian Reichel
Cc: devicetree, olteanv, robh, conor+dt, vkoul, neil.armstrong,
linux-phy
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-6-47f682987895@collabora.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unconditionally disabling the active DP PHY while the upstream display controller is still streaming data causes an AXI bus stall and system hang on Rockchip SoCs.
- [High] Forcing DP HPD to disconnected during probe permanently breaks fixed eDP/DP displays.
--
commit 31addd0a9ff6667916e1a40ca73d061ababa8583
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Drop seamless DP takeover
This commit drops support for seamless DP takeover from the bootloader. If a
pre-enabled DP PHY is detected, it logs a warning, forces HPD low, and
unconditionally disables the PHY.
Link: https://lore.kernel.org/linux-phy/20260612164107.C7DB21F000E9@smtp.kernel.org/
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 3fc8222fcaec2..f7ff55f03b7cf 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -938,10 +939,12 @@ static int rk_udphy_get_initial_status(struct rk_udphy *udphy)
> rk_udphy_reset_deassert_all(udphy);
>
> regmap_read(udphy->pma_regmap, CMN_LANE_MUX_AND_EN_OFFSET, &value);
> - if (FIELD_GET(CMN_DP_LANE_MUX_ALL, value) && FIELD_GET(CMN_DP_LANE_EN_ALL, value))
> - udphy->status = UDPHY_MODE_DP;
> - else
> - rk_udphy_disable(udphy);
> + if (FIELD_GET(CMN_DP_LANE_MUX_ALL, value) && FIELD_GET(CMN_DP_LANE_EN_ALL, value)) {
> + dev_info(udphy->dev, "Started with DP PHY pre-enabled; seamless takeover unsupported\n");
> + rk_udphy_grfreg_write(udphy->vogrf, &cfg->vogrfcfg[udphy->id].hpd_trigger, false);
[Severity: High]
Will forcing HPD low here permanently break fixed eDP or DP displays?
While this pulls HPD low to signal a disconnect, fixed eDP/DP boards do not
use Type-C muxes and will never trigger a re-assertion via Type-C events.
Since the upstream DP controller driver does not know about the VO GRF
override, the HPD signal remains permanently forced low, and the display
will be seen as disconnected by the kernel.
> + }
> +
> + rk_udphy_disable(udphy);
[Severity: High]
Could calling rk_udphy_disable() synchronously right after forcing HPD low
trigger an AXI bus stall?
If the DP PHY was left enabled by the bootloader, the upstream display
controller (VOP) might still be actively issuing AXI/AHB transactions to the
PHY.
The HPD state change via GRF is asynchronous, but this disables the PHY clocks
immediately on the very next line. Removing the clock from an active AXI slave
often causes the bus to stall indefinitely, resulting in a hard system freeze.
>
> return 0;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=6
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 25/29] phy: rockchip: usbdp: Clear USB status on PHY exit
From: sashiko-bot @ 2026-06-26 21:17 UTC (permalink / raw)
To: Sebastian Reichel
Cc: robh, olteanv, conor+dt, devicetree, vkoul, neil.armstrong,
linux-phy
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-25-47f682987895@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Clearing `udphy->status` and returning early in `rk_udphy_usb3_phy_exit` bypasses hardware disablement when the status drops to 0, leading to a permanent clock enable count leak and keeping the PHY hardware incorrectly powered on.
--
commit 0522ec66e66f7228817b6bf6fcd2ff4639f6af01
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Clear USB status on PHY exit
This commit modifies the USB3 PHY exit routine to ensure the USB status
flag is cleared when the system is in DP-only mode. This addresses a
case where the USB3 controller is unbound while a DP-only adapter is
plugged in.
Closes: https://sashiko.dev/#/message/20260625-rockchip-usbdp-cleanup-v7-24-38eb3cf654fd%40collabora.com
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 4566822d70c4a..15c18c6e30935 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1322,8 +1322,10 @@ static int rk_udphy_usb3_phy_exit(struct phy *phy)
> udphy->usb_in_use = false;
>
> /* DP only or high-speed */
> - if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
> + if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs) {
> + udphy->status &= ~UDPHY_MODE_USB;
> return 0;
[Severity: Medium]
If udphy->status drops to 0 after this bitwise clear, does returning early
bypass hardware disablement?
It appears this might lead to a permanent clock enable count leak and keep
the PHY hardware incorrectly powered on when a USB3 connection is unbound
or exited while the PHY is in a DP-only mode.
> + }
>
> rk_udphy_power_off(udphy, UDPHY_MODE_USB);
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=25
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 13/29] phy: rockchip: usbdp: Limit DP lane count to muxed lanes
From: sashiko-bot @ 2026-06-26 21:17 UTC (permalink / raw)
To: Sebastian Reichel
Cc: olteanv, robh, conor+dt, devicetree, vkoul, neil.armstrong,
linux-phy
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-13-47f682987895@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the patch's fix is incomplete. The lane count validation added to prevent DP configurations from corrupting USB3 PHY state is bypassed during voltage-only updates. If the Type-C state dynamically reduces the available DP lanes, `udphy->dp_lanes` is updated but the cached `udphy->lanes` remains stale. A subsequent voltage-only update will iterate up to the stale `udphy->lanes` and write DP voltage configurations to the physically reassigned USB3 lanes, corrupting the USB3 PHY state.
--
commit e11a3cd1b181c4fdc9e18a16f2893d3ae7465cb5
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Limit DP lane count to muxed lanes
This commit limits the DisplayPort lane count to 2 if the PHY is
restricted due to USB3 usage. It aims to prevent USB3 state corruption
during voltage configuration.
Closes: https://lore.kernel.org/linux-phy/20260612165546.98E1F1F000E9@smtp.kernel.org/
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 5c38da0dadd22..6de8ac3f91441 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1132,6 +1132,9 @@ static int rk_udphy_dp_phy_verify_link_rate(struct rk_udphy *udphy,
> static int rk_udphy_dp_phy_verify_lanes(struct rk_udphy *udphy,
> struct phy_configure_opts_dp *dp)
> {
> + if (dp->lanes > udphy->dp_lanes)
> + return -EINVAL;
> +
[Severity: High]
This isn't a bug introduced by this patch, but is the fix incomplete for
voltage-only updates?
If a Type-C event renegotiates the connection and restricts udphy->dp_lanes
to 2, udphy->lanes could still hold a stale value (like 4).
Looking at rk_udphy_dp_phy_configure(), the lane verification is skipped
if dp->set_lanes is false:
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_dp_phy_configure() {
...
if (dp->set_lanes) {
ret = rk_udphy_dp_phy_verify_lanes(udphy, dp);
if (ret)
return ret;
}
...
}
If the driver then issues a phy_configure() with set_lanes=false and
set_voltages=true, the voltage loop will iterate using the stale
udphy->lanes value:
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_dp_phy_configure() {
...
if (dp->set_voltages) {
for (i = 0; i < udphy->lanes; i++) {
lane = udphy->dp_lane_sel[i];
...
}
Could this still result in writing DP voltage configurations to reassigned
USB3 lanes, corrupting the USB3 PHY state?
> switch (dp->lanes) {
> case 1:
> case 2:
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=13
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 24/29] phy: rockchip: usbdp: Support going from DP-only mode to USB mode
From: sashiko-bot @ 2026-06-26 21:17 UTC (permalink / raw)
To: Sebastian Reichel
Cc: vkoul, conor+dt, neil.armstrong, robh, olteanv, devicetree,
linux-phy
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-24-47f682987895@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The logic introduced to restore USB functionality fails on subsequent adapter replugs because `udphy->status` permanently retains the `UDPHY_MODE_USB` flag.
--
commit b83098c186ec9041e5338df8dff07f7fa436a60c
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Support going from DP-only mode to USB mode
When a USB-C adapter, which maps all Superspeed lanes to DP is plugged
in, the USB support is disabled in the PHY. When the adapter is
unplugged and a different adapter with USB functionality is plugged in
afterwards, USB functionality is not restored as the USB controller
keeps the PHY enabled for the entire time.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 837a4cb3e4b60..4566822d70c4a 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -1347,6 +1362,17 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
>
> rk_udphy_set_typec_state(udphy, state->mode);
>
> + /*
> + * If the new mode includes USB, but it has not yet been powered
> + * (because the previous mode was DP-only) and the USB PHY was
> + * already initialized by the USB controller, we need to power on
> + * the USB side now since no subsequent phy_init call will come
> + * from the controller.
> + */
> + if ((udphy->mode & UDPHY_MODE_USB) && !(udphy->status & UDPHY_MODE_USB) &&
> + udphy->usb_in_use && !udphy->hs)
> + return rk_udphy_power_on(udphy, UDPHY_MODE_USB);
[Severity: High]
Does this logic handle subsequent replugs correctly if a USB device is plugged
in first?
Since the USB controller keeps the PHY enabled during hotplug (as noted in the
commit message), the UDPHY_MODE_USB flag in udphy->status is not cleared.
If the user then plugs in a DP-only adapter, the PHY is reinitialized for DP
and physically disables the USB3 port.
If the user later unplugs the DP-only adapter and plugs in a USB-only adapter,
won't !(udphy->status & UDPHY_MODE_USB) evaluate to false because the flag was
never cleared? It seems this would skip the call to rk_udphy_power_on() and
leave the USB3 port disabled.
> +
> return 0;
> }
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=24
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 20/29] phy: rockchip: usbdp: Re-init the PHY on orientation change
From: sashiko-bot @ 2026-06-26 21:18 UTC (permalink / raw)
To: Sebastian Reichel
Cc: linux-phy, vkoul, neil.armstrong, robh, conor+dt, devicetree,
olteanv
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-20-47f682987895@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Hardware lane muxing is never actively updated for USB-only devices upon replug or orientation change, breaking USB SuperSpeed in reverse orientation on ports that do not power-cycle the PHY.
--
commit 52b656ef41c03fa9932c56f88505027225e05d96
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Re-init the PHY on orientation change
This commit updates the driver to record cable orientation changes and set the
phy_needs_reinit flag. It aims to fix DP functionality breaking when the cable
is re-plugged with a swapped orientation.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 1bb22fc18c9f9..3d222768dc3c9 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -626,6 +626,7 @@ static int rk_udphy_orien_sw_set(struct typec_switch_dev *sw,
> enum typec_orientation orien)
> {
> struct rk_udphy *udphy = typec_switch_get_drvdata(sw);
> + bool flipped = orien == TYPEC_ORIENTATION_REVERSE;
>
> mutex_lock(&udphy->mutex);
>
> @@ -637,7 +638,10 @@ static int rk_udphy_orien_sw_set(struct typec_switch_dev *sw,
> goto unlock_ret;
> }
>
> - udphy->flip = orien == TYPEC_ORIENTATION_REVERSE;
> + if (udphy->flip != flipped)
> + udphy->phy_needs_reinit = true;
> +
> + udphy->flip = flipped;
[Severity: High]
This is a pre-existing issue, but does this actually flush the re-initialization
to the hardware if the USB PHY is already running?
When a user replugs a USB-only Type-C device in reverse orientation on a port
where the USB controller does not power-cycle the PHY on unplug, for example in
host mode, rk_udphy_orien_sw_set() notes the orientation change and sets
udphy->phy_needs_reinit = true.
However, because the USB controller keeps the PHY powered on,
udphy->status & UDPHY_MODE_USB remains true.
When rk_udphy_typec_mux_set() is called later to handle the mux event, it
appears to skip actively re-initializing the PHY if the USB mode is already
active:
if ((udphy->mode & UDPHY_MODE_USB) && !(udphy->status & UDPHY_MODE_USB) &&
udphy->usb_in_use && !udphy->hs)
return rk_udphy_power_on(udphy, UDPHY_MODE_USB);
return 0;
Could this leave the hardware stuck in the old orientation state and break USB
SuperSpeed, causing it to fall back to USB 2.0 upon reverse replug?
> rk_udphy_set_typec_default_mapping(udphy);
> rk_udphy_usb_bvalid_enable(udphy, true);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=20
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 12/29] phy: rockchip: usbdp: Support single-lane DP
From: sashiko-bot @ 2026-06-26 21:19 UTC (permalink / raw)
To: Sebastian Reichel
Cc: neil.armstrong, vkoul, robh, conor+dt, devicetree, olteanv,
linux-phy
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-12-47f682987895@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Unclocked PHY register access caused by returning 0 on unsupported mode in `rk_udphy_power_on`.
--
commit 835e45901918a9b24aa9d7ccd7b8538db264146b
Author: Zhang Yubing <yubing.zhang@rock-chips.com>
phy: rockchip: usbdp: Support single-lane DP
Implement support for using just a single DisplayPort line.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -1083,18 +1072,17 @@ static int rk_udphy_dp_phy_exit(struct phy *phy)
> static int rk_udphy_dp_phy_power_on(struct phy *phy)
> {
[ ... ]
> ret = rk_udphy_power_on(udphy, UDPHY_MODE_DP);
> if (ret)
> goto unlock;
>
> - rk_udphy_dplane_enable(udphy, dp_lanes);
> + rk_udphy_dplane_enable(udphy, udphy->dp_lanes);
>
> rk_udphy_dplane_select(udphy);
[Severity: High]
This is a pre-existing issue, but I noticed a potential problem in how
we handle the return value of rk_udphy_power_on() here.
If the DP controller powers on the DP PHY when DP mode is not currently
enabled, such as before Type-C negotiation completes or when forced by
userspace bypassing HPD checks, rk_udphy_power_on() returns 0 for
unsupported modes.
Because the return value is 0, execution proceeds to rk_udphy_dplane_enable()
and rk_udphy_dplane_select(). Can this cause a kernel crash or SError
resulting from an MMIO access to unclocked UDPHY_PMA registers?
Should rk_udphy_power_on() return an error code like -EOPNOTSUPP instead of
0 when the mode is not supported, so the check here can properly abort?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=12
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 28/29] phy: rockchip: usbdp: Avoid xHCI SErrors
From: sashiko-bot @ 2026-06-26 21:24 UTC (permalink / raw)
To: Sebastian Reichel
Cc: neil.armstrong, conor+dt, devicetree, robh, linux-phy, olteanv,
vkoul
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-28-47f682987895@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Unconditionally re-enabling the USB3 port during PHY re-initialization overrides the High-Speed only (hs) configuration, potentially leading to an SError on subsequent PHY power off.
--
commit 9d1b08c311cabde1792e9f8ef67ab6b7a5f76005
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Avoid xHCI SErrors
This patch avoids xHCI SErrors by disabling the USB3 port before
re-initializing it when switching between USB3-only, USB3 + DP and DP-only
modes.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index dc166392ba199..7c8b9eaaf3526 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1033,8 +1033,8 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> rk_udphy_u3_port_disable(udphy, false);
[Severity: High]
This is a pre-existing issue, but does this unconditionally re-enable the
USB3 port and override the High-Speed only (hs) configuration?
If the system is configured with a high-speed only USB device, the PHY
initialization via rk_udphy_usb3_phy_init() correctly disables the USB3 port
to safely route the xHCI PIPE clock to the UTMI fallback.
When a DisplayPort cable is plugged in, rk_udphy_power_on() is called. Since
udphy->mode still contains UDPHY_MODE_USB for combo modes, this code
switches the clock source back to the PHY, overriding the disablement.
When the DP cable is disconnected, rk_udphy_dp_phy_power_off() drops the
status to UDPHY_MODE_NONE and powers off the PHY. At this point, the active
xHCI controller runs without a clock, causing an immediate SError on its
next register access.
Should the re-enable conditions here, and similarly after the PHY
re-initialization further down in this function, check if high-speed only
is disabled?
For example:
if ((udphy->mode & UDPHY_MODE_USB) && !udphy->hs)
> udphy->phy_needs_reinit = false;
> } else if (udphy->phy_needs_reinit) {
> - if (udphy->mode == UDPHY_MODE_DP)
> - rk_udphy_u3_port_disable(udphy, true);
> + rk_udphy_u3_port_disable(udphy, true);
> + udelay(10);
>
> ret = rk_udphy_init(udphy);
> if (ret)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=28
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 29/29] phy: rockchip: usbdp: Disable USB3 on probe
From: sashiko-bot @ 2026-06-26 21:24 UTC (permalink / raw)
To: Sebastian Reichel
Cc: devicetree, linux-phy, vkoul, robh, neil.armstrong, olteanv,
conor+dt
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-29-47f682987895@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but missing USB3 port disable in the power-off path recreates the U-Boot failure state on subsequent initialization, which will now permanently fail due to the removal of the -EPROBE_DEFER retry mechanism.
--
Note: The format of this report has been altered to use a free-form summary
due to strict anti-recitation policies preventing direct quoting of the patch diff.
commit 8c774704e1cc3fa0c1dd6e73117e4641a08efb29
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
Subject: phy: rockchip: usbdp: Disable USB3 on probe
This commit disables USB3 support during probe to start with a clean state,
and removes the -EPROBE_DEFER retry mechanism for cmn ana lcpll lock timeouts
since they should only happen on the first try if U-Boot had enabled USB.
[Severity: High]
This isn't a bug introduced by this patch, but removing the -EPROBE_DEFER
retry mechanism makes a pre-existing power-off issue permanently fatal.
In drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_power_off(), when the
PHY transitions to UDPHY_MODE_NONE, the code disables the PHY but does not
disable the USB3 port in the GRF:
rk_udphy_power_off()
udphy->status &= ~mode;
if (udphy->status == UDPHY_MODE_NONE)
rk_udphy_disable(udphy);
When the USB subsystem resumes from suspend or driver unbind/bind, it calls
rk_udphy_power_on(). Since udphy->status is UDPHY_MODE_NONE, it calls
rk_udphy_setup(), which executes the PLL lock:
rk_udphy_power_on()
if (udphy->status == UDPHY_MODE_NONE) {
ret = rk_udphy_setup(udphy);
Because the USB3 port was never disabled in the GRF during power-off, this
recreates the exact unclean state that caused the U-Boot lock timeout. Without
the -EPROBE_DEFER retry mechanism removed by this patch, the PLL lock will
time out and result in a permanent failure to resume.
Should rk_udphy_power_off() also disable the USB3 port by calling
rk_udphy_u3_port_disable(udphy, true) when the status reaches UDPHY_MODE_NONE,
so the next power-on starts with a clean state?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=29
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v2] phy: Add USB3 PHY support to Google Tensor SoC USB PHY driver
From: RD Babiera @ 2026-06-26 21:41 UTC (permalink / raw)
To: sashiko-reviews
Cc: linux-phy, neil.armstrong, olteanv, vkoul, Badhri Jagan Sridharan
In-Reply-To: <20260622180542.4108F1F000E9@smtp.kernel.org>
On Mon, Jun 22, 2026 at 11:05 AM <sashiko-bot@kernel.org> wrote:
> [Severity: High]
> Will this error path leak the reference clock request?
>
> Since set_pmgt_ref_clk_req_n(gphy, true) was called earlier, if
> clk_bulk_prepare_enable() fails we return without deasserting the clock
> request.
>
> The driver core doesn't invoke phy_exit() if phy_init() fails, so the
> request will remain permanently asserted.
>
> [Severity: High]
> Does this error path also leak the reference clock request?
>
> It properly unwinds the clocks and resets, but it misses calling
> set_pmgt_ref_clk_req_n(gphy, false) before returning.
I addressed this in V1 where a phy_init failure would likely result
in the hardware powering off and on again and the request would
clear, but I'll change it for the next version for correctness.
> [Severity: High]
> If program_tca_locked() fails, will hardware programming retries fail
> because the software state was prematurely updated?
>
> If the USB subsystem retries powering on the PHY after an error, the early
> success check at the top of google_usb3_phy_power_on() will hit and return 0
> without actually re-attempting the hardware configuration.
I'll change this so that COMBO_PHY_TCA_READY is set when the initial
PoR => NC check passes, but subsequent power_on calls skip waiting
for the PoR => NC request and allow the program_tca_locked() call
to be made while the internal PHY state is COMBO_PHY_TCA_READY.
> [Severity: High]
> Is the missing .power_off callback going to cause the state machine to
> desync on suspend and resume?
>
> During suspend, without a power off hook, gphy->phy_state remains
> COMBO_PHY_TCA_READY. On resume, google_usb3_phy_power_on() checks this
> state and returns 0 without reprogramming the TCA hardware, which could
> permanently break the link.
The change above should cover this case.
> [Severity: Critical]
> Will unconditionally requesting the usb3 clocks break backwards compatibility
> on systems using older device trees?
>
> If an older device tree only provides USB2 resources, devm_clk_bulk_get()
> will fail here and abort the entire probe sequence.
>
> [Severity: Critical]
> Similarly, will unconditionally requiring the usb3_tca resource break probe
> for older device trees that only define USB2 resources?
>
> If the memory region is missing from the DT, this will return an error and
> prevent the USB2 PHY from working on those devices.
The PHY IP supports the USB3 resources by default, so it makes sense
to bake in the requirements.
Best,
RD
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH net v4 2/2] net: phy: mdio-i2c: defer RollBall bridge probe to PHY discovery
From: Aleksander Jan Bajkowski @ 2026-06-27 10:51 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Petr Wozniak, Russell King, Andrew Lunn, Heiner Kallweit,
David S . Miller, Eric Dumazet, Paolo Abeni, netdev, linux-kernel,
linux-phy, Maxime Chevallier, Bjorn Mork, Marek Behun
In-Reply-To: <20260625082335.3d13d875@kernel.org>
Hi Jakub,
On 25/06/2026 17:23, Jakub Kicinski wrote:
> On Wed, 24 Jun 2026 23:44:19 +0200 Aleksander Jan Bajkowski wrote:
>>> For genuine RollBall modules (e.g. FLYPRO SFP-10GT-CS-30M with Aquantia
>>> AQR113C) the probe now runs after initialization is complete and
>>> correctly returns 0, so PHY detection proceeds normally.
>> The FLPRO SFP module still fails to detect the PHY. It is necessary to
>> increase `module_t_wait` to 20 seconds. Most likely, during this time
>> the module loads the PHY firmware from SPI memory or from the
>> microcontroller (rollball bridge) via MDIO. Same probably applies to
>> most SFP modules with a PHY that load firmware at start-up (AQR113,
>> RTL8261C etc.).
> Just to clarify is FLPRO a typo or a knock off ?
Typo on my comment FLPRO->FLYPRO :)
> Do you want something to be changed here or you're just flagging that
> more follow ups are needed if we want to cover more modules?
I don’t know how this should be fixed. I’m just sharing information.
Even with this patch, most Rollball modules still don’t work. This
patch at least fixes one bug. According to SFF-8472[1], we shouldn’t
communicate with the SFP module until 300ms have elapsed since the
module was inserted. The second problem still remains. 300ms is still
not enough for most Rollball modules. Unfortunately, we’d need to test
a quirk for each module to find out what delay is required. We need a
quirk similar to sfp_fixup_rollball_wait4s. But with a longer delay.
Should the fix be included in this series or follow up?
1. SFF-8472 Rev 12.5a. Table 8-7
Best regards,
Aleksander
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH net v4 2/2] net: phy: mdio-i2c: defer RollBall bridge probe to PHY discovery
From: Maxime Chevallier @ 2026-06-27 12:03 UTC (permalink / raw)
To: Petr Wozniak, olek2, linux, andrew, hkallweit1
Cc: kuba, davem, edumazet, pabeni, netdev, linux-kernel, linux-phy,
bjorn, kabel
In-Reply-To: <20260626163519.10678-1-petr.wozniak@gmail.com>
Hi Petr
On 6/26/26 18:35, Petr Wozniak wrote:
> Maxime Chevallier wrote:
>> I finally got time to test this with a RollBall module, and I
>> confirm what Aleksander says, the RollBall module's PHY doesn't
>> get detected even with this patch. It does work on v7.0 though,
>> so before the bridge probing was introduced.
>
> Thanks a lot for taking the time to test, Maxime - and Aleksander
> for the original report.
>
> That settles it: the deferred-probe approach in patch 2/2 doesn't
> actually restore genuine RollBall PHY detection, and as you both
> confirm it worked before 8fe125892f40 introduced the bridge probing.
> Sashiko's static review flagged the same thing (the probe bypasses the
> PHY discovery retry loop for slow-initializing modules), so the static
> analysis and the two hardware reports all point at the same flaw.
>
> I only have RTL8261BE-based copper modules here, not a genuine RollBall
> one, so I can't develop and verify a proper slow-init timing fix
> (module_t_wait / a retry that waits for the bridge) without the
> hardware to test against.
>
> Given that, my suggestion:
>
> - Please drop patch 2/2 from the series.
>
> - Since 8fe125892f40 regressed genuine RollBall detection and the
> deferred probe doesn't restore it, I think the cleanest fix is to
> revert 8fe125892f40. I'm happy to send that revert if you'd prefer.
> The 5-minute RTL8261BE probe loop it was addressing is handled in our
> downstream tree, so reverting it upstream is fine on our side.
>
> - Patch 1/2 (the mii_bus leak fix) is independent of all this and
> already has Reviewed-by from Maxime and Larysa - it would be good to
> take that one regardless. I can resend it standalone if that's easier.
>
> A proper fix covering slow-firmware modules really needs a genuine
> RollBall module to validate, so it's better owned by someone who has
> that hardware - happy to help review.
>
I only have a single rollball module here, and none with the missing
bridge, so I can't test the full thing either.
In that case, I agree with reverting 8fe125892f40 as this is breaking
all rollball modules right now.
Can you send the revert ?
Maxime
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox