Hi Biju,
On Wed, Sep 20, 2023 at 1:59 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> The resources allocated in alarmtimer_rtc_add_device() are not freed
> leading to re-bind failure for the endpoint driver. Fix this issue
> by adding alarmtimer_rtc_remove_device().
>
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
Thanks for your patch!
Does this need a Fixes tag?
> --- a/kernel/time/alarmtimer.c
> +++ b/kernel/time/alarmtimer.c
> @@ -61,6 +61,7 @@ static DEFINE_SPINLOCK(freezer_delta_lock);
> /* rtc timer and device for setting alarm wakeups at suspend */
> static struct rtc_timer rtctimer;
> static struct rtc_device *rtcdev;
> +static struct platform_device *rtc_pdev;
> static DEFINE_SPINLOCK(rtcdev_lock);
>
> /**
> @@ -109,6 +110,7 @@ static int alarmtimer_rtc_add_device(struct device *dev)
> }
>
> rtcdev = rtc;
> + rtc_pdev = pdev;
> /* hold a reference so it doesn't go away */
> get_device(dev);
> pdev = NULL;
> @@ -123,6 +125,23 @@ static int alarmtimer_rtc_add_device(struct device *dev)
> return ret;
> }
>
> +static void alarmtimer_rtc_remove_device(struct device *dev)
> +{
> + struct rtc_device *rtc = to_rtc_device(dev);
> +
> + if (rtc_pdev) {
As the return value of class_interface.add_dev() is never checked
(alarmtimer_rtc_add_device() returns -EBUSY on adding a second
alarmtimer), multiple timers may have been added, but only one of them
will be the real alarmtimer.
Hence this function should check if rtcdev == rtc before unregistering
the real alarmtimer. Of course all of this should be protected by
rtcdev_lock.
> + module_put(rtc->owner);
> + if (device_may_wakeup(rtc->dev.parent))
> + device_init_wakeup(&rtc_pdev->dev, false);
> +
> + platform_device_unregister(rtc_pdev);
> + put_device(dev);
Perhaps use the reverse order of operations as in
alarmtimer_rtc_add_device()?
> + }
> +
> + rtcdev = NULL;
> + rtc_pdev = NULL;
> +}
> +
> static inline void alarmtimer_rtc_timer_init(void)
> {
> rtc_timer_init(&rtctimer, NULL, NULL);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
