Skip to contentSkip to conversation

clocksource: Add brcm,bcm6345-timer/brcm,bcm6318-timer device tree binding

PATCH In review Simon Arlott 2 participants 12 messages Dec 1, 2015 – Dec 3, 2015

Subsystems (6)
Device tree bindings MIPS Watchdog device drivers irqchip drivers +2 more subsystems
lore
Share & tools
lore

Simon Arlott

author
#0 lore raw

PATCH 1/11 clocksource: Add brcm,bcm6345-timer/brcm,bcm6318-timer device tree binding

Add device tree bindings for the BCM6345/BCM6318 timers. This is required for the BCM6345 watchdog which needs to respond to one of the timer interrupts.

Signed-off-by Simon Arlott <[email protected]>
Acked-by Rob Herring <[email protected]>
---
 .../bindings/timer/brcm,bcm6318-timer.txt          | 44 ++++++++++++++++++++
 .../bindings/timer/brcm,bcm6345-timer.txt          | 47 ++++++++++++++++++++++
 2 files changed, 91 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/brcm,bcm6318-timer.txt
 create mode 100644 Documentation/devicetree/bindings/timer/brcm,bcm6345-timer.txt
Documentation/devicetree/bindings/timer/brcm,bcm6318-timer.txt new file +44 -0
new file mode 100644
index 0000000..cf4be7e
@@ -0,0 +1,44 @@
+Broadcom BCM6318 Timer
+
+This block is a timer that is connected to multiple interrupts on the main
+interrupt controller and functions as a programmable interrupt controller for
+timer events. There is a main timer interrupt for all timers.
+
+- 4 independent timers with their own interrupt, and own maskable level
+  interrupt bit in the main timer interrupt
+
+- 1 watchdog timer with an unmaskable level interrupt bit in the main timer
+  interrupt
+
+- Contains one enable/status word pair
+
+- No atomic set/clear operations
+
+Required properties:
+
+- compatible: should be "brcm,bcm<soc>-timer", "brcm,bcm6318-timer"
+- reg: specifies the base physical address and size of the registers, excluding
+  the watchdog registers
+- interrupt-controller: identifies the node as an interrupt controller
+- #interrupt-cells: specifies the number of cells needed to encode an interrupt
+  source, should be 1.
+- interrupt-parent: specifies the phandle to the parent interrupt controller(s)
+  this one is cascaded from
+- interrupts: specifies the interrupt line(s) in the interrupt-parent controller
+  node for the main timer interrupt, followed by the individual timer
+  interrupts; valid values depend on the type of parent interrupt controller
+- clocks: phandle of timer reference clock (periph)
+
+Example:
+
+timer: timer@10000040 {
+	compatible = "brcm,bcm63148-timer", "brcm,bcm6318-timer";
+	reg = <0x10000040 0x28>;
+
+	interrupt-controller;
+	#interrupt-cells = <1>;
+
+	interrupt-parent = <&periph_intc>;
+	interrupts = <31>, <0>, <1>, <2>, <3>;
+	clock = <&periph_osc>;
+};
Documentation/devicetree/bindings/timer/brcm,bcm6345-timer.txt new file +47 -0

47 lines not on this page · raw message

Signature · 8 lines
2.1.4

-- 
Simon Arlott
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#1 lore raw

PATCH 2/11 MIPS: bmips: Add bcm6345-l2-timer interrupt controller

Add the BCM6345/BCM6318 timer as an interrupt controller so that it can be used by the watchdog to warn that its timer will expire soon.

Support for clocksource/clockevents is not implemented as the timer interrupt is not per CPU (except on the BCM6318) and the MIPS clock is better. This could be added later if required without changing the device tree binding.

Signed-off-by Simon Arlott <[email protected]>
---
 drivers/irqchip/Kconfig                |   5 +
 drivers/irqchip/Makefile               |   1 +
 drivers/irqchip/irq-bcm6345-l2-timer.c | 386 +++++++++++++++++++++++++++++++++
 3 files changed, 392 insertions(+)
 create mode 100644 drivers/irqchip/irq-bcm6345-l2-timer.c
drivers/irqchip/Kconfig +5 -0
index d307bb3..21c3d9b 100644
@@ -70,6 +70,11 @@ config BCM6345_L1_IRQ
 	select GENERIC_IRQ_CHIP
 	select IRQ_DOMAIN
 
+config BCM6345_L2_TIMER_IRQ
+	bool
+	select GENERIC_IRQ_CHIP
+	select IRQ_DOMAIN
+
 config BCM7038_L1_IRQ
 	bool
 	select GENERIC_IRQ_CHIP
drivers/irqchip/Makefile +1 -0
index ded59cf..2687dea 100644
@@ -44,6 +44,7 @@ obj-$(CONFIG_XTENSA_MX)			+= irq-xtensa-mx.o
 obj-$(CONFIG_IRQ_CROSSBAR)		+= irq-crossbar.o
 obj-$(CONFIG_SOC_VF610)			+= irq-vf610-mscm-ir.o
 obj-$(CONFIG_BCM6345_L1_IRQ)		+= irq-bcm6345-l1.o
+obj-$(CONFIG_BCM6345_L2_TIMER_IRQ)	+= irq-bcm6345-l2-timer.o
 obj-$(CONFIG_BCM7038_L1_IRQ)		+= irq-bcm7038-l1.o
 obj-$(CONFIG_BCM7120_L2_IRQ)		+= irq-bcm7120-l2.o
 obj-$(CONFIG_BRCMSTB_L2_IRQ)		+= irq-brcmstb-l2.o
drivers/irqchip/irq-bcm6345-l2-timer.c new file +386 -0

386 lines not on this page · raw message

Signature · 4 lines
2.1.4

-- 
Simon Arlott
Acked-by on v1 2/11
#2 lore raw

Re: [PATCH 02/11] MIPS: bmips: Add bcm6345-l2-timer interrupt controller

Quoted 8 lines · Simon Arlott

Add the BCM6345/BCM6318 timer as an interrupt controller so that it can be used by the watchdog to warn that its timer will expire soon.

Support for clocksource/clockevents is not implemented as the timer interrupt is not per CPU (except on the BCM6318) and the MIPS clock is better. This could be added later if required without changing the device tree binding.

Quoted 7 lines
Signed-off-by Simon Arlott <[email protected]>
---
 drivers/irqchip/Kconfig                |   5 +
 drivers/irqchip/Makefile               |   1 +
 drivers/irqchip/irq-bcm6345-l2-timer.c | 386 +++++++++++++++++++++++++++++++++
 3 files changed, 392 insertions(+)
 create mode 100644 drivers/irqchip/irq-bcm6345-l2-timer.c

I'm not sure how useful it is to have a bunch of timers that are just turned off, but from an irqchip PoV:

Acked-by Marc Zyngier <[email protected]>
	M.
Signature · 1 line
Jazz is not dead. It just smells funny...
author
#3 lore raw

PATCH 3/11 watchdog: Add brcm,bcm6345-wdt device tree binding

In reply to Simon Arlott

Add device tree binding for the BCM6345 watchdog.

This uses the BCM6345 timer for its warning interrupt.

Signed-off-by Simon Arlott <[email protected]>
Acked-by Rob Herring <[email protected]>
---
 .../bindings/watchdog/brcm,bcm6345-wdt.txt         | 35 ++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/brcm,bcm6345-wdt.txt
Documentation/devicetree/bindings/watchdog/brcm,bcm6345-wdt.txt new file +35 -0
new file mode 100644
index 0000000..9d852d4
@@ -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>;
+};
Signature · 5 lines
2.1.4


-- 
Simon Arlott
author
#4 lore raw

PATCH 4/11 watchdog: bcm63xx_wdt: Handle hardware interrupt and remove software timer

In reply to Simon Arlott

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 <[email protected]>
---
 drivers/watchdog/bcm63xx_wdt.c | 125 ++++++++++++++++++++++++-----------------
 1 file changed, 73 insertions(+), 52 deletions(-)
drivers/watchdog/bcm63xx_wdt.c +73 -52
index ab26fd9..3f55cba 100644
@@ -3,6 +3,7 @@
  *
  *  Copyright (C) 2007, Miguel Gaio <[email protected]>
  *  Copyright (C) 2008, Florian Fainelli <[email protected]>
+ *  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 = {
Signature · 5 lines
2.1.4


-- 
Simon Arlott
author
#5 lore raw

PATCH 5/11 watchdog: bcm63xx_wdt: Use WATCHDOG_CORE

In reply to Simon Arlott

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 <[email protected]>
---
 drivers/watchdog/Kconfig       |   1 +
 drivers/watchdog/bcm63xx_wdt.c | 259 +++++++++++++----------------------------
 2 files changed, 79 insertions(+), 181 deletions(-)
drivers/watchdog/Kconfig +1 -0
index 7a8a6c6..6815b74 100644
@@ -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.
drivers/watchdog/bcm63xx_wdt.c +78 -181

356 lines not on this page · raw message

Signature · 5 lines
2.1.4


-- 
Simon Arlott
author
#6 lore raw

PATCH 6/11 watchdog: bcm63xx_wdt: Obtain watchdog clock HZ from "periph" clk

In reply to Simon Arlott

Instead of using a fixed clock HZ in the driver, obtain it from the "periph" clk that the watchdog timer uses.

Signed-off-by Simon Arlott <[email protected]>
Reviewed-by Florian Fainelli <[email protected]>
---
 drivers/watchdog/bcm63xx_wdt.c | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)
drivers/watchdog/bcm63xx_wdt.c +31 -5
index 2257924..0a19731 100644
@@ -13,6 +13,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/clk.h>
 #include <linux/errno.h>
 #include <linux/io.h>
 #include <linux/kernel.h>

@@ -32,12 +33,14 @@
 
 #define PFX KBUILD_MODNAME
 
-#define WDT_HZ			50000000		/* Fclk */
+#define WDT_CLK_NAME		"periph"
 
 struct bcm63xx_wdt_hw {
 	struct watchdog_device wdd;
 	raw_spinlock_t lock;
 	void __iomem *regs;
+	struct clk *clk;
+	unsigned long clock_hz;
 	bool running;
 };
 

@@ -54,7 +57,7 @@ static int bcm63xx_wdt_start(struct watchdog_device *wdd)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&hw->lock, flags);
-	bcm_writel(wdd->timeout * WDT_HZ, hw->regs + WDT_DEFVAL_REG);
+	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);
 	hw->running = true;

@@ -118,7 +121,7 @@ static void bcm63xx_wdt_isr(void *data)
 			die(PFX ": watchdog timer expired\n", get_irq_regs());
 		}
 
-		ms = timeleft / (WDT_HZ / 1000);
+		ms = timeleft / (hw->clock_hz / 1000);
 		dev_alert(hw->wdd.dev,
 			"warning timer fired, reboot in %ums\n", ms);
 	}

@@ -162,6 +165,25 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
+	hw->clk = devm_clk_get(&pdev->dev, WDT_CLK_NAME);
+	if (IS_ERR(hw->clk)) {
+		if (PTR_ERR(hw->clk) != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "unable to request clock\n");
+		return PTR_ERR(hw->clk);
+	}
+
+	hw->clock_hz = clk_get_rate(hw->clk);
+	if (!hw->clock_hz) {
+		dev_err(&pdev->dev, "unable to fetch clock rate\n");
+		return -EINVAL;
+	}
+
+	ret = clk_prepare_enable(hw->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "unable to enable clock\n");
+		return ret;
+	}
+
 	raw_spin_lock_init(&hw->lock);
 	hw->running = false;
 

@@ -169,7 +191,7 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
 	wdd->ops = &bcm63xx_wdt_ops;
 	wdd->info = &bcm63xx_wdt_info;
 	wdd->min_timeout = 1;
-	wdd->max_timeout = 0xffffffff / WDT_HZ;
+	wdd->max_timeout = 0xffffffff / hw->clock_hz;
 	wdd->timeout = min(30U, wdd->max_timeout);
 
 	platform_set_drvdata(pdev, hw);

@@ -180,7 +202,7 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
 	ret = watchdog_register_device(wdd);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to register watchdog device\n");
-		return ret;
+		goto disable_clk;
 	}
 
 	ret = bcm63xx_timer_register(TIMER_WDT_ID, bcm63xx_wdt_isr, hw);

@@ -198,6 +220,9 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
 
 unregister_watchdog:
 	watchdog_unregister_device(wdd);
+
+disable_clk:
+	clk_disable_unprepare(hw->clk);
 	return ret;
 }
 

@@ -207,6 +232,7 @@ static int bcm63xx_wdt_remove(struct platform_device *pdev)
 
 	bcm63xx_timer_unregister(TIMER_WDT_ID);
 	watchdog_unregister_device(&hw->wdd);
+	clk_disable_unprepare(hw->clk);
 	return 0;
 }
Signature · 5 lines
2.1.4


-- 
Simon Arlott
author
#7 lore raw

PATCH 7/11 watchdog: bcm63xx_wdt: Add get_timeleft function

In reply to Simon Arlott

Return the remaining time from the hardware control register.

Signed-off-by Simon Arlott <[email protected]>
---
 drivers/watchdog/bcm63xx_wdt.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
drivers/watchdog/bcm63xx_wdt.c +15 -0
index 0a19731..ab4a794 100644
@@ -78,6 +78,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 +145,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,
 };
 

@@ -256,6 +270,7 @@ module_platform_driver(bcm63xx_wdt_driver);
 
 MODULE_AUTHOR("Miguel Gaio <[email protected]>");
 MODULE_AUTHOR("Florian Fainelli <[email protected]>");
+MODULE_AUTHOR("Simon Arlott");
 MODULE_DESCRIPTION("Driver for the Broadcom BCM63xx SoC watchdog");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("platform:bcm63xx-wdt");
Signature · 5 lines
2.1.4


-- 
Simon Arlott
author
#8 lore raw

PATCH 8/11 watchdog: bcm63xx_wdt: Warn if the watchdog is currently running

In reply to Simon Arlott

Warn when the device is registered if the hardware watchdog is currently running and report the remaining time left.

Signed-off-by Simon Arlott <[email protected]>
---
 drivers/watchdog/bcm63xx_wdt.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)
drivers/watchdog/bcm63xx_wdt.c +22 -1
index ab4a794..2312dc2 100644
@@ -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>

@@ -159,6 +160,8 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
 	struct bcm63xx_wdt_hw *hw;
 	struct watchdog_device *wdd;
 	struct resource *r;
+	u32 timeleft1, timeleft2;
+	unsigned int timeleft;
 	int ret;
 
 	hw = devm_kzalloc(&pdev->dev, sizeof(*hw), GFP_KERNEL);

@@ -199,7 +202,6 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
 	}
 
 	raw_spin_lock_init(&hw->lock);
-	hw->running = false;
 
 	wdd->parent = &pdev->dev;
 	wdd->ops = &bcm63xx_wdt_ops;

@@ -213,6 +215,23 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
 	watchdog_init_timeout(wdd, 0, &pdev->dev);
 	watchdog_set_nowayout(wdd, nowayout);
 
+	/* Compare two reads of the time left value, 2 clock ticks apart */
+	rmb();
+	timeleft1 = __raw_readl(hw->regs + WDT_CTL_REG);
+	udelay(DIV_ROUND_UP(1000000, hw->clock_hz / 2));
+	/* Ensure the register is read twice */
+	rmb();
+	timeleft2 = __raw_readl(hw->regs + WDT_CTL_REG);
+
+	/* If the time left is changing, the watchdog is running */
+	if (timeleft1 != timeleft2) {
+		hw->running = true;
+		timeleft = bcm63xx_wdt_get_timeleft(wdd);
+	} else {
+		hw->running = false;
+		timeleft = 0;
+	}
+
 	ret = watchdog_register_device(wdd);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to register watchdog device\n");

@@ -230,6 +249,8 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
 		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:
Signature · 5 lines
2.1.4


-- 
Simon Arlott
author
#9 lore raw

PATCH 9/11 watchdog: bcm63xx_wdt: Remove dependency on mach-bcm63xx functions/defines

In reply to Simon Arlott

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 <[email protected]>
---
 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
MAINTAINERS +1 -0
index cd553a0..a381176 100644
@@ -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-dY08KVG/lbpWk0Htik3J/[email protected]>
arch/mips/bcm63xx/prom.c +1 -0
index 7019e29..d2800fb 100644
@@ -6,6 +6,7 @@
  * Copyright (C) 2008 Maxime Bizon <[email protected]>
  */
 
+#include <linux/bcm63xx_wdt.h>
 #include <linux/init.h>
 #include <linux/bootmem.h>
 #include <linux/smp.h>
arch/mips/bcm63xx/setup.c +1 -0
index 2be9caa..181402f 100644
@@ -6,6 +6,7 @@
  * Copyright (C) 2008 Maxime Bizon <[email protected]>
  */
 
+#include <linux/bcm63xx_wdt.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/delay.h>
arch/mips/include/asm/mach-bcm63xx/bcm63xx_regs.h +0 -22
index 5035f09..16a745b 100644
@@ -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
  *************************************************************************/
drivers/watchdog/bcm63xx_wdt.c +12 -13
index 2312dc2..f409523 100644
@@ -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>
 

@@ -58,9 +57,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;

@@ -72,8 +71,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;

@@ -108,10 +107,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) {

@@ -125,9 +124,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).
include/linux/bcm63xx_wdt.h new file +22 -0
new file mode 100644
index 0000000..2e9210b
@@ -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
Signature · 10 lines
2.1.4



-- 
Simon Arlott
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#10 lore raw

PATCH 10/11 watchdog: bcm63xx_wdt: Use bcm63xx_timer interrupt directly

In reply to Simon Arlott

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 <[email protected]>
---
 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
arch/mips/bcm63xx/dev-wdt.c +7 -0
index 2a2346a..a7a5497 100644
@@ -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);
arch/mips/bcm63xx/timer.c +4 -177

200 lines not on this page · raw message

arch/mips/include/asm/mach-bcm63xx/bcm63xx_timer.h deleted +0 -11
deleted file mode 100644
index c0fce83..0000000
@@ -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_ */
drivers/watchdog/bcm63xx_wdt.c +25 -16

90 lines not on this page · raw message

Signature · 5 lines
2.1.4


-- 
Simon Arlott
author
#11 lore raw

PATCH 11/11 watchdog: bcm63xx_wdt: Use brcm,bcm6345-wdt device tree binding

In reply to Simon Arlott

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 <[email protected]>
---
 drivers/watchdog/Kconfig       |  3 ++-
 drivers/watchdog/bcm63xx_wdt.c | 14 +++++++++++++-
 2 files changed, 15 insertions(+), 2 deletions(-)
drivers/watchdog/Kconfig +2 -1
index 6815b74..0c50add 100644
@@ -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.
drivers/watchdog/bcm63xx_wdt.c +13 -1
index fa6c28b..4db4145 100644
@@ -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,
 	}
 };
Signature · 5 lines
2.1.4


-- 
Simon Arlott