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
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]>
---
: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
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]>
Acked-by: Kevin Hilman <[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]>
Acked-by: Kevin Hilman <[email protected]>
Acked-by: Sekhar Nori <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Rob Landley <[email protected]>
Cc: Alessandro Zummo <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
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
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]>
Acked-by: Kevin Hilman <[email protected]>
Acked-by: Sekhar Nori <[email protected]>
Cc: Russell King <[email protected]>
---
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
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 <[email protected]>
> Acked-by: Kevin Hilman <[email protected]>
> Acked-by: Sekhar Nori <[email protected]>
> Cc: Grant Likely <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Rob Landley <[email protected]>
> Cc: Alessandro Zummo <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> 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
>
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 <[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>;
>
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 <[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";
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.
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 <[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";
>
> 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
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 <[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";
> >
> > 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.
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 <[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";
>>>
>>> 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.
>
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 <[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";
> >>>
> >>> 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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/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 <[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";
>>>>>
>>>>> 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
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/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 <[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";
> >>>>>
> >>>>> 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
> >> [email protected]
> >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/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 <[email protected]>
>
> davinci: da8xx/omap-l1: mark RTC as a wakeup source
>
> Signed-off-by: Hebbar Gururaja <[email protected]>
> Acked-by: Kevin Hilman <[email protected]>
> Acked-by: Sekhar Nori <[email protected]>
> Cc: Russell King <[email protected]>
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