Re: [PATCH v7 06/27] phy: rockchip: usbdp: Drop seamless DP takeover - sashiko-bot

From: sashiko-bot@kernel.org
To: "Sebastian Reichel" <sebastian.reichel@collabora.com>
Cc: conor+dt@kernel.org, robh@kernel.org, vkoul@kernel.org,
	linux-phy@lists.infradead.org, neil.armstrong@linaro.org,
	olteanv@gmail.com, devicetree@vger.kernel.org
Subject: Re: [PATCH v7 06/27] phy: rockchip: usbdp: Drop seamless DP takeover
Date: Thu, 25 Jun 2026 17:58:33 +0000	[thread overview]
Message-ID: <20260625175834.AC26D1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260625-rockchip-usbdp-cleanup-v7-6-38eb3cf654fd@collabora.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditionally disabling the DP PHY during probe can cause a fatal bus stall if the bootloader left the display pipeline active.
--

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

phy: rockchip: usbdp: Drop seamless DP takeover

This commit removes seamless DP takeover support by unconditionally disabling
the DP PHY during initial status checks. It aims to prevent clock resource
leaks on module removal since the DRM stack does not yet support seamless
takeover.

Closes: 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
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -939,9 +939,9 @@ static int rk_udphy_get_initial_status(struct rk_udphy *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);
> +		dev_dbg(udphy->dev, "Started with DP PHY pre-enabled; seamless takeover unsupported\n");
> +
> +	rk_udphy_disable(udphy);

[Severity: High]
Will this unconditional call to rk_udphy_disable() cause a fatal bus stall
if the bootloader left the display pipeline active?

If the bootloader leaves the DP PHY enabled and streaming for a splash
screen, the Video Output Processor (VOP) is actively pushing pixels to the
PHY and depending on its clocks.

By shutting down the PHY, cutting its clocks, and asserting resets here
before the VOP is stopped, the VOP could stall the AXI bus when its
downstream sink abruptly stops. This can lead to an asynchronous external
abort (SError) or a complete system hang.

Even without seamless DRM takeover support, should the PHY remain enabled
until the DRM driver gracefully tears down the display pipeline in the
correct hardware-mandated order (VOP first, then PHY)?

>  
>  	return 0;
>  }

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

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