On 12/18/24 04:16, Florian Fainelli wrote:
On 12/11/24 18:50, Joe Hattori wrote:
Current code leaves the device's wakeup enabled in .remove() and in the
error path of .probe(), which results in a memory leak. Therefore, add
the device_init_wakeup(dev, false) call.
This bug was found by an experimental static analysis tool that I am
developing.
Fixes: 2cd98b22c144 ("rtc: brcmstb-waketimer: non-functional code
changes")
Signed-off-by: Joe Hattori <joe@xxxxxxxxxxxxxxxxxxxxx>
Looks like you forgot to copy the maintainer (Doug Berger), added him.
Thanks!
---
drivers/rtc/rtc-brcmstb-waketimer.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/rtc/rtc-brcmstb-waketimer.c
b/drivers/rtc/rtc-brcmstb-waketimer.c
index fb47c32ab5ff..1bdfec94c693 100644
--- a/drivers/rtc/rtc-brcmstb-waketimer.c
+++ b/drivers/rtc/rtc-brcmstb-waketimer.c
@@ -298,15 +298,17 @@ static int brcmstb_waketmr_probe(struct
platform_device *pdev)
device_init_wakeup(dev, true);
ret = platform_get_irq(pdev, 0);
- if (ret < 0)
- return -ENODEV;
+ if (ret < 0) {
+ ret = -ENODEV;
+ goto err_disable_wakeup;
+ }
timer->wake_irq = (unsigned int)ret;
timer->clk = devm_clk_get(dev, NULL);
if (!IS_ERR(timer->clk)) {
ret = clk_prepare_enable(timer->clk);
if (ret)
- return ret;
+ goto err_disable_wakeup;
timer->rate = clk_get_rate(timer->clk);
if (!timer->rate)
timer->rate = BRCMSTB_WKTMR_DEFAULT_FREQ;
@@ -351,6 +353,8 @@ static int brcmstb_waketmr_probe(struct
platform_device *pdev)
err_clk:
clk_disable_unprepare(timer->clk);
+err_disable_wakeup:
+ device_init_wakeup(dev, false);
return ret;
The error path change looks good to me,
}
@@ -360,6 +364,7 @@ static void brcmstb_waketmr_remove(struct
platform_device *pdev)
unregister_reboot_notifier(&timer->reboot_notifier);
clk_disable_unprepare(timer->clk);
+ device_init_wakeup(&pdev->dev, false);
That part, I don't think is necessary at all, since the interrupts
handlers have been freed and disabled, they will not be causing any
system wake-up, besides that, the device is completely gone from /sys.
Thank you for the review. I am currently working on adding the devm_
version of the device_init_wakeup() [1], so let me come back to this
patch after the function is added.
[1]:
https://lore.kernel.org/linux-pm/20241214021652.3432500-1-joe@xxxxxxxxxxxxxxxxxxxxx/
Best,
Joe
