Re: [PATCH v10 08/36] phy: rockchip: usbdp: Amend SSC modulation deviation

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Medium] Partial state update during the configuration verification phase leaves inconsistent driver state on error.
--

commit 90f0c5bc1613dc3c61ed76de74f75f85b147f759
Author: Frank Wang <frank.wang@rock-chips.com>

phy: rockchip: usbdp: Amend SSC modulation deviation
    
Move SSC modulation deviation into private config of clock.
    
 - 24M: 0x00d4[5:0] = 0x30
 - 26M: 0x00d4[5:0] = 0x33

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index e3f5a26c876a5..cb6acadfbccfb 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c

[ ... ]

While reviewing this, I noticed a pre-existing issue in this file:

drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_dp_phy_configure() {
...
	if (dp->set_rate) {
		ret = rk_udphy_dp_phy_verify_link_rate(udphy, dp);
		if (ret)
			return ret;
	}

	if (dp->set_lanes) {
		ret = rk_udphy_dp_phy_verify_lanes(udphy, dp);
		if (ret)
			return ret;
	}

	if (dp->set_voltages) {
		ret = rk_udphy_dp_phy_verify_voltages(udphy, dp);
		if (ret)
			return ret;
	}
...
}

[Severity: Medium]
This isn't a bug introduced by this patch, but does partial state update
during the configuration verification phase leave inconsistent driver state
on error?

The helpers rk_udphy_dp_phy_verify_link_rate() and
rk_udphy_dp_phy_verify_lanes() prematurely mutate the driver's internal
state before guaranteeing overall configuration success.

If a caller invokes phy_configure() with set_rate=1 and set_voltages=1,
the verify_link_rate check might pass and immediately overwrite
udphy->link_rate.

If the verify_voltages check then fails, the function aborts without
programming the hardware or restoring the old link_rate.

Could a subsequent call setting only voltages then use the corrupted
link_rate to configure the PHY clock inversion registers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703-rockchip-usbdp-cleanup-v10-0-a392711ca8a9@collabora.com?part=8