Several drivers (about 20) follow the same pattern:

 1. get a pointer to a bridge (typically the next bridge in the chain) by
    calling of_drm_find_bridge()
 2. store the returned pointer in the private driver data, keep it until
    driver .remove
 3. dereference the pointer at attach time and possibly at other times

of_drm_find_bridge() is now deprecated because it does not increment the
refcount and should be replaced with of_drm_get_bridge() +
drm_bridge_put().

However some of those drivers have a complex code flow and adding a
drm_bridge_put() call in all the appropriate locations is error-prone,
leads to ugly and more complex code, and can lead to errors over time with
code flow changes.

To handle all those drivers in a straightforward way, add a devm variant of
of_drm_get_bridge() that adds a devm action to invoke drm_bridge_put()
when the said driver is removed. This allows all those drivers to put the
reference automatically and safely with a one line change:

  - priv->next_bridge = of_drm_find_bridge(remote_np);
  + priv->next_bridge = devm_of_drm_get_bridge(dev, remote_np);

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

---

Changes in v2:
- fix return value: NULL on error, as documented, not an ERR_PTR
---
 drivers/gpu/drm/drm_bridge.c | 28 ++++++++++++++++++++++++++++
 include/drm/drm_bridge.h     |  5 +++++
 2 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 9b7e3f859973..59575a84eff6 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -1442,6 +1442,34 @@ struct drm_bridge *of_drm_get_bridge(struct device_node *np)
 }
 EXPORT_SYMBOL(of_drm_get_bridge);
 
+/**
+ * devm_of_drm_get_bridge - find the bridge corresponding to the device
+ *			    node in the global bridge list and add a devm
+ *			    action to put it
+ *
+ * @dev: device requesting the bridge
+ * @np: device node
+ *
+ * On success the returned bridge refcount is incremented, and a devm
+ * action is added to call drm_bridge_put() when @dev is removed. So the
+ * caller does not have to put the returned bridge explicitly.
+ *
+ * RETURNS:
+ * drm_bridge control struct on success, NULL on failure
+ */
+struct drm_bridge *devm_of_drm_get_bridge(struct device *dev, struct device_node *np)
+{
+	struct drm_bridge *bridge = of_drm_get_bridge(np);
+
+	if (bridge) {
+		if (devm_add_action_or_reset(dev, drm_bridge_put_void, bridge))
+			return NULL;
+	}
+
+	return bridge;
+}
+EXPORT_SYMBOL(devm_of_drm_get_bridge);
+
 /**
  * of_drm_find_bridge - find the bridge corresponding to the device node in
  *			the global bridge list
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 0977eab78aac..da69cb252cad 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -1314,12 +1314,17 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
 
 #ifdef CONFIG_OF
 struct drm_bridge *of_drm_get_bridge(struct device_node *np);
+struct drm_bridge *devm_of_drm_get_bridge(struct device *dev, struct device_node *np);
 struct drm_bridge *of_drm_find_bridge(struct device_node *np);
 #else
 static inline struct drm_bridge *of_drm_get_bridge(struct device_node *np)
 {
 	return NULL;
 }
+static inline struct drm_bridge *devm_of_drm_get_bridge(struct device *dev, struct device_node *np)
+{
+	return NULL;
+}
 static inline struct drm_bridge *of_drm_find_bridge(struct device_node *np)
 {
 	return NULL;

-- 
2.51.1

On Fri, Nov 28, 2025 at 05:50:16PM +0100, Luca Ceresoli wrote:
> Several drivers (about 20) follow the same pattern:
> 
>  1. get a pointer to a bridge (typically the next bridge in the chain) by
>     calling of_drm_find_bridge()
>  2. store the returned pointer in the private driver data, keep it until
>     driver .remove
>  3. dereference the pointer at attach time and possibly at other times
> 
> of_drm_find_bridge() is now deprecated because it does not increment the
> refcount and should be replaced with of_drm_get_bridge() +
> drm_bridge_put().
> 
> However some of those drivers have a complex code flow and adding a
> drm_bridge_put() call in all the appropriate locations is error-prone,
> leads to ugly and more complex code, and can lead to errors over time with
> code flow changes.
> 
> To handle all those drivers in a straightforward way, add a devm variant of
> of_drm_get_bridge() that adds a devm action to invoke drm_bridge_put()
> when the said driver is removed. This allows all those drivers to put the
> reference automatically and safely with a one line change:
> 
>   - priv->next_bridge = of_drm_find_bridge(remote_np);
>   + priv->next_bridge = devm_of_drm_get_bridge(dev, remote_np);
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> ---
> 
> Changes in v2:
> - fix return value: NULL on error, as documented, not an ERR_PTR
> ---
>  drivers/gpu/drm/drm_bridge.c | 28 ++++++++++++++++++++++++++++
>  include/drm/drm_bridge.h     |  5 +++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 9b7e3f859973..59575a84eff6 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -1442,6 +1442,34 @@ struct drm_bridge *of_drm_get_bridge(struct device_node *np)
>  }
>  EXPORT_SYMBOL(of_drm_get_bridge);
>  
> +/**
> + * devm_of_drm_get_bridge - find the bridge corresponding to the device
> + *			    node in the global bridge list and add a devm
> + *			    action to put it
> + *
> + * @dev: device requesting the bridge
> + * @np: device node
> + *
> + * On success the returned bridge refcount is incremented, and a devm
> + * action is added to call drm_bridge_put() when @dev is removed. So the
> + * caller does not have to put the returned bridge explicitly.
> + *
> + * RETURNS:
> + * drm_bridge control struct on success, NULL on failure
> + */

I still think that, if we want to introduce it, we need to be very clear
that it's not safe, and we need to add a TODO to remove it later on. But
why should we introduce a helper, and convert dozens of drivers to it,
for something that is neutral?

If anything, I'd rather see them call of_drm_get_bridge(under the new
name), and put back the reference in destroy.

Maxime