linux-kernel - Re: [PATCH] drm/kmb: Replace of_node_put() with automatic cleanup handler

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Mon, 29 Apr 2024 12:18:03 +0200
From: Javier Carrasco <javier.carrasco.cruz@...il.com>
To: Anitha Chrisanthus <anitha.chrisanthus@...el.com>,
 David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
 Julia Lawall <julia.lawall@...ia.fr>
Subject: Re: [PATCH] drm/kmb: Replace of_node_put() with automatic cleanup
 handler

On 4/10/24 22:45, Javier Carrasco wrote:
> Make use of the __free() cleanup handler to automatically free nodes
> when they get out of scope.
> 
> Suggested-by: Julia Lawall <julia.lawall@...ia.fr>
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@...il.com>
> ---
> The patch is based on the latest linux-next tag (next-20240410).
> ---
>  drivers/gpu/drm/kmb/kmb_drv.c | 13 ++++---------
>  drivers/gpu/drm/kmb/kmb_dsi.c | 11 ++++-------
>  2 files changed, 8 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
> index 169b83987ce2..1a743840688a 100644
> --- a/drivers/gpu/drm/kmb/kmb_drv.c
> +++ b/drivers/gpu/drm/kmb/kmb_drv.c
> @@ -480,8 +480,8 @@ static int kmb_probe(struct platform_device *pdev)
>  	struct device *dev = get_device(&pdev->dev);
>  	struct kmb_drm_private *kmb;
>  	int ret = 0;
> -	struct device_node *dsi_in;
> -	struct device_node *dsi_node;
> +	struct device_node *dsi_in __free(device_node) =
> +		of_graph_get_endpoint_by_regs(dev->of_node, 0, 0);
>  	struct platform_device *dsi_pdev;
>  
>  	/* The bridge (ADV 7535) will return -EPROBE_DEFER until it
> @@ -491,28 +491,23 @@ static int kmb_probe(struct platform_device *pdev)
>  	 *  and then the rest of the driver initialization can proceed
>  	 *  afterwards and the bridge can be successfully attached.
>  	 */
> -	dsi_in = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0);
>  	if (!dsi_in) {
>  		DRM_ERROR("Failed to get dsi_in node info from DT");
>  		return -EINVAL;
>  	}
> -	dsi_node = of_graph_get_remote_port_parent(dsi_in);
> +	struct device_node *dsi_node __free(device_node) =
> +		of_graph_get_remote_port_parent(dsi_in);
>  	if (!dsi_node) {
> -		of_node_put(dsi_in);
>  		DRM_ERROR("Failed to get dsi node from DT\n");
>  		return -EINVAL;
>  	}
>  
>  	dsi_pdev = of_find_device_by_node(dsi_node);
>  	if (!dsi_pdev) {
> -		of_node_put(dsi_in);
> -		of_node_put(dsi_node);
>  		DRM_ERROR("Failed to get dsi platform device\n");
>  		return -EINVAL;
>  	}
>  
> -	of_node_put(dsi_in);
> -	of_node_put(dsi_node);
>  	ret = kmb_dsi_host_bridge_init(get_device(&dsi_pdev->dev));
>  
>  	if (ret == -EPROBE_DEFER) {
> diff --git a/drivers/gpu/drm/kmb/kmb_dsi.c b/drivers/gpu/drm/kmb/kmb_dsi.c
> index cf7cf0b07541..61f02462b778 100644
> --- a/drivers/gpu/drm/kmb/kmb_dsi.c
> +++ b/drivers/gpu/drm/kmb/kmb_dsi.c
> @@ -216,8 +216,6 @@ static const struct mipi_dsi_host_ops kmb_dsi_host_ops = {
>  
>  int kmb_dsi_host_bridge_init(struct device *dev)
>  {
> -	struct device_node *encoder_node, *dsi_out;
> -
>  	/* Create and register MIPI DSI host */
>  	if (!dsi_host) {
>  		dsi_host = kzalloc(sizeof(*dsi_host), GFP_KERNEL);
> @@ -239,21 +237,20 @@ int kmb_dsi_host_bridge_init(struct device *dev)
>  	}
>  
>  	/* Find ADV7535 node and initialize it */
> -	dsi_out = of_graph_get_endpoint_by_regs(dev->of_node, 0, 1);
> +	struct device_node *dsi_out __free(device_node) =
> +		of_graph_get_endpoint_by_regs(dev->of_node, 0, 1);
>  	if (!dsi_out) {
>  		DRM_ERROR("Failed to get dsi_out node info from DT\n");
>  		return -EINVAL;
>  	}
> -	encoder_node = of_graph_get_remote_port_parent(dsi_out);
> +	struct device_node *encoder_node __free(device_node) =
> +		of_graph_get_remote_port_parent(dsi_out);
>  	if (!encoder_node) {
> -		of_node_put(dsi_out);
>  		DRM_ERROR("Failed to get bridge info from DT\n");
>  		return -EINVAL;
>  	}
>  	/* Locate drm bridge from the hdmi encoder DT node */
>  	adv_bridge = of_drm_find_bridge(encoder_node);
> -	of_node_put(dsi_out);
> -	of_node_put(encoder_node);
>  	if (!adv_bridge) {
>  		DRM_DEBUG("Wait for external bridge driver DT\n");
>  		return -EPROBE_DEFER;
> 
> ---
> base-commit: 6ebf211bb11dfc004a2ff73a9de5386fa309c430
> change-id: 20240410-kmb_of_node_put-aaf1c77d9610
> 
> Best regards,

Hi,

according to Patchwork, this patch is still marked as "new", but also
"archived", and still did not get any feedback.

Apparently, this new cleanup mechanism has already been applied to the
subsystem's code (at least in drm/exynos/exynos_hdmi.c in linux-next),
and this one would not be the first case anymore.

Thanks and best regards,
Javier Carrasco