Gentle ping on this....<br clear="all"><br><br><br>Thanks n regards<br>Lokesh Vutla<div>LDC MPUSS Platform Team</div><br>
<br><br><div class="gmail_quote">On Mon, Jun 18, 2012 at 11:11 AM, Shilimkar, Santosh <span dir="ltr"><<a href="mailto:santosh.shilimkar@ti.com" target="_blank">santosh.shilimkar@ti.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

(You should cc linux-omap too.  CC'ing linux-omap)<br>
<div><div class="h5"><br>
On Mon, Jun 18, 2012 at 10:53 AM, Lokesh Vutla <<a href="mailto:lokeshvutla@ti.com">lokeshvutla@ti.com</a>> wrote:<br>
> OMAP watchdong driver is adapted to runtime PM like a general device<br>
> driver but it is not appropriate. It is causing couple of functional<br>
> issues.<br>
><br>
> 1. On OMAP4 SYSCLK can't be gated, because of issue with WDTIMER2 module,<br>
> which constantly stays in "in transition" state. Value of register<br>
> CM_WKUP_WDTIMER2_CLKCTRL is always 0x00010000 in this case.<br>
> Issue occurs immediately after first idle, when hwmod framework tries<br>
> to disable WDTIMER2 functional clock - "wd_timer2_fck". After this<br>
> module falls to "in transition" state, and SYSCLK gating is blocked.<br>
><br>
> 2. Due to runtime PM, watchdog timer may be completely disabled.<br>
> In current code base watchdog timer is not disabled only because of<br>
> issue 1. Otherwise state of WDTIMER2 module will be "Disabled", and there<br>
> will be no interrupts from omap_wdt. In other words watchdog will not<br>
> work at all.<br>
><br>
> Watchdong is a special IP and it should not be disabled otherwise<br>
> purpose of it itself is defeated. Watchdog functional clock should<br>
> never be disabled. This patch updates the runtime PM handling in<br>
> driver so that runtime PM is limited only during probe/shutdown<br>
> and suspend/resume.<br>
><br>
> The patch fixes issue 1 and 2<br>
><br>
> Signed-off-by: Lokesh Vutla <<a href="mailto:lokeshvutla@ti.com">lokeshvutla@ti.com</a>><br>
> Acked-by: Santosh Shilimkar <<a href="mailto:santosh.shilimkar@ti.com">santosh.shilimkar@ti.com</a>><br>
> Cc: Wim Van Sebroeck <<a href="mailto:wim@iguana.be">wim@iguana.be</a>><br>
> ---<br>
> Tested on OMAP4430SDP.<br>
><br>
> Issue #1 can be easily reproduced on mainline kernel.<br>
> - Take latest mainline kernel and create uImage using omap2plus_defconfig<br>
> - Write a program to open watchdog,like<br>
> void main(void)<br>
> {<br>
>        int fd = open("/dev/watchdog", O_WRONLY);<br>
><br>
>        if (fd == -1) {<br>
>                perror("Watchdog device interface is not available!\n");<br>
>        }<br>
> }<br>
> - Build with arm compiler and copy it to your file syatem<br>
> - Boot the image and run the executable.<br>
><br>
><br>
>  drivers/watchdog/omap_wdt.c |   17 -----------------<br>
>  1 files changed, 0 insertions(+), 17 deletions(-)<br>
><br>
> diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c<br>
> index 8285d65..27ab8db 100644<br>
> --- a/drivers/watchdog/omap_wdt.c<br>
> +++ b/drivers/watchdog/omap_wdt.c<br>
> @@ -126,8 +126,6 @@ static void omap_wdt_set_timeout(struct omap_wdt_dev *wdev)<br>
>        u32 pre_margin = GET_WLDR_VAL(timer_margin);<br>
>        void __iomem *base = wdev->base;<br>
><br>
> -       pm_runtime_get_sync(wdev->dev);<br>
> -<br>
>        /* just count up at 32 KHz */<br>
>        while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x04)<br>
>                cpu_relax();<br>
> @@ -135,8 +133,6 @@ static void omap_wdt_set_timeout(struct omap_wdt_dev *wdev)<br>
>        __raw_writel(pre_margin, base + OMAP_WATCHDOG_LDR);<br>
>        while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x04)<br>
>                cpu_relax();<br>
> -<br>
> -       pm_runtime_put_sync(wdev->dev);<br>
>  }<br>
><br>
>  /*<br>
> @@ -166,8 +162,6 @@ static int omap_wdt_open(struct inode *inode, struct file *file)<br>
>        omap_wdt_ping(wdev); /* trigger loading of new timeout value */<br>
>        omap_wdt_enable(wdev);<br>
><br>
> -       pm_runtime_put_sync(wdev->dev);<br>
> -<br>
>        return nonseekable_open(inode, file);<br>
>  }<br>
><br>
> @@ -179,8 +173,6 @@ static int omap_wdt_release(struct inode *inode, struct file *file)<br>
>         *      Shut off the timer unless NOWAYOUT is defined.<br>
>         */<br>
>  #ifndef CONFIG_WATCHDOG_NOWAYOUT<br>
> -       pm_runtime_get_sync(wdev->dev);<br>
> -<br>
>        omap_wdt_disable(wdev);<br>
><br>
>        pm_runtime_put_sync(wdev->dev);<br>
> @@ -199,11 +191,9 @@ static ssize_t omap_wdt_write(struct file *file, const char __user *data,<br>
><br>
>        /* Refresh LOAD_TIME. */<br>
>        if (len) {<br>
> -               pm_runtime_get_sync(wdev->dev);<br>
>                spin_lock(&wdt_lock);<br>
>                omap_wdt_ping(wdev);<br>
>                spin_unlock(&wdt_lock);<br>
> -               pm_runtime_put_sync(wdev->dev);<br>
>        }<br>
>        return len;<br>
>  }<br>
> @@ -236,18 +226,15 @@ static long omap_wdt_ioctl(struct file *file, unsigned int cmd,<br>
>                                        (int __user *)arg);<br>
>                return put_user(0, (int __user *)arg);<br>
>        case WDIOC_KEEPALIVE:<br>
> -               pm_runtime_get_sync(wdev->dev);<br>
>                spin_lock(&wdt_lock);<br>
>                omap_wdt_ping(wdev);<br>
>                spin_unlock(&wdt_lock);<br>
> -               pm_runtime_put_sync(wdev->dev);<br>
>                return 0;<br>
>        case WDIOC_SETTIMEOUT:<br>
>                if (get_user(new_margin, (int __user *)arg))<br>
>                        return -EFAULT;<br>
>                omap_wdt_adjust_timeout(new_margin);<br>
><br>
> -               pm_runtime_get_sync(wdev->dev);<br>
>                spin_lock(&wdt_lock);<br>
>                omap_wdt_disable(wdev);<br>
>                omap_wdt_set_timeout(wdev);<br>
> @@ -255,7 +242,6 @@ static long omap_wdt_ioctl(struct file *file, unsigned int cmd,<br>
><br>
>                omap_wdt_ping(wdev);<br>
>                spin_unlock(&wdt_lock);<br>
> -               pm_runtime_put_sync(wdev->dev);<br>
>                /* Fall */<br>
>        case WDIOC_GETTIMEOUT:<br>
>                return put_user(timer_margin, (int __user *)arg);<br>
> @@ -363,7 +349,6 @@ static void omap_wdt_shutdown(struct platform_device *pdev)<br>
>        struct omap_wdt_dev *wdev = platform_get_drvdata(pdev);<br>
><br>
>        if (wdev->omap_wdt_users) {<br>
> -               pm_runtime_get_sync(wdev->dev);<br>
>                omap_wdt_disable(wdev);<br>
>                pm_runtime_put_sync(wdev->dev);<br>
>        }<br>
> @@ -403,7 +388,6 @@ static int omap_wdt_suspend(struct platform_device *pdev, pm_message_t state)<br>
>        struct omap_wdt_dev *wdev = platform_get_drvdata(pdev);<br>
><br>
>        if (wdev->omap_wdt_users) {<br>
> -               pm_runtime_get_sync(wdev->dev);<br>
>                omap_wdt_disable(wdev);<br>
>                pm_runtime_put_sync(wdev->dev);<br>
>        }<br>
> @@ -419,7 +403,6 @@ static int omap_wdt_resume(struct platform_device *pdev)<br>
>                pm_runtime_get_sync(wdev->dev);<br>
>                omap_wdt_enable(wdev);<br>
>                omap_wdt_ping(wdev);<br>
> -               pm_runtime_put_sync(wdev->dev);<br>
>        }<br>
><br>
>        return 0;<br>
> --<br>
> 1.7.5.4<br>
><br>
><br>
</div></div>> _______________________________________________<br>
> linux-arm-kernel mailing list<br>
> <a href="mailto:linux-arm-kernel@lists.infradead.org">linux-arm-kernel@lists.infradead.org</a><br>
> <a href="http://lists.infradead.org/mailman/listinfo/linux-arm-kernel" target="_blank">http://lists.infradead.org/mailman/listinfo/linux-arm-kernel</a><br>
</blockquote></div><br>