On Thu, 04 Jun 2009 22:24:56 +0200
matthieu castet <castet.matthieu@xxxxxxx> wrote:
>
>
> This add watchdog driver for broadcom 47xx device.
> It uses the ssb subsytem to access embeded watchdog device.
>
> Because the watchdog timeout is very short (about 2s), a soft timer is used
> to increase the watchdog period.
>
> Note : A patch for exporting the ssb_watchdog_timer_set will
> be submitted on next linux-mips merge. Without this patch it can't
> be build as a module.
>
>
> ...
>
> --- linux-2.6.orig/drivers/watchdog/Kconfig 2009-05-25 22:22:02.000000000 +0200
> +++ linux-2.6/drivers/watchdog/Kconfig 2009-05-25 22:26:06.000000000 +0200
> @@ -764,6 +764,12 @@
> help
> Hardware driver for the built-in watchdog timer on TXx9 MIPS SoCs.
>
> +config BCM47XX_WDT
> + tristate "Broadcom BCM47xx Watchdog Timer"
> + depends on BCM47XX
> + help
> + Hardware driver for the Broadcom BCM47xx Watchog Timer.
> +
Please indent the kconfig body with a single tab character.
>
> ...
>
> +#define DRV_NAME "bcm47xx_wdt"
> +
> +#define WDT_DEFAULT_TIME 30 /* seconds */
> +#define WDT_MAX_TIME 256 /* seconds */
> +
> +static int wdt_time = WDT_DEFAULT_TIME;
> +static int nowayout = WATCHDOG_NOWAYOUT;
> +
> +module_param(wdt_time, int, 0);
> +MODULE_PARM_DESC(wdt_time, "Watchdog time in seconds. (default="
> + __MODULE_STRING(WDT_DEFAULT_TIME) ")");
> +
> +#ifdef CONFIG_WATCHDOG_NOWAYOUT
> +module_param(nowayout, int, 0);
> +MODULE_PARM_DESC(nowayout,
> + "Watchdog cannot be stopped once started (default="
> + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> +#endif
hm, now what's happening with the third arg to module_param()?
> +static struct platform_device *bcm47xx_wdt_platform_device;
> +
> +static unsigned long bcm47xx_wdt_busy;
> +static char expect_release;
> +static struct timer_list wdt_timer;
> +static atomic_t ticks;
> +
> +static inline void bcm47xx_wdt_hw_start(void)
> +{
> + /* this is 2,5s on 100Mhz clock and 2s on 133 Mhz */
> + ssb_watchdog_timer_set(&ssb_bcm47xx, 0xfffffff);
> +}
> +
> +static inline int bcm47xx_wdt_hw_stop(void)
> +{
> + return ssb_watchdog_timer_set(&ssb_bcm47xx, 0);
> +}
> +
> +static void bcm47xx_timer_tick(unsigned long unused)
> +{
> + if(!atomic_dec_and_test(&ticks)) {
Please pass this patch (and all others) through scripts/checkpatch.pl
and review the resulting output.
> + bcm47xx_wdt_hw_start();
> + mod_timer(&wdt_timer, jiffies + HZ);
> + }
> + else {
> + printk(KERN_CRIT PFX "Watchdog will fire soon!!!.\n");
> + }
> +}
> +
> +static inline void bcm47xx_wdt_pet(void)
> +{
> + atomic_set(&ticks, wdt_time);
> +}
What does "pet" stand for?
> +static void bcm47xx_wdt_start(void)
> +{
> + bcm47xx_wdt_pet();
> + bcm47xx_timer_tick(0);
> +}
> +
> +static void bcm47xx_wdt_pause(void)
> +{
> + del_timer(&wdt_timer);
Should this be del_timer_sync()? The timer callback can still be
executing after del_timer() returns.
> + bcm47xx_wdt_hw_stop();
> +}
> +
> +static void bcm47xx_wdt_stop(void)
> +{
> + bcm47xx_wdt_pause();
> +}
> +
> +static int bcm47xx_wdt_settimeout(int new_time)
> +{
> + if ((new_time <= 0) || (new_time > WDT_MAX_TIME))
> + return -EINVAL;
> +
> + wdt_time = new_time;
> + return 0;
> +}
> +
> +static int bcm47xx_wdt_open(struct inode *inode, struct file *file)
> +{
> + if (test_and_set_bit(0, &bcm47xx_wdt_busy))
> + return -EBUSY;
> +
> + bcm47xx_wdt_start();
> + return nonseekable_open(inode, file);
> +}
> +
> +static int bcm47xx_wdt_release(struct inode *inode, struct file *file)
> +{
> + if (expect_release == 42) {
> + bcm47xx_wdt_stop();
> + } else {
> + printk(KERN_CRIT DRV_NAME ": Unexpected close, not stopping watchdog!\n");
Can this happen?
> + bcm47xx_wdt_start();
> + }
> +
> + clear_bit(0, &bcm47xx_wdt_busy);
> + expect_release = 0;
> + return 0;
> +}
> +
>
> ...
>
