Message ID | 20230920115935.144391-1-biju.das.jz@bp.renesas.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:172:b0:3f2:4152:657d with SMTP id h50csp4088616vqi; Wed, 20 Sep 2023 05:12:22 -0700 (PDT) X-Google-Smtp-Source: AGHT+IE0fDn71rDCWalulAMXWEHXSk5Zr7xEbVrxqT5rpN5zqYHDc4Tdn8VS0phB6P5fx1iH6mmp X-Received: by 2002:a17:90b:1d0a:b0:26f:d6f4:9646 with SMTP id on10-20020a17090b1d0a00b0026fd6f49646mr2168574pjb.40.1695211941652; Wed, 20 Sep 2023 05:12:21 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1695211941; cv=none; d=google.com; s=arc-20160816; b=OKuZ5EgFGoowmTeCOGKWc5aXwm1mqMS86dK4Ea4FhiV/hrUahKXes02uKKWZxlz5Ns dZmeh6r1YlNUliCba8xULSv+m995kmXeoT2KS1mAr1pOr4b6elSttda24HyeW+rRIOAc 1uUq+vSKaFqn7rkmPnF7i9RUHqJ0THbtJ5M9Jz1N7db6SaZaaFFVgueCIMdPhJR2krx4 fEYDhEX+z+RFkCZQb9lWlrh8eUi/hyZhSHqXbLzNqZvyHxWSXsCqFDx1u3N7Nsby8v75 EjgZR/TTaR/ViEAzAe0JjGHYwox2vWLKGAkWTkRtFNCsWzQxqX7XskKrDET2709DK5xl 3A6Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from; bh=LlWgTvcAEAEwZVAyBoShnP0taLAAG7WsZ650UmqEUOo=; fh=WMUUeMYmv79Rn6bPL1/AfxUtlZSy2VxFGa1kqVVs5bw=; b=oUU8DQtVg+kQK8i5zu81rKteejHsJpdqelCFzJ2HPs8yC4g+IsvLbRYUAF8dMyT0i0 krii0T6+XO/fZSymhDH2w25KlUvRWfHmI4HRwTD4EowA6+Rqp/bc4k0XxySsQp71VTBh 5GQa3zlKeTgKHYNBzK5qxkrSdRNJ2VzddHNWADU1Bo7GPk8iYAWn2dTXy+YJBxKsMQm7 xFaKmmSCmw2d3+6kKCB/LJUwJYoMbqYCl1l5qqQvgKx96v+hpX9lPlNnC3cUcs95zPnb +RuZmn7Ej9wGWgaDb/sBjJbaiiV0InhtHc3a/gsqgkFfzuGwReFmBQjiTybhTbhXhH5g 4m8Q== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=renesas.com Received: from morse.vger.email (morse.vger.email. [2620:137:e000::3:1]) by mx.google.com with ESMTPS id lr18-20020a17090b4b9200b00271a9dda68csi1582196pjb.131.2023.09.20.05.12.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Sep 2023 05:12:21 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=renesas.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by morse.vger.email (Postfix) with ESMTP id 0B7968313D1E; Wed, 20 Sep 2023 05:00:16 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at morse.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235018AbjITMAI (ORCPT <rfc822;toshivichauhan@gmail.com> + 26 others); Wed, 20 Sep 2023 08:00:08 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39788 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235036AbjITL7s (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 20 Sep 2023 07:59:48 -0400 Received: from relmlie5.idc.renesas.com (relmlor1.renesas.com [210.160.252.171]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 303BDA3; Wed, 20 Sep 2023 04:59:42 -0700 (PDT) X-IronPort-AV: E=Sophos;i="6.02,161,1688396400"; d="scan'208";a="176655460" Received: from unknown (HELO relmlir6.idc.renesas.com) ([10.200.68.152]) by relmlie5.idc.renesas.com with ESMTP; 20 Sep 2023 20:59:41 +0900 Received: from localhost.localdomain (unknown [10.226.93.39]) by relmlir6.idc.renesas.com (Postfix) with ESMTP id 19F804328CA5; Wed, 20 Sep 2023 20:59:37 +0900 (JST) From: Biju Das <biju.das.jz@bp.renesas.com> To: Thomas Gleixner <tglx@linutronix.de>, Alessandro Zummo <a.zummo@towertech.it>, Alexandre Belloni <alexandre.belloni@bootlin.com> Cc: Biju Das <biju.das.jz@bp.renesas.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-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org Subject: [PATCH] alarmtimer: Fix rebind failure Date: Wed, 20 Sep 2023 12:59:35 +0100 Message-Id: <20230920115935.144391-1-biju.das.jz@bp.renesas.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-0.8 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Wed, 20 Sep 2023 05:00:16 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1777558557171083418 X-GMAIL-MSGID: 1777558557171083418 |
Series |
alarmtimer: Fix rebind failure
|
|
Commit Message
Biju Das
Sept. 20, 2023, 11:59 a.m. UTC
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(+)
Comments
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
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
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 >
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 > > > > -- >
> 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 > > > > > > > -- > >
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
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)