Re: [PATCH 3/3] drm/bridge: fsl-ldb: simplify device_node error handling

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Marco,

On Thu, May 15, 2025 at 12:24:53AM +0200, Marco Felsch wrote:
> Make use of __free(device_node) to simplify the of_node_put() error
> handling paths. No functional changes.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  drivers/gpu/drm/bridge/fsl-ldb.c | 24 +++++++++---------------
>  1 file changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c
> index e0a229c91953..cea9ddaa5e01 100644
> --- a/drivers/gpu/drm/bridge/fsl-ldb.c
> +++ b/drivers/gpu/drm/bridge/fsl-ldb.c
> @@ -287,8 +287,9 @@ static const struct drm_bridge_funcs funcs = {
>  static int fsl_ldb_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> -	struct device_node *panel_node;
> -	struct device_node *remote1, *remote2;
> +	struct device_node *panel_node __free(device_node) = NULL;
> +	struct device_node *remote1 __free(device_node) = NULL;
> +	struct device_node *remote2 __free(device_node) = NULL;
>  	struct drm_panel *panel;
>  	struct fsl_ldb *fsl_ldb;
>  	int dual_link;
> @@ -321,21 +322,16 @@ static int fsl_ldb_probe(struct platform_device *pdev)
>  	remote2 = of_graph_get_remote_node(dev->of_node, 2, 0);
>  	fsl_ldb->ch0_enabled = (remote1 != NULL);
>  	fsl_ldb->ch1_enabled = (remote2 != NULL);
> -	panel_node = of_node_get(remote1 ? remote1 : remote2);
> -	of_node_put(remote1);
> -	of_node_put(remote2);
> +	panel_node = remote1 ? remote1 : remote2;

This will cause a double put of panel_node, once due to __free() on
remote1 or remote2, and the second time due to __free() on panel_node.

>  
> -	if (!fsl_ldb->ch0_enabled && !fsl_ldb->ch1_enabled) {
> -		of_node_put(panel_node);
> +	if (!fsl_ldb->ch0_enabled && !fsl_ldb->ch1_enabled)
>  		return dev_err_probe(dev, -ENXIO, "No panel node found");
> -	}
>  
>  	dev_dbg(dev, "Using %s\n",
>  		fsl_ldb_is_dual(fsl_ldb) ? "dual-link mode" :
>  		fsl_ldb->ch0_enabled ? "channel 0" : "channel 1");
>  
>  	panel = of_drm_find_panel(panel_node);
> -	of_node_put(panel_node);
>  	if (IS_ERR(panel))
>  		return dev_err_probe(dev, PTR_ERR(panel), "drm panel not found\n");
>  
> @@ -345,14 +341,12 @@ static int fsl_ldb_probe(struct platform_device *pdev)
>  				     "drm panel-bridge add failed\n");
>  
>  	if (fsl_ldb_is_dual(fsl_ldb)) {
> -		struct device_node *port1, *port2;
> +		struct device_node *port1 __free(device_node) =
> +			of_graph_get_port_by_id(dev->of_node, 1);
> +		struct device_node *port2 __free(device_node) =
> +			of_graph_get_port_by_id(dev->of_node, 2);
>  
> -		port1 = of_graph_get_port_by_id(dev->of_node, 1);
> -		port2 = of_graph_get_port_by_id(dev->of_node, 2);
>  		dual_link = drm_of_lvds_get_dual_link_pixel_order(port1, port2);
> -		of_node_put(port1);
> -		of_node_put(port2);
> -
>  		if (dual_link < 0)
>  			return dev_err_probe(dev, dual_link,
>  					     "Error getting dual link configuration\n");

-- 
Regards,

Laurent Pinchart