Re: [PATCH v7 02/27] phy: rockchip: usbdp: Update mode_change after error handling - sashiko-bot

From: sashiko-bot@kernel.org
To: "Sebastian Reichel" <sebastian.reichel@collabora.com>
Cc: devicetree@vger.kernel.org, olteanv@gmail.com,
	neil.armstrong@linaro.org, linux-phy@lists.infradead.org,
	conor+dt@kernel.org, vkoul@kernel.org, robh@kernel.org
Subject: Re: [PATCH v7 02/27] phy: rockchip: usbdp: Update mode_change after error handling
Date: Thu, 25 Jun 2026 18:00:00 +0000	[thread overview]
Message-ID: <20260625180000.CD5371F00A3D@smtp.kernel.org> (raw)
In-Reply-To: <20260625-rockchip-usbdp-cleanup-v7-2-38eb3cf654fd@collabora.com>

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

Pre-existing issues:
- [High] Asynchronous mode changes orphan PHY status bits causing permanent clock and PHY power leaks.
--

commit 1706823bf92c7aae72801ef7a1f3c4258e09e79e
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

phy: rockchip: usbdp: Update mode_change after error handling

This commit updates rk_udphy_power_on() to clear the mode_change flag
only after all potential errors have been handled. This ensures that
if initialization fails, the driver will correctly attempt to
reinitialize on the next try.

Closes: https://lore.kernel.org/linux-phy/20260612163835.8D5471F000E9@smtp.kernel.org/

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index fba35510d88ce..a5ffdf7a6e76b 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1000,15 +1000,14 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
>  	}
>  
>  	if (udphy->status == UDPHY_MODE_NONE) {
> -		udphy->mode_change = false;
>  		ret = rk_udphy_setup(udphy);
>  		if (ret)
>  			return ret;
>  
>  		if (udphy->mode & UDPHY_MODE_USB)
>  			rk_udphy_u3_port_disable(udphy, false);

[Severity: High]
This isn't a bug introduced by this patch, but could asynchronous mode
changes orphan PHY status bits here and cause permanent clock and power
leaks?

If a Type-C AltMode is negotiated that drops a previously available mode
(for example, dropping USB when transitioning to TYPEC_DP_STATE_C), the
Type-C mux callback in rk_udphy_typec_mux_set() updates the volatile
udphy->mode asynchronously:

    rk_udphy_set_typec_state(udphy, state->mode);

When the USB framework reacts and calls phy_exit(), rk_udphy_usb3_phy_exit()
checks the newly updated mode instead of the active status:

    if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
        return 0;

    rk_udphy_power_off(udphy, UDPHY_MODE_USB);

Because udphy->mode no longer contains the USB bit, the driver aborts early
without actually powering off the hardware.

Additionally, rk_udphy_power_off() itself redundantly checks udphy->mode:

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

This leaves the active mode bit permanently orphaned in udphy->status,
which prevents rk_udphy_disable() from ever executing. Should the teardown
paths be validating against udphy->status instead of udphy->mode to avoid
this desynchronization?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625-rockchip-usbdp-cleanup-v7-0-38eb3cf654fd@collabora.com?part=2

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy