diff mbox

[1/3] drm/exynos: free DP if probe fails to find a panel or bridge

Message ID 1416527664-10553-1-git-send-email-gustavo@padovan.org (mailing list archive)
State New, archived
Headers show

Commit Message

Gustavo Padovan Nov. 20, 2014, 11:54 p.m. UTC
From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

DP was leaked everytime function returns EPROBE_DEFER, free it before
returning.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_dp_core.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

Comments

Ajay kumar Nov. 21, 2014, 5:40 p.m. UTC | #1
Hi Gustavo,


On Fri, Nov 21, 2014 at 5:24 AM, Gustavo Padovan <gustavo@padovan.org> wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>
> DP was leaked everytime function returns EPROBE_DEFER, free it before
> returning.
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  drivers/gpu/drm/exynos/exynos_dp_core.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
> index 85762cf..6fd4a46 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
> +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
> @@ -1336,8 +1336,10 @@ static int exynos_dp_probe(struct platform_device *pdev)
>         if (panel_node) {
>                 dp->panel = of_drm_find_panel(panel_node);
>                 of_node_put(panel_node);
> -               if (!dp->panel)
> -                       return -EPROBE_DEFER;
> +               if (!dp->panel) {
> +                       ret = -EPROBE_DEFER;
> +                       goto free_dp;
> +               }
>         }
>
>         endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
> @@ -1346,10 +1348,14 @@ static int exynos_dp_probe(struct platform_device *pdev)
>                 if (bridge_node) {
>                         dp->bridge = of_drm_find_bridge(bridge_node);
>                         of_node_put(bridge_node);
> -                       if (!dp->bridge)
> -                               return -EPROBE_DEFER;
> -               } else
> -                       return -EPROBE_DEFER;
> +                       if (!dp->bridge) {
> +                               ret = -EPROBE_DEFER;
> +                               goto free_dp;
> +                       }
> +               } else {
> +                       ret = -EPROBE_DEFER;
> +                       goto free_dp;
> +               }
>         }
>
>         exynos_dp_display.ctx = dp;
> @@ -1359,6 +1365,9 @@ static int exynos_dp_probe(struct platform_device *pdev)
>                 exynos_drm_component_del(&pdev->dev,
>                                                 EXYNOS_DEVICE_TYPE_CONNECTOR);
>
> +free_dp:
> +       devm_kfree(dev, dp);
I guess the driver core takes care of freeing the devm memory when the
probe fails?
Will it not happen during PROBE_DEFER?

Inki/Jingoo - Is this change really necessary?

Ajay

>         return ret;
>  }
>
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Inki Dae Nov. 22, 2014, 2:56 p.m. UTC | #2
2014-11-21 8:54 GMT+09:00 Gustavo Padovan <gustavo@padovan.org>:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>
> DP was leaked everytime function returns EPROBE_DEFER, free it before
> returning.

It seems that you misunderstood devm_* api.

Thanks,
Inki Dae

>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  drivers/gpu/drm/exynos/exynos_dp_core.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
> index 85762cf..6fd4a46 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
> +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
> @@ -1336,8 +1336,10 @@ static int exynos_dp_probe(struct platform_device *pdev)
>         if (panel_node) {
>                 dp->panel = of_drm_find_panel(panel_node);
>                 of_node_put(panel_node);
> -               if (!dp->panel)
> -                       return -EPROBE_DEFER;
> +               if (!dp->panel) {
> +                       ret = -EPROBE_DEFER;
> +                       goto free_dp;
> +               }
>         }
>
>         endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
> @@ -1346,10 +1348,14 @@ static int exynos_dp_probe(struct platform_device *pdev)
>                 if (bridge_node) {
>                         dp->bridge = of_drm_find_bridge(bridge_node);
>                         of_node_put(bridge_node);
> -                       if (!dp->bridge)
> -                               return -EPROBE_DEFER;
> -               } else
> -                       return -EPROBE_DEFER;
> +                       if (!dp->bridge) {
> +                               ret = -EPROBE_DEFER;
> +                               goto free_dp;
> +                       }
> +               } else {
> +                       ret = -EPROBE_DEFER;
> +                       goto free_dp;
> +               }
>         }
>
>         exynos_dp_display.ctx = dp;
> @@ -1359,6 +1365,9 @@ static int exynos_dp_probe(struct platform_device *pdev)
>                 exynos_drm_component_del(&pdev->dev,
>                                                 EXYNOS_DEVICE_TYPE_CONNECTOR);
>
> +free_dp:
> +       devm_kfree(dev, dp);
> +
>         return ret;
>  }
>
> --
> 1.9.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Jingoo Han Nov. 23, 2014, 9:22 a.m. UTC | #3
On Saturday, November 22, 2014 2:40 AM, Ajay kumar wrote:
> 
> On Fri, Nov 21, 2014 at 5:24 AM, Gustavo Padovan <gustavo@padovan.org> wrote:
> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> >
> > DP was leaked everytime function returns EPROBE_DEFER, free it before
> > returning.
> >
> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > ---
> >  drivers/gpu/drm/exynos/exynos_dp_core.c | 21 +++++++++++++++------
> >  1 file changed, 15 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
> > index 85762cf..6fd4a46 100644
> > --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
> > +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
> > @@ -1336,8 +1336,10 @@ static int exynos_dp_probe(struct platform_device *pdev)
> >         if (panel_node) {
> >                 dp->panel = of_drm_find_panel(panel_node);
> >                 of_node_put(panel_node);
> > -               if (!dp->panel)
> > -                       return -EPROBE_DEFER;
> > +               if (!dp->panel) {
> > +                       ret = -EPROBE_DEFER;
> > +                       goto free_dp;
> > +               }
> >         }
> >
> >         endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
> > @@ -1346,10 +1348,14 @@ static int exynos_dp_probe(struct platform_device *pdev)
> >                 if (bridge_node) {
> >                         dp->bridge = of_drm_find_bridge(bridge_node);
> >                         of_node_put(bridge_node);
> > -                       if (!dp->bridge)
> > -                               return -EPROBE_DEFER;
> > -               } else
> > -                       return -EPROBE_DEFER;
> > +                       if (!dp->bridge) {
> > +                               ret = -EPROBE_DEFER;
> > +                               goto free_dp;
> > +                       }
> > +               } else {
> > +                       ret = -EPROBE_DEFER;
> > +                       goto free_dp;
> > +               }
> >         }
> >
> >         exynos_dp_display.ctx = dp;
> > @@ -1359,6 +1365,9 @@ static int exynos_dp_probe(struct platform_device *pdev)
> >                 exynos_drm_component_del(&pdev->dev,
> >                                                 EXYNOS_DEVICE_TYPE_CONNECTOR);
> >
> > +free_dp:
> > +       devm_kfree(dev, dp);
> I guess the driver core takes care of freeing the devm memory when the
> probe fails?
> Will it not happen during PROBE_DEFER?
> 
> Inki/Jingoo - Is this change really necessary?

As far as I know, this devm_kfree() in unnecessary,
even though PROBE_DEFER is returned.
If I am wrong, please let me know. Thanks.

Best regards,
Jingoo Han

> 
> Ajay
> 
> >         return ret;
> >  }
> >
> > --
> > 1.9.3
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gustavo Padovan Nov. 24, 2014, 1:02 p.m. UTC | #4
2014-11-22 Inki Dae <inki.dae@samsung.com>:

> 2014-11-21 8:54 GMT+09:00 Gustavo Padovan <gustavo@padovan.org>:
> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> >
> > DP was leaked everytime function returns EPROBE_DEFER, free it before
> > returning.
> 
> It seems that you misunderstood devm_* api.

Yeah, I though it would only free memory at unload of the module but it also
free it when the probe fails.

> 
> Thanks,
> Inki Dae
> 
> >
> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > ---
> >  drivers/gpu/drm/exynos/exynos_dp_core.c | 21 +++++++++++++++------
> >  1 file changed, 15 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
> > index 85762cf..6fd4a46 100644
> > --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
> > +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
> > @@ -1336,8 +1336,10 @@ static int exynos_dp_probe(struct platform_device *pdev)
> >         if (panel_node) {
> >                 dp->panel = of_drm_find_panel(panel_node);
> >                 of_node_put(panel_node);
> > -               if (!dp->panel)
> > -                       return -EPROBE_DEFER;
> > +               if (!dp->panel) {
> > +                       ret = -EPROBE_DEFER;
> > +                       goto free_dp;
> > +               }
> >         }
> >
> >         endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
> > @@ -1346,10 +1348,14 @@ static int exynos_dp_probe(struct platform_device *pdev)
> >                 if (bridge_node) {
> >                         dp->bridge = of_drm_find_bridge(bridge_node);
> >                         of_node_put(bridge_node);
> > -                       if (!dp->bridge)
> > -                               return -EPROBE_DEFER;
> > -               } else
> > -                       return -EPROBE_DEFER;
> > +                       if (!dp->bridge) {
> > +                               ret = -EPROBE_DEFER;
> > +                               goto free_dp;
> > +                       }
> > +               } else {
> > +                       ret = -EPROBE_DEFER;
> > +                       goto free_dp;
> > +               }
> >         }
> >
> >         exynos_dp_display.ctx = dp;
> > @@ -1359,6 +1365,9 @@ static int exynos_dp_probe(struct platform_device *pdev)
> >                 exynos_drm_component_del(&pdev->dev,
> >                                                 EXYNOS_DEVICE_TYPE_CONNECTOR);
> >
> > +free_dp:
> > +       devm_kfree(dev, dp);
> > +
> >         return ret;
> >  }
> >
> > --
> > 1.9.3
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel

	Gustavo
diff mbox

Patch

diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
index 85762cf..6fd4a46 100644
--- a/drivers/gpu/drm/exynos/exynos_dp_core.c
+++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
@@ -1336,8 +1336,10 @@  static int exynos_dp_probe(struct platform_device *pdev)
 	if (panel_node) {
 		dp->panel = of_drm_find_panel(panel_node);
 		of_node_put(panel_node);
-		if (!dp->panel)
-			return -EPROBE_DEFER;
+		if (!dp->panel) {
+			ret = -EPROBE_DEFER;
+			goto free_dp;
+		}
 	}
 
 	endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
@@ -1346,10 +1348,14 @@  static int exynos_dp_probe(struct platform_device *pdev)
 		if (bridge_node) {
 			dp->bridge = of_drm_find_bridge(bridge_node);
 			of_node_put(bridge_node);
-			if (!dp->bridge)
-				return -EPROBE_DEFER;
-		} else
-			return -EPROBE_DEFER;
+			if (!dp->bridge) {
+				ret = -EPROBE_DEFER;
+				goto free_dp;
+			}
+		} else {
+			ret = -EPROBE_DEFER;
+			goto free_dp;
+		}
 	}
 
 	exynos_dp_display.ctx = dp;
@@ -1359,6 +1365,9 @@  static int exynos_dp_probe(struct platform_device *pdev)
 		exynos_drm_component_del(&pdev->dev,
 						EXYNOS_DEVICE_TYPE_CONNECTOR);
 
+free_dp:
+	devm_kfree(dev, dp);
+
 	return ret;
 }