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 <d...@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 <blo...@openwrt.org>"); > MODULE_LICENSE("GPL"); > MODULE_ALIAS("platform:" DRIVER_NAME); > > > -- > Sergey mailto:d...@bittu.org.ru > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel -- BR, Sergey _______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel