Re: [PATCH 06/26] drm/bridge: add devm_drm_of_find_bridge

[Louis Chauvet <louis.chauvet@bootlin.com>]

On 11/19/25 13:05, 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 drm_of_find_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
> drm_of_find_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_drm_of_find_bridge(dev, remote_np);
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>   drivers/gpu/drm/drm_bridge.c | 30 ++++++++++++++++++++++++++++++
>   include/drm/drm_bridge.h     |  5 +++++
>   2 files changed, 35 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 09ad825f9cb8..c7baafbe5695 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -1446,6 +1446,36 @@ struct drm_bridge *drm_of_find_bridge(struct device_node *np)
>   }
>   EXPORT_SYMBOL(drm_of_find_bridge);
>   
> +/**
> + * devm_drm_of_find_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 am not sure for the "NULL on failure", you return ERR_PTR(err), which 
is probably not NULL but an error code.

> + */
> +struct drm_bridge *devm_drm_of_find_bridge(struct device *dev, struct device_node *np)
> +{
> +	struct drm_bridge *bridge = drm_of_find_bridge(np);
> +
> +	if (bridge) {
> +		int err = devm_add_action_or_reset(dev, drm_bridge_put_void, bridge);
> +
> +		if (err)
> +			return ERR_PTR(err);
> +	}
> +
> +	return bridge;
> +}
> +EXPORT_SYMBOL(devm_drm_of_find_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 e74e91004c48..98d5433f7d35 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 *drm_of_find_bridge(struct device_node *np);
> +struct drm_bridge *devm_drm_of_find_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 *drm_of_find_bridge(struct device_node *np)
>   {
>   	return NULL;
>   }
> +static inline struct drm_bridge *devm_drm_of_find_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;
>