[OpenWrt-Devel] Watchdog support for AR531x and potential lockup

Sergey Korolew ds at bittu.org.ru
Mon Oct 13 03:49:59 EDT 2014


Hello !

OpenWrt already support watchdog for some Atheros devices (newer ar2315),
but still lack support for older ar531x. This topic already opened here:
https://lists.openwrt.org/pipermail/openwrt-devel/2008-April/002039.html
but without any response from devs. Hope today someone will answer :)

I also found potential lockup involving WDT for those devices
(have DWL-2100AP based ar2313a in my disposal for experiments)

1. Watchdog timer always decrement until zero, it cant be stopped at all.
2. Misc watchdog interrupt _always_ generated if timer is zero, does not
matter contents of CTRL register ! Flag in ISR register always set if
wdt timer equal zero.
3. Misc wdt interrupt flag in ISR can't be cleared until timer set to
some non-zero value !

Unfortunately code in current ar2315-wtd set timer to zero, with followed
eternal loop because ISR flag can't be cleared and interrupt routine
always called again and again (if not masked).

I added support for older ar531x (actually the same, but registers swapped,
so platform device now contain 2 mem resources instead of 1)
and add ability to turn hardware reset on during load (wdt without hardware
reset seems useless for me).

Those patches for code checking only, they not supposed to be committed !
If all ok I can create separated patches, because now watchdog support
exist in 100-board.patch (platform device) and 130-watchdog.patch.

--- ar5312.c.old        2014-10-09 20:24:22.000000000 +0400
+++ ar5312.c    2014-10-12 14:12:19.299881573 +0400
@@ -49,9 +49,10 @@
                do_IRQ(AR5312_MISC_IRQ_AHB_PROC);
        else if ((ar231x_misc_intrs & AR5312_ISR_UART0))
                do_IRQ(AR5312_MISC_IRQ_UART0);
-       else if (ar231x_misc_intrs & AR5312_ISR_WD)
+       else if (ar231x_misc_intrs & AR5312_ISR_WD) {
                do_IRQ(AR5312_MISC_IRQ_WATCHDOG);
-       else
+               ar231x_write_reg(AR5312_ISR, AR5312_ISR_WD);
+       } else
                do_IRQ(AR5312_MISC_IRQ_NONE);
 }
 
@@ -255,6 +256,31 @@
 };
 #endif
 
+static struct resource ar5312_wdt_res[] = {
+       {
+               .flags = IORESOURCE_MEM,
+               .start = AR5312_WD_TIMER,
+               .end = AR5312_WD_TIMER + 4 - 1,
+       },
+       {
+               .flags = IORESOURCE_MEM,
+               .start = AR5312_WD_CTRL,
+               .end = AR5312_WD_CTRL + 4 - 1,
+       },
+       {
+               .flags = IORESOURCE_IRQ,
+               .start = AR5312_MISC_IRQ_WATCHDOG,
+               .end = AR5312_MISC_IRQ_WATCHDOG,
+       }
+};
+
+static struct platform_device ar5312_wdt = {
+       .id = 0,
+       .name = "ar231x-wdt",
+       .resource = ar5312_wdt_res,
+       .num_resources = ARRAY_SIZE(ar5312_wdt_res)
+};
+
 /*
  * NB: This mapping size is larger than the actual flash size,
  * but this shouldn't be a problem here, because the flash
@@ -327,6 +353,7 @@
        }
 
        platform_device_register(&ar5312_physmap_flash);
+       platform_device_register(&ar5312_wdt);
 
 #ifdef CONFIG_LEDS_GPIO
        ar5312_leds[0].gpio = config->sys_led_gpio;


--- ar2315.c.old        2014-10-09 20:24:22.000000000 +0400
+++ ar2315.c    2014-10-11 11:46:09.598278049 +0400
@@ -335,7 +335,12 @@
        {
                .flags = IORESOURCE_MEM,
                .start = AR2315_WD,
-               .end = AR2315_WD + 8 - 1,
+               .end = AR2315_WD + 4 - 1,
+       },
+       {
+               .flags = IORESOURCE_MEM,
+               .start = AR2315_WDC,
+               .end = AR2315_WDC + 4 - 1,
        },
        {
                .flags = IORESOURCE_IRQ,
@@ -346,7 +351,7 @@
 
 static struct platform_device ar2315_wdt = {
        .id = 0,
-       .name = "ar2315-wdt",
+       .name = "ar231x-wdt",
        .resource = ar2315_wdt_res,
        .num_resources = ARRAY_SIZE(ar2315_wdt_res)
 };

--- ar2315-wtd.c        2014-10-09 20:24:22.745638862 +0400
+++ ar231x-wdt.c        2014-10-12 14:11:37.463673909 +0400
@@ -32,62 +32,71 @@
 #include <linux/io.h>
 #include <linux/uaccess.h>
 
-#define DRIVER_NAME    "ar2315-wdt"
+#define DRIVER_NAME    "ar231x-wdt"
 
 #define CLOCK_RATE 40000000
 #define HEARTBEAT(x) (x < 1 || x > 90 ? 20 : x)
 
-#define WDT_REG_TIMER          0x00
-#define WDT_REG_CTRL           0x04
+// comment this line if does not want WDT started during boot
+//#define WDT_START_ON_BOOT
 
 #define WDT_CTRL_ACT_NONE      0x00000000      /* No action */
 #define WDT_CTRL_ACT_NMI       0x00000001      /* NMI on watchdog */
 #define WDT_CTRL_ACT_RESET     0x00000002      /* reset on watchdog */
 
-static int wdt_timeout = 20;
+#define WDT_CTRL_ACTION                WDT_CTRL_ACT_RESET
+
+static int wdt_timeout = 60;
 static int started;
 static int in_use;
-static void __iomem *wdt_base;
+static void __iomem *wdt_timer_reg;
+static void __iomem *wdt_ctrl_reg;
 
-static inline void ar2315_wdt_wr(unsigned reg, u32 val)
+static inline void ar231x_wdt_wr_timer(u32 val)
 {
-       iowrite32(val, wdt_base + reg);
+       iowrite32(val, wdt_timer_reg);
+}
+
+static inline void ar231x_wdt_wr_ctrl(u32 val)
+{
+       iowrite32(val, wdt_ctrl_reg);
 }
 
 static void
-ar2315_wdt_enable(void)
+ar231x_wdt_enable(void)
 {
-       ar2315_wdt_wr(WDT_REG_TIMER, wdt_timeout * CLOCK_RATE);
+       ar231x_wdt_wr_timer(wdt_timeout * CLOCK_RATE);
+       ar231x_wdt_wr_ctrl(WDT_CTRL_ACTION);
 }
 
 static ssize_t
-ar2315_wdt_write(struct file *file, const char __user *data, size_t len,
+ar231x_wdt_write(struct file *file, const char __user *data, size_t len,
                 loff_t *ppos)
 {
        if (len)
-               ar2315_wdt_enable();
+               ar231x_wdt_enable();
        return len;
 }
 
 static int
-ar2315_wdt_open(struct inode *inode, struct file *file)
+ar231x_wdt_open(struct inode *inode, struct file *file)
 {
        if (in_use)
                return -EBUSY;
-       ar2315_wdt_enable();
+       ar231x_wdt_enable();
        in_use = started = 1;
        return nonseekable_open(inode, file);
 }
 
 static int
-ar2315_wdt_release(struct inode *inode, struct file *file)
+ar231x_wdt_release(struct inode *inode, struct file *file)
 {
        in_use = 0;
        return 0;
 }
 
 static irqreturn_t
-ar2315_wdt_interrupt(int irq, void *dev)
+ar231x_wdt_interrupt(int irq, void *dev)
 {
        struct platform_device *pdev = (struct platform_device *)dev;
 
@@ -95,19 +104,20 @@
                dev_crit(&pdev->dev, "watchdog expired, rebooting system\n");
                emergency_restart();
        } else {
-               ar2315_wdt_wr(WDT_REG_CTRL, 0);
-               ar2315_wdt_wr(WDT_REG_TIMER, 0);
+               // Interrupt flag can't be cleared until timer set to non-zero
+               ar231x_wdt_wr_timer(0xFFFFFFFF);
+               ar231x_wdt_wr_ctrl(WDT_CTRL_ACT_NONE);
        }
        return IRQ_HANDLED;
 }
 
 static struct watchdog_info ident = {
        .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING,
-       .identity = "ar2315 Watchdog",
+       .identity = "ar231x Watchdog",
 };
 
 static long
-ar2315_wdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+ar231x_wdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
        int new_wdt_timeout;
        int ret = -ENOIOCTLCMD;
@@ -118,7 +128,7 @@
                      -EFAULT : 0;
                break;
        case WDIOC_KEEPALIVE:
-               ar2315_wdt_enable();
+               ar231x_wdt_enable();
                ret = 0;
                break;
        case WDIOC_SETTIMEOUT:
@@ -126,7 +136,7 @@
                if (ret)
                        break;
                wdt_timeout = HEARTBEAT(new_wdt_timeout);
-               ar2315_wdt_enable();
+               ar231x_wdt_enable();
                break;
        case WDIOC_GETTIMEOUT:
                ret = put_user(wdt_timeout, (int __user *)arg);
@@ -135,28 +145,28 @@
        return ret;
 }
 
-static const struct file_operations ar2315_wdt_fops = {
+static const struct file_operations ar231x_wdt_fops = {
        .owner          = THIS_MODULE,
        .llseek         = no_llseek,
-       .write          = ar2315_wdt_write,
-       .unlocked_ioctl = ar2315_wdt_ioctl,
-       .open           = ar2315_wdt_open,
-       .release        = ar2315_wdt_release,
+       .write          = ar231x_wdt_write,
+       .unlocked_ioctl = ar231x_wdt_ioctl,
+       .open           = ar231x_wdt_open,
+       .release        = ar231x_wdt_release,
 };
 
-static struct miscdevice ar2315_wdt_miscdev = {
+static struct miscdevice ar231x_wdt_miscdev = {
        .minor  = WATCHDOG_MINOR,
        .name   = "watchdog",
-       .fops   = &ar2315_wdt_fops,
+       .fops   = &ar231x_wdt_fops,
 };
 
 static int
-ar2315_wdt_probe(struct platform_device *dev)
+ar231x_wdt_probe(struct platform_device *dev)
 {
        struct resource *mem_res, *irq_res;
        int ret = 0;
 
-       if (wdt_base)
+       if (wdt_timer_reg)
                return -EBUSY;
 
        irq_res = platform_get_resource(dev, IORESOURCE_IRQ, 0);
@@ -164,46 +174,55 @@
                dev_err(&dev->dev, "no IRQ resource\n");
                return -ENOENT;
        }
-
+       
        mem_res = platform_get_resource(dev, IORESOURCE_MEM, 0);
-       wdt_base = devm_ioremap_resource(&dev->dev, mem_res);
-       if (IS_ERR(wdt_base))
-               return PTR_ERR(wdt_base);
+       wdt_timer_reg = devm_ioremap_resource(&dev->dev, mem_res);
+       if (IS_ERR(wdt_timer_reg))
+               return PTR_ERR(wdt_timer_reg);
+
+       mem_res = platform_get_resource(dev, IORESOURCE_MEM, 1);
+       wdt_ctrl_reg = devm_ioremap_resource(&dev->dev, mem_res);
+       if (IS_ERR(wdt_ctrl_reg))
+               return PTR_ERR(wdt_ctrl_reg);
 
-       ret = devm_request_irq(&dev->dev, irq_res->start, ar2315_wdt_interrupt,
+       ret = devm_request_irq(&dev->dev, irq_res->start, ar231x_wdt_interrupt,
                               IRQF_DISABLED, DRIVER_NAME, dev);
        if (ret) {
-               dev_err(&dev->dev, "failed to register inetrrupt\n");
+               dev_err(&dev->dev, "failed to register interrupt\n");
                goto out;
        }
 
-       ret = misc_register(&ar2315_wdt_miscdev);
+       ret = misc_register(&ar231x_wdt_miscdev);
        if (ret)
                dev_err(&dev->dev, "failed to register miscdev\n");
 
+#ifdef WDT_START_ON_BOOT
+       ar231x_wdt_enable();
+       started = 1;
+#endif
 out:
        return ret;
 }
 
 static int
-ar2315_wdt_remove(struct platform_device *dev)
+ar231x_wdt_remove(struct platform_device *dev)
 {
-       misc_deregister(&ar2315_wdt_miscdev);
+       misc_deregister(&ar231x_wdt_miscdev);
        return 0;
 }
 
-static struct platform_driver ar2315_wdt_driver = {
-       .probe = ar2315_wdt_probe,
-       .remove = ar2315_wdt_remove,
+static struct platform_driver ar231x_wdt_driver = {
+       .probe = ar231x_wdt_probe,
+       .remove = ar231x_wdt_remove,
        .driver = {
                .name = DRIVER_NAME,
                .owner = THIS_MODULE,
        },
 };
 
-module_platform_driver(ar2315_wdt_driver);
+module_platform_driver(ar231x_wdt_driver);
 
-MODULE_DESCRIPTION("Atheros AR2315 hardware watchdog driver");
+MODULE_DESCRIPTION("Atheros AR231x hardware watchdog driver");
 MODULE_AUTHOR("John Crispin <blogic at openwrt.org>");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("platform:" DRIVER_NAME);


-- 
 Sergey                          mailto:ds at bittu.org.ru
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel



More information about the openwrt-devel mailing list