linux-rtc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] alarmtimer: Fix rebind failure
@ 2023-09-20 11:59 Biju Das
  2023-09-20 12:24 ` Geert Uytterhoeven
  2023-09-20 13:26 ` Alexandre Belloni
  0 siblings, 2 replies; 7+ messages in thread
From: Biju Das @ 2023-09-20 11:59 UTC (permalink / raw)
  To: Thomas Gleixner, Alessandro Zummo, Alexandre Belloni
  Cc: Biju Das, John Stultz, Stephen Boyd, Geert Uytterhoeven,
	Biju Das, linux-rtc, linux-kernel, linux-renesas-soc

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@bp.renesas.com>
---
This issue is found while adding irq support for built in RTC
found on Renesas PMIC RAA215300 device. This issue should present
on all RTC drivers which calls device_init_wakeup() in probe(). 
---
 kernel/time/alarmtimer.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index 8d9f13d847f0..592668136bb5 100644
--- 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) {
+		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);
+	}
+
+	rtcdev = NULL;
+	rtc_pdev = NULL;
+}
+
 static inline void alarmtimer_rtc_timer_init(void)
 {
 	rtc_timer_init(&rtctimer, NULL, NULL);
@@ -130,6 +149,7 @@ static inline void alarmtimer_rtc_timer_init(void)
 
 static struct class_interface alarmtimer_rtc_interface = {
 	.add_dev = &alarmtimer_rtc_add_device,
+	.remove_dev = &alarmtimer_rtc_remove_device,
 };
 
 static int alarmtimer_rtc_interface_setup(void)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] alarmtimer: Fix rebind failure
  2023-09-20 11:59 [PATCH] alarmtimer: Fix rebind failure Biju Das
@ 2023-09-20 12:24 ` Geert Uytterhoeven
  2023-09-20 12:47   ` Biju Das
  2023-09-20 13:26 ` Alexandre Belloni
  1 sibling, 1 reply; 7+ messages in thread
From: Geert Uytterhoeven @ 2023-09-20 12:24 UTC (permalink / raw)
  To: Biju Das
  Cc: Thomas Gleixner, Alessandro Zummo, Alexandre Belloni,
	John Stultz, Stephen Boyd, Geert Uytterhoeven, Biju Das,
	linux-rtc, linux-kernel, linux-renesas-soc

Hi Biju,

On Wed, Sep 20, 2023 at 1:59 PM Biju Das <biju.das.jz@bp.renesas.com> 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@bp.renesas.com>

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@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] alarmtimer: Fix rebind failure
  2023-09-20 12:24 ` Geert Uytterhoeven
@ 2023-09-20 12:47   ` Biju Das
  2023-09-22  7:14     ` Biju Das
  0 siblings, 1 reply; 7+ messages in thread
From: Biju Das @ 2023-09-20 12:47 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Thomas Gleixner, Alessandro Zummo, Alexandre Belloni,
	John Stultz, Stephen Boyd, Geert Uytterhoeven, Biju Das,
	linux-rtc, linux-kernel, linux-renesas-soc

Hi Geert Uytterhoeven,

Thanks for the feedback.

> Subject: Re: [PATCH] alarmtimer: Fix rebind failure
> 
> Hi Biju,
> 
> On Wed, Sep 20, 2023 at 1:59 PM Biju Das <biju.das.jz@bp.renesas.com>
> 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@bp.renesas.com>
> 
> Thanks for your patch!
> 
> Does this need a Fixes tag?

I think so, as it breaks unbind/bind on lot of RTC drivers.

There are 2 commits, I will add both as fixes tag.

c79108bd19a8 ("alarmtimer: Make alarmtimer platform device child of RTC device")

7c94caca877b ("alarmtimer: Use wakeup source from alarmtimer platform device"

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

Ok will add lock here and the check.
> 
> > +               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()?

Platform device is child of rtc device. So it has to be
at the last as already there is put_device() call in 
devm_rtc_release_device()

Cheers,
Biju


> 
> > +       }
> > +
> > +       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@linux-
> m68k.org
> 
> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] alarmtimer: Fix rebind failure
  2023-09-20 11:59 [PATCH] alarmtimer: Fix rebind failure Biju Das
  2023-09-20 12:24 ` Geert Uytterhoeven
@ 2023-09-20 13:26 ` Alexandre Belloni
  2023-09-20 13:31   ` Biju Das
  1 sibling, 1 reply; 7+ messages in thread
From: Alexandre Belloni @ 2023-09-20 13:26 UTC (permalink / raw)
  To: Biju Das
  Cc: Thomas Gleixner, Alessandro Zummo, John Stultz, Stephen Boyd,
	Geert Uytterhoeven, Biju Das, linux-rtc, linux-kernel,
	linux-renesas-soc

On 20/09/2023 12:59:35+0100, Biju Das 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@bp.renesas.com>
> ---
> This issue is found while adding irq support for built in RTC
> found on Renesas PMIC RAA215300 device. This issue should present
> on all RTC drivers which calls device_init_wakeup() in probe(). 
> ---
>  kernel/time/alarmtimer.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
> index 8d9f13d847f0..592668136bb5 100644
> --- 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;

This is the alarmtimer pdev, not the RTC one, right?

>  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) {
> +		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);
> +	}
> +
> +	rtcdev = NULL;
> +	rtc_pdev = NULL;
> +}
> +
>  static inline void alarmtimer_rtc_timer_init(void)
>  {
>  	rtc_timer_init(&rtctimer, NULL, NULL);
> @@ -130,6 +149,7 @@ static inline void alarmtimer_rtc_timer_init(void)
>  
>  static struct class_interface alarmtimer_rtc_interface = {
>  	.add_dev = &alarmtimer_rtc_add_device,
> +	.remove_dev = &alarmtimer_rtc_remove_device,
>  };
>  
>  static int alarmtimer_rtc_interface_setup(void)
> -- 
> 2.25.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] alarmtimer: Fix rebind failure
  2023-09-20 13:26 ` Alexandre Belloni
@ 2023-09-20 13:31   ` Biju Das
  2023-09-20 13:33     ` Biju Das
  0 siblings, 1 reply; 7+ messages in thread
From: Biju Das @ 2023-09-20 13:31 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Thomas Gleixner, Alessandro Zummo, John Stultz, Stephen Boyd,
	Geert Uytterhoeven, Biju Das, linux-rtc, linux-kernel,
	linux-renesas-soc

Hi Alexandre Belloni,

> Subject: Re: [PATCH] alarmtimer: Fix rebind failure
> 
> On 20/09/2023 12:59:35+0100, Biju Das 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@bp.renesas.com>
> > ---
> > This issue is found while adding irq support for built in RTC found on
> > Renesas PMIC RAA215300 device. This issue should present on all RTC
> > drivers which calls device_init_wakeup() in probe().
> > ---
> >  kernel/time/alarmtimer.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c index
> > 8d9f13d847f0..592668136bb5 100644
> > --- 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;
> 
> This is the alarmtimer pdev, not the RTC one, right?

Yes, it is alarmtimer pdev.

Cheers,
Biju

> 
> >  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) {
> > +		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);
> > +	}
> > +
> > +	rtcdev = NULL;
> > +	rtc_pdev = NULL;
> > +}
> > +
> >  static inline void alarmtimer_rtc_timer_init(void)  {
> >  	rtc_timer_init(&rtctimer, NULL, NULL); @@ -130,6 +149,7 @@ static
> > inline void alarmtimer_rtc_timer_init(void)
> >
> >  static struct class_interface alarmtimer_rtc_interface = {
> >  	.add_dev = &alarmtimer_rtc_add_device,
> > +	.remove_dev = &alarmtimer_rtc_remove_device,
> >  };
> >
> >  static int alarmtimer_rtc_interface_setup(void)
> > --
> > 2.25.1
> >
> 
> --
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] alarmtimer: Fix rebind failure
  2023-09-20 13:31   ` Biju Das
@ 2023-09-20 13:33     ` Biju Das
  0 siblings, 0 replies; 7+ messages in thread
From: Biju Das @ 2023-09-20 13:33 UTC (permalink / raw)
  To: Biju Das, Alexandre Belloni
  Cc: Thomas Gleixner, Alessandro Zummo, John Stultz, Stephen Boyd,
	Geert Uytterhoeven, Biju Das, linux-rtc, linux-kernel,
	linux-renesas-soc

> Subject: RE: [PATCH] alarmtimer: Fix rebind failure
> 
> Hi Alexandre Belloni,
> 
> > Subject: Re: [PATCH] alarmtimer: Fix rebind failure
> >
> > On 20/09/2023 12:59:35+0100, Biju Das 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@bp.renesas.com>
> > > ---
> > > This issue is found while adding irq support for built in RTC found
> > > on Renesas PMIC RAA215300 device. This issue should present on all
> > > RTC drivers which calls device_init_wakeup() in probe().
> > > ---
> > >  kernel/time/alarmtimer.c | 20 ++++++++++++++++++++
> > >  1 file changed, 20 insertions(+)
> > >
> > > diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
> > > index
> > > 8d9f13d847f0..592668136bb5 100644
> > > --- 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;
> >
> > This is the alarmtimer pdev, not the RTC one, right?
> 
> Yes, it is alarmtimer pdev.


OK, I will change it to alarmtimer_pdev to avoid confusion.

Cheers,
Biju

> 
> >
> > >  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) {
> > > +		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);
> > > +	}
> > > +
> > > +	rtcdev = NULL;
> > > +	rtc_pdev = NULL;
> > > +}
> > > +
> > >  static inline void alarmtimer_rtc_timer_init(void)  {
> > >  	rtc_timer_init(&rtctimer, NULL, NULL); @@ -130,6 +149,7 @@ static
> > > inline void alarmtimer_rtc_timer_init(void)
> > >
> > >  static struct class_interface alarmtimer_rtc_interface = {
> > >  	.add_dev = &alarmtimer_rtc_add_device,
> > > +	.remove_dev = &alarmtimer_rtc_remove_device,
> > >  };
> > >
> > >  static int alarmtimer_rtc_interface_setup(void)
> > > --
> > > 2.25.1
> > >
> >
> > --
> >

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] alarmtimer: Fix rebind failure
  2023-09-20 12:47   ` Biju Das
@ 2023-09-22  7:14     ` Biju Das
  0 siblings, 0 replies; 7+ messages in thread
From: Biju Das @ 2023-09-22  7:14 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Thomas Gleixner, Alessandro Zummo, Alexandre Belloni,
	John Stultz, Stephen Boyd, Geert Uytterhoeven, Biju Das,
	linux-rtc, linux-kernel, linux-renesas-soc

Hi Geert Uytterhoeven,

> Subject: RE: [PATCH] alarmtimer: Fix rebind failure
> 
> Hi Geert Uytterhoeven,
> 
> Thanks for the feedback.
> 
> > Subject: Re: [PATCH] alarmtimer: Fix rebind failure
> >
> > Hi Biju,
> >
> > On Wed, Sep 20, 2023 at 1:59 PM Biju Das <biju.das.jz@bp.renesas.com>
> > 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@bp.renesas.com>
> >
> > Thanks for your patch!
> >
> > Does this need a Fixes tag?
> 
> I think so, as it breaks unbind/bind on lot of RTC drivers.
> 
> There are 2 commits, I will add both as fixes tag.
> 
> c79108bd19a8 ("alarmtimer: Make alarmtimer platform device child of RTC
> device")
> 
> 7c94caca877b ("alarmtimer: Use wakeup source from alarmtimer platform
> device"
> 
> >
> > > --- 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.
> 
> Ok will add lock here and the check.

I won't be able to add lock here as it is giving

1) BUG invalid context
2) Scheduling while atomic() as lock is held by delete function.

Cheers,
Biju

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-09-22  7:15 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-20 11:59 [PATCH] alarmtimer: Fix rebind failure Biju Das
2023-09-20 12:24 ` Geert Uytterhoeven
2023-09-20 12:47   ` Biju Das
2023-09-22  7:14     ` Biju Das
2023-09-20 13:26 ` Alexandre Belloni
2023-09-20 13:31   ` Biju Das
2023-09-20 13:33     ` Biju Das

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).