Re: [PATCH v2 2/3] powerpc/pseries: Handle and log pseries-wdt registration failures

[Ritesh Harjani (IBM) <ritesh.list@gmail.com>]

Sourabh Jain <sourabhjain@linux.ibm.com> writes:

> The pseries watchdog initialization registers the pseries-wdt platform
> device using platform_device_register_simple(), but currently ignores
> its return value.
>
> Check the returned pointer for errors, log a descriptive error message
> when registration fails, and propagate the failure code to the caller.
> This avoids silently ignoring platform device registration failures.
>

Fair enough.

> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/setup.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
> index 1223dc961242..bbb2813f8ede 100644
> --- a/arch/powerpc/platforms/pseries/setup.c
> +++ b/arch/powerpc/platforms/pseries/setup.c
> @@ -191,8 +191,18 @@ static void __init fwnmi_init(void)
>   */
>  static __init int pseries_wdt_init(void)
>  {
> -	if (firmware_has_feature(FW_FEATURE_WATCHDOG))
> -		platform_device_register_simple("pseries-wdt", 0, NULL, 0);
> +	struct platform_device *pseries_wdt_dev;

minor nit: we should rename this to pdev, since it is already under
pseries_wdt_init(). That is generally how all platform drivers use it
unless it requires more than one platform device.

But either ways the patch looks good to me:

Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>

> +
> +	if (!firmware_has_feature(FW_FEATURE_WATCHDOG))
> +		return 0;
> +
> +	pseries_wdt_dev = platform_device_register_simple("pseries-wdt", 0, NULL, 0);
> +
> +	if (IS_ERR(pseries_wdt_dev)) {
> +		pr_err("Failed to register pseries-wdt platform device\n");
> +		return PTR_ERR(pseries_wdt_dev);
> +	}
> +
>  	return 0;
>  }
>  machine_subsys_initcall(pseries, pseries_wdt_init);
> -- 
> 2.52.0