| Message ID | 1267965354-16805-1-git-send-email-nsekhar@ti.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi Dave, On Sun, Mar 07, 2010 at 18:05:54, Nori, Sekhar wrote: > The rtc-omap driver currently hardcodes the RTC wakeup capability > to be "not capable". While this seems to be true for existing OMAP1 > boards which are not wired for this, the DA850/OMAP-L138 SoC, the > RTC can always be wake up source from its "deep sleep" mode. > > This patch lets the wakeup capability to be set from platform data and > does not override the setting from the driver. For DA850/OMAP-L138, this > is done from arch/arm/mach-davinci/devices-da8xx.c:da8xx_register_rtc() > > Note that this patch does not change the behavior on any existing OMAP1 > board since the platform device registration sets the wakeup capability > to 0 by default. > > Signed-off-by: Sekhar Nori <nsekhar@ti.com> > Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com> > --- > > Dave, would you please ack this patch if you are satisfied? Can you please ack this patch if it looks good (or suggest any other changes you would like to see)? Thanks, Sekhar > > drivers/rtc/rtc-omap.c | 12 +++++++----- > 1 files changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c > index 64d9727..73377b0 100644 > --- a/drivers/rtc/rtc-omap.c > +++ b/drivers/rtc/rtc-omap.c > @@ -34,7 +34,8 @@ > * Board-specific wiring options include using split power mode with > * RTC_OFF_NOFF used as the reset signal (so the RTC won't be reset), > * and wiring RTC_WAKE_INT (so the RTC alarm can wake the system from > - * low power modes). See the BOARD-SPECIFIC CUSTOMIZATION comment. > + * low power modes) for OMAP1 boards (OMAP-L138 has this built into > + * the SoC). See the BOARD-SPECIFIC CUSTOMIZATION comment. > */ > > #define OMAP_RTC_BASE 0xfffb4800 > @@ -401,16 +402,17 @@ static int __init omap_rtc_probe(struct platform_device *pdev) > > /* BOARD-SPECIFIC CUSTOMIZATION CAN GO HERE: > * > - * - Boards wired so that RTC_WAKE_INT does something, and muxed > - * right (W13_1610_RTC_WAKE_INT is the default after chip reset), > - * should initialize the device wakeup flag appropriately. > + * - Device wake-up capability setting should come through chip > + * init logic. OMAP1 boards should initialize the "wakeup capable" > + * flag in the platform device if the board is wired right for > + * being woken up by RTC alarm. For OMAP-L138, this capability > + * is built into the SoC by the "Deep Sleep" capability. > * > * - Boards wired so RTC_ON_nOFF is used as the reset signal, > * rather than nPWRON_RESET, should forcibly enable split > * power mode. (Some chip errata report that RTC_CTRL_SPLIT > * is write-only, and always reads as zero...) > */ > - device_init_wakeup(&pdev->dev, 0); > > if (new_ctrl & (u8) OMAP_RTC_CTRL_SPLIT) > pr_info("%s: split power mode\n", pdev->name); > -- > 1.6.2.4 > >
> On Sun, Mar 07, 2010 at 18:05:54, Nori, Sekhar wrote: > > The rtc-omap driver currently hardcodes the RTC wakeup > capability > > to be "not capable". While this seems to be true for > existing OMAP1 > > boards which are not wired for this, the > DA850/OMAP-L138 SoC, the > > RTC can always be wake up source from its "deep sleep" > mode. Good. Linux has work to do yet getting wake events to behave, and I keep thinking that RTC support is the best place for such work to start ... (since most RTCs seem to issue wake events, even on ACPI/PC hardware). So having RTCs do this right is forward motion, and will help (I'd hope!) get other wake event sources working too. That driver, as you know, started with OMAP1 where few boards were actually wired so the RTC would wake. Which is why it ignored the wake events ... :( > > > > This patch lets the wakeup capability to be set from > platform data and > > does not override the setting from the driver. For > DA850/OMAP-L138, this > > is done from > arch/arm/mach-davinci/devices-da8xx.c:da8xx_register_rtc() > > > > Note that this patch does not change the behavior on > any existing OMAP1 > > board since the platform device registration sets the > wakeup capability > > to 0 by default. > > Dave, would you please ack this patch if you are > satisfied? I don't see how it could work, since it always sets the wake capability to the default of "can't wake"... Even if the board *can* wake... The model I've worked with is that for devices which can issue wake events, the board init code sets that flag, and the driver responds appropriately. Presumably that could work here too... > Can you please ack this patch if it looks good (or suggest > any other changes you would like to see)? > > device_init_wakeup(&pdev->dev, 0); Having that line seems to defeat the purpose...
Hi Dave, On Thu, May 20, 2010 at 22:21:59, David Brownell wrote: > > > > This patch lets the wakeup capability to be set from > > platform data and > > > does not override the setting from the driver. For > > DA850/OMAP-L138, this > > > is done from > > arch/arm/mach-davinci/devices-da8xx.c:da8xx_register_rtc() > > > > > > Note that this patch does not change the behavior on > > any existing OMAP1 > > > board since the platform device registration sets the > > wakeup capability > > > to 0 by default. Just to be clear, I was actually referring to the fact that the device_initialize() in drivers/base/core.c is already clearing the 'can wake-up' flag to 0, so there should be no need to do so again in the driver. > > > Dave, would you please ack this patch if you are > > satisfied? > > I don't see how it could work, since it always sets the > wake capability to the default of "can't wake"... Even if > the board *can* wake... Are you referring to da8xx_register_rtc() here? It sets the 'can wake' flag to true. ret = platform_device_register(&da8xx_rtc_device); if (!ret) /* Atleast on DA850, RTC is a wakeup source */ device_init_wakeup(&da8xx_rtc_device.dev, true); > > The model I've worked with is that for devices which can issue > wake events, the board init code sets that flag, and the driver > responds appropriately. Presumably that could work here too... Yes, except that the response to the wakeup event is really specific to DaVinci/DA850 and is handled in arch/arm/mach-davinci/sleep.S, not in the RTC driver. > > > > Can you please ack this patch if it looks good (or suggest > > any other changes you would like to see)? > > > > device_init_wakeup(&pdev->dev, 0); > > Having that line seems to defeat the purpose... Agreed, and the patch removes it for that reason. Thanks, Sekhar
Hi Dave, Any thoughts on my responses below? If you are satisfied, would you please Ack my patch? Thanks, Sekhar On Mon, May 24, 2010 at 11:36:45, Nori, Sekhar wrote: > Hi Dave, > > On Thu, May 20, 2010 at 22:21:59, David Brownell wrote: > > > > > > This patch lets the wakeup capability to be set from > > > platform data and > > > > does not override the setting from the driver. For > > > DA850/OMAP-L138, this > > > > is done from > > > arch/arm/mach-davinci/devices-da8xx.c:da8xx_register_rtc() > > > > > > > > Note that this patch does not change the behavior on > > > any existing OMAP1 > > > > board since the platform device registration sets the > > > wakeup capability > > > > to 0 by default. > > Just to be clear, I was actually referring to the fact > that the device_initialize() in drivers/base/core.c is > already clearing the 'can wake-up' flag to 0, so there > should be no need to do so again in the driver. > > > > > Dave, would you please ack this patch if you are > > > satisfied? > > > > I don't see how it could work, since it always sets the > > wake capability to the default of "can't wake"... Even if > > the board *can* wake... > > Are you referring to da8xx_register_rtc() here? It sets the > 'can wake' flag to true. > > ret = platform_device_register(&da8xx_rtc_device); > if (!ret) > /* Atleast on DA850, RTC is a wakeup source */ > device_init_wakeup(&da8xx_rtc_device.dev, true); > > > > > The model I've worked with is that for devices which can issue > > wake events, the board init code sets that flag, and the driver > > responds appropriately. Presumably that could work here too... > > Yes, except that the response to the wakeup event is really specific > to DaVinci/DA850 and is handled in arch/arm/mach-davinci/sleep.S, not > in the RTC driver. > > > > > > > > Can you please ack this patch if it looks good (or suggest > > > any other changes you would like to see)? > > > > > > device_init_wakeup(&pdev->dev, 0); > > > > Having that line seems to defeat the purpose... > > Agreed, and the patch removes it for that reason. > > Thanks, > Sekhar > _______________________________________________ > Davinci-linux-open-source mailing list > Davinci-linux-open-source@linux.davincidsp.com > http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source >
--- On Wed, 6/16/10, Nori, Sekhar <nsekhar@ti.com> wrote: <rtc-linux@googlegroups.com> > Date: Wednesday, June 16, 2010, 9:46 PM > Hi Dave, > > Any thoughts on my responses below? If you are > satisfied, would you please Ack my patch? I don't have time to properly review it. If it works, go for it ... bugs can be fixed later, and the principle behind the patch is fine.
Hi Alessandro, On Mon, Jul 19, 2010 at 02:22:13, David Brownell wrote: > > > --- On Wed, 6/16/10, Nori, Sekhar <nsekhar@ti.com> wrote: > <rtc-linux@googlegroups.com> > > Date: Wednesday, June 16, 2010, 9:46 PM > > Hi Dave, > > > > Any thoughts on my responses below? If you are > > satisfied, would you please Ack my patch? > > I don't have time to properly review it. If it > works, go for it ... bugs can be fixed later, and > the principle behind the patch is fine. > Since Dave is OK with merging this, would you please queue this for 2.6.36? Thanks, Sekhar
2010/7/22 Nori, Sekhar <nsekhar@ti.com>: > Hi Alessandro, > > On Mon, Jul 19, 2010 at 02:22:13, David Brownell wrote: >> >> >> --- On Wed, 6/16/10, Nori, Sekhar <nsekhar@ti.com> wrote: >> <rtc-linux@googlegroups.com> >> > Date: Wednesday, June 16, 2010, 9:46 PM >> > Hi Dave, >> > >> > Any thoughts on my responses below? If you are >> > satisfied, would you please Ack my patch? >> >> I don't have time to properly review it. If it >> works, go for it ... bugs can be fixed later, and >> the principle behind the patch is fine. >> > > Since Dave is OK with merging this, would you please > queue this for 2.6.36? Alessandro is so busy too, you can get merging help from Andrew. > > Thanks, > Sekhar > > -- > You received this message because you are subscribed to "rtc-linux". > Membership options at http://groups.google.com/group/rtc-linux . > Please read http://groups.google.com/group/rtc-linux/web/checklist > before submitting a driver.
Hi Andrew, On Fri, Jul 23, 2010 at 15:16:19, Wan ZongShun wrote: > 2010/7/22 Nori, Sekhar <nsekhar@ti.com>: > > Hi Alessandro, > > > > On Mon, Jul 19, 2010 at 02:22:13, David Brownell wrote: > >> > >> > >> --- On Wed, 6/16/10, Nori, Sekhar <nsekhar@ti.com> wrote: > >> <rtc-linux@googlegroups.com> > >> > Date: Wednesday, June 16, 2010, 9:46 PM > >> > Hi Dave, > >> > > >> > Any thoughts on my responses below? If you are > >> > satisfied, would you please Ack my patch? > >> > >> I don't have time to properly review it. If it > >> works, go for it ... bugs can be fixed later, and > >> the principle behind the patch is fine. > >> > > > > Since Dave is OK with merging this, would you please > > queue this for 2.6.36? > > Alessandro is so busy too, you can get merging help from Andrew. Can you please help merge this patch? Here is the patch in question: http://patchwork.ozlabs.org/patch/47089/ I can resend it to the list if that’s more convenient. Thanks, Sekhar
diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c index 64d9727..73377b0 100644 --- a/drivers/rtc/rtc-omap.c +++ b/drivers/rtc/rtc-omap.c @@ -34,7 +34,8 @@ * Board-specific wiring options include using split power mode with * RTC_OFF_NOFF used as the reset signal (so the RTC won't be reset), * and wiring RTC_WAKE_INT (so the RTC alarm can wake the system from - * low power modes). See the BOARD-SPECIFIC CUSTOMIZATION comment. + * low power modes) for OMAP1 boards (OMAP-L138 has this built into + * the SoC). See the BOARD-SPECIFIC CUSTOMIZATION comment. */ #define OMAP_RTC_BASE 0xfffb4800 @@ -401,16 +402,17 @@ static int __init omap_rtc_probe(struct platform_device *pdev) /* BOARD-SPECIFIC CUSTOMIZATION CAN GO HERE: * - * - Boards wired so that RTC_WAKE_INT does something, and muxed - * right (W13_1610_RTC_WAKE_INT is the default after chip reset), - * should initialize the device wakeup flag appropriately. + * - Device wake-up capability setting should come through chip + * init logic. OMAP1 boards should initialize the "wakeup capable" + * flag in the platform device if the board is wired right for + * being woken up by RTC alarm. For OMAP-L138, this capability + * is built into the SoC by the "Deep Sleep" capability. * * - Boards wired so RTC_ON_nOFF is used as the reset signal, * rather than nPWRON_RESET, should forcibly enable split * power mode. (Some chip errata report that RTC_CTRL_SPLIT * is write-only, and always reads as zero...) */ - device_init_wakeup(&pdev->dev, 0); if (new_ctrl & (u8) OMAP_RTC_CTRL_SPLIT) pr_info("%s: split power mode\n", pdev->name);