Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1692888

Re: [PATCH v2 5/7] drm/stm: ltdc: add devm_reset_control & platform_get_ressource

Show all headers | View raw

Hi Philippe,

On Thu, 2017-07-20 at 14:05 +0200, Philippe CORNU wrote:
> Use devm_reset_control_get_exclusive to avoid resource leakage (based
> on patch "Convert drivers to explicit reset API" from Philipp Zabel).
> 
> Also use platform_get_resource, which is more usual and
> consistent with platform_get_irq called later.
> 
> Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
> Signed-off-by: Philippe CORNU <philippe.cornu@st.com>
> Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>

Looking at the usage below, this driver only seems to care about the
reset deassertion before register use, so this could use the shared API.
Further, it seems that this reset is optional.

> ---
>  drivers/gpu/drm/stm/ltdc.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> index 92e58ba..d826045 100644
> --- a/drivers/gpu/drm/stm/ltdc.c
> +++ b/drivers/gpu/drm/stm/ltdc.c
> @@ -874,7 +874,7 @@ int ltdc_load(struct drm_device *ddev)
>  	struct drm_panel *panel;
>  	struct drm_crtc *crtc;
>  	struct reset_control *rstc;
> -	struct resource res;
> +	struct resource *res;
>  	int irq, ret, i;
>  
>  	DRM_DEBUG_DRIVER("\n");
> @@ -883,7 +883,7 @@ int ltdc_load(struct drm_device *ddev)
>  	if (ret)
>  		return ret;
>  
> -	rstc = of_reset_control_get(np, NULL);
> +	rstc = devm_reset_control_get_exclusive(dev, NULL);

I would suggest to change this to

-	rstc = of_reset_control_get(np, NULL);
+	rstc = devm_reset_control_get_optional_shared(dev, NULL);
+	if (IS_ERR(rstc))
+		return PTR_ERR(rstc);

>  	mutex_init(&ldev->err_lock);
>  
> @@ -898,13 +898,14 @@ int ltdc_load(struct drm_device *ddev)
>  		return -ENODEV;
>  	}
>  
> -	if (of_address_to_resource(np, 0, &res)) {
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
>  		DRM_ERROR("Unable to get resource\n");
>  		ret = -ENODEV;
>  		goto err;
>  	}
>  
> -	ldev->regs = devm_ioremap_resource(dev, &res);
> +	ldev->regs = devm_ioremap_resource(dev, res);
>  	if (IS_ERR(ldev->regs)) {
>  		DRM_ERROR("Unable to get ltdc registers\n");
>  		ret = PTR_ERR(ldev->regs);

then below you can change:

-	if (!IS_ERR(rstc))
-		reset_control_deassert(rstc);
+	reset_control_deassert(rstc);

regards
Philipp

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v2 0/7] drm/stm: Various cleanups Philippe CORNU <philippe.cornu@st.com> - 2017-07-20 14:10 +0200
  [PATCH v2 5/7] drm/stm: ltdc: add devm_reset_control & platform_get_ressource Philippe CORNU <philippe.cornu@st.com> - 2017-07-20 14:10 +0200
    Re: [PATCH v2 5/7] drm/stm: ltdc: add devm_reset_control &  platform_get_ressource Philipp Zabel <p.zabel@pengutronix.de> - 2017-07-20 14:40 +0200
  [PATCH v2 3/7] drm/stm: ltdc: Lindent and minor cleanups Philippe CORNU <philippe.cornu@st.com> - 2017-07-20 14:10 +0200
  [PATCH v2 6/7] drm/stm: ltdc: Cleanup rename returned value Philippe CORNU <philippe.cornu@st.com> - 2017-07-20 14:10 +0200
  [PATCH v2 7/7] drm/stm: dsi: Constify phy ops structure Philippe CORNU <philippe.cornu@st.com> - 2017-07-20 14:10 +0200
  [PATCH v2 4/7] drm/stm: ltdc: Constify funcs structures Philippe CORNU <philippe.cornu@st.com> - 2017-07-20 14:10 +0200
  [PATCH v2 1/7] drm/stm: drv: Rename platform driver name Philippe CORNU <philippe.cornu@st.com> - 2017-07-20 14:10 +0200
  Re: [PATCH v2 0/7] drm/stm: Various cleanups Benjamin Gaignard <benjamin.gaignard@linaro.org> - 2017-07-20 14:50 +0200

csiph-web