Re: [PATCH v14 7/9] drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection

From: "Luca Ceresoli" <luca.ceresoli@bootlin.com>
To: "Chaoyi Chen" <chaoyi.chen@rock-chips.com>
Cc: "Heikki Krogerus" <heikki.krogerus@linux.intel.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Dmitry Baryshkov" <dmitry.baryshkov@oss.qualcomm.com>,
	"Peter Chen" <hzpeterchen@gmail.com>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Vinod Koul" <vkoul@kernel.org>,
	"Kishon Vijay Abraham I" <kishon@kernel.org>,
	"Heiko Stuebner" <heiko@sntech.de>,
	"Sandy Huang" <hjc@rock-chips.com>,
	"Andy Yan" <andy.yan@rock-chips.com>,
	"Yubing Zhang" <yubing.zhang@rock-chips.com>,
	"Frank Wang" <frank.wang@rock-chips.com>,
	"Andrzej Hajda" <andrzej.hajda@intel.com>,
	"Neil Armstrong" <neil.armstrong@linaro.org>,
	"Robert Foss" <rfoss@kernel.org>,
	"Laurent Pinchart" <Laurent.pinchart@ideasonboard.com>,
	"Jonas Karlman" <jonas@kwiboo.se>,
	"Jernej Skrabec" <jernej.skrabec@gmail.com>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Amit Sunil Dhamne" <amitsd@google.com>,
	"Dragan Simic" <dsimic@manjaro.org>,
	"Johan Jonker" <jbx6244@gmail.com>,
	"Diederik de Haas" <didi.debian@cknow.org>,
	"Peter Robinson" <pbrobinson@gmail.com>,
	<linux-usb@vger.kernel.org>, <devicetree@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-phy@lists.infradead.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-rockchip@lists.infradead.org>,
	<dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH v14 7/9] drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection
Date: Tue, 03 Mar 2026 09:52:49 +0100	[thread overview]
Message-ID: <DGT0Q3TGTG26.KD9DBZYH63DY@bootlin.com> (raw)
In-Reply-To: <601a8b51-7411-429d-91a1-0633cabce9ee@rock-chips.com>

Hello Chaoyi,

On Tue Mar 3, 2026 at 1:44 AM CET, Chaoyi Chen wrote:
>>> +static int cdn_dp_parse_next_bridge_dt(struct cdn_dp_device *dp)
>>> +{
>>> +	struct device_node *np = dp->dev->of_node;
>>> +	struct device_node *port __free(device_node) = of_graph_get_port_by_id(np, 1);
>>> +	struct drm_bridge *bridge;
>>> +	int count = 0;
>>> +	int ret = 0;
>>> +	int i;
>>> +
>>> +	/* If device use extcon, do not use hpd bridge */
>>> +	for (i = 0; i < dp->ports; i++) {
>>> +		if (dp->port[i]->extcon) {
>>> +			dp->bridge_count = 1;
>>> +			return 0;
>>> +		}
>>> +	}
>>> +
>>> +	/* One endpoint may correspond to one next bridge. */
>>> +	for_each_of_graph_port_endpoint(port, dp_ep) {
>>> +		struct device_node *next_bridge_node __free(device_node) =
>>> +			of_graph_get_remote_port_parent(dp_ep);
>>> +
>>> +		bridge = of_drm_find_bridge(next_bridge_node);
>>> +		if (!bridge) {
>>> +			ret = -EPROBE_DEFER;
>>> +			goto out;
>>> +		}
>>> +
>>> +		drm_bridge_get(bridge);
>>> +		dp->next_bridge_valid = true;
>>> +		dp->next_bridge_list[count] = bridge;
>>
>> Correct, but the drm_bridge_get() slightly far away from the assignement is
>> a bit misleading. I hadn't seen it initially so I suspected a missing get.
>>
>> I suggest to do it in a single statement, for clarity:
>>
>> 		dp->next_bridge_list[count] = bridgedrm_bridge_get(bridge);
>>
>
> I think the bridgedrm_bridge_get here seems to be a typo?

Oops, of course! :)

As you probably guessed, I was meaning:

		dp->next_bridge_list[count] = drm_bridge_get(bridge);

> Anyway, thank you very much for your efforts on the bridge lifetime.
> I will fix this in the next version :)

Thanks!

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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