[PATCH v2 0/9] Orion watchdog DT changes to support more SoCs

[PATCH v2 0/9] Orion watchdog DT changes to support more SoCs

[Ezequiel Garcia]

As part of my work to add watchdog support to Armada 370/XP SoCs,
here's the second version of the early patches that make the orion_wdt
driver multiplatform-friendly.

This patchset allows to build the orion_wdt driver in any Orion
platforms, without any ugly mach-dependencies.

This is done by splitting the single memory resource into two memory
resources: timer control, watchdog counter. These resources are then handled
as independent memory resources (as they actually are semantically different).

The only drawback with this approach is the breakage of devicetree backwards
compatibility such change produces. Of course, this is an important issue,
and it's only acceptable given the DT for these platforms is still considered
partially in transition.

Changes from v1:

* Instead of having RSTOUT as a third orion-wdt resource, and thus
  enable/disable the watchdog reset from the driver, simply enable
  it on board-initialization time.

  This must be done with care, because the orion_wdt is used by
  all the Orion machines (except mvebu) in their DT and legacy variants.

Thanks!

Ezequiel Garcia (9):
  watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
  watchdog: orion: Make counter register a separate resource
  ARM: orion: Assert watchdog RSTOUT enable bit
  watchdog: orion: Remove RSTOUT bit enable/disable
  watchdog: orion: Allow to build in any Orion platform
  watchdog: orion: Rename device-tree binding documentation
  watchdog: orion: Update device-tree binding documentation
  ARM: kirkwood: Update watchdog 'reg' property
  ARM: orion5x: Update orion-wdt DT node

 .../watchdog/{marvel.txt => orion-wdt.txt}         |  7 +++--
 arch/arm/boot/dts/kirkwood.dtsi                    |  3 +-
 arch/arm/boot/dts/orion5x.dtsi                     |  3 +-
 arch/arm/mach-dove/board-dt.c                      |  2 ++
 arch/arm/mach-dove/common.c                        |  2 ++
 arch/arm/mach-dove/include/mach/bridge-regs.h      |  2 ++
 arch/arm/mach-kirkwood/board-dt.c                  |  2 ++
 arch/arm/mach-kirkwood/common.c                    |  2 ++
 arch/arm/mach-kirkwood/include/mach/bridge-regs.h  |  2 ++
 arch/arm/mach-mv78xx0/common.c                     |  2 ++
 arch/arm/mach-mv78xx0/include/mach/bridge-regs.h   |  2 ++
 arch/arm/mach-orion5x/board-dt.c                   |  4 +++
 arch/arm/mach-orion5x/common.c                     |  2 ++
 arch/arm/mach-orion5x/include/mach/bridge-regs.h   |  2 ++
 arch/arm/plat-orion/common.c                       | 15 +++++++---
 arch/arm/plat-orion/include/plat/common.h          |  1 +
 drivers/watchdog/Kconfig                           |  2 +-
 drivers/watchdog/orion_wdt.c                       | 32 ++++++++--------------
 18 files changed, 57 insertions(+), 30 deletions(-)
 rename Documentation/devicetree/bindings/watchdog/{marvel.txt => orion-wdt.txt} (58%)

-- 
1.8.1.5

[PATCH v2 1/9] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear

[Ezequiel Garcia]

With the introduction of the orion irqchip driver, now the BRIDGE_CAUSE
bit is cleared by it. There's no longer a need to do it in the watchdog
driver, so we can simply remove it.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/watchdog/orion_wdt.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 4ea5fcc..74d3b72 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -39,7 +39,6 @@
 #define WDT_OK_TO_CLOSE		1
 
 #define WDT_RESET_OUT_EN	BIT(1)
-#define WDT_INT_REQ		BIT(3)
 
 static bool nowayout = WATCHDOG_NOWAYOUT;
 static int heartbeat = -1;		/* module parameter (seconds) */
@@ -69,9 +68,6 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
 	/* Set watchdog duration */
 	writel(wdt_tclk * wdt_dev->timeout, wdt_reg + WDT_VAL);
 
-	/* Clear watchdog timer interrupt */
-	writel(~WDT_INT_REQ, BRIDGE_CAUSE);
-
 	/* Enable watchdog timer */
 	reg = readl(wdt_reg + TIMER_CTRL);
 	reg |= WDT_EN;
-- 
1.8.1.5

[PATCH v2 2/9] watchdog: orion: Make counter register a separate resource

[Ezequiel Garcia]

In order to support other SoC, it's required to distinguish
the 'control' timer register, from the 'counter' watchdog register
as different memory resources.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-dove/include/mach/bridge-regs.h     |  1 +
 arch/arm/mach-kirkwood/include/mach/bridge-regs.h |  1 +
 arch/arm/mach-mv78xx0/include/mach/bridge-regs.h  |  1 +
 arch/arm/mach-orion5x/include/mach/bridge-regs.h  |  1 +
 arch/arm/plat-orion/common.c                      | 10 ++++++----
 drivers/watchdog/orion_wdt.c                      | 15 +++++++++++----
 6 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-dove/include/mach/bridge-regs.h b/arch/arm/mach-dove/include/mach/bridge-regs.h
index 5362df3..a0ddc94 100644
--- a/arch/arm/mach-dove/include/mach/bridge-regs.h
+++ b/arch/arm/mach-dove/include/mach/bridge-regs.h
@@ -52,5 +52,6 @@
 
 #define TIMER_VIRT_BASE		(BRIDGE_VIRT_BASE + 0x0300)
 #define TIMER_PHYS_BASE         (BRIDGE_PHYS_BASE + 0x0300)
+#define WDT_COUNTER_OFF		0x24
 
 #endif
diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
index 91242c9..05a1164 100644
--- a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
+++ b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
@@ -38,6 +38,7 @@
 
 #define TIMER_VIRT_BASE		(BRIDGE_VIRT_BASE + 0x0300)
 #define TIMER_PHYS_BASE		(BRIDGE_PHYS_BASE + 0x0300)
+#define WDT_COUNTER_OFF		0x24
 
 #define L2_CONFIG_REG		(BRIDGE_VIRT_BASE + 0x0128)
 #define L2_WRITETHROUGH		0x00000010
diff --git a/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h b/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h
index 5f03484..fa49bb6 100644
--- a/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h
+++ b/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h
@@ -32,5 +32,6 @@
 
 #define TIMER_VIRT_BASE		(BRIDGE_VIRT_BASE + 0x0300)
 #define TIMER_PHYS_BASE		(BRIDGE_PHYS_BASE + 0x0300)
+#define WDT_COUNTER_OFF		0x24
 
 #endif
diff --git a/arch/arm/mach-orion5x/include/mach/bridge-regs.h b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
index f727d03..634ce6a 100644
--- a/arch/arm/mach-orion5x/include/mach/bridge-regs.h
+++ b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
@@ -33,4 +33,5 @@
 
 #define TIMER_VIRT_BASE		(ORION5X_BRIDGE_VIRT_BASE + 0x300)
 #define TIMER_PHYS_BASE		(ORION5X_BRIDGE_PHYS_BASE + 0x300)
+#define WDT_COUNTER_OFF		0x24
 #endif
diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
index c66d163..4e30ed6 100644
--- a/arch/arm/plat-orion/common.c
+++ b/arch/arm/plat-orion/common.c
@@ -594,14 +594,16 @@ void __init orion_spi_1_init(unsigned long mapbase)
 /*****************************************************************************
  * Watchdog
  ****************************************************************************/
-static struct resource orion_wdt_resource =
-		DEFINE_RES_MEM(TIMER_PHYS_BASE, 0x28);
+static struct resource orion_wdt_resource[] = {
+		DEFINE_RES_MEM(TIMER_PHYS_BASE, 0x04),
+		DEFINE_RES_MEM(TIMER_PHYS_BASE + WDT_COUNTER_OFF, 0x04),
+};
 
 static struct platform_device orion_wdt_device = {
 	.name		= "orion_wdt",
 	.id		= -1,
-	.num_resources	= 1,
-	.resource	= &orion_wdt_resource,
+	.num_resources	= ARRAY_SIZE(orion_wdt_resource),
+	.resource	= orion_wdt_resource,
 };
 
 void __init orion_wdt_init(void)
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 74d3b72..83350ad 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -32,7 +32,6 @@
  */
 #define TIMER_CTRL		0x0000
 #define WDT_EN			0x0010
-#define WDT_VAL			0x0024
 
 #define WDT_MAX_CYCLE_COUNT	0xffffffff
 #define WDT_IN_USE		0
@@ -46,6 +45,7 @@ static unsigned int wdt_max_duration;	/* (seconds) */
 static struct clk *clk;
 static unsigned int wdt_tclk;
 static void __iomem *wdt_reg;
+static void __iomem *wdt_val;
 static DEFINE_SPINLOCK(wdt_lock);
 
 static int orion_wdt_ping(struct watchdog_device *wdt_dev)
@@ -53,7 +53,7 @@ static int orion_wdt_ping(struct watchdog_device *wdt_dev)
 	spin_lock(&wdt_lock);
 
 	/* Reload watchdog duration */
-	writel(wdt_tclk * wdt_dev->timeout, wdt_reg + WDT_VAL);
+	writel(wdt_tclk * wdt_dev->timeout, wdt_val);
 
 	spin_unlock(&wdt_lock);
 	return 0;
@@ -66,7 +66,7 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
 	spin_lock(&wdt_lock);
 
 	/* Set watchdog duration */
-	writel(wdt_tclk * wdt_dev->timeout, wdt_reg + WDT_VAL);
+	writel(wdt_tclk * wdt_dev->timeout, wdt_val);
 
 	/* Enable watchdog timer */
 	reg = readl(wdt_reg + TIMER_CTRL);
@@ -107,7 +107,7 @@ static unsigned int orion_wdt_get_timeleft(struct watchdog_device *wdt_dev)
 	unsigned int time_left;
 
 	spin_lock(&wdt_lock);
-	time_left = readl(wdt_reg + WDT_VAL) / wdt_tclk;
+	time_left = readl(wdt_val) / wdt_tclk;
 	spin_unlock(&wdt_lock);
 
 	return time_left;
@@ -160,6 +160,13 @@ static int orion_wdt_probe(struct platform_device *pdev)
 	if (!wdt_reg)
 		return -ENOMEM;
 
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!res)
+		return -ENODEV;
+	wdt_val = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+	if (!wdt_val)
+		return -ENOMEM;
+
 	wdt_max_duration = WDT_MAX_CYCLE_COUNT / wdt_tclk;
 
 	orion_wdt.timeout = wdt_max_duration;
-- 
1.8.1.5

[PATCH v2 3/9] ARM: orion: Assert watchdog RSTOUT enable bit

[Ezequiel Garcia]

In order to prevent orion_wdt from accessing the RSTOUTn_MASK register,
it's cleaner to enable the watchdog RSTOUT bit in board initialization time.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-dove/board-dt.c                     | 2 ++
 arch/arm/mach-dove/common.c                       | 2 ++
 arch/arm/mach-dove/include/mach/bridge-regs.h     | 1 +
 arch/arm/mach-kirkwood/board-dt.c                 | 2 ++
 arch/arm/mach-kirkwood/common.c                   | 2 ++
 arch/arm/mach-kirkwood/include/mach/bridge-regs.h | 1 +
 arch/arm/mach-mv78xx0/common.c                    | 2 ++
 arch/arm/mach-mv78xx0/include/mach/bridge-regs.h  | 1 +
 arch/arm/mach-orion5x/board-dt.c                  | 4 ++++
 arch/arm/mach-orion5x/common.c                    | 2 ++
 arch/arm/mach-orion5x/include/mach/bridge-regs.h  | 1 +
 arch/arm/plat-orion/common.c                      | 5 +++++
 arch/arm/plat-orion/include/plat/common.h         | 1 +
 13 files changed, 26 insertions(+)

diff --git a/arch/arm/mach-dove/board-dt.c b/arch/arm/mach-dove/board-dt.c
index f3755ac..6e9570c 100644
--- a/arch/arm/mach-dove/board-dt.c
+++ b/arch/arm/mach-dove/board-dt.c
@@ -60,6 +60,8 @@ static void __init dove_dt_init(void)
 {
 	pr_info("Dove 88AP510 SoC\n");
 
+	orion_wdt_reset_enable();
+
 #ifdef CONFIG_CACHE_TAUROS2
 	tauros2_init(0);
 #endif
diff --git a/arch/arm/mach-dove/common.c b/arch/arm/mach-dove/common.c
index 00247c7..ac8b4bb 100644
--- a/arch/arm/mach-dove/common.c
+++ b/arch/arm/mach-dove/common.c
@@ -367,6 +367,8 @@ void __init dove_init(void)
 	pr_info("Dove 88AP510 SoC, TCLK = %d MHz.\n",
 		(dove_tclk + 499999) / 1000000);
 
+	orion_wdt_reset_enable();
+
 #ifdef CONFIG_CACHE_TAUROS2
 	tauros2_init(0);
 #endif
diff --git a/arch/arm/mach-dove/include/mach/bridge-regs.h b/arch/arm/mach-dove/include/mach/bridge-regs.h
index a0ddc94..bdd9b00 100644
--- a/arch/arm/mach-dove/include/mach/bridge-regs.h
+++ b/arch/arm/mach-dove/include/mach/bridge-regs.h
@@ -22,6 +22,7 @@
 
 #define RSTOUTn_MASK		(BRIDGE_VIRT_BASE + 0x0108)
 #define  SOFT_RESET_OUT_EN	0x00000004
+#define  WDT_RESET_OUT_EN	0x00000002
 
 #define SYSTEM_SOFT_RESET	(BRIDGE_VIRT_BASE + 0x010c)
 #define  SOFT_RESET		0x00000001
diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c
index 6e122ed..aa8d9d6 100644
--- a/arch/arm/mach-kirkwood/board-dt.c
+++ b/arch/arm/mach-kirkwood/board-dt.c
@@ -87,6 +87,8 @@ static void __init kirkwood_dt_init(void)
 	 */
 	writel(readl(CPU_CONFIG) & ~CPU_CONFIG_ERROR_PROP, CPU_CONFIG);
 
+	orion_wdt_reset_enable();
+
 	kirkwood_setup_wins();
 
 	kirkwood_l2_init();
diff --git a/arch/arm/mach-kirkwood/common.c b/arch/arm/mach-kirkwood/common.c
index e9238b5..dc0f353 100644
--- a/arch/arm/mach-kirkwood/common.c
+++ b/arch/arm/mach-kirkwood/common.c
@@ -703,6 +703,8 @@ void __init kirkwood_init(void)
 	 */
 	writel(readl(CPU_CONFIG) & ~CPU_CONFIG_ERROR_PROP, CPU_CONFIG);
 
+	orion_wdt_reset_enable();
+
 	kirkwood_setup_wins();
 
 	kirkwood_l2_init();
diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
index 05a1164..d6ff3cf 100644
--- a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
+++ b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
@@ -22,6 +22,7 @@
 
 #define RSTOUTn_MASK		(BRIDGE_VIRT_BASE + 0x0108)
 #define SOFT_RESET_OUT_EN	0x00000004
+#define WDT_RESET_OUT_EN	0x00000002
 
 #define SYSTEM_SOFT_RESET	(BRIDGE_VIRT_BASE + 0x010c)
 #define SOFT_RESET		0x00000001
diff --git a/arch/arm/mach-mv78xx0/common.c b/arch/arm/mach-mv78xx0/common.c
index 75062ef..be326bb 100644
--- a/arch/arm/mach-mv78xx0/common.c
+++ b/arch/arm/mach-mv78xx0/common.c
@@ -394,6 +394,8 @@ void __init mv78xx0_init(void)
 	int pclk;
 	int l2clk;
 
+	orion_wdt_reset_enable();
+
 	core_index = mv78xx0_core_index();
 	hclk = get_hclk();
 	get_pclk_l2clk(hclk, core_index, &pclk, &l2clk);
diff --git a/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h b/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h
index fa49bb6..1c85678 100644
--- a/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h
+++ b/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h
@@ -16,6 +16,7 @@
 
 #define RSTOUTn_MASK		(BRIDGE_VIRT_BASE + 0x0108)
 #define SOFT_RESET_OUT_EN	0x00000004
+#define WDT_RESET_OUT_EN	0x00000002
 
 #define SYSTEM_SOFT_RESET	(BRIDGE_VIRT_BASE + 0x010c)
 #define SOFT_RESET		0x00000001
diff --git a/arch/arm/mach-orion5x/board-dt.c b/arch/arm/mach-orion5x/board-dt.c
index b91002c..61fbdc1 100644
--- a/arch/arm/mach-orion5x/board-dt.c
+++ b/arch/arm/mach-orion5x/board-dt.c
@@ -15,10 +15,12 @@
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/cpu.h>
+#include <linux/clk-provider.h>
 #include <asm/system_misc.h>
 #include <asm/mach/arch.h>
 #include <mach/orion5x.h>
 #include <plat/irq.h>
+#include <plat/common.h>
 #include "common.h"
 
 struct of_dev_auxdata orion5x_auxdata_lookup[] __initdata = {
@@ -39,6 +41,8 @@ static void __init orion5x_dt_init(void)
 	orion5x_id(&dev, &rev, &dev_name);
 	printk(KERN_INFO "Orion ID: %s. TCLK=%d.\n", dev_name, orion5x_tclk);
 
+	orion_wdt_reset_enable();
+
 	/*
 	 * Setup Orion address map
 	 */
diff --git a/arch/arm/mach-orion5x/common.c b/arch/arm/mach-orion5x/common.c
index b41599f..a2583ac 100644
--- a/arch/arm/mach-orion5x/common.c
+++ b/arch/arm/mach-orion5x/common.c
@@ -316,6 +316,8 @@ void __init orion5x_init(void)
 	orion5x_id(&dev, &rev, &dev_name);
 	printk(KERN_INFO "Orion ID: %s. TCLK=%d.\n", dev_name, orion5x_tclk);
 
+	orion_wdt_reset_enable();
+
 	/*
 	 * Setup Orion address map
 	 */
diff --git a/arch/arm/mach-orion5x/include/mach/bridge-regs.h b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
index 634ce6a..5fc0d62 100644
--- a/arch/arm/mach-orion5x/include/mach/bridge-regs.h
+++ b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
@@ -18,6 +18,7 @@
 #define CPU_CTRL		(ORION5X_BRIDGE_VIRT_BASE + 0x104)
 
 #define RSTOUTn_MASK		(ORION5X_BRIDGE_VIRT_BASE + 0x108)
+#define WDT_RESET_OUT_EN	0x00000002
 
 #define CPU_SOFT_RESET		(ORION5X_BRIDGE_VIRT_BASE + 0x10c)
 
diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
index 4e30ed6..cfad2d6 100644
--- a/arch/arm/plat-orion/common.c
+++ b/arch/arm/plat-orion/common.c
@@ -606,6 +606,11 @@ static struct platform_device orion_wdt_device = {
 	.resource	= orion_wdt_resource,
 };
 
+void __init orion_wdt_reset_enable(void)
+{
+	writel(readl(RSTOUTn_MASK) | WDT_RESET_OUT_EN, RSTOUTn_MASK);
+}
+
 void __init orion_wdt_init(void)
 {
 	platform_device_register(&orion_wdt_device);
diff --git a/arch/arm/plat-orion/include/plat/common.h b/arch/arm/plat-orion/include/plat/common.h
index d9a24f6..3c81aa6 100644
--- a/arch/arm/plat-orion/include/plat/common.h
+++ b/arch/arm/plat-orion/include/plat/common.h
@@ -75,6 +75,7 @@ void __init orion_spi_init(unsigned long mapbase);
 
 void __init orion_spi_1_init(unsigned long mapbase);
 
+void __init orion_wdt_reset_enable(void);
 void __init orion_wdt_init(void);
 
 void __init orion_xor0_init(unsigned long mapbase_low,
-- 
1.8.1.5

[PATCH v2 4/9] watchdog: orion: Remove RSTOUT bit enable/disable

[Ezequiel Garcia]

The watchdog RSTOUT bit must be enabled on a per-machine
basis at machine initialization time, either at watchdog device
registration or at reset-controller initialization.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/watchdog/orion_wdt.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 83350ad..d6f6128 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -37,8 +37,6 @@
 #define WDT_IN_USE		0
 #define WDT_OK_TO_CLOSE		1
 
-#define WDT_RESET_OUT_EN	BIT(1)
-
 static bool nowayout = WATCHDOG_NOWAYOUT;
 static int heartbeat = -1;		/* module parameter (seconds) */
 static unsigned int wdt_max_duration;	/* (seconds) */
@@ -73,11 +71,6 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
 	reg |= WDT_EN;
 	writel(reg, wdt_reg + TIMER_CTRL);
 
-	/* Enable reset on watchdog */
-	reg = readl(RSTOUTn_MASK);
-	reg |= WDT_RESET_OUT_EN;
-	writel(reg, RSTOUTn_MASK);
-
 	spin_unlock(&wdt_lock);
 	return 0;
 }
@@ -88,11 +81,6 @@ static int orion_wdt_stop(struct watchdog_device *wdt_dev)
 
 	spin_lock(&wdt_lock);
 
-	/* Disable reset on watchdog */
-	reg = readl(RSTOUTn_MASK);
-	reg &= ~WDT_RESET_OUT_EN;
-	writel(reg, RSTOUTn_MASK);
-
 	/* Disable watchdog timer */
 	reg = readl(wdt_reg + TIMER_CTRL);
 	reg &= ~WDT_EN;
-- 
1.8.1.5

[PATCH v2 5/9] watchdog: orion: Allow to build in any Orion platform

[Ezequiel Garcia]

This driver has no need for the mach-specific header, so remove it.
Therefore, it's no possible to allow builds in any Orion platform.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/watchdog/Kconfig     | 2 +-
 drivers/watchdog/orion_wdt.c | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 362085d..524f30a 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -282,7 +282,7 @@ config DAVINCI_WATCHDOG
 
 config ORION_WATCHDOG
 	tristate "Orion watchdog"
-	depends on ARCH_ORION5X || ARCH_KIRKWOOD || ARCH_DOVE
+	depends on PLAT_ORION
 	select WATCHDOG_CORE
 	help
 	  Say Y here if to include support for the watchdog timer
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index d6f6128..d508ff3 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -25,7 +25,6 @@
 #include <linux/clk.h>
 #include <linux/err.h>
 #include <linux/of.h>
-#include <mach/bridge-regs.h>
 
 /*
  * Watchdog timer block registers.
-- 
1.8.1.5

[PATCH v2 6/9] watchdog: orion: Rename device-tree binding documentation

[Ezequiel Garcia]

Name this file to something a bit more judicious.

Cc: devicetree at vger.kernel.org
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 Documentation/devicetree/bindings/watchdog/{marvel.txt => orion-wdt.txt} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename Documentation/devicetree/bindings/watchdog/{marvel.txt => orion-wdt.txt} (100%)

diff --git a/Documentation/devicetree/bindings/watchdog/marvel.txt b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
similarity index 100%
rename from Documentation/devicetree/bindings/watchdog/marvel.txt
rename to Documentation/devicetree/bindings/watchdog/orion-wdt.txt
-- 
1.8.1.5

[PATCH v2 7/9] watchdog: orion: Update device-tree binding documentation

[Ezequiel Garcia]

Change the 'reg' property meaning, by defining two required cells.
It's important to note this commit breaks DT-compatibility for this
device.

Cc: devicetree at vger.kernel.org
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 Documentation/devicetree/bindings/watchdog/orion-wdt.txt | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
index 5dc8d30..a74c9c8 100644
--- a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
@@ -3,7 +3,9 @@
 Required Properties:
 
 - Compatibility : "marvell,orion-wdt"
-- reg		: Address of the timer registers
+- reg		: Two cells are required:
+		  First  cell contains the global timer control register.
+		  Second cell contains the watchdog counter register.
 
 Optional properties:
 
@@ -13,7 +15,8 @@ Example:
 
 	wdt at 20300 {
 		compatible = "marvell,orion-wdt";
-		reg = <0x20300 0x28>;
+		reg = <0x20300 0x4
+		       0x20324 0x4>;
 		timeout-sec = <10>;
 		status = "okay";
 	};
-- 
1.8.1.5

[PATCH v2 8/9] ARM: kirkwood: Update watchdog 'reg' property

[Ezequiel Garcia]

According to the new DT binding, the 'reg' property requires
two cells, which mean: timer control register and watchdog counter
register respectively.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/boot/dts/kirkwood.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi
index 9809fc1..6443e12 100644
--- a/arch/arm/boot/dts/kirkwood.dtsi
+++ b/arch/arm/boot/dts/kirkwood.dtsi
@@ -105,7 +105,8 @@
 
 		wdt at 20300 {
 			compatible = "marvell,orion-wdt";
-			reg = <0x20300 0x28>;
+			reg = <0x20300 0x4>, /* Timer control */
+			      <0x20324 0x4>; /* Watchdog counter */
 			clocks = <&gate_clk 7>;
 			status = "okay";
 		};
-- 
1.8.1.5

[PATCH v2 9/9] ARM: orion5x: Update orion-wdt DT node

[Ezequiel Garcia]

According to the new DT binding, the 'reg' property requires
two cells, which mean: timer control register and watchdog
counter register respectively.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/boot/dts/orion5x.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/orion5x.dtsi b/arch/arm/boot/dts/orion5x.dtsi
index 892c64e..0cce136 100644
--- a/arch/arm/boot/dts/orion5x.dtsi
+++ b/arch/arm/boot/dts/orion5x.dtsi
@@ -70,7 +70,8 @@
 
 		wdt at 20300 {
 			compatible = "marvell,orion-wdt";
-			reg = <0x20300 0x28>;
+			reg = <0x20300 0x4>,
+			      <0x20324 0x4>;
 			status = "okay";
 		};
 
-- 
1.8.1.5

Re: [PATCH v2 7/9] watchdog: orion: Update device-tree binding documentation

[Jason Cooper]

On Fri, Aug 23, 2013 at 07:12:20PM -0300, Ezequiel Garcia wrote:
> Change the 'reg' property meaning, by defining two required cells.
> It's important to note this commit breaks DT-compatibility for this
> device.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  Documentation/devicetree/bindings/watchdog/orion-wdt.txt | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> index 5dc8d30..a74c9c8 100644
> --- a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> +++ b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> @@ -3,7 +3,9 @@
>  Required Properties:
>  
>  - Compatibility : "marvell,orion-wdt"
> -- reg		: Address of the timer registers
> +- reg		: Two cells are required:
> +		  First  cell contains the global timer control register.
> +		  Second cell contains the watchdog counter register.
>  
>  Optional properties:
>  
> @@ -13,7 +15,8 @@ Example:
>  
>  	wdt@20300 {
>  		compatible = "marvell,orion-wdt";
> -		reg = <0x20300 0x28>;
> +		reg = <0x20300 0x4


> +		       0x20324 0x4>;

Is there a reason we're going this route over adding a new compatible
string? iow, why can't we keep this knowledge in the driver and have a
"marvell,armada-wdt" or similar compat string that the orion-wdt driver
also serves?

thx,

Jason.

[PATCH v2 7/9] watchdog: orion: Update device-tree binding documentation

[Jason Cooper]

On Fri, Aug 23, 2013 at 07:12:20PM -0300, Ezequiel Garcia wrote:
> Change the 'reg' property meaning, by defining two required cells.
> It's important to note this commit breaks DT-compatibility for this
> device.
> 
> Cc: devicetree at vger.kernel.org
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  Documentation/devicetree/bindings/watchdog/orion-wdt.txt | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> index 5dc8d30..a74c9c8 100644
> --- a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> +++ b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> @@ -3,7 +3,9 @@
>  Required Properties:
>  
>  - Compatibility : "marvell,orion-wdt"
> -- reg		: Address of the timer registers
> +- reg		: Two cells are required:
> +		  First  cell contains the global timer control register.
> +		  Second cell contains the watchdog counter register.
>  
>  Optional properties:
>  
> @@ -13,7 +15,8 @@ Example:
>  
>  	wdt at 20300 {
>  		compatible = "marvell,orion-wdt";
> -		reg = <0x20300 0x28>;
> +		reg = <0x20300 0x4


> +		       0x20324 0x4>;

Is there a reason we're going this route over adding a new compatible
string? iow, why can't we keep this knowledge in the driver and have a
"marvell,armada-wdt" or similar compat string that the orion-wdt driver
also serves?

thx,

Jason.

Re: [PATCH v2 7/9] watchdog: orion: Update device-tree binding documentation

[Ezequiel Garcia]

On Mon, Aug 26, 2013 at 10:08:43AM -0400, Jason Cooper wrote:
> On Fri, Aug 23, 2013 at 07:12:20PM -0300, Ezequiel Garcia wrote:
> > Change the 'reg' property meaning, by defining two required cells.
> > It's important to note this commit breaks DT-compatibility for this
> > device.
> > 
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > ---
> >  Documentation/devicetree/bindings/watchdog/orion-wdt.txt | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> > index 5dc8d30..a74c9c8 100644
> > --- a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> > +++ b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> > @@ -3,7 +3,9 @@
> >  Required Properties:
> >  
> >  - Compatibility : "marvell,orion-wdt"
> > -- reg		: Address of the timer registers
> > +- reg		: Two cells are required:
> > +		  First  cell contains the global timer control register.
> > +		  Second cell contains the watchdog counter register.
> >  
> >  Optional properties:
> >  
> > @@ -13,7 +15,8 @@ Example:
> >  
> >  	wdt@20300 {
> >  		compatible = "marvell,orion-wdt";
> > -		reg = <0x20300 0x28>;
> > +		reg = <0x20300 0x4
> 
> 
> > +		       0x20324 0x4>;
> 
> Is there a reason we're going this route over adding a new compatible
> string?

Well, it seemed to me that this register splitting was more device-treeish:
it prevents you from fixing your driver, adding a new compatible-string,
and rebuilding a kernel each time a new SoC appears with a different offset
between registers.

Instead, and trying to follow the DT-preachers, we would just change the
"reg" values and -bang!- have forward-compatibility :-)

That said...

> iow, why can't we keep this knowledge in the driver and have a
> "marvell,armada-wdt" or similar compat string that the orion-wdt driver
> also serves?
> 

... we still need new compatible strings for armada-xp-wdt and
armada-370-wdt, for they have differences between each other
and with the orion-wdt:

* The clock input is obtained in a different way in each case

* The watchdog enable bit inside the timer control register
  is at a different location.

So, thinking about this again, perhaps we should simply let alone the
"reg" property and add the watchdog counter offset as yet another field
in the compatible-data?

What do you think?
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 7/9] watchdog: orion: Update device-tree binding documentation

[Ezequiel Garcia]

On Mon, Aug 26, 2013 at 10:08:43AM -0400, Jason Cooper wrote:
> On Fri, Aug 23, 2013 at 07:12:20PM -0300, Ezequiel Garcia wrote:
> > Change the 'reg' property meaning, by defining two required cells.
> > It's important to note this commit breaks DT-compatibility for this
> > device.
> > 
> > Cc: devicetree at vger.kernel.org
> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > ---
> >  Documentation/devicetree/bindings/watchdog/orion-wdt.txt | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> > index 5dc8d30..a74c9c8 100644
> > --- a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> > +++ b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> > @@ -3,7 +3,9 @@
> >  Required Properties:
> >  
> >  - Compatibility : "marvell,orion-wdt"
> > -- reg		: Address of the timer registers
> > +- reg		: Two cells are required:
> > +		  First  cell contains the global timer control register.
> > +		  Second cell contains the watchdog counter register.
> >  
> >  Optional properties:
> >  
> > @@ -13,7 +15,8 @@ Example:
> >  
> >  	wdt at 20300 {
> >  		compatible = "marvell,orion-wdt";
> > -		reg = <0x20300 0x28>;
> > +		reg = <0x20300 0x4
> 
> 
> > +		       0x20324 0x4>;
> 
> Is there a reason we're going this route over adding a new compatible
> string?

Well, it seemed to me that this register splitting was more device-treeish:
it prevents you from fixing your driver, adding a new compatible-string,
and rebuilding a kernel each time a new SoC appears with a different offset
between registers.

Instead, and trying to follow the DT-preachers, we would just change the
"reg" values and -bang!- have forward-compatibility :-)

That said...

> iow, why can't we keep this knowledge in the driver and have a
> "marvell,armada-wdt" or similar compat string that the orion-wdt driver
> also serves?
> 

... we still need new compatible strings for armada-xp-wdt and
armada-370-wdt, for they have differences between each other
and with the orion-wdt:

* The clock input is obtained in a different way in each case

* The watchdog enable bit inside the timer control register
  is at a different location.

So, thinking about this again, perhaps we should simply let alone the
"reg" property and add the watchdog counter offset as yet another field
in the compatible-data?

What do you think?
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v2 7/9] watchdog: orion: Update device-tree binding documentation

[Thomas Petazzoni]

Dear Ezequiel Garcia,

On Tue, 27 Aug 2013 05:36:04 -0300, Ezequiel Garcia wrote:

> > Is there a reason we're going this route over adding a new compatible
> > string?
> 
> Well, it seemed to me that this register splitting was more device-treeish:
> it prevents you from fixing your driver, adding a new compatible-string,
> and rebuilding a kernel each time a new SoC appears with a different offset
> between registers.

I don't think encoding register offsets in the DT is a good idea. The
compatible string is here to identify different revisions/versions of
the same hardware block, and the driver should abstract out the
details. Otherwise, you'll start encoding bit numbers, mask values, and
other crazy things in the Device Tree.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH v2 7/9] watchdog: orion: Update device-tree binding documentation

[Thomas Petazzoni]

Dear Ezequiel Garcia,

On Tue, 27 Aug 2013 05:36:04 -0300, Ezequiel Garcia wrote:

> > Is there a reason we're going this route over adding a new compatible
> > string?
> 
> Well, it seemed to me that this register splitting was more device-treeish:
> it prevents you from fixing your driver, adding a new compatible-string,
> and rebuilding a kernel each time a new SoC appears with a different offset
> between registers.

I don't think encoding register offsets in the DT is a good idea. The
compatible string is here to identify different revisions/versions of
the same hardware block, and the driver should abstract out the
details. Otherwise, you'll start encoding bit numbers, mask values, and
other crazy things in the Device Tree.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH v2 7/9] watchdog: orion: Update device-tree binding documentation

[Jason Cooper]

On Tue, Aug 27, 2013 at 10:39:07AM +0200, Thomas Petazzoni wrote:
> Dear Ezequiel Garcia,
> 
> On Tue, 27 Aug 2013 05:36:04 -0300, Ezequiel Garcia wrote:
> 
> > > Is there a reason we're going this route over adding a new compatible
> > > string?
> > 
> > Well, it seemed to me that this register splitting was more device-treeish:
> > it prevents you from fixing your driver, adding a new compatible-string,
> > and rebuilding a kernel each time a new SoC appears with a different offset
> > between registers.
> 
> I don't think encoding register offsets in the DT is a good idea. The
> compatible string is here to identify different revisions/versions of
> the same hardware block, and the driver should abstract out the
> details. Otherwise, you'll start encoding bit numbers, mask values, and
> other crazy things in the Device Tree.

Agreed.  DT is for describing how the hardware is wired together on a
specific board.  The compatible strings should tell you which variant of
an IP block you have.  The information you're trying to encode doesn't
belong in the DT, it should be in the driver.

As an added bonus, by doing so, you avoid breaking compatibility. ;-)

thx,

Jason.

[PATCH v2 7/9] watchdog: orion: Update device-tree binding documentation

[Jason Cooper]

On Tue, Aug 27, 2013 at 10:39:07AM +0200, Thomas Petazzoni wrote:
> Dear Ezequiel Garcia,
> 
> On Tue, 27 Aug 2013 05:36:04 -0300, Ezequiel Garcia wrote:
> 
> > > Is there a reason we're going this route over adding a new compatible
> > > string?
> > 
> > Well, it seemed to me that this register splitting was more device-treeish:
> > it prevents you from fixing your driver, adding a new compatible-string,
> > and rebuilding a kernel each time a new SoC appears with a different offset
> > between registers.
> 
> I don't think encoding register offsets in the DT is a good idea. The
> compatible string is here to identify different revisions/versions of
> the same hardware block, and the driver should abstract out the
> details. Otherwise, you'll start encoding bit numbers, mask values, and
> other crazy things in the Device Tree.

Agreed.  DT is for describing how the hardware is wired together on a
specific board.  The compatible strings should tell you which variant of
an IP block you have.  The information you're trying to encode doesn't
belong in the DT, it should be in the driver.

As an added bonus, by doing so, you avoid breaking compatibility. ;-)

thx,

Jason.

Re: [PATCH v2 7/9] watchdog: orion: Update device-tree binding documentation

[Stephen Warren]

On 08/27/2013 02:36 AM, Ezequiel Garcia wrote:
> On Mon, Aug 26, 2013 at 10:08:43AM -0400, Jason Cooper wrote:
>> On Fri, Aug 23, 2013 at 07:12:20PM -0300, Ezequiel Garcia wrote:
>>> Change the 'reg' property meaning, by defining two required cells.
>>> It's important to note this commit breaks DT-compatibility for this
>>> device.
>>>
>>> Cc: devicetree@vger.kernel.org
>>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
>>> ---
>>>  Documentation/devicetree/bindings/watchdog/orion-wdt.txt | 7 +++++--
>>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
>>> index 5dc8d30..a74c9c8 100644
>>> --- a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
>>> +++ b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
>>> @@ -3,7 +3,9 @@
>>>  Required Properties:
>>>  
>>>  - Compatibility : "marvell,orion-wdt"
>>> -- reg		: Address of the timer registers
>>> +- reg		: Two cells are required:
>>> +		  First  cell contains the global timer control register.
>>> +		  Second cell contains the watchdog counter register.

BTW, you don't have 2 cells here, but 4 cells (assuming typical
#address-cells and #size-cells values). Instead, you have 2 register
specifiers.

[PATCH v2 7/9] watchdog: orion: Update device-tree binding documentation

[Stephen Warren]

On 08/27/2013 02:36 AM, Ezequiel Garcia wrote:
> On Mon, Aug 26, 2013 at 10:08:43AM -0400, Jason Cooper wrote:
>> On Fri, Aug 23, 2013 at 07:12:20PM -0300, Ezequiel Garcia wrote:
>>> Change the 'reg' property meaning, by defining two required cells.
>>> It's important to note this commit breaks DT-compatibility for this
>>> device.
>>>
>>> Cc: devicetree at vger.kernel.org
>>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
>>> ---
>>>  Documentation/devicetree/bindings/watchdog/orion-wdt.txt | 7 +++++--
>>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
>>> index 5dc8d30..a74c9c8 100644
>>> --- a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
>>> +++ b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
>>> @@ -3,7 +3,9 @@
>>>  Required Properties:
>>>  
>>>  - Compatibility : "marvell,orion-wdt"
>>> -- reg		: Address of the timer registers
>>> +- reg		: Two cells are required:
>>> +		  First  cell contains the global timer control register.
>>> +		  Second cell contains the watchdog counter register.

BTW, you don't have 2 cells here, but 4 cells (assuming typical
#address-cells and #size-cells values). Instead, you have 2 register
specifiers.