From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 7A5DAD1951A for ; Mon, 26 Jan 2026 21:57:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:References: Subject:From:To:Cc:Message-Id:Date:Content-Type:Content-Transfer-Encoding: Mime-Version:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=JPwZyDe4OThoaY1OrbYDXoRov92xdqHIGYafSht4NfM=; b=hDJE0GUPpWub17RsHJDFM9RZNi 2uP5oUKQWjpDfWc9Qf7CVyqc5GEBJq/rQ49CWEHRvqyKEoNYDuWj1qvxH9Nw2aIssbf3pYfhBXF+S iGA6WgKdMeVhMMh1ENxwFkluPVWNnOxKEWnfg1pDtbllB1xRgyK0HyfOHPCF2+odokZjhCEeJHwqh oc/C8tVE+PgKxbfRFp9j5T6fXJkN8p2+OJXcFGLGHCKB4pKUhHuwQN0LkQNgEpxbEwi8AE8g3iupp p5vlCKn1JhUoIISweOgLT12zs+uFmhLLf2X5gPivT796DXAngMI+V6QNG/VM8Rf6fhle2fI1YyIBl 0cu1+YOA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1vkUak-0000000DI7X-0rPf; Mon, 26 Jan 2026 21:57:50 +0000 Received: from smtpout-04.galae.net ([185.171.202.116]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1vkUag-0000000DI70-3Dxd for linux-arm-kernel@lists.infradead.org; Mon, 26 Jan 2026 21:57:48 +0000 Received: from smtpout-01.galae.net (smtpout-01.galae.net [212.83.139.233]) by smtpout-04.galae.net (Postfix) with ESMTPS id 47492C214D3; Mon, 26 Jan 2026 21:57:45 +0000 (UTC) Received: from mail.galae.net (mail.galae.net [212.83.136.155]) by smtpout-01.galae.net (Postfix) with ESMTPS id 5396260717; Mon, 26 Jan 2026 21:57:43 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id 874EC119A8652; Mon, 26 Jan 2026 22:57:36 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=dkim; t=1769464662; h=from:subject:date:message-id:to:cc:mime-version:content-type: content-transfer-encoding:in-reply-to:references; bh=JPwZyDe4OThoaY1OrbYDXoRov92xdqHIGYafSht4NfM=; b=LCXGMY+ZTWlhL1gecMErzIScZDyGU0NQZPJmDtsoIaTKfhqkz/dw0BOfPXjNi+zRc53RN8 gZ+HMhMCK7t8zArJcfx9ysmuPANr0TGU5iT6jyyteXZr5mQmCEBvtCMI/wCpXK+vQUIKBh 5JLvSGLcyi4iik+iFHZW7TpDhBmqiOi6mrVxtIN5dTlqJChxN05zbgtWK5qhl7N6N1XpTm jQchp++A4t4Hg+f7SR55Z6Gm+pcNUUMp4voqvkFZS5eaggk+kqMygbagGhGI92yko1Iq4j cZqCoDJ/KgHtQpjNEDADwc/TP1VB2/DSmujZ6JyRxeObbxSWkCA1a51BafitOg== Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Mon, 26 Jan 2026 22:57:35 +0100 Message-Id: Cc: "Hui Pu" , "Thomas Petazzoni" , , , , To: "Luca Ceresoli" , "Liu Ying" , "Andrzej Hajda" , "Neil Armstrong" , "Robert Foss" , "Laurent Pinchart" , "Jonas Karlman" , "Jernej Skrabec" , "Maarten Lankhorst" , "Maxime Ripard" , "Thomas Zimmermann" , "David Airlie" , "Simona Vetter" , "Shawn Guo" , "Sascha Hauer" , "Pengutronix Kernel Team" , "Fabio Estevam" From: "Luca Ceresoli" Subject: Re: [PATCH v4 4/4] drm/bridge: imx8qxp-pixel-link: get/put the next bridge X-Mailer: aerc 0.20.1 References: <20260107-drm-bridge-alloc-getput-drm_of_find_bridge-v4-0-a62b4399a6bf@bootlin.com> <20260107-drm-bridge-alloc-getput-drm_of_find_bridge-v4-4-a62b4399a6bf@bootlin.com> In-Reply-To: X-Last-TLS-Session-Version: TLSv1.3 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20260126_135746_951556_1E4C1C72 X-CRM114-Status: GOOD ( 19.05 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hello Liu, On Mon Jan 26, 2026 at 7:18 PM CET, Luca Ceresoli wrote: >>> @@ -260,7 +259,7 @@ static int imx8qxp_pixel_link_find_next_bridge(stru= ct imx8qxp_pixel_link *pl) >>> { >>> struct device_node *np =3D pl->dev->of_node; >>> struct device_node *port; >>> - struct drm_bridge *selected_bridge =3D NULL; >>> + struct drm_bridge *selected_bridge __free(drm_bridge_put) =3D NULL; >>> u32 port_id; >>> bool found_port =3D false; >>> int reg; >>> @@ -297,7 +296,8 @@ static int imx8qxp_pixel_link_find_next_bridge(stru= ct imx8qxp_pixel_link *pl) >>> continue; >>> } >>> >>> - struct drm_bridge *next_bridge =3D of_drm_find_bridge(remote); >>> + struct drm_bridge *next_bridge __free(drm_bridge_put) =3D >>> + of_drm_find_and_get_bridge(remote); >>> if (!next_bridge) >>> return -EPROBE_DEFER; >>> >>> @@ -305,12 +305,14 @@ static int imx8qxp_pixel_link_find_next_bridge(st= ruct imx8qxp_pixel_link *pl) >>> * Select the next bridge with companion PXL2DPI if >>> * present, otherwise default to the first bridge >>> */ >>> - if (!selected_bridge || of_property_present(remote, "fsl,companion-p= xl2dpi")) >>> - selected_bridge =3D next_bridge; >>> + if (!selected_bridge || of_property_present(remote, "fsl,companion-p= xl2dpi")) { >>> + drm_bridge_put(selected_bridge); >>> + selected_bridge =3D drm_bridge_get(next_bridge); >> >> Considering selecting the first bridge without the companion pxl2dpi, >> there would be a superfluous refcount for the selected bridge: >> >> 1) of_drm_find_and_get_bridge: refcount =3D 1 >> 2) drm_bridge_put: noop, since selected_bridge is NULL, refcount =3D 1 >> 3) drm_bridge_get: refcount =3D 2 >> 4) drm_bridge_put(__free): refcount =3D 1 >> 5) drm_bridge_get: for the pl->bridge.next_bridge, refcount =3D 2 > > Here you are missing one put. There are two drm_bridge_put(__free), one f= or > next_bridge and one for selected_bridge. So your list should rather be: > > 1) next_bridge =3D of_drm_find_and_get_bridge: refcount =3D 1 > 2) drm_bridge_put(selected_bridge): noop, since selected_bridge is NULL, = refcount =3D 1 > 3) selected_bridge =3D drm_bridge_get: refcount =3D 2 > 4) drm_bridge_put(next_bridge) [__free at loop scope end]: refcount =3D 1 > 5) pl->bridge.next_bridge =3D drm_bridge_get(), refcount =3D 2 > 6) drm_bridge_put(selected_bridge) [__free at function scope end]: refcou= nt =3D 1 > > The idea is that for each pointer (which is a reference) we get a referen= ce > (refcount++) when the pointer is set and put the reference when that same > pointer goes out of scope or is reset to NULL. "the pointer is set" can b= e > either of_drm_find_and_get_bridge() or an assignment, as each of these > operations creates another reference (pointer) to the same bridge. > > Does it look correct? Based on this discussion I thought the commit message could be clearer, and rewrote it as: -----[no changes from here...]----- drm/bridge: imx8qxp-pixel-link: get/put the next bridge This driver obtains a bridge pointer from of_drm_find_bridge() in the p= robe function and stores it until driver removal. of_drm_find_bridge() is deprecated. Move to of_drm_find_and_get_bridge() for the bridge to be refcounted and use bridge->next_bridge to put the reference on deallocation. -----[...to here]----- To keep the code as simple and reliable as possible, get a reference fo= r each pointer that stores a drm_bridge address when it is stored and rel= ease when the pointer is set to NULL or goes out of scope. The involved poin= ters are: * next_bridge loop-local variable: - get reference by of_drm_find_and_get_bridge() - put reference at the end of the loop iteration (__free) * selected_bridge function-local variable: - get reference when written (by copy from next_bridge) - put reference at function exit (__free) - put reference before reassignment too * pl->bridge.next_bridge, tied to struct imx8qxp_pixel_link lifetime: - get reference when written (by copy from selected_bridge) - put reference when the struct imx8qxp_pixel_link embedding the struct drm_bridge is destroyed (struct drm_bridge::next_bridge) Do you think it's better now? Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com