[PATCH v13 27/35] phy: rockchip: usbdp: Only enable USB3 when not in high-speed mode

[sashiko-bot at kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Returning 0 instead of an error when a requested PHY mode is unsupported causes unclocked register accesses that will trigger a Synchronous External Abort (SError).
--

--- Patch [27]: [PATCH v13 27/35] phy: rockchip: usbdp: Only enable USB3 when not in high-speed mode ---
commit 80f8e58c736697054e63cee31e4afb6d94fb9e90
Author: Sebastian Reichel <sebastian.reichel at collabora.com>

phy: rockchip: usbdp: Only enable USB3 when not in high-speed mode

This commit ensures that USB3 mode is not accidentally enabled during
PHY re-initialization for systems configured as high-speed only via
device tree.

Fixes: 2f70bbddeb45 ("phy: rockchip: add usbdp combo phy driver")
Reported-by: Sashiko <sashiko-bot at kernel.org>
Closes: https://sashiko.dev/#/message/20260626212424.C215E1F000E9%40smtp.kernel.org
Signed-off-by: Sebastian Reichel <sebastian.reichel at collabora.com>

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 701c92ed6e2de..59b836a3796ce 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1009,7 +1009,7 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)

[Severity: High]
This is a pre-existing issue, but I noticed a potential problem in
rk_udphy_power_on() when an unsupported mode is requested.

Earlier in the same function:

    if (!(udphy->mode & mode)) {
        dev_info(udphy->dev, "mode 0x%02x is not support\n", mode);
        return 0;
    }

If phy_power_on() is called by a consumer like the DP driver while
udphy->mode lacks UDPHY_MODE_DP, this returns 0 instead of an error code.

The caller rk_udphy_dp_phy_power_on() then receives this false success
value:

    ret = rk_udphy_power_on(udphy, UDPHY_MODE_DP);
    if (ret)
        return ret;

    rk_udphy_dp_lane_enable(udphy, udphy->dp_lanes);

Could this cause rk_udphy_dp_lane_enable() to access pma_regmap while
the PHY APB clocks remain disabled? 

Since rk_udphy_setup() would not have been called, accessing unclocked
APB registers here could trigger a Synchronous External Abort (SError).

Would it be safer to return an error like -EOPNOTSUPP instead of 0 when
the requested PHY mode is unsupported?

>  		if (ret)
>  			return ret;
>  
> -		if (udphy->mode & UDPHY_MODE_USB)
> +		if (!udphy->hs && udphy->mode & UDPHY_MODE_USB)
>  			rk_udphy_u3_port_disable(udphy, false);
>  		udphy->phy_needs_reinit = false;
>  	} else if (udphy->phy_needs_reinit) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714-rockchip-usbdp-cleanup-v13-0-6cb3e769d4c5@collabora.com?part=27