[PATCH] drm/omap: dsi: Fix missing of_platform_depopulate()

[PATCH] drm/omap: dsi: Fix missing of_platform_depopulate()

[Tony Lindgren]

We're missing a call to of_platform_depopulate() on errors for dsi.
Looks like dss is already doing this.

Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/gpu/drm/omapdrm/dss/dsi.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
--- a/drivers/gpu/drm/omapdrm/dss/dsi.c
+++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
@@ -5429,15 +5429,19 @@ static int dsi_probe(struct platform_device *pdev)
 	}
 
 	r = of_platform_populate(dev->of_node, NULL, NULL, dev);
-	if (r)
+	if (r) {
 		DSSERR("Failed to populate DSI child devices: %d\n", r);
+		goto err_uninit_output;
+	}
 
 	r = component_add(&pdev->dev, &dsi_component_ops);
 	if (r)
-		goto err_uninit_output;
+		goto err_of_depopulate;
 
 	return 0;
 
+err_of_depopulate:
+	of_platform_depopulate(dev);
 err_uninit_output:
 	dsi_uninit_output(dsi);
 err_pm_disable:
-- 
2.19.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/omap: dsi: Fix missing of_platform_depopulate()

[Laurent Pinchart]

Hi Tony,

Thank you for the patch.

On Tuesday, 6 November 2018 17:28:02 EET Tony Lindgren wrote:
> We're missing a call to of_platform_depopulate() on errors for dsi.
> Looks like dss is already doing this.
> 
> Signed-off-by: Tony Lindgren <tony@atomide.com>

This makes sense to me. You may however want to note in the commit message 
that this patch also turns the of_platform_populate() failures into fatal 
errors.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/omapdrm/dss/dsi.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c
> b/drivers/gpu/drm/omapdrm/dss/dsi.c --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
> @@ -5429,15 +5429,19 @@ static int dsi_probe(struct platform_device *pdev)
>  	}
> 
>  	r = of_platform_populate(dev->of_node, NULL, NULL, dev);
> -	if (r)
> +	if (r) {
>  		DSSERR("Failed to populate DSI child devices: %d\n", r);
> +		goto err_uninit_output;
> +	}
> 
>  	r = component_add(&pdev->dev, &dsi_component_ops);
>  	if (r)
> -		goto err_uninit_output;
> +		goto err_of_depopulate;
> 
>  	return 0;
> 
> +err_of_depopulate:
> +	of_platform_depopulate(dev);
>  err_uninit_output:
>  	dsi_uninit_output(dsi);
>  err_pm_disable:

-- 
Regards,

Laurent Pinchart



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

Re: [PATCH] drm/omap: dsi: Fix missing of_platform_depopulate()

[Tony Lindgren]

* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [181107 05:30]:
> Hi Tony,
> 
> Thank you for the patch.
> 
> On Tuesday, 6 November 2018 17:28:02 EET Tony Lindgren wrote:
> > We're missing a call to of_platform_depopulate() on errors for dsi.
> > Looks like dss is already doing this.
> > 
> > Signed-off-by: Tony Lindgren <tony@atomide.com>
> 
> This makes sense to me. You may however want to note in the commit message 
> that this patch also turns the of_platform_populate() failures into fatal 
> errors.

Well of_platform_populate() failure should be fatal here,right? :)
Tomi, let me know if you need a resend with updated comments or
if you want to update the message yourself to your liking.

Regards,

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

Re: [PATCH] drm/omap: dsi: Fix missing of_platform_depopulate()

[Laurent Pinchart]

Hi Tony,

On Thursday, 8 November 2018 19:01:45 EET Tony Lindgren wrote:
> * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [181107 05:30]:
> > On Tuesday, 6 November 2018 17:28:02 EET Tony Lindgren wrote:
> > > We're missing a call to of_platform_depopulate() on errors for dsi.
> > > Looks like dss is already doing this.
> > > 
> > > Signed-off-by: Tony Lindgren <tony@atomide.com>
> > 
> > This makes sense to me. You may however want to note in the commit message
> > that this patch also turns the of_platform_populate() failures into fatal
> > errors.
> 
> Well of_platform_populate() failure should be fatal here,right? :)

I'm not complaining about the functional change, just the lack of visibility 
:-)

> Tomi, let me know if you need a resend with updated comments or
> if you want to update the message yourself to your liking.

-- 
Regards,

Laurent Pinchart



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

Re: [PATCH] drm/omap: dsi: Fix missing of_platform_depopulate()

[Sebastian Reichel]

[-- Attachment #1.1: Type: text/plain, Size: 1192 bytes --]

Hi,

On Tue, Nov 06, 2018 at 07:28:02AM -0800, Tony Lindgren wrote:
> We're missing a call to of_platform_depopulate() on errors for dsi.
> Looks like dss is already doing this.
> 
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

>  drivers/gpu/drm/omapdrm/dss/dsi.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
> @@ -5429,15 +5429,19 @@ static int dsi_probe(struct platform_device *pdev)
>  	}
>  
>  	r = of_platform_populate(dev->of_node, NULL, NULL, dev);
> -	if (r)
> +	if (r) {
>  		DSSERR("Failed to populate DSI child devices: %d\n", r);
> +		goto err_uninit_output;
> +	}
>  
>  	r = component_add(&pdev->dev, &dsi_component_ops);
>  	if (r)
> -		goto err_uninit_output;
> +		goto err_of_depopulate;
>  
>  	return 0;
>  
> +err_of_depopulate:
> +	of_platform_depopulate(dev);
>  err_uninit_output:
>  	dsi_uninit_output(dsi);
>  err_pm_disable:
> -- 
> 2.19.1

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH] drm/omap: dsi: Fix missing of_platform_depopulate()

[Tomi Valkeinen]

On 06/11/18 17:28, Tony Lindgren wrote:
> We're missing a call to of_platform_depopulate() on errors for dsi.
> Looks like dss is already doing this.
> 
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  drivers/gpu/drm/omapdrm/dss/dsi.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
> @@ -5429,15 +5429,19 @@ static int dsi_probe(struct platform_device *pdev)
>  	}
>  
>  	r = of_platform_populate(dev->of_node, NULL, NULL, dev);
> -	if (r)
> +	if (r) {
>  		DSSERR("Failed to populate DSI child devices: %d\n", r);
> +		goto err_uninit_output;
> +	}
>  
>  	r = component_add(&pdev->dev, &dsi_component_ops);
>  	if (r)
> -		goto err_uninit_output;
> +		goto err_of_depopulate;
>  
>  	return 0;
>  
> +err_of_depopulate:
> +	of_platform_depopulate(dev);
>  err_uninit_output:
>  	dsi_uninit_output(dsi);
>  err_pm_disable:
> 

Thanks, looks good to me. I'll push to drm-misc-fixes.

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel