Re: [PATCH v3 1/2] drm/etnaviv: Check for platform_device_register_simple() failure

[Philipp Zabel <p.zabel@pengutronix.de>]

On Mon, 2018-06-25 at 16:16 -0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@nxp.com>
> 
> platform_device_register_simple() may fail, so we should better
> check its return value and propagate it in the case of error.
> 
> Cc: <stable@vger.kernel.org>
> Fixes: 246774d17fc0 ("drm/etnaviv: remove the need for a gpu-subsystem DT node")
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
> Changes since v2:
> - Newly introduced in this version
> 
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index e5013a9..ac23c0f 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -631,6 +631,8 @@ static struct platform_driver etnaviv_platform_driver = {
>  	},
>  };
>  
> +static struct platform_device *etnaviv_drm;
> +
>  static int __init etnaviv_init(void)
>  {
>  	int ret;
> @@ -644,7 +646,7 @@ static int __init etnaviv_init(void)
>  
>  	ret = platform_driver_register(&etnaviv_platform_driver);
>  	if (ret != 0)
> -		platform_driver_unregister(&etnaviv_gpu_driver);
> +		goto unregister_gpu_driver;
>  
>  	/*
>  	 * If the DT contains at least one available GPU device, instantiate
> @@ -653,12 +655,22 @@ static int __init etnaviv_init(void)
>  	for_each_compatible_node(np, NULL, "vivante,gc") {
>  		if (!of_device_is_available(np))
>  			continue;
> -
> -		platform_device_register_simple("etnaviv", -1, NULL, 0);
> +		etnaviv_drm = platform_device_register_simple("etnaviv", -1,
> +							      NULL, 0);

Rather than store the error value in the global variable, I'd use a
local variable here and only set etnaviv_drm if this doesn't fail.

> +		if (IS_ERR(etnaviv_drm)) {
> +			ret = PTR_ERR(etnaviv_drm);
> +			goto unregister_platform_driver;
> +		}
>  		of_node_put(np);
>  		break;

Ah, focusing on etnaviv_exit last time I've completely overlooked this
break.

>  	}
>  
> +	return 0;
> +
> +unregister_platform_driver:
> +	platform_driver_unregister(&etnaviv_platform_driver);
> +unregister_gpu_driver:
> +	platform_driver_unregister(&etnaviv_gpu_driver);
>  	return ret;
>  }
>  module_init(etnaviv_init);
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel