Re: [PATCH v4 1/4] drm: renesas: rz-du: rzg2l_du_encoder: convert to of_drm_find_and_get_bridge()

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

On 4/2/26 18:27, Luca Ceresoli wrote:
> of_drm_find_bridge() is deprecated. Move to its replacement
> of_drm_find_and_get_bridge() which gets a bridge reference, and ensure it
> is put when done.
> 
> This is made somewhat simpler by the fact that 'bridge' is a local
> variable.
> 
> However we need to handle both branches of the main if().
> 
> In the 'else' case, just switch to of_drm_find_and_get_bridge() to ensure
> the bridge is not freed while in use in the function tail
> (drm_bridge_attach() mainly).
> 
> In the 'then' case, devm_drm_panel_bridge_add_typed() already increments
> the refcount using devres which ties the bridge allocation lifetime to the
> device lifetime, so we would not need to do anything. However to have the
> same behaviour in both branches take an additional reference here, so that
> the bridge needs to be put whichever branch is taken without more
> complicated logic. Ensure to clear the bridge pointer however, to avoid
> calling drm_bridge_put() on an ERR_PTR.
> 
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> Tested-by: Biju Das <biju.das.jz@bp.renesas.com>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> ---
> 
> Changes in v4:
> - Added comment to clarify the additional drm_bridge_get() in the
>    panel_bridge case (keeping the R-by and T-by tags as it's just a comment)
> ---
>   drivers/gpu/drm/renesas/rz-du/rzg2l_du_encoder.c | 19 +++++++++++++++----
>   1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_encoder.c b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_encoder.c
> index 5e6dd16705e6..9edb90059830 100644
> --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_encoder.c
> +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_encoder.c
> @@ -67,7 +67,7 @@ int rzg2l_du_encoder_init(struct rzg2l_du_device  *rcdu,
>   {
>   	struct rzg2l_du_encoder *renc;
>   	struct drm_connector *connector;
> -	struct drm_bridge *bridge;
> +	struct drm_bridge *bridge __free(drm_bridge_put) = NULL;
>   	int ret;
>   
>   	/*
> @@ -83,10 +83,21 @@ int rzg2l_du_encoder_init(struct rzg2l_du_device  *rcdu,
>   
>   		bridge = devm_drm_panel_bridge_add_typed(rcdu->dev, panel,
>   							 DRM_MODE_CONNECTOR_DPI);
> -		if (IS_ERR(bridge))
> -			return PTR_ERR(bridge);
> +		if (IS_ERR(bridge)) {
> +			// Inhibit the cleanup action on an ERR_PTR
> +			ret = PTR_ERR(bridge);
> +			bridge = NULL;
> +			return ret;
> +		}

Can't you use 
https://elixir.bootlin.com/linux/v6.19.11/source/include/linux/cleanup.h#L230?

return PTR_ERR(no_free_ptr(bridge));

With or without this modification:

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>

> +
> +		/*
> +		 * The reference taken by devm_drm_panel_bridge_add_typed() is
> +		 * released automatically. Take a second one for the __free()
> +		 * when this function will return.
> +		 */
> +		drm_bridge_get(bridge);
>   	} else {
> -		bridge = of_drm_find_bridge(enc_node);
> +		bridge = of_drm_find_and_get_bridge(enc_node);
>   		if (!bridge)
>   			return -EPROBE_DEFER;
>   	}
>