[PATCH 0/4] rtc: omap: handle rtc wakeup support in driver

[PATCH 0/4] rtc: omap: handle rtc wakeup support in driver

[Hebbar Gururaja]

rtc-omap driver modules is used both by OMAP1/2, Davinci SoC platforms.

However, rtc wake support on OMAP1 is broken. Hence the
device_init_wakeup() was removed from rtc-omap driver and moved to
platform board files that supported it (DA850/OMAP-L138). [1]

However, recently [2] it was suggested that driver should always do a
device_init_wakeup(dev, true). Platforms that don't want/need
wakeup support can disable it from userspace via:

    echo disabled > /sys/devices/.../power/wakeup

Also, with the new DT boot-up, board file doesn't exist and hence there
is no way to have device wakeup support rtc.

The fix for above issues, is to hard code device_init_wakeup() inside
driver and let platforms that don't need this, handle it through the
sysfs power entry.


Also, update Davinci & AM335x files to above changes.

[1]
https://patchwork.kernel.org/patch/136731/

[2]
http://www.mail-archive.com/davinci-linux-open-source at linux.
davincidsp.com/msg26077.html

Hebbar Gururaja (4):
  rtc: omap: restore back (hard-code) wakeup support
  davinci: da8xx/omap-l1: Remove hard coding of rtc device wakeup
  rtc: omap: add rtc wakeup support to alarm events
  ARM: dts: AM33XX: update rtc node compatibility

 Documentation/devicetree/bindings/rtc/rtc-omap.txt |    6 +-
 arch/arm/boot/dts/am33xx.dtsi                      |    2 +-
 arch/arm/mach-davinci/devices-da8xx.c              |    9 +--
 drivers/rtc/rtc-omap.c                             |   58 +++++++++++++++++---
 4 files changed, 58 insertions(+), 17 deletions(-)

-- 
1.7.9.5

[PATCH 1/4] rtc: omap: restore back (hard-code) wakeup support

[Hebbar Gururaja]

rtc-omap driver modules is used both by OMAP1/2, Davinci SoC platforms.

However, rtc wake support on OMAP1 is broken. Hence the
device_init_wakeup() was removed from rtc-omap driver and moved to
platform board files that supported it (DA850/OMAP-L138). [1]

However, recently [2] it was suggested that driver should always do a
device_init_wakeup(dev, true). Platforms that don't want/need
wakeup support can disable it from userspace via:

    echo disabled > /sys/devices/.../power/wakeup

Also, with the new DT boot-up, board file doesn't exist and hence there
is no way to have device wakeup support rtc.

The fix for above issues, is to hard code device_init_wakeup() inside
driver and let platforms that don't need this, handle it through the
sysfs power entry.

[1]
https://patchwork.kernel.org/patch/136731/

[2]
http://www.mail-archive.com/davinci-linux-open-source at linux.
davincidsp.com/msg26077.html

Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: rtc-linux at googlegroups.com
---
:100644 100644 b0ba3fc... 761919d... M	drivers/rtc/rtc-omap.c
 drivers/rtc/rtc-omap.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
index b0ba3fc..761919d 100644
--- a/drivers/rtc/rtc-omap.c
+++ b/drivers/rtc/rtc-omap.c
@@ -423,6 +423,8 @@ static int __init omap_rtc_probe(struct platform_device *pdev)
 	 *    is write-only, and always reads as zero...)
 	 */
 
+	device_init_wakeup(&pdev->dev, true);
+
 	if (new_ctrl & (u8) OMAP_RTC_CTRL_SPLIT)
 		pr_info("%s: split power mode\n", pdev->name);
 
-- 
1.7.9.5

[PATCH 1/4] rtc: omap: restore back (hard-code) wakeup support

[Kevin Hilman]

Hebbar Gururaja <gururaja.hebbar@ti.com> writes:

> rtc-omap driver modules is used both by OMAP1/2, Davinci SoC platforms.
>
> However, rtc wake support on OMAP1 is broken. Hence the
> device_init_wakeup() was removed from rtc-omap driver and moved to
> platform board files that supported it (DA850/OMAP-L138). [1]
>
> However, recently [2] it was suggested that driver should always do a
> device_init_wakeup(dev, true). Platforms that don't want/need
> wakeup support can disable it from userspace via:
>
>     echo disabled > /sys/devices/.../power/wakeup
>
> Also, with the new DT boot-up, board file doesn't exist and hence there
> is no way to have device wakeup support rtc.
>
> The fix for above issues, is to hard code device_init_wakeup() inside
> driver and let platforms that don't need this, handle it through the
> sysfs power entry.
>
> [1]
> https://patchwork.kernel.org/patch/136731/
>
> [2]
> http://www.mail-archive.com/davinci-linux-open-source at linux.
> davincidsp.com/msg26077.html
>
> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: rtc-linux at googlegroups.com

Acked-by: Kevin Hilman <khilman@linaro.org>

> ---
> :100644 100644 b0ba3fc... 761919d... M	drivers/rtc/rtc-omap.c
>  drivers/rtc/rtc-omap.c |    2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> index b0ba3fc..761919d 100644
> --- a/drivers/rtc/rtc-omap.c
> +++ b/drivers/rtc/rtc-omap.c
> @@ -423,6 +423,8 @@ static int __init omap_rtc_probe(struct platform_device *pdev)
>  	 *    is write-only, and always reads as zero...)
>  	 */
>  
> +	device_init_wakeup(&pdev->dev, true);
> +
>  	if (new_ctrl & (u8) OMAP_RTC_CTRL_SPLIT)
>  		pr_info("%s: split power mode\n", pdev->name);

[PATCH 2/4] davinci: da8xx/omap-l1: Remove hard coding of rtc device wakeup

[Hebbar Gururaja]

Since now rtc-omap driver itself calls deice_init_wakeup(dev, true),
duplicate call from the rtc device registration can be removed.

This is basically a partial revert of the prev commit

commit 75c99bb0006ee065b4e2995078d779418b0fab54
Author: Sekhar Nori <nsekhar@ti.com>

    davinci: da8xx/omap-l1: mark RTC as a wakeup source

Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Kevin Hilman <khilman@linaro.org>
Cc: Russell King <linux@arm.linux.org.uk>

---
:100644 100644 bf57252... 85a900c... M	arch/arm/mach-davinci/devices-da8xx.c
 arch/arm/mach-davinci/devices-da8xx.c |    9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index bf57252..85a900c 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -827,14 +827,7 @@ static struct platform_device da8xx_rtc_device = {
 
 int da8xx_register_rtc(void)
 {
-	int ret;
-
-	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);
-
-	return ret;
+	return  platform_device_register(&da8xx_rtc_device);
 }
 
 static void __iomem *da8xx_ddr2_ctlr_base;
-- 
1.7.9.5

[PATCH 2/4] davinci: da8xx/omap-l1: Remove hard coding of rtc device wakeup

[Manjunathappa, Prakash]

Hi,

On Fri, Jun 28, 2013 at 15:05:07, Hebbar, Gururaja wrote:
> Since now rtc-omap driver itself calls deice_init_wakeup(dev, true),
> duplicate call from the rtc device registration can be removed.
> 

Tested on da850-evm for rtc wake along with patch 1/2.
$rtcwake -s 2 -d /dev/rtc0 -m mem

Tested-by: Manjunathappa, Prakash <prakash.pm@ti.com>

> This is basically a partial revert of the prev commit
> 
> commit 75c99bb0006ee065b4e2995078d779418b0fab54
> Author: Sekhar Nori <nsekhar@ti.com>
> 
>     davinci: da8xx/omap-l1: mark RTC as a wakeup source
> 
> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Kevin Hilman <khilman@linaro.org>
> Cc: Russell King <linux@arm.linux.org.uk>
> 
> ---
> :100644 100644 bf57252... 85a900c... M	arch/arm/mach-davinci/devices-da8xx.c
>  arch/arm/mach-davinci/devices-da8xx.c |    9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
> index bf57252..85a900c 100644
> --- a/arch/arm/mach-davinci/devices-da8xx.c
> +++ b/arch/arm/mach-davinci/devices-da8xx.c
> @@ -827,14 +827,7 @@ static struct platform_device da8xx_rtc_device = {
>  
>  int da8xx_register_rtc(void)
>  {
> -	int ret;
> -
> -	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);
> -
> -	return ret;
> +	return  platform_device_register(&da8xx_rtc_device);
>  }
>  
>  static void __iomem *da8xx_ddr2_ctlr_base;
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Davinci-linux-open-source mailing list
> Davinci-linux-open-source at linux.davincidsp.com
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
> 

[PATCH 2/4] davinci: da8xx/omap-l1: Remove hard coding of rtc device wakeup

[Kevin Hilman]

Hebbar Gururaja <gururaja.hebbar@ti.com> writes:

> Since now rtc-omap driver itself calls deice_init_wakeup(dev, true),
> duplicate call from the rtc device registration can be removed.
>
> This is basically a partial revert of the prev commit
>
> commit 75c99bb0006ee065b4e2995078d779418b0fab54
> Author: Sekhar Nori <nsekhar@ti.com>
>
>     davinci: da8xx/omap-l1: mark RTC as a wakeup source
>
> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Kevin Hilman <khilman@linaro.org>
> Cc: Russell King <linux@arm.linux.org.uk>
>
> ---
> :100644 100644 bf57252... 85a900c... M	arch/arm/mach-davinci/devices-da8xx.c
>  arch/arm/mach-davinci/devices-da8xx.c |    9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
> index bf57252..85a900c 100644
> --- a/arch/arm/mach-davinci/devices-da8xx.c
> +++ b/arch/arm/mach-davinci/devices-da8xx.c
> @@ -827,14 +827,7 @@ static struct platform_device da8xx_rtc_device = {
>  
>  int da8xx_register_rtc(void)
>  {
> -	int ret;
> -
> -	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);
> -
> -	return ret;
> +	return  platform_device_register(&da8xx_rtc_device);

nit: extra space between 'return' and 'platform_'

>  }
>  
>  static void __iomem *da8xx_ddr2_ctlr_base;

Otherwise,

Acked-by: Kevin Hilman <khilman@linaro.org>

[PATCH 2/4] davinci: da8xx/omap-l1: Remove hard coding of rtc device wakeup

[Hebbar, Gururaja]

On Tue, Jul 02, 2013 at 05:37:43, Kevin Hilman wrote:
> Hebbar Gururaja <gururaja.hebbar@ti.com> writes:
> 
> > Since now rtc-omap driver itself calls deice_init_wakeup(dev, true),
> > duplicate call from the rtc device registration can be removed.
> >
> > This is basically a partial revert of the prev commit
> >
> > commit 75c99bb0006ee065b4e2995078d779418b0fab54
> > Author: Sekhar Nori <nsekhar@ti.com>
> >
> >     davinci: da8xx/omap-l1: mark RTC as a wakeup source
> >
> > Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
> > Cc: Sekhar Nori <nsekhar@ti.com>
> > Cc: Kevin Hilman <khilman@linaro.org>
> > Cc: Russell King <linux@arm.linux.org.uk>
> >
> > ---
> > :100644 100644 bf57252... 85a900c... M	arch/arm/mach-davinci/devices-da8xx.c
> >  arch/arm/mach-davinci/devices-da8xx.c |    9 +--------
> >  1 file changed, 1 insertion(+), 8 deletions(-)
> >
> > diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
> > index bf57252..85a900c 100644
> > --- a/arch/arm/mach-davinci/devices-da8xx.c
> > +++ b/arch/arm/mach-davinci/devices-da8xx.c
> > @@ -827,14 +827,7 @@ static struct platform_device da8xx_rtc_device = {
> >  
> >  int da8xx_register_rtc(void)
> >  {
> > -	int ret;
> > -
> > -	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);
> > -
> > -	return ret;
> > +	return  platform_device_register(&da8xx_rtc_device);
> 
> nit: extra space between 'return' and 'platform_'

Thanks for the review. V2 will soon follow.

> 
> >  }
> >  
> >  static void __iomem *da8xx_ddr2_ctlr_base;
> 
> Otherwise,
> 
> Acked-by: Kevin Hilman <khilman@linaro.org>
> 
> 


Regards, 
Gururaja

[PATCH 2/4] davinci: da8xx/omap-l1: Remove hard coding of rtc device wakeup

[Sekhar Nori]

On 6/28/2013 3:05 PM, Hebbar Gururaja wrote:
> Since now rtc-omap driver itself calls deice_init_wakeup(dev, true),
> duplicate call from the rtc device registration can be removed.
> 
> This is basically a partial revert of the prev commit
> 
> commit 75c99bb0006ee065b4e2995078d779418b0fab54
> Author: Sekhar Nori <nsekhar@ti.com>
> 
>     davinci: da8xx/omap-l1: mark RTC as a wakeup source
> 
> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Kevin Hilman <khilman@linaro.org>
> Cc: Russell King <linux@arm.linux.org.uk>

Subject line should be prefixed with ARM: keeping with arch/arm
convention. Otherwise looks good.

Acked-by: Sekhar Nori <nsekhar@ti.com>

Thanks,
Sekhar

[PATCH 2/4] davinci: da8xx/omap-l1: Remove hard coding of rtc device wakeup

[Hebbar, Gururaja]

On Tue, Jul 02, 2013 at 11:10:14, Nori, Sekhar wrote:
> 
> On 6/28/2013 3:05 PM, Hebbar Gururaja wrote:
> > Since now rtc-omap driver itself calls deice_init_wakeup(dev, true),
> > duplicate call from the rtc device registration can be removed.
> > 
> > This is basically a partial revert of the prev commit
> > 
> > commit 75c99bb0006ee065b4e2995078d779418b0fab54
> > Author: Sekhar Nori <nsekhar@ti.com>
> > 
> >     davinci: da8xx/omap-l1: mark RTC as a wakeup source
> > 
> > Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
> > Cc: Sekhar Nori <nsekhar@ti.com>
> > Cc: Kevin Hilman <khilman@linaro.org>
> > Cc: Russell King <linux@arm.linux.org.uk>
> 
> Subject line should be prefixed with ARM: keeping with arch/arm
> convention. Otherwise looks good.

Will fix it in v2.

> 
> Acked-by: Sekhar Nori <nsekhar@ti.com>

Thanks for the review.

> 
> Thanks,
> Sekhar
> 


Regards, 
Gururaja

[PATCH 3/4] rtc: omap: add rtc wakeup support to alarm events

[Hebbar Gururaja]

On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN)
is available to enable Alarm Wakeup feature. This register needs to be
properly handled for the rtcwake to work properly.

Platforms using such IP should set "ti,am3352-rtc" in rtc device dt
compatibility node.

Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Rob Landley <rob@landley.net>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Kevin Hilman <khilman@linaro.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: rtc-linux at googlegroups.com
Cc: devicetree-discuss at lists.ozlabs.org
Cc: linux-doc at vger.kernel.org
---
:100644 100644 b47aa41... 5a0f02d... M	Documentation/devicetree/bindings/rtc/rtc-omap.txt
:100644 100644 761919d... 666b0c2... M	drivers/rtc/rtc-omap.c
 Documentation/devicetree/bindings/rtc/rtc-omap.txt |    6 ++-
 drivers/rtc/rtc-omap.c                             |   56 +++++++++++++++++---
 2 files changed, 54 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
index b47aa41..5a0f02d 100644
--- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
+++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
@@ -1,7 +1,11 @@
 TI Real Time Clock
 
 Required properties:
-- compatible: "ti,da830-rtc"
+- compatible:
+	- "ti,da830-rtc"  - for RTC IP used similar to that on DA8xx SoC family.
+	- "ti,am3352-rtc" - for RTC IP used similar to that on AM335x SoC family.
+			    This RTC IP has special WAKE-EN Register to enable
+			    Wakeup generation for event Alarm.
 - reg: Address range of rtc register set
 - interrupts: rtc timer, alarm interrupts in order
 - interrupt-parent: phandle for the interrupt controller
diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
index 761919d..666b0c2 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,12 +98,21 @@
 #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
 
 #define	OMAP_RTC_HAS_KICKER		0x1
 
+/*
+ * Few RTC IP revisions has special WAKE-EN Register to enable Wakeup
+ * generation for event Alarm.
+ */
+#define	OMAP_RTC_HAS_IRQWAKEEN		0x2
+
 static void __iomem	*rtc_base;
 
 #define rtc_read(addr)		readb(rtc_base + (addr))
@@ -301,7 +312,8 @@ static struct rtc_class_ops omap_rtc_ops = {
 static int omap_rtc_alarm;
 static int omap_rtc_timer;
 
-#define	OMAP_RTC_DATA_DA830_IDX	1
+#define	OMAP_RTC_DATA_DA830_IDX		1
+#define	OMAP_RTC_DATA_AM335X_IDX	2
 
 static struct platform_device_id omap_rtc_devtype[] = {
 	{
@@ -309,6 +321,9 @@ static struct platform_device_id omap_rtc_devtype[] = {
 	}, {
 		.name	= "da830-rtc",
 		.driver_data = OMAP_RTC_HAS_KICKER,
+	}, {
+		.name	= "am335x-rtc",
+		.driver_data = OMAP_RTC_HAS_KICKER | OMAP_RTC_HAS_IRQWAKEEN,
 	},
 	{},
 };
@@ -318,6 +333,9 @@ static const struct of_device_id omap_rtc_of_match[] = {
 	{	.compatible	= "ti,da830-rtc",
 		.data		= &omap_rtc_devtype[OMAP_RTC_DATA_DA830_IDX],
 	},
+	{	.compatible	= "ti,am3352-rtc",
+		.data		= &omap_rtc_devtype[OMAP_RTC_DATA_AM335X_IDX],
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, omap_rtc_of_match);
@@ -466,16 +484,28 @@ static u8 irqstat;
 
 static int omap_rtc_suspend(struct device *dev)
 {
+	u8 irqwake_stat;
+	struct platform_device *pdev = to_platform_device(dev);
+	const struct platform_device_id *id_entry =
+					platform_get_device_id(pdev);
+
 	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...
+	 * source on some platforms, 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 (id_entry->driver_data & OMAP_RTC_HAS_IRQWAKEEN) {
+			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);
@@ -485,13 +515,25 @@ static int omap_rtc_suspend(struct device *dev)
 
 static int omap_rtc_resume(struct device *dev)
 {
+	u8 irqwake_stat;
+	struct platform_device *pdev = to_platform_device(dev);
+	const struct platform_device_id *id_entry =
+				platform_get_device_id(pdev);
+
 	/* 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 (id_entry->driver_data & OMAP_RTC_HAS_IRQWAKEEN) {
+			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
-- 
1.7.9.5

[PATCH 3/4] rtc: omap: add rtc wakeup support to alarm events

[Kevin Hilman]

Hebbar Gururaja <gururaja.hebbar@ti.com> writes:

> On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN)
> is available to enable Alarm Wakeup feature. This register needs to be
> properly handled for the rtcwake to work properly.
>
> Platforms using such IP should set "ti,am3352-rtc" in rtc device dt
> compatibility node.
>
> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Rob Landley <rob@landley.net>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Kevin Hilman <khilman@linaro.org>
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: rtc-linux at googlegroups.com
> Cc: devicetree-discuss at lists.ozlabs.org
> Cc: linux-doc at vger.kernel.org

Acked-by: Kevin Hilman <khilman@linaro.org>

...with a minor nit below...

> ---
> :100644 100644 b47aa41... 5a0f02d... M	Documentation/devicetree/bindings/rtc/rtc-omap.txt
> :100644 100644 761919d... 666b0c2... M	drivers/rtc/rtc-omap.c
>  Documentation/devicetree/bindings/rtc/rtc-omap.txt |    6 ++-
>  drivers/rtc/rtc-omap.c                             |   56 +++++++++++++++++---
>  2 files changed, 54 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> index b47aa41..5a0f02d 100644
> --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> @@ -1,7 +1,11 @@
>  TI Real Time Clock
>  
>  Required properties:
> -- compatible: "ti,da830-rtc"
> +- compatible:
> +	- "ti,da830-rtc"  - for RTC IP used similar to that on DA8xx SoC family.
> +	- "ti,am3352-rtc" - for RTC IP used similar to that on AM335x SoC family.
> +			    This RTC IP has special WAKE-EN Register to enable
> +			    Wakeup generation for event Alarm.
>  - reg: Address range of rtc register set
>  - interrupts: rtc timer, alarm interrupts in order
>  - interrupt-parent: phandle for the interrupt controller
> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> index 761919d..666b0c2 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
> +

nit: letters in hex numbers are usually lower-case.


Kevin

[PATCH 3/4] rtc: omap: add rtc wakeup support to alarm events

[Hebbar, Gururaja]

On Tue, Jul 02, 2013 at 05:45:01, Kevin Hilman wrote:
> Hebbar Gururaja <gururaja.hebbar@ti.com> writes:
> 
> > On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN)
> > is available to enable Alarm Wakeup feature. This register needs to be
> > properly handled for the rtcwake to work properly.
> >
> > Platforms using such IP should set "ti,am3352-rtc" in rtc device dt
> > compatibility node.
> >
> > Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
> > Cc: Grant Likely <grant.likely@linaro.org>
> > Cc: Rob Herring <rob.herring@calxeda.com>
> > Cc: Rob Landley <rob@landley.net>
> > Cc: Sekhar Nori <nsekhar@ti.com>
> > Cc: Kevin Hilman <khilman@linaro.org>
> > Cc: Alessandro Zummo <a.zummo@towertech.it>
> > Cc: rtc-linux at googlegroups.com
> > Cc: devicetree-discuss at lists.ozlabs.org
> > Cc: linux-doc at vger.kernel.org
> 
> Acked-by: Kevin Hilman <khilman@linaro.org>
> 
> ...with a minor nit below...
> 
> > ---
> > :100644 100644 b47aa41... 5a0f02d... M	Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > :100644 100644 761919d... 666b0c2... M	drivers/rtc/rtc-omap.c
> >  Documentation/devicetree/bindings/rtc/rtc-omap.txt |    6 ++-
> >  drivers/rtc/rtc-omap.c                             |   56 +++++++++++++++++---
> >  2 files changed, 54 insertions(+), 8 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > index b47aa41..5a0f02d 100644
> > --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > @@ -1,7 +1,11 @@
> >  TI Real Time Clock
> >  
> >  Required properties:
> > -- compatible: "ti,da830-rtc"
> > +- compatible:
> > +	- "ti,da830-rtc"  - for RTC IP used similar to that on DA8xx SoC family.
> > +	- "ti,am3352-rtc" - for RTC IP used similar to that on AM335x SoC family.
> > +			    This RTC IP has special WAKE-EN Register to enable
> > +			    Wakeup generation for event Alarm.
> >  - reg: Address range of rtc register set
> >  - interrupts: rtc timer, alarm interrupts in order
> >  - interrupt-parent: phandle for the interrupt controller
> > diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> > index 761919d..666b0c2 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
> > +
> 
> nit: letters in hex numbers are usually lower-case.

Thanks for the review. V2 will soon follow.

> 
> 
> Kevin
> 


Regards, 
Gururaja

[PATCH 3/4] rtc: omap: add rtc wakeup support to alarm events

[Sekhar Nori]

On 6/28/2013 3:05 PM, Hebbar Gururaja wrote:
> On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN)
> is available to enable Alarm Wakeup feature. This register needs to be
> properly handled for the rtcwake to work properly.
> 
> Platforms using such IP should set "ti,am3352-rtc" in rtc device dt
> compatibility node.
> 
> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Rob Landley <rob@landley.net>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Kevin Hilman <khilman@linaro.org>
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: rtc-linux at googlegroups.com
> Cc: devicetree-discuss at lists.ozlabs.org
> Cc: linux-doc at vger.kernel.org
> ---

[...]

> -#define	OMAP_RTC_DATA_DA830_IDX	1
> +#define	OMAP_RTC_DATA_DA830_IDX		1
> +#define	OMAP_RTC_DATA_AM335X_IDX	2
>  
>  static struct platform_device_id omap_rtc_devtype[] = {
>  	{
> @@ -309,6 +321,9 @@ static struct platform_device_id omap_rtc_devtype[] = {
>  	}, {
>  		.name	= "da830-rtc",
>  		.driver_data = OMAP_RTC_HAS_KICKER,
> +	}, {
> +		.name	= "am335x-rtc",

may be use am3352-rtc here just to keep the platform device name and of
compatible in sync.

> +		.driver_data = OMAP_RTC_HAS_KICKER | OMAP_RTC_HAS_IRQWAKEEN,
>  	},
>  	{},

It is better to use the index defined above in the static initialization
so they remain in sync.

	...
	[OMAP_RTC_DATA_DA830_IDX] = {
		.name	= "da830-rtc",
		.driver_data = OMAP_RTC_HAS_KICKER,
	},
	...

>  };
> @@ -318,6 +333,9 @@ static const struct of_device_id omap_rtc_of_match[] = {
>  	{	.compatible	= "ti,da830-rtc",
>  		.data		= &omap_rtc_devtype[OMAP_RTC_DATA_DA830_IDX],
>  	},
> +	{	.compatible	= "ti,am3352-rtc",
> +		.data		= &omap_rtc_devtype[OMAP_RTC_DATA_AM335X_IDX],
> +	},
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, omap_rtc_of_match);

Apart from these minor issues, the patch looks good to me.

Acked-by: Sekhar Nori <nsekhar@ti.com>

Thanks,
Sekhar

[PATCH 3/4] rtc: omap: add rtc wakeup support to alarm events

[Hebbar, Gururaja]

On Tue, Jul 02, 2013 at 11:32:34, Nori, Sekhar wrote:
> On 6/28/2013 3:05 PM, Hebbar Gururaja wrote:
> > On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN)
> > is available to enable Alarm Wakeup feature. This register needs to be
> > properly handled for the rtcwake to work properly.
> > 
> > Platforms using such IP should set "ti,am3352-rtc" in rtc device dt
> > compatibility node.
> > 
> > Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
> > Cc: Grant Likely <grant.likely@linaro.org>
> > Cc: Rob Herring <rob.herring@calxeda.com>
> > Cc: Rob Landley <rob@landley.net>
> > Cc: Sekhar Nori <nsekhar@ti.com>
> > Cc: Kevin Hilman <khilman@linaro.org>
> > Cc: Alessandro Zummo <a.zummo@towertech.it>
> > Cc: rtc-linux at googlegroups.com
> > Cc: devicetree-discuss at lists.ozlabs.org
> > Cc: linux-doc at vger.kernel.org
> > ---
> 
> [...]
> 
> > -#define	OMAP_RTC_DATA_DA830_IDX	1
> > +#define	OMAP_RTC_DATA_DA830_IDX		1
> > +#define	OMAP_RTC_DATA_AM335X_IDX	2
> >  
> >  static struct platform_device_id omap_rtc_devtype[] = {
> >  	{
> > @@ -309,6 +321,9 @@ static struct platform_device_id omap_rtc_devtype[] = {
> >  	}, {
> >  		.name	= "da830-rtc",
> >  		.driver_data = OMAP_RTC_HAS_KICKER,
> > +	}, {
> > +		.name	= "am335x-rtc",
> 
> may be use am3352-rtc here just to keep the platform device name and of
> compatible in sync.

Correct. I will update the same in v2.

> 
> > +		.driver_data = OMAP_RTC_HAS_KICKER | OMAP_RTC_HAS_IRQWAKEEN,
> >  	},
> >  	{},
> 
> It is better to use the index defined above in the static initialization
> so they remain in sync.

Sorry. I didn?t get this.

> 
> 	...
> 	[OMAP_RTC_DATA_DA830_IDX] = {
> 		.name	= "da830-rtc",
> 		.driver_data = OMAP_RTC_HAS_KICKER,
> 	},
> 	...
> 
> >  };
> > @@ -318,6 +333,9 @@ static const struct of_device_id omap_rtc_of_match[] = {
> >  	{	.compatible	= "ti,da830-rtc",
> >  		.data		= &omap_rtc_devtype[OMAP_RTC_DATA_DA830_IDX],
> >  	},
> > +	{	.compatible	= "ti,am3352-rtc",
> > +		.data		= &omap_rtc_devtype[OMAP_RTC_DATA_AM335X_IDX],
> > +	},
> >  	{},
> >  };
> >  MODULE_DEVICE_TABLE(of, omap_rtc_of_match);
> 
> Apart from these minor issues, the patch looks good to me.
> 
> Acked-by: Sekhar Nori <nsekhar@ti.com>
> 
> Thanks,
> Sekhar
> 


Regards, 
Gururaja

[PATCH 3/4] rtc: omap: add rtc wakeup support to alarm events

[Sekhar Nori]

On 7/2/2013 11:34 AM, Hebbar, Gururaja wrote:
> On Tue, Jul 02, 2013 at 11:32:34, Nori, Sekhar wrote:
>> On 6/28/2013 3:05 PM, Hebbar Gururaja wrote:
>>> On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN)
>>> is available to enable Alarm Wakeup feature. This register needs to be
>>> properly handled for the rtcwake to work properly.
>>>
>>> Platforms using such IP should set "ti,am3352-rtc" in rtc device dt
>>> compatibility node.
>>>
>>> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
>>> Cc: Grant Likely <grant.likely@linaro.org>
>>> Cc: Rob Herring <rob.herring@calxeda.com>
>>> Cc: Rob Landley <rob@landley.net>
>>> Cc: Sekhar Nori <nsekhar@ti.com>
>>> Cc: Kevin Hilman <khilman@linaro.org>
>>> Cc: Alessandro Zummo <a.zummo@towertech.it>
>>> Cc: rtc-linux at googlegroups.com
>>> Cc: devicetree-discuss at lists.ozlabs.org
>>> Cc: linux-doc at vger.kernel.org
>>> ---
>>
>> [...]
>>
>>> -#define	OMAP_RTC_DATA_DA830_IDX	1
>>> +#define	OMAP_RTC_DATA_DA830_IDX		1
>>> +#define	OMAP_RTC_DATA_AM335X_IDX	2
>>>  
>>>  static struct platform_device_id omap_rtc_devtype[] = {
>>>  	{
>>> @@ -309,6 +321,9 @@ static struct platform_device_id omap_rtc_devtype[] = {
>>>  	}, {
>>>  		.name	= "da830-rtc",
>>>  		.driver_data = OMAP_RTC_HAS_KICKER,
>>> +	}, {
>>> +		.name	= "am335x-rtc",
>>
>> may be use am3352-rtc here just to keep the platform device name and of
>> compatible in sync.
> 
> Correct. I will update the same in v2.
> 
>>
>>> +		.driver_data = OMAP_RTC_HAS_KICKER | OMAP_RTC_HAS_IRQWAKEEN,
>>>  	},
>>>  	{},
>>
>> It is better to use the index defined above in the static initialization
>> so they remain in sync.
> 
> Sorry. I didn?t get this.
> 

See example below I provided. If its still not clear, let me know what
is not clear.

>> 	...
>> 	[OMAP_RTC_DATA_DA830_IDX] = {
>> 		.name	= "da830-rtc",
>> 		.driver_data = OMAP_RTC_HAS_KICKER,
>> 	},

Thanks,
Sekhar

[PATCH 3/4] rtc: omap: add rtc wakeup support to alarm events

[Hebbar, Gururaja]

On Tue, Jul 02, 2013 at 11:39:28, Nori, Sekhar wrote:
> On 7/2/2013 11:34 AM, Hebbar, Gururaja wrote:
> > On Tue, Jul 02, 2013 at 11:32:34, Nori, Sekhar wrote:
> >> On 6/28/2013 3:05 PM, Hebbar Gururaja wrote:
> >>> On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN)
> >>> is available to enable Alarm Wakeup feature. This register needs to be
> >>> properly handled for the rtcwake to work properly.
> >>>
> >>> Platforms using such IP should set "ti,am3352-rtc" in rtc device dt
> >>> compatibility node.
> >>>
> >>> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
> >>> Cc: Grant Likely <grant.likely@linaro.org>
> >>> Cc: Rob Herring <rob.herring@calxeda.com>
> >>> Cc: Rob Landley <rob@landley.net>
> >>> Cc: Sekhar Nori <nsekhar@ti.com>
> >>> Cc: Kevin Hilman <khilman@linaro.org>
> >>> Cc: Alessandro Zummo <a.zummo@towertech.it>
> >>> Cc: rtc-linux at googlegroups.com
> >>> Cc: devicetree-discuss at lists.ozlabs.org
> >>> Cc: linux-doc at vger.kernel.org
> >>> ---
> >>
> >> [...]
> >>
> >>> -#define	OMAP_RTC_DATA_DA830_IDX	1
> >>> +#define	OMAP_RTC_DATA_DA830_IDX		1
> >>> +#define	OMAP_RTC_DATA_AM335X_IDX	2
> >>>  
> >>>  static struct platform_device_id omap_rtc_devtype[] = {
> >>>  	{
> >>> @@ -309,6 +321,9 @@ static struct platform_device_id omap_rtc_devtype[] = {
> >>>  	}, {
> >>>  		.name	= "da830-rtc",
> >>>  		.driver_data = OMAP_RTC_HAS_KICKER,
> >>> +	}, {
> >>> +		.name	= "am335x-rtc",
> >>
> >> may be use am3352-rtc here just to keep the platform device name and of
> >> compatible in sync.
> > 
> > Correct. I will update the same in v2.
> > 
> >>
> >>> +		.driver_data = OMAP_RTC_HAS_KICKER | OMAP_RTC_HAS_IRQWAKEEN,
> >>>  	},
> >>>  	{},
> >>
> >> It is better to use the index defined above in the static initialization
> >> so they remain in sync.
> > 
> > Sorry. I didn?t get this.
> > 
> 
> See example below I provided. If its still not clear, let me know what
> is not clear.
> 
> >> 	...
> >> 	[OMAP_RTC_DATA_DA830_IDX] = {
> >> 		.name	= "da830-rtc",
> >> 		.driver_data = OMAP_RTC_HAS_KICKER,
> >> 	},

Thanks for the clarification. In this case will it ok if I update the previous
member also.

> 
> Thanks,
> Sekhar
> 


Regards, 
Gururaja

[PATCH 3/4] rtc: omap: add rtc wakeup support to alarm events

[Sekhar Nori]

On 7/2/2013 11:41 AM, Hebbar, Gururaja wrote:
> On Tue, Jul 02, 2013 at 11:39:28, Nori, Sekhar wrote:
>> On 7/2/2013 11:34 AM, Hebbar, Gururaja wrote:
>>> On Tue, Jul 02, 2013 at 11:32:34, Nori, Sekhar wrote:
>>>> On 6/28/2013 3:05 PM, Hebbar Gururaja wrote:
>>>>> On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN)
>>>>> is available to enable Alarm Wakeup feature. This register needs to be
>>>>> properly handled for the rtcwake to work properly.
>>>>>
>>>>> Platforms using such IP should set "ti,am3352-rtc" in rtc device dt
>>>>> compatibility node.
>>>>>
>>>>> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
>>>>> Cc: Grant Likely <grant.likely@linaro.org>
>>>>> Cc: Rob Herring <rob.herring@calxeda.com>
>>>>> Cc: Rob Landley <rob@landley.net>
>>>>> Cc: Sekhar Nori <nsekhar@ti.com>
>>>>> Cc: Kevin Hilman <khilman@linaro.org>
>>>>> Cc: Alessandro Zummo <a.zummo@towertech.it>
>>>>> Cc: rtc-linux at googlegroups.com
>>>>> Cc: devicetree-discuss at lists.ozlabs.org
>>>>> Cc: linux-doc at vger.kernel.org
>>>>> ---
>>>>
>>>> [...]
>>>>
>>>>> -#define	OMAP_RTC_DATA_DA830_IDX	1
>>>>> +#define	OMAP_RTC_DATA_DA830_IDX		1
>>>>> +#define	OMAP_RTC_DATA_AM335X_IDX	2
>>>>>  
>>>>>  static struct platform_device_id omap_rtc_devtype[] = {
>>>>>  	{
>>>>> @@ -309,6 +321,9 @@ static struct platform_device_id omap_rtc_devtype[] = {
>>>>>  	}, {
>>>>>  		.name	= "da830-rtc",
>>>>>  		.driver_data = OMAP_RTC_HAS_KICKER,
>>>>> +	}, {
>>>>> +		.name	= "am335x-rtc",
>>>>
>>>> may be use am3352-rtc here just to keep the platform device name and of
>>>> compatible in sync.
>>>
>>> Correct. I will update the same in v2.
>>>
>>>>
>>>>> +		.driver_data = OMAP_RTC_HAS_KICKER | OMAP_RTC_HAS_IRQWAKEEN,
>>>>>  	},
>>>>>  	{},
>>>>
>>>> It is better to use the index defined above in the static initialization
>>>> so they remain in sync.
>>>
>>> Sorry. I didn?t get this.
>>>
>>
>> See example below I provided. If its still not clear, let me know what
>> is not clear.
>>
>>>> 	...
>>>> 	[OMAP_RTC_DATA_DA830_IDX] = {
>>>> 		.name	= "da830-rtc",
>>>> 		.driver_data = OMAP_RTC_HAS_KICKER,
>>>> 	},
> 
> Thanks for the clarification. In this case will it ok if I update the previous
> member also.

You dont really reference [0] in omap_rtc_of_match[] so even if you
leave it as-is, that's fine with me. I am mostly concerned with the
index definitions and initialization order being out of sync and that's
really not an issue with [0].

Thanks,
Sekhar

[PATCH 3/4] rtc: omap: add rtc wakeup support to alarm events

[Hebbar, Gururaja]

Hi all,

Kindly ignore this message. It was sent in wrong format.

Sorry for the noise

Regards, 
Gururaja

On Wed, Jul 03, 2013 at 10:26:57, Hebbar, Gururaja wrote:
> Below is the code snippet I was referring to
>  
>  
> From drivers/rtc/rtc-omap.c
>  
> static struct platform_device_id omap_rtc_devtype[] = {
>       {
>             .name = DRIVER_NAME,
>       },
>       [OMAP_RTC_DATA_AM3352_IDX] = {
>             .name = "am3352-rtc",
>             .driver_data = OMAP_RTC_HAS_KICKER | OMAP_RTC_HAS_IRQWAKEEN,
>       },
>       [OMAP_RTC_DATA_DA830_IDX] = {
>             .name = "da830-rtc",
>             .driver_data = OMAP_RTC_HAS_KICKER,
>      },
>       {},
> };
> MODULE_DEVICE_TABLE(platform, omap_rtc_devtype);
>  
> static const struct of_device_id omap_rtc_of_match[] = {
>       {     .compatible = "ti,da830-rtc",
>             .data       = &omap_rtc_devtype[OMAP_RTC_DATA_DA830_IDX],
>       },
>       {     .compatible = "ti,am3352-rtc",
>             .data       = &omap_rtc_devtype[OMAP_RTC_DATA_AM3352_IDX],
>       },
>       {},
> };
> MODULE_DEVICE_TABLE(of, omap_rtc_of_match);
>  
>  
> From arch/arm/boot/dts/am33xx.dtsi
>  
>             rtc at 44e3e000 {
>                   compatible = "ti,da830-rtc", "ti,am3352-rtc";
>                   reg = <0x44e3e000 0x1000>;
>                   interrupts = <75
>                               76>;
>                   ti,hwmods = "rtc";
>             };
>  
>  
> As seen from above snippet, 2 compatible items are specified for
> compatible dt property (ti,da830-rtc" & "ti,am3352-rtc?)
> These are the same compatibles that are mentioned in the of_device_id
> structure inside rtc-omap driver.
>  
> What I observed is, if we mention both compatible in the .dtsi file that
> are under one single of_device_id structure, the first match from the
> of_device_id structure is considered (ti,da830-rtc in above case)
>  
> To confirm, I switched the 2 compatible inside of_device_id structure as
> below
>  
>  
> static const struct of_device_id omap_rtc_of_match[] = {
>       {     .compatible = "ti,am3352-rtc",
>             .data       = &omap_rtc_devtype[OMAP_RTC_DATA_AM3352_IDX],
>       },
>       {     .compatible = "ti,da830-rtc",
>             .data       = &omap_rtc_devtype[OMAP_RTC_DATA_DA830_IDX],
>       },
>       {},
> };
>  
> In this case, the first match from compatible field was chosen
> (ti,am3352-rtc now).
>  
>  
> Hope this is clear.
>  
> Kindly let me know when you are free to discuss.
>  
>  
> Regards
> Gururaja
>  
> > -----Original Message-----
> > From: Nori, Sekhar
> > Sent: Tuesday, July 02, 2013 11:47 AM
> > To: Hebbar, Gururaja
> > Cc: khilman at linaro.org; tony at atomide.com; Cousson, Benoit; linux-
> > omap at vger.kernel.org; devicetree-discuss at lists.ozlabs.org; linux-
> > kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org;
> > davinci-linux-open-source at linux.davincidsp.com; Bedia, Vaibhav;
> > Rajashekhara, Sudhakar; Grant Likely; Rob Herring; Rob Landley;
> > Alessandro Zummo; rtc-linux at googlegroups.com; linux-
> > doc at vger.kernel.org
> > Subject: Re: [PATCH 3/4] rtc: omap: add rtc wakeup support to
> > alarm events
> > 
> > 
> > 
> > On 7/2/2013 11:41 AM, Hebbar, Gururaja wrote:
> > > On Tue, Jul 02, 2013 at 11:39:28, Nori, Sekhar wrote:
> > >> On 7/2/2013 11:34 AM, Hebbar, Gururaja wrote:
> > >>> On Tue, Jul 02, 2013 at 11:32:34, Nori, Sekhar wrote:
> > >>>> On 6/28/2013 3:05 PM, Hebbar Gururaja wrote:
> > >>>>> On some platforms (like AM33xx), a special register
> > (RTC_IRQWAKEEN)
> > >>>>> is available to enable Alarm Wakeup feature. This register
> > needs to be
> > >>>>> properly handled for the rtcwake to work properly.
> > >>>>>
> > >>>>> Platforms using such IP should set "ti,am3352-rtc" in rtc
> > device dt
> > >>>>> compatibility node.
> > >>>>>
> > >>>>> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com
> <mailto:gururaja.hebbar@ti.com> >
> > >>>>> Cc: Grant Likely <grant.likely@linaro.org
> <mailto:grant.likely@linaro.org> >
> > >>>>> Cc: Rob Herring <rob.herring@calxeda.com
> <mailto:rob.herring@calxeda.com> >
> > >>>>> Cc: Rob Landley <rob at landley.net <mailto:rob@landley.net> >
> > >>>>> Cc: Sekhar Nori <nsekhar at ti.com <mailto:nsekhar@ti.com> >
> > >>>>> Cc: Kevin Hilman <khilman at linaro.org <mailto:khilman@linaro.org>
> >
> > >>>>> Cc: Alessandro Zummo <a.zummo@towertech.it
> <mailto:a.zummo@towertech.it> >
> > >>>>> Cc: rtc-linux at googlegroups.com
> <mailto:rtc-linux@googlegroups.com> 
> > >>>>> Cc: devicetree-discuss at lists.ozlabs.org
> <mailto:devicetree-discuss@lists.ozlabs.org> 
> > >>>>> Cc: linux-doc at vger.kernel.org <mailto:linux-doc@vger.kernel.org>
> > >>>>> ---
> > >>>>
> > >>>> [...]
> > >>>>
> > >>>>> -#define  OMAP_RTC_DATA_DA830_IDX 1
> > >>>>> +#define  OMAP_RTC_DATA_DA830_IDX       1
> > >>>>> +#define  OMAP_RTC_DATA_AM335X_IDX      2
> > >>>>>
> > >>>>>  static struct platform_device_id omap_rtc_devtype[] = {
> > >>>>>     {
> > >>>>> @@ -309,6 +321,9 @@ static struct platform_device_id
> > omap_rtc_devtype[] = {
> > >>>>>     }, {
> > >>>>>           .name = "da830-rtc",
> > >>>>>           .driver_data = OMAP_RTC_HAS_KICKER,
> > >>>>> +   }, {
> > >>>>> +         .name = "am335x-rtc",
> > >>>>
> > >>>> may be use am3352-rtc here just to keep the platform device
> > name and of
> > >>>> compatible in sync.
> > >>>
> > >>> Correct. I will update the same in v2.
> > >>>
> > >>>>
> > >>>>> +         .driver_data = OMAP_RTC_HAS_KICKER |
> > OMAP_RTC_HAS_IRQWAKEEN,
> > >>>>>     },
> > >>>>>     {},
> > >>>>
> > >>>> It is better to use the index defined above in the static
> > initialization
> > >>>> so they remain in sync.
> > >>>
> > >>> Sorry. I didn?t get this.
> > >>>
> > >>
> > >> See example below I provided. If its still not clear, let me
> > know what
> > >> is not clear.
> > >>
> > >>>>      ...
> > >>>>      [OMAP_RTC_DATA_DA830_IDX] = {
> > >>>>            .name = "da830-rtc",
> > >>>>            .driver_data = OMAP_RTC_HAS_KICKER,
> > >>>>      },
> > >
> > > Thanks for the clarification. In this case will it ok if I
> > update the previous
> > > member also.
> > 
> > You dont really reference [0] in omap_rtc_of_match[] so even if
> > you
> > leave it as-is, that's fine with me. I am mostly concerned with
> > the
> > index definitions and initialization order being out of sync and
> > that's
> > really not an issue with [0].
> > 
> > Thanks,
> > Sekhar
> 

[PATCH 4/4] ARM: dts: AM33XX: update rtc node compatibility

[Hebbar Gururaja]

Since AM33xx  RTC IP has RTC_IRQWAKEEN to support Alarm Wake-up.

Update the rtc compatible property to "ti,am3352-rtc" to enable handling
of this feature inside rtc-omap driver.

Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Kevin Hilman <khilman@linaro.org>
Cc: b-cousson at ti.com
---
:100644 100644 77aa1b0... dde180a... M	arch/arm/boot/dts/am33xx.dtsi
 arch/arm/boot/dts/am33xx.dtsi |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 77aa1b0..dde180a 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -297,7 +297,7 @@
 		};
 
 		rtc at 44e3e000 {
-			compatible = "ti,da830-rtc";
+			compatible = "ti,am3352-rtc";
 			reg = <0x44e3e000 0x1000>;
 			interrupts = <75
 				      76>;
-- 
1.7.9.5

[PATCH 4/4] ARM: dts: AM33XX: update rtc node compatibility

[Sekhar Nori]

Changing to Benoit's gmail id since he apparently wont access TI mail
anymore.

On 6/28/2013 3:05 PM, Hebbar Gururaja wrote:
> Since AM33xx  RTC IP has RTC_IRQWAKEEN to support Alarm Wake-up.
> 
> Update the rtc compatible property to "ti,am3352-rtc" to enable handling
> of this feature inside rtc-omap driver.
> 
> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Kevin Hilman <khilman@linaro.org>
> Cc: b-cousson at ti.com
> ---
> :100644 100644 77aa1b0... dde180a... M	arch/arm/boot/dts/am33xx.dtsi
>  arch/arm/boot/dts/am33xx.dtsi |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index 77aa1b0..dde180a 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -297,7 +297,7 @@
>  		};
>  
>  		rtc at 44e3e000 {
> -			compatible = "ti,da830-rtc";
> +			compatible = "ti,am3352-rtc";

compatible is a list so you can instead do:
	
	compatible = "ti,am3352-rtc", "ti,da830-rtc";

That way the dts works irrespective of driver updates. When driver
supports enhanced features of hardware, they are available to the user
else the basic functionality still works.

Thanks,
Sekhar

[PATCH 4/4] ARM: dts: AM33XX: update rtc node compatibility

[Hebbar, Gururaja]

On Tue, Jul 02, 2013 at 11:42:49, Nori, Sekhar wrote:
> Changing to Benoit's gmail id since he apparently wont access TI mail
> anymore.
> 
> On 6/28/2013 3:05 PM, Hebbar Gururaja wrote:
> > Since AM33xx  RTC IP has RTC_IRQWAKEEN to support Alarm Wake-up.
> > 
> > Update the rtc compatible property to "ti,am3352-rtc" to enable handling
> > of this feature inside rtc-omap driver.
> > 
> > Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
> > Cc: Tony Lindgren <tony@atomide.com>
> > Cc: Sekhar Nori <nsekhar@ti.com>
> > Cc: Kevin Hilman <khilman@linaro.org>
> > Cc: b-cousson at ti.com
> > ---
> > :100644 100644 77aa1b0... dde180a... M	arch/arm/boot/dts/am33xx.dtsi
> >  arch/arm/boot/dts/am33xx.dtsi |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> > index 77aa1b0..dde180a 100644
> > --- a/arch/arm/boot/dts/am33xx.dtsi
> > +++ b/arch/arm/boot/dts/am33xx.dtsi
> > @@ -297,7 +297,7 @@
> >  		};
> >  
> >  		rtc at 44e3e000 {
> > -			compatible = "ti,da830-rtc";
> > +			compatible = "ti,am3352-rtc";
> 
> compatible is a list so you can instead do:
> 	
> 	compatible = "ti,am3352-rtc", "ti,da830-rtc";
> 
> That way the dts works irrespective of driver updates. When driver
> supports enhanced features of hardware, they are available to the user
> else the basic functionality still works.

Ok. I will update the same now in v2.

> 
> Thanks,
> Sekhar
> 


Regards, 
Gururaja

[PATCH 4/4] ARM: dts: AM33XX: update rtc node compatibility

[Hebbar, Gururaja]

On Tue, Jul 02, 2013 at 11:42:49, Nori, Sekhar wrote:
> Changing to Benoit's gmail id since he apparently wont access TI mail
> anymore.
> 
> On 6/28/2013 3:05 PM, Hebbar Gururaja wrote:
> > Since AM33xx  RTC IP has RTC_IRQWAKEEN to support Alarm Wake-up.
> > 
> > Update the rtc compatible property to "ti,am3352-rtc" to enable handling
> > of this feature inside rtc-omap driver.
> > 
> > Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
> > Cc: Tony Lindgren <tony@atomide.com>
> > Cc: Sekhar Nori <nsekhar@ti.com>
> > Cc: Kevin Hilman <khilman@linaro.org>
> > Cc: b-cousson at ti.com
> > ---
> > :100644 100644 77aa1b0... dde180a... M	arch/arm/boot/dts/am33xx.dtsi
> >  arch/arm/boot/dts/am33xx.dtsi |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> > index 77aa1b0..dde180a 100644
> > --- a/arch/arm/boot/dts/am33xx.dtsi
> > +++ b/arch/arm/boot/dts/am33xx.dtsi
> > @@ -297,7 +297,7 @@
> >  		};
> >  
> >  		rtc at 44e3e000 {
> > -			compatible = "ti,da830-rtc";
> > +			compatible = "ti,am3352-rtc";
> 
> compatible is a list so you can instead do:
> 	
> 	compatible = "ti,am3352-rtc", "ti,da830-rtc";

I believe the order is not important here. I mean, below is also fine. Right?

	compatible = "ti,da830-rtc", "ti,am3352-rtc";


> 
> That way the dts works irrespective of driver updates. When driver
> supports enhanced features of hardware, they are available to the user
> else the basic functionality still works.
> 
> Thanks,
> Sekhar
> 


Regards, 
Gururaja

[PATCH 4/4] ARM: dts: AM33XX: update rtc node compatibility

[Sekhar Nori]

Gururaja,

On 7/2/2013 11:42 AM, Sekhar Nori wrote:
> Changing to Benoit's gmail id since he apparently wont access TI mail
> anymore.
> 
> On 6/28/2013 3:05 PM, Hebbar Gururaja wrote:
>> Since AM33xx  RTC IP has RTC_IRQWAKEEN to support Alarm Wake-up.
>>
>> Update the rtc compatible property to "ti,am3352-rtc" to enable handling
>> of this feature inside rtc-omap driver.
>>
>> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
>> Cc: Tony Lindgren <tony@atomide.com>
>> Cc: Sekhar Nori <nsekhar@ti.com>
>> Cc: Kevin Hilman <khilman@linaro.org>
>> Cc: b-cousson at ti.com
>> ---
>> :100644 100644 77aa1b0... dde180a... M	arch/arm/boot/dts/am33xx.dtsi
>>  arch/arm/boot/dts/am33xx.dtsi |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
>> index 77aa1b0..dde180a 100644
>> --- a/arch/arm/boot/dts/am33xx.dtsi
>> +++ b/arch/arm/boot/dts/am33xx.dtsi
>> @@ -297,7 +297,7 @@
>>  		};
>>  
>>  		rtc at 44e3e000 {
>> -			compatible = "ti,da830-rtc";
>> +			compatible = "ti,am3352-rtc";
> 
> compatible is a list so you can instead do:
> 	
> 	compatible = "ti,am3352-rtc", "ti,da830-rtc";
> 
> That way the dts works irrespective of driver updates. When driver
> supports enhanced features of hardware, they are available to the user
> else the basic functionality still works.

On doing some experiments myself, the of_device_id which gets selected
during probe depends on the order in which its entry appears in the
match table inside the driver rather than how the compatible string is
written. I think this puts undue dependency on how the driver is
written, so I am okay with providing a single compatible value like the
way you have done ATM.

I do think the string appearing first in the compatible list is what
should be selected if a match is available but I am not sure if there
are other considerations due to which of_match_device() is written the
way it is written.

Thanks,
Sekhar

[PATCH 0/4] rtc: omap: handle rtc wakeup support in driver

[Kevin Hilman]

Hi Hebbar,

Hebbar Gururaja <gururaja.hebbar@ti.com> writes:

> rtc-omap driver modules is used both by OMAP1/2, Davinci SoC platforms.
>
> However, rtc wake support on OMAP1 is broken. Hence the
> device_init_wakeup() was removed from rtc-omap driver and moved to
> platform board files that supported it (DA850/OMAP-L138). [1]
>
> However, recently [2] it was suggested that driver should always do a
> device_init_wakeup(dev, true). Platforms that don't want/need
> wakeup support can disable it from userspace via:
>
>     echo disabled > /sys/devices/.../power/wakeup
>
> Also, with the new DT boot-up, board file doesn't exist and hence there
> is no way to have device wakeup support rtc.
>
> The fix for above issues, is to hard code device_init_wakeup() inside
> driver and let platforms that don't need this, handle it through the
> sysfs power entry.

This series looks good now, thanks for your persistence.

Kevin