RE: [v2 PATCH 2/8] watchdog/at91sam9_wdt: Convert to use the watchdog framework

2012-12-05 Thread Yang, Wenyou
Hi Florian,

> -Original Message-
> From: Florian Fainelli [mailto:f.faine...@gmail.com] On Behalf Of Florian 
> Fainelli
> Sent: 2012年12月5日 18:48
> To: Yang, Wenyou
> Cc: linux-arm-ker...@lists.infradead.org; Ferre, Nicolas; 
> plagn...@jcrosoft.com; Lin,
> JM; w...@iguana.be; linux-watch...@vger.kernel.org; 
> linux-kernel@vger.kernel.org
> Subject: Re: [v2 PATCH 2/8] watchdog/at91sam9_wdt: Convert to use the watchdog
> framework
> 
> Hello Wenyou,
> 
> On Wednesday 05 December 2012 09:34:21 Wenyou Yang wrote:
> > According to the kernel document: convert_drivers_to_kernel_api.txt,
> > remove the file_operations struct, miscdevice, and obsolete includes
> >
> > Since the at91sam watchdog inherent characteristics, add the watchdog
> > operations: at91wdt_start, at91wdt_stop and at91wdt_ping.
> >
> 
> [snip]
> 
> >
> > +static inline bool watchdog_is_open(struct watchdog_device *wddev)
> > +{
> > +   return test_bit(WDOG_DEV_OPEN, &wddev->status);
> > +}
> 
> This helper should be moved to include/linux/watchdog.h as it can be useful
> for other watchdog drivers as well.

Thanks, I will move it in next version.

> --
> Florian

Best Regards
Wenyou Yang
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�&j:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

Re: [v2 PATCH 2/8] watchdog/at91sam9_wdt: Convert to use the watchdog framework

2012-12-05 Thread Florian Fainelli
Hello Wenyou,

On Wednesday 05 December 2012 09:34:21 Wenyou Yang wrote:
> According to the kernel document: convert_drivers_to_kernel_api.txt,
> remove the file_operations struct, miscdevice, and obsolete includes
> 
> Since the at91sam watchdog inherent characteristics, add the watchdog
> operations: at91wdt_start, at91wdt_stop and at91wdt_ping.
> 

[snip]

>  
> +static inline bool watchdog_is_open(struct watchdog_device *wddev)
> +{
> + return test_bit(WDOG_DEV_OPEN, &wddev->status);
> +}

This helper should be moved to include/linux/watchdog.h as it can be useful
for other watchdog drivers as well. 
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[v2 PATCH 2/8] watchdog/at91sam9_wdt: Convert to use the watchdog framework

2012-12-04 Thread Wenyou Yang
According to the kernel document: convert_drivers_to_kernel_api.txt,
remove the file_operations struct, miscdevice, and obsolete includes

Since the at91sam watchdog inherent characteristics, add the watchdog
operations: at91wdt_start, at91wdt_stop and at91wdt_ping.

Signed-off-by: Wenyou Yang 
Cc: w...@iguana.be
Cc: linux-watch...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/watchdog/at91sam9_wdt.c |  203 +++
 1 file changed, 75 insertions(+), 128 deletions(-)

diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
index f10a897..92467d4 100644
--- a/drivers/watchdog/at91sam9_wdt.c
+++ b/drivers/watchdog/at91sam9_wdt.c
@@ -13,16 +13,17 @@
  * The Watchdog Timer Mode Register can be only written to once. If the
  * timeout need to be set from Linux, be sure that the bootstrap or the
  * bootloader doesn't write to this register.
+ * The Watchdog Timer default is running with maximum counter value
+ * (WDV=0xfff) at reset, i.e., at power-up. It MUST be either disabled
+ * or be reprogrammed within the maxinum margin(16s).
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include 
-#include 
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -31,7 +32,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 #include "at91sam9_wdt.h"
@@ -65,8 +65,6 @@ module_param(nowayout, bool, 0);
 MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started "
"(default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
 
-static void at91_ping(unsigned long data);
-
 struct at91wdt_drvdata {
void __iomem*phybase;
boolis_enable;  /* indicate if the watchdog is eabled */
@@ -88,6 +86,11 @@ static inline void wdt_write(struct at91wdt_drvdata 
*driver_data,
writel_relaxed((val), driver_data->phybase + field);
 }
 
+static inline bool watchdog_is_open(struct watchdog_device *wddev)
+{
+   return test_bit(WDOG_DEV_OPEN, &wddev->status);
+}
+
 /*
  * Reload the watchdog timer.  (ie, pat the watchdog)
  */
@@ -99,7 +102,7 @@ static inline void at91_wdt_reset(struct at91wdt_drvdata 
*driver_data)
 /*
  * Timer tick
  */
-static void at91_ping(unsigned long data)
+static void at91wdt_timer_tick(unsigned long data)
 {
struct watchdog_device *wddev = (struct watchdog_device *)data;
struct at91wdt_drvdata *driver_data = watchdog_get_drvdata(wddev);
@@ -107,45 +110,30 @@ static void at91_ping(unsigned long data)
if (time_before(jiffies, driver_data->next_heartbeat)) {
at91_wdt_reset(driver_data);
mod_timer(&driver_data->timer, jiffies + WDT_TIMEOUT);
+
+   if (!watchdog_is_open(wddev))
+   driver_data->next_heartbeat = jiffies
+   + wddev->timeout * HZ;
} else
pr_crit("I will reset your machine !\n");
 }
 
-/*
- * Watchdog device is opened, and watchdog starts running.
- */
-static int at91_wdt_open(struct inode *inode, struct file *file)
-{
-   driver_data->next_heartbeat = jiffies + heartbeat * HZ;
-   mod_timer(&driver_data->timer, jiffies + WDT_TIMEOUT);
-
-   return nonseekable_open(inode, file);
-}
-
-/*
- * Close the watchdog device.
- */
-static int at91_wdt_close(struct inode *inode, struct file *file)
-{
-   del_timer(&driver_data->timer);
-
-   return 0;
-}
-
-/*
- * Set the watchdog time interval in 1/256Hz (write-once)
- * Counter is 12 bit.
- */
-static int at91_wdt_settimeout(unsigned int timeout)
+static int at91wdt_enable(struct watchdog_device *wddev, unsigned int timeout)
 {
+   struct at91wdt_drvdata *driver_data = watchdog_get_drvdata(wddev);
unsigned int reg;
-   unsigned int mr;
 
-   /* Check if disabled */
-   mr = wdt_read(AT91_WDT_MR);
-   if (mr & AT91_WDT_WDDIS) {
-   pr_err("sorry, watchdog is disabled\n");
-   return -EIO;
+   /*
+* Check if the watchdog is disabled,
+* if disabled, the reason is the bootstrap or the bootloader has
+* written the Watchdog Timer Mode Register to disable the
+* watchdog timer
+*/
+   reg = wdt_read(driver_data, AT91_WDT_MR);
+   if (reg & AT91_WDT_WDDIS) {
+   driver_data->is_enable = false;
+   pr_info("sorry, watchdog is disabled\n");
+   return -1;
}
 
/*
@@ -159,7 +147,9 @@ static int at91_wdt_settimeout(unsigned int timeout)
| AT91_WDT_WDDBGHLT /* disabled in debug mode */
| AT91_WDT_WDD  /* restart at any time */
| (timeout & AT91_WDT_WDV);  /* timer value */
-   wdt_write(AT91_WDT_MR, reg);
+   wdt_write(driver_data, AT91_WDT_MR, reg);
+
+   driver_data->is_enable = true;
 
return 0;
 }
@@ -170,99 +160,61 @@ static const struct watchdog_info at91_wdt_info = {