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
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
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 <[email protected]>
Cc: Tony Lindgren <[email protected]>
Cc: Sekhar Nori <[email protected]>
Cc: Kevin Hilman <[email protected]>
Cc: [email protected]
---
: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
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 <[email protected]>
davinci: da8xx/omap-l1: mark RTC as a wakeup source
Signed-off-by: Hebbar Gururaja <[email protected]>
Cc: Sekhar Nori <[email protected]>
Cc: Kevin Hilman <[email protected]>
Cc: Russell King <[email protected]>
---
: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
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 <[email protected]>
Cc: Alessandro Zummo <[email protected]>
Cc: [email protected]
---
: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
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 <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Rob Landley <[email protected]>
Cc: Sekhar Nori <[email protected]>
Cc: Kevin Hilman <[email protected]>
Cc: Alessandro Zummo <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
: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
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 <[email protected]>
> This is basically a partial revert of the prev commit
>
> commit 75c99bb0006ee065b4e2995078d779418b0fab54
> Author: Sekhar Nori <[email protected]>
>
> davinci: da8xx/omap-l1: mark RTC as a wakeup source
>
> Signed-off-by: Hebbar Gururaja <[email protected]>
> Cc: Sekhar Nori <[email protected]>
> Cc: Kevin Hilman <[email protected]>
> Cc: Russell King <[email protected]>
>
> ---
> :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
> [email protected]
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
>
Hebbar Gururaja <[email protected]> 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@linux.
> davincidsp.com/msg26077.html
>
> Signed-off-by: Hebbar Gururaja <[email protected]>
> Cc: Alessandro Zummo <[email protected]>
> Cc: [email protected]
Acked-by: Kevin Hilman <[email protected]>
> ---
> :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);
Hebbar Gururaja <[email protected]> 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 <[email protected]>
>
> davinci: da8xx/omap-l1: mark RTC as a wakeup source
>
> Signed-off-by: Hebbar Gururaja <[email protected]>
> Cc: Sekhar Nori <[email protected]>
> Cc: Kevin Hilman <[email protected]>
> Cc: Russell King <[email protected]>
>
> ---
> :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 <[email protected]>
Hebbar Gururaja <[email protected]> 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 <[email protected]>
> Cc: Grant Likely <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Rob Landley <[email protected]>
> Cc: Sekhar Nori <[email protected]>
> Cc: Kevin Hilman <[email protected]>
> Cc: Alessandro Zummo <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
Acked-by: Kevin Hilman <[email protected]>
...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
Hi Hebbar,
Hebbar Gururaja <[email protected]> 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
On Tue, Jul 02, 2013 at 05:37:43, Kevin Hilman wrote:
> Hebbar Gururaja <[email protected]> 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 <[email protected]>
> >
> > davinci: da8xx/omap-l1: mark RTC as a wakeup source
> >
> > Signed-off-by: Hebbar Gururaja <[email protected]>
> > Cc: Sekhar Nori <[email protected]>
> > Cc: Kevin Hilman <[email protected]>
> > Cc: Russell King <[email protected]>
> >
> > ---
> > :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 <[email protected]>
>
>
Regards,
Gururaja
On Tue, Jul 02, 2013 at 05:45:01, Kevin Hilman wrote:
> Hebbar Gururaja <[email protected]> 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 <[email protected]>
> > Cc: Grant Likely <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Cc: Rob Landley <[email protected]>
> > Cc: Sekhar Nori <[email protected]>
> > Cc: Kevin Hilman <[email protected]>
> > Cc: Alessandro Zummo <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
>
> Acked-by: Kevin Hilman <[email protected]>
>
> ...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
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 <[email protected]>
>
> davinci: da8xx/omap-l1: mark RTC as a wakeup source
>
> Signed-off-by: Hebbar Gururaja <[email protected]>
> Cc: Sekhar Nori <[email protected]>
> Cc: Kevin Hilman <[email protected]>
> Cc: Russell King <[email protected]>
Subject line should be prefixed with ARM: keeping with arch/arm
convention. Otherwise looks good.
Acked-by: Sekhar Nori <[email protected]>
Thanks,
Sekhar
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 <[email protected]>
> >
> > davinci: da8xx/omap-l1: mark RTC as a wakeup source
> >
> > Signed-off-by: Hebbar Gururaja <[email protected]>
> > Cc: Sekhar Nori <[email protected]>
> > Cc: Kevin Hilman <[email protected]>
> > Cc: Russell King <[email protected]>
>
> Subject line should be prefixed with ARM: keeping with arch/arm
> convention. Otherwise looks good.
Will fix it in v2.
>
> Acked-by: Sekhar Nori <[email protected]>
Thanks for the review.
>
> Thanks,
> Sekhar
>
Regards,
Gururaja
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
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 <[email protected]>
> Cc: Grant Likely <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Rob Landley <[email protected]>
> Cc: Sekhar Nori <[email protected]>
> Cc: Kevin Hilman <[email protected]>
> Cc: Alessandro Zummo <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
[...]
> -#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 <[email protected]>
Thanks,
Sekhar
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 <[email protected]>
> > Cc: Grant Likely <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Cc: Rob Landley <[email protected]>
> > Cc: Sekhar Nori <[email protected]>
> > Cc: Kevin Hilman <[email protected]>
> > Cc: Alessandro Zummo <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > ---
>
> [...]
>
> > -#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 <[email protected]>
>
> Thanks,
> Sekhar
>
Regards,
Gururaja
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
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 <[email protected]>
>>> Cc: Grant Likely <[email protected]>
>>> Cc: Rob Herring <[email protected]>
>>> Cc: Rob Landley <[email protected]>
>>> Cc: Sekhar Nori <[email protected]>
>>> Cc: Kevin Hilman <[email protected]>
>>> Cc: Alessandro Zummo <[email protected]>
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> ---
>>
>> [...]
>>
>>> -#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
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 <[email protected]>
> >>> Cc: Grant Likely <[email protected]>
> >>> Cc: Rob Herring <[email protected]>
> >>> Cc: Rob Landley <[email protected]>
> >>> Cc: Sekhar Nori <[email protected]>
> >>> Cc: Kevin Hilman <[email protected]>
> >>> Cc: Alessandro Zummo <[email protected]>
> >>> Cc: [email protected]
> >>> Cc: [email protected]
> >>> Cc: [email protected]
> >>> ---
> >>
> >> [...]
> >>
> >>> -#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
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
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 <[email protected]>
> Cc: Tony Lindgren <[email protected]>
> Cc: Sekhar Nori <[email protected]>
> Cc: Kevin Hilman <[email protected]>
> Cc: [email protected]
> ---
> :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";
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
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 <[email protected]>
> > Cc: Tony Lindgren <[email protected]>
> > Cc: Sekhar Nori <[email protected]>
> > Cc: Kevin Hilman <[email protected]>
> > Cc: [email protected]
> > ---
> > :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";
>
> 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
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
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 <[email protected]>
>>>>> Cc: Grant Likely <[email protected]>
>>>>> Cc: Rob Herring <[email protected]>
>>>>> Cc: Rob Landley <[email protected]>
>>>>> Cc: Sekhar Nori <[email protected]>
>>>>> Cc: Kevin Hilman <[email protected]>
>>>>> Cc: Alessandro Zummo <[email protected]>
>>>>> Cc: [email protected]
>>>>> Cc: [email protected]
>>>>> Cc: [email protected]
>>>>> ---
>>>>
>>>> [...]
>>>>
>>>>> -#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
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 <[email protected]>
> > Cc: Tony Lindgren <[email protected]>
> > Cc: Sekhar Nori <[email protected]>
> > Cc: Kevin Hilman <[email protected]>
> > Cc: [email protected]
> > ---
> > :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";
>
> 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
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
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@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: [email protected]; [email protected]; Cousson, Benoit; linux-
> > [email protected]; [email protected]; linux-
> > [email protected]; [email protected];
> > [email protected]; Bedia, Vaibhav;
> > Rajashekhara, Sudhakar; Grant Likely; Rob Herring; Rob Landley;
> > Alessandro Zummo; [email protected]; linux-
> > [email protected]
> > 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 <[email protected]
> <mailto:[email protected]> >
> > >>>>> Cc: Grant Likely <[email protected]
> <mailto:[email protected]> >
> > >>>>> Cc: Rob Herring <[email protected]
> <mailto:[email protected]> >
> > >>>>> Cc: Rob Landley <[email protected] <mailto:[email protected]> >
> > >>>>> Cc: Sekhar Nori <[email protected] <mailto:[email protected]> >
> > >>>>> Cc: Kevin Hilman <[email protected] <mailto:[email protected]>
> >
> > >>>>> Cc: Alessandro Zummo <[email protected]
> <mailto:[email protected]> >
> > >>>>> Cc: [email protected]
> <mailto:[email protected]>
> > >>>>> Cc: [email protected]
> <mailto:[email protected]>
> > >>>>> Cc: [email protected] <mailto:[email protected]>
> > >>>>> ---
> > >>>>
> > >>>> [...]
> > >>>>
> > >>>>> -#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
>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
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 <[email protected]>
>> Cc: Tony Lindgren <[email protected]>
>> Cc: Sekhar Nori <[email protected]>
>> Cc: Kevin Hilman <[email protected]>
>> Cc: [email protected]
>> ---
>> :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";
>
> 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