Hi Matti,
Thank you for your review.
On 12/12/24 18:20, Matti Vaittinen wrote:
Hi Joe,
Thanks for the patch :)
On 12/12/2024 03:51, Joe Hattori wrote:
Current code leaves the device's wakeup enabled in the error path of
.probe(), which results in a memory leak. Therefore, add the
device_init_wakeup(&device->dev, false) calls before returning an error.
This bug was found by an experimental static analysis tool that I am
developing.
Fixes: 32a4a4ebf768 ("rtc: bd70528: Initial support for ROHM bd70528
RTC")
Signed-off-by: Joe Hattori <joe@xxxxxxxxxxxxxxxxxxxxx>
---
drivers/rtc/rtc-bd70528.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/rtc/rtc-bd70528.c b/drivers/rtc/rtc-bd70528.c
index 954ac4ef53e8..50059f7ba6d0 100644
--- a/drivers/rtc/rtc-bd70528.c
+++ b/drivers/rtc/rtc-bd70528.c
@@ -312,12 +312,12 @@ static int bd70528_probe(struct platform_device
*pdev)
}
}
- device_set_wakeup_capable(&pdev->dev, true);
- device_wakeup_enable(&pdev->dev);
+ device_init_wakeup(&pdev->dev, true);
Is there a reason to do this prior rtc-device allocation/registration?
If not, then the flow would be cleaner if this was done as the last step
in probe() because we could omit the clean-up.
Enabling the device's wakeup in the last step makes much more sense.
Applied to the V2 patch.
rtc = devm_rtc_allocate_device(&pdev->dev);
if (IS_ERR(rtc)) {
dev_err(&pdev->dev, "RTC device creation failed\n");
+ device_init_wakeup(&pdev->dev, false);
return PTR_ERR(rtc);
}
@@ -328,10 +328,15 @@ static int bd70528_probe(struct platform_device
*pdev)
/* Request alarm IRQ prior to registerig the RTC */
ret = devm_request_threaded_irq(&pdev->dev, irq, NULL, &alm_hndlr,
IRQF_ONESHOT, "bd70528-rtc", rtc);
- if (ret)
+ if (ret) {
+ device_init_wakeup(&pdev->dev, false);
return ret;
+ }
- return devm_rtc_register_device(rtc);
+ ret = devm_rtc_register_device(rtc);
+ if (ret)
+ device_init_wakeup(&pdev->dev, false);
+ return ret;
}
static const struct platform_device_id bd718x7_rtc_id[] = {
Yours,
-- Matti
Best,
Joe
