OMAP watchdong driver is adapted to runtime PM like a general device
driver but it is not appropriate. It is causing couple of functional
issues.
1. On OMAP4 SYSCLK can't be gated, because of issue with WDTIMER2 module,
which constantly stays in "in transition" state. Value of register
CM_WKUP_WDTIMER2_CLKCTRL is always 0x00010000 in this case.
Issue occurs immediately after first idle, when hwmod framework tries
to disable WDTIMER2 functional clock - "wd_timer2_fck". After this
module falls to "in transition" state, and SYSCLK gating is blocked.
2. Due to runtime PM, watchdog timer may be completely disabled.
In current code base watchdog timer is not disabled only because of
issue 1. Otherwise state of WDTIMER2 module will be "Disabled", and there
will be no interrupts from omap_wdt. In other words watchdog will not
work at all.
Watchdong is a special IP and it should not be disabled otherwise
purpose of it itself is defeated. Watchdog functional clock should
never be disabled. This patch updates the runtime PM handling in
driver so that runtime PM is limited only during probe/shutdown
and suspend/resume.
The patch fixes issue 1 and 2
Signed-off-by: Lokesh Vutla <redacted>
Acked-by: Santosh Shilimkar <redacted>
Cc: Wim Van Sebroeck <redacted>
---
Tested on OMAP4430SDP.
Issue #1 can be easily reproduced on mainline kernel.
- Take latest mainline kernel and create uImage using omap2plus_defconfig
- Write a program to open watchdog,like
void main(void)
{
int fd = open("/dev/watchdog", O_WRONLY);
if (fd == -1) {
perror("Watchdog device interface is not available!\n");
}
}
- Build with arm compiler and copy it to your file syatem
- Boot the image and run the executable.
drivers/watchdog/omap_wdt.c | 17 -----------------
1 files changed, 0 insertions(+), 17 deletions(-)
@@ -166,8 +162,6 @@ static int omap_wdt_open(struct inode *inode, struct file *file)omap_wdt_ping(wdev);/* trigger loading of new timeout value */omap_wdt_enable(wdev);-pm_runtime_put_sync(wdev->dev);-returnnonseekable_open(inode,file);}
@@ -236,18 +226,15 @@ static long omap_wdt_ioctl(struct file *file, unsigned int cmd,(int__user*)arg);returnput_user(0,(int__user*)arg);caseWDIOC_KEEPALIVE:-pm_runtime_get_sync(wdev->dev);spin_lock(&wdt_lock);omap_wdt_ping(wdev);spin_unlock(&wdt_lock);-pm_runtime_put_sync(wdev->dev);return0;caseWDIOC_SETTIMEOUT:if(get_user(new_margin,(int__user*)arg))return-EFAULT;omap_wdt_adjust_timeout(new_margin);-pm_runtime_get_sync(wdev->dev);spin_lock(&wdt_lock);omap_wdt_disable(wdev);omap_wdt_set_timeout(wdev);
@@ -255,7 +242,6 @@ static long omap_wdt_ioctl(struct file *file, unsigned int cmd,omap_wdt_ping(wdev);spin_unlock(&wdt_lock);-pm_runtime_put_sync(wdev->dev);/* Fall */caseWDIOC_GETTIMEOUT:returnput_user(timer_margin,(int__user*)arg);
(You should cc linux-omap too. CC'ing linux-omap)
On Mon, Jun 18, 2012 at 10:53 AM, Lokesh Vutla [off-list ref] wrote:
quoted hunk
OMAP watchdong driver is adapted to runtime PM like a general device
driver but it is not appropriate. It is causing couple of functional
issues.
1. On OMAP4 SYSCLK can't be gated, because of issue with WDTIMER2 module,
which constantly stays in "in transition" state. Value of register
CM_WKUP_WDTIMER2_CLKCTRL is always 0x00010000 in this case.
Issue occurs immediately after first idle, when hwmod framework tries
to disable WDTIMER2 functional clock - "wd_timer2_fck". After this
module falls to "in transition" state, and SYSCLK gating is blocked.
2. Due to runtime PM, watchdog timer may be completely disabled.
In current code base watchdog timer is not disabled only because of
issue 1. Otherwise state of WDTIMER2 module will be "Disabled", and there
will be no interrupts from omap_wdt. In other words watchdog will not
work at all.
Watchdong is a special IP and it should not be disabled otherwise
purpose of it itself is defeated. Watchdog functional clock should
never be disabled. This patch updates the runtime PM handling in
driver so that runtime PM is limited only during probe/shutdown
and suspend/resume.
The patch fixes issue 1 and 2
Signed-off-by: Lokesh Vutla <redacted>
Acked-by: Santosh Shilimkar <redacted>
Cc: Wim Van Sebroeck <redacted>
---
Tested on OMAP4430SDP.
Issue #1 can be easily reproduced on mainline kernel.
- Take latest mainline kernel and create uImage using omap2plus_defconfig
- Write a program to open watchdog,like
void main(void)
{
? ? ? ?int fd = open("/dev/watchdog", O_WRONLY);
? ? ? ?if (fd == -1) {
? ? ? ? ? ? ? ?perror("Watchdog device interface is not available!\n");
? ? ? ?}
}
- Build with arm compiler and copy it to your file syatem
- Boot the image and run the executable.
?drivers/watchdog/omap_wdt.c | ? 17 -----------------
?1 files changed, 0 insertions(+), 17 deletions(-)
Gentle ping on this....
Thanks n regards
Lokesh Vutla
LDC MPUSS Platform Team
On Mon, Jun 18, 2012 at 11:11 AM, Shilimkar, Santosh <
santosh.shilimkar@ti.com> wrote:
(You should cc linux-omap too. CC'ing linux-omap)
On Mon, Jun 18, 2012 at 10:53 AM, Lokesh Vutla [off-list ref] wrote:
quoted
OMAP watchdong driver is adapted to runtime PM like a general device
driver but it is not appropriate. It is causing couple of functional
issues.
1. On OMAP4 SYSCLK can't be gated, because of issue with WDTIMER2 module,
which constantly stays in "in transition" state. Value of register
CM_WKUP_WDTIMER2_CLKCTRL is always 0x00010000 in this case.
Issue occurs immediately after first idle, when hwmod framework tries
to disable WDTIMER2 functional clock - "wd_timer2_fck". After this
module falls to "in transition" state, and SYSCLK gating is blocked.
2. Due to runtime PM, watchdog timer may be completely disabled.
In current code base watchdog timer is not disabled only because of
issue 1. Otherwise state of WDTIMER2 module will be "Disabled", and there
will be no interrupts from omap_wdt. In other words watchdog will not
work at all.
Watchdong is a special IP and it should not be disabled otherwise
purpose of it itself is defeated. Watchdog functional clock should
never be disabled. This patch updates the runtime PM handling in
driver so that runtime PM is limited only during probe/shutdown
and suspend/resume.
The patch fixes issue 1 and 2
Signed-off-by: Lokesh Vutla <redacted>
Acked-by: Santosh Shilimkar <redacted>
Cc: Wim Van Sebroeck <redacted>
---
Tested on OMAP4430SDP.
Issue #1 can be easily reproduced on mainline kernel.
- Take latest mainline kernel and create uImage using omap2plus_defconfig
- Write a program to open watchdog,like
void main(void)
{
int fd = open("/dev/watchdog", O_WRONLY);
if (fd == -1) {
perror("Watchdog device interface is not available!\n");
}
}
- Build with arm compiler and copy it to your file syatem
- Boot the image and run the executable.
drivers/watchdog/omap_wdt.c | 17 -----------------
1 files changed, 0 insertions(+), 17 deletions(-)
Wim,
On Mon, Jun 18, 2012 at 11:11 AM, Shilimkar, Santosh
[off-list ref] wrote:
(You should cc linux-omap too. CC'ing linux-omap)
On Mon, Jun 18, 2012 at 10:53 AM, Lokesh Vutla [off-list ref] wrote:
quoted
OMAP watchdong driver is adapted to runtime PM like a general device
driver but it is not appropriate. It is causing couple of functional
issues.
1. On OMAP4 SYSCLK can't be gated, because of issue with WDTIMER2 module,
which constantly stays in "in transition" state. Value of register
CM_WKUP_WDTIMER2_CLKCTRL is always 0x00010000 in this case.
Issue occurs immediately after first idle, when hwmod framework tries
to disable WDTIMER2 functional clock - "wd_timer2_fck". After this
module falls to "in transition" state, and SYSCLK gating is blocked.
2. Due to runtime PM, watchdog timer may be completely disabled.
In current code base watchdog timer is not disabled only because of
issue 1. Otherwise state of WDTIMER2 module will be "Disabled", and there
will be no interrupts from omap_wdt. In other words watchdog will not
work at all.
Watchdong is a special IP and it should not be disabled otherwise
purpose of it itself is defeated. Watchdog functional clock should
never be disabled. This patch updates the runtime PM handling in
driver so that runtime PM is limited only during probe/shutdown
and suspend/resume.
The patch fixes issue 1 and 2
Signed-off-by: Lokesh Vutla <redacted>
Acked-by: Santosh Shilimkar <redacted>
Cc: Wim Van Sebroeck <redacted>
---
Any comments on this patch ? If not, can you please
queue this up for the 3.5-rc?
Regards
Santosh
Hi,
On Mon, Jun 18, 2012 at 10:53:16, Vutla, Lokesh wrote:
OMAP watchdong driver is adapted to runtime PM like a general device
driver but it is not appropriate. It is causing couple of functional
issues.
A few questions based on the description given in the commit message.
1. On OMAP4 SYSCLK can't be gated, because of issue with WDTIMER2 module,
which constantly stays in "in transition" state. Value of register
CM_WKUP_WDTIMER2_CLKCTRL is always 0x00010000 in this case.
Issue occurs immediately after first idle, when hwmod framework tries
to disable WDTIMER2 functional clock - "wd_timer2_fck". After this
module falls to "in transition" state, and SYSCLK gating is blocked.
From what I know, a value of 0x00010000 for timers (WDT or DMTIMERs)
indicates that the iclk is gated but the fclk is running. In fact,
if the IP supports swakeup mechanism this is the value expected in the
*_CLKCTRL registers of the timers for the swakeup to work.
Sounds like on OMAP4 the WDT needs to be stopped first and then the
PRCM idle request sent otherwise SYSCLK gating will be blocked.
2. Due to runtime PM, watchdog timer may be completely disabled.
In current code base watchdog timer is not disabled only because of
issue 1. Otherwise state of WDTIMER2 module will be "Disabled", and there
will be no interrupts from omap_wdt. In other words watchdog will not
work at all.
But the current driver doesn't make use of any interrupts, right?
If the WDT was never started, runtime PM handling for the WDT should be
able to get the IP to a "disabled" state. Is the issue over here due
to the WDT counter incrementing and still the PRCM idle request being
sent for disabling it? If so, perhaps a better solution would be have
a custom runtime PM handling for WDT which checks if the counter
is incrementing or not. If it is not incrementing then it can just
go ahead and disable the clocks. However, if the counter is incrementing
then the runtime PM activities on the driver should be forbidden till
an entry to a low power state where SYSCLK needs to be gated is required.
Regards,
Vaibhav B.
quoted hunk
Watchdong is a special IP and it should not be disabled otherwise
purpose of it itself is defeated. Watchdog functional clock should
never be disabled. This patch updates the runtime PM handling in
driver so that runtime PM is limited only during probe/shutdown
and suspend/resume.
The patch fixes issue 1 and 2
Signed-off-by: Lokesh Vutla <redacted>
Acked-by: Santosh Shilimkar <redacted>
Cc: Wim Van Sebroeck <redacted>
---
Tested on OMAP4430SDP.
Issue #1 can be easily reproduced on mainline kernel.
- Take latest mainline kernel and create uImage using omap2plus_defconfig
- Write a program to open watchdog,like
void main(void)
{
int fd = open("/dev/watchdog", O_WRONLY);
if (fd == -1) {
perror("Watchdog device interface is not available!\n");
}
}
- Build with arm compiler and copy it to your file syatem
- Boot the image and run the executable.
drivers/watchdog/omap_wdt.c | 17 -----------------
1 files changed, 0 insertions(+), 17 deletions(-)
@@ -166,8 +162,6 @@ static int omap_wdt_open(struct inode *inode, struct file *file)omap_wdt_ping(wdev);/* trigger loading of new timeout value */omap_wdt_enable(wdev);-pm_runtime_put_sync(wdev->dev);-returnnonseekable_open(inode,file);}
@@ -236,18 +226,15 @@ static long omap_wdt_ioctl(struct file *file, unsigned int cmd,(int__user*)arg);returnput_user(0,(int__user*)arg);caseWDIOC_KEEPALIVE:-pm_runtime_get_sync(wdev->dev);spin_lock(&wdt_lock);omap_wdt_ping(wdev);spin_unlock(&wdt_lock);-pm_runtime_put_sync(wdev->dev);return0;caseWDIOC_SETTIMEOUT:if(get_user(new_margin,(int__user*)arg))return-EFAULT;omap_wdt_adjust_timeout(new_margin);-pm_runtime_get_sync(wdev->dev);spin_lock(&wdt_lock);omap_wdt_disable(wdev);omap_wdt_set_timeout(wdev);
@@ -255,7 +242,6 @@ static long omap_wdt_ioctl(struct file *file, unsigned int cmd,omap_wdt_ping(wdev);spin_unlock(&wdt_lock);-pm_runtime_put_sync(wdev->dev);/* Fall */caseWDIOC_GETTIMEOUT:returnput_user(timer_margin,(int__user*)arg);
@@ -419,7 +403,6 @@ static int omap_wdt_resume(struct platform_device *pdev)pm_runtime_get_sync(wdev->dev);omap_wdt_enable(wdev);omap_wdt_ping(wdev);-pm_runtime_put_sync(wdev->dev);}return0;
--
1.7.5.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
(+ linux-omap, linux-watchdog)
Vaibhav,
On Thu, Jul 5, 2012 at 8:06 PM, Bedia, Vaibhav [off-list ref] wrote:
Hi,
On Mon, Jun 18, 2012 at 10:53:16, Vutla, Lokesh wrote:
quoted
OMAP watchdong driver is adapted to runtime PM like a general device
driver but it is not appropriate. It is causing couple of functional
issues.
A few questions based on the description given in the commit message.
quoted
1. On OMAP4 SYSCLK can't be gated, because of issue with WDTIMER2 module,
which constantly stays in "in transition" state. Value of register
CM_WKUP_WDTIMER2_CLKCTRL is always 0x00010000 in this case.
Issue occurs immediately after first idle, when hwmod framework tries
to disable WDTIMER2 functional clock - "wd_timer2_fck". After this
module falls to "in transition" state, and SYSCLK gating is blocked.
From what I know, a value of 0x00010000 for timers (WDT or DMTIMERs)
indicates that the iclk is gated but the fclk is running. In fact,
if the IP supports swakeup mechanism this is the value expected in the
*_CLKCTRL registers of the timers for the swakeup to work.
Nope. That case will be 0x00020000
Read 0x1: Module is performing transition: wakeup, or
sleep, or sleep abortion
Read 0x2: Module is in idle mode (only INTRCONN part).
It is functional if using separate functional clock
Read 0x3: Module is disabled and cannot be accessed
Sounds like on OMAP4 the WDT needs to be stopped first and then the
PRCM idle request sent otherwise SYSCLK gating will be blocked.
Any module stuck in-transition will get the clock-domain from idle.
quoted
2. Due to runtime PM, watchdog timer may be completely disabled.
In current code base watchdog timer is not disabled only because of
issue 1. Otherwise state of WDTIMER2 module will be "Disabled", and there
will be no interrupts from omap_wdt. In other words watchdog will not
work at all.
But the current driver doesn't make use of any interrupts, right?
How is the interrupt related. You enable that when you enable WDT
petting using delay_interrupt()
If the WDT was never started, runtime PM handling for the WDT should be
able to get the IP to a "disabled" state. Is the issue over here due
to the WDT counter incrementing and still the PRCM idle request being
sent for disabling it? If so, perhaps a better solution would be have
a custom runtime PM handling for WDT which checks if the counter
is incrementing or not. If it is not incrementing then it can just
go ahead and disable the clocks. However, if the counter is incrementing
then the runtime PM activities on the driver should be forbidden till
an entry to a low power state where SYSCLK needs to be gated is required.
If you look at the test case mentioned, the watchdong is started. Your
first observation is not as per the hardware behavior, so other points
becomes not relevant.
Regards
Santosh
Hi Santosh,
On Fri, Jul 06, 2012 at 12:51:03, Shilimkar, Santosh wrote:
[...]
quoted
A few questions based on the description given in the commit message.
quoted
1. On OMAP4 SYSCLK can't be gated, because of issue with WDTIMER2 module,
which constantly stays in "in transition" state. Value of register
CM_WKUP_WDTIMER2_CLKCTRL is always 0x00010000 in this case.
Issue occurs immediately after first idle, when hwmod framework tries
to disable WDTIMER2 functional clock - "wd_timer2_fck". After this
module falls to "in transition" state, and SYSCLK gating is blocked.
From what I know, a value of 0x00010000 for timers (WDT or DMTIMERs)
indicates that the iclk is gated but the fclk is running. In fact,
if the IP supports swakeup mechanism this is the value expected in the
*_CLKCTRL registers of the timers for the swakeup to work.
Nope. That case will be 0x00020000
Read 0x1: Module is performing transition: wakeup, or
sleep, or sleep abortion
Read 0x2: Module is in idle mode (only INTRCONN part).
It is functional if using separate functional clock
Read 0x3: Module is disabled and cannot be accessed
What you mentioned is obviously correct :)
I somehow recall seeing something else but mostly likely I am wrong here.
quoted
Sounds like on OMAP4 the WDT needs to be stopped first and then the
PRCM idle request sent otherwise SYSCLK gating will be blocked.
Any module stuck in-transition will get the clock-domain from idle.
Yes agreed.
quoted
quoted
2. Due to runtime PM, watchdog timer may be completely disabled.
In current code base watchdog timer is not disabled only because of
issue 1. Otherwise state of WDTIMER2 module will be "Disabled", and there
will be no interrupts from omap_wdt. In other words watchdog will not
work at all.
But the current driver doesn't make use of any interrupts, right?
How is the interrupt related. You enable that when you enable WDT
petting using delay_interrupt()
Even I don't understand the interrupt part here.
quoted
If the WDT was never started, runtime PM handling for the WDT should be
able to get the IP to a "disabled" state. Is the issue over here due
to the WDT counter incrementing and still the PRCM idle request being
sent for disabling it? If so, perhaps a better solution would be have
a custom runtime PM handling for WDT which checks if the counter
is incrementing or not. If it is not incrementing then it can just
go ahead and disable the clocks. However, if the counter is incrementing
then the runtime PM activities on the driver should be forbidden till
an entry to a low power state where SYSCLK needs to be gated is required.
If you look at the test case mentioned, the watchdong is started. Your
first observation is not as per the hardware behavior, so other points
becomes not relevant.
Ok. I'll double-check my observations on AM335x on Monday.
Regards,
Vaibhav B.
OMAP watchdong driver is adapted to runtime PM like a general device
driver but it is not appropriate. It is causing couple of functional
issues.
1. On OMAP4 SYSCLK can't be gated, because of issue with WDTIMER2 module,
which constantly stays in "in transition" state. Value of register
CM_WKUP_WDTIMER2_CLKCTRL is always 0x00010000 in this case.
Issue occurs immediately after first idle, when hwmod framework tries
to disable WDTIMER2 functional clock - "wd_timer2_fck". After this
module falls to "in transition" state, and SYSCLK gating is blocked.
2. Due to runtime PM, watchdog timer may be completely disabled.
In current code base watchdog timer is not disabled only because of
issue 1. Otherwise state of WDTIMER2 module will be "Disabled", and there
will be no interrupts from omap_wdt. In other words watchdog will not
work at all.
Watchdong is a special IP and it should not be disabled otherwise
purpose of it itself is defeated. Watchdog functional clock should
never be disabled. This patch updates the runtime PM handling in
driver so that runtime PM is limited only during probe/shutdown
and suspend/resume.
The patch fixes issue 1 and 2
Signed-off-by: Lokesh Vutla<redacted>
Acked-by: Santosh Shilimkar<redacted>
Cc: Wim Van Sebroeck<redacted>
---
Tested on OMAP4430SDP.
Issue #1 can be easily reproduced on mainline kernel.
- Take latest mainline kernel and create uImage using omap2plus_defconfig
- Write a program to open watchdog,like
void main(void)
{
int fd = open("/dev/watchdog", O_WRONLY);
if (fd == -1) {
perror("Watchdog device interface is not available!\n");
}
}
Hello, Lokesh,
One question: Does "echo 1 > /dev/watchdog" work well?
Regards,
Zumeng
quoted hunk
- Build with arm compiler and copy it to your file syatem
- Boot the image and run the executable.
drivers/watchdog/omap_wdt.c | 17 -----------------
1 files changed, 0 insertions(+), 17 deletions(-)
@@ -166,8 +162,6 @@ static int omap_wdt_open(struct inode *inode, struct file *file)omap_wdt_ping(wdev);/* trigger loading of new timeout value */omap_wdt_enable(wdev);-pm_runtime_put_sync(wdev->dev);-returnnonseekable_open(inode,file);}
@@ -236,18 +226,15 @@ static long omap_wdt_ioctl(struct file *file, unsigned int cmd,(int__user*)arg);returnput_user(0,(int__user*)arg);caseWDIOC_KEEPALIVE:-pm_runtime_get_sync(wdev->dev);spin_lock(&wdt_lock);omap_wdt_ping(wdev);spin_unlock(&wdt_lock);-pm_runtime_put_sync(wdev->dev);return0;caseWDIOC_SETTIMEOUT:if(get_user(new_margin,(int__user*)arg))return-EFAULT;omap_wdt_adjust_timeout(new_margin);-pm_runtime_get_sync(wdev->dev);spin_lock(&wdt_lock);omap_wdt_disable(wdev);omap_wdt_set_timeout(wdev);
@@ -255,7 +242,6 @@ static long omap_wdt_ioctl(struct file *file, unsigned int cmd,omap_wdt_ping(wdev);spin_unlock(&wdt_lock);-pm_runtime_put_sync(wdev->dev);/* Fall */caseWDIOC_GETTIMEOUT:returnput_user(timer_margin,(int__user*)arg);