On 13/07/26 09:59, Ritesh Harjani (IBM) wrote:

Sourabh Jain [off-list ref] writes:

quoted

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.

quoted

Signed-off-by: Sourabh Jain <redacted>
---
  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>

Thanks for the review. I will rename the variable in next version.

- Sourabh Jain

quoted

+
+	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