[OpenWrt-Devel] Watchdog support for AR531x and potential lockup
Sergey Ryazanov
ryazanov.s.a at gmail.com
Mon Oct 13 09:23:52 EDT 2014
Hi Sergey,
Interesting coincidence, I just spent the entire Sunday evening to
study the watchdog in the AR2315. I prepare patches for upstream
merging [1]. And also thinking about AR5312 support.
1. http://www.spinics.net/lists/linux-watchdog/msg05202.html
2014-10-13 11:49 GMT+04:00 Sergey Korolew <ds at bittu.org.ru>:
> 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.
Yep, same for AR2315+ SoCs. And if interrupt acknowledged by writing
one to ISR, then the timer starts counting again from 0xffffffff and
generates another one interrupt.
> 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.
Same for AR2315+ SoCs.
> 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).
>
Hardware reset doesn't work on AR2315 since hw bug and cause freeze if
issued by watchdog. See details in AR2315 reset routine in
arch/mips/ar231x/ar2315.c
I would like to propose use different device id strings (e.g.
ar2315-watchdog and ar5312-watchdog) to distinguish SoCs models. This
would help to solve several issues:
- twisted registers (passing adjacent registers via different
resources seems a bit odd),
- possibility of hardware reset,
- detection of watchdog clock frequency, since according to Axel Gembe
patch DWL-2100AP's watchdog timer ticks at 48MHz.
> 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
--
BR,
Sergey
_______________________________________________
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