linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [Patch V2 0/4] rtc: omap: handle rtc wakeup support in driver
@ 2013-07-03  8:47 Hebbar Gururaja
  2013-07-03  8:47 ` [Patch V2 1/4] rtc: omap: restore back (hard-code) wakeup support Hebbar Gururaja
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Hebbar Gururaja @ 2013-07-03  8:47 UTC (permalink / raw)
  To: khilman, tony, benoit.cousson
  Cc: linux-omap, devicetree-discuss, linux-kernel, linux-arm-kernel,
	davinci-linux-open-source, vaibhav.bedia, sudhakar.raj,
	gururaja.hebbar

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@linux.
davincidsp.com/msg26077.html

Changes in V2:
	- Coding style corrections (remove extra space, use lower case
	  for hex numbers
	- use prefix ARM: for commit subject keeping with arch/arm
	  convention)
	- use "[AM/am]3352" instead of "[AM/am]335x" to keep the all
	  usages in sync.
	- Use index defined for struct members so they remain in sync
	- Add new compatible to existing one so that when driver
	  supports enhanced features of hardware, they are available
	  to the user else the basic functionality still works

Hebbar Gururaja (4):
  rtc: omap: restore back (hard-code) wakeup support
  ARM: 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                             |   62 +++++++++++++++++---
 4 files changed, 61 insertions(+), 18 deletions(-)

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Patch V2 1/4] rtc: omap: restore back (hard-code) wakeup support
  2013-07-03  8:47 [Patch V2 0/4] rtc: omap: handle rtc wakeup support in driver Hebbar Gururaja
@ 2013-07-03  8:47 ` Hebbar Gururaja
  2013-07-03  8:47 ` [Patch V2 2/4] ARM: Davinci: da8xx/omap-l1: Remove hard coding of rtc device wakeup Hebbar Gururaja
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 15+ messages in thread
From: Hebbar Gururaja @ 2013-07-03  8:47 UTC (permalink / raw)
  To: khilman, tony, benoit.cousson
  Cc: linux-omap, devicetree-discuss, linux-kernel, linux-arm-kernel,
	davinci-linux-open-source, vaibhav.bedia, sudhakar.raj,
	gururaja.hebbar, Alessandro Zummo, rtc-linux

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@linux.
davincidsp.com/msg26077.html

Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
Acked-by: Kevin Hilman <khilman@linaro.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: rtc-linux@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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Patch V2 2/4] ARM: Davinci: da8xx/omap-l1: Remove hard coding of rtc device wakeup
  2013-07-03  8:47 [Patch V2 0/4] rtc: omap: handle rtc wakeup support in driver Hebbar Gururaja
  2013-07-03  8:47 ` [Patch V2 1/4] rtc: omap: restore back (hard-code) wakeup support Hebbar Gururaja
@ 2013-07-03  8:47 ` Hebbar Gururaja
  2013-08-16 12:02   ` Sekhar Nori
  2013-07-03  8:47 ` [Patch V2 3/4] rtc: omap: add rtc wakeup support to alarm events Hebbar Gururaja
  2013-07-03  8:47 ` [Patch V2 4/4] ARM: dts: AM33XX: update rtc node compatibility Hebbar Gururaja
  3 siblings, 1 reply; 15+ messages in thread
From: Hebbar Gururaja @ 2013-07-03  8:47 UTC (permalink / raw)
  To: khilman, tony, benoit.cousson
  Cc: linux-omap, devicetree-discuss, linux-kernel, linux-arm-kernel,
	davinci-linux-open-source, vaibhav.bedia, sudhakar.raj,
	gururaja.hebbar, Russell King

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>
Acked-by: Kevin Hilman <khilman@linaro.org>
Acked-by: Sekhar Nori <nsekhar@ti.com>
Cc: Russell King <linux@arm.linux.org.uk>
---
Changes in V2:
	- Coding style corrections (remove extra space)
	- use prefix ARM: for commit subject keeping with arch/arm convention

:100644 100644 bf57252... 9140b1c... 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..9140b1c 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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Patch V2 3/4] rtc: omap: add rtc wakeup support to alarm events
  2013-07-03  8:47 [Patch V2 0/4] rtc: omap: handle rtc wakeup support in driver Hebbar Gururaja
  2013-07-03  8:47 ` [Patch V2 1/4] rtc: omap: restore back (hard-code) wakeup support Hebbar Gururaja
  2013-07-03  8:47 ` [Patch V2 2/4] ARM: Davinci: da8xx/omap-l1: Remove hard coding of rtc device wakeup Hebbar Gururaja
@ 2013-07-03  8:47 ` Hebbar Gururaja
  2013-07-24 13:14   ` Gururaja Hebbar
  2013-07-03  8:47 ` [Patch V2 4/4] ARM: dts: AM33XX: update rtc node compatibility Hebbar Gururaja
  3 siblings, 1 reply; 15+ messages in thread
From: Hebbar Gururaja @ 2013-07-03  8:47 UTC (permalink / raw)
  To: khilman, tony, benoit.cousson
  Cc: linux-omap, devicetree-discuss, linux-kernel, linux-arm-kernel,
	davinci-linux-open-source, vaibhav.bedia, sudhakar.raj,
	gururaja.hebbar, Grant Likely, Rob Herring, Rob Landley,
	Alessandro Zummo, rtc-linux, linux-doc

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>
Acked-by: Kevin Hilman <khilman@linaro.org>
Acked-by: Sekhar Nori <nsekhar@ti.com>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Rob Landley <rob@landley.net>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: rtc-linux@googlegroups.com
Cc: devicetree-discuss@lists.ozlabs.org
Cc: linux-doc@vger.kernel.org
---
Changes in V2:
	- Coding style corrections (use lower case for hex numbers)
	- use "[AM/am]3352" instead of "[AM/am]335x" to keep the all
	  usages in sync.
	- Use index defined for struct members so they remain in sync

:100644 100644 b47aa41... 5a0f02d... M	Documentation/devicetree/bindings/rtc/rtc-omap.txt
:100644 100644 761919d... c2e18fe... M	drivers/rtc/rtc-omap.c
 Documentation/devicetree/bindings/rtc/rtc-omap.txt |    6 +-
 drivers/rtc/rtc-omap.c                             |   60 +++++++++++++++++---
 2 files changed, 57 insertions(+), 9 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..c2e18fe 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,12 +312,18 @@ 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_AM3352_IDX	1
+#define	OMAP_RTC_DATA_DA830_IDX		2
 
 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,
 	},
@@ -318,6 +335,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_AM3352_IDX],
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, omap_rtc_of_match);
@@ -466,16 +486,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 +517,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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Patch V2 4/4] ARM: dts: AM33XX: update rtc node compatibility
  2013-07-03  8:47 [Patch V2 0/4] rtc: omap: handle rtc wakeup support in driver Hebbar Gururaja
                   ` (2 preceding siblings ...)
  2013-07-03  8:47 ` [Patch V2 3/4] rtc: omap: add rtc wakeup support to alarm events Hebbar Gururaja
@ 2013-07-03  8:47 ` Hebbar Gururaja
  2013-07-30  5:05   ` Gururaja Hebbar
  3 siblings, 1 reply; 15+ messages in thread
From: Hebbar Gururaja @ 2013-07-03  8:47 UTC (permalink / raw)
  To: khilman, tony, benoit.cousson
  Cc: linux-omap, devicetree-discuss, linux-kernel, linux-arm-kernel,
	davinci-linux-open-source, vaibhav.bedia, sudhakar.raj,
	gururaja.hebbar

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>
---
: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@44e3e000 {
-			compatible = "ti,da830-rtc";
+			compatible = "ti,am3352-rtc";
 			reg = <0x44e3e000 0x1000>;
 			interrupts = <75
 				      76>;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [Patch V2 3/4] rtc: omap: add rtc wakeup support to alarm events
  2013-07-03  8:47 ` [Patch V2 3/4] rtc: omap: add rtc wakeup support to alarm events Hebbar Gururaja
@ 2013-07-24 13:14   ` Gururaja Hebbar
  0 siblings, 0 replies; 15+ messages in thread
From: Gururaja Hebbar @ 2013-07-24 13:14 UTC (permalink / raw)
  To: akpm
  Cc: khilman, tony, benoit.cousson, linux-omap, devicetree-discuss,
	linux-kernel, linux-arm-kernel, davinci-linux-open-source,
	vaibhav.bedia, sudhakar.raj, Grant Likely, Rob Herring,
	Rob Landley, Alessandro Zummo, rtc-linux, linux-doc

Hi Andrew,

On 7/3/2013 2:17 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.

I just checked that the 1st patch in this series is pulled in and this
patch [3/4] is not pulled . If you do not have any comments, can you 
pull this one also. I have already got Acks from Sekhar & Kevin for the 
same.

Thanks & regards
Gururaja

>
> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
> Acked-by: Kevin Hilman <khilman@linaro.org>
> Acked-by: Sekhar Nori <nsekhar@ti.com>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Rob Landley <rob@landley.net>
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: rtc-linux@googlegroups.com
> Cc: devicetree-discuss@lists.ozlabs.org
> Cc: linux-doc@vger.kernel.org
> ---
> Changes in V2:
> 	- Coding style corrections (use lower case for hex numbers)
> 	- use "[AM/am]3352" instead of "[AM/am]335x" to keep the all
> 	  usages in sync.
> 	- Use index defined for struct members so they remain in sync
>
> :100644 100644 b47aa41... 5a0f02d... M	Documentation/devicetree/bindings/rtc/rtc-omap.txt
> :100644 100644 761919d... c2e18fe... M	drivers/rtc/rtc-omap.c
>   Documentation/devicetree/bindings/rtc/rtc-omap.txt |    6 +-
>   drivers/rtc/rtc-omap.c                             |   60 +++++++++++++++++---
>   2 files changed, 57 insertions(+), 9 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..c2e18fe 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,12 +312,18 @@ 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_AM3352_IDX	1
> +#define	OMAP_RTC_DATA_DA830_IDX		2
>
>   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,
>   	},
> @@ -318,6 +335,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_AM3352_IDX],
> +	},
>   	{},
>   };
>   MODULE_DEVICE_TABLE(of, omap_rtc_of_match);
> @@ -466,16 +486,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 +517,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
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Patch V2 4/4] ARM: dts: AM33XX: update rtc node compatibility
  2013-07-03  8:47 ` [Patch V2 4/4] ARM: dts: AM33XX: update rtc node compatibility Hebbar Gururaja
@ 2013-07-30  5:05   ` Gururaja Hebbar
  2013-07-30 14:55     ` Mark Rutland
  0 siblings, 1 reply; 15+ messages in thread
From: Gururaja Hebbar @ 2013-07-30  5:05 UTC (permalink / raw)
  To: benoit.cousson
  Cc: khilman, tony, benoit.cousson, linux-omap, devicetree-discuss,
	linux-kernel, linux-arm-kernel, davinci-linux-open-source,
	vaibhav.bedia, sudhakar.raj

Hi,

On 7/3/2013 2:17 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.

The other 2 rtc driver related patches have been pulled up. If you have 
no comments, can you please pull this up.

Regards
Gururaja

>
> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@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@44e3e000 {
> -			compatible = "ti,da830-rtc";
> +			compatible = "ti,am3352-rtc";
>   			reg = <0x44e3e000 0x1000>;
>   			interrupts = <75
>   				      76>;
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Patch V2 4/4] ARM: dts: AM33XX: update rtc node compatibility
  2013-07-30  5:05   ` Gururaja Hebbar
@ 2013-07-30 14:55     ` Mark Rutland
  2013-07-30 16:21       ` Sekhar Nori
  0 siblings, 1 reply; 15+ messages in thread
From: Mark Rutland @ 2013-07-30 14:55 UTC (permalink / raw)
  To: Gururaja Hebbar
  Cc: benoit.cousson, davinci-linux-open-source, khilman, tony,
	benoit.cousson, linux-kernel, vaibhav.bedia, sudhakar.raj,
	linux-omap, devicetree-discuss, linux-arm-kernel

On Tue, Jul 30, 2013 at 06:05:52AM +0100, Gururaja Hebbar wrote:
> Hi,
> 
> On 7/3/2013 2:17 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.
> 
> The other 2 rtc driver related patches have been pulled up. If you have 
> no comments, can you please pull this up.
> 
> Regards
> Gururaja
> 
> >
> > Signed-off-by: Hebbar Gururaja <gururaja.hebbar@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@44e3e000 {
> > -			compatible = "ti,da830-rtc";
> > +			compatible = "ti,am3352-rtc";

Given that this is backwards compatible with the old binding, it would
be nicer to have this as:

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

We must get into the habit of changing dts files in a
backwards-compatible fashion.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Patch V2 4/4] ARM: dts: AM33XX: update rtc node compatibility
  2013-07-30 14:55     ` Mark Rutland
@ 2013-07-30 16:21       ` Sekhar Nori
  2013-08-01 17:05         ` Mark Rutland
  0 siblings, 1 reply; 15+ messages in thread
From: Sekhar Nori @ 2013-07-30 16:21 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Gururaja Hebbar, davinci-linux-open-source, tony, benoit.cousson,
	linux-kernel, vaibhav.bedia, benoit.cousson, linux-omap,
	devicetree-discuss, linux-arm-kernel

On 7/30/2013 8:25 PM, Mark Rutland wrote:
> On Tue, Jul 30, 2013 at 06:05:52AM +0100, Gururaja Hebbar wrote:
>> Hi,
>>
>> On 7/3/2013 2:17 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.
>>
>> The other 2 rtc driver related patches have been pulled up. If you have 
>> no comments, can you please pull this up.
>>
>> Regards
>> Gururaja
>>
>>>
>>> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@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@44e3e000 {
>>> -			compatible = "ti,da830-rtc";
>>> +			compatible = "ti,am3352-rtc";
> 
> Given that this is backwards compatible with the old binding, it would
> be nicer to have this as:
> 
> 	compatible = "ti,am3352-rtc", "ti,da830-rtc";
> 
> We must get into the habit of changing dts files in a
> backwards-compatible fashion.

Right, I suggested this when v1 was posted. It turns out the current
kernel does not handle the compatilble list correctly and the string
selected actually depends on the order in which it appears in match
table in driver instead.

I saw there were patches being discussed to fix this issue, but until
that is fixed, we cannot really use what you (and I before) suggested.

Thanks,
Sekhar

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Patch V2 4/4] ARM: dts: AM33XX: update rtc node compatibility
  2013-07-30 16:21       ` Sekhar Nori
@ 2013-08-01 17:05         ` Mark Rutland
  2013-08-02 11:07           ` Gururaja Hebbar
  0 siblings, 1 reply; 15+ messages in thread
From: Mark Rutland @ 2013-08-01 17:05 UTC (permalink / raw)
  To: Sekhar Nori
  Cc: Gururaja Hebbar, davinci-linux-open-source, tony, benoit.cousson,
	linux-kernel, vaibhav.bedia, benoit.cousson, linux-omap,
	devicetree-discuss, linux-arm-kernel

On Tue, Jul 30, 2013 at 05:21:14PM +0100, Sekhar Nori wrote:
> On 7/30/2013 8:25 PM, Mark Rutland wrote:
> > On Tue, Jul 30, 2013 at 06:05:52AM +0100, Gururaja Hebbar wrote:
> >> Hi,
> >>
> >> On 7/3/2013 2:17 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.
> >>
> >> The other 2 rtc driver related patches have been pulled up. If you have 
> >> no comments, can you please pull this up.
> >>
> >> Regards
> >> Gururaja
> >>
> >>>
> >>> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@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@44e3e000 {
> >>> -			compatible = "ti,da830-rtc";
> >>> +			compatible = "ti,am3352-rtc";
> > 
> > Given that this is backwards compatible with the old binding, it would
> > be nicer to have this as:
> > 
> > 	compatible = "ti,am3352-rtc", "ti,da830-rtc";
> > 
> > We must get into the habit of changing dts files in a
> > backwards-compatible fashion.
> 
> Right, I suggested this when v1 was posted. It turns out the current
> kernel does not handle the compatilble list correctly and the string
> selected actually depends on the order in which it appears in match
> table in driver instead.
> 
> I saw there were patches being discussed to fix this issue, but until
> that is fixed, we cannot really use what you (and I before) suggested.

A temporary solution would be to list the "ti,am3352-rtc" first in the
of_match_table kernel-side. That way you can keep the old compatible
string in the dts compatible list, and maintain backwards compatibilty
with older kernels.

Later if the way Linux matches compatible strings is changed, this
shouldn't break the probe order, and the of_match_table order could be
changed.

Have I missed something?

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Patch V2 4/4] ARM: dts: AM33XX: update rtc node compatibility
  2013-08-01 17:05         ` Mark Rutland
@ 2013-08-02 11:07           ` Gururaja Hebbar
  2013-08-02 11:20             ` Mark Rutland
  0 siblings, 1 reply; 15+ messages in thread
From: Gururaja Hebbar @ 2013-08-02 11:07 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Sekhar Nori, davinci-linux-open-source, tony, benoit.cousson,
	linux-kernel, vaibhav.bedia, benoit.cousson, linux-omap,
	devicetree-discuss, linux-arm-kernel

On 8/1/2013 10:35 PM, Mark Rutland wrote:
> On Tue, Jul 30, 2013 at 05:21:14PM +0100, Sekhar Nori wrote:
>> On 7/30/2013 8:25 PM, Mark Rutland wrote:
>>> On Tue, Jul 30, 2013 at 06:05:52AM +0100, Gururaja Hebbar wrote:
>>>> Hi,
>>>>
>>>> On 7/3/2013 2:17 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.
>>>>
>>>> The other 2 rtc driver related patches have been pulled up. If you have
>>>> no comments, can you please pull this up.
>>>>
>>>> Regards
>>>> Gururaja
>>>>
>>>>>
>>>>> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@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@44e3e000 {
>>>>> -			compatible = "ti,da830-rtc";
>>>>> +			compatible = "ti,am3352-rtc";
>>>
>>> Given that this is backwards compatible with the old binding, it would
>>> be nicer to have this as:
>>>
>>> 	compatible = "ti,am3352-rtc", "ti,da830-rtc";
>>>
>>> We must get into the habit of changing dts files in a
>>> backwards-compatible fashion.
>>
>> Right, I suggested this when v1 was posted. It turns out the current
>> kernel does not handle the compatilble list correctly and the string
>> selected actually depends on the order in which it appears in match
>> table in driver instead.
>>
>> I saw there were patches being discussed to fix this issue, but until
>> that is fixed, we cannot really use what you (and I before) suggested.
>
> A temporary solution would be to list the "ti,am3352-rtc" first in the
> of_match_table kernel-side.

If above method is followed, then it would cause trouble on davinci 
platform because this rtc-omap driver is also used by Davinci platform.
On davinci Plaform the driver would match with "ti,am3352-rtc" for 
compatible.

Regards
Gururaja

> That way you can keep the old compatible
> string in the dts compatible list, and maintain backwards compatibilty
> with older kernels.
>
> Later if the way Linux matches compatible strings is changed, this
> shouldn't break the probe order, and the of_match_table order could be
> changed.
>
> Have I missed something?
>
> Thanks,
> Mark.
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Patch V2 4/4] ARM: dts: AM33XX: update rtc node compatibility
  2013-08-02 11:07           ` Gururaja Hebbar
@ 2013-08-02 11:20             ` Mark Rutland
  2013-08-02 11:48               ` Gururaja Hebbar
  0 siblings, 1 reply; 15+ messages in thread
From: Mark Rutland @ 2013-08-02 11:20 UTC (permalink / raw)
  To: Gururaja Hebbar
  Cc: davinci-linux-open-source, tony, benoit.cousson, Sekhar Nori,
	linux-kernel, vaibhav.bedia, benoit.cousson, linux-omap,
	devicetree-discuss, linux-arm-kernel

On Fri, Aug 02, 2013 at 12:07:36PM +0100, Gururaja Hebbar wrote:
> On 8/1/2013 10:35 PM, Mark Rutland wrote:
> > On Tue, Jul 30, 2013 at 05:21:14PM +0100, Sekhar Nori wrote:
> >> On 7/30/2013 8:25 PM, Mark Rutland wrote:
> >>> On Tue, Jul 30, 2013 at 06:05:52AM +0100, Gururaja Hebbar wrote:
> >>>> Hi,
> >>>>
> >>>> On 7/3/2013 2:17 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.
> >>>>
> >>>> The other 2 rtc driver related patches have been pulled up. If you have
> >>>> no comments, can you please pull this up.
> >>>>
> >>>> Regards
> >>>> Gururaja
> >>>>
> >>>>>
> >>>>> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@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@44e3e000 {
> >>>>> -			compatible = "ti,da830-rtc";
> >>>>> +			compatible = "ti,am3352-rtc";
> >>>
> >>> Given that this is backwards compatible with the old binding, it would
> >>> be nicer to have this as:
> >>>
> >>> 	compatible = "ti,am3352-rtc", "ti,da830-rtc";
> >>>
> >>> We must get into the habit of changing dts files in a
> >>> backwards-compatible fashion.
> >>
> >> Right, I suggested this when v1 was posted. It turns out the current
> >> kernel does not handle the compatilble list correctly and the string
> >> selected actually depends on the order in which it appears in match
> >> table in driver instead.
> >>
> >> I saw there were patches being discussed to fix this issue, but until
> >> that is fixed, we cannot really use what you (and I before) suggested.
> >
> > A temporary solution would be to list the "ti,am3352-rtc" first in the
> > of_match_table kernel-side.
> 
> If above method is followed, then it would cause trouble on davinci 
> platform because this rtc-omap driver is also used by Davinci platform.
> On davinci Plaform the driver would match with "ti,am3352-rtc" for 
> compatible.

Sorry, I don't follow. Does the davinci dt have "ti,am3352-rtc" in it's
compatible string list?

If so, and it's not compatible, the dts is wrong.

If not, then we won't use the behaviour specific to "ti,am3352-rtc", and
there's no problem.

What trouble would this cause?

Thanks,
Mark.

> 
> Regards
> Gururaja
> 
> > That way you can keep the old compatible
> > string in the dts compatible list, and maintain backwards compatibilty
> > with older kernels.
> >
> > Later if the way Linux matches compatible strings is changed, this
> > shouldn't break the probe order, and the of_match_table order could be
> > changed.
> >
> > Have I missed something?
> >
> > Thanks,
> > Mark.
> >
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Patch V2 4/4] ARM: dts: AM33XX: update rtc node compatibility
  2013-08-02 11:20             ` Mark Rutland
@ 2013-08-02 11:48               ` Gururaja Hebbar
  2013-08-02 12:40                 ` Mark Rutland
  0 siblings, 1 reply; 15+ messages in thread
From: Gururaja Hebbar @ 2013-08-02 11:48 UTC (permalink / raw)
  To: Mark Rutland
  Cc: davinci-linux-open-source, tony, benoit.cousson, Sekhar Nori,
	linux-kernel, vaibhav.bedia, benoit.cousson, linux-omap,
	devicetree-discuss, linux-arm-kernel

On 8/2/2013 4:50 PM, Mark Rutland wrote:
> On Fri, Aug 02, 2013 at 12:07:36PM +0100, Gururaja Hebbar wrote:
>> On 8/1/2013 10:35 PM, Mark Rutland wrote:
>>> On Tue, Jul 30, 2013 at 05:21:14PM +0100, Sekhar Nori wrote:
>>>> On 7/30/2013 8:25 PM, Mark Rutland wrote:
>>>>> On Tue, Jul 30, 2013 at 06:05:52AM +0100, Gururaja Hebbar wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 7/3/2013 2:17 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.
>>>>>>
>>>>>> The other 2 rtc driver related patches have been pulled up. If you have
>>>>>> no comments, can you please pull this up.
>>>>>>
>>>>>> Regards
>>>>>> Gururaja
>>>>>>
>>>>>>>
>>>>>>> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@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@44e3e000 {
>>>>>>> -			compatible = "ti,da830-rtc";
>>>>>>> +			compatible = "ti,am3352-rtc";
>>>>>
>>>>> Given that this is backwards compatible with the old binding, it would
>>>>> be nicer to have this as:
>>>>>
>>>>> 	compatible = "ti,am3352-rtc", "ti,da830-rtc";
>>>>>
>>>>> We must get into the habit of changing dts files in a
>>>>> backwards-compatible fashion.
>>>>
>>>> Right, I suggested this when v1 was posted. It turns out the current
>>>> kernel does not handle the compatilble list correctly and the string
>>>> selected actually depends on the order in which it appears in match
>>>> table in driver instead.
>>>>
>>>> I saw there were patches being discussed to fix this issue, but until
>>>> that is fixed, we cannot really use what you (and I before) suggested.
>>>
>>> A temporary solution would be to list the "ti,am3352-rtc" first in the
>>> of_match_table kernel-side.
>>
>> If above method is followed, then it would cause trouble on davinci
>> platform because this rtc-omap driver is also used by Davinci platform.
>> On davinci Plaform the driver would match with "ti,am3352-rtc" for
>> compatible.
>
> Sorry, I don't follow. Does the davinci dt have "ti,am3352-rtc" in it's
> compatible string list?

no no. Davinci .dts uses "ti,da830-rtc".

I was saying if we reverse the order of "of_device_id" structure in 
rtc-omap driver, then it would work nicely for am335x but will cause 
trouble on davinci platform.

>
> If so, and it's not compatible, the dts is wrong.
>
> If not, then we won't use the behaviour specific to "ti,am3352-rtc", and
> there's no problem.
>
> What trouble would this cause?
>
> Thanks,
> Mark.
>
>>
>> Regards
>> Gururaja
>>
>>> That way you can keep the old compatible
>>> string in the dts compatible list, and maintain backwards compatibilty
>>> with older kernels.
>>>
>>> Later if the way Linux matches compatible strings is changed, this
>>> shouldn't break the probe order, and the of_match_table order could be
>>> changed.
>>>
>>> Have I missed something?
>>>
>>> Thanks,
>>> Mark.
>>>
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Patch V2 4/4] ARM: dts: AM33XX: update rtc node compatibility
  2013-08-02 11:48               ` Gururaja Hebbar
@ 2013-08-02 12:40                 ` Mark Rutland
  0 siblings, 0 replies; 15+ messages in thread
From: Mark Rutland @ 2013-08-02 12:40 UTC (permalink / raw)
  To: Gururaja Hebbar
  Cc: davinci-linux-open-source, tony, benoit.cousson, Sekhar Nori,
	linux-kernel, vaibhav.bedia, benoit.cousson, linux-omap,
	devicetree-discuss, linux-arm-kernel

On Fri, Aug 02, 2013 at 12:48:07PM +0100, Gururaja Hebbar wrote:
> On 8/2/2013 4:50 PM, Mark Rutland wrote:
> > On Fri, Aug 02, 2013 at 12:07:36PM +0100, Gururaja Hebbar wrote:
> >> On 8/1/2013 10:35 PM, Mark Rutland wrote:
> >>> On Tue, Jul 30, 2013 at 05:21:14PM +0100, Sekhar Nori wrote:
> >>>> On 7/30/2013 8:25 PM, Mark Rutland wrote:
> >>>>> On Tue, Jul 30, 2013 at 06:05:52AM +0100, Gururaja Hebbar wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> On 7/3/2013 2:17 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.
> >>>>>>
> >>>>>> The other 2 rtc driver related patches have been pulled up. If you have
> >>>>>> no comments, can you please pull this up.
> >>>>>>
> >>>>>> Regards
> >>>>>> Gururaja
> >>>>>>
> >>>>>>>
> >>>>>>> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@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@44e3e000 {
> >>>>>>> -			compatible = "ti,da830-rtc";
> >>>>>>> +			compatible = "ti,am3352-rtc";
> >>>>>
> >>>>> Given that this is backwards compatible with the old binding, it would
> >>>>> be nicer to have this as:
> >>>>>
> >>>>> 	compatible = "ti,am3352-rtc", "ti,da830-rtc";
> >>>>>
> >>>>> We must get into the habit of changing dts files in a
> >>>>> backwards-compatible fashion.
> >>>>
> >>>> Right, I suggested this when v1 was posted. It turns out the current
> >>>> kernel does not handle the compatilble list correctly and the string
> >>>> selected actually depends on the order in which it appears in match
> >>>> table in driver instead.
> >>>>
> >>>> I saw there were patches being discussed to fix this issue, but until
> >>>> that is fixed, we cannot really use what you (and I before) suggested.
> >>>
> >>> A temporary solution would be to list the "ti,am3352-rtc" first in the
> >>> of_match_table kernel-side.
> >>
> >> If above method is followed, then it would cause trouble on davinci
> >> platform because this rtc-omap driver is also used by Davinci platform.
> >> On davinci Plaform the driver would match with "ti,am3352-rtc" for
> >> compatible.
> >
> > Sorry, I don't follow. Does the davinci dt have "ti,am3352-rtc" in it's
> > compatible string list?
> 
> no no. Davinci .dts uses "ti,da830-rtc".

Ok.

> 
> I was saying if we reverse the order of "of_device_id" structure in 
> rtc-omap driver, then it would work nicely for am335x but will cause 
> trouble on davinci platform.

I don't understand. If we place "ti,am3352-rtc" first in the list, it
won't be match the davinci dts compatible string, and the code will go
over the of_device_id entires until it finds the "ti,da830-rtc" entry
used by davinci.

What problem would this cause, and why?

Thanks,
Mark.

> 
> >
> > If so, and it's not compatible, the dts is wrong.
> >
> > If not, then we won't use the behaviour specific to "ti,am3352-rtc", and
> > there's no problem.
> >
> > What trouble would this cause?
> >
> > Thanks,
> > Mark.
> >
> >>
> >> Regards
> >> Gururaja
> >>
> >>> That way you can keep the old compatible
> >>> string in the dts compatible list, and maintain backwards compatibilty
> >>> with older kernels.
> >>>
> >>> Later if the way Linux matches compatible strings is changed, this
> >>> shouldn't break the probe order, and the of_match_table order could be
> >>> changed.
> >>>
> >>> Have I missed something?
> >>>
> >>> Thanks,
> >>> Mark.
> >>>
> >>
> >>
> >> _______________________________________________
> >> linux-arm-kernel mailing list
> >> linux-arm-kernel@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >>
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Patch V2 2/4] ARM: Davinci: da8xx/omap-l1: Remove hard coding of rtc device wakeup
  2013-07-03  8:47 ` [Patch V2 2/4] ARM: Davinci: da8xx/omap-l1: Remove hard coding of rtc device wakeup Hebbar Gururaja
@ 2013-08-16 12:02   ` Sekhar Nori
  0 siblings, 0 replies; 15+ messages in thread
From: Sekhar Nori @ 2013-08-16 12:02 UTC (permalink / raw)
  To: Hebbar Gururaja
  Cc: khilman, tony, benoit.cousson, davinci-linux-open-source,
	Russell King, devicetree-discuss, linux-kernel, linux-omap,
	linux-arm-kernel

On Wednesday 03 July 2013 02:17 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>
> Acked-by: Kevin Hilman <khilman@linaro.org>
> Acked-by: Sekhar Nori <nsekhar@ti.com>
> Cc: Russell King <linux@arm.linux.org.uk>

Queuing for v3.12. In subject line, please use lower case except for
acronyms and for "ARM". Also, we dont use omap-l1 to avoid confusion
with the real omaps. Just da8xx is enough. I have fixed these locally
this time.

Thanks,
Sekhar


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2013-08-16 12:06 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-03  8:47 [Patch V2 0/4] rtc: omap: handle rtc wakeup support in driver Hebbar Gururaja
2013-07-03  8:47 ` [Patch V2 1/4] rtc: omap: restore back (hard-code) wakeup support Hebbar Gururaja
2013-07-03  8:47 ` [Patch V2 2/4] ARM: Davinci: da8xx/omap-l1: Remove hard coding of rtc device wakeup Hebbar Gururaja
2013-08-16 12:02   ` Sekhar Nori
2013-07-03  8:47 ` [Patch V2 3/4] rtc: omap: add rtc wakeup support to alarm events Hebbar Gururaja
2013-07-24 13:14   ` Gururaja Hebbar
2013-07-03  8:47 ` [Patch V2 4/4] ARM: dts: AM33XX: update rtc node compatibility Hebbar Gururaja
2013-07-30  5:05   ` Gururaja Hebbar
2013-07-30 14:55     ` Mark Rutland
2013-07-30 16:21       ` Sekhar Nori
2013-08-01 17:05         ` Mark Rutland
2013-08-02 11:07           ` Gururaja Hebbar
2013-08-02 11:20             ` Mark Rutland
2013-08-02 11:48               ` Gururaja Hebbar
2013-08-02 12:40                 ` Mark Rutland

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).