Re: [PATCH v8 17/29] phy: rockchip: usbdp: Register DP aux bridge - sashiko-bot

From: sashiko-bot@kernel.org
To: "Sebastian Reichel" <sebastian.reichel@collabora.com>
Cc: vkoul@kernel.org, linux-phy@lists.infradead.org,
	devicetree@vger.kernel.org, conor+dt@kernel.org, robh@kernel.org,
	olteanv@gmail.com, neil.armstrong@linaro.org
Subject: Re: [PATCH v8 17/29] phy: rockchip: usbdp: Register DP aux bridge
Date: Fri, 26 Jun 2026 21:14:44 +0000	[thread overview]
Message-ID: <20260626211445.1D1151F000E9@smtp.kernel.org> (raw)
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