Skip to contentSkip to conversation

RTC: TWL: ensure all interrupts are disabled during probe

PATCH New Kevin Hilman 4 participants 7 messages Sep 13, 2012 – Sep 14, 2012 Real time clock (RTC)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 RTC: TWL: ensure all interrupts are disabled during probe

From: Kevin Hilman <[email protected]>

On some platforms, bootloaders are known to do some interesting RTC programming. Without going into the obscurities as to why this may be the case, suffice it to say the the driver should not make any assumptions about the state of the RTC when the driver loads. In particular, the driver probe should be sure that all interrupts are disabled until otherwise programmed.

This was discovered when finding bursty I2C traffic every second on Overo platforms. This I2C overhead was keeping the SoC from hitting deep power states. The cause was found to be the RTC firing every second on the I2C-connected TWL PMIC.

Special thanks to Felipe Balbi for suggesting to look for a rogue driver as the source of the I2C traffic rather than the I2C driver itself.

Special thanks to Steve Sakoman for helping track down the source of the continuous RTC interrups on the Overo boards.

Cc Felipe Balbi <[email protected]>
Cc Steve Sakoman <[email protected]>

---
Patch applies to v3.6-rc5

 drivers/rtc/rtc-twl.c |    5 +++++
 1 file changed, 5 insertions(+)
drivers/rtc/rtc-twl.c +5 -0
index c5d06fe..9277d94 100644
@@ -495,6 +495,11 @@ static int __devinit twl_rtc_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto out1;
 
+	/* ensure interrupts are disabled, bootloaders can be strange */
+	ret = twl_rtc_write_u8(0, REG_RTC_INTERRUPTS_REG);
+	if (ret < 0)
+		dev_warn(&pdev->dev, "unable to disable interrupt\n");
+
 	/* init cached IRQ enable bits */
 	ret = twl_rtc_read_u8(&rtc_irq_bits, REG_RTC_INTERRUPTS_REG);
 	if (ret < 0)
Signature · 1 line
1.7.9.2
#1 lore raw

PATCH 1/1 RTC: TWL: ensure all interrupts are disabled during probe

Hi Kevin,

Quoted 14 lines · Kevin Hilman

From: Kevin Hilman <[email protected]>

On some platforms, bootloaders are known to do some interesting RTC programming. Without going into the obscurities as to why this may be the case, suffice it to say the the driver should not make any assumptions about the state of the RTC when the driver loads. In particular, the driver probe should be sure that all interrupts are disabled until otherwise programmed.

This was discovered when finding bursty I2C traffic every second on Overo platforms. This I2C overhead was keeping the SoC from hitting deep power states. The cause was found to be the RTC firing every second on the I2C-connected TWL PMIC.

Quoted 7 lines

Special thanks to Felipe Balbi for suggesting to look for a rogue driver as the source of the I2C traffic rather than the I2C driver itself.

Special thanks to Steve Sakoman for helping track down the source of the continuous RTC interrups on the Overo boards.

Tested that the continuous interrupt issue after doing a i2c mm on omap4sdp. This patch solves the issue.
thanks,

Quoted 14 lines
Cc Felipe Balbi <[email protected]>
Cc Steve Sakoman <[email protected]>

---
Patch applies to v3.6-rc5

 drivers/rtc/rtc-twl.c |    5 +++++
 1 file changed, 5 insertions(+)
drivers/rtc/rtc-twl.c +0 -0
index c5d06fe..9277d94 100644

drivers/rtc/rtc-twl.c static int __devinit twl_rtc_probe(struct platform_device *pdev) · line 502

Quoted 8 lines
drivers/rtc/rtc-twl.c +5 -0
@@ -495,6 +495,11 @@ static int __devinit twl_rtc_probe(struct platform_device *pdev)
        if (ret < 0)
                goto out1;
 
+       /* ensure interrupts are disabled, bootloaders can be strange */
+       ret = twl_rtc_write_u8(0, REG_RTC_INTERRUPTS_REG);
+       if (ret < 0)
+               dev_warn(&pdev->dev, "unable to disable interrupt\n");
+

Now that it is always 0 can the below read be removed as it is redundant now.

Quoted 11 lines
drivers/rtc/rtc-twl.c +0 -1
        /* init cached IRQ enable bits */
        ret = twl_rtc_read_u8(&rtc_irq_bits, REG_RTC_INTERRUPTS_REG);
        if (ret < 0)
--

1.7.9.2

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

#2 lore raw

PATCH 1/1 RTC: TWL: ensure all interrupts are disabled during probe

Quoted 47 lines · Shubhrajyoti Datta

Hi Kevin,

Quoted 14 lines · Kevin Hilman

From: Kevin Hilman <[email protected]>

On some platforms, bootloaders are known to do some interesting RTC programming. Without going into the obscurities as to why this may be the case, suffice it to say the the driver should not make any assumptions about the state of the RTC when the driver loads. In particular, the driver probe should be sure that all interrupts are disabled until otherwise programmed.

This was discovered when finding bursty I2C traffic every second on Overo platforms. This I2C overhead was keeping the SoC from hitting deep power states. The cause was found to be the RTC firing every second on the I2C-connected TWL PMIC.

Quoted 7 lines

Special thanks to Felipe Balbi for suggesting to look for a rogue driver as the source of the I2C traffic rather than the I2C driver itself.

Special thanks to Steve Sakoman for helping track down the source of the continuous RTC interrups on the Overo boards.

Tested that the continuous interrupt issue after doing a i2c mm on omap4sdp. This patch solves the issue.
thanks,

Quoted 17 lines
Cc Felipe Balbi <[email protected]>
Cc Steve Sakoman <[email protected]>

---
Patch applies to v3.6-rc5

 drivers/rtc/rtc-twl.c |    5 +++++
 1 file changed, 5 insertions(+)
drivers/rtc/rtc-twl.c +0 -0
index c5d06fe..9277d94 100644
@@ -495,6 +495,11 @@ static int __devinit twl_rtc_probe(struct platform_device *pdev)
        if (ret < 0)
                goto out1;
 
Quoted 6 lines

drivers/rtc/rtc-twl.c static int __devinit twl_rtc_probe(struct platform_device *pdev) · line 502

Quoted 5 lines
drivers/rtc/rtc-twl.c +5 -0
+       /* ensure interrupts are disabled, bootloaders can be strange */
+       ret = twl_rtc_write_u8(0, REG_RTC_INTERRUPTS_REG);
+       if (ret < 0)
+               dev_warn(&pdev->dev, "unable to disable interrupt\n");
+

Now that it is always 0 can the below read be removed as it is redundant now.

Possibly, but I don't know this HW well enough to know if there are any persistent bits in that register on any of the various derivations of this PMIC. Since this read-back value is used throughout the driver, I decided not to mess with it when doing this targetted fix.

Kevin

#3 lore raw

PATCH 1/1 RTC: TWL: ensure all interrupts are disabled during probe

Quoted 56 lines · Kevin Hilman
Quoted 47 lines · Shubhrajyoti Datta

Hi Kevin,

Quoted 14 lines · Kevin Hilman

From: Kevin Hilman <[email protected]>

On some platforms, bootloaders are known to do some interesting RTC programming. Without going into the obscurities as to why this may be the case, suffice it to say the the driver should not make any assumptions about the state of the RTC when the driver loads. In particular, the driver probe should be sure that all interrupts are disabled until otherwise programmed.

This was discovered when finding bursty I2C traffic every second on Overo platforms. This I2C overhead was keeping the SoC from hitting deep power states. The cause was found to be the RTC firing every second on the I2C-connected TWL PMIC.

Quoted 7 lines

Special thanks to Felipe Balbi for suggesting to look for a rogue driver as the source of the I2C traffic rather than the I2C driver itself.

Special thanks to Steve Sakoman for helping track down the source of the continuous RTC interrups on the Overo boards.

Tested that the continuous interrupt issue after doing a i2c mm on omap4sdp. This patch solves the issue.
thanks,

Quoted 17 lines
Cc Felipe Balbi <[email protected]>
Cc Steve Sakoman <[email protected]>

---
Patch applies to v3.6-rc5

 drivers/rtc/rtc-twl.c |    5 +++++
 1 file changed, 5 insertions(+)
drivers/rtc/rtc-twl.c +0 -0
index c5d06fe..9277d94 100644
@@ -495,6 +495,11 @@ static int __devinit twl_rtc_probe(struct platform_device *pdev)
        if (ret < 0)
                goto out1;
 
Quoted 6 lines

drivers/rtc/rtc-twl.c static int __devinit twl_rtc_probe(struct platform_device *pdev) · line 502

Quoted 5 lines
drivers/rtc/rtc-twl.c +5 -0
+       /* ensure interrupts are disabled, bootloaders can be strange */
+       ret = twl_rtc_write_u8(0, REG_RTC_INTERRUPTS_REG);
+       if (ret < 0)
+               dev_warn(&pdev->dev, "unable to disable interrupt\n");
+

Now that it is always 0 can the below read be removed as it is redundant now.

Quoted 4 lines

Possibly, but I don't know this HW well enough to know if there are any persistent bits in that register on any of the various derivations of this PMIC. Since this read-back value is used throughout the driver, I decided not to mess with it when doing this targetted fix.

Indeed makes sense.

Quoted 3 lines

Kevin

#4 lore raw

PATCH 1/1 RTC: TWL: ensure all interrupts are disabled during probe

In reply to Kevin Hilman
Quoted 18 lines · Kevin Hilman

From: Kevin Hilman <[email protected]>

On some platforms, bootloaders are known to do some interesting RTC programming. Without going into the obscurities as to why this may be the case, suffice it to say the the driver should not make any assumptions about the state of the RTC when the driver loads. In particular, the driver probe should be sure that all interrupts are disabled until otherwise programmed.

This was discovered when finding bursty I2C traffic every second on Overo platforms. This I2C overhead was keeping the SoC from hitting deep power states. The cause was found to be the RTC firing every second on the I2C-connected TWL PMIC.

Special thanks to Felipe Balbi for suggesting to look for a rogue driver as the source of the I2C traffic rather than the I2C driver itself.

Quoted 6 lines

Special thanks to Steve Sakoman for helping track down the source of the continuous RTC interrups on the Overo boards.

Cc Felipe Balbi <[email protected]>
Cc Steve Sakoman <[email protected]>

Tested on Overo/Tobi, and I no longer see the 1/second interrupts. Also verified that alarm interrupts still work using rtcwake.

#5 lore raw

PATCH 1/1 RTC: TWL: ensure all interrupts are disabled during probe

Quoted 1 line · On Fri, 14 Sep 2012 08:33:42 -0700 Steve Sakoman <[email protected]> wrote:

Thanks.

Given the tested-by's that are rolling in, I will assume that people are hitting this problem in 3.5 and perhaps earlier kernels, so I scheduled the fix for 3.6, with a -stable backport.

I might have been wrong about that - in future, please do make these issues clear in the patch changelog?

#6 lore raw

PATCH 1/1 RTC: TWL: ensure all interrupts are disabled during probe

Quoted 3 lines · Andrew Morton

Given the tested-by's that are rolling in, I will assume that people are hitting this problem in 3.5 and perhaps earlier kernels, so I scheduled the fix for 3.6, with a -stable backport.

Yes, I just checked an Overo setup here that is running 3.2 and it has the same issue.

So your plan for 3.6 and -stable is a good one.

Steve