Re: [PATCH v4 05/15] drivers/firmware/sdei: Unregister driver on error in sdei_init()
From: James Morse <james.morse@arm.com>
Date: 2020-09-18 16:14:17
Hi Gavin, On 30/07/2020 02:45, Gavin Shan wrote:
The platform driver needs to be unregistered on error to create the platform device, so that the state is restored to original one.
Why is this important? Is it just symmetry? 'needs' causes me to look at this as a bug-fix.
DT systems leave a dangling platform device in this case. Is that a problem?
See commit 3aa0582fdb82 ("of: platform: populate /firmware/ node from
of_platform_default_populate_init()").
On DT systems, sdei_init() doesn't create the platform device, so it doesn't remove it
either. ACPI leaves it dangling because ... why should ACPI behave differently?
Besides, the errno (@ret) should be updated acccording in that case.
(according)
quoted hunk ↗ jump to hunk
diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c index c9f2bedfb6b5..909f27abf8a7 100644 --- a/drivers/firmware/arm_sdei.c +++ b/drivers/firmware/arm_sdei.c@@ -1090,9 +1090,12 @@ static int __init sdei_init(void) pdev = platform_device_register_simple(sdei_driver.driver.name, 0, NULL, 0); - if (IS_ERR(pdev)) - pr_info("Failed to register ACPI:SDEI platform device %ld\n", - PTR_ERR(pdev)); + if (IS_ERR(pdev)) { + ret = PTR_ERR(pdev); + platform_driver_unregister(&sdei_driver); + pr_info("Failed to register ACPI:SDEI platform device %d\n", + ret); + } return ret; }
If the argument here is about symmetry, sure. Please stuff sneak the word symmetry into the commit message - I keep reading this as if its a bug. Its probably worth a comment in the commit message that its only like this for ACPI. Previously the motivation was to keep these things as similar as possible. With that: Reviewed-by: James Morse <james.morse@arm.com> Thanks, James _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel