linux-kernel - Re: [PATCH v6 15/26] drm/bridge: devm_drm_of_get_bridge and drmm_of_get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <w3qufv73ldzdcfuz6n3prx4di2dhoq2wfqmmvxvxkea6uqxkge@pjwmugvicsbt>
Date: Fri, 7 Feb 2025 05:17:43 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Luca Ceresoli <luca.ceresoli@...tlin.com>
Cc: Simona Vetter <simona@...ll.ch>, Inki Dae <inki.dae@...sung.com>, 
	Jagan Teki <jagan@...rulasolutions.com>, Marek Szyprowski <m.szyprowski@...sung.com>, 
	Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>, Shawn Guo <shawnguo@...nel.org>, 
	Sascha Hauer <s.hauer@...gutronix.de>, Pengutronix Kernel Team <kernel@...gutronix.de>, 
	Fabio Estevam <festevam@...il.com>, Daniel Thompson <danielt@...nel.org>, 
	Andrzej Hajda <andrzej.hajda@...el.com>, Jonathan Corbet <corbet@....net>, 
	Sam Ravnborg <sam@...nborg.org>, Boris Brezillon <bbrezillon@...nel.org>, 
	Nicolas Ferre <nicolas.ferre@...rochip.com>, Alexandre Belloni <alexandre.belloni@...tlin.com>, 
	Claudiu Beznea <claudiu.beznea@...on.dev>, Jessica Zhang <quic_jesszhan@...cinc.com>, 
	Paul Kocialkowski <contact@...lk.fr>, Maxime Ripard <mripard@...nel.org>, 
	Neil Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>, 
	Laurent Pinchart <Laurent.pinchart@...asonboard.com>, Jonas Karlman <jonas@...boo.se>, 
	Jernej Skrabec <jernej.skrabec@...il.com>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, 
	Hervé Codina <herve.codina@...tlin.com>, Thomas Petazzoni <thomas.petazzoni@...tlin.com>, 
	linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org, linux-doc@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, Paul Kocialkowski <paul.kocialkowski@...tlin.com>
Subject: Re: [PATCH v6 15/26] drm/bridge: devm_drm_of_get_bridge and
 drmm_of_get_bridge: automatically put the bridge

On Thu, Feb 06, 2025 at 07:14:30PM +0100, Luca Ceresoli wrote:
> Add a devm/drmm action to these functions so the bridge reference is
> dropped automatically when the caller is removed.

I think the get() should go to the underlying of_drm_bridge_find() function.

Also it really feels like it's an overkill to keep the wrappers. After
getting bridge being handled by the panel code would it be possible to
drop all of them? Then this patch might introduce one new devm_
function? Or are drmm_ functions actually being used to store data in
the drmm-managed memory?

> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@...tlin.com>
> 
> ---
> 
> This patch was added in v6.
> ---
>  drivers/gpu/drm/drm_bridge.c | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index f694b32ca59cb91c32846bc00b43360df41cc1ad..497ec06dfcb05ab5dee8ea5e8f1eafb9c2807612 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -33,6 +33,7 @@
>  #include <drm/drm_edid.h>
>  #include <drm/drm_encoder.h>
>  #include <drm/drm_file.h>
> +#include <drm/drm_managed.h>
>  #include <drm/drm_of.h>
>  #include <drm/drm_print.h>
>  
> @@ -1459,6 +1460,13 @@ static int of_drm_find_bridge_by_endpoint(const struct device_node *np,
>  	return ret;
>  }
>  
> +static void devm_drm_bridge_put_void(void *data)
> +{
> +	struct drm_bridge *bridge = (struct drm_bridge *)data;
> +
> +	drm_bridge_put(bridge);
> +}
> +
>  /**
>   * devm_drm_of_get_bridge - Return next bridge in the chain
>   * @dev: device to tie the bridge lifetime to
> @@ -1469,6 +1477,10 @@ static int of_drm_find_bridge_by_endpoint(const struct device_node *np,
>   * Given a DT node's port and endpoint number, finds the connected node
>   * and returns the associated bridge if any.
>   *
> + * The refcount of the returned bridge is incremented, but the caller does
> + * not have to call drm_bridge_put() when done with the bridge. It will be
> + * done by devres when @dev is removed.
> + *
>   * Returns a pointer to the bridge if successful, or an error pointer
>   * otherwise.
>   */
> @@ -1483,6 +1495,10 @@ struct drm_bridge *devm_drm_of_get_bridge(struct device *dev,
>  	if (ret)
>  		return ERR_PTR(ret);
>  
> +	ret = devm_add_action_or_reset(dev, devm_drm_bridge_put_void, bridge);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
>  	return bridge;
>  }
>  EXPORT_SYMBOL(devm_drm_of_get_bridge);
> @@ -1497,6 +1513,10 @@ EXPORT_SYMBOL(devm_drm_of_get_bridge);
>   * graph link search is not enough, e.g. for drivers that need to support
>   * panels described only as subnodes.
>   *
> + * The refcount of the returned bridge is incremented, but the caller does
> + * not have to call drm_bridge_put() when done with the bridge. It will be
> + * done by devres when @dev is removed.
> + *
>   * RETURNS:
>   * A pointer to the bridge if successful, or an error pointer otherwise.
>   */
> @@ -1513,10 +1533,21 @@ struct drm_bridge *devm_drm_of_get_bridge_by_node(struct device *dev,
>  	if (!bridge)
>  		return ERR_PTR(-ENODEV);
>  
> +	ret = devm_add_action_or_reset(dev, devm_drm_bridge_put_void, bridge);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
>  	return bridge;
>  }
>  EXPORT_SYMBOL(devm_drm_of_get_bridge_by_node);
>  
> +static void drmm_bridge_put_void(struct drm_device *drm, void *ptr)
> +{
> +	struct drm_bridge *bridge = ptr;
> +
> +	drm_bridge_put(bridge);
> +}
> +
>  /**
>   * drmm_of_get_bridge - Return next bridge in the chain
>   * @drm: device to tie the bridge lifetime to
> @@ -1527,6 +1558,10 @@ EXPORT_SYMBOL(devm_drm_of_get_bridge_by_node);
>   * Given a DT node's port and endpoint number, finds the connected node
>   * and returns the associated bridge if any.
>   *
> + * The refcount of the returned bridge is incremented, but the caller does
> + * not have to call drm_bridge_put() when done with the bridge. It will be
> + * done by drmm when @dev is removed.
> + *
>   * Returns a drmm managed pointer to the bridge if successful, or an error
>   * pointer otherwise.
>   */
> @@ -1541,6 +1576,10 @@ struct drm_bridge *drmm_of_get_bridge(struct drm_device *drm,
>  	if (ret)
>  		return ERR_PTR(ret);
>  
> +	ret = drmm_add_action_or_reset(drm, drmm_bridge_put_void, bridge);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
>  	return bridge;
>  }
>  EXPORT_SYMBOL(drmm_of_get_bridge);
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry