Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1274735 > unrolled thread

[PATCH 1/4] clocksource: Add brcm,bcm6345-timer device tree binding

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/4] clocksource: Add brcm,bcm6345-timer device tree binding Simon Arlott <simon@fire.lp0.eu> - 2015-11-21 20:10 +0100
    [PATCH 2/4] MIPS: bmips: Add bcm6345-l2-timer interrupt controller Simon Arlott <simon@fire.lp0.eu> - 2015-11-21 20:10 +0100
    [PATCH 4/4] MIPS: bmips: Convert bcm63xx_wdt to use WATCHDOG_CORE Simon Arlott <simon@fire.lp0.eu> - 2015-11-21 20:10 +0100
      Re: [PATCH 4/4] MIPS: bmips: Convert bcm63xx_wdt to use WATCHDOG_CORE Guenter Roeck <linux@roeck-us.net> - 2015-11-21 22:40 +0100
        Re: [PATCH 4/4] MIPS: bmips: Convert bcm63xx_wdt to use WATCHDOG_CORE Simon Arlott <simon@fire.lp0.eu> - 2015-11-21 22:50 +0100
          Re: [PATCH 4/4] MIPS: bmips: Convert bcm63xx_wdt to use WATCHDOG_CORE Guenter Roeck <linux@roeck-us.net> - 2015-11-22 03:40 +0100
            [PATCH 6/10] watchdog: bcm63xx_wdt: Obtain watchdog clock HZ from  "periph" clk Simon Arlott <simon@fire.lp0.eu> - 2015-11-22 15:10 +0100
              Re: [PATCH 6/10] watchdog: bcm63xx_wdt: Obtain watchdog clock HZ from  "periph" clk Jonas Gorski <jogo@openwrt.org> - 2015-11-23 16:10 +0100
                Re: [PATCH 6/10] watchdog: bcm63xx_wdt: Obtain watchdog clock HZ  from "periph" clk Florian Fainelli <f.fainelli@gmail.com> - 2015-11-23 19:30 +0100
                  Re: [PATCH 6/10] watchdog: bcm63xx_wdt: Obtain watchdog clock HZ from  "periph" clk Simon Arlott <simon@fire.lp0.eu> - 2015-11-23 20:10 +0100
              [PATCH (v2) 6/10] watchdog: bcm63xx_wdt: Obtain watchdog clock HZ  from "periph" clk "Simon Arlott" <simon@fire.lp0.eu> - 2015-11-24 23:20 +0100
                Re: [PATCH (v2) 6/10] watchdog: bcm63xx_wdt: Obtain watchdog clock  HZ from "periph" clk Florian Fainelli <f.fainelli@gmail.com> - 2015-11-24 23:50 +0100
                  [PATCH (v3) 6/11] watchdog: bcm63xx_wdt: Obtain watchdog clock HZ  from "periph" clk Simon Arlott <simon@fire.lp0.eu> - 2015-11-25 23:50 +0100
            [PATCH 7/10] watchdog: bcm63xx_wdt: Add get_timeleft function Simon Arlott <simon@fire.lp0.eu> - 2015-11-22 15:10 +0100
              [PATCH (v2) 7/10] watchdog: bcm63xx_wdt: Add get_timeleft function "Simon Arlott" <simon@fire.lp0.eu> - 2015-11-24 23:20 +0100
                Re: [PATCH (v2) 7/10] watchdog: bcm63xx_wdt: Add get_timeleft function Florian Fainelli <f.fainelli@gmail.com> - 2015-11-24 23:50 +0100
                Re: [PATCH (v2) 7/10] watchdog: bcm63xx_wdt: Add get_timeleft  function Guenter Roeck <linux@roeck-us.net> - 2015-11-25 04:00 +0100
                  Re: [PATCH (v2) 7/10] watchdog: bcm63xx_wdt: Add get_timeleft  function Simon Arlott <simon@fire.lp0.eu> - 2015-11-25 09:20 +0100
                    [PATCH (v4) 7/11] watchdog: bcm63xx_wdt: Add get_timeleft function Simon Arlott <simon@fire.lp0.eu> - 2015-11-26 00:00 +0100
                      [PATCH (v4) 8/11] watchdog: bcm63xx_wdt: Warn if the watchdog is  currently running Simon Arlott <simon@fire.lp0.eu> - 2015-11-26 00:00 +0100
                    [PATCH (v3) 7/11] watchdog: bcm63xx_wdt: Add get_timeleft function Simon Arlott <simon@fire.lp0.eu> - 2015-11-26 00:00 +0100
            Re: [PATCH 4/10] (Was: [PATCH 4/4]) MIPS: bmips: Convert bcm63xx_wdt  to use WATCHDOG_CORE Simon Arlott <simon@fire.lp0.eu> - 2015-11-22 15:10 +0100
              [PATCH 5/10] watchdog: bcm63xx_wdt: Use WATCHDOG_CORE Simon Arlott <simon@fire.lp0.eu> - 2015-11-22 15:10 +0100
                Re: [PATCH 5/10] watchdog: bcm63xx_wdt: Use WATCHDOG_CORE Guenter Roeck <linux@roeck-us.net> - 2015-11-25 03:50 +0100
                  Re: [PATCH 5/10] watchdog: bcm63xx_wdt: Use WATCHDOG_CORE "Simon Arlott" <simon@fire.lp0.eu> - 2015-11-25 14:10 +0100
                    Re: [PATCH 5/10] watchdog: bcm63xx_wdt: Use WATCHDOG_CORE Guenter Roeck <linux@roeck-us.net> - 2015-11-25 15:20 +0100
                      Re: [PATCH 5/10] watchdog: bcm63xx_wdt: Use WATCHDOG_CORE Simon Arlott <simon@fire.lp0.eu> - 2015-11-25 20:50 +0100
                  [PATCH (v3) 5/11] watchdog: bcm63xx_wdt: Use WATCHDOG_CORE Simon Arlott <simon@fire.lp0.eu> - 2015-11-25 23:50 +0100
              [PATCH 4/10] watchdog: bcm63xx_wdt: Handle hardware interrupt and  remove software timer Simon Arlott <simon@fire.lp0.eu> - 2015-11-22 15:10 +0100
                Re: [PATCH 4/10] watchdog: bcm63xx_wdt: Handle hardware interrupt  and remove software timer Guenter Roeck <linux@roeck-us.net> - 2015-11-24 19:30 +0100
                  Re: [PATCH 4/10] watchdog: bcm63xx_wdt: Handle hardware interrupt and  remove software timer Jonas Gorski <jogo@openwrt.org> - 2015-11-25 21:20 +0100
                    Re: [PATCH 4/10] watchdog: bcm63xx_wdt: Handle hardware interrupt and  remove software timer Simon Arlott <simon@fire.lp0.eu> - 2015-11-25 21:30 +0100
                [PATCH (v2) 4/10] watchdog: bcm63xx_wdt: Handle hardware interrupt  and remove software timer Simon Arlott <simon@fire.lp0.eu> - 2015-11-25 23:40 +0100
              [PATCH 8/10] watchdog: bcm63xx_wdt: Remove dependency on mach-bcm63xx  functions/defines Simon Arlott <simon@fire.lp0.eu> - 2015-11-22 15:20 +0100
              [PATCH 10/10] watchdog: bcm63xx_wdt: Use brcm,bcm6345-wdt device tree  binding Simon Arlott <simon@fire.lp0.eu> - 2015-11-22 15:20 +0100
                [PATCH (v2) 11/11] watchdog: bcm63xx_wdt: Use brcm,bcm6345-wdt device  tree binding Simon Arlott <simon@fire.lp0.eu> - 2015-11-26 00:20 +0100
              [PATCH 9/10] watchdog: bcm63xx_wdt: Use bcm63xx_timer interrupt  directly Simon Arlott <simon@fire.lp0.eu> - 2015-11-22 15:20 +0100
                [PATCH (v2) 10/11] watchdog: bcm63xx_wdt: Use bcm63xx_timer interrupt  directly Simon Arlott <simon@fire.lp0.eu> - 2015-11-26 00:10 +0100
    [PATCH 3/4] watchdog: Add brcm,bcm6345-wdt device tree binding Simon Arlott <simon@fire.lp0.eu> - 2015-11-21 20:10 +0100
      Re: [PATCH 3/4] watchdog: Add brcm,bcm6345-wdt device tree binding Rob Herring <robh@kernel.org> - 2015-11-22 23:20 +0100
    Re: [PATCH 1/4] clocksource: Add brcm,bcm6345-timer device tree  binding Rob Herring <robh@kernel.org> - 2015-11-22 23:20 +0100
    Re: [PATCH 1/4] clocksource: Add brcm,bcm6345-timer device tree binding Jonas Gorski <jogo@openwrt.org> - 2015-11-23 16:40 +0100
      [PATCH (v2) 2/10] MIPS: bmips: Add bcm6345-l2-timer interrupt  controller Simon Arlott <simon@fire.lp0.eu> - 2015-11-23 20:00 +0100
        [PATCH (v3) 2/10] MIPS: bmips: Add bcm6345-l2-timer interrupt  controller "Simon Arlott" <simon@fire.lp0.eu> - 2015-11-24 23:20 +0100
          Re: [PATCH (v3) 2/10] MIPS: bmips: Add bcm6345-l2-timer interrupt  controller Florian Fainelli <f.fainelli@gmail.com> - 2015-11-24 23:40 +0100
            [PATCH (v4) 2/11] MIPS: bmips: Add bcm6345-l2-timer interrupt  controller Simon Arlott <simon@fire.lp0.eu> - 2015-11-26 23:40 +0100
              Re: [PATCH (v4) 2/11] MIPS: bmips: Add bcm6345-l2-timer interrupt  controller Thomas Gleixner <tglx@linutronix.de> - 2015-11-27 09:40 +0100
                [PATCH (v5) 3/11] MIPS: bmips: Add bcm6345-l2-timer interrupt  controller Simon Arlott <simon@fire.lp0.eu> - 2015-11-28 13:30 +0100
      [PATCH (v2) 1/10] clocksource: Add brcm,bcm6345-timer device tree  binding Simon Arlott <simon@fire.lp0.eu> - 2015-11-23 20:00 +0100
        Re: [PATCH (v2) 1/10] clocksource: Add brcm,bcm6345-timer device  tree binding Rob Herring <robh@kernel.org> - 2015-11-25 04:10 +0100

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

#1277863 — [PATCH (v3) 7/11] watchdog: bcm63xx_wdt: Add get_timeleft function

Return the remaining time from the hardware control register.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
On 25/11/15 02:51, Guenter Roeck wrote:
> This is really two logical changes, isn't it ?

Patch 7 split into two patches.

 drivers/watchdog/bcm63xx_wdt.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/bcm63xx_wdt.c b/drivers/watchdog/bcm63xx_wdt.c
index 0a19731..5615277 100644
--- a/drivers/watchdog/bcm63xx_wdt.c
+++ b/drivers/watchdog/bcm63xx_wdt.c
@@ -14,6 +14,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/clk.h>
+#include <linux/delay.h>
 #include <linux/errno.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
@@ -78,6 +79,19 @@ static int bcm63xx_wdt_stop(struct watchdog_device *wdd)
 	return 0;
 }
 
+static unsigned int bcm63xx_wdt_get_timeleft(struct watchdog_device *wdd)
+{
+	struct bcm63xx_wdt_hw *hw = to_wdt_hw(wdd);
+	unsigned long flags;
+	u32 val;
+
+	raw_spin_lock_irqsave(&hw->lock, flags);
+	val = __raw_readl(hw->regs + WDT_CTL_REG);
+	val /= hw->clock_hz;
+	raw_spin_unlock_irqrestore(&hw->lock, flags);
+	return val;
+}
+
 static int bcm63xx_wdt_set_timeout(struct watchdog_device *wdd,
 	unsigned int timeout)
 {
@@ -132,6 +146,7 @@ static struct watchdog_ops bcm63xx_wdt_ops = {
 	.owner = THIS_MODULE,
 	.start = bcm63xx_wdt_start,
 	.stop = bcm63xx_wdt_stop,
+	.get_timeleft = bcm63xx_wdt_get_timeleft,
 	.set_timeout = bcm63xx_wdt_set_timeout,
 };
 
@@ -215,7 +230,6 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
 		"%s at MMIO 0x%p (timeout = %us, max_timeout = %us)",
 		dev_name(wdd->dev), hw->regs,
 		wdd->timeout, wdd->max_timeout);
-
 	return 0;
 
 unregister_watchdog:
@@ -256,6 +270,7 @@ module_platform_driver(bcm63xx_wdt_driver);
 
 MODULE_AUTHOR("Miguel Gaio <miguel.gaio@efixo.com>");
 MODULE_AUTHOR("Florian Fainelli <florian@openwrt.org>");
+MODULE_AUTHOR("Simon Arlott");
 MODULE_DESCRIPTION("Driver for the Broadcom BCM63xx SoC watchdog");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("platform:bcm63xx-wdt");
-- 
2.1.4

-- 
Simon Arlott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1274875 — Re: [PATCH 4/10] (Was: [PATCH 4/4]) MIPS: bmips: Convert bcm63xx_wdt to use WATCHDOG_CORE

On 22/11/15 02:32, Guenter Roeck wrote:
> On 11/21/2015 01:44 PM, Simon Arlott wrote:
>> On 21/11/15 21:32, Guenter Roeck wrote:
>>> this is really doing a bit too much in a single patch.
>>> Conversion to the watchdog infrastructure should probably be
>>> the first step, followed by further optimizations and improvements.
>>

I've split patch 4 up into 7 patches, which will be patches 4/10..10/10
in this thread.

Instead of using bcm63xx_timer_register in #ifdefs, I'll remove most of
bcm63xx_timer because it's only used by the watchdog.

>>> We have some infrastructure changes in the works which will move
>>> the need for soft-timers from individual drivers into the watchdog core.
>>> Would this possibly be helpful here ? The timer-driven watchdog ping
>>> seems to accomplish pretty much the same.
>>
>> There is no need for a software timer. This is not a timer-driven
>> watchdog ping, there is an unmaskable timer interrupt when the watchdog
>> timer has less than 50% remaining.
>>
> Ok. Maybe I got confused by the interrupt-triggered watchdog ping.
> I'll have to look into that much more closely; it is quite unusual
> and complex. The explanation is also not easy to understand.
> What does "The only way to stop this interrupt" mean ? Repeatedly

The interrupt is level triggered at less than 50% of the time remaining.
Unless the watchdog is stopped or restarted, the interrupt will not stop
occurring.

> triggering the interrupt in 1/2, 1/4, 1/8 of the remaining time is
> really odd.

It's a bit odd but there's no way to ack the interrupt. This seemed like
the best approach without adding the complexity of a software timer or
trying to mask and unmask the timer interrupt. I don't  want to ignore
the interrupt entirely because I'd like to know if the watchdog is going
to cause a reboot.

> On side note, the subject tag should be "watchdog:", not "MIPS:".

Fixed.

-- 
Simon Arlott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1274876 — [PATCH 5/10] watchdog: bcm63xx_wdt: Use WATCHDOG_CORE

Convert bcm63xx_wdt to use WATCHDOG_CORE.

The default and maximum time constants that are only used once have been
moved to the initialisation of the struct watchdog_device.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
 drivers/watchdog/Kconfig       |   1 +
 drivers/watchdog/bcm63xx_wdt.c | 249 ++++++++++++-----------------------------
 2 files changed, 74 insertions(+), 176 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 7a8a6c6..6815b74 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1273,6 +1273,7 @@ config OCTEON_WDT
 config BCM63XX_WDT
 	tristate "Broadcom BCM63xx hardware watchdog"
 	depends on BCM63XX
+	select WATCHDOG_CORE
 	help
 	  Watchdog driver for the built in watchdog hardware in Broadcom
 	  BCM63xx SoC.
diff --git a/drivers/watchdog/bcm63xx_wdt.c b/drivers/watchdog/bcm63xx_wdt.c
index f88fc97..1d2a501 100644
--- a/drivers/watchdog/bcm63xx_wdt.c
+++ b/drivers/watchdog/bcm63xx_wdt.c
@@ -13,20 +13,15 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/bitops.h>
 #include <linux/errno.h>
-#include <linux/fs.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
-#include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/spinlock.h>
 #include <linux/types.h>
-#include <linux/uaccess.h>
 #include <linux/watchdog.h>
 #include <linux/interrupt.h>
-#include <linux/ptrace.h>
 #include <linux/resource.h>
 #include <linux/platform_device.h>
 
@@ -38,53 +33,57 @@
 #define PFX KBUILD_MODNAME
 
 #define WDT_HZ			50000000		/* Fclk */
-#define WDT_DEFAULT_TIME	30			/* seconds */
-#define WDT_MAX_TIME		(0xffffffff / WDT_HZ)	/* seconds */
 
 struct bcm63xx_wdt_hw {
 	raw_spinlock_t lock;
 	void __iomem *regs;
-	unsigned long inuse;
 	bool running;
 };
-static struct bcm63xx_wdt_hw bcm63xx_wdt_device;
 
-static int expect_close;
-
-static int wdt_time = WDT_DEFAULT_TIME;
 static bool nowayout = WATCHDOG_NOWAYOUT;
 module_param(nowayout, bool, 0);
 MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
 	__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
 
-/* HW functions */
-static void bcm63xx_wdt_hw_start(void)
+static int bcm63xx_wdt_start(struct watchdog_device *wdd)
 {
+	struct bcm63xx_wdt_hw *hw = watchdog_get_drvdata(wdd);
 	unsigned long flags;
 
-	raw_spin_lock_irqsave(&bcm63xx_wdt_device.lock, flags);
-	bcm_writel(wdt_time * WDT_HZ, bcm63xx_wdt_device.regs + WDT_DEFVAL_REG);
-	bcm_writel(WDT_START_1, bcm63xx_wdt_device.regs + WDT_CTL_REG);
-	bcm_writel(WDT_START_2, bcm63xx_wdt_device.regs + WDT_CTL_REG);
-	bcm63xx_wdt_device.running = true;
-	raw_spin_unlock_irqrestore(&bcm63xx_wdt_device.lock, flags);
+	raw_spin_lock_irqsave(&hw->lock, flags);
+	bcm_writel(wdd->timeout * WDT_HZ, hw->regs + WDT_DEFVAL_REG);
+	bcm_writel(WDT_START_1, hw->regs + WDT_CTL_REG);
+	bcm_writel(WDT_START_2, hw->regs + WDT_CTL_REG);
+	hw->running = true;
+	raw_spin_unlock_irqrestore(&hw->lock, flags);
+	return 0;
 }
 
-static void bcm63xx_wdt_hw_stop(void)
+static int bcm63xx_wdt_stop(struct watchdog_device *wdd)
 {
+	struct bcm63xx_wdt_hw *hw = watchdog_get_drvdata(wdd);
 	unsigned long flags;
 
-	raw_spin_lock_irqsave(&bcm63xx_wdt_device.lock, flags);
-	bcm_writel(WDT_STOP_1, bcm63xx_wdt_device.regs + WDT_CTL_REG);
-	bcm_writel(WDT_STOP_2, bcm63xx_wdt_device.regs + WDT_CTL_REG);
-	bcm63xx_wdt_device.running = false;
-	raw_spin_unlock_irqrestore(&bcm63xx_wdt_device.lock, flags);
+	raw_spin_lock_irqsave(&hw->lock, flags);
+	bcm_writel(WDT_STOP_1, hw->regs + WDT_CTL_REG);
+	bcm_writel(WDT_STOP_2, hw->regs + WDT_CTL_REG);
+	hw->running = false;
+	raw_spin_unlock_irqrestore(&hw->lock, flags);
+	return 0;
+}
+
+static int bcm63xx_wdt_set_timeout(struct watchdog_device *wdd,
+	unsigned int timeout)
+{
+	wdd->timeout = timeout;
+	return bcm63xx_wdt_start(wdd);
 }
 
 /* The watchdog interrupt occurs when half the timeout is remaining */
 static void bcm63xx_wdt_isr(void *data)
 {
-	struct bcm63xx_wdt_hw *hw = &bcm63xx_wdt_device;
+	struct watchdog_device *wdd = data;
+	struct bcm63xx_wdt_hw *hw = watchdog_get_drvdata(wdd);
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&hw->lock, flags);
@@ -118,147 +117,36 @@ static void bcm63xx_wdt_isr(void *data)
 		}
 
 		ms = timeleft / (WDT_HZ / 1000);
-		pr_alert("warning timer fired, reboot in %ums\n", ms);
+		dev_alert(wdd->dev,
+			"warning timer fired, reboot in %ums\n", ms);
 	}
 	raw_spin_unlock_irqrestore(&hw->lock, flags);
 }
 
-static int bcm63xx_wdt_settimeout(int new_time)
-{
-	if ((new_time <= 0) || (new_time > WDT_MAX_TIME))
-		return -EINVAL;
-
-	wdt_time = new_time;
-
-	return 0;
-}
-
-static int bcm63xx_wdt_open(struct inode *inode, struct file *file)
-{
-	if (test_and_set_bit(0, &bcm63xx_wdt_device.inuse))
-		return -EBUSY;
-
-	bcm63xx_wdt_hw_start();
-	return nonseekable_open(inode, file);
-}
-
-static int bcm63xx_wdt_release(struct inode *inode, struct file *file)
-{
-	if (expect_close == 42)
-		bcm63xx_wdt_hw_stop();
-	else {
-		pr_crit("Unexpected close, not stopping watchdog!\n");
-		bcm63xx_wdt_hw_start();
-	}
-	clear_bit(0, &bcm63xx_wdt_device.inuse);
-	expect_close = 0;
-	return 0;
-}
-
-static ssize_t bcm63xx_wdt_write(struct file *file, const char *data,
-				size_t len, loff_t *ppos)
-{
-	if (len) {
-		if (!nowayout) {
-			size_t i;
-
-			/* In case it was set long ago */
-			expect_close = 0;
-
-			for (i = 0; i != len; i++) {
-				char c;
-				if (get_user(c, data + i))
-					return -EFAULT;
-				if (c == 'V')
-					expect_close = 42;
-			}
-		}
-		bcm63xx_wdt_hw_start();
-	}
-	return len;
-}
-
-static struct watchdog_info bcm63xx_wdt_info = {
-	.identity       = PFX,
-	.options        = WDIOF_SETTIMEOUT |
-				WDIOF_KEEPALIVEPING |
-				WDIOF_MAGICCLOSE,
-};
-
-
-static long bcm63xx_wdt_ioctl(struct file *file, unsigned int cmd,
-				unsigned long arg)
-{
-	void __user *argp = (void __user *)arg;
-	int __user *p = argp;
-	int new_value, retval = -EINVAL;
-
-	switch (cmd) {
-	case WDIOC_GETSUPPORT:
-		return copy_to_user(argp, &bcm63xx_wdt_info,
-			sizeof(bcm63xx_wdt_info)) ? -EFAULT : 0;
-
-	case WDIOC_GETSTATUS:
-	case WDIOC_GETBOOTSTATUS:
-		return put_user(0, p);
-
-	case WDIOC_SETOPTIONS:
-		if (get_user(new_value, p))
-			return -EFAULT;
-
-		if (new_value & WDIOS_DISABLECARD) {
-			bcm63xx_wdt_hw_stop();
-			retval = 0;
-		}
-		if (new_value & WDIOS_ENABLECARD) {
-			bcm63xx_wdt_hw_start();
-			retval = 0;
-		}
-
-		return retval;
-
-	case WDIOC_KEEPALIVE:
-		bcm63xx_wdt_hw_start();
-		return 0;
-
-	case WDIOC_SETTIMEOUT:
-		if (get_user(new_value, p))
-			return -EFAULT;
-
-		if (bcm63xx_wdt_settimeout(new_value))
-			return -EINVAL;
-
-		bcm63xx_wdt_hw_start();
-
-	case WDIOC_GETTIMEOUT:
-		return put_user(wdt_time, p);
-
-	default:
-		return -ENOTTY;
-
-	}
-}
-
-static const struct file_operations bcm63xx_wdt_fops = {
-	.owner		= THIS_MODULE,
-	.llseek		= no_llseek,
-	.write		= bcm63xx_wdt_write,
-	.unlocked_ioctl	= bcm63xx_wdt_ioctl,
-	.open		= bcm63xx_wdt_open,
-	.release	= bcm63xx_wdt_release,
+static struct watchdog_ops bcm63xx_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = bcm63xx_wdt_start,
+	.stop = bcm63xx_wdt_stop,
+	.set_timeout = bcm63xx_wdt_set_timeout,
 };
 
-static struct miscdevice bcm63xx_wdt_miscdev = {
-	.minor	= WATCHDOG_MINOR,
-	.name	= "watchdog",
-	.fops	= &bcm63xx_wdt_fops,
+static const struct watchdog_info bcm63xx_wdt_info = {
+	.options = WDIOC_GETTIMELEFT | WDIOF_SETTIMEOUT |
+			WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+	.identity = "BCM63xx Watchdog",
 };
 
-
 static int bcm63xx_wdt_probe(struct platform_device *pdev)
 {
-	int ret;
+	struct bcm63xx_wdt_hw *hw;
+	struct watchdog_device *wdd;
 	struct resource *r;
+	int ret;
+
+	hw = devm_kzalloc(&pdev->dev, sizeof(*hw), GFP_KERNEL);
+	wdd = devm_kzalloc(&pdev->dev, sizeof(*wdd), GFP_KERNEL);
+	if (!hw || !wdd)
+		return -ENOMEM;
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!r) {
@@ -266,36 +154,44 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	bcm63xx_wdt_device.regs = devm_ioremap_nocache(&pdev->dev, r->start,
-							resource_size(r));
-	if (!bcm63xx_wdt_device.regs) {
+	hw->regs = devm_ioremap_nocache(&pdev->dev, r->start, resource_size(r));
+	if (!hw->regs) {
 		dev_err(&pdev->dev, "failed to remap I/O resources\n");
 		return -ENXIO;
 	}
 
-	raw_spin_lock_init(&bcm63xx_wdt_device.lock);
+	raw_spin_lock_init(&hw->lock);
+	hw->running = false;
+
+	wdd->parent = &pdev->dev;
+	wdd->ops = &bcm63xx_wdt_ops;
+	wdd->info = &bcm63xx_wdt_info;
+	wdd->min_timeout = 1;
+	wdd->max_timeout = 0xffffffff / WDT_HZ;
+	wdd->timeout = min(30U, wdd->max_timeout);
+
+	watchdog_set_drvdata(wdd, hw);
+	platform_set_drvdata(pdev, wdd);
+
+	watchdog_init_timeout(wdd, 0, &pdev->dev);
+	watchdog_set_nowayout(wdd, nowayout);
 
-	ret = bcm63xx_timer_register(TIMER_WDT_ID, bcm63xx_wdt_isr, NULL);
+	ret = bcm63xx_timer_register(TIMER_WDT_ID, bcm63xx_wdt_isr, wdd);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to register wdt timer isr\n");
 		return ret;
 	}
 
-	if (bcm63xx_wdt_settimeout(wdt_time)) {
-		bcm63xx_wdt_settimeout(WDT_DEFAULT_TIME);
-		dev_info(&pdev->dev,
-			": wdt_time value must be 1 <= wdt_time <= %d, using %d\n",
-			WDT_MAX_TIME, wdt_time);
-	}
-
-	ret = misc_register(&bcm63xx_wdt_miscdev);
+	ret = watchdog_register_device(wdd);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to register watchdog device\n");
 		goto unregister_timer;
 	}
 
-	dev_info(&pdev->dev, " started, timer margin: %d sec\n",
-						WDT_DEFAULT_TIME);
+	dev_info(&pdev->dev,
+		"%s at MMIO 0x%p (timeout = %us, max_timeout = %us)",
+		dev_name(wdd->dev), hw->regs,
+		wdd->timeout, wdd->max_timeout);
 
 	return 0;
 
@@ -306,17 +202,18 @@ unregister_timer:
 
 static int bcm63xx_wdt_remove(struct platform_device *pdev)
 {
-	if (!nowayout)
-		bcm63xx_wdt_hw_stop();
+	struct watchdog_device *wdd = platform_get_drvdata(pdev);
 
-	misc_deregister(&bcm63xx_wdt_miscdev);
 	bcm63xx_timer_unregister(TIMER_WDT_ID);
+	watchdog_unregister_device(wdd);
 	return 0;
 }
 
 static void bcm63xx_wdt_shutdown(struct platform_device *pdev)
 {
-	bcm63xx_wdt_hw_stop();
+	struct watchdog_device *wdd = platform_get_drvdata(pdev);
+
+	bcm63xx_wdt_stop(wdd);
 }
 
 static struct platform_driver bcm63xx_wdt_driver = {
-- 
2.1.4

-- 
Simon Arlott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1276981 — Re: [PATCH 5/10] watchdog: bcm63xx_wdt: Use WATCHDOG_CORE

Hi Simon,

On 11/22/2015 06:06 AM, Simon Arlott wrote:
> Convert bcm63xx_wdt to use WATCHDOG_CORE.
>
> The default and maximum time constants that are only used once have been
> moved to the initialisation of the struct watchdog_device.
>
Comments inline.

Thanks,
Guenter

> Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
> ---
>   drivers/watchdog/Kconfig       |   1 +
>   drivers/watchdog/bcm63xx_wdt.c | 249 ++++++++++++-----------------------------
>   2 files changed, 74 insertions(+), 176 deletions(-)
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 7a8a6c6..6815b74 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1273,6 +1273,7 @@ config OCTEON_WDT
>   config BCM63XX_WDT
>   	tristate "Broadcom BCM63xx hardware watchdog"
>   	depends on BCM63XX
> +	select WATCHDOG_CORE
>   	help
>   	  Watchdog driver for the built in watchdog hardware in Broadcom
>   	  BCM63xx SoC.
> diff --git a/drivers/watchdog/bcm63xx_wdt.c b/drivers/watchdog/bcm63xx_wdt.c
> index f88fc97..1d2a501 100644
> --- a/drivers/watchdog/bcm63xx_wdt.c
> +++ b/drivers/watchdog/bcm63xx_wdt.c
> @@ -13,20 +13,15 @@
>
>   #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> -#include <linux/bitops.h>
>   #include <linux/errno.h>
> -#include <linux/fs.h>
>   #include <linux/io.h>
>   #include <linux/kernel.h>
> -#include <linux/miscdevice.h>
>   #include <linux/module.h>
>   #include <linux/moduleparam.h>
>   #include <linux/spinlock.h>
>   #include <linux/types.h>
> -#include <linux/uaccess.h>
>   #include <linux/watchdog.h>
>   #include <linux/interrupt.h>
> -#include <linux/ptrace.h>
>   #include <linux/resource.h>
>   #include <linux/platform_device.h>
>
> @@ -38,53 +33,57 @@
>   #define PFX KBUILD_MODNAME
>
>   #define WDT_HZ			50000000		/* Fclk */
> -#define WDT_DEFAULT_TIME	30			/* seconds */
> -#define WDT_MAX_TIME		(0xffffffff / WDT_HZ)	/* seconds */
>
>   struct bcm63xx_wdt_hw {
>   	raw_spinlock_t lock;
>   	void __iomem *regs;
> -	unsigned long inuse;
>   	bool running;

The "running" flag should no longer be needed. watchdog_active()
should provide that information.

>   };
> -static struct bcm63xx_wdt_hw bcm63xx_wdt_device;
>
> -static int expect_close;
> -
> -static int wdt_time = WDT_DEFAULT_TIME;
>   static bool nowayout = WATCHDOG_NOWAYOUT;
>   module_param(nowayout, bool, 0);
>   MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
>   	__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
>
> -/* HW functions */
> -static void bcm63xx_wdt_hw_start(void)
> +static int bcm63xx_wdt_start(struct watchdog_device *wdd)
>   {
> +	struct bcm63xx_wdt_hw *hw = watchdog_get_drvdata(wdd);
>   	unsigned long flags;
>
> -	raw_spin_lock_irqsave(&bcm63xx_wdt_device.lock, flags);
> -	bcm_writel(wdt_time * WDT_HZ, bcm63xx_wdt_device.regs + WDT_DEFVAL_REG);
> -	bcm_writel(WDT_START_1, bcm63xx_wdt_device.regs + WDT_CTL_REG);
> -	bcm_writel(WDT_START_2, bcm63xx_wdt_device.regs + WDT_CTL_REG);
> -	bcm63xx_wdt_device.running = true;
> -	raw_spin_unlock_irqrestore(&bcm63xx_wdt_device.lock, flags);
> +	raw_spin_lock_irqsave(&hw->lock, flags);
> +	bcm_writel(wdd->timeout * WDT_HZ, hw->regs + WDT_DEFVAL_REG);
> +	bcm_writel(WDT_START_1, hw->regs + WDT_CTL_REG);
> +	bcm_writel(WDT_START_2, hw->regs + WDT_CTL_REG);
> +	hw->running = true;
> +	raw_spin_unlock_irqrestore(&hw->lock, flags);
> +	return 0;
>   }
>
> -static void bcm63xx_wdt_hw_stop(void)
> +static int bcm63xx_wdt_stop(struct watchdog_device *wdd)
>   {
> +	struct bcm63xx_wdt_hw *hw = watchdog_get_drvdata(wdd);
>   	unsigned long flags;
>
> -	raw_spin_lock_irqsave(&bcm63xx_wdt_device.lock, flags);
> -	bcm_writel(WDT_STOP_1, bcm63xx_wdt_device.regs + WDT_CTL_REG);
> -	bcm_writel(WDT_STOP_2, bcm63xx_wdt_device.regs + WDT_CTL_REG);
> -	bcm63xx_wdt_device.running = false;
> -	raw_spin_unlock_irqrestore(&bcm63xx_wdt_device.lock, flags);
> +	raw_spin_lock_irqsave(&hw->lock, flags);
> +	bcm_writel(WDT_STOP_1, hw->regs + WDT_CTL_REG);
> +	bcm_writel(WDT_STOP_2, hw->regs + WDT_CTL_REG);
> +	hw->running = false;
> +	raw_spin_unlock_irqrestore(&hw->lock, flags);
> +	return 0;
> +}
> +
> +static int bcm63xx_wdt_set_timeout(struct watchdog_device *wdd,
> +	unsigned int timeout)
> +{
> +	wdd->timeout = timeout;
> +	return bcm63xx_wdt_start(wdd);

If I see correctly, there is no ping function. In that case, the watchdog core
will call the start function after updating the timeout, so there is no need
to do it here.

>   }
>
>   /* The watchdog interrupt occurs when half the timeout is remaining */
>   static void bcm63xx_wdt_isr(void *data)
>   {
> -	struct bcm63xx_wdt_hw *hw = &bcm63xx_wdt_device;
> +	struct watchdog_device *wdd = data;
> +	struct bcm63xx_wdt_hw *hw = watchdog_get_drvdata(wdd);
>   	unsigned long flags;
>
>   	raw_spin_lock_irqsave(&hw->lock, flags);
> @@ -118,147 +117,36 @@ static void bcm63xx_wdt_isr(void *data)
>   		}
>
>   		ms = timeleft / (WDT_HZ / 1000);
> -		pr_alert("warning timer fired, reboot in %ums\n", ms);
> +		dev_alert(wdd->dev,
> +			"warning timer fired, reboot in %ums\n", ms);
>   	}
>   	raw_spin_unlock_irqrestore(&hw->lock, flags);
>   }
>
> -static int bcm63xx_wdt_settimeout(int new_time)
> -{
> -	if ((new_time <= 0) || (new_time > WDT_MAX_TIME))
> -		return -EINVAL;
> -
> -	wdt_time = new_time;
> -
> -	return 0;
> -}
> -
> -static int bcm63xx_wdt_open(struct inode *inode, struct file *file)
> -{
> -	if (test_and_set_bit(0, &bcm63xx_wdt_device.inuse))
> -		return -EBUSY;
> -
> -	bcm63xx_wdt_hw_start();
> -	return nonseekable_open(inode, file);
> -}
> -
> -static int bcm63xx_wdt_release(struct inode *inode, struct file *file)
> -{
> -	if (expect_close == 42)
> -		bcm63xx_wdt_hw_stop();
> -	else {
> -		pr_crit("Unexpected close, not stopping watchdog!\n");
> -		bcm63xx_wdt_hw_start();
> -	}
> -	clear_bit(0, &bcm63xx_wdt_device.inuse);
> -	expect_close = 0;
> -	return 0;
> -}
> -
> -static ssize_t bcm63xx_wdt_write(struct file *file, const char *data,
> -				size_t len, loff_t *ppos)
> -{
> -	if (len) {
> -		if (!nowayout) {
> -			size_t i;
> -
> -			/* In case it was set long ago */
> -			expect_close = 0;
> -
> -			for (i = 0; i != len; i++) {
> -				char c;
> -				if (get_user(c, data + i))
> -					return -EFAULT;
> -				if (c == 'V')
> -					expect_close = 42;
> -			}
> -		}
> -		bcm63xx_wdt_hw_start();
> -	}
> -	return len;
> -}
> -
> -static struct watchdog_info bcm63xx_wdt_info = {
> -	.identity       = PFX,
> -	.options        = WDIOF_SETTIMEOUT |
> -				WDIOF_KEEPALIVEPING |
> -				WDIOF_MAGICCLOSE,
> -};
> -
> -
> -static long bcm63xx_wdt_ioctl(struct file *file, unsigned int cmd,
> -				unsigned long arg)
> -{
> -	void __user *argp = (void __user *)arg;
> -	int __user *p = argp;
> -	int new_value, retval = -EINVAL;
> -
> -	switch (cmd) {
> -	case WDIOC_GETSUPPORT:
> -		return copy_to_user(argp, &bcm63xx_wdt_info,
> -			sizeof(bcm63xx_wdt_info)) ? -EFAULT : 0;
> -
> -	case WDIOC_GETSTATUS:
> -	case WDIOC_GETBOOTSTATUS:
> -		return put_user(0, p);
> -
> -	case WDIOC_SETOPTIONS:
> -		if (get_user(new_value, p))
> -			return -EFAULT;
> -
> -		if (new_value & WDIOS_DISABLECARD) {
> -			bcm63xx_wdt_hw_stop();
> -			retval = 0;
> -		}
> -		if (new_value & WDIOS_ENABLECARD) {
> -			bcm63xx_wdt_hw_start();
> -			retval = 0;
> -		}
> -
> -		return retval;
> -
> -	case WDIOC_KEEPALIVE:
> -		bcm63xx_wdt_hw_start();
> -		return 0;
> -
> -	case WDIOC_SETTIMEOUT:
> -		if (get_user(new_value, p))
> -			return -EFAULT;
> -
> -		if (bcm63xx_wdt_settimeout(new_value))
> -			return -EINVAL;
> -
> -		bcm63xx_wdt_hw_start();
> -
> -	case WDIOC_GETTIMEOUT:
> -		return put_user(wdt_time, p);
> -
> -	default:
> -		return -ENOTTY;
> -
> -	}
> -}
> -
> -static const struct file_operations bcm63xx_wdt_fops = {
> -	.owner		= THIS_MODULE,
> -	.llseek		= no_llseek,
> -	.write		= bcm63xx_wdt_write,
> -	.unlocked_ioctl	= bcm63xx_wdt_ioctl,
> -	.open		= bcm63xx_wdt_open,
> -	.release	= bcm63xx_wdt_release,
> +static struct watchdog_ops bcm63xx_wdt_ops = {
> +	.owner = THIS_MODULE,
> +	.start = bcm63xx_wdt_start,
> +	.stop = bcm63xx_wdt_stop,
> +	.set_timeout = bcm63xx_wdt_set_timeout,
>   };
>
> -static struct miscdevice bcm63xx_wdt_miscdev = {
> -	.minor	= WATCHDOG_MINOR,
> -	.name	= "watchdog",
> -	.fops	= &bcm63xx_wdt_fops,
> +static const struct watchdog_info bcm63xx_wdt_info = {
> +	.options = WDIOC_GETTIMELEFT | WDIOF_SETTIMEOUT |

Where is the gettimeleft function ? I think you are adding it with a later patch,
but then you should set the flag there, not here.

> +			WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> +	.identity = "BCM63xx Watchdog",
>   };
>
> -
>   static int bcm63xx_wdt_probe(struct platform_device *pdev)
>   {
> -	int ret;
> +	struct bcm63xx_wdt_hw *hw;
> +	struct watchdog_device *wdd;
>   	struct resource *r;
> +	int ret;
> +
> +	hw = devm_kzalloc(&pdev->dev, sizeof(*hw), GFP_KERNEL);
> +	wdd = devm_kzalloc(&pdev->dev, sizeof(*wdd), GFP_KERNEL);

It would be better to allocate wdd as part of struct bcm63xx_wdt_hw.
Then you only need a single allocation. You can still use
	wdd = &hw->wdd;
to simplify the rest of this function.

> +	if (!hw || !wdd)
> +		return -ENOMEM;
>
>   	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   	if (!r) {
> @@ -266,36 +154,44 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
>   		return -ENODEV;
>   	}
>
> -	bcm63xx_wdt_device.regs = devm_ioremap_nocache(&pdev->dev, r->start,
> -							resource_size(r));
> -	if (!bcm63xx_wdt_device.regs) {
> +	hw->regs = devm_ioremap_nocache(&pdev->dev, r->start, resource_size(r));
> +	if (!hw->regs) {
>   		dev_err(&pdev->dev, "failed to remap I/O resources\n");
>   		return -ENXIO;
>   	}
>
> -	raw_spin_lock_init(&bcm63xx_wdt_device.lock);
> +	raw_spin_lock_init(&hw->lock);
> +	hw->running = false;
> +
> +	wdd->parent = &pdev->dev;
> +	wdd->ops = &bcm63xx_wdt_ops;
> +	wdd->info = &bcm63xx_wdt_info;
> +	wdd->min_timeout = 1;
> +	wdd->max_timeout = 0xffffffff / WDT_HZ;
> +	wdd->timeout = min(30U, wdd->max_timeout);
> +
> +	watchdog_set_drvdata(wdd, hw);
> +	platform_set_drvdata(pdev, wdd);
> +
> +	watchdog_init_timeout(wdd, 0, &pdev->dev);
> +	watchdog_set_nowayout(wdd, nowayout);
>
> -	ret = bcm63xx_timer_register(TIMER_WDT_ID, bcm63xx_wdt_isr, NULL);
> +	ret = bcm63xx_timer_register(TIMER_WDT_ID, bcm63xx_wdt_isr, wdd);
>   	if (ret < 0) {
>   		dev_err(&pdev->dev, "failed to register wdt timer isr\n");
>   		return ret;
>   	}
>
> -	if (bcm63xx_wdt_settimeout(wdt_time)) {
> -		bcm63xx_wdt_settimeout(WDT_DEFAULT_TIME);
> -		dev_info(&pdev->dev,
> -			": wdt_time value must be 1 <= wdt_time <= %d, using %d\n",
> -			WDT_MAX_TIME, wdt_time);
> -	}
> -
> -	ret = misc_register(&bcm63xx_wdt_miscdev);
> +	ret = watchdog_register_device(wdd);
>   	if (ret < 0) {
>   		dev_err(&pdev->dev, "failed to register watchdog device\n");
>   		goto unregister_timer;
>   	}
>
> -	dev_info(&pdev->dev, " started, timer margin: %d sec\n",
> -						WDT_DEFAULT_TIME);
> +	dev_info(&pdev->dev,
> +		"%s at MMIO 0x%p (timeout = %us, max_timeout = %us)",
> +		dev_name(wdd->dev), hw->regs,
> +		wdd->timeout, wdd->max_timeout);
>
>   	return 0;
>
> @@ -306,17 +202,18 @@ unregister_timer:
>
>   static int bcm63xx_wdt_remove(struct platform_device *pdev)
>   {
> -	if (!nowayout)
> -		bcm63xx_wdt_hw_stop();
> +	struct watchdog_device *wdd = platform_get_drvdata(pdev);
>
> -	misc_deregister(&bcm63xx_wdt_miscdev);
>   	bcm63xx_timer_unregister(TIMER_WDT_ID);
> +	watchdog_unregister_device(wdd);

Shouldn't that come first, before unregistering the timer ?

>   	return 0;
>   }
>
>   static void bcm63xx_wdt_shutdown(struct platform_device *pdev)
>   {
> -	bcm63xx_wdt_hw_stop();
> +	struct watchdog_device *wdd = platform_get_drvdata(pdev);
> +
> +	bcm63xx_wdt_stop(wdd);
>   }
>
>   static struct platform_driver bcm63xx_wdt_driver = {
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1277377 — Re: [PATCH 5/10] watchdog: bcm63xx_wdt: Use WATCHDOG_CORE

On Wed, November 25, 2015 02:44, Guenter Roeck wrote:
> The "running" flag should no longer be needed. watchdog_active()
> should provide that information.

I'm going to need to keep that because I need to know if it's running
in the interrupt handler, and wdd->lock is a mutex.

>> @@ -306,17 +202,18 @@ unregister_timer:
>>
>>   static int bcm63xx_wdt_remove(struct platform_device *pdev)
>>   {
>> -	if (!nowayout)
>> -		bcm63xx_wdt_hw_stop();
>> +	struct watchdog_device *wdd = platform_get_drvdata(pdev);
>>
>> -	misc_deregister(&bcm63xx_wdt_miscdev);
>>   	bcm63xx_timer_unregister(TIMER_WDT_ID);
>> +	watchdog_unregister_device(wdd);
>
> Shouldn't that come first, before unregistering the timer ?

No, because wdd->dev is used in the interrupt handler. I will have to
move registration of the interrupt to after creating the watchdog
because it could currently be used before wdd->dev is set.

-- 
Simon Arlott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1277417 — Re: [PATCH 5/10] watchdog: bcm63xx_wdt: Use WATCHDOG_CORE

On 11/25/2015 05:02 AM, Simon Arlott wrote:
> On Wed, November 25, 2015 02:44, Guenter Roeck wrote:
>> The "running" flag should no longer be needed. watchdog_active()
>> should provide that information.
>
> I'm going to need to keep that because I need to know if it's running
> in the interrupt handler, and wdd->lock is a mutex.
>
>>> @@ -306,17 +202,18 @@ unregister_timer:
>>>
>>>    static int bcm63xx_wdt_remove(struct platform_device *pdev)
>>>    {
>>> -	if (!nowayout)
>>> -		bcm63xx_wdt_hw_stop();
>>> +	struct watchdog_device *wdd = platform_get_drvdata(pdev);
>>>
>>> -	misc_deregister(&bcm63xx_wdt_miscdev);
>>>    	bcm63xx_timer_unregister(TIMER_WDT_ID);
>>> +	watchdog_unregister_device(wdd);
>>
>> Shouldn't that come first, before unregistering the timer ?
>
> No, because wdd->dev is used in the interrupt handler. I will have to
> move registration of the interrupt to after creating the watchdog
> because it could currently be used before wdd->dev is set.
>

Does unregistering the timer disable the interrupt ?

Thanks,
Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1277747 — Re: [PATCH 5/10] watchdog: bcm63xx_wdt: Use WATCHDOG_CORE

On 25/11/15 14:10, Guenter Roeck wrote:
> On 11/25/2015 05:02 AM, Simon Arlott wrote:
>> On Wed, November 25, 2015 02:44, Guenter Roeck wrote:
>>> The "running" flag should no longer be needed. watchdog_active()
>>> should provide that information.
>>
>> I'm going to need to keep that because I need to know if it's running
>> in the interrupt handler, and wdd->lock is a mutex.
>>
>>>> @@ -306,17 +202,18 @@ unregister_timer:
>>>>
>>>>    static int bcm63xx_wdt_remove(struct platform_device *pdev)
>>>>    {
>>>> -	if (!nowayout)
>>>> -		bcm63xx_wdt_hw_stop();
>>>> +	struct watchdog_device *wdd = platform_get_drvdata(pdev);
>>>>
>>>> -	misc_deregister(&bcm63xx_wdt_miscdev);
>>>>    	bcm63xx_timer_unregister(TIMER_WDT_ID);
>>>> +	watchdog_unregister_device(wdd);
>>>
>>> Shouldn't that come first, before unregistering the timer ?
>>
>> No, because wdd->dev is used in the interrupt handler. I will have to
>> move registration of the interrupt to after creating the watchdog
>> because it could currently be used before wdd->dev is set.
>>
> 
> Does unregistering the timer disable the interrupt ?

No, it sets the callback for that timer to NULL so that it won't be
called.

-- 
Simon Arlott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1277859 — [PATCH (v3) 5/11] watchdog: bcm63xx_wdt: Use WATCHDOG_CORE

Convert bcm63xx_wdt to use WATCHDOG_CORE.

The default and maximum time constants that are only used once have been
moved to the initialisation of the struct watchdog_device.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
Patch 7 split into two patches.

On 25/11/15 02:44, Guenter Roeck wrote:
> If I see correctly, there is no ping function. In that case, the watchdog core
> will call the start function after updating the timeout, so there is no need
> to do it here.

Fixed.
 
>> +static const struct watchdog_info bcm63xx_wdt_info = {
>> +	.options = WDIOC_GETTIMELEFT | WDIOF_SETTIMEOUT |
> 
> Where is the gettimeleft function ? I think you are adding it with a later patch,
> but then you should set the flag there, not here.

Removed WDIOC_GETTIMELEFT completely because it's not a flag.

>> +	hw = devm_kzalloc(&pdev->dev, sizeof(*hw), GFP_KERNEL);
>> +	wdd = devm_kzalloc(&pdev->dev, sizeof(*wdd), GFP_KERNEL);
> 
> It would be better to allocate wdd as part of struct bcm63xx_wdt_hw.

Fixed.

>> -	misc_deregister(&bcm63xx_wdt_miscdev);
>>   	bcm63xx_timer_unregister(TIMER_WDT_ID);
>> +	watchdog_unregister_device(wdd);
> 
> Shouldn't that come first, before unregistering the timer ?

No, because wdd->dev is used in the interrupt handler. The handler will
not be called after bcm63xx_timer_unregister() is called.

Moved registration of the timer in the probe function to after register
of the watchdog device because the interrupt handler uses wdd->dev.

 drivers/watchdog/Kconfig       |   1 +
 drivers/watchdog/bcm63xx_wdt.c | 259 +++++++++++++----------------------------
 2 files changed, 79 insertions(+), 181 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 7a8a6c6..6815b74 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1273,6 +1273,7 @@ config OCTEON_WDT
 config BCM63XX_WDT
 	tristate "Broadcom BCM63xx hardware watchdog"
 	depends on BCM63XX
+	select WATCHDOG_CORE
 	help
 	  Watchdog driver for the built in watchdog hardware in Broadcom
 	  BCM63xx SoC.
diff --git a/drivers/watchdog/bcm63xx_wdt.c b/drivers/watchdog/bcm63xx_wdt.c
index 3f55cba..2257924 100644
--- a/drivers/watchdog/bcm63xx_wdt.c
+++ b/drivers/watchdog/bcm63xx_wdt.c
@@ -13,20 +13,15 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/bitops.h>
 #include <linux/errno.h>
-#include <linux/fs.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
-#include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/spinlock.h>
 #include <linux/types.h>
-#include <linux/uaccess.h>
 #include <linux/watchdog.h>
 #include <linux/interrupt.h>
-#include <linux/ptrace.h>
 #include <linux/resource.h>
 #include <linux/platform_device.h>
 
@@ -38,53 +33,59 @@
 #define PFX KBUILD_MODNAME
 
 #define WDT_HZ			50000000		/* Fclk */
-#define WDT_DEFAULT_TIME	30			/* seconds */
-#define WDT_MAX_TIME		(0xffffffff / WDT_HZ)	/* seconds */
 
 struct bcm63xx_wdt_hw {
+	struct watchdog_device wdd;
 	raw_spinlock_t lock;
 	void __iomem *regs;
-	unsigned long inuse;
 	bool running;
 };
-static struct bcm63xx_wdt_hw bcm63xx_wdt_device;
 
-static int expect_close;
+#define to_wdt_hw(x) container_of(x, struct bcm63xx_wdt_hw, wdd)
 
-static int wdt_time = WDT_DEFAULT_TIME;
 static bool nowayout = WATCHDOG_NOWAYOUT;
 module_param(nowayout, bool, 0);
 MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
 	__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
 
-/* HW functions */
-static void bcm63xx_wdt_hw_start(void)
+static int bcm63xx_wdt_start(struct watchdog_device *wdd)
 {
+	struct bcm63xx_wdt_hw *hw = to_wdt_hw(wdd);
 	unsigned long flags;
 
-	raw_spin_lock_irqsave(&bcm63xx_wdt_device.lock, flags);
-	bcm_writel(wdt_time * WDT_HZ, bcm63xx_wdt_device.regs + WDT_DEFVAL_REG);
-	bcm_writel(WDT_START_1, bcm63xx_wdt_device.regs + WDT_CTL_REG);
-	bcm_writel(WDT_START_2, bcm63xx_wdt_device.regs + WDT_CTL_REG);
-	bcm63xx_wdt_device.running = true;
-	raw_spin_unlock_irqrestore(&bcm63xx_wdt_device.lock, flags);
+	raw_spin_lock_irqsave(&hw->lock, flags);
+	bcm_writel(wdd->timeout * WDT_HZ, hw->regs + WDT_DEFVAL_REG);
+	bcm_writel(WDT_START_1, hw->regs + WDT_CTL_REG);
+	bcm_writel(WDT_START_2, hw->regs + WDT_CTL_REG);
+	hw->running = true;
+	raw_spin_unlock_irqrestore(&hw->lock, flags);
+	return 0;
 }
 
-static void bcm63xx_wdt_hw_stop(void)
+static int bcm63xx_wdt_stop(struct watchdog_device *wdd)
 {
+	struct bcm63xx_wdt_hw *hw = to_wdt_hw(wdd);
 	unsigned long flags;
 
-	raw_spin_lock_irqsave(&bcm63xx_wdt_device.lock, flags);
-	bcm_writel(WDT_STOP_1, bcm63xx_wdt_device.regs + WDT_CTL_REG);
-	bcm_writel(WDT_STOP_2, bcm63xx_wdt_device.regs + WDT_CTL_REG);
-	bcm63xx_wdt_device.running = false;
-	raw_spin_unlock_irqrestore(&bcm63xx_wdt_device.lock, flags);
+	raw_spin_lock_irqsave(&hw->lock, flags);
+	bcm_writel(WDT_STOP_1, hw->regs + WDT_CTL_REG);
+	bcm_writel(WDT_STOP_2, hw->regs + WDT_CTL_REG);
+	hw->running = false;
+	raw_spin_unlock_irqrestore(&hw->lock, flags);
+	return 0;
+}
+
+static int bcm63xx_wdt_set_timeout(struct watchdog_device *wdd,
+	unsigned int timeout)
+{
+	wdd->timeout = timeout;
+	return 0;
 }
 
 /* The watchdog interrupt occurs when half the timeout is remaining */
 static void bcm63xx_wdt_isr(void *data)
 {
-	struct bcm63xx_wdt_hw *hw = &bcm63xx_wdt_device;
+	struct bcm63xx_wdt_hw *hw = data;
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&hw->lock, flags);
@@ -118,147 +119,36 @@ static void bcm63xx_wdt_isr(void *data)
 		}
 
 		ms = timeleft / (WDT_HZ / 1000);
-		pr_alert("warning timer fired, reboot in %ums\n", ms);
+		dev_alert(hw->wdd.dev,
+			"warning timer fired, reboot in %ums\n", ms);
 	}
 	raw_spin_unlock_irqrestore(&hw->lock, flags);
 }
 
-static int bcm63xx_wdt_settimeout(int new_time)
-{
-	if ((new_time <= 0) || (new_time > WDT_MAX_TIME))
-		return -EINVAL;
-
-	wdt_time = new_time;
-
-	return 0;
-}
-
-static int bcm63xx_wdt_open(struct inode *inode, struct file *file)
-{
-	if (test_and_set_bit(0, &bcm63xx_wdt_device.inuse))
-		return -EBUSY;
-
-	bcm63xx_wdt_hw_start();
-	return nonseekable_open(inode, file);
-}
-
-static int bcm63xx_wdt_release(struct inode *inode, struct file *file)
-{
-	if (expect_close == 42)
-		bcm63xx_wdt_hw_stop();
-	else {
-		pr_crit("Unexpected close, not stopping watchdog!\n");
-		bcm63xx_wdt_hw_start();
-	}
-	clear_bit(0, &bcm63xx_wdt_device.inuse);
-	expect_close = 0;
-	return 0;
-}
-
-static ssize_t bcm63xx_wdt_write(struct file *file, const char *data,
-				size_t len, loff_t *ppos)
-{
-	if (len) {
-		if (!nowayout) {
-			size_t i;
-
-			/* In case it was set long ago */
-			expect_close = 0;
-
-			for (i = 0; i != len; i++) {
-				char c;
-				if (get_user(c, data + i))
-					return -EFAULT;
-				if (c == 'V')
-					expect_close = 42;
-			}
-		}
-		bcm63xx_wdt_hw_start();
-	}
-	return len;
-}
-
-static struct watchdog_info bcm63xx_wdt_info = {
-	.identity       = PFX,
-	.options        = WDIOF_SETTIMEOUT |
-				WDIOF_KEEPALIVEPING |
-				WDIOF_MAGICCLOSE,
+static struct watchdog_ops bcm63xx_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = bcm63xx_wdt_start,
+	.stop = bcm63xx_wdt_stop,
+	.set_timeout = bcm63xx_wdt_set_timeout,
 };
 
-
-static long bcm63xx_wdt_ioctl(struct file *file, unsigned int cmd,
-				unsigned long arg)
-{
-	void __user *argp = (void __user *)arg;
-	int __user *p = argp;
-	int new_value, retval = -EINVAL;
-
-	switch (cmd) {
-	case WDIOC_GETSUPPORT:
-		return copy_to_user(argp, &bcm63xx_wdt_info,
-			sizeof(bcm63xx_wdt_info)) ? -EFAULT : 0;
-
-	case WDIOC_GETSTATUS:
-	case WDIOC_GETBOOTSTATUS:
-		return put_user(0, p);
-
-	case WDIOC_SETOPTIONS:
-		if (get_user(new_value, p))
-			return -EFAULT;
-
-		if (new_value & WDIOS_DISABLECARD) {
-			bcm63xx_wdt_hw_stop();
-			retval = 0;
-		}
-		if (new_value & WDIOS_ENABLECARD) {
-			bcm63xx_wdt_hw_start();
-			retval = 0;
-		}
-
-		return retval;
-
-	case WDIOC_KEEPALIVE:
-		bcm63xx_wdt_hw_start();
-		return 0;
-
-	case WDIOC_SETTIMEOUT:
-		if (get_user(new_value, p))
-			return -EFAULT;
-
-		if (bcm63xx_wdt_settimeout(new_value))
-			return -EINVAL;
-
-		bcm63xx_wdt_hw_start();
-
-	case WDIOC_GETTIMEOUT:
-		return put_user(wdt_time, p);
-
-	default:
-		return -ENOTTY;
-
-	}
-}
-
-static const struct file_operations bcm63xx_wdt_fops = {
-	.owner		= THIS_MODULE,
-	.llseek		= no_llseek,
-	.write		= bcm63xx_wdt_write,
-	.unlocked_ioctl	= bcm63xx_wdt_ioctl,
-	.open		= bcm63xx_wdt_open,
-	.release	= bcm63xx_wdt_release,
-};
-
-static struct miscdevice bcm63xx_wdt_miscdev = {
-	.minor	= WATCHDOG_MINOR,
-	.name	= "watchdog",
-	.fops	= &bcm63xx_wdt_fops,
+static const struct watchdog_info bcm63xx_wdt_info = {
+	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+	.identity = "BCM63xx Watchdog",
 };
 
-
 static int bcm63xx_wdt_probe(struct platform_device *pdev)
 {
-	int ret;
+	struct bcm63xx_wdt_hw *hw;
+	struct watchdog_device *wdd;
 	struct resource *r;
+	int ret;
+
+	hw = devm_kzalloc(&pdev->dev, sizeof(*hw), GFP_KERNEL);
+	if (!hw)
+		return -ENOMEM;
+
+	wdd = &hw->wdd;
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!r) {
@@ -266,58 +156,65 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	bcm63xx_wdt_device.regs = devm_ioremap_nocache(&pdev->dev, r->start,
-							resource_size(r));
-	if (!bcm63xx_wdt_device.regs) {
+	hw->regs = devm_ioremap_nocache(&pdev->dev, r->start, resource_size(r));
+	if (!hw->regs) {
 		dev_err(&pdev->dev, "failed to remap I/O resources\n");
 		return -ENXIO;
 	}
 
-	raw_spin_lock_init(&bcm63xx_wdt_device.lock);
-	bcm63xx_wdt_device.running = false;
+	raw_spin_lock_init(&hw->lock);
+	hw->running = false;
 
-	ret = bcm63xx_timer_register(TIMER_WDT_ID, bcm63xx_wdt_isr, NULL);
+	wdd->parent = &pdev->dev;
+	wdd->ops = &bcm63xx_wdt_ops;
+	wdd->info = &bcm63xx_wdt_info;
+	wdd->min_timeout = 1;
+	wdd->max_timeout = 0xffffffff / WDT_HZ;
+	wdd->timeout = min(30U, wdd->max_timeout);
+
+	platform_set_drvdata(pdev, hw);
+
+	watchdog_init_timeout(wdd, 0, &pdev->dev);
+	watchdog_set_nowayout(wdd, nowayout);
+
+	ret = watchdog_register_device(wdd);
 	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to register wdt timer isr\n");
+		dev_err(&pdev->dev, "failed to register watchdog device\n");
 		return ret;
 	}
 
-	if (bcm63xx_wdt_settimeout(wdt_time)) {
-		bcm63xx_wdt_settimeout(WDT_DEFAULT_TIME);
-		dev_info(&pdev->dev,
-			": wdt_time value must be 1 <= wdt_time <= %d, using %d\n",
-			WDT_MAX_TIME, wdt_time);
-	}
-
-	ret = misc_register(&bcm63xx_wdt_miscdev);
+	ret = bcm63xx_timer_register(TIMER_WDT_ID, bcm63xx_wdt_isr, hw);
 	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to register watchdog device\n");
-		goto unregister_timer;
+		dev_err(&pdev->dev, "failed to register wdt timer isr\n");
+		goto unregister_watchdog;
 	}
 
-	dev_info(&pdev->dev, " started, timer margin: %d sec\n",
-						WDT_DEFAULT_TIME);
+	dev_info(&pdev->dev,
+		"%s at MMIO 0x%p (timeout = %us, max_timeout = %us)",
+		dev_name(wdd->dev), hw->regs,
+		wdd->timeout, wdd->max_timeout);
 
 	return 0;
 
-unregister_timer:
-	bcm63xx_timer_unregister(TIMER_WDT_ID);
+unregister_watchdog:
+	watchdog_unregister_device(wdd);
 	return ret;
 }
 
 static int bcm63xx_wdt_remove(struct platform_device *pdev)
 {
-	if (!nowayout)
-		bcm63xx_wdt_hw_stop();
+	struct bcm63xx_wdt_hw *hw = platform_get_drvdata(pdev);
 
-	misc_deregister(&bcm63xx_wdt_miscdev);
 	bcm63xx_timer_unregister(TIMER_WDT_ID);
+	watchdog_unregister_device(&hw->wdd);
 	return 0;
 }
 
 static void bcm63xx_wdt_shutdown(struct platform_device *pdev)
 {
-	bcm63xx_wdt_hw_stop();
+	struct bcm63xx_wdt_hw *hw = platform_get_drvdata(pdev);
+
+	bcm63xx_wdt_stop(&hw->wdd);
 }
 
 static struct platform_driver bcm63xx_wdt_driver = {
-- 
2.1.4

-- 
Simon Arlott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1274878 — [PATCH 4/10] watchdog: bcm63xx_wdt: Handle hardware interrupt and remove software timer

There is a level triggered interrupt for the watchdog timer as part of
the bcm63xx_timer device. The interrupt occurs when the hardware watchdog
timer reaches 50% of the remaining time.

It is not possible to mask the interrupt within the bcm63xx_timer device.
To get around this limitation, handle the interrupt by restarting the
watchdog with the current remaining time (which will be half the previous
timeout) so that the interrupt occurs again at 1/4th, 1/8th, etc. of the
original timeout value until the watchdog forces a reboot.

The software timer was restarting the hardware watchdog with a 85 second
timeout until the software timer expired, and then causing a panic()
about 42.5 seconds later when the hardware interrupt occurred. The
hardware watchdog would not reboot until a further 42.5 seconds had
passed.

Remove the software timer and rely on the hardware timer directly,
reducing the maximum timeout from 256 seconds to 85 seconds
(2^32 / WDT_HZ).

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
 drivers/watchdog/bcm63xx_wdt.c | 124 ++++++++++++++++++++++++-----------------
 1 file changed, 72 insertions(+), 52 deletions(-)

diff --git a/drivers/watchdog/bcm63xx_wdt.c b/drivers/watchdog/bcm63xx_wdt.c
index ab26fd9..f88fc97 100644
--- a/drivers/watchdog/bcm63xx_wdt.c
+++ b/drivers/watchdog/bcm63xx_wdt.c
@@ -3,6 +3,7 @@
  *
  *  Copyright (C) 2007, Miguel Gaio <miguel.gaio@efixo.com>
  *  Copyright (C) 2008, Florian Fainelli <florian@openwrt.org>
+ *  Copyright 2015 Simon Arlott
  *
  *  This program is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU General Public License
@@ -20,11 +21,10 @@
 #include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
+#include <linux/spinlock.h>
 #include <linux/types.h>
 #include <linux/uaccess.h>
 #include <linux/watchdog.h>
-#include <linux/timer.h>
-#include <linux/jiffies.h>
 #include <linux/interrupt.h>
 #include <linux/ptrace.h>
 #include <linux/resource.h>
@@ -37,16 +37,17 @@
 
 #define PFX KBUILD_MODNAME
 
-#define WDT_HZ		50000000 /* Fclk */
-#define WDT_DEFAULT_TIME	30      /* seconds */
-#define WDT_MAX_TIME		256     /* seconds */
+#define WDT_HZ			50000000		/* Fclk */
+#define WDT_DEFAULT_TIME	30			/* seconds */
+#define WDT_MAX_TIME		(0xffffffff / WDT_HZ)	/* seconds */
 
-static struct {
+struct bcm63xx_wdt_hw {
+	raw_spinlock_t lock;
 	void __iomem *regs;
-	struct timer_list timer;
 	unsigned long inuse;
-	atomic_t ticks;
-} bcm63xx_wdt_device;
+	bool running;
+};
+static struct bcm63xx_wdt_hw bcm63xx_wdt_device;
 
 static int expect_close;
 
@@ -59,48 +60,67 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
 /* HW functions */
 static void bcm63xx_wdt_hw_start(void)
 {
-	bcm_writel(0xfffffffe, bcm63xx_wdt_device.regs + WDT_DEFVAL_REG);
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&bcm63xx_wdt_device.lock, flags);
+	bcm_writel(wdt_time * WDT_HZ, bcm63xx_wdt_device.regs + WDT_DEFVAL_REG);
 	bcm_writel(WDT_START_1, bcm63xx_wdt_device.regs + WDT_CTL_REG);
 	bcm_writel(WDT_START_2, bcm63xx_wdt_device.regs + WDT_CTL_REG);
+	bcm63xx_wdt_device.running = true;
+	raw_spin_unlock_irqrestore(&bcm63xx_wdt_device.lock, flags);
 }
 
 static void bcm63xx_wdt_hw_stop(void)
 {
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&bcm63xx_wdt_device.lock, flags);
 	bcm_writel(WDT_STOP_1, bcm63xx_wdt_device.regs + WDT_CTL_REG);
 	bcm_writel(WDT_STOP_2, bcm63xx_wdt_device.regs + WDT_CTL_REG);
+	bcm63xx_wdt_device.running = false;
+	raw_spin_unlock_irqrestore(&bcm63xx_wdt_device.lock, flags);
 }
 
+/* The watchdog interrupt occurs when half the timeout is remaining */
 static void bcm63xx_wdt_isr(void *data)
 {
-	struct pt_regs *regs = get_irq_regs();
-
-	die(PFX " fire", regs);
-}
-
-static void bcm63xx_timer_tick(unsigned long unused)
-{
-	if (!atomic_dec_and_test(&bcm63xx_wdt_device.ticks)) {
-		bcm63xx_wdt_hw_start();
-		mod_timer(&bcm63xx_wdt_device.timer, jiffies + HZ);
-	} else
-		pr_crit("watchdog will restart system\n");
-}
-
-static void bcm63xx_wdt_pet(void)
-{
-	atomic_set(&bcm63xx_wdt_device.ticks, wdt_time);
-}
-
-static void bcm63xx_wdt_start(void)
-{
-	bcm63xx_wdt_pet();
-	bcm63xx_timer_tick(0);
-}
+	struct bcm63xx_wdt_hw *hw = &bcm63xx_wdt_device;
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&hw->lock, flags);
+	if (!hw->running) {
+		/* Stop the watchdog as it shouldn't be running */
+		bcm_writel(WDT_STOP_1, hw->regs + WDT_CTL_REG);
+		bcm_writel(WDT_STOP_2, hw->regs + WDT_CTL_REG);
+	} else {
+		u32 timeleft = bcm_readl(hw->regs + WDT_CTL_REG);
+		u32 ms;
+
+		if (timeleft >= 2) {
+			/* The only way to clear this level triggered interrupt
+			 * without disrupting the normal running of the watchdog
+			 * is to restart the watchdog with the current remaining
+			 * time value (which will be half the previous timeout)
+			 * so the interrupt occurs again at 1/4th, 1/8th, etc.
+			 * of the original timeout value until we reboot.
+			 *
+			 * This is done with a lock held in case userspace is
+			 * trying to restart the watchdog on another CPU.
+			 */
+			bcm_writel(timeleft, hw->regs + WDT_DEFVAL_REG);
+			bcm_writel(WDT_START_1, hw->regs + WDT_CTL_REG);
+			bcm_writel(WDT_START_2, hw->regs + WDT_CTL_REG);
+		} else {
+			/* The watchdog cannot be started with a time of less
+			 * than 2 ticks (it won't fire).
+			 */
+			die(PFX ": watchdog timer expired\n", get_irq_regs());
+		}
 
-static void bcm63xx_wdt_pause(void)
-{
-	del_timer_sync(&bcm63xx_wdt_device.timer);
-	bcm63xx_wdt_hw_stop();
+		ms = timeleft / (WDT_HZ / 1000);
+		pr_alert("warning timer fired, reboot in %ums\n", ms);
+	}
+	raw_spin_unlock_irqrestore(&hw->lock, flags);
 }
 
 static int bcm63xx_wdt_settimeout(int new_time)
@@ -118,17 +138,17 @@ static int bcm63xx_wdt_open(struct inode *inode, struct file *file)
 	if (test_and_set_bit(0, &bcm63xx_wdt_device.inuse))
 		return -EBUSY;
 
-	bcm63xx_wdt_start();
+	bcm63xx_wdt_hw_start();
 	return nonseekable_open(inode, file);
 }
 
 static int bcm63xx_wdt_release(struct inode *inode, struct file *file)
 {
 	if (expect_close == 42)
-		bcm63xx_wdt_pause();
+		bcm63xx_wdt_hw_stop();
 	else {
 		pr_crit("Unexpected close, not stopping watchdog!\n");
-		bcm63xx_wdt_start();
+		bcm63xx_wdt_hw_start();
 	}
 	clear_bit(0, &bcm63xx_wdt_device.inuse);
 	expect_close = 0;
@@ -153,7 +173,7 @@ static ssize_t bcm63xx_wdt_write(struct file *file, const char *data,
 					expect_close = 42;
 			}
 		}
-		bcm63xx_wdt_pet();
+		bcm63xx_wdt_hw_start();
 	}
 	return len;
 }
@@ -187,18 +207,18 @@ static long bcm63xx_wdt_ioctl(struct file *file, unsigned int cmd,
 			return -EFAULT;
 
 		if (new_value & WDIOS_DISABLECARD) {
-			bcm63xx_wdt_pause();
+			bcm63xx_wdt_hw_stop();
 			retval = 0;
 		}
 		if (new_value & WDIOS_ENABLECARD) {
-			bcm63xx_wdt_start();
+			bcm63xx_wdt_hw_start();
 			retval = 0;
 		}
 
 		return retval;
 
 	case WDIOC_KEEPALIVE:
-		bcm63xx_wdt_pet();
+		bcm63xx_wdt_hw_start();
 		return 0;
 
 	case WDIOC_SETTIMEOUT:
@@ -208,7 +228,7 @@ static long bcm63xx_wdt_ioctl(struct file *file, unsigned int cmd,
 		if (bcm63xx_wdt_settimeout(new_value))
 			return -EINVAL;
 
-		bcm63xx_wdt_pet();
+		bcm63xx_wdt_hw_start();
 
 	case WDIOC_GETTIMEOUT:
 		return put_user(wdt_time, p);
@@ -240,8 +260,6 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
 	int ret;
 	struct resource *r;
 
-	setup_timer(&bcm63xx_wdt_device.timer, bcm63xx_timer_tick, 0L);
-
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!r) {
 		dev_err(&pdev->dev, "failed to get resources\n");
@@ -255,6 +273,8 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
+	raw_spin_lock_init(&bcm63xx_wdt_device.lock);
+
 	ret = bcm63xx_timer_register(TIMER_WDT_ID, bcm63xx_wdt_isr, NULL);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to register wdt timer isr\n");
@@ -264,8 +284,8 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
 	if (bcm63xx_wdt_settimeout(wdt_time)) {
 		bcm63xx_wdt_settimeout(WDT_DEFAULT_TIME);
 		dev_info(&pdev->dev,
-			": wdt_time value must be 1 <= wdt_time <= 256, using %d\n",
-			wdt_time);
+			": wdt_time value must be 1 <= wdt_time <= %d, using %d\n",
+			WDT_MAX_TIME, wdt_time);
 	}
 
 	ret = misc_register(&bcm63xx_wdt_miscdev);
@@ -287,7 +307,7 @@ unregister_timer:
 static int bcm63xx_wdt_remove(struct platform_device *pdev)
 {
 	if (!nowayout)
-		bcm63xx_wdt_pause();
+		bcm63xx_wdt_hw_stop();
 
 	misc_deregister(&bcm63xx_wdt_miscdev);
 	bcm63xx_timer_unregister(TIMER_WDT_ID);
@@ -296,7 +316,7 @@ static int bcm63xx_wdt_remove(struct platform_device *pdev)
 
 static void bcm63xx_wdt_shutdown(struct platform_device *pdev)
 {
-	bcm63xx_wdt_pause();
+	bcm63xx_wdt_hw_stop();
 }
 
 static struct platform_driver bcm63xx_wdt_driver = {
-- 
2.1.4

-- 
Simon Arlott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1276708 — Re: [PATCH 4/10] watchdog: bcm63xx_wdt: Handle hardware interrupt and remove software timer

On Sun, Nov 22, 2015 at 02:05:16PM +0000, Simon Arlott wrote:
> There is a level triggered interrupt for the watchdog timer as part of
> the bcm63xx_timer device. The interrupt occurs when the hardware watchdog
> timer reaches 50% of the remaining time.
> 
> It is not possible to mask the interrupt within the bcm63xx_timer device.
> To get around this limitation, handle the interrupt by restarting the
> watchdog with the current remaining time (which will be half the previous
> timeout) so that the interrupt occurs again at 1/4th, 1/8th, etc. of the
> original timeout value until the watchdog forces a reboot.
> 
> The software timer was restarting the hardware watchdog with a 85 second
> timeout until the software timer expired, and then causing a panic()
> about 42.5 seconds later when the hardware interrupt occurred. The
> hardware watchdog would not reboot until a further 42.5 seconds had
> passed.
> 
> Remove the software timer and rely on the hardware timer directly,
> reducing the maximum timeout from 256 seconds to 85 seconds
> (2^32 / WDT_HZ).
> 

Florian,

can you have a look into this patch and confirm that there is no better
way to clear the interrupt status ?

Thanks,
Guenter

> Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
> ---
>  drivers/watchdog/bcm63xx_wdt.c | 124 ++++++++++++++++++++++++-----------------
>  1 file changed, 72 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/watchdog/bcm63xx_wdt.c b/drivers/watchdog/bcm63xx_wdt.c
> index ab26fd9..f88fc97 100644
> --- a/drivers/watchdog/bcm63xx_wdt.c
> +++ b/drivers/watchdog/bcm63xx_wdt.c
> @@ -3,6 +3,7 @@
>   *
>   *  Copyright (C) 2007, Miguel Gaio <miguel.gaio@efixo.com>
>   *  Copyright (C) 2008, Florian Fainelli <florian@openwrt.org>
> + *  Copyright 2015 Simon Arlott
>   *
>   *  This program is free software; you can redistribute it and/or
>   *  modify it under the terms of the GNU General Public License
> @@ -20,11 +21,10 @@
>  #include <linux/miscdevice.h>
>  #include <linux/module.h>
>  #include <linux/moduleparam.h>
> +#include <linux/spinlock.h>
>  #include <linux/types.h>
>  #include <linux/uaccess.h>
>  #include <linux/watchdog.h>
> -#include <linux/timer.h>
> -#include <linux/jiffies.h>
>  #include <linux/interrupt.h>
>  #include <linux/ptrace.h>
>  #include <linux/resource.h>
> @@ -37,16 +37,17 @@
>  
>  #define PFX KBUILD_MODNAME
>  
> -#define WDT_HZ		50000000 /* Fclk */
> -#define WDT_DEFAULT_TIME	30      /* seconds */
> -#define WDT_MAX_TIME		256     /* seconds */
> +#define WDT_HZ			50000000		/* Fclk */
> +#define WDT_DEFAULT_TIME	30			/* seconds */
> +#define WDT_MAX_TIME		(0xffffffff / WDT_HZ)	/* seconds */
>  
> -static struct {
> +struct bcm63xx_wdt_hw {
> +	raw_spinlock_t lock;
>  	void __iomem *regs;
> -	struct timer_list timer;
>  	unsigned long inuse;
> -	atomic_t ticks;
> -} bcm63xx_wdt_device;
> +	bool running;
> +};
> +static struct bcm63xx_wdt_hw bcm63xx_wdt_device;
>  
>  static int expect_close;
>  
> @@ -59,48 +60,67 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
>  /* HW functions */
>  static void bcm63xx_wdt_hw_start(void)
>  {
> -	bcm_writel(0xfffffffe, bcm63xx_wdt_device.regs + WDT_DEFVAL_REG);
> +	unsigned long flags;
> +
> +	raw_spin_lock_irqsave(&bcm63xx_wdt_device.lock, flags);
> +	bcm_writel(wdt_time * WDT_HZ, bcm63xx_wdt_device.regs + WDT_DEFVAL_REG);
>  	bcm_writel(WDT_START_1, bcm63xx_wdt_device.regs + WDT_CTL_REG);
>  	bcm_writel(WDT_START_2, bcm63xx_wdt_device.regs + WDT_CTL_REG);
> +	bcm63xx_wdt_device.running = true;
> +	raw_spin_unlock_irqrestore(&bcm63xx_wdt_device.lock, flags);
>  }
>  
>  static void bcm63xx_wdt_hw_stop(void)
>  {
> +	unsigned long flags;
> +
> +	raw_spin_lock_irqsave(&bcm63xx_wdt_device.lock, flags);
>  	bcm_writel(WDT_STOP_1, bcm63xx_wdt_device.regs + WDT_CTL_REG);
>  	bcm_writel(WDT_STOP_2, bcm63xx_wdt_device.regs + WDT_CTL_REG);
> +	bcm63xx_wdt_device.running = false;
> +	raw_spin_unlock_irqrestore(&bcm63xx_wdt_device.lock, flags);
>  }
>  
> +/* The watchdog interrupt occurs when half the timeout is remaining */
>  static void bcm63xx_wdt_isr(void *data)
>  {
> -	struct pt_regs *regs = get_irq_regs();
> -
> -	die(PFX " fire", regs);
> -}
> -
> -static void bcm63xx_timer_tick(unsigned long unused)
> -{
> -	if (!atomic_dec_and_test(&bcm63xx_wdt_device.ticks)) {
> -		bcm63xx_wdt_hw_start();
> -		mod_timer(&bcm63xx_wdt_device.timer, jiffies + HZ);
> -	} else
> -		pr_crit("watchdog will restart system\n");
> -}
> -
> -static void bcm63xx_wdt_pet(void)
> -{
> -	atomic_set(&bcm63xx_wdt_device.ticks, wdt_time);
> -}
> -
> -static void bcm63xx_wdt_start(void)
> -{
> -	bcm63xx_wdt_pet();
> -	bcm63xx_timer_tick(0);
> -}
> +	struct bcm63xx_wdt_hw *hw = &bcm63xx_wdt_device;
> +	unsigned long flags;
> +
> +	raw_spin_lock_irqsave(&hw->lock, flags);
> +	if (!hw->running) {
> +		/* Stop the watchdog as it shouldn't be running */
> +		bcm_writel(WDT_STOP_1, hw->regs + WDT_CTL_REG);
> +		bcm_writel(WDT_STOP_2, hw->regs + WDT_CTL_REG);
> +	} else {
> +		u32 timeleft = bcm_readl(hw->regs + WDT_CTL_REG);
> +		u32 ms;
> +
> +		if (timeleft >= 2) {
> +			/* The only way to clear this level triggered interrupt
> +			 * without disrupting the normal running of the watchdog
> +			 * is to restart the watchdog with the current remaining
> +			 * time value (which will be half the previous timeout)
> +			 * so the interrupt occurs again at 1/4th, 1/8th, etc.
> +			 * of the original timeout value until we reboot.
> +			 *
> +			 * This is done with a lock held in case userspace is
> +			 * trying to restart the watchdog on another CPU.
> +			 */
> +			bcm_writel(timeleft, hw->regs + WDT_DEFVAL_REG);
> +			bcm_writel(WDT_START_1, hw->regs + WDT_CTL_REG);
> +			bcm_writel(WDT_START_2, hw->regs + WDT_CTL_REG);
> +		} else {
> +			/* The watchdog cannot be started with a time of less
> +			 * than 2 ticks (it won't fire).
> +			 */
> +			die(PFX ": watchdog timer expired\n", get_irq_regs());
> +		}
>  
> -static void bcm63xx_wdt_pause(void)
> -{
> -	del_timer_sync(&bcm63xx_wdt_device.timer);
> -	bcm63xx_wdt_hw_stop();
> +		ms = timeleft / (WDT_HZ / 1000);
> +		pr_alert("warning timer fired, reboot in %ums\n", ms);
> +	}
> +	raw_spin_unlock_irqrestore(&hw->lock, flags);
>  }
>  
>  static int bcm63xx_wdt_settimeout(int new_time)
> @@ -118,17 +138,17 @@ static int bcm63xx_wdt_open(struct inode *inode, struct file *file)
>  	if (test_and_set_bit(0, &bcm63xx_wdt_device.inuse))
>  		return -EBUSY;
>  
> -	bcm63xx_wdt_start();
> +	bcm63xx_wdt_hw_start();
>  	return nonseekable_open(inode, file);
>  }
>  
>  static int bcm63xx_wdt_release(struct inode *inode, struct file *file)
>  {
>  	if (expect_close == 42)
> -		bcm63xx_wdt_pause();
> +		bcm63xx_wdt_hw_stop();
>  	else {
>  		pr_crit("Unexpected close, not stopping watchdog!\n");
> -		bcm63xx_wdt_start();
> +		bcm63xx_wdt_hw_start();
>  	}
>  	clear_bit(0, &bcm63xx_wdt_device.inuse);
>  	expect_close = 0;
> @@ -153,7 +173,7 @@ static ssize_t bcm63xx_wdt_write(struct file *file, const char *data,
>  					expect_close = 42;
>  			}
>  		}
> -		bcm63xx_wdt_pet();
> +		bcm63xx_wdt_hw_start();
>  	}
>  	return len;
>  }
> @@ -187,18 +207,18 @@ static long bcm63xx_wdt_ioctl(struct file *file, unsigned int cmd,
>  			return -EFAULT;
>  
>  		if (new_value & WDIOS_DISABLECARD) {
> -			bcm63xx_wdt_pause();
> +			bcm63xx_wdt_hw_stop();
>  			retval = 0;
>  		}
>  		if (new_value & WDIOS_ENABLECARD) {
> -			bcm63xx_wdt_start();
> +			bcm63xx_wdt_hw_start();
>  			retval = 0;
>  		}
>  
>  		return retval;
>  
>  	case WDIOC_KEEPALIVE:
> -		bcm63xx_wdt_pet();
> +		bcm63xx_wdt_hw_start();
>  		return 0;
>  
>  	case WDIOC_SETTIMEOUT:
> @@ -208,7 +228,7 @@ static long bcm63xx_wdt_ioctl(struct file *file, unsigned int cmd,
>  		if (bcm63xx_wdt_settimeout(new_value))
>  			return -EINVAL;
>  
> -		bcm63xx_wdt_pet();
> +		bcm63xx_wdt_hw_start();
>  
>  	case WDIOC_GETTIMEOUT:
>  		return put_user(wdt_time, p);
> @@ -240,8 +260,6 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
>  	int ret;
>  	struct resource *r;
>  
> -	setup_timer(&bcm63xx_wdt_device.timer, bcm63xx_timer_tick, 0L);
> -
>  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!r) {
>  		dev_err(&pdev->dev, "failed to get resources\n");
> @@ -255,6 +273,8 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
>  		return -ENXIO;
>  	}
>  
> +	raw_spin_lock_init(&bcm63xx_wdt_device.lock);
> +
>  	ret = bcm63xx_timer_register(TIMER_WDT_ID, bcm63xx_wdt_isr, NULL);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "failed to register wdt timer isr\n");
> @@ -264,8 +284,8 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
>  	if (bcm63xx_wdt_settimeout(wdt_time)) {
>  		bcm63xx_wdt_settimeout(WDT_DEFAULT_TIME);
>  		dev_info(&pdev->dev,
> -			": wdt_time value must be 1 <= wdt_time <= 256, using %d\n",
> -			wdt_time);
> +			": wdt_time value must be 1 <= wdt_time <= %d, using %d\n",
> +			WDT_MAX_TIME, wdt_time);
>  	}
>  
>  	ret = misc_register(&bcm63xx_wdt_miscdev);
> @@ -287,7 +307,7 @@ unregister_timer:
>  static int bcm63xx_wdt_remove(struct platform_device *pdev)
>  {
>  	if (!nowayout)
> -		bcm63xx_wdt_pause();
> +		bcm63xx_wdt_hw_stop();
>  
>  	misc_deregister(&bcm63xx_wdt_miscdev);
>  	bcm63xx_timer_unregister(TIMER_WDT_ID);
> @@ -296,7 +316,7 @@ static int bcm63xx_wdt_remove(struct platform_device *pdev)
>  
>  static void bcm63xx_wdt_shutdown(struct platform_device *pdev)
>  {
> -	bcm63xx_wdt_pause();
> +	bcm63xx_wdt_hw_stop();
>  }
>  
>  static struct platform_driver bcm63xx_wdt_driver = {
> -- 
> 2.1.4
> 
> -- 
> Simon Arlott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1277771 — Re: [PATCH 4/10] watchdog: bcm63xx_wdt: Handle hardware interrupt and remove software timer

On Tue, Nov 24, 2015 at 7:21 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> On Sun, Nov 22, 2015 at 02:05:16PM +0000, Simon Arlott wrote:
>> There is a level triggered interrupt for the watchdog timer as part of
>> the bcm63xx_timer device. The interrupt occurs when the hardware watchdog
>> timer reaches 50% of the remaining time.
>>
>> It is not possible to mask the interrupt within the bcm63xx_timer device.
>> To get around this limitation, handle the interrupt by restarting the
>> watchdog with the current remaining time (which will be half the previous
>> timeout) so that the interrupt occurs again at 1/4th, 1/8th, etc. of the
>> original timeout value until the watchdog forces a reboot.
>>
>> The software timer was restarting the hardware watchdog with a 85 second
>> timeout until the software timer expired, and then causing a panic()
>> about 42.5 seconds later when the hardware interrupt occurred. The
>> hardware watchdog would not reboot until a further 42.5 seconds had
>> passed.
>>
>> Remove the software timer and rely on the hardware timer directly,
>> reducing the maximum timeout from 256 seconds to 85 seconds
>> (2^32 / WDT_HZ).
>>
>
> Florian,
>
> can you have a look into this patch and confirm that there is no better
> way to clear the interrupt status ?

While the watchdog interrupt can't be masked, it should be able to be
cleared by writing 1 to the appropriate bit in the timer block's
interrupt status register. At least the broadcom sources do so.


Jonas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1277780 — Re: [PATCH 4/10] watchdog: bcm63xx_wdt: Handle hardware interrupt and remove software timer

On 25/11/15 20:14, Jonas Gorski wrote:
> On Tue, Nov 24, 2015 at 7:21 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>> On Sun, Nov 22, 2015 at 02:05:16PM +0000, Simon Arlott wrote:
>>> There is a level triggered interrupt for the watchdog timer as part of
>>> the bcm63xx_timer device. The interrupt occurs when the hardware watchdog
>>> timer reaches 50% of the remaining time.
>>>
>>> It is not possible to mask the interrupt within the bcm63xx_timer device.
>>> To get around this limitation, handle the interrupt by restarting the
>>> watchdog with the current remaining time (which will be half the previous
>>> timeout) so that the interrupt occurs again at 1/4th, 1/8th, etc. of the
>>> original timeout value until the watchdog forces a reboot.
>>>
>>> The software timer was restarting the hardware watchdog with a 85 second
>>> timeout until the software timer expired, and then causing a panic()
>>> about 42.5 seconds later when the hardware interrupt occurred. The
>>> hardware watchdog would not reboot until a further 42.5 seconds had
>>> passed.
>>>
>>> Remove the software timer and rely on the hardware timer directly,
>>> reducing the maximum timeout from 256 seconds to 85 seconds
>>> (2^32 / WDT_HZ).
>>>
>>
>> Florian,
>>
>> can you have a look into this patch and confirm that there is no better
>> way to clear the interrupt status ?
> 
> While the watchdog interrupt can't be masked, it should be able to be
> cleared by writing 1 to the appropriate bit in the timer block's
> interrupt status register. At least the broadcom sources do so.

Not according to the hardware itself:
[    6.674626] watchdog watchdog0: warning timer fired, reboot in 7499ms
[    6.681212] irq_bcm6345_l2_timer: bcm6345_timer_write_int_status: b0000083=08
[    6.688583] watchdog watchdog0: warning timer fired, reboot in 7486ms
[    6.695181] irq_bcm6345_l2_timer: bcm6345_timer_write_int_status: b0000083=08
[    6.702554] watchdog watchdog0: warning timer fired, reboot in 7472ms
[    6.709158] irq_bcm6345_l2_timer: bcm6345_timer_write_int_status: b0000083=08
[    6.716529] watchdog watchdog0: warning timer fired, reboot in 7458ms
[    6.723135] irq_bcm6345_l2_timer: bcm6345_timer_write_int_status: b0000083=08
[    6.730538] watchdog watchdog0: warning timer fired, reboot in 7444ms
[    6.737121] irq_bcm6345_l2_timer: bcm6345_timer_write_int_status: b0000083=08
[    6.744482] watchdog watchdog0: warning timer fired, reboot in 7430ms
[    6.751090] irq_bcm6345_l2_timer: bcm6345_timer_write_int_status: b0000083=08


typedef struct Timer {
    uint16        unused0;
    byte          TimerMask;
#define TIMER0EN        0x01
#define TIMER1EN        0x02
#define TIMER2EN        0x04
    byte          TimerInts;
#define TIMER0          0x01
#define TIMER1          0x02
#define TIMER2          0x04
#define WATCHDOG        0x08
...

-- 
Simon Arlott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1277851 — [PATCH (v2) 4/10] watchdog: bcm63xx_wdt: Handle hardware interrupt and remove software timer

There is a level triggered interrupt for the watchdog timer as part of
the bcm63xx_timer device. The interrupt occurs when the hardware watchdog
timer reaches 50% of the remaining time.

It is not possible to mask the interrupt within the bcm63xx_timer device.
To get around this limitation, handle the interrupt by restarting the
watchdog with the current remaining time (which will be half the previous
timeout) so that the interrupt occurs again at 1/4th, 1/8th, etc. of the
original timeout value until the watchdog forces a reboot.

The software timer was restarting the hardware watchdog with a 85 second
timeout until the software timer expired, and then causing a panic()
about 42.5 seconds later when the hardware interrupt occurred. The
hardware watchdog would not reboot until a further 42.5 seconds had
passed.

Remove the software timer and rely on the hardware timer directly,
reducing the maximum timeout from 256 seconds to 85 seconds
(2^32 / WDT_HZ).

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
Initialise bcm63xx_wdt_device.running to false.

 drivers/watchdog/bcm63xx_wdt.c | 125 ++++++++++++++++++++++++-----------------
 1 file changed, 73 insertions(+), 52 deletions(-)

diff --git a/drivers/watchdog/bcm63xx_wdt.c b/drivers/watchdog/bcm63xx_wdt.c
index ab26fd9..3f55cba 100644
--- a/drivers/watchdog/bcm63xx_wdt.c
+++ b/drivers/watchdog/bcm63xx_wdt.c
@@ -3,6 +3,7 @@
  *
  *  Copyright (C) 2007, Miguel Gaio <miguel.gaio@efixo.com>
  *  Copyright (C) 2008, Florian Fainelli <florian@openwrt.org>
+ *  Copyright 2015 Simon Arlott
  *
  *  This program is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU General Public License
@@ -20,11 +21,10 @@
 #include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
+#include <linux/spinlock.h>
 #include <linux/types.h>
 #include <linux/uaccess.h>
 #include <linux/watchdog.h>
-#include <linux/timer.h>
-#include <linux/jiffies.h>
 #include <linux/interrupt.h>
 #include <linux/ptrace.h>
 #include <linux/resource.h>
@@ -37,16 +37,17 @@
 
 #define PFX KBUILD_MODNAME
 
-#define WDT_HZ		50000000 /* Fclk */
-#define WDT_DEFAULT_TIME	30      /* seconds */
-#define WDT_MAX_TIME		256     /* seconds */
+#define WDT_HZ			50000000		/* Fclk */
+#define WDT_DEFAULT_TIME	30			/* seconds */
+#define WDT_MAX_TIME		(0xffffffff / WDT_HZ)	/* seconds */
 
-static struct {
+struct bcm63xx_wdt_hw {
+	raw_spinlock_t lock;
 	void __iomem *regs;
-	struct timer_list timer;
 	unsigned long inuse;
-	atomic_t ticks;
-} bcm63xx_wdt_device;
+	bool running;
+};
+static struct bcm63xx_wdt_hw bcm63xx_wdt_device;
 
 static int expect_close;
 
@@ -59,48 +60,67 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
 /* HW functions */
 static void bcm63xx_wdt_hw_start(void)
 {
-	bcm_writel(0xfffffffe, bcm63xx_wdt_device.regs + WDT_DEFVAL_REG);
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&bcm63xx_wdt_device.lock, flags);
+	bcm_writel(wdt_time * WDT_HZ, bcm63xx_wdt_device.regs + WDT_DEFVAL_REG);
 	bcm_writel(WDT_START_1, bcm63xx_wdt_device.regs + WDT_CTL_REG);
 	bcm_writel(WDT_START_2, bcm63xx_wdt_device.regs + WDT_CTL_REG);
+	bcm63xx_wdt_device.running = true;
+	raw_spin_unlock_irqrestore(&bcm63xx_wdt_device.lock, flags);
 }
 
 static void bcm63xx_wdt_hw_stop(void)
 {
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&bcm63xx_wdt_device.lock, flags);
 	bcm_writel(WDT_STOP_1, bcm63xx_wdt_device.regs + WDT_CTL_REG);
 	bcm_writel(WDT_STOP_2, bcm63xx_wdt_device.regs + WDT_CTL_REG);
+	bcm63xx_wdt_device.running = false;
+	raw_spin_unlock_irqrestore(&bcm63xx_wdt_device.lock, flags);
 }
 
+/* The watchdog interrupt occurs when half the timeout is remaining */
 static void bcm63xx_wdt_isr(void *data)
 {
-	struct pt_regs *regs = get_irq_regs();
-
-	die(PFX " fire", regs);
-}
-
-static void bcm63xx_timer_tick(unsigned long unused)
-{
-	if (!atomic_dec_and_test(&bcm63xx_wdt_device.ticks)) {
-		bcm63xx_wdt_hw_start();
-		mod_timer(&bcm63xx_wdt_device.timer, jiffies + HZ);
-	} else
-		pr_crit("watchdog will restart system\n");
-}
-
-static void bcm63xx_wdt_pet(void)
-{
-	atomic_set(&bcm63xx_wdt_device.ticks, wdt_time);
-}
-
-static void bcm63xx_wdt_start(void)
-{
-	bcm63xx_wdt_pet();
-	bcm63xx_timer_tick(0);
-}
+	struct bcm63xx_wdt_hw *hw = &bcm63xx_wdt_device;
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&hw->lock, flags);
+	if (!hw->running) {
+		/* Stop the watchdog as it shouldn't be running */
+		bcm_writel(WDT_STOP_1, hw->regs + WDT_CTL_REG);
+		bcm_writel(WDT_STOP_2, hw->regs + WDT_CTL_REG);
+	} else {
+		u32 timeleft = bcm_readl(hw->regs + WDT_CTL_REG);
+		u32 ms;
+
+		if (timeleft >= 2) {
+			/* The only way to clear this level triggered interrupt
+			 * without disrupting the normal running of the watchdog
+			 * is to restart the watchdog with the current remaining
+			 * time value (which will be half the previous timeout)
+			 * so the interrupt occurs again at 1/4th, 1/8th, etc.
+			 * of the original timeout value until we reboot.
+			 *
+			 * This is done with a lock held in case userspace is
+			 * trying to restart the watchdog on another CPU.
+			 */
+			bcm_writel(timeleft, hw->regs + WDT_DEFVAL_REG);
+			bcm_writel(WDT_START_1, hw->regs + WDT_CTL_REG);
+			bcm_writel(WDT_START_2, hw->regs + WDT_CTL_REG);
+		} else {
+			/* The watchdog cannot be started with a time of less
+			 * than 2 ticks (it won't fire).
+			 */
+			die(PFX ": watchdog timer expired\n", get_irq_regs());
+		}
 
-static void bcm63xx_wdt_pause(void)
-{
-	del_timer_sync(&bcm63xx_wdt_device.timer);
-	bcm63xx_wdt_hw_stop();
+		ms = timeleft / (WDT_HZ / 1000);
+		pr_alert("warning timer fired, reboot in %ums\n", ms);
+	}
+	raw_spin_unlock_irqrestore(&hw->lock, flags);
 }
 
 static int bcm63xx_wdt_settimeout(int new_time)
@@ -118,17 +138,17 @@ static int bcm63xx_wdt_open(struct inode *inode, struct file *file)
 	if (test_and_set_bit(0, &bcm63xx_wdt_device.inuse))
 		return -EBUSY;
 
-	bcm63xx_wdt_start();
+	bcm63xx_wdt_hw_start();
 	return nonseekable_open(inode, file);
 }
 
 static int bcm63xx_wdt_release(struct inode *inode, struct file *file)
 {
 	if (expect_close == 42)
-		bcm63xx_wdt_pause();
+		bcm63xx_wdt_hw_stop();
 	else {
 		pr_crit("Unexpected close, not stopping watchdog!\n");
-		bcm63xx_wdt_start();
+		bcm63xx_wdt_hw_start();
 	}
 	clear_bit(0, &bcm63xx_wdt_device.inuse);
 	expect_close = 0;
@@ -153,7 +173,7 @@ static ssize_t bcm63xx_wdt_write(struct file *file, const char *data,
 					expect_close = 42;
 			}
 		}
-		bcm63xx_wdt_pet();
+		bcm63xx_wdt_hw_start();
 	}
 	return len;
 }
@@ -187,18 +207,18 @@ static long bcm63xx_wdt_ioctl(struct file *file, unsigned int cmd,
 			return -EFAULT;
 
 		if (new_value & WDIOS_DISABLECARD) {
-			bcm63xx_wdt_pause();
+			bcm63xx_wdt_hw_stop();
 			retval = 0;
 		}
 		if (new_value & WDIOS_ENABLECARD) {
-			bcm63xx_wdt_start();
+			bcm63xx_wdt_hw_start();
 			retval = 0;
 		}
 
 		return retval;
 
 	case WDIOC_KEEPALIVE:
-		bcm63xx_wdt_pet();
+		bcm63xx_wdt_hw_start();
 		return 0;
 
 	case WDIOC_SETTIMEOUT:
@@ -208,7 +228,7 @@ static long bcm63xx_wdt_ioctl(struct file *file, unsigned int cmd,
 		if (bcm63xx_wdt_settimeout(new_value))
 			return -EINVAL;
 
-		bcm63xx_wdt_pet();
+		bcm63xx_wdt_hw_start();
 
 	case WDIOC_GETTIMEOUT:
 		return put_user(wdt_time, p);
@@ -240,8 +260,6 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
 	int ret;
 	struct resource *r;
 
-	setup_timer(&bcm63xx_wdt_device.timer, bcm63xx_timer_tick, 0L);
-
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!r) {
 		dev_err(&pdev->dev, "failed to get resources\n");
@@ -255,6 +273,9 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
+	raw_spin_lock_init(&bcm63xx_wdt_device.lock);
+	bcm63xx_wdt_device.running = false;
+
 	ret = bcm63xx_timer_register(TIMER_WDT_ID, bcm63xx_wdt_isr, NULL);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to register wdt timer isr\n");
@@ -264,8 +285,8 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
 	if (bcm63xx_wdt_settimeout(wdt_time)) {
 		bcm63xx_wdt_settimeout(WDT_DEFAULT_TIME);
 		dev_info(&pdev->dev,
-			": wdt_time value must be 1 <= wdt_time <= 256, using %d\n",
-			wdt_time);
+			": wdt_time value must be 1 <= wdt_time <= %d, using %d\n",
+			WDT_MAX_TIME, wdt_time);
 	}
 
 	ret = misc_register(&bcm63xx_wdt_miscdev);
@@ -287,7 +308,7 @@ unregister_timer:
 static int bcm63xx_wdt_remove(struct platform_device *pdev)
 {
 	if (!nowayout)
-		bcm63xx_wdt_pause();
+		bcm63xx_wdt_hw_stop();
 
 	misc_deregister(&bcm63xx_wdt_miscdev);
 	bcm63xx_timer_unregister(TIMER_WDT_ID);
@@ -296,7 +317,7 @@ static int bcm63xx_wdt_remove(struct platform_device *pdev)
 
 static void bcm63xx_wdt_shutdown(struct platform_device *pdev)
 {
-	bcm63xx_wdt_pause();
+	bcm63xx_wdt_hw_stop();
 }
 
 static struct platform_driver bcm63xx_wdt_driver = {
-- 
2.1.4

-- 
Simon Arlott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1274879 — [PATCH 8/10] watchdog: bcm63xx_wdt: Remove dependency on mach-bcm63xx functions/defines

The bcm_readl() and bcm_writel() functions are only defined for
mach-bcm63xx, replace them with __raw_readl() and raw_writel().

The register defines required for the watchdog are in a mach-bcm63xx
header file. Move them to include/linux/bcm63xx_wdt.h so that they are
also available on other machine types.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
 MAINTAINERS                                       |  1 +
 arch/mips/bcm63xx/prom.c                          |  1 +
 arch/mips/bcm63xx/setup.c                         |  1 +
 arch/mips/include/asm/mach-bcm63xx/bcm63xx_regs.h | 22 --------------------
 drivers/watchdog/bcm63xx_wdt.c                    | 25 +++++++++++------------
 include/linux/bcm63xx_wdt.h                       | 22 ++++++++++++++++++++
 6 files changed, 37 insertions(+), 35 deletions(-)
 create mode 100644 include/linux/bcm63xx_wdt.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 63bf54a..a27b99f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2376,6 +2376,7 @@ F:	arch/mips/boot/dts/brcm/bcm*.dts*
 F:	drivers/irqchip/irq-bcm63*
 F:	drivers/irqchip/irq-bcm7*
 F:	drivers/irqchip/irq-brcmstb*
+F:	include/linux/bcm63xx_wdt.h
 
 BROADCOM TG3 GIGABIT ETHERNET DRIVER
 M:	Prashant Sreedharan <prashant@broadcom.com>
diff --git a/arch/mips/bcm63xx/prom.c b/arch/mips/bcm63xx/prom.c
index 7019e29..d2800fb 100644
--- a/arch/mips/bcm63xx/prom.c
+++ b/arch/mips/bcm63xx/prom.c
@@ -6,6 +6,7 @@
  * Copyright (C) 2008 Maxime Bizon <mbizon@freebox.fr>
  */
 
+#include <linux/bcm63xx_wdt.h>
 #include <linux/init.h>
 #include <linux/bootmem.h>
 #include <linux/smp.h>
diff --git a/arch/mips/bcm63xx/setup.c b/arch/mips/bcm63xx/setup.c
index 240fb4f..d1d666d 100644
--- a/arch/mips/bcm63xx/setup.c
+++ b/arch/mips/bcm63xx/setup.c
@@ -6,6 +6,7 @@
  * Copyright (C) 2008 Maxime Bizon <mbizon@freebox.fr>
  */
 
+#include <linux/bcm63xx_wdt.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/delay.h>
diff --git a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_regs.h b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_regs.h
index 5035f09..16a745b 100644
--- a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_regs.h
+++ b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_regs.h
@@ -441,28 +441,6 @@
 
 
 /*************************************************************************
- * _REG relative to RSET_WDT
- *************************************************************************/
-
-/* Watchdog default count register */
-#define WDT_DEFVAL_REG			0x0
-
-/* Watchdog control register */
-#define WDT_CTL_REG			0x4
-
-/* Watchdog control register constants */
-#define WDT_START_1			(0xff00)
-#define WDT_START_2			(0x00ff)
-#define WDT_STOP_1			(0xee00)
-#define WDT_STOP_2			(0x00ee)
-
-/* Watchdog reset length register */
-#define WDT_RSTLEN_REG			0x8
-
-/* Watchdog soft reset register (BCM6328 only) */
-#define WDT_SOFTRESET_REG		0xc
-
-/*************************************************************************
  * _REG relative to RSET_GPIO
  *************************************************************************/
 
diff --git a/drivers/watchdog/bcm63xx_wdt.c b/drivers/watchdog/bcm63xx_wdt.c
index 7109eb4..7522624 100644
--- a/drivers/watchdog/bcm63xx_wdt.c
+++ b/drivers/watchdog/bcm63xx_wdt.c
@@ -13,6 +13,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/bcm63xx_wdt.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/errno.h>
@@ -27,8 +28,6 @@
 #include <linux/resource.h>
 #include <linux/platform_device.h>
 
-#include <bcm63xx_cpu.h>
-#include <bcm63xx_io.h>
 #include <bcm63xx_regs.h>
 #include <bcm63xx_timer.h>
 
@@ -55,9 +54,9 @@ static int bcm63xx_wdt_start(struct watchdog_device *wdd)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&hw->lock, flags);
-	bcm_writel(wdd->timeout * hw->clock_hz, hw->regs + WDT_DEFVAL_REG);
-	bcm_writel(WDT_START_1, hw->regs + WDT_CTL_REG);
-	bcm_writel(WDT_START_2, hw->regs + WDT_CTL_REG);
+	__raw_writel(wdd->timeout * hw->clock_hz, hw->regs + WDT_DEFVAL_REG);
+	__raw_writel(WDT_START_1, hw->regs + WDT_CTL_REG);
+	__raw_writel(WDT_START_2, hw->regs + WDT_CTL_REG);
 	hw->running = true;
 	raw_spin_unlock_irqrestore(&hw->lock, flags);
 	return 0;
@@ -69,8 +68,8 @@ static int bcm63xx_wdt_stop(struct watchdog_device *wdd)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&hw->lock, flags);
-	bcm_writel(WDT_STOP_1, hw->regs + WDT_CTL_REG);
-	bcm_writel(WDT_STOP_2, hw->regs + WDT_CTL_REG);
+	__raw_writel(WDT_STOP_1, hw->regs + WDT_CTL_REG);
+	__raw_writel(WDT_STOP_2, hw->regs + WDT_CTL_REG);
 	hw->running = false;
 	raw_spin_unlock_irqrestore(&hw->lock, flags);
 	return 0;
@@ -106,10 +105,10 @@ static void bcm63xx_wdt_isr(void *data)
 	raw_spin_lock_irqsave(&hw->lock, flags);
 	if (!hw->running) {
 		/* Stop the watchdog as it shouldn't be running */
-		bcm_writel(WDT_STOP_1, hw->regs + WDT_CTL_REG);
-		bcm_writel(WDT_STOP_2, hw->regs + WDT_CTL_REG);
+		__raw_writel(WDT_STOP_1, hw->regs + WDT_CTL_REG);
+		__raw_writel(WDT_STOP_2, hw->regs + WDT_CTL_REG);
 	} else {
-		u32 timeleft = bcm_readl(hw->regs + WDT_CTL_REG);
+		u32 timeleft = __raw_readl(hw->regs + WDT_CTL_REG);
 		u32 ms;
 
 		if (timeleft >= 2) {
@@ -123,9 +122,9 @@ static void bcm63xx_wdt_isr(void *data)
 			 * This is done with a lock held in case userspace is
 			 * trying to restart the watchdog on another CPU.
 			 */
-			bcm_writel(timeleft, hw->regs + WDT_DEFVAL_REG);
-			bcm_writel(WDT_START_1, hw->regs + WDT_CTL_REG);
-			bcm_writel(WDT_START_2, hw->regs + WDT_CTL_REG);
+			__raw_writel(timeleft, hw->regs + WDT_DEFVAL_REG);
+			__raw_writel(WDT_START_1, hw->regs + WDT_CTL_REG);
+			__raw_writel(WDT_START_2, hw->regs + WDT_CTL_REG);
 		} else {
 			/* The watchdog cannot be started with a time of less
 			 * than 2 ticks (it won't fire).
diff --git a/include/linux/bcm63xx_wdt.h b/include/linux/bcm63xx_wdt.h
new file mode 100644
index 0000000..2e9210b
--- /dev/null
+++ b/include/linux/bcm63xx_wdt.h
@@ -0,0 +1,22 @@
+#ifndef LINUX_BCM63XX_WDT_H_
+#define LINUX_BCM63XX_WDT_H_
+
+/* Watchdog default count register */
+#define WDT_DEFVAL_REG                  0x0
+
+/* Watchdog control register */
+#define WDT_CTL_REG                     0x4
+
+/* Watchdog control register constants */
+#define WDT_START_1                     (0xff00)
+#define WDT_START_2                     (0x00ff)
+#define WDT_STOP_1                      (0xee00)
+#define WDT_STOP_2                      (0x00ee)
+
+/* Watchdog reset length register */
+#define WDT_RSTLEN_REG                  0x8
+
+/* Watchdog soft reset register (BCM6328 only) */
+#define WDT_SOFTRESET_REG               0xc
+
+#endif
-- 
2.1.4

-- 
Simon Arlott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1274881 — [PATCH 10/10] watchdog: bcm63xx_wdt: Use brcm,bcm6345-wdt device tree binding

Add of_match_table for "brcm,bcm6345-wdt".

Use a NULL clock name when not on mach-bcm63xx so that the device tree
clock name does not have to be "periph".

Allow the watchdog to be selected on BMIPS_GENERIC and select the BCM6345
timer interrupt handler.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
 drivers/watchdog/Kconfig       |  3 ++-
 drivers/watchdog/bcm63xx_wdt.c | 14 +++++++++++++-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 6815b74..0c50add 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1272,8 +1272,9 @@ config OCTEON_WDT
 
 config BCM63XX_WDT
 	tristate "Broadcom BCM63xx hardware watchdog"
-	depends on BCM63XX
+	depends on BCM63XX || BMIPS_GENERIC
 	select WATCHDOG_CORE
+	select BCM6345_L2_TIMER_IRQ if BMIPS_GENERIC
 	help
 	  Watchdog driver for the built in watchdog hardware in Broadcom
 	  BCM63xx SoC.
diff --git a/drivers/watchdog/bcm63xx_wdt.c b/drivers/watchdog/bcm63xx_wdt.c
index 9989efe..51b855e 100644
--- a/drivers/watchdog/bcm63xx_wdt.c
+++ b/drivers/watchdog/bcm63xx_wdt.c
@@ -22,6 +22,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
+#include <linux/mod_devicetable.h>
 #include <linux/spinlock.h>
 #include <linux/types.h>
 #include <linux/watchdog.h>
@@ -31,7 +32,11 @@
 
 #define PFX KBUILD_MODNAME
 
-#define WDT_CLK_NAME		"periph"
+#ifdef CONFIG_BCM63XX
+# define WDT_CLK_NAME		"periph"
+#else
+# define WDT_CLK_NAME		NULL
+#endif
 
 struct bcm63xx_wdt_hw {
 	raw_spinlock_t lock;
@@ -285,12 +290,19 @@ static void bcm63xx_wdt_shutdown(struct platform_device *pdev)
 	bcm63xx_wdt_stop(wdd);
 }
 
+static const struct of_device_id bcm63xx_wdt_dt_ids[] = {
+	{ .compatible = "brcm,bcm6345-wdt" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, bcm63xx_wdt_dt_ids);
+
 static struct platform_driver bcm63xx_wdt_driver = {
 	.probe	= bcm63xx_wdt_probe,
 	.remove = bcm63xx_wdt_remove,
 	.shutdown = bcm63xx_wdt_shutdown,
 	.driver = {
 		.name = "bcm63xx-wdt",
+		.of_match_table = bcm63xx_wdt_dt_ids,
 	}
 };
 
-- 
2.1.4

-- 
Simon Arlott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1277872 — [PATCH (v2) 11/11] watchdog: bcm63xx_wdt: Use brcm,bcm6345-wdt device tree binding

Add of_match_table for "brcm,bcm6345-wdt".

Use a NULL clock name when not on mach-bcm63xx so that the device tree
clock name does not have to be "periph".

Allow the watchdog to be selected on BMIPS_GENERIC and select the BCM6345
timer interrupt handler.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
Patch 7 split into two patches.

On 25/11/15 02:44, Guenter Roeck wrote:
>> +	hw = devm_kzalloc(&pdev->dev, sizeof(*hw), GFP_KERNEL);
>> +	wdd = devm_kzalloc(&pdev->dev, sizeof(*wdd), GFP_KERNEL);
> 
> It would be better to allocate wdd as part of struct bcm63xx_wdt_hw.

This altered the context of this patch (was 10/10 now 11/11) because
platform_get_drvdata() is now "hw" instead of "wdd".

 drivers/watchdog/Kconfig       |  3 ++-
 drivers/watchdog/bcm63xx_wdt.c | 14 +++++++++++++-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 6815b74..0c50add 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1272,8 +1272,9 @@ config OCTEON_WDT
 
 config BCM63XX_WDT
 	tristate "Broadcom BCM63xx hardware watchdog"
-	depends on BCM63XX
+	depends on BCM63XX || BMIPS_GENERIC
 	select WATCHDOG_CORE
+	select BCM6345_L2_TIMER_IRQ if BMIPS_GENERIC
 	help
 	  Watchdog driver for the built in watchdog hardware in Broadcom
 	  BCM63xx SoC.
diff --git a/drivers/watchdog/bcm63xx_wdt.c b/drivers/watchdog/bcm63xx_wdt.c
index fa6c28b..4db4145 100644
--- a/drivers/watchdog/bcm63xx_wdt.c
+++ b/drivers/watchdog/bcm63xx_wdt.c
@@ -22,6 +22,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
+#include <linux/mod_devicetable.h>
 #include <linux/spinlock.h>
 #include <linux/types.h>
 #include <linux/watchdog.h>
@@ -31,7 +32,11 @@
 
 #define PFX KBUILD_MODNAME
 
-#define WDT_CLK_NAME		"periph"
+#ifdef CONFIG_BCM63XX
+# define WDT_CLK_NAME		"periph"
+#else
+# define WDT_CLK_NAME		NULL
+#endif
 
 struct bcm63xx_wdt_hw {
 	struct watchdog_device wdd;
@@ -286,12 +291,19 @@ static void bcm63xx_wdt_shutdown(struct platform_device *pdev)
 	bcm63xx_wdt_stop(&hw->wdd);
 }
 
+static const struct of_device_id bcm63xx_wdt_dt_ids[] = {
+	{ .compatible = "brcm,bcm6345-wdt" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, bcm63xx_wdt_dt_ids);
+
 static struct platform_driver bcm63xx_wdt_driver = {
 	.probe	= bcm63xx_wdt_probe,
 	.remove = bcm63xx_wdt_remove,
 	.shutdown = bcm63xx_wdt_shutdown,
 	.driver = {
 		.name = "bcm63xx-wdt",
+		.of_match_table = bcm63xx_wdt_dt_ids,
 	}
 };
 
-- 
2.1.4

-- 
Simon Arlott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1274882 — [PATCH 9/10] watchdog: bcm63xx_wdt: Use bcm63xx_timer interrupt directly

There is only one user of bcm63xx_timer and that is the watchdog.
To allow the watchdog driver to be used on machine types other than
mach-bcm63xx, it needs to use an interrupt instead of a custom register
function.

Modify bcm63xx_timer to only disable the timers (so that they don't
interfere with the watchdog if an interrupt occurs) and remove its
exported functions.

Use the timer interrupt directly in bcm63xx_wdt.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
 arch/mips/bcm63xx/dev-wdt.c                        |   7 +
 arch/mips/bcm63xx/timer.c                          | 181 +--------------------
 arch/mips/include/asm/mach-bcm63xx/bcm63xx_timer.h |  11 --
 drivers/watchdog/bcm63xx_wdt.c                     |  45 +++--
 4 files changed, 38 insertions(+), 206 deletions(-)
 delete mode 100644 arch/mips/include/asm/mach-bcm63xx/bcm63xx_timer.h

diff --git a/arch/mips/bcm63xx/dev-wdt.c b/arch/mips/bcm63xx/dev-wdt.c
index 2a2346a..a7a5497 100644
--- a/arch/mips/bcm63xx/dev-wdt.c
+++ b/arch/mips/bcm63xx/dev-wdt.c
@@ -17,6 +17,11 @@ static struct resource wdt_resources[] = {
 		.end		= -1, /* filled at runtime */
 		.flags		= IORESOURCE_MEM,
 	},
+	{
+		.start		= -1, /* filled at runtime */
+		.end		= -1, /* filled at runtime */
+		.flags		= IORESOURCE_IRQ,
+	},
 };
 
 static struct platform_device bcm63xx_wdt_device = {
@@ -32,6 +37,8 @@ int __init bcm63xx_wdt_register(void)
 	wdt_resources[0].end = wdt_resources[0].start;
 	wdt_resources[0].end += RSET_WDT_SIZE - 1;
 
+	wdt_resources[1].start = bcm63xx_get_irq_number(IRQ_TIMER);
+
 	return platform_device_register(&bcm63xx_wdt_device);
 }
 arch_initcall(bcm63xx_wdt_register);
diff --git a/arch/mips/bcm63xx/timer.c b/arch/mips/bcm63xx/timer.c
index 5f11359..9c7b41a6 100644
--- a/arch/mips/bcm63xx/timer.c
+++ b/arch/mips/bcm63xx/timer.c
@@ -9,196 +9,23 @@
 #include <linux/kernel.h>
 #include <linux/err.h>
 #include <linux/module.h>
-#include <linux/spinlock.h>
-#include <linux/interrupt.h>
-#include <linux/clk.h>
 #include <bcm63xx_cpu.h>
 #include <bcm63xx_io.h>
-#include <bcm63xx_timer.h>
 #include <bcm63xx_regs.h>
 
-static DEFINE_RAW_SPINLOCK(timer_reg_lock);
-static DEFINE_RAW_SPINLOCK(timer_data_lock);
-static struct clk *periph_clk;
-
-static struct timer_data {
-	void	(*cb)(void *);
-	void	*data;
-} timer_data[BCM63XX_TIMER_COUNT];
-
-static irqreturn_t timer_interrupt(int irq, void *dev_id)
-{
-	u32 stat;
-	int i;
-
-	raw_spin_lock(&timer_reg_lock);
-	stat = bcm_timer_readl(TIMER_IRQSTAT_REG);
-	bcm_timer_writel(stat, TIMER_IRQSTAT_REG);
-	raw_spin_unlock(&timer_reg_lock);
-
-	for (i = 0; i < BCM63XX_TIMER_COUNT; i++) {
-		if (!(stat & TIMER_IRQSTAT_TIMER_CAUSE(i)))
-			continue;
-
-		raw_spin_lock(&timer_data_lock);
-		if (!timer_data[i].cb) {
-			raw_spin_unlock(&timer_data_lock);
-			continue;
-		}
-
-		timer_data[i].cb(timer_data[i].data);
-		raw_spin_unlock(&timer_data_lock);
-	}
-
-	return IRQ_HANDLED;
-}
-
-int bcm63xx_timer_enable(int id)
-{
-	u32 reg;
-	unsigned long flags;
-
-	if (id >= BCM63XX_TIMER_COUNT)
-		return -EINVAL;
-
-	raw_spin_lock_irqsave(&timer_reg_lock, flags);
-
-	reg = bcm_timer_readl(TIMER_CTLx_REG(id));
-	reg |= TIMER_CTL_ENABLE_MASK;
-	bcm_timer_writel(reg, TIMER_CTLx_REG(id));
-
-	reg = bcm_timer_readl(TIMER_IRQSTAT_REG);
-	reg |= TIMER_IRQSTAT_TIMER_IR_EN(id);
-	bcm_timer_writel(reg, TIMER_IRQSTAT_REG);
-
-	raw_spin_unlock_irqrestore(&timer_reg_lock, flags);
-	return 0;
-}
-
-EXPORT_SYMBOL(bcm63xx_timer_enable);
-
-int bcm63xx_timer_disable(int id)
+static int bcm63xx_timer_init(void)
 {
 	u32 reg;
-	unsigned long flags;
-
-	if (id >= BCM63XX_TIMER_COUNT)
-		return -EINVAL;
-
-	raw_spin_lock_irqsave(&timer_reg_lock, flags);
-
-	reg = bcm_timer_readl(TIMER_CTLx_REG(id));
-	reg &= ~TIMER_CTL_ENABLE_MASK;
-	bcm_timer_writel(reg, TIMER_CTLx_REG(id));
-
-	reg = bcm_timer_readl(TIMER_IRQSTAT_REG);
-	reg &= ~TIMER_IRQSTAT_TIMER_IR_EN(id);
-	bcm_timer_writel(reg, TIMER_IRQSTAT_REG);
-
-	raw_spin_unlock_irqrestore(&timer_reg_lock, flags);
-	return 0;
-}
-
-EXPORT_SYMBOL(bcm63xx_timer_disable);
-
-int bcm63xx_timer_register(int id, void (*callback)(void *data), void *data)
-{
-	unsigned long flags;
-	int ret;
-
-	if (id >= BCM63XX_TIMER_COUNT || !callback)
-		return -EINVAL;
-
-	ret = 0;
-	raw_spin_lock_irqsave(&timer_data_lock, flags);
-	if (timer_data[id].cb) {
-		ret = -EBUSY;
-		goto out;
-	}
-
-	timer_data[id].cb = callback;
-	timer_data[id].data = data;
-
-out:
-	raw_spin_unlock_irqrestore(&timer_data_lock, flags);
-	return ret;
-}
-
-EXPORT_SYMBOL(bcm63xx_timer_register);
-
-void bcm63xx_timer_unregister(int id)
-{
-	unsigned long flags;
-
-	if (id >= BCM63XX_TIMER_COUNT)
-		return;
-
-	raw_spin_lock_irqsave(&timer_data_lock, flags);
-	timer_data[id].cb = NULL;
-	raw_spin_unlock_irqrestore(&timer_data_lock, flags);
-}
-
-EXPORT_SYMBOL(bcm63xx_timer_unregister);
-
-unsigned int bcm63xx_timer_countdown(unsigned int countdown_us)
-{
-	return (clk_get_rate(periph_clk) / (1000 * 1000)) * countdown_us;
-}
-
-EXPORT_SYMBOL(bcm63xx_timer_countdown);
-
-int bcm63xx_timer_set(int id, int monotonic, unsigned int countdown_us)
-{
-	u32 reg, countdown;
-	unsigned long flags;
-
-	if (id >= BCM63XX_TIMER_COUNT)
-		return -EINVAL;
-
-	countdown = bcm63xx_timer_countdown(countdown_us);
-	if (countdown & ~TIMER_CTL_COUNTDOWN_MASK)
-		return -EINVAL;
-
-	raw_spin_lock_irqsave(&timer_reg_lock, flags);
-	reg = bcm_timer_readl(TIMER_CTLx_REG(id));
-
-	if (monotonic)
-		reg &= ~TIMER_CTL_MONOTONIC_MASK;
-	else
-		reg |= TIMER_CTL_MONOTONIC_MASK;
-
-	reg &= ~TIMER_CTL_COUNTDOWN_MASK;
-	reg |= countdown;
-	bcm_timer_writel(reg, TIMER_CTLx_REG(id));
-
-	raw_spin_unlock_irqrestore(&timer_reg_lock, flags);
-	return 0;
-}
-
-EXPORT_SYMBOL(bcm63xx_timer_set);
-
-int bcm63xx_timer_init(void)
-{
-	int ret, irq;
-	u32 reg;
 
+	/* Disable all timers so that they won't interfere with use of the
+	 * timer interrupt by the watchdog.
+	 */
 	reg = bcm_timer_readl(TIMER_IRQSTAT_REG);
 	reg &= ~TIMER_IRQSTAT_TIMER0_IR_EN;
 	reg &= ~TIMER_IRQSTAT_TIMER1_IR_EN;
 	reg &= ~TIMER_IRQSTAT_TIMER2_IR_EN;
 	bcm_timer_writel(reg, TIMER_IRQSTAT_REG);
 
-	periph_clk = clk_get(NULL, "periph");
-	if (IS_ERR(periph_clk))
-		return -ENODEV;
-
-	irq = bcm63xx_get_irq_number(IRQ_TIMER);
-	ret = request_irq(irq, timer_interrupt, 0, "bcm63xx_timer", NULL);
-	if (ret) {
-		printk(KERN_ERR "bcm63xx_timer: failed to register irq\n");
-		return ret;
-	}
-
 	return 0;
 }
 
diff --git a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_timer.h b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_timer.h
deleted file mode 100644
index c0fce83..0000000
--- a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_timer.h
+++ /dev/null
@@ -1,11 +0,0 @@
-#ifndef BCM63XX_TIMER_H_
-#define BCM63XX_TIMER_H_
-
-int bcm63xx_timer_register(int id, void (*callback)(void *data), void *data);
-void bcm63xx_timer_unregister(int id);
-int bcm63xx_timer_set(int id, int monotonic, unsigned int countdown_us);
-int bcm63xx_timer_enable(int id);
-int bcm63xx_timer_disable(int id);
-unsigned int bcm63xx_timer_countdown(unsigned int countdown_us);
-
-#endif /* !BCM63XX_TIMER_H_ */
diff --git a/drivers/watchdog/bcm63xx_wdt.c b/drivers/watchdog/bcm63xx_wdt.c
index 7522624..9989efe 100644
--- a/drivers/watchdog/bcm63xx_wdt.c
+++ b/drivers/watchdog/bcm63xx_wdt.c
@@ -17,6 +17,7 @@
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/errno.h>
+#include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -28,9 +29,6 @@
 #include <linux/resource.h>
 #include <linux/platform_device.h>
 
-#include <bcm63xx_regs.h>
-#include <bcm63xx_timer.h>
-
 #define PFX KBUILD_MODNAME
 
 #define WDT_CLK_NAME		"periph"
@@ -40,6 +38,7 @@ struct bcm63xx_wdt_hw {
 	void __iomem *regs;
 	struct clk *clk;
 	unsigned long clock_hz;
+	int irq;
 	bool running;
 };
 
@@ -96,7 +95,7 @@ static int bcm63xx_wdt_set_timeout(struct watchdog_device *wdd,
 }
 
 /* The watchdog interrupt occurs when half the timeout is remaining */
-static void bcm63xx_wdt_isr(void *data)
+static irqreturn_t bcm63xx_wdt_interrupt(int irq, void *data)
 {
 	struct watchdog_device *wdd = data;
 	struct bcm63xx_wdt_hw *hw = watchdog_get_drvdata(wdd);
@@ -137,6 +136,7 @@ static void bcm63xx_wdt_isr(void *data)
 			"warning timer fired, reboot in %ums\n", ms);
 	}
 	raw_spin_unlock_irqrestore(&hw->lock, flags);
+	return IRQ_HANDLED;
 }
 
 static struct watchdog_ops bcm63xx_wdt_ops = {
@@ -229,30 +229,37 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
 		timeleft = 0;
 	}
 
-	ret = bcm63xx_timer_register(TIMER_WDT_ID, bcm63xx_wdt_isr, wdd);
+	ret = watchdog_register_device(wdd);
 	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to register wdt timer isr\n");
+		dev_err(&pdev->dev, "failed to register watchdog device\n");
 		goto disable_clk;
 	}
 
-	ret = watchdog_register_device(wdd);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to register watchdog device\n");
-		goto unregister_timer;
+	hw->irq = platform_get_irq(pdev, 0);
+	if (hw->irq >= 0) {
+		ret = devm_request_irq(&pdev->dev, hw->irq,
+			bcm63xx_wdt_interrupt, IRQF_TIMER,
+			dev_name(&pdev->dev), wdd);
+		if (ret)
+			hw->irq = -1;
 	}
 
-	dev_info(&pdev->dev,
-		"%s at MMIO 0x%p (timeout = %us, max_timeout = %us)",
-		dev_name(wdd->dev), hw->regs,
-		wdd->timeout, wdd->max_timeout);
+	if (hw->irq >= 0) {
+		dev_info(&pdev->dev,
+			"%s at MMIO 0x%p (irq = %d, timeout = %us, max_timeout = %us)",
+			dev_name(wdd->dev), hw->regs, hw->irq,
+			wdd->timeout, wdd->max_timeout);
+	} else {
+		dev_info(&pdev->dev,
+			"%s at MMIO 0x%p (timeout = %us, max_timeout = %us)",
+			dev_name(wdd->dev), hw->regs,
+			wdd->timeout, wdd->max_timeout);
+	}
 
 	if (timeleft > 0)
 		dev_alert(wdd->dev, "running, reboot in %us\n", timeleft);
 	return 0;
 
-unregister_timer:
-	bcm63xx_timer_unregister(TIMER_WDT_ID);
-
 disable_clk:
 	clk_disable_unprepare(hw->clk);
 	return ret;
@@ -263,7 +270,9 @@ static int bcm63xx_wdt_remove(struct platform_device *pdev)
 	struct watchdog_device *wdd = platform_get_drvdata(pdev);
 	struct bcm63xx_wdt_hw *hw = watchdog_get_drvdata(wdd);
 
-	bcm63xx_timer_unregister(TIMER_WDT_ID);
+	if (hw->irq >= 0)
+		devm_free_irq(&pdev->dev, hw->irq, wdd);
+
 	watchdog_unregister_device(wdd);
 	clk_disable_unprepare(hw->clk);
 	return 0;
-- 
2.1.4

-- 
Simon Arlott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1277866 — [PATCH (v2) 10/11] watchdog: bcm63xx_wdt: Use bcm63xx_timer interrupt directly

There is only one user of bcm63xx_timer and that is the watchdog.
To allow the watchdog driver to be used on machine types other than
mach-bcm63xx, it needs to use an interrupt instead of a custom register
function.

Modify bcm63xx_timer to only disable the timers (so that they don't
interfere with the watchdog if an interrupt occurs) and remove its
exported functions.

Use the timer interrupt directly in bcm63xx_wdt.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
Patch 7 split into two patches.

There was no change to patch 8/10 which is now 9/11.

On 25/11/15 22:40, Simon Arlott wrote:
> Moved registration of the timer in the probe function to after register
> of the watchdog device because the interrupt handler uses wdd->dev.

This patch (9/10 now 10/11) was affected by this reordering, and an
earlier change to use "hw" instead of "wdd" as the interrupt data.

 arch/mips/bcm63xx/dev-wdt.c                        |   7 +
 arch/mips/bcm63xx/timer.c                          | 181 +--------------------
 arch/mips/include/asm/mach-bcm63xx/bcm63xx_timer.h |  11 --
 drivers/watchdog/bcm63xx_wdt.c                     |  41 +++--
 4 files changed, 36 insertions(+), 204 deletions(-)
 delete mode 100644 arch/mips/include/asm/mach-bcm63xx/bcm63xx_timer.h

diff --git a/arch/mips/bcm63xx/dev-wdt.c b/arch/mips/bcm63xx/dev-wdt.c
index 2a2346a..a7a5497 100644
--- a/arch/mips/bcm63xx/dev-wdt.c
+++ b/arch/mips/bcm63xx/dev-wdt.c
@@ -17,6 +17,11 @@ static struct resource wdt_resources[] = {
 		.end		= -1, /* filled at runtime */
 		.flags		= IORESOURCE_MEM,
 	},
+	{
+		.start		= -1, /* filled at runtime */
+		.end		= -1, /* filled at runtime */
+		.flags		= IORESOURCE_IRQ,
+	},
 };
 
 static struct platform_device bcm63xx_wdt_device = {
@@ -32,6 +37,8 @@ int __init bcm63xx_wdt_register(void)
 	wdt_resources[0].end = wdt_resources[0].start;
 	wdt_resources[0].end += RSET_WDT_SIZE - 1;
 
+	wdt_resources[1].start = bcm63xx_get_irq_number(IRQ_TIMER);
+
 	return platform_device_register(&bcm63xx_wdt_device);
 }
 arch_initcall(bcm63xx_wdt_register);
diff --git a/arch/mips/bcm63xx/timer.c b/arch/mips/bcm63xx/timer.c
index 2110359..9c7b41a6 100644
--- a/arch/mips/bcm63xx/timer.c
+++ b/arch/mips/bcm63xx/timer.c
@@ -9,196 +9,23 @@
 #include <linux/kernel.h>
 #include <linux/err.h>
 #include <linux/module.h>
-#include <linux/spinlock.h>
-#include <linux/interrupt.h>
-#include <linux/clk.h>
 #include <bcm63xx_cpu.h>
 #include <bcm63xx_io.h>
-#include <bcm63xx_timer.h>
 #include <bcm63xx_regs.h>
 
-static DEFINE_RAW_SPINLOCK(timer_reg_lock);
-static DEFINE_RAW_SPINLOCK(timer_data_lock);
-static struct clk *periph_clk;
-
-static struct timer_data {
-	void	(*cb)(void *);
-	void	*data;
-} timer_data[BCM63XX_TIMER_COUNT];
-
-static irqreturn_t timer_interrupt(int irq, void *dev_id)
-{
-	u32 stat;
-	int i;
-
-	raw_spin_lock(&timer_reg_lock);
-	stat = bcm_timer_readl(TIMER_IRQSTAT_REG);
-	bcm_timer_writel(stat, TIMER_IRQSTAT_REG);
-	raw_spin_unlock(&timer_reg_lock);
-
-	for (i = 0; i < BCM63XX_TIMER_COUNT; i++) {
-		if (!(stat & TIMER_IRQSTAT_TIMER_CAUSE(i)))
-			continue;
-
-		raw_spin_lock(&timer_data_lock);
-		if (!timer_data[i].cb) {
-			raw_spin_unlock(&timer_data_lock);
-			continue;
-		}
-
-		timer_data[i].cb(timer_data[i].data);
-		raw_spin_unlock(&timer_data_lock);
-	}
-
-	return IRQ_HANDLED;
-}
-
-int bcm63xx_timer_enable(int id)
-{
-	u32 reg;
-	unsigned long flags;
-
-	if (id >= BCM63XX_TIMER_COUNT)
-		return -EINVAL;
-
-	raw_spin_lock_irqsave(&timer_reg_lock, flags);
-
-	reg = bcm_timer_readl(TIMER_CTLx_REG(id));
-	reg |= TIMER_CTL_ENABLE_MASK;
-	bcm_timer_writel(reg, TIMER_CTLx_REG(id));
-
-	reg = bcm_timer_readl(TIMER_IRQSTAT_REG);
-	reg |= TIMER_IRQSTAT_TIMER_IR_EN(id);
-	bcm_timer_writel(reg, TIMER_IRQSTAT_REG);
-
-	raw_spin_unlock_irqrestore(&timer_reg_lock, flags);
-	return 0;
-}
-
-EXPORT_SYMBOL(bcm63xx_timer_enable);
-
-int bcm63xx_timer_disable(int id)
+static int bcm63xx_timer_init(void)
 {
 	u32 reg;
-	unsigned long flags;
-
-	if (id >= BCM63XX_TIMER_COUNT)
-		return -EINVAL;
-
-	raw_spin_lock_irqsave(&timer_reg_lock, flags);
-
-	reg = bcm_timer_readl(TIMER_CTLx_REG(id));
-	reg &= ~TIMER_CTL_ENABLE_MASK;
-	bcm_timer_writel(reg, TIMER_CTLx_REG(id));
-
-	reg = bcm_timer_readl(TIMER_IRQSTAT_REG);
-	reg &= ~TIMER_IRQSTAT_TIMER_IR_EN(id);
-	bcm_timer_writel(reg, TIMER_IRQSTAT_REG);
-
-	raw_spin_unlock_irqrestore(&timer_reg_lock, flags);
-	return 0;
-}
-
-EXPORT_SYMBOL(bcm63xx_timer_disable);
-
-int bcm63xx_timer_register(int id, void (*callback)(void *data), void *data)
-{
-	unsigned long flags;
-	int ret;
-
-	if (id >= BCM63XX_TIMER_COUNT || !callback)
-		return -EINVAL;
-
-	ret = 0;
-	raw_spin_lock_irqsave(&timer_data_lock, flags);
-	if (timer_data[id].cb) {
-		ret = -EBUSY;
-		goto out;
-	}
-
-	timer_data[id].cb = callback;
-	timer_data[id].data = data;
-
-out:
-	raw_spin_unlock_irqrestore(&timer_data_lock, flags);
-	return ret;
-}
-
-EXPORT_SYMBOL(bcm63xx_timer_register);
-
-void bcm63xx_timer_unregister(int id)
-{
-	unsigned long flags;
-
-	if (id >= BCM63XX_TIMER_COUNT)
-		return;
-
-	raw_spin_lock_irqsave(&timer_data_lock, flags);
-	timer_data[id].cb = NULL;
-	raw_spin_unlock_irqrestore(&timer_data_lock, flags);
-}
-
-EXPORT_SYMBOL(bcm63xx_timer_unregister);
-
-unsigned int bcm63xx_timer_countdown(unsigned int countdown_us)
-{
-	return (clk_get_rate(periph_clk) / (1000 * 1000)) * countdown_us;
-}
-
-EXPORT_SYMBOL(bcm63xx_timer_countdown);
-
-int bcm63xx_timer_set(int id, int monotonic, unsigned int countdown_us)
-{
-	u32 reg, countdown;
-	unsigned long flags;
-
-	if (id >= BCM63XX_TIMER_COUNT)
-		return -EINVAL;
-
-	countdown = bcm63xx_timer_countdown(countdown_us);
-	if (countdown & ~TIMER_CTL_COUNTDOWN_MASK)
-		return -EINVAL;
-
-	raw_spin_lock_irqsave(&timer_reg_lock, flags);
-	reg = bcm_timer_readl(TIMER_CTLx_REG(id));
-
-	if (monotonic)
-		reg &= ~TIMER_CTL_MONOTONIC_MASK;
-	else
-		reg |= TIMER_CTL_MONOTONIC_MASK;
-
-	reg &= ~TIMER_CTL_COUNTDOWN_MASK;
-	reg |= countdown;
-	bcm_timer_writel(reg, TIMER_CTLx_REG(id));
-
-	raw_spin_unlock_irqrestore(&timer_reg_lock, flags);
-	return 0;
-}
-
-EXPORT_SYMBOL(bcm63xx_timer_set);
-
-int bcm63xx_timer_init(void)
-{
-	int ret, irq;
-	u32 reg;
 
+	/* Disable all timers so that they won't interfere with use of the
+	 * timer interrupt by the watchdog.
+	 */
 	reg = bcm_timer_readl(TIMER_IRQSTAT_REG);
 	reg &= ~TIMER_IRQSTAT_TIMER0_IR_EN;
 	reg &= ~TIMER_IRQSTAT_TIMER1_IR_EN;
 	reg &= ~TIMER_IRQSTAT_TIMER2_IR_EN;
 	bcm_timer_writel(reg, TIMER_IRQSTAT_REG);
 
-	periph_clk = clk_get(NULL, "periph");
-	if (IS_ERR(periph_clk))
-		return -ENODEV;
-
-	irq = bcm63xx_get_irq_number(IRQ_TIMER);
-	ret = request_irq(irq, timer_interrupt, 0, "bcm63xx_timer", NULL);
-	if (ret) {
-		pr_err("%s: failed to register irq\n", __func__);
-		return ret;
-	}
-
 	return 0;
 }
 
diff --git a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_timer.h b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_timer.h
deleted file mode 100644
index c0fce83..0000000
--- a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_timer.h
+++ /dev/null
@@ -1,11 +0,0 @@
-#ifndef BCM63XX_TIMER_H_
-#define BCM63XX_TIMER_H_
-
-int bcm63xx_timer_register(int id, void (*callback)(void *data), void *data);
-void bcm63xx_timer_unregister(int id);
-int bcm63xx_timer_set(int id, int monotonic, unsigned int countdown_us);
-int bcm63xx_timer_enable(int id);
-int bcm63xx_timer_disable(int id);
-unsigned int bcm63xx_timer_countdown(unsigned int countdown_us);
-
-#endif /* !BCM63XX_TIMER_H_ */
diff --git a/drivers/watchdog/bcm63xx_wdt.c b/drivers/watchdog/bcm63xx_wdt.c
index f409523..fa6c28b 100644
--- a/drivers/watchdog/bcm63xx_wdt.c
+++ b/drivers/watchdog/bcm63xx_wdt.c
@@ -17,6 +17,7 @@
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/errno.h>
+#include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -28,9 +29,6 @@
 #include <linux/resource.h>
 #include <linux/platform_device.h>
 
-#include <bcm63xx_regs.h>
-#include <bcm63xx_timer.h>
-
 #define PFX KBUILD_MODNAME
 
 #define WDT_CLK_NAME		"periph"
@@ -41,6 +39,7 @@ struct bcm63xx_wdt_hw {
 	void __iomem *regs;
 	struct clk *clk;
 	unsigned long clock_hz;
+	int irq;
 	bool running;
 };
 
@@ -99,7 +98,7 @@ static int bcm63xx_wdt_set_timeout(struct watchdog_device *wdd,
 }
 
 /* The watchdog interrupt occurs when half the timeout is remaining */
-static void bcm63xx_wdt_isr(void *data)
+static irqreturn_t bcm63xx_wdt_interrupt(int irq, void *data)
 {
 	struct bcm63xx_wdt_hw *hw = data;
 	unsigned long flags;
@@ -139,6 +138,7 @@ static void bcm63xx_wdt_isr(void *data)
 			"warning timer fired, reboot in %ums\n", ms);
 	}
 	raw_spin_unlock_irqrestore(&hw->lock, flags);
+	return IRQ_HANDLED;
 }
 
 static struct watchdog_ops bcm63xx_wdt_ops = {
@@ -237,24 +237,31 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
 		goto disable_clk;
 	}
 
-	ret = bcm63xx_timer_register(TIMER_WDT_ID, bcm63xx_wdt_isr, hw);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to register wdt timer isr\n");
-		goto unregister_watchdog;
+	hw->irq = platform_get_irq(pdev, 0);
+	if (hw->irq >= 0) {
+		ret = devm_request_irq(&pdev->dev, hw->irq,
+			bcm63xx_wdt_interrupt, IRQF_TIMER,
+			dev_name(&pdev->dev), hw);
+		if (ret)
+			hw->irq = -1;
 	}
 
-	dev_info(&pdev->dev,
-		"%s at MMIO 0x%p (timeout = %us, max_timeout = %us)",
-		dev_name(wdd->dev), hw->regs,
-		wdd->timeout, wdd->max_timeout);
+	if (hw->irq >= 0) {
+		dev_info(&pdev->dev,
+			"%s at MMIO 0x%p (irq = %d, timeout = %us, max_timeout = %us)",
+			dev_name(wdd->dev), hw->regs, hw->irq,
+			wdd->timeout, wdd->max_timeout);
+	} else {
+		dev_info(&pdev->dev,
+			"%s at MMIO 0x%p (timeout = %us, max_timeout = %us)",
+			dev_name(wdd->dev), hw->regs,
+			wdd->timeout, wdd->max_timeout);
+	}
 
 	if (hw->running)
 		dev_alert(wdd->dev, "running, reboot in %us\n", timeleft);
 	return 0;
 
-unregister_watchdog:
-	watchdog_unregister_device(wdd);
-
 disable_clk:
 	clk_disable_unprepare(hw->clk);
 	return ret;
@@ -264,7 +271,9 @@ static int bcm63xx_wdt_remove(struct platform_device *pdev)
 {
 	struct bcm63xx_wdt_hw *hw = platform_get_drvdata(pdev);
 
-	bcm63xx_timer_unregister(TIMER_WDT_ID);
+	if (hw->irq >= 0)
+		devm_free_irq(&pdev->dev, hw->irq, hw);
+
 	watchdog_unregister_device(&hw->wdd);
 	clk_disable_unprepare(hw->clk);
 	return 0;
-- 
2.1.4

-- 
Simon Arlott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1274738 — [PATCH 3/4] watchdog: Add brcm,bcm6345-wdt device tree binding

Add device tree binding for the BCM6345 watchdog.

This uses the BCM6345 timer for its warning interrupt.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
 .../bindings/watchdog/brcm,bcm6345-wdt.txt         | 35 ++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/brcm,bcm6345-wdt.txt

diff --git a/Documentation/devicetree/bindings/watchdog/brcm,bcm6345-wdt.txt b/Documentation/devicetree/bindings/watchdog/brcm,bcm6345-wdt.txt
new file mode 100644
index 0000000..9d852d4
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/brcm,bcm6345-wdt.txt
@@ -0,0 +1,35 @@
+BCM6345 Watchdog timer
+
+Required properties:
+
+- compatible: should be "brcm,bcm63<soc>-wdt", "brcm,bcm6345-wdt"
+- reg: Specifies base physical address and size of the registers.
+- clocks: Specify the clock used for timing
+
+Optional properties:
+
+- interrupt-parent: phandle to the interrupt controller
+- interrupts: Specify the interrupt used for the watchdog timout warning
+- timeout-sec: Contains the default watchdog timeout in seconds
+
+Example:
+
+watchdog {
+	compatible = "brcm,bcm63168-wdt", "brcm,bcm6345-wdt";
+	reg = <0x1000009c 0x0c>;
+	clocks = <&periph_clk>;
+
+	interrupt-parent = <&timer>;
+	interrupts = <3>;
+	timeout-sec = <30>;
+};
+
+watchdog {
+	compatible = "brcm,bcm6318-wdt", "brcm,bcm6345-wdt";
+	reg = <0x10000068 0x0c>;
+	clocks = <&periph_clk>;
+
+	interrupt-parent = <&timer>;
+	interrupts = <3>;
+	timeout-sec = <30>;
+};
-- 
2.1.4

-- 
Simon Arlott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1274977 — Re: [PATCH 3/4] watchdog: Add brcm,bcm6345-wdt device tree binding

On Sat, Nov 21, 2015 at 07:04:39PM +0000, Simon Arlott wrote:
> Add device tree binding for the BCM6345 watchdog.
> 
> This uses the BCM6345 timer for its warning interrupt.
> 
> Signed-off-by: Simon Arlott <simon@fire.lp0.eu>

Acked-by: Rob Herring <robh@kernel.org>

> ---
>  .../bindings/watchdog/brcm,bcm6345-wdt.txt         | 35 ++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/brcm,bcm6345-wdt.txt
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/brcm,bcm6345-wdt.txt b/Documentation/devicetree/bindings/watchdog/brcm,bcm6345-wdt.txt
> new file mode 100644
> index 0000000..9d852d4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/brcm,bcm6345-wdt.txt
> @@ -0,0 +1,35 @@
> +BCM6345 Watchdog timer
> +
> +Required properties:
> +
> +- compatible: should be "brcm,bcm63<soc>-wdt", "brcm,bcm6345-wdt"
> +- reg: Specifies base physical address and size of the registers.
> +- clocks: Specify the clock used for timing
> +
> +Optional properties:
> +
> +- interrupt-parent: phandle to the interrupt controller
> +- interrupts: Specify the interrupt used for the watchdog timout warning
> +- timeout-sec: Contains the default watchdog timeout in seconds
> +
> +Example:
> +
> +watchdog {
> +	compatible = "brcm,bcm63168-wdt", "brcm,bcm6345-wdt";
> +	reg = <0x1000009c 0x0c>;
> +	clocks = <&periph_clk>;
> +
> +	interrupt-parent = <&timer>;
> +	interrupts = <3>;
> +	timeout-sec = <30>;
> +};
> +
> +watchdog {
> +	compatible = "brcm,bcm6318-wdt", "brcm,bcm6345-wdt";
> +	reg = <0x10000068 0x0c>;
> +	clocks = <&periph_clk>;
> +
> +	interrupt-parent = <&timer>;
> +	interrupts = <3>;
> +	timeout-sec = <30>;
> +};
> -- 
> 2.1.4
> 
> -- 
> Simon Arlott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

Back to top | Article view | linux.kernel

csiph-web