From: Biju Das <biju.das.jz@bp.renesas.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
Alessandro Zummo <a.zummo@towertech.it>,
Alexandre Belloni <alexandre.belloni@bootlin.com>,
John Stultz <jstultz@google.com>, Stephen Boyd <sboyd@kernel.org>,
Geert Uytterhoeven <geert+renesas@glider.be>,
Biju Das <biju.das.au@gmail.com>,
"linux-rtc@vger.kernel.org" <linux-rtc@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-renesas-soc@vger.kernel.org"
<linux-renesas-soc@vger.kernel.org>
Subject: RE: [PATCH] alarmtimer: Fix rebind failure
Date: Fri, 22 Sep 2023 07:14:53 +0000 [thread overview]
Message-ID: <OS0PR01MB592211963202423E46CC2B4D86FFA@OS0PR01MB5922.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <OS0PR01MB59221716AD7E6A10CDB295D986F9A@OS0PR01MB5922.jpnprd01.prod.outlook.com>
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
next prev parent reply other threads:[~2023-09-22 7:15 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2023-09-20 13:26 ` Alexandre Belloni
2023-09-20 13:31 ` Biju Das
2023-09-20 13:33 ` Biju Das
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=OS0PR01MB592211963202423E46CC2B4D86FFA@OS0PR01MB5922.jpnprd01.prod.outlook.com \
--to=biju.das.jz@bp.renesas.com \
--cc=a.zummo@towertech.it \
--cc=alexandre.belloni@bootlin.com \
--cc=biju.das.au@gmail.com \
--cc=geert+renesas@glider.be \
--cc=geert@linux-m68k.org \
--cc=jstultz@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=linux-rtc@vger.kernel.org \
--cc=sboyd@kernel.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).