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

Laurent Pinchart
2026-05-04T13:53:17Z

On Mon, May 04, 2026 at 12:45:04PM +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.

The change in drm_bridge_put() looks good to me. I'm less sure about
drm_bridge_get(), is there a valid use case to call get() on a bridge
that is not valid ? Doesn't it indicate a clear error in the caller ?

> 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/
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>
> Changes in v4:
> - removed incorrect change to drm_bridge_clear_and_put() kdoc
>
> Patch added in v2
> ---
> drivers/gpu/drm/drm_bridge.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index d4b3478258ec..6fb71de6d22a 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);

--
Regards,

Laurent Pinchart