Linux ARM-MSM sub-architecture
 help / color / mirror / Atom feed
* [PATCH v2] drm/msm/dpu: Don't use devm for component devices
@ 2018-11-02 18:25 ` Jordan Crouse
  2018-11-05  6:54   ` Andrzej Hajda
  0 siblings, 1 reply; 3+ messages in thread
From: Jordan Crouse @ 2018-11-02 18:25 UTC (permalink / raw)
  To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: jshekhar-sgV2jX0FEOL9JmXXK+q4OQ,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	seanpaul-F7+t8E8rja9g9hUCZPvPmw, abhinavk-sgV2jX0FEOL9JmXXK+q4OQ,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Devices that are bound as components should not use devm since
device managed memory is not freed when the component is
unbound.

In particular this is an issue if the compoent bind fails
due to an -EPROBE_DEFER. In this case the bind would try again
later and any devm managed meory allocated during the former
aborted attempt would be leaked until the device itself was
destroyed.  Therefore, all the memory allocated during a bind
should be freed during an unbind (or bind error case) and there
isn't any reason to use devm for resources that have a explicit
teardown step.

This doesn't remove devm for all resources - in particular
msm_ioremap() still uses devm_ioremap() but thats a more
generic condition that can easily be addressed as a cleanup
later and the unbind code already does the requisite devm
calls to unmap it.

v2: free mp->clk_config on failure

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  4 ++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c |  6 +++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c     | 10 ++++++----
 drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c    |  8 +++++---
 4 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 82c55efb500f..287d4c3e58c3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2220,14 +2220,14 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev,
 	struct dpu_encoder_virt *dpu_enc = NULL;
 	int rc = 0;
 
-	dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL);
+	dpu_enc = kzalloc(sizeof(*dpu_enc), GFP_KERNEL);
 	if (!dpu_enc)
 		return ERR_PTR(ENOMEM);
 
 	rc = drm_encoder_init(dev, &dpu_enc->base, &dpu_encoder_funcs,
 			drm_enc_mode, NULL);
 	if (rc) {
-		devm_kfree(dev->dev, dpu_enc);
+		kfree(dpu_enc);
 		return ERR_PTR(rc);
 	}
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
index 89ee4b36beff..14fecf00e032 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
@@ -155,9 +155,7 @@ int msm_dss_parse_clock(struct platform_device *pdev,
 		return 0;
 	}
 
-	mp->clk_config = devm_kzalloc(&pdev->dev,
-				      sizeof(struct dss_clk) * num_clk,
-				      GFP_KERNEL);
+	mp->clk_config = kcalloc(num_clk, sizeof(struct dss_clk), GFP_KERNEL);
 	if (!mp->clk_config)
 		return -ENOMEM;
 
@@ -201,5 +199,7 @@ int msm_dss_parse_clock(struct platform_device *pdev,
 
 err:
 	msm_dss_put_clk(mp->clk_config, num_clk);
+	kfree(mp->clk_config);
+	mp->clk_config = NULL;
 	return rc;
 }
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 985c855796ae..5ac3c3f3b08d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1086,13 +1086,14 @@ static int dpu_bind(struct device *dev, struct device *master, void *data)
 	struct dss_module_power *mp;
 	int ret = 0;
 
-	dpu_kms = devm_kzalloc(&pdev->dev, sizeof(*dpu_kms), GFP_KERNEL);
+	dpu_kms = kzalloc(sizeof(*dpu_kms), GFP_KERNEL);
 	if (!dpu_kms)
 		return -ENOMEM;
 
 	mp = &dpu_kms->mp;
 	ret = msm_dss_parse_clock(pdev, mp);
 	if (ret) {
+		kfree(dpu_kms);
 		DPU_ERROR("failed to parse clocks, ret=%d\n", ret);
 		return ret;
 	}
@@ -1109,7 +1110,7 @@ static int dpu_bind(struct device *dev, struct device *master, void *data)
 	dpu_kms->rpm_enabled = true;
 
 	priv->kms = &dpu_kms->base;
-	return ret;
+	return 0;
 }
 
 static void dpu_unbind(struct device *dev, struct device *master, void *data)
@@ -1120,11 +1121,12 @@ static void dpu_unbind(struct device *dev, struct device *master, void *data)
 
 	dpu_power_resource_deinit(pdev, &dpu_kms->phandle);
 	msm_dss_put_clk(mp->clk_config, mp->num_clk);
-	devm_kfree(&pdev->dev, mp->clk_config);
-	mp->num_clk = 0;
+	kfree(mp->clk_config);
 
 	if (dpu_kms->rpm_enabled)
 		pm_runtime_disable(&pdev->dev);
+
+	kfree(dpu_kms);
 }
 
 static const struct component_ops dpu_ops = {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
index 2235ef8129f4..c82347afc967 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
@@ -161,7 +161,7 @@ static void dpu_mdss_destroy(struct drm_device *dev)
 	free_irq(platform_get_irq(pdev, 0), dpu_mdss);
 
 	msm_dss_put_clk(mp->clk_config, mp->num_clk);
-	devm_kfree(&pdev->dev, mp->clk_config);
+	kfree(mp->clk_config);
 
 	if (dpu_mdss->mmio)
 		devm_iounmap(&pdev->dev, dpu_mdss->mmio);
@@ -169,6 +169,8 @@ static void dpu_mdss_destroy(struct drm_device *dev)
 
 	pm_runtime_disable(dev->dev);
 	priv->mdss = NULL;
+
+	kfree(dpu_mdss);
 }
 
 static const struct msm_mdss_funcs mdss_funcs = {
@@ -186,7 +188,7 @@ int dpu_mdss_init(struct drm_device *dev)
 	struct dss_module_power *mp;
 	int ret = 0;
 
-	dpu_mdss = devm_kzalloc(dev->dev, sizeof(*dpu_mdss), GFP_KERNEL);
+	dpu_mdss = kzalloc(sizeof(*dpu_mdss), GFP_KERNEL);
 	if (!dpu_mdss)
 		return -ENOMEM;
 
@@ -238,8 +240,8 @@ int dpu_mdss_init(struct drm_device *dev)
 	_dpu_mdss_irq_domain_fini(dpu_mdss);
 irq_domain_error:
 	msm_dss_put_clk(mp->clk_config, mp->num_clk);
+	kfree(mp->clk_config);
 clk_parse_err:
-	devm_kfree(&pdev->dev, mp->clk_config);
 	if (dpu_mdss->mmio)
 		devm_iounmap(&pdev->dev, dpu_mdss->mmio);
 	dpu_mdss->mmio = NULL;
-- 
2.18.0

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] drm/msm/dpu: Don't use devm for component devices
  2018-11-02 18:25 ` [PATCH v2] drm/msm/dpu: Don't use devm for component devices Jordan Crouse
@ 2018-11-05  6:54   ` Andrzej Hajda
       [not found]     ` <8aa14e93-d219-d3c5-fe1a-0d77d0d53620-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
  0 siblings, 1 reply; 3+ messages in thread
From: Andrzej Hajda @ 2018-11-05  6:54 UTC (permalink / raw)
  To: Jordan Crouse, freedreno
  Cc: jshekhar, linux-arm-msm, seanpaul, dri-devel, abhinavk

On 02.11.2018 19:25, Jordan Crouse wrote:
> Devices that are bound as components should not use devm since
> device managed memory is not freed when the component is
> unbound.
>
> In particular this is an issue if the compoent bind fails
> due to an -EPROBE_DEFER. In this case the bind would try again
> later and any devm managed meory allocated during the former
> aborted attempt would be leaked until the device itself was
> destroyed.  Therefore, all the memory allocated during a bind
> should be freed during an unbind (or bind error case) and there
> isn't any reason to use devm for resources that have a explicit
> teardown step.

It does not look correct, component framework should properly free devm
resources, ie if devm resource was allocated in bind callback, it should
be released after unbind or failed bind.
See comments inside component_bind[1]. If it does not work it should be
fixed in components.

[1]:
https://elixir.bootlin.com/linux/latest/source/drivers/base/component.c#L464

Regards
Andrzej



>
> This doesn't remove devm for all resources - in particular
> msm_ioremap() still uses devm_ioremap() but thats a more
> generic condition that can easily be addressed as a cleanup
> later and the unbind code already does the requisite devm
> calls to unmap it.
>
> v2: free mp->clk_config on failure
>
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  4 ++--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c |  6 +++---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c     | 10 ++++++----
>  drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c    |  8 +++++---
>  4 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 82c55efb500f..287d4c3e58c3 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -2220,14 +2220,14 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev,
>  	struct dpu_encoder_virt *dpu_enc = NULL;
>  	int rc = 0;
>  
> -	dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL);
> +	dpu_enc = kzalloc(sizeof(*dpu_enc), GFP_KERNEL);
>  	if (!dpu_enc)
>  		return ERR_PTR(ENOMEM);
>  
>  	rc = drm_encoder_init(dev, &dpu_enc->base, &dpu_encoder_funcs,
>  			drm_enc_mode, NULL);
>  	if (rc) {
> -		devm_kfree(dev->dev, dpu_enc);
> +		kfree(dpu_enc);
>  		return ERR_PTR(rc);
>  	}
>  
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
> index 89ee4b36beff..14fecf00e032 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
> @@ -155,9 +155,7 @@ int msm_dss_parse_clock(struct platform_device *pdev,
>  		return 0;
>  	}
>  
> -	mp->clk_config = devm_kzalloc(&pdev->dev,
> -				      sizeof(struct dss_clk) * num_clk,
> -				      GFP_KERNEL);
> +	mp->clk_config = kcalloc(num_clk, sizeof(struct dss_clk), GFP_KERNEL);
>  	if (!mp->clk_config)
>  		return -ENOMEM;
>  
> @@ -201,5 +199,7 @@ int msm_dss_parse_clock(struct platform_device *pdev,
>  
>  err:
>  	msm_dss_put_clk(mp->clk_config, num_clk);
> +	kfree(mp->clk_config);
> +	mp->clk_config = NULL;
>  	return rc;
>  }
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 985c855796ae..5ac3c3f3b08d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -1086,13 +1086,14 @@ static int dpu_bind(struct device *dev, struct device *master, void *data)
>  	struct dss_module_power *mp;
>  	int ret = 0;
>  
> -	dpu_kms = devm_kzalloc(&pdev->dev, sizeof(*dpu_kms), GFP_KERNEL);
> +	dpu_kms = kzalloc(sizeof(*dpu_kms), GFP_KERNEL);
>  	if (!dpu_kms)
>  		return -ENOMEM;
>  
>  	mp = &dpu_kms->mp;
>  	ret = msm_dss_parse_clock(pdev, mp);
>  	if (ret) {
> +		kfree(dpu_kms);
>  		DPU_ERROR("failed to parse clocks, ret=%d\n", ret);
>  		return ret;
>  	}
> @@ -1109,7 +1110,7 @@ static int dpu_bind(struct device *dev, struct device *master, void *data)
>  	dpu_kms->rpm_enabled = true;
>  
>  	priv->kms = &dpu_kms->base;
> -	return ret;
> +	return 0;
>  }
>  
>  static void dpu_unbind(struct device *dev, struct device *master, void *data)
> @@ -1120,11 +1121,12 @@ static void dpu_unbind(struct device *dev, struct device *master, void *data)
>  
>  	dpu_power_resource_deinit(pdev, &dpu_kms->phandle);
>  	msm_dss_put_clk(mp->clk_config, mp->num_clk);
> -	devm_kfree(&pdev->dev, mp->clk_config);
> -	mp->num_clk = 0;
> +	kfree(mp->clk_config);
>  
>  	if (dpu_kms->rpm_enabled)
>  		pm_runtime_disable(&pdev->dev);
> +
> +	kfree(dpu_kms);
>  }
>  
>  static const struct component_ops dpu_ops = {
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> index 2235ef8129f4..c82347afc967 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> @@ -161,7 +161,7 @@ static void dpu_mdss_destroy(struct drm_device *dev)
>  	free_irq(platform_get_irq(pdev, 0), dpu_mdss);
>  
>  	msm_dss_put_clk(mp->clk_config, mp->num_clk);
> -	devm_kfree(&pdev->dev, mp->clk_config);
> +	kfree(mp->clk_config);
>  
>  	if (dpu_mdss->mmio)
>  		devm_iounmap(&pdev->dev, dpu_mdss->mmio);
> @@ -169,6 +169,8 @@ static void dpu_mdss_destroy(struct drm_device *dev)
>  
>  	pm_runtime_disable(dev->dev);
>  	priv->mdss = NULL;
> +
> +	kfree(dpu_mdss);
>  }
>  
>  static const struct msm_mdss_funcs mdss_funcs = {
> @@ -186,7 +188,7 @@ int dpu_mdss_init(struct drm_device *dev)
>  	struct dss_module_power *mp;
>  	int ret = 0;
>  
> -	dpu_mdss = devm_kzalloc(dev->dev, sizeof(*dpu_mdss), GFP_KERNEL);
> +	dpu_mdss = kzalloc(sizeof(*dpu_mdss), GFP_KERNEL);
>  	if (!dpu_mdss)
>  		return -ENOMEM;
>  
> @@ -238,8 +240,8 @@ int dpu_mdss_init(struct drm_device *dev)
>  	_dpu_mdss_irq_domain_fini(dpu_mdss);
>  irq_domain_error:
>  	msm_dss_put_clk(mp->clk_config, mp->num_clk);
> +	kfree(mp->clk_config);
>  clk_parse_err:
> -	devm_kfree(&pdev->dev, mp->clk_config);
>  	if (dpu_mdss->mmio)
>  		devm_iounmap(&pdev->dev, dpu_mdss->mmio);
>  	dpu_mdss->mmio = NULL;


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] drm/msm/dpu: Don't use devm for component devices
       [not found]     ` <8aa14e93-d219-d3c5-fe1a-0d77d0d53620-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
@ 2018-11-05 16:40       ` Jordan Crouse
  0 siblings, 0 replies; 3+ messages in thread
From: Jordan Crouse @ 2018-11-05 16:40 UTC (permalink / raw)
  To: Andrzej Hajda
  Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	jshekhar-sgV2jX0FEOL9JmXXK+q4OQ, seanpaul-F7+t8E8rja9g9hUCZPvPmw,
	abhinavk-sgV2jX0FEOL9JmXXK+q4OQ,
	freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

On Mon, Nov 05, 2018 at 07:54:52AM +0100, Andrzej Hajda wrote:
> On 02.11.2018 19:25, Jordan Crouse wrote:
> > Devices that are bound as components should not use devm since
> > device managed memory is not freed when the component is
> > unbound.
> >
> > In particular this is an issue if the compoent bind fails
> > due to an -EPROBE_DEFER. In this case the bind would try again
> > later and any devm managed meory allocated during the former
> > aborted attempt would be leaked until the device itself was
> > destroyed.  Therefore, all the memory allocated during a bind
> > should be freed during an unbind (or bind error case) and there
> > isn't any reason to use devm for resources that have a explicit
> > teardown step.
> 
> It does not look correct, component framework should properly free devm
> resources, ie if devm resource was allocated in bind callback, it should
> be released after unbind or failed bind.
> See comments inside component_bind[1]. If it does not work it should be
> fixed in components.

I looked again - you are right.  There is a devres_release_group() in the
failure path. I'll rework this patch to remove the devm release calls instead
and rely on that.

Thanks,
Jordan

> [1]:
> https://elixir.bootlin.com/linux/latest/source/drivers/base/component.c#L464
> 
> Regards
> Andrzej
> 
> 
> 
> >
> > This doesn't remove devm for all resources - in particular
> > msm_ioremap() still uses devm_ioremap() but thats a more
> > generic condition that can easily be addressed as a cleanup
> > later and the unbind code already does the requisite devm
> > calls to unmap it.
> >
> > v2: free mp->clk_config on failure
> >
> > Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> > ---
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  4 ++--
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c |  6 +++---
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c     | 10 ++++++----
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c    |  8 +++++---
> >  4 files changed, 16 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > index 82c55efb500f..287d4c3e58c3 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > @@ -2220,14 +2220,14 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev,
> >  	struct dpu_encoder_virt *dpu_enc = NULL;
> >  	int rc = 0;
> >  
> > -	dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL);
> > +	dpu_enc = kzalloc(sizeof(*dpu_enc), GFP_KERNEL);
> >  	if (!dpu_enc)
> >  		return ERR_PTR(ENOMEM);
> >  
> >  	rc = drm_encoder_init(dev, &dpu_enc->base, &dpu_encoder_funcs,
> >  			drm_enc_mode, NULL);
> >  	if (rc) {
> > -		devm_kfree(dev->dev, dpu_enc);
> > +		kfree(dpu_enc);
> >  		return ERR_PTR(rc);
> >  	}
> >  
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
> > index 89ee4b36beff..14fecf00e032 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
> > @@ -155,9 +155,7 @@ int msm_dss_parse_clock(struct platform_device *pdev,
> >  		return 0;
> >  	}
> >  
> > -	mp->clk_config = devm_kzalloc(&pdev->dev,
> > -				      sizeof(struct dss_clk) * num_clk,
> > -				      GFP_KERNEL);
> > +	mp->clk_config = kcalloc(num_clk, sizeof(struct dss_clk), GFP_KERNEL);
> >  	if (!mp->clk_config)
> >  		return -ENOMEM;
> >  
> > @@ -201,5 +199,7 @@ int msm_dss_parse_clock(struct platform_device *pdev,
> >  
> >  err:
> >  	msm_dss_put_clk(mp->clk_config, num_clk);
> > +	kfree(mp->clk_config);
> > +	mp->clk_config = NULL;
> >  	return rc;
> >  }
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > index 985c855796ae..5ac3c3f3b08d 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > @@ -1086,13 +1086,14 @@ static int dpu_bind(struct device *dev, struct device *master, void *data)
> >  	struct dss_module_power *mp;
> >  	int ret = 0;
> >  
> > -	dpu_kms = devm_kzalloc(&pdev->dev, sizeof(*dpu_kms), GFP_KERNEL);
> > +	dpu_kms = kzalloc(sizeof(*dpu_kms), GFP_KERNEL);
> >  	if (!dpu_kms)
> >  		return -ENOMEM;
> >  
> >  	mp = &dpu_kms->mp;
> >  	ret = msm_dss_parse_clock(pdev, mp);
> >  	if (ret) {
> > +		kfree(dpu_kms);
> >  		DPU_ERROR("failed to parse clocks, ret=%d\n", ret);
> >  		return ret;
> >  	}
> > @@ -1109,7 +1110,7 @@ static int dpu_bind(struct device *dev, struct device *master, void *data)
> >  	dpu_kms->rpm_enabled = true;
> >  
> >  	priv->kms = &dpu_kms->base;
> > -	return ret;
> > +	return 0;
> >  }
> >  
> >  static void dpu_unbind(struct device *dev, struct device *master, void *data)
> > @@ -1120,11 +1121,12 @@ static void dpu_unbind(struct device *dev, struct device *master, void *data)
> >  
> >  	dpu_power_resource_deinit(pdev, &dpu_kms->phandle);
> >  	msm_dss_put_clk(mp->clk_config, mp->num_clk);
> > -	devm_kfree(&pdev->dev, mp->clk_config);
> > -	mp->num_clk = 0;
> > +	kfree(mp->clk_config);
> >  
> >  	if (dpu_kms->rpm_enabled)
> >  		pm_runtime_disable(&pdev->dev);
> > +
> > +	kfree(dpu_kms);
> >  }
> >  
> >  static const struct component_ops dpu_ops = {
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> > index 2235ef8129f4..c82347afc967 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> > @@ -161,7 +161,7 @@ static void dpu_mdss_destroy(struct drm_device *dev)
> >  	free_irq(platform_get_irq(pdev, 0), dpu_mdss);
> >  
> >  	msm_dss_put_clk(mp->clk_config, mp->num_clk);
> > -	devm_kfree(&pdev->dev, mp->clk_config);
> > +	kfree(mp->clk_config);
> >  
> >  	if (dpu_mdss->mmio)
> >  		devm_iounmap(&pdev->dev, dpu_mdss->mmio);
> > @@ -169,6 +169,8 @@ static void dpu_mdss_destroy(struct drm_device *dev)
> >  
> >  	pm_runtime_disable(dev->dev);
> >  	priv->mdss = NULL;
> > +
> > +	kfree(dpu_mdss);
> >  }
> >  
> >  static const struct msm_mdss_funcs mdss_funcs = {
> > @@ -186,7 +188,7 @@ int dpu_mdss_init(struct drm_device *dev)
> >  	struct dss_module_power *mp;
> >  	int ret = 0;
> >  
> > -	dpu_mdss = devm_kzalloc(dev->dev, sizeof(*dpu_mdss), GFP_KERNEL);
> > +	dpu_mdss = kzalloc(sizeof(*dpu_mdss), GFP_KERNEL);
> >  	if (!dpu_mdss)
> >  		return -ENOMEM;
> >  
> > @@ -238,8 +240,8 @@ int dpu_mdss_init(struct drm_device *dev)
> >  	_dpu_mdss_irq_domain_fini(dpu_mdss);
> >  irq_domain_error:
> >  	msm_dss_put_clk(mp->clk_config, mp->num_clk);
> > +	kfree(mp->clk_config);
> >  clk_parse_err:
> > -	devm_kfree(&pdev->dev, mp->clk_config);
> >  	if (dpu_mdss->mmio)
> >  		devm_iounmap(&pdev->dev, dpu_mdss->mmio);
> >  	dpu_mdss->mmio = NULL;
> 
> 

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-11-05 16:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CGME20181102182641epcas5p4ce8ab41382cb0e40e305bf85f5e09759@epcas5p4.samsung.com>
2018-11-02 18:25 ` [PATCH v2] drm/msm/dpu: Don't use devm for component devices Jordan Crouse
2018-11-05  6:54   ` Andrzej Hajda
     [not found]     ` <8aa14e93-d219-d3c5-fe1a-0d77d0d53620-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2018-11-05 16:40       ` Jordan Crouse

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox