| Message ID | 1369982005-8572-3-git-send-email-gururaja.hebbar@ti.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi Gururaja, Thanks for the patch. On Fri, May 31, 2013 at 12:03 PM, Hebbar Gururaja <gururaja.hebbar@ti.com> wrote: > On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN) [snip] > - ti,wakeup_capable: Inform the rtc driver that this module is wake-up > capable so that rtcwake and suspend tests can work. > +- ti,has_irq_wake_enb: Inform rtc driver that this module has a a same comment applies here as well. s/has a a/ has a > + special register to enable Wakeup feature for Alarm events. > > Example: > > diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c > index 000a02f..5e9c0dd 100644 > --- a/drivers/rtc/rtc-omap.c > +++ b/drivers/rtc/rtc-omap.c > @@ -72,6 +72,8 @@ > #define OMAP_RTC_KICK0_REG 0x6c > #define OMAP_RTC_KICK1_REG 0x70 > > +#define OMAP_RTC_IRQWAKEEN 0x7C > + why not have the hex values in smaller case as done in the rest of the file ? Regards, --Prabhakar Lad
Hebbar Gururaja <gururaja.hebbar@ti.com> writes: > On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN) > is available to enable Wakeup feature for Alarm Events. > > Since new platforms/Boards are now added through DT only, this feature > is supported via DT property only. > Platforms using such IP should add the property "ti,has_irq_wake_enb" > in rtc DT node. This is a property of the IP, not the board. Can't you detect this based on the revision of the IP? Kevin
On Fri, May 31, 2013 at 23:11:22, Kevin Hilman wrote: > Hebbar Gururaja <gururaja.hebbar@ti.com> writes: > > > On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN) > > is available to enable Wakeup feature for Alarm Events. > > > > Since new platforms/Boards are now added through DT only, this feature > > is supported via DT property only. > > > Platforms using such IP should add the property "ti,has_irq_wake_enb" > > in rtc DT node. > > This is a property of the IP, not the board. Correct. > Can't you detect this > based on the revision of the IP? Will check this and return. > > Kevin > Regards, Gururaja
On Fri, May 31, 2013 at 23:11:22, Kevin Hilman wrote: > Hebbar Gururaja <gururaja.hebbar@ti.com> writes: > > > On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN) > > is available to enable Wakeup feature for Alarm Events. > > > > Since new platforms/Boards are now added through DT only, this feature > > is supported via DT property only. > > > Platforms using such IP should add the property "ti,has_irq_wake_enb" > > in rtc DT node. > > This is a property of the IP, not the board. Can't you detect this > based on the revision of the IP? Here is what I found out till now. 1. rtc-omap driver is used by Davinci, OMAP-1/2 & AM33xx SoC. 2. Only AM33xx soc rtc ip has revision register (& populated). Older OMAP Or davinci doesn't have this register. 3. AFAIK, this was the reason why Afzal used platform_device_id & of_device_id->data method to detect new versions (commit 9e0344dcc225fe1a0e8b8af9ff7df44ec4613580) So now either a. I can follow the same method and add new member to omap_rtc_devtype & add a new compatible in omap_rtc_of_match in rtc-omap.c or b. use dt property to indicate existence of above property. Kindly let me know your opinion about the same. > > Kevin > Regards, Gururaja
Hi Kevin, On Mon, Jun 10, 2013 at 16:55:17, Hebbar, Gururaja wrote: > On Fri, May 31, 2013 at 23:11:22, Kevin Hilman wrote: > > Hebbar Gururaja <gururaja.hebbar@ti.com> writes: > > > > > On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN) > > > is available to enable Wakeup feature for Alarm Events. > > > > > > Since new platforms/Boards are now added through DT only, this feature > > > is supported via DT property only. > > > > > Platforms using such IP should add the property "ti,has_irq_wake_enb" > > > in rtc DT node. > > > > This is a property of the IP, not the board. Can't you detect this > > based on the revision of the IP? > > Here is what I found out till now. > > 1. rtc-omap driver is used by Davinci, OMAP-1/2 & AM33xx SoC. > > 2. Only AM33xx soc rtc ip has revision register (& populated). Older OMAP > Or davinci doesn't have this register. > > 3. AFAIK, this was the reason why Afzal used platform_device_id & > of_device_id->data method to detect new versions (commit > 9e0344dcc225fe1a0e8b8af9ff7df44ec4613580) > > > So now either > a. I can follow the same method and add new member to omap_rtc_devtype & add a new compatible in > omap_rtc_of_match in rtc-omap.c > > or > > b. use dt property to indicate existence of above property. > > > Kindly let me know your opinion about the same. Any update on this. I have patch ready for both the choices. Waiting for your ok to send > > > > > Kevin > > > > > Regards, > Gururaja > Regards, Gururaja
"Hebbar, Gururaja" <gururaja.hebbar@ti.com> writes: > Hi Kevin, > > On Mon, Jun 10, 2013 at 16:55:17, Hebbar, Gururaja wrote: >> On Fri, May 31, 2013 at 23:11:22, Kevin Hilman wrote: >> > Hebbar Gururaja <gururaja.hebbar@ti.com> writes: >> > >> > > On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN) >> > > is available to enable Wakeup feature for Alarm Events. >> > > >> > > Since new platforms/Boards are now added through DT only, this feature >> > > is supported via DT property only. >> > >> > > Platforms using such IP should add the property "ti,has_irq_wake_enb" >> > > in rtc DT node. >> > >> > This is a property of the IP, not the board. Can't you detect this >> > based on the revision of the IP? >> >> Here is what I found out till now. >> >> 1. rtc-omap driver is used by Davinci, OMAP-1/2 & AM33xx SoC. >> >> 2. Only AM33xx soc rtc ip has revision register (& populated). Older OMAP >> Or davinci doesn't have this register. >> >> 3. AFAIK, this was the reason why Afzal used platform_device_id & >> of_device_id->data method to detect new versions (commit >> 9e0344dcc225fe1a0e8b8af9ff7df44ec4613580) >> >> >> So now either >> a. I can follow the same method and add new member to omap_rtc_devtype & add a new compatible in >> omap_rtc_of_match in rtc-omap.c >> >> or >> >> b. use dt property to indicate existence of above property. >> >> >> Kindly let me know your opinion about the same. > > Any update on this. I have patch ready for both the choices. Waiting for your ok to send I think (a) is better. The driver should always do a device_init_wakeup(dev, true), *except* for devtypes that don't have this feature. Kevin
On Fri, Jun 14, 2013 at 03:51:43, Kevin Hilman wrote: > "Hebbar, Gururaja" <gururaja.hebbar@ti.com> writes: > > > Hi Kevin, > > > > On Mon, Jun 10, 2013 at 16:55:17, Hebbar, Gururaja wrote: > >> On Fri, May 31, 2013 at 23:11:22, Kevin Hilman wrote: > >> > Hebbar Gururaja <gururaja.hebbar@ti.com> writes: > >> > > >> > > On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN) > >> > > is available to enable Wakeup feature for Alarm Events. > >> > > > >> > > Since new platforms/Boards are now added through DT only, this feature > >> > > is supported via DT property only. > >> > > >> > > Platforms using such IP should add the property "ti,has_irq_wake_enb" > >> > > in rtc DT node. > >> > > >> > This is a property of the IP, not the board. Can't you detect this > >> > based on the revision of the IP? > >> > >> Here is what I found out till now. > >> > >> 1. rtc-omap driver is used by Davinci, OMAP-1/2 & AM33xx SoC. > >> > >> 2. Only AM33xx soc rtc ip has revision register (& populated). Older OMAP > >> Or davinci doesn't have this register. > >> > >> 3. AFAIK, this was the reason why Afzal used platform_device_id & > >> of_device_id->data method to detect new versions (commit > >> 9e0344dcc225fe1a0e8b8af9ff7df44ec4613580) > >> > >> > >> So now either > >> a. I can follow the same method and add new member to omap_rtc_devtype & add a new compatible in > >> omap_rtc_of_match in rtc-omap.c > >> > >> or > >> > >> b. use dt property to indicate existence of above property. > >> > >> > >> Kindly let me know your opinion about the same. > > > > Any update on this. I have patch ready for both the choices. Waiting for your ok to send > > I think (a) is better. Ok. I will add the property using a new DT compatibility. > > The driver should always do a device_init_wakeup(dev, true), *except* > for devtypes that don't have this feature. "wakeup capable" and "irq-wake-enable-for-alarm-event" are different For "irq-wake-enable-for-alarm-event", I will add a new DT compability. However, wakeup capable notification is done in another patch using the a DT property "ti,wakeup_capable". The reason for using a dt property is specified below commit fa5b07820fe3a0fc06ac368516e71f10a59b9539 Author: Sekhar Nori <nsekhar@ti.com> Date: Wed Oct 27 15:33:05 2010 -0700 rtc: omap: let device wakeup capability be configured from chip init logic 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. So I cannot directly add device_init_wakeup(dev, true), inside driver code. So, I added a DT property > > Kevin > > Regards, Gururaja
diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt index 108a629..b870d87 100644 --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt @@ -9,6 +9,8 @@ Required properties: Optional properties: - ti,wakeup_capable: Inform the rtc driver that this module is wake-up capable so that rtcwake and suspend tests can work. +- ti,has_irq_wake_enb: Inform rtc driver that this module has a a + special register to enable Wakeup feature for Alarm events. Example: diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c index 000a02f..5e9c0dd 100644 --- a/drivers/rtc/rtc-omap.c +++ b/drivers/rtc/rtc-omap.c @@ -72,6 +72,8 @@ #define OMAP_RTC_KICK0_REG 0x6c #define OMAP_RTC_KICK1_REG 0x70 +#define OMAP_RTC_IRQWAKEEN 0x7C + /* OMAP_RTC_CTRL_REG bit fields: */ #define OMAP_RTC_CTRL_SPLIT (1<<7) #define OMAP_RTC_CTRL_DISABLE (1<<6) @@ -96,6 +98,9 @@ #define OMAP_RTC_INTERRUPTS_IT_ALARM (1<<3) #define OMAP_RTC_INTERRUPTS_IT_TIMER (1<<2) +/* OMAP_RTC_IRQWAKEEN bit fields: */ +#define OMAP_RTC_IRQWAKEEN_ALARM_WAKEEN (1<<1) + /* OMAP_RTC_KICKER values */ #define KICK0_VALUE 0x83e70b13 #define KICK1_VALUE 0x95a4f1e0 @@ -103,6 +108,7 @@ #define OMAP_RTC_HAS_KICKER 0x1 static void __iomem *rtc_base; +static unsigned int has_irq_wake_enb_bit; #define rtc_read(addr) readb(rtc_base + (addr)) #define rtc_write(val, addr) writeb(val, rtc_base + (addr)) @@ -425,9 +431,14 @@ static int __init omap_rtc_probe(struct platform_device *pdev) /* Fixup wakeup-enable feature based on the device tree */ if (of_id && of_find_property(pdev->dev.of_node, - "ti,wakeup_capable", NULL)) + "ti,wakeup_capable", NULL)) { device_init_wakeup(&pdev->dev, 1); + if (of_find_property(pdev->dev.of_node, + "ti,has_irq_wake_enb", NULL)) + has_irq_wake_enb_bit = true; + } + if (new_ctrl & (u8) OMAP_RTC_CTRL_SPLIT) pr_info("%s: split power mode\n", pdev->name); @@ -469,16 +480,26 @@ static u8 irqstat; static int omap_rtc_suspend(struct device *dev) { + u8 irqwake_stat; + irqstat = rtc_read(OMAP_RTC_INTERRUPTS_REG); - /* FIXME the RTC alarm is not currently acting as a wakeup event - * source, and in fact this enable() call is just saving a flag - * that's never used... + /* + * FIXME. On some platforms the RTC alarm is not currently acting as a + * wakeup event source, and in fact this enable() call is just saving a + * flag that's never used... */ - if (device_may_wakeup(dev)) + if (device_may_wakeup(dev)) { enable_irq_wake(omap_rtc_alarm); - else + + if (has_irq_wake_enb_bit == true) { + irqwake_stat = rtc_read(OMAP_RTC_IRQWAKEEN); + irqwake_stat |= OMAP_RTC_IRQWAKEEN_ALARM_WAKEEN; + rtc_write(irqwake_stat, OMAP_RTC_IRQWAKEEN); + } + } else { rtc_write(0, OMAP_RTC_INTERRUPTS_REG); + } /* Disable the clock/module */ pm_runtime_put_sync(dev); @@ -488,13 +509,23 @@ static int omap_rtc_suspend(struct device *dev) static int omap_rtc_resume(struct device *dev) { + u8 irqwake_stat; + /* Enable the clock/module so that we can access the registers */ pm_runtime_get_sync(dev); - if (device_may_wakeup(dev)) + if (device_may_wakeup(dev)) { disable_irq_wake(omap_rtc_alarm); - else + + if (has_irq_wake_enb_bit == true) { + irqwake_stat = rtc_read(OMAP_RTC_IRQWAKEEN); + irqwake_stat &= ~OMAP_RTC_IRQWAKEEN_ALARM_WAKEEN; + rtc_write(irqwake_stat, OMAP_RTC_IRQWAKEEN); + } + } else { rtc_write(irqstat, OMAP_RTC_INTERRUPTS_REG); + } + return 0; } #endif
On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN) is available to enable Wakeup feature for Alarm Events. Since new platforms/Boards are now added through DT only, this feature is supported via DT property only. Platforms using such IP should add the property "ti,has_irq_wake_enb" in rtc DT node. Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com> --- :100644 100644 108a629... b870d87... M Documentation/devicetree/bindings/rtc/rtc-omap.txt :100644 100644 000a02f... 5e9c0dd... M drivers/rtc/rtc-omap.c Documentation/devicetree/bindings/rtc/rtc-omap.txt | 2 + drivers/rtc/rtc-omap.c | 47 ++++++++++++++++---- 2 files changed, 41 insertions(+), 8 deletions(-)