[PATCH] drm/kmb: Replace of_node_put() with automatic cleanup handler

[PATCH] drm/kmb: Replace of_node_put() with automatic cleanup handler

[Javier Carrasco]

Make use of the __free() cleanup handler to automatically free nodes
when they get out of scope.

Suggested-by: Julia Lawall <julia.lawall@inria.fr>
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.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,
-- 
Javier Carrasco <javier.carrasco.cruz@gmail.com>

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

[Javier Carrasco]

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@inria.fr>
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.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