From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtpout-03.galae.net (smtpout-03.galae.net [185.246.85.4]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 30DEB480965 for ; Tue, 28 Apr 2026 13:19:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.246.85.4 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777382393; cv=none; b=Sc9JmjGDMc3MhQbpqJ9d2NyrK2Fl5zUOnClAgGPq3hDtGFsl/WDQCgC5ayDfwfKH6UM5GTJczSYRkgq9bYb14ZM8cEV5Nrx3KurO4OtRAHaSguM0QVvbFAQsas4R3pflHYq9G6lWImxVb0R8yJooM7CYGtE0jeAGgrGdyyad2Bc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777382393; c=relaxed/simple; bh=O2//zOa7pQv2fJddqgvoxuhX0wrMeb8/Yxiuw1cjemM=; h=Mime-Version:Content-Type:Date:Message-Id:Subject:Cc:To:From: References:In-Reply-To; b=rIwVl730fmGmuWfT/oTV31l4RlkUAufUHygvukWHWB2piYIIPSp/8bDXk/BrFc0hnCDBG/G7fGusE70lORQG+8mkM2PnyG6B6rjoqanbYkmbLbZYsY3ONxQTZyl+klCN2kbO1V+neR4sOKsEvgNM8fGa9jNQeUb19v5dBUBjho0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com; spf=pass smtp.mailfrom=bootlin.com; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b=sWOth0ln; arc=none smtp.client-ip=185.246.85.4 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bootlin.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b="sWOth0ln" Received: from smtpout-01.galae.net (smtpout-01.galae.net [212.83.139.233]) by smtpout-03.galae.net (Postfix) with ESMTPS id 664EE4E42B54; Tue, 28 Apr 2026 13:19:49 +0000 (UTC) Received: from mail.galae.net (mail.galae.net [212.83.136.155]) by smtpout-01.galae.net (Postfix) with ESMTPS id 35812601D0; Tue, 28 Apr 2026 13:19:49 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id 8F75A10728B8B; Tue, 28 Apr 2026 15:19:36 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=dkim; t=1777382387; h=from:subject:date:message-id:to:cc:mime-version:content-type: content-transfer-encoding:in-reply-to:references; bh=/aAmCRebqazUQNBKgBXCiYfFsMA4sAwZ/UUaHHc6oSE=; b=sWOth0lnhQkt1SKJlEOb35K37SRf1IZJdLJs1KtiRkwzxIW3XgVAjw0s+sltiQox7EIO/q RHhfsWsgSTSSEhF93sPb0/Dy3zu9Dmw7mAsyViz9qMwLm+Qq12w9/OaVoMn8gQL6XhCiUh wrg3ym3JZEBGoe4ISfu02867GIQoCSEy33t7hxBysza/OCCiq1nQZOzCHguDaXJnIFrMIT 6+n/b2bYml2USn5ypFL1JD5gOVqcs/K5HXR9RZj9aD362Ai7NHp7VTR2x946YYQ77/2bRJ HY/SPjD4ROWVUHhOKQMazhZ1NiAQkUCEi1gDLWKzrWF1hdpY+ETyTFBs3Srhpg== Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Tue, 28 Apr 2026 15:19:35 +0200 Message-Id: Subject: Re: [PATCH v2 08/11] drm/bridge: adv7511: switch to of_drm_get_bridge_by_endpoint() Cc: "Hui Pu" , "Ian Ray" , "Thomas Petazzoni" , "dri-devel@lists.freedesktop.org" , "linux-kernel@vger.kernel.org" , "linux-arm-msm@vger.kernel.org" , "freedreno@lists.freedesktop.org" , "linux-arm-kernel@lists.infradead.org" To: "Biju Das" , "Maarten Lankhorst" , "Maxime Ripard" , "Thomas Zimmermann" , "David Airlie" , "Simona Vetter" , "Rob Clark" , "Dmitry Baryshkov" , "Abhinav Kumar" , "Jessica Zhang" , "Sean Paul" , "Marijn Suijten" , "Tian Tao" , "Xinwei Kong" , "Sumit Semwal" , "John Stultz" , "Andrzej Hajda" , "Neil Armstrong" , "Robert Foss" , "laurent.pinchart" , "Jonas Karlman" , "Jernej Skrabec" , "tomi.valkeinen" , "Michal Simek" From: "Luca Ceresoli" X-Mailer: aerc 0.20.1 References: <20260428-drm-bridge-alloc-getput-panel_or_bridge-v2-0-4300744a1c47@bootlin.com> <20260428-drm-bridge-alloc-getput-panel_or_bridge-v2-8-4300744a1c47@bootlin.com> In-Reply-To: X-Last-TLS-Session-Version: TLSv1.3 On Tue Apr 28, 2026 at 1:58 PM CEST, Biju Das wrote: > Hi all, > >> -----Original Message----- >> From: dri-devel On Behalf Of B= iju Das >> Sent: 28 April 2026 12:50 >> Subject: RE: [PATCH v2 08/11] drm/bridge: adv7511: switch to of_drm_get_= bridge_by_endpoint() >> >> >> Hi, >> >> Thanks for the patch. >> >> > -----Original Message----- >> > From: dri-devel On Behalf Of >> > Luca Ceresoli >> > Sent: 28 April 2026 10:16 >> > Subject: [PATCH v2 08/11] drm/bridge: adv7511: switch to >> > of_drm_get_bridge_by_endpoint() >> > >> > This driver calls drm_of_find_panel_or_bridge() with a NULL pointer in >> > the @panel parameter, thus using a reduced feature set of that functio= n. >> > Replace this call with the simpler of_drm_get_bridge_by_endpoint(). >> > >> > Since of_drm_get_bridge_by_endpoint() increases the refcount of the >> > returned bridge, ensure it is put on removal. To achieve this, instead >> > of adding an explicit drm_bridge_put(), migrate to the bridge::next_br= idge pointer which is >> automatically put when the bridge is eventually freed. >> > >> > Signed-off-by: Luca Ceresoli >> > --- >> > drivers/gpu/drm/bridge/adv7511/adv7511.h | 1 - >> > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 11 +++++------ >> > 2 files changed, 5 insertions(+), 7 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h >> > b/drivers/gpu/drm/bridge/adv7511/adv7511.h >> > index 8be7266fd4f4..12c95198d9a4 100644 >> > --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h >> > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h >> > @@ -354,7 +354,6 @@ struct adv7511 { >> > enum drm_connector_status status; >> > bool powered; >> > >> > - struct drm_bridge *next_bridge; >> > struct drm_display_mode curr_mode; >> > >> > unsigned int f_tmds; >> > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >> > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >> > index 6bd76c1fb007..498e38579a0f 100644 >> > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >> > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >> > @@ -851,8 +851,8 @@ static int adv7511_bridge_attach(struct drm_bridge= *bridge, >> > struct adv7511 *adv =3D bridge_to_adv7511(bridge); >> > int ret =3D 0; >> > >> > - if (adv->next_bridge) { >> > - ret =3D drm_bridge_attach(encoder, adv->next_bridge, bridge, >> > + if (adv->bridge.next_bridge) { >> > + ret =3D drm_bridge_attach(encoder, adv->bridge.next_bridge, bridge, >> > flags | DRM_BRIDGE_ATTACH_NO_CONNECTOR); >> > if (ret) >> > return ret; >> > @@ -1251,10 +1251,9 @@ static int adv7511_probe(struct i2c_client >> > *i2c) >> > >> > memset(&link_config, 0, sizeof(link_config)); >> > >> > - ret =3D drm_of_find_panel_or_bridge(dev->of_node, 1, -1, NULL, >> > - &adv7511->next_bridge); >> > - if (ret && ret !=3D -ENODEV) >> > - return ret; >> > + adv7511->bridge.next_bridge =3D of_drm_get_bridge_by_endpoint(dev->o= f_node, 1, -1); >> > + if (IS_ERR(adv7511->bridge.next_bridge) && PTR_ERR(adv7511->bridge.n= ext_bridge) !=3D -ENODEV) >> > + return PTR_ERR(adv7511->bridge.next_bridge); >> >> Does it crash, if the error is -EPROBE_DEFER? > > I see a crash with patch [1], which is fixed by avoiding the direct assig= nment. Ah, dammit! Good catch, thanks for the quick testing and follow-up! Indeed this driver assumes next_bridge is either NULL or a valid pointer, but due to the 'if(IS_ERR() && some_other_condition)' now it can also be -ENODEV (not -EPROBE_DEFER, but that's irrelevant). This affects the msm and zynqmp_dp patches equally. I'm sending a v3 soon with these fixed. I'm just not sure which approach to use to fix (same for all the 3 patches). Alternatives are: 1. -ENODEV is accepted, set next_bridge to NULL when it happens: - if (IS_ERR(adv7511->bridge.next_bridge) && PTR_ERR(adv7511->b= ridge.next_bridge) !=3D -ENODEV) - return PTR_ERR(adv7511->bridge.next_bridge); + if (IS_ERR(adv7511->bridge.next_bridge)) { + if (PTR_ERR(adv7511->bridge.next_bridge) =3D=3D -ENOD= EV) + adv7511->bridge.next_bridge =3D NULL; + else + return PTR_ERR(adv7511->bridge.next_bridge); + } 2. let nexxt_bridge hold -ENODEV but ignore it when it is used (only in the attach op, for all 3 drivers): - if (adv->bridge.next_bridge) { + if (!IS_ERR_OR_NULL(adv->bridge.next_bridge)) { While the latter approach involves less code, it might let errors sneak in in case new usages of next_bridge are added with just a NULL check. Opinions about the two? Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com