[PATCH v2 00/11] drm/bridge: handle refcounting for bridge-only callers of drm_of_find_panel_or_bridge()

[PATCH v2 00/11] drm/bridge: handle refcounting for bridge-only callers of drm_of_find_panel_or_bridge()

[Luca Ceresoli]

This series converts all the bridge-only callers of the deprecated
drm_of_find_panel_or_bridge() API to a new, simpler API that handles bridge
refcounting.

== Series description

 * Patch 1 (new in v2) makes drm_bridge_get/put() ignore ERR_PTR pointers,
   not only NULL pointers
 * Patch 2 introduces of_drm_get_bridge_by_endpoint() as a replacement for
   bridge-only calls to drm_of_find_panel_or_bridge(); the new function
   refcounts the bridge and is simpler
 * The following patches convert all bridge-only users to the new API
 * The last patch forbids new bridge-only calls to
   drm_of_find_panel_or_bridge()

== Grand plan

This is part of the work to support hotplug of DRM bridges. The grand plan
was discussed in [0].

Here's the work breakdown (➜ marks the current series):

 1. ➜ add refcounting to DRM bridges struct drm_bridge,
      based on devm_drm_bridge_alloc()
    A. ✔ add new alloc API and refcounting (v6.16)
    B. ✔ convert all bridge drivers to new API (v6.17)
    C. ✔ kunit tests (v6.17)
    D. ✔ add get/put to drm_bridge_add/remove() + attach/detach()
         and warn on old allocation pattern (v6.17)
    E. ➜ add get/put on drm_bridge accessors
       1. ✔ drm_bridge_chain_get_first_bridge(), add cleanup action (v6.18)
       2. ✔ drm_bridge_get_prev_bridge() (v6.18)
       3. ✔ drm_bridge_get_next_bridge() (v6.19)
       4. ✔ drm_for_each_bridge_in_chain() (v6.19)
       5. ✔ drm_bridge_connector_init (v6.19)
       6. ✔ protect encoder bridge chain with a mutex (v7.2)
       7. ➜ of_drm_find_bridge
          a. ✔ add of_drm_get_bridge() (v7.0),
               convert basic direct users (v7.0-v7.1)
          b. ✔ convert direct of_drm_get_bridge() users, part 2 (v7.0)
          c. ✔ convert direct of_drm_get_bridge() users, part 3 (v7.0)
          d. ✔ convert direct of_drm_get_bridge() users, part 4 (v7.1-v7.2)
          e. ➜ convert bridge-only drm_of_find_panel_or_bridge() users
       8. drm_of_find_panel_or_bridge, *_of_get_bridge
       9. ✔ enforce drm_bridge_add before drm_bridge_attach (v6.19)
    F. ✔ debugfs improvements
       1. ✔ add top-level 'bridges' file (v6.16)
       2. ✔ show refcount and list lingering bridges (v6.19)
 2. … handle gracefully atomic updates during bridge removal
    A. ✔ Add drm_bridge_enter/exit() to protect device resources (v7.0)
    B. … protect private_obj removal from list
    C. ✔ Add drm_bridge_clear_and_put() (v7.1)
 3. … DSI host-device driver interaction
 4. ✔ removing the need for the "always-disconnected" connector
 5. ✔ Migrate i.MX LCDIF driver to bridge-connector (v7.2)
 6. … DRM bridge hotplug
    A. … Bridge hotplug management in the DRM core
       1. … bridge-connector: attach encoder to the connector
    B.   Device tree description

[0] https://lore.kernel.org/lkml/20250206-hotplug-drm-bridge-v6-0-9d6f2c9c3058@bootlin.com/#t

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
Changes in v2:
- Added patch to ignore ERR_PTR values in drm_bridge_get/put()
- Changed API to return the bridge (or a ERR_PTR) in the return value,
  not as a double-pointer output parameter
- Adapted all patches to the new API, dropped Dmitry's review tags as the
  patches are all modified
- Removed bouncing addresses from Cc list
- Link to v1: https://patch.msgid.link/20260413-drm-bridge-alloc-getput-panel_or_bridge-v1-0-acd01cd79a1f@bootlin.com

---
Luca Ceresoli (11):
      drm/bridge: drm_bridge_get/put(): ignore ERR_PTR
      drm/bridge: add of_drm_get_bridge_by_endpoint()
      drm/msm/hdmi: switch to of_drm_get_bridge_by_endpoint()
      drm/hisilicon/kirin: switch to of_drm_get_bridge_by_endpoint()
      drm/bridge: chrontel-ch7033: switch to of_drm_get_bridge_by_endpoint()
      drm/bridge: lontium-lt9611uxc: switch to of_drm_get_bridge_by_endpoint()
      drm/bridge: lt9611: switch to of_drm_get_bridge_by_endpoint()
      drm/bridge: adv7511: switch to of_drm_get_bridge_by_endpoint()
      drm/bridge: lt8713sx: switch to of_drm_get_bridge_by_endpoint()
      drm: zynqmp_dp: switch to of_drm_get_bridge_by_endpoint()
      drm: of: forbid bridge-only calls to drm_of_find_panel_or_bridge()

 drivers/gpu/drm/bridge/adv7511/adv7511.h     |  1 -
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 11 +++---
 drivers/gpu/drm/bridge/chrontel-ch7033.c     | 28 +++++++--------
 drivers/gpu/drm/bridge/lontium-lt8713sx.c    | 10 +++---
 drivers/gpu/drm/bridge/lontium-lt9611.c      |  9 +++--
 drivers/gpu/drm/bridge/lontium-lt9611uxc.c   |  9 +++--
 drivers/gpu/drm/drm_bridge.c                 | 53 ++++++++++++++++++++++++----
 drivers/gpu/drm/drm_of.c                     | 26 +++++++-------
 drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c |  9 +++--
 drivers/gpu/drm/msm/hdmi/hdmi.c              |  7 ++--
 drivers/gpu/drm/xlnx/zynqmp_dp.c             | 13 ++++---
 include/drm/drm_bridge.h                     |  8 +++++
 12 files changed, 115 insertions(+), 69 deletions(-)
---
base-commit: 6dbe0c5375e4f88c8d7c90790e0ea29d565b5d0a
change-id: 20260410-drm-bridge-alloc-getput-panel_or_bridge-42501b38eaad

Best regards,
--  
Luca Ceresoli <luca.ceresoli@bootlin.com>

[PATCH v2 01/11] drm/bridge: drm_bridge_get/put(): ignore ERR_PTR

[Luca Ceresoli]

Most functions returning a struct drm_bridge pointer currently return a
valid pointer or NULL, but this restricts their ability to return an error
code describing the error kind.

In preparation to have new APIs that can return a struct drm_bridge pointer
holding an ERR_PTR (and for those which already do) make drm_bridge_get()
and drm_bridge_put() ignore ERR_PTR values, just like they ignore NULL
pointers.

This will avoid annoying error checking in many places and the risk of
missing error checks.

Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Link: https://lore.kernel.org/all/20260318152533.GA633439@killaraus.ideasonboard.com/
Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Link: https://lore.kernel.org/all/omlnswxukeqgnatzdvooaashgkfcacjevkvbkm6xt33itgua2k@jcmzll2w6kdq/
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

Patch added in v2
---
 drivers/gpu/drm/drm_bridge.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 986e4c79a4e0..cea5babbe99e 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -282,7 +282,7 @@ static void __drm_bridge_free(struct kref *kref)
 
 /**
  * drm_bridge_get - Acquire a bridge reference
- * @bridge: DRM bridge; if NULL this function does nothing
+ * @bridge: DRM bridge; if NULL or an ERR_PTR this function does nothing
  *
  * This function increments the bridge's refcount.
  *
@@ -291,7 +291,7 @@ static void __drm_bridge_free(struct kref *kref)
  */
 struct drm_bridge *drm_bridge_get(struct drm_bridge *bridge)
 {
-	if (bridge)
+	if (!IS_ERR_OR_NULL(bridge))
 		kref_get(&bridge->refcount);
 
 	return bridge;
@@ -300,7 +300,7 @@ EXPORT_SYMBOL(drm_bridge_get);
 
 /**
  * drm_bridge_put - Release a bridge reference
- * @bridge: DRM bridge; if NULL this function does nothing
+ * @bridge: DRM bridge; if NULL or an ERR_PTR this function does nothing
  *
  * This function decrements the bridge's reference count and frees the
  * object if the reference count drops to zero.
@@ -310,7 +310,7 @@ EXPORT_SYMBOL(drm_bridge_get);
  */
 void drm_bridge_put(struct drm_bridge *bridge)
 {
-	if (bridge)
+	if (!IS_ERR_OR_NULL(bridge))
 		kref_put(&bridge->refcount, __drm_bridge_free);
 }
 EXPORT_SYMBOL(drm_bridge_put);
@@ -319,8 +319,8 @@ EXPORT_SYMBOL(drm_bridge_put);
  * drm_bridge_clear_and_put - Given a bridge pointer, clear the pointer
  *                            then put the bridge
  * @bridge_pp: pointer to pointer to a struct drm_bridge; ``bridge_pp``
- *             must be non-NULL; if ``*bridge_pp`` is NULL this function
- *             does nothing
+ *             must be non-NULL; if ``*bridge_pp`` is NULL or an ERR_PTR
+ *             this function does nothing
  *
  * Helper to put a DRM bridge, but only after setting its pointer to
  * NULL. Useful when a struct drm_bridge reference must be dropped without

-- 
2.53.0

[PATCH v2 02/11] drm/bridge: add of_drm_get_bridge_by_endpoint()

[Luca Ceresoli]

drm_of_find_panel_or_bridge() is widely used, but many callers pass NULL
into the @panel or the @bridge arguments, thus making a very partial usage
of this rather complex function.

Besides, the bridge returned in @bridge is not refcounted, thus making this
API unsafe when DRM bridge hotplug will be introduced.

Solve both issues for the cases of calls to drm_of_find_panel_or_bridge()
with a NULL @panel pointer by adding a new function that only looks for
bridges (and is thus much simpler) and increments the refcount of the
returned bridge.

The new function is identical to drm_of_find_panel_or_bridge() except it:

 - handles bridge refcounting: uses of_drm_find_and_get_bridge() instead of
   of_drm_find_bridge() internally to return a refcounted bridge
 - is simpler to use: just takes no @panel parameter, returns the pointer
   in the return value instead of a double pointer argument
 - has a simpler implementation: it is equal to
   drm_of_find_panel_or_bridge() after removing the code that becomes dead
   when @panel == NULL

Also add this function to drm_bridge.c and not drm_of.c because it returns
bridges only.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

Changes in v2:
- return the bridge in the return value, not a double pointer
---
 drivers/gpu/drm/drm_bridge.c | 41 +++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_bridge.h     |  8 ++++++++
 2 files changed, 49 insertions(+)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index cea5babbe99e..8a4141bb7b31 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -1582,6 +1582,47 @@ struct drm_bridge *of_drm_find_bridge(struct device_node *np)
 	return bridge;
 }
 EXPORT_SYMBOL(of_drm_find_bridge);
+
+/**
+ * of_drm_get_bridge_by_endpoint - return DRM bridge connected to a port/endpoint
+ * @np: device tree node containing output ports
+ * @port: port in the device tree node, or -1 for the first port found
+ * @endpoint: endpoint in the device tree node, or -1 for the first endpoint found
+ *
+ * Given a DT node's port and endpoint number, find the connected node and
+ * return the associated drm_bridge device.
+ *
+ * The refcount of the returned bridge is incremented. Use drm_bridge_put()
+ * when done with it.
+ *
+ * Returns a pointer to the connected drm_bridge, or a negative error on failure
+ */
+struct drm_bridge *of_drm_get_bridge_by_endpoint(const struct device_node *np,
+						 int port, int endpoint)
+{
+	struct drm_bridge *bridge;
+
+	/*
+	 * of_graph_get_remote_node() produces a noisy error message if port
+	 * node isn't found and the absence of the port is a legit case here,
+	 * so at first we silently check whether graph presents in the
+	 * device-tree node.
+	 */
+	if (!of_graph_is_present(np))
+		return ERR_PTR(-ENODEV);
+
+	struct device_node *remote __free(device_node) =
+		of_graph_get_remote_node(np, port, endpoint);
+	if (!remote)
+		return ERR_PTR(-ENODEV);
+
+	bridge = of_drm_find_and_get_bridge(remote);
+	if (!bridge)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	return bridge;
+}
+EXPORT_SYMBOL_GPL(of_drm_get_bridge_by_endpoint);
 #endif
 
 /**
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index d6cd0f5af045..c96398460618 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -1327,6 +1327,8 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
 #ifdef CONFIG_OF
 struct drm_bridge *of_drm_find_and_get_bridge(struct device_node *np);
 struct drm_bridge *of_drm_find_bridge(struct device_node *np);
+struct drm_bridge *of_drm_get_bridge_by_endpoint(const struct device_node *np,
+						 int port, int endpoint);
 #else
 static inline struct drm_bridge *of_drm_find_and_get_bridge(struct device_node *np)
 {
@@ -1336,6 +1338,12 @@ static inline struct drm_bridge *of_drm_find_bridge(struct device_node *np)
 {
 	return NULL;
 }
+static inline int of_drm_get_bridge_by_endpoint(const struct device_node *np,
+						int port, int endpoint,
+						struct drm_bridge **bridge)
+{
+	return -ENODEV;
+}
 #endif
 
 static inline bool drm_bridge_is_last(struct drm_bridge *bridge)

-- 
2.53.0

[PATCH v2 03/11] drm/msm/hdmi: switch to of_drm_get_bridge_by_endpoint()

[Luca Ceresoli]

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 function.
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.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 drivers/gpu/drm/msm/hdmi/hdmi.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 852abb2466f0..57c25a09b354 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -287,9 +287,9 @@ static int msm_hdmi_dev_probe(struct platform_device *pdev)
 	spin_lock_init(&hdmi->reg_lock);
 	mutex_init(&hdmi->state_mutex);
 
-	ret = drm_of_find_panel_or_bridge(dev_of_node(dev), 1, 0, NULL, &hdmi->next_bridge);
-	if (ret && ret != -ENODEV)
-		return ret;
+	hdmi->next_bridge = of_drm_get_bridge_by_endpoint(dev_of_node(dev), 1, 0);
+	if (IS_ERR(hdmi->next_bridge) && PTR_ERR(hdmi->next_bridge) != -ENODEV)
+		return PTR_ERR(hdmi->next_bridge);
 
 	hdmi->mmio = msm_ioremap(pdev, "core_physical");
 	if (IS_ERR(hdmi->mmio))
@@ -383,6 +383,7 @@ static void msm_hdmi_dev_remove(struct platform_device *pdev)
 	component_del(&pdev->dev, &msm_hdmi_ops);
 
 	msm_hdmi_put_phy(hdmi);
+	drm_bridge_put(hdmi->next_bridge);
 }
 
 static int msm_hdmi_runtime_suspend(struct device *dev)

-- 
2.53.0

[PATCH v2 04/11] drm/hisilicon/kirin: switch to of_drm_get_bridge_by_endpoint()

[Luca Ceresoli]

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 function.
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. Here the bridge pointer is
only stored in a temporary variable, so a cleanup action is enough.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
index e80debdc4176..ab3cd309505a 100644
--- a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
+++ b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
@@ -778,17 +778,16 @@ static int dsi_host_init(struct device *dev, struct dw_dsi *dsi)
 static int dsi_bridge_init(struct drm_device *dev, struct dw_dsi *dsi)
 {
 	struct drm_encoder *encoder = &dsi->encoder;
-	struct drm_bridge *bridge;
+	struct drm_bridge *bridge __free(drm_bridge_put) = NULL;
 	struct device_node *np = dsi->dev->of_node;
-	int ret;
 
 	/*
 	 * Get the endpoint node. In our case, dsi has one output port1
 	 * to which the external HDMI bridge is connected.
 	 */
-	ret = drm_of_find_panel_or_bridge(np, 1, 0, NULL, &bridge);
-	if (ret)
-		return ret;
+	bridge = of_drm_get_bridge_by_endpoint(np, 1, 0);
+	if (IS_ERR(bridge))
+		return PTR_ERR(bridge);
 
 	/* associate the bridge to dsi encoder */
 	return drm_bridge_attach(encoder, bridge, NULL, 0);

-- 
2.53.0

[PATCH v2 05/11] drm/bridge: chrontel-ch7033: switch to of_drm_get_bridge_by_endpoint()

[Luca Ceresoli]

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 function.
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_bridge
pointer which is automatically put when the bridge is eventually freed.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 drivers/gpu/drm/bridge/chrontel-ch7033.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/bridge/chrontel-ch7033.c b/drivers/gpu/drm/bridge/chrontel-ch7033.c
index 54d49d4882c8..a237c65ebd69 100644
--- a/drivers/gpu/drm/bridge/chrontel-ch7033.c
+++ b/drivers/gpu/drm/bridge/chrontel-ch7033.c
@@ -199,7 +199,6 @@ enum {
 
 struct ch7033_priv {
 	struct regmap *regmap;
-	struct drm_bridge *next_bridge;
 	struct drm_bridge bridge;
 	struct drm_connector connector;
 };
@@ -215,7 +214,7 @@ static enum drm_connector_status ch7033_connector_detect(
 {
 	struct ch7033_priv *priv = conn_to_ch7033_priv(connector);
 
-	return drm_bridge_detect(priv->next_bridge, connector);
+	return drm_bridge_detect(priv->bridge.next_bridge, connector);
 }
 
 static const struct drm_connector_funcs ch7033_connector_funcs = {
@@ -233,7 +232,7 @@ static int ch7033_connector_get_modes(struct drm_connector *connector)
 	const struct drm_edid *drm_edid;
 	int ret;
 
-	drm_edid = drm_bridge_edid_read(priv->next_bridge, connector);
+	drm_edid = drm_bridge_edid_read(priv->bridge.next_bridge, connector);
 	drm_edid_connector_update(connector, drm_edid);
 	if (drm_edid) {
 		ret = drm_edid_connector_add_modes(connector);
@@ -275,7 +274,7 @@ static int ch7033_bridge_attach(struct drm_bridge *bridge,
 	struct drm_connector *connector = &priv->connector;
 	int ret;
 
-	ret = drm_bridge_attach(encoder, priv->next_bridge, bridge,
+	ret = drm_bridge_attach(encoder, priv->bridge.next_bridge, bridge,
 				DRM_BRIDGE_ATTACH_NO_CONNECTOR);
 	if (ret)
 		return ret;
@@ -283,15 +282,15 @@ static int ch7033_bridge_attach(struct drm_bridge *bridge,
 	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
 		return 0;
 
-	if (priv->next_bridge->ops & DRM_BRIDGE_OP_DETECT) {
+	if (priv->bridge.next_bridge->ops & DRM_BRIDGE_OP_DETECT) {
 		connector->polled = DRM_CONNECTOR_POLL_HPD;
 	} else {
 		connector->polled = DRM_CONNECTOR_POLL_CONNECT |
 				    DRM_CONNECTOR_POLL_DISCONNECT;
 	}
 
-	if (priv->next_bridge->ops & DRM_BRIDGE_OP_HPD) {
-		drm_bridge_hpd_enable(priv->next_bridge, ch7033_hpd_event,
+	if (priv->bridge.next_bridge->ops & DRM_BRIDGE_OP_HPD) {
+		drm_bridge_hpd_enable(priv->bridge.next_bridge, ch7033_hpd_event,
 				      priv);
 	}
 
@@ -299,8 +298,8 @@ static int ch7033_bridge_attach(struct drm_bridge *bridge,
 				 &ch7033_connector_helper_funcs);
 	ret = drm_connector_init_with_ddc(bridge->dev, &priv->connector,
 					  &ch7033_connector_funcs,
-					  priv->next_bridge->type,
-					  priv->next_bridge->ddc);
+					  priv->bridge.next_bridge->type,
+					  priv->bridge.next_bridge->ddc);
 	if (ret) {
 		DRM_ERROR("Failed to initialize connector\n");
 		return ret;
@@ -313,8 +312,8 @@ static void ch7033_bridge_detach(struct drm_bridge *bridge)
 {
 	struct ch7033_priv *priv = bridge_to_ch7033_priv(bridge);
 
-	if (priv->next_bridge->ops & DRM_BRIDGE_OP_HPD)
-		drm_bridge_hpd_disable(priv->next_bridge);
+	if (priv->bridge.next_bridge->ops & DRM_BRIDGE_OP_HPD)
+		drm_bridge_hpd_disable(priv->bridge.next_bridge);
 	drm_connector_cleanup(&priv->connector);
 }
 
@@ -543,10 +542,9 @@ static int ch7033_probe(struct i2c_client *client)
 
 	dev_set_drvdata(dev, priv);
 
-	ret = drm_of_find_panel_or_bridge(dev->of_node, 1, -1, NULL,
-					  &priv->next_bridge);
-	if (ret)
-		return ret;
+	priv->bridge.next_bridge = of_drm_get_bridge_by_endpoint(dev->of_node, 1, -1);
+	if (IS_ERR(priv->bridge.next_bridge))
+		return PTR_ERR(priv->bridge.next_bridge);
 
 	priv->regmap = devm_regmap_init_i2c(client, &ch7033_regmap_config);
 	if (IS_ERR(priv->regmap)) {

-- 
2.53.0

[PATCH v2 06/11] drm/bridge: lontium-lt9611uxc: switch to of_drm_get_bridge_by_endpoint()

[Luca Ceresoli]

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 function.
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_bridge
pointer which is automatically put when the bridge is eventually freed.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
index 11aab07d88df..9427cc2358ae 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
@@ -35,7 +35,6 @@
 struct lt9611uxc {
 	struct device *dev;
 	struct drm_bridge bridge;
-	struct drm_bridge *next_bridge;
 
 	struct regmap *regmap;
 	/* Protects all accesses to registers by stopping the on-chip MCU */
@@ -284,7 +283,7 @@ static int lt9611uxc_bridge_attach(struct drm_bridge *bridge,
 {
 	struct lt9611uxc *lt9611uxc = bridge_to_lt9611uxc(bridge);
 
-	return drm_bridge_attach(encoder, lt9611uxc->next_bridge,
+	return drm_bridge_attach(encoder, lt9611uxc->bridge.next_bridge,
 				 bridge, flags);
 }
 
@@ -487,7 +486,11 @@ static int lt9611uxc_parse_dt(struct device *dev,
 
 	lt9611uxc->dsi1_node = of_graph_get_remote_node(dev->of_node, 1, -1);
 
-	return drm_of_find_panel_or_bridge(dev->of_node, 2, -1, NULL, &lt9611uxc->next_bridge);
+	lt9611uxc->bridge.next_bridge = of_drm_get_bridge_by_endpoint(dev->of_node, 2, -1);
+	if (IS_ERR(lt9611uxc->bridge.next_bridge))
+		return PTR_ERR(lt9611uxc->bridge.next_bridge);
+
+	return 0;
 }
 
 static int lt9611uxc_gpio_init(struct lt9611uxc *lt9611uxc)

-- 
2.53.0

[PATCH v2 07/11] drm/bridge: lt9611: switch to of_drm_get_bridge_by_endpoint()

[Luca Ceresoli]

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 function.
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_bridge
pointer which is automatically put when the bridge is eventually freed.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 drivers/gpu/drm/bridge/lontium-lt9611.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c b/drivers/gpu/drm/bridge/lontium-lt9611.c
index 4517aee83332..4b4418cc83bf 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9611.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
@@ -37,7 +37,6 @@
 struct lt9611 {
 	struct device *dev;
 	struct drm_bridge bridge;
-	struct drm_bridge *next_bridge;
 
 	struct regmap *regmap;
 
@@ -761,7 +760,7 @@ static int lt9611_bridge_attach(struct drm_bridge *bridge,
 {
 	struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
 
-	return drm_bridge_attach(encoder, lt9611->next_bridge,
+	return drm_bridge_attach(encoder, lt9611->bridge.next_bridge,
 				 bridge, flags);
 }
 
@@ -1058,7 +1057,11 @@ static int lt9611_parse_dt(struct device *dev,
 
 	lt9611->ac_mode = of_property_read_bool(dev->of_node, "lt,ac-mode");
 
-	return drm_of_find_panel_or_bridge(dev->of_node, 2, -1, NULL, &lt9611->next_bridge);
+	lt9611->bridge.next_bridge = of_drm_get_bridge_by_endpoint(dev->of_node, 2, -1);
+	if (IS_ERR(lt9611->bridge.next_bridge))
+		return PTR_ERR(lt9611->bridge.next_bridge);
+
+	return 0;
 }
 
 static int lt9611_gpio_init(struct lt9611 *lt9611)

-- 
2.53.0

[PATCH v2 08/11] drm/bridge: adv7511: switch to of_drm_get_bridge_by_endpoint()

[Luca Ceresoli]

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 function.
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_bridge
pointer which is automatically put when the bridge is eventually freed.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 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 = bridge_to_adv7511(bridge);
 	int ret = 0;
 
-	if (adv->next_bridge) {
-		ret = drm_bridge_attach(encoder, adv->next_bridge, bridge,
+	if (adv->bridge.next_bridge) {
+		ret = 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 = drm_of_find_panel_or_bridge(dev->of_node, 1, -1, NULL,
-					  &adv7511->next_bridge);
-	if (ret && ret != -ENODEV)
-		return ret;
+	adv7511->bridge.next_bridge = of_drm_get_bridge_by_endpoint(dev->of_node, 1, -1);
+	if (IS_ERR(adv7511->bridge.next_bridge) && PTR_ERR(adv7511->bridge.next_bridge) != -ENODEV)
+		return PTR_ERR(adv7511->bridge.next_bridge);
 
 	if (adv7511->info->link_config)
 		ret = adv7511_parse_dt(dev->of_node, &link_config);

-- 
2.53.0

[PATCH v2 09/11] drm/bridge: lt8713sx: switch to of_drm_get_bridge_by_endpoint()

[Luca Ceresoli]

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 function.
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_bridge
pointer which is automatically put when the bridge is eventually freed.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 drivers/gpu/drm/bridge/lontium-lt8713sx.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt8713sx.c b/drivers/gpu/drm/bridge/lontium-lt8713sx.c
index 18fac6a46db4..cee485adf5e5 100644
--- a/drivers/gpu/drm/bridge/lontium-lt8713sx.c
+++ b/drivers/gpu/drm/bridge/lontium-lt8713sx.c
@@ -32,7 +32,6 @@ DECLARE_CRC8_TABLE(lt8713sx_crc_table);
 struct lt8713sx {
 	struct device *dev;
 	struct drm_bridge bridge;
-	struct drm_bridge *next_bridge;
 
 	struct regmap *regmap;
 	/* Protects all accesses to registers by stopping the on-chip MCU */
@@ -458,7 +457,7 @@ static int lt8713sx_bridge_attach(struct drm_bridge *bridge,
 	struct lt8713sx *lt8713sx = container_of(bridge, struct lt8713sx, bridge);
 
 	return drm_bridge_attach(encoder,
-				lt8713sx->next_bridge,
+				lt8713sx->bridge.next_bridge,
 				bridge, flags);
 }
 
@@ -537,10 +536,9 @@ static int lt8713sx_probe(struct i2c_client *client)
 	if (IS_ERR(lt8713sx->regmap))
 		return dev_err_probe(dev, PTR_ERR(lt8713sx->regmap), "regmap i2c init failed\n");
 
-	ret = drm_of_find_panel_or_bridge(lt8713sx->dev->of_node, 1, -1, NULL,
-					  &lt8713sx->next_bridge);
-	if (ret < 0)
-		return ret;
+	lt8713sx->bridge.next_bridge = of_drm_get_bridge_by_endpoint(lt8713sx->dev->of_node, 1, -1);
+	if (IS_ERR(lt8713sx->bridge.next_bridge))
+		return PTR_ERR(lt8713sx->bridge.next_bridge);
 
 	ret = lt8713sx_gpio_init(lt8713sx);
 	if (ret < 0)

-- 
2.53.0

[PATCH v2 10/11] drm: zynqmp_dp: switch to of_drm_get_bridge_by_endpoint()

[Luca Ceresoli]

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 function.
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_bridge
pointer which is automatically put when the bridge is eventually freed.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 drivers/gpu/drm/xlnx/zynqmp_dp.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
index 379180fb3004..c724250d794f 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
@@ -353,7 +353,6 @@ struct zynqmp_dp_train_set_priv {
  * @lock: Mutex protecting this struct and register access (but not AUX)
  * @irq: irq
  * @bridge: DRM bridge for the DP encoder
- * @next_bridge: The downstream bridge
  * @test: Configuration for test mode
  * @config: IP core configuration from DTS
  * @aux: aux channel
@@ -385,7 +384,6 @@ struct zynqmp_dp {
 	struct completion aux_done;
 	struct mutex lock;
 
-	struct drm_bridge *next_bridge;
 	struct device *dev;
 	struct zynqmp_dpsub *dpsub;
 	void __iomem *iomem;
@@ -1494,8 +1492,8 @@ static int zynqmp_dp_bridge_attach(struct drm_bridge *bridge,
 		return ret;
 	}
 
-	if (dp->next_bridge) {
-		ret = drm_bridge_attach(encoder, dp->next_bridge,
+	if (dp->bridge.next_bridge) {
+		ret = drm_bridge_attach(encoder, dp->bridge.next_bridge,
 					bridge, flags);
 		if (ret < 0)
 			goto error;
@@ -2461,10 +2459,11 @@ int zynqmp_dp_probe(struct zynqmp_dpsub *dpsub)
 	 * Acquire the next bridge in the chain. Ignore errors caused by port@5
 	 * not being connected for backward-compatibility with older DTs.
 	 */
-	ret = drm_of_find_panel_or_bridge(dp->dev->of_node, 5, 0, NULL,
-					  &dp->next_bridge);
-	if (ret < 0 && ret != -ENODEV)
+	dp->bridge.next_bridge = of_drm_get_bridge_by_endpoint(dp->dev->of_node, 5, 0);
+	if (IS_ERR(dp->bridge.next_bridge) && PTR_ERR(dp->bridge.next_bridge) != -ENODEV) {
+		ret = PTR_ERR(dp->bridge.next_bridge);
 		goto err_reset;
+	}
 
 	/* Initialize the hardware. */
 	dp->config.misc0 &= ~ZYNQMP_DP_MAIN_STREAM_MISC0_SYNC_LOCK;

-- 
2.53.0

[PATCH v2 11/11] drm: of: forbid bridge-only calls to drm_of_find_panel_or_bridge()

[Luca Ceresoli]

Up to now drm_of_find_panel_or_bridge() can be called with a bridge pointer
only, a panel pointer only, or both a bridge and a panel pointers. The
logic to handle all the three cases is somewhat complex to read however.

Now all bridge-only callers have been converted to
of_drm_get_bridge_by_endpoint(), which is simpler and handles bridge
refcounting. So forbid new bridge-only users by mandating a non-NULL panel
pointer in the docs and in the sanity checks along with a warning.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 drivers/gpu/drm/drm_of.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
index ef6b09316963..d03ada82eac9 100644
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -225,15 +225,15 @@ EXPORT_SYMBOL_GPL(drm_of_encoder_active_endpoint);
  * @np: device tree node containing encoder output ports
  * @port: port in the device tree node
  * @endpoint: endpoint in the device tree node
- * @panel: pointer to hold returned drm_panel
+ * @panel: pointer to hold returned drm_panel, must not be NULL
  * @bridge: pointer to hold returned drm_bridge
  *
  * Given a DT node's port and endpoint number, find the connected node and
- * return either the associated struct drm_panel or drm_bridge device. Either
- * @panel or @bridge must not be NULL.
+ * return either the associated struct drm_panel or drm_bridge device.
  *
  * This function is deprecated and should not be used in new drivers. Use
- * devm_drm_of_get_bridge() instead.
+ * of_drm_get_bridge_by_endpoint() instead when not looking for a panel, or
+ * devm_drm_of_get_bridge() otherwise.
  *
  * Returns zero if successful, or one of the standard error codes if it fails.
  */
@@ -245,10 +245,10 @@ int drm_of_find_panel_or_bridge(const struct device_node *np,
 	int ret = -EPROBE_DEFER;
 	struct device_node *remote;
 
-	if (!panel && !bridge)
+	if (WARN_ON(!panel))
 		return -EINVAL;
-	if (panel)
-		*panel = NULL;
+
+	*panel = NULL;
 
 	/*
 	 * of_graph_get_remote_node() produces a noisy error message if port
@@ -263,13 +263,11 @@ int drm_of_find_panel_or_bridge(const struct device_node *np,
 	if (!remote)
 		return -ENODEV;
 
-	if (panel) {
-		*panel = of_drm_find_panel(remote);
-		if (!IS_ERR(*panel))
-			ret = 0;
-		else
-			*panel = NULL;
-	}
+	*panel = of_drm_find_panel(remote);
+	if (!IS_ERR(*panel))
+		ret = 0;
+	else
+		*panel = NULL;
 
 	if (bridge) {
 		if (ret) {

-- 
2.53.0

Re: [PATCH v2 01/11] drm/bridge: drm_bridge_get/put(): ignore ERR_PTR

[Dmitry Baryshkov]

On Tue, Apr 28, 2026 at 11:15:47AM +0200, Luca Ceresoli wrote:
> Most functions returning a struct drm_bridge pointer currently return a
> valid pointer or NULL, but this restricts their ability to return an error
> code describing the error kind.
> 
> In preparation to have new APIs that can return a struct drm_bridge pointer
> holding an ERR_PTR (and for those which already do) make drm_bridge_get()
> and drm_bridge_put() ignore ERR_PTR values, just like they ignore NULL
> pointers.
> 
> This will avoid annoying error checking in many places and the risk of
> missing error checks.
> 
> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Link: https://lore.kernel.org/all/20260318152533.GA633439@killaraus.ideasonboard.com/
> Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Link: https://lore.kernel.org/all/omlnswxukeqgnatzdvooaashgkfcacjevkvbkm6xt33itgua2k@jcmzll2w6kdq/
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> ---
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH v2 02/11] drm/bridge: add of_drm_get_bridge_by_endpoint()

[Dmitry Baryshkov]

On Tue, Apr 28, 2026 at 11:15:48AM +0200, Luca Ceresoli wrote:
> drm_of_find_panel_or_bridge() is widely used, but many callers pass NULL
> into the @panel or the @bridge arguments, thus making a very partial usage
> of this rather complex function.
> 
> Besides, the bridge returned in @bridge is not refcounted, thus making this
> API unsafe when DRM bridge hotplug will be introduced.
> 
> Solve both issues for the cases of calls to drm_of_find_panel_or_bridge()
> with a NULL @panel pointer by adding a new function that only looks for
> bridges (and is thus much simpler) and increments the refcount of the
> returned bridge.
> 
> The new function is identical to drm_of_find_panel_or_bridge() except it:
> 
>  - handles bridge refcounting: uses of_drm_find_and_get_bridge() instead of
>    of_drm_find_bridge() internally to return a refcounted bridge
>  - is simpler to use: just takes no @panel parameter, returns the pointer
>    in the return value instead of a double pointer argument
>  - has a simpler implementation: it is equal to
>    drm_of_find_panel_or_bridge() after removing the code that becomes dead
>    when @panel == NULL
> 
> Also add this function to drm_bridge.c and not drm_of.c because it returns
> bridges only.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> ---
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH v2 03/11] drm/msm/hdmi: switch to of_drm_get_bridge_by_endpoint()

[Dmitry Baryshkov]

On Tue, Apr 28, 2026 at 11:15:49AM +0200, Luca Ceresoli wrote:
> 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 function.
> 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.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  drivers/gpu/drm/msm/hdmi/hdmi.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 

Acked-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH v2 06/11] drm/bridge: lontium-lt9611uxc: switch to of_drm_get_bridge_by_endpoint()

[Dmitry Baryshkov]

On Tue, Apr 28, 2026 at 11:15:52AM +0200, Luca Ceresoli wrote:
> 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 function.
> 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_bridge
> pointer which is automatically put when the bridge is eventually freed.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH v2 07/11] drm/bridge: lt9611: switch to of_drm_get_bridge_by_endpoint()

[Dmitry Baryshkov]

On Tue, Apr 28, 2026 at 11:15:53AM +0200, Luca Ceresoli wrote:
> 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 function.
> 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_bridge
> pointer which is automatically put when the bridge is eventually freed.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  drivers/gpu/drm/bridge/lontium-lt9611.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH v2 08/11] drm/bridge: adv7511: switch to of_drm_get_bridge_by_endpoint()

[Dmitry Baryshkov]

On Tue, Apr 28, 2026 at 11:15:54AM +0200, Luca Ceresoli wrote:
> 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 function.
> 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_bridge
> pointer which is automatically put when the bridge is eventually freed.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  drivers/gpu/drm/bridge/adv7511/adv7511.h     |  1 -
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 11 +++++------
>  2 files changed, 5 insertions(+), 7 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH v2 09/11] drm/bridge: lt8713sx: switch to of_drm_get_bridge_by_endpoint()

[Dmitry Baryshkov]

On Tue, Apr 28, 2026 at 11:15:55AM +0200, Luca Ceresoli wrote:
> 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 function.
> 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_bridge
> pointer which is automatically put when the bridge is eventually freed.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  drivers/gpu/drm/bridge/lontium-lt8713sx.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH v2 11/11] drm: of: forbid bridge-only calls to drm_of_find_panel_or_bridge()

[Dmitry Baryshkov]

On Tue, Apr 28, 2026 at 11:15:57AM +0200, Luca Ceresoli wrote:
> Up to now drm_of_find_panel_or_bridge() can be called with a bridge pointer
> only, a panel pointer only, or both a bridge and a panel pointers. The
> logic to handle all the three cases is somewhat complex to read however.
> 
> Now all bridge-only callers have been converted to
> of_drm_get_bridge_by_endpoint(), which is simpler and handles bridge
> refcounting. So forbid new bridge-only users by mandating a non-NULL panel
> pointer in the docs and in the sanity checks along with a warning.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  drivers/gpu/drm/drm_of.c | 26 ++++++++++++--------------
>  1 file changed, 12 insertions(+), 14 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH v2 05/11] drm/bridge: chrontel-ch7033: switch to of_drm_get_bridge_by_endpoint()

[Dmitry Baryshkov]

On Tue, Apr 28, 2026 at 11:15:51AM +0200, Luca Ceresoli wrote:
> 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 function.
> 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_bridge
> pointer which is automatically put when the bridge is eventually freed.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  drivers/gpu/drm/bridge/chrontel-ch7033.c | 28 +++++++++++++---------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

RE: [PATCH v2 08/11] drm/bridge: adv7511: switch to of_drm_get_bridge_by_endpoint()

[Biju Das]

Hi,

Thanks for the patch.

> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> 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 function.
> 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_bridge pointer which is automatically put when the bridge is eventually freed.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  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 = bridge_to_adv7511(bridge);
>  	int ret = 0;
> 
> -	if (adv->next_bridge) {
> -		ret = drm_bridge_attach(encoder, adv->next_bridge, bridge,
> +	if (adv->bridge.next_bridge) {
> +		ret = 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 = drm_of_find_panel_or_bridge(dev->of_node, 1, -1, NULL,
> -					  &adv7511->next_bridge);
> -	if (ret && ret != -ENODEV)
> -		return ret;
> +	adv7511->bridge.next_bridge = of_drm_get_bridge_by_endpoint(dev->of_node, 1, -1);
> +	if (IS_ERR(adv7511->bridge.next_bridge) && PTR_ERR(adv7511->bridge.next_bridge) != -ENODEV)
> +		return PTR_ERR(adv7511->bridge.next_bridge);

Does it crash, if the error is  -EPROBE_DEFER?

Cheers,
Biju

RE: [PATCH v2 08/11] drm/bridge: adv7511: switch to of_drm_get_bridge_by_endpoint()

[Biju Das]

Hi all,

> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Biju 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 <dri-devel-bounces@lists.freedesktop.org> 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 function.
> > 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_bridge pointer which is
> automatically put when the bridge is eventually freed.
> >
> > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > ---
> >  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 = bridge_to_adv7511(bridge);
> >  	int ret = 0;
> >
> > -	if (adv->next_bridge) {
> > -		ret = drm_bridge_attach(encoder, adv->next_bridge, bridge,
> > +	if (adv->bridge.next_bridge) {
> > +		ret = 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 = drm_of_find_panel_or_bridge(dev->of_node, 1, -1, NULL,
> > -					  &adv7511->next_bridge);
> > -	if (ret && ret != -ENODEV)
> > -		return ret;
> > +	adv7511->bridge.next_bridge = of_drm_get_bridge_by_endpoint(dev->of_node, 1, -1);
> > +	if (IS_ERR(adv7511->bridge.next_bridge) && PTR_ERR(adv7511->bridge.next_bridge) != -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 assignment.

[1] https://lore.kernel.org/linux-renesas-soc/20260421172910.218497-4-biju.das.jz@bp.renesas.com/

+	next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
+	if (IS_ERR(next_bridge))
+		return dev_err_probe(dev, PTR_ERR(next_bridge),
+				     "failed to get next bridge\n");
+
+	lvds->bridge.next_bridge = next_bridge;

Cheers,
Biju

Re: [PATCH v2 07/11] drm/bridge: lt9611: switch to of_drm_get_bridge_by_endpoint()

[Gyeyoung Baek]

>
> @@ -1058,7 +1057,11 @@ static int lt9611_parse_dt(struct device *dev,
>
>         lt9611->ac_mode = of_property_read_bool(dev->of_node, "lt,ac-mode");
>
> -       return drm_of_find_panel_or_bridge(dev->of_node, 2, -1, NULL, &lt9611->next_bridge);
> +       lt9611->bridge.next_bridge = of_drm_get_bridge_by_endpoint(dev->of_node, 2, -1);
> +       if (IS_ERR(lt9611->bridge.next_bridge))
> +               return PTR_ERR(lt9611->bridge.next_bridge);
> +
> +       return 0;
>  }

Hello Luca,
Could you take a look at this alternative?
------
struct drm_bridge *next_bridge =
    of_drm_get_bridge_by_endpoint(dev->of_node, 2, -1);

if (IS_ERR(next_bridge))
    return PTR_ERR(next_bridge);

lt9611->bridge.next_bridge = next_bridge;
return 0;
------

 I think it reads a bit cleaner and avoids holding the ERR_PTR in the struct.

-- 
Thanks,
Gyeyoung

Re: [PATCH v2 08/11] drm/bridge: adv7511: switch to of_drm_get_bridge_by_endpoint()

[Luca Ceresoli]

On Tue Apr 28, 2026 at 1:58 PM CEST, Biju Das wrote:
> Hi all,
>
>> -----Original Message-----
>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Biju 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 <dri-devel-bounces@lists.freedesktop.org> 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 function.
>> > 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_bridge pointer which is
>> automatically put when the bridge is eventually freed.
>> >
>> > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
>> > ---
>> >  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 = bridge_to_adv7511(bridge);
>> >  	int ret = 0;
>> >
>> > -	if (adv->next_bridge) {
>> > -		ret = drm_bridge_attach(encoder, adv->next_bridge, bridge,
>> > +	if (adv->bridge.next_bridge) {
>> > +		ret = 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 = drm_of_find_panel_or_bridge(dev->of_node, 1, -1, NULL,
>> > -					  &adv7511->next_bridge);
>> > -	if (ret && ret != -ENODEV)
>> > -		return ret;
>> > +	adv7511->bridge.next_bridge = of_drm_get_bridge_by_endpoint(dev->of_node, 1, -1);
>> > +	if (IS_ERR(adv7511->bridge.next_bridge) && PTR_ERR(adv7511->bridge.next_bridge) != -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 assignment.

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->bridge.next_bridge) != -ENODEV)
      -               return PTR_ERR(adv7511->bridge.next_bridge);
      +       if (IS_ERR(adv7511->bridge.next_bridge)) {
      +               if (PTR_ERR(adv7511->bridge.next_bridge) == -ENODEV)
      +                       adv7511->bridge.next_bridge = 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

RE: [PATCH v2 08/11] drm/bridge: adv7511: switch to of_drm_get_bridge_by_endpoint()

[Biju Das]

Hi Luca,

> -----Original Message-----
> From: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Sent: 28 April 2026 14:20
> Subject: Re: [PATCH v2 08/11] drm/bridge: adv7511: switch to of_drm_get_bridge_by_endpoint()
> 
> On Tue Apr 28, 2026 at 1:58 PM CEST, Biju Das wrote:
> > Hi all,
> >
> >> -----Original Message-----
> >> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf
> >> Of Biju 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 <dri-devel-bounces@lists.freedesktop.org> 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 function.
> >> > 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_bridge pointer which is
> >> automatically put when the bridge is eventually freed.
> >> >
> >> > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> >> > ---
> >> >  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 = bridge_to_adv7511(bridge);
> >> >  	int ret = 0;
> >> >
> >> > -	if (adv->next_bridge) {
> >> > -		ret = drm_bridge_attach(encoder, adv->next_bridge, bridge,
> >> > +	if (adv->bridge.next_bridge) {
> >> > +		ret = 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 = drm_of_find_panel_or_bridge(dev->of_node, 1, -1, NULL,
> >> > -					  &adv7511->next_bridge);
> >> > -	if (ret && ret != -ENODEV)
> >> > -		return ret;
> >> > +	adv7511->bridge.next_bridge = of_drm_get_bridge_by_endpoint(dev->of_node, 1, -1);
> >> > +	if (IS_ERR(adv7511->bridge.next_bridge) && PTR_ERR(adv7511->bridge.next_bridge) != -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 assignment.
> 
> 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->bridge.next_bridge) != -
> ENODEV)
>       -               return PTR_ERR(adv7511->bridge.next_bridge);
>       +       if (IS_ERR(adv7511->bridge.next_bridge)) {
>       +               if (PTR_ERR(adv7511->bridge.next_bridge) == -ENODEV)
>       +                       adv7511->bridge.next_bridge = NULL;
>       +               else
>       +                       return PTR_ERR(adv7511->bridge.next_bridge);

The point is you cannot return PTR_ERR as it will lead to crash, if it is
direct assignment.

   if (IS_ERR(adv7511->bridge.next_bridge)) {
	int err = PTR_ERR(adv7511->bridge.next_bridge);
	adv7511->bridge.next_bridge = NULL;
	return err;
  }

Cheers,
Biju

>  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

Re: [PATCH v2 08/11] drm/bridge: adv7511: switch to of_drm_get_bridge_by_endpoint()

[Luca Ceresoli]

Hello,

On Tue Apr 28, 2026 at 3:31 PM CEST, Biju Das wrote:
>> >> > @@ -1251,10 +1251,9 @@ static int adv7511_probe(struct i2c_client
>> >> > *i2c)
>> >> >
>> >> >  	memset(&link_config, 0, sizeof(link_config));
>> >> >
>> >> > -	ret = drm_of_find_panel_or_bridge(dev->of_node, 1, -1, NULL,
>> >> > -					  &adv7511->next_bridge);
>> >> > -	if (ret && ret != -ENODEV)
>> >> > -		return ret;
>> >> > +	adv7511->bridge.next_bridge = of_drm_get_bridge_by_endpoint(dev->of_node, 1, -1);
>> >> > +	if (IS_ERR(adv7511->bridge.next_bridge) && PTR_ERR(adv7511->bridge.next_bridge) != -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 assignment.
>>
>> 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->bridge.next_bridge) != -
>> ENODEV)
>>       -               return PTR_ERR(adv7511->bridge.next_bridge);
>>       +       if (IS_ERR(adv7511->bridge.next_bridge)) {
>>       +               if (PTR_ERR(adv7511->bridge.next_bridge) == -ENODEV)
>>       +                       adv7511->bridge.next_bridge = NULL;
>>       +               else
>>       +                       return PTR_ERR(adv7511->bridge.next_bridge);
>
> The point is you cannot return PTR_ERR as it will lead to crash, if it is
> direct assignment.

It would definitely crash when the next_bridge is dereferenced (which
happens in adv7511_bridge_attach()) but I don't see how it can crash
here. Here it _can_ be assigned -ENODEV, but it would be immediately be
cleared to NULL, or to enother error, but we'd immediately return. And in
case of return, when next_bridge is cleared by __drm_bridge_free ->
drm_bridge_put, the error value would be ignored thanks to patch 1.

>
>    if (IS_ERR(adv7511->bridge.next_bridge)) {
> 	int err = PTR_ERR(adv7511->bridge.next_bridge);
> 	adv7511->bridge.next_bridge = NULL;
> 	return err;
>   }

Is this if() condition wrong? The driver needs to accept the -ENODEV return
value, the next_bridge is conditional in the curent driver code.

>
> Cheers,
> Biju
>
>>  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

Re: [PATCH v2 07/11] drm/bridge: lt9611: switch to of_drm_get_bridge_by_endpoint()

[Luca Ceresoli]

On Tue Apr 28, 2026 at 3:18 PM CEST, Gyeyoung Baek wrote:
>>
>> @@ -1058,7 +1057,11 @@ static int lt9611_parse_dt(struct device *dev,
>>
>>         lt9611->ac_mode = of_property_read_bool(dev->of_node, "lt,ac-mode");
>>
>> -       return drm_of_find_panel_or_bridge(dev->of_node, 2, -1, NULL, &lt9611->next_bridge);
>> +       lt9611->bridge.next_bridge = of_drm_get_bridge_by_endpoint(dev->of_node, 2, -1);
>> +       if (IS_ERR(lt9611->bridge.next_bridge))
>> +               return PTR_ERR(lt9611->bridge.next_bridge);
>> +
>> +       return 0;
>>  }
>
> Hello Luca,
> Could you take a look at this alternative?
> ------
> struct drm_bridge *next_bridge =
>     of_drm_get_bridge_by_endpoint(dev->of_node, 2, -1);
>
> if (IS_ERR(next_bridge))
>     return PTR_ERR(next_bridge);
>
> lt9611->bridge.next_bridge = next_bridge;
> return 0;
> ------
>
>  I think it reads a bit cleaner and avoids holding the ERR_PTR in the struct.

Yes, but more verbose.

Based on this and the patch 8 fallback reported by Biju, I started
wondering whether the v1 version of this function, while being a bit ugly,
was leading to cleaner code... An example: [0].

Dmitry, your opinion?

[0] https://lore.kernel.org/all/20260413-drm-bridge-alloc-getput-panel_or_bridge-v1-6-acd01cd79a1f@bootlin.com/

Luca

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

RE: [PATCH v2 08/11] drm/bridge: adv7511: switch to of_drm_get_bridge_by_endpoint()

[Biju Das]

Hi Luca,

> -----Original Message-----
> From: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Sent: 28 April 2026 14:48
> Subject: Re: [PATCH v2 08/11] drm/bridge: adv7511: switch to of_drm_get_bridge_by_endpoint()
> 
> Hello,
> 
> On Tue Apr 28, 2026 at 3:31 PM CEST, Biju Das wrote:
> >> >> > @@ -1251,10 +1251,9 @@ static int adv7511_probe(struct
> >> >> > i2c_client
> >> >> > *i2c)
> >> >> >
> >> >> >  	memset(&link_config, 0, sizeof(link_config));
> >> >> >
> >> >> > -	ret = drm_of_find_panel_or_bridge(dev->of_node, 1, -1, NULL,
> >> >> > -					  &adv7511->next_bridge);
> >> >> > -	if (ret && ret != -ENODEV)
> >> >> > -		return ret;
> >> >> > +	adv7511->bridge.next_bridge = of_drm_get_bridge_by_endpoint(dev->of_node, 1, -1);
> >> >> > +	if (IS_ERR(adv7511->bridge.next_bridge) && PTR_ERR(adv7511->bridge.next_bridge) != -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 assignment.
> >>
> >> 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->bridge.next_bridge) != -
> >> ENODEV)
> >>       -               return PTR_ERR(adv7511->bridge.next_bridge);
> >>       +       if (IS_ERR(adv7511->bridge.next_bridge)) {
> >>       +               if (PTR_ERR(adv7511->bridge.next_bridge) == -ENODEV)
> >>       +                       adv7511->bridge.next_bridge = NULL;
> >>       +               else
> >>       +                       return PTR_ERR(adv7511->bridge.next_bridge);
> >
> > The point is you cannot return PTR_ERR as it will lead to crash, if it
> > is direct assignment.
> 
> It would definitely crash when the next_bridge is dereferenced (which happens in
> adv7511_bridge_attach()) but I don't see how it can crash here. Here it _can_ be assigned -ENODEV, but
> it would be immediately be cleared to NULL, or to enother error, but we'd immediately return. And in
> case of return, when next_bridge is cleared by __drm_bridge_free -> drm_bridge_put, the error value
> would be ignored thanks to patch 1.

OK, I haven't noticed the newly introduced check in drm_bridge_put() in patch#1.

I guess, we should avoid putting error values in bridge.next_bridge.
It should be either NULL or Valid pointer, not PTR_ERR.

Cheers,
Biju

RE: [PATCH v2 08/11] drm/bridge: adv7511: switch to of_drm_get_bridge_by_endpoint()

[Biju Das]

> -----Original Message-----
> From: Biju Das
> Sent: 28 April 2026 15:02
> Subject: RE: [PATCH v2 08/11] drm/bridge: adv7511: switch to of_drm_get_bridge_by_endpoint()
> 
> Hi Luca,
> 
> > -----Original Message-----
> > From: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > Sent: 28 April 2026 14:48
> > Subject: Re: [PATCH v2 08/11] drm/bridge: adv7511: switch to
> > of_drm_get_bridge_by_endpoint()
> >
> > Hello,
> >
> > On Tue Apr 28, 2026 at 3:31 PM CEST, Biju Das wrote:
> > >> >> > @@ -1251,10 +1251,9 @@ static int adv7511_probe(struct
> > >> >> > i2c_client
> > >> >> > *i2c)
> > >> >> >
> > >> >> >  	memset(&link_config, 0, sizeof(link_config));
> > >> >> >
> > >> >> > -	ret = drm_of_find_panel_or_bridge(dev->of_node, 1, -1, NULL,
> > >> >> > -					  &adv7511->next_bridge);
> > >> >> > -	if (ret && ret != -ENODEV)
> > >> >> > -		return ret;
> > >> >> > +	adv7511->bridge.next_bridge = of_drm_get_bridge_by_endpoint(dev->of_node, 1, -1);
> > >> >> > +	if (IS_ERR(adv7511->bridge.next_bridge) && PTR_ERR(adv7511->bridge.next_bridge) != -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 assignment.
> > >>
> > >> 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->bridge.next_bridge) != -
> > >> ENODEV)
> > >>       -               return PTR_ERR(adv7511->bridge.next_bridge);
> > >>       +       if (IS_ERR(adv7511->bridge.next_bridge)) {
> > >>       +               if (PTR_ERR(adv7511->bridge.next_bridge) == -ENODEV)
> > >>       +                       adv7511->bridge.next_bridge = NULL;
> > >>       +               else
> > >>       +                       return PTR_ERR(adv7511->bridge.next_bridge);
> > >
> > > The point is you cannot return PTR_ERR as it will lead to crash, if
> > > it is direct assignment.
> >
> > It would definitely crash when the next_bridge is dereferenced (which
> > happens in
> > adv7511_bridge_attach()) but I don't see how it can crash here. Here
> > it _can_ be assigned -ENODEV, but it would be immediately be cleared
> > to NULL, or to enother error, but we'd immediately return. And in case
> > of return, when next_bridge is cleared by __drm_bridge_free -> drm_bridge_put, the error value would
> be ignored thanks to patch 1.
> 
> OK, I haven't noticed the newly introduced check in drm_bridge_put() in patch#1.
> 
> I guess, we should avoid putting error values in bridge.next_bridge.
> It should be either NULL or Valid pointer, not PTR_ERR.

FTR, I get a crash in attach. I will apply the suggested changes and will let you know the result.

[   18.957324] pc : drm_bridge_attach+0x34/0x210 [drm]
[   18.969425] lr : adv7511_bridge_attach+0x38/0xb8 [adv7511]

[   18.969610]  drm_bridge_attach+0x34/0x210 [drm] (P)
[   18.969845]  adv7511_bridge_attach+0x38/0xb8 [adv7511]
[   18.969867]  drm_bridge_attach+0xf0/0x210 [drm]
[   18.970042]  rzg2l_mipi_dsi_attach+0x24/0x3c [rzg2l_mipi_dsi]
[   18.970064]  drm_bridge_attach+0xf0/0x210 [drm]
[   18.970262]  rzg2l_du_encoder_init+0x9c/0x250 [rzg2l_du_drm]
[   18.970293]  rzg2l_du_modeset_init+0x30c/0x4d0 [rzg2l_du_drm]
[   18.970307]  rzg2l_du_probe+0xc8/0x174 [rzg2l_du_drm]
[   18.970321]  platform_probe+0x5c/0xa4
[   18.970336]  really_probe+0xbc/0x2c0
[   18.970348]  __driver_probe_device+0x80/0x14c
[   18.970359]  driver_probe_device+0x3c/0x164
[   18.970369]  __driver_attach+0x90/0x1a4
[   18.970379]  bus_for_each_dev+0x7c/0xdc
[   18.970388]  driver_attach+0x24/0x30
[   18.970397]  bus_add_driver+0xe4/0x208
[   18.970406]  driver_register+0x68/0x130
[   18.970416]  __platform_driver_register+0x24/0x30


Cheers,
Biju

RE: [PATCH v2 08/11] drm/bridge: adv7511: switch to of_drm_get_bridge_by_endpoint()

[Biju Das]

Hi Luca,

> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Biju Das
> Sent: 28 April 2026 15:39
> Subject: RE: [PATCH v2 08/11] drm/bridge: adv7511: switch to of_drm_get_bridge_by_endpoint()
> 
> 
> 
> > -----Original Message-----
> > From: Biju Das
> > Sent: 28 April 2026 15:02
> > Subject: RE: [PATCH v2 08/11] drm/bridge: adv7511: switch to
> > of_drm_get_bridge_by_endpoint()
> >
> > Hi Luca,
> >
> > > -----Original Message-----
> > > From: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > > Sent: 28 April 2026 14:48
> > > Subject: Re: [PATCH v2 08/11] drm/bridge: adv7511: switch to
> > > of_drm_get_bridge_by_endpoint()
> > >
> > > Hello,
> > >
> > > On Tue Apr 28, 2026 at 3:31 PM CEST, Biju Das wrote:
> > > >> >> > @@ -1251,10 +1251,9 @@ static int adv7511_probe(struct
> > > >> >> > i2c_client
> > > >> >> > *i2c)
> > > >> >> >
> > > >> >> >  	memset(&link_config, 0, sizeof(link_config));
> > > >> >> >
> > > >> >> > -	ret = drm_of_find_panel_or_bridge(dev->of_node, 1, -1, NULL,
> > > >> >> > -					  &adv7511->next_bridge);
> > > >> >> > -	if (ret && ret != -ENODEV)
> > > >> >> > -		return ret;
> > > >> >> > +	adv7511->bridge.next_bridge = of_drm_get_bridge_by_endpoint(dev->of_node, 1, -1);
> > > >> >> > +	if (IS_ERR(adv7511->bridge.next_bridge) && PTR_ERR(adv7511->bridge.next_bridge) != -
> 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 assignment.
> > > >>
> > > >> 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->bridge.next_bridge) !=
> -
> > > >> ENODEV)
> > > >>       -               return PTR_ERR(adv7511->bridge.next_bridge);
> > > >>       +       if (IS_ERR(adv7511->bridge.next_bridge)) {
> > > >>       +               if (PTR_ERR(adv7511->bridge.next_bridge) == -ENODEV)
> > > >>       +                       adv7511->bridge.next_bridge = NULL;
> > > >>       +               else
> > > >>       +                       return PTR_ERR(adv7511->bridge.next_bridge);
> > > >
> > > > The point is you cannot return PTR_ERR as it will lead to crash,
> > > > if it is direct assignment.
> > >
> > > It would definitely crash when the next_bridge is dereferenced
> > > (which happens in
> > > adv7511_bridge_attach()) but I don't see how it can crash here. Here
> > > it _can_ be assigned -ENODEV, but it would be immediately be cleared
> > > to NULL, or to enother error, but we'd immediately return. And in
> > > case of return, when next_bridge is cleared by __drm_bridge_free ->
> > > drm_bridge_put, the error value would
> > be ignored thanks to patch 1.
> >
> > OK, I haven't noticed the newly introduced check in drm_bridge_put() in patch#1.
> >
> > I guess, we should avoid putting error values in bridge.next_bridge.
> > It should be either NULL or Valid pointer, not PTR_ERR.
> 
> FTR, I get a crash in attach. I will apply the suggested changes and will let you know the result.
> 
> [   18.957324] pc : drm_bridge_attach+0x34/0x210 [drm]
> [   18.969425] lr : adv7511_bridge_attach+0x38/0xb8 [adv7511]
> 
> [   18.969610]  drm_bridge_attach+0x34/0x210 [drm] (P)
> [   18.969845]  adv7511_bridge_attach+0x38/0xb8 [adv7511]
> [   18.969867]  drm_bridge_attach+0xf0/0x210 [drm]
> [   18.970042]  rzg2l_mipi_dsi_attach+0x24/0x3c [rzg2l_mipi_dsi]
> [   18.970064]  drm_bridge_attach+0xf0/0x210 [drm]
> [   18.970262]  rzg2l_du_encoder_init+0x9c/0x250 [rzg2l_du_drm]
> [   18.970293]  rzg2l_du_modeset_init+0x30c/0x4d0 [rzg2l_du_drm]
> [   18.970307]  rzg2l_du_probe+0xc8/0x174 [rzg2l_du_drm]
> [   18.970321]  platform_probe+0x5c/0xa4
> [   18.970336]  really_probe+0xbc/0x2c0
> [   18.970348]  __driver_probe_device+0x80/0x14c
> [   18.970359]  driver_probe_device+0x3c/0x164
> [   18.970369]  __driver_attach+0x90/0x1a4
> [   18.970379]  bus_for_each_dev+0x7c/0xdc
> [   18.970388]  driver_attach+0x24/0x30
> [   18.970397]  bus_add_driver+0xe4/0x208
> [   18.970406]  driver_register+0x68/0x130
> [   18.970416]  __platform_driver_register+0x24/0x30
> 

I confirm the crash is fixed by your suggested changes for V3.

Cheers,
Biju

Re: [PATCH v2 08/11] drm/bridge: adv7511: switch to of_drm_get_bridge_by_endpoint()

[Luca Ceresoli]

Hello Biju,

On Tue Apr 28, 2026 at 4:45 PM CEST, Biju Das wrote:
> Hi Luca,
>
>> -----Original Message-----
>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Biju Das
>> Sent: 28 April 2026 15:39
>> Subject: RE: [PATCH v2 08/11] drm/bridge: adv7511: switch to of_drm_get_bridge_by_endpoint()
>>
>>
>>
>> > -----Original Message-----
>> > From: Biju Das
>> > Sent: 28 April 2026 15:02
>> > Subject: RE: [PATCH v2 08/11] drm/bridge: adv7511: switch to
>> > of_drm_get_bridge_by_endpoint()
>> >
>> > Hi Luca,
>> >
>> > > -----Original Message-----
>> > > From: Luca Ceresoli <luca.ceresoli@bootlin.com>
>> > > Sent: 28 April 2026 14:48
>> > > Subject: Re: [PATCH v2 08/11] drm/bridge: adv7511: switch to
>> > > of_drm_get_bridge_by_endpoint()
>> > >
>> > > Hello,
>> > >
>> > > On Tue Apr 28, 2026 at 3:31 PM CEST, Biju Das wrote:
>> > > >> >> > @@ -1251,10 +1251,9 @@ static int adv7511_probe(struct
>> > > >> >> > i2c_client
>> > > >> >> > *i2c)
>> > > >> >> >
>> > > >> >> >  	memset(&link_config, 0, sizeof(link_config));
>> > > >> >> >
>> > > >> >> > -	ret = drm_of_find_panel_or_bridge(dev->of_node, 1, -1, NULL,
>> > > >> >> > -					  &adv7511->next_bridge);
>> > > >> >> > -	if (ret && ret != -ENODEV)
>> > > >> >> > -		return ret;
>> > > >> >> > +	adv7511->bridge.next_bridge = of_drm_get_bridge_by_endpoint(dev->of_node, 1, -1);
>> > > >> >> > +	if (IS_ERR(adv7511->bridge.next_bridge) && PTR_ERR(adv7511->bridge.next_bridge) != -
>> 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 assignment.
>> > > >>
>> > > >> 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->bridge.next_bridge) !=
>> -
>> > > >> ENODEV)
>> > > >>       -               return PTR_ERR(adv7511->bridge.next_bridge);
>> > > >>       +       if (IS_ERR(adv7511->bridge.next_bridge)) {
>> > > >>       +               if (PTR_ERR(adv7511->bridge.next_bridge) == -ENODEV)
>> > > >>       +                       adv7511->bridge.next_bridge = NULL;
>> > > >>       +               else
>> > > >>       +                       return PTR_ERR(adv7511->bridge.next_bridge);
>> > > >
>> > > > The point is you cannot return PTR_ERR as it will lead to crash,
>> > > > if it is direct assignment.
>> > >
>> > > It would definitely crash when the next_bridge is dereferenced
>> > > (which happens in
>> > > adv7511_bridge_attach()) but I don't see how it can crash here. Here
>> > > it _can_ be assigned -ENODEV, but it would be immediately be cleared
>> > > to NULL, or to enother error, but we'd immediately return. And in
>> > > case of return, when next_bridge is cleared by __drm_bridge_free ->
>> > > drm_bridge_put, the error value would
>> > be ignored thanks to patch 1.
>> >
>> > OK, I haven't noticed the newly introduced check in drm_bridge_put() in patch#1.
>> >
>> > I guess, we should avoid putting error values in bridge.next_bridge.
>> > It should be either NULL or Valid pointer, not PTR_ERR.
>>
>> FTR, I get a crash in attach. I will apply the suggested changes and will let you know the result.
>>
>> [   18.957324] pc : drm_bridge_attach+0x34/0x210 [drm]
>> [   18.969425] lr : adv7511_bridge_attach+0x38/0xb8 [adv7511]
>>
>> [   18.969610]  drm_bridge_attach+0x34/0x210 [drm] (P)
>> [   18.969845]  adv7511_bridge_attach+0x38/0xb8 [adv7511]
>> [   18.969867]  drm_bridge_attach+0xf0/0x210 [drm]
>> [   18.970042]  rzg2l_mipi_dsi_attach+0x24/0x3c [rzg2l_mipi_dsi]
>> [   18.970064]  drm_bridge_attach+0xf0/0x210 [drm]
>> [   18.970262]  rzg2l_du_encoder_init+0x9c/0x250 [rzg2l_du_drm]
>> [   18.970293]  rzg2l_du_modeset_init+0x30c/0x4d0 [rzg2l_du_drm]
>> [   18.970307]  rzg2l_du_probe+0xc8/0x174 [rzg2l_du_drm]
>> [   18.970321]  platform_probe+0x5c/0xa4
>> [   18.970336]  really_probe+0xbc/0x2c0
>> [   18.970348]  __driver_probe_device+0x80/0x14c
>> [   18.970359]  driver_probe_device+0x3c/0x164
>> [   18.970369]  __driver_attach+0x90/0x1a4
>> [   18.970379]  bus_for_each_dev+0x7c/0xdc
>> [   18.970388]  driver_attach+0x24/0x30
>> [   18.970397]  bus_add_driver+0xe4/0x208
>> [   18.970406]  driver_register+0x68/0x130
>> [   18.970416]  __platform_driver_register+0x24/0x30
>>
>
> I confirm the crash is fixed by your suggested changes for V3.

Very quick feedback loop! Thanks a lot Biju.

Sending v3 in a moment.

Luca

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