Re: [PATCH v3 1/3] EDAC: mv64x60: check driver registration success

[Chris Packham]

Hi Borislav,

On 30/05/17 09:21, Chris Packham wrote:
> Check the return status of platform_driver_register() in
> mv64x60_edac_init(). Only output messages and initialise the
> edac_op_state if the registration is successful.
> 
> Signed-off-by: Chris Packham <chris.packham@xxxxxxxxxxxxxxxxxxx>
> ---
> Changes in v3:
> - catch the retval of platform_register_drivers and return early on failure
>    (thanks Borislav).
> 
>   drivers/edac/mv64x60_edac.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/edac/mv64x60_edac.c b/drivers/edac/mv64x60_edac.c
> index 14b7e7b71eaa..172081551a70 100644
> --- a/drivers/edac/mv64x60_edac.c
> +++ b/drivers/edac/mv64x60_edac.c
> @@ -853,7 +853,11 @@ static struct platform_driver * const drivers[] = {
>   
>   static int __init mv64x60_edac_init(void)
>   {
> -	int ret = 0;
> +	int ret;
> +
> +	ret = platform_register_drivers(drivers, ARRAY_SIZE(drivers));
> +	if (ret)
> +		return ret;

I actually think this was wrong. The edac_op_state is set as a module 
param and affects the behaviour of the driver probe function which on 
success could be called before we sanity check it below.

I'm back to thinking that my original v1 of just removing the unused ret 
was appropriate. If we still consider the driver too noisy we could just 
remove the printks.

What do you think?

>   
>   	printk(KERN_INFO "Marvell MV64x60 EDAC driver " MV64x60_REVISION "\n");
>   	printk(KERN_INFO "\t(C) 2006-2007 MontaVista Software\n");
> @@ -867,7 +871,7 @@ static int __init mv64x60_edac_init(void)
>   		break;
>   	}
>   
> -	return platform_register_drivers(drivers, ARRAY_SIZE(drivers));
> +	return 0;
>   }
>   module_init(mv64x60_edac_init);
>   
>