[PATCH 00/10] Orion Watchdog fixes

[PATCH 00/10] Orion Watchdog fixes

[Ezequiel Garcia]

This patchset introduces a bunch of fixes that removes the direct use of
the shared timer control register, and also removes the need to include
a mach-specific header.

With this changes the driver can be included in multiplatforms builds,
and in particular be used for the Armada 370/XP SoC.

I'm sending this early patchset mainly to discuss the possibility of
introducing this changes. As cleared stated in each patch, this series
breaks device-tree compatilibity by changing the meaning of the watchdog
node 'reg' property.

I'm aware of the implications of this, but I'm not sure how 'stable' the
Kirkwood DT is considered. So, given we're currently moving things around,
maybe there is still a chance to do this.

This patchset applies on top of Sebastian Hesselbarth's branch:

  git://github.com/shesselba/linux-dove.git orion-irqchip-for-v3.11_v4

Ezequiel Garcia (10):
  clocksource: orion: Add thread-safe API header
  watchdog: orion: Use thread-safe clocksource API
  watchdog: orion: Rename device-tree binding documentation
  watchdog: orion: Use the proper watchdog register
  watchdog: orion: Add a memory resource for RSTOUT register
  watchdog: orion: Update device-tree binding documentation
  watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
  watchdog: orion: Remove mach-specific unneeded header
  watchdog: orion: Use BIT()
  ARM: kirkwood: Fix the device-tree watchdog's node reg property

 .../watchdog/{marvel.txt => orion-wdt.txt}         |  8 ++--
 arch/arm/boot/dts/kirkwood.dtsi                    |  3 +-
 arch/arm/mach-kirkwood/include/mach/bridge-regs.h  |  2 +
 arch/arm/mach-orion5x/include/mach/bridge-regs.h   |  2 +
 arch/arm/plat-orion/common.c                       | 10 +++--
 drivers/watchdog/orion_wdt.c                       | 46 +++++++++-------------
 include/linux/time-orion.h                         |  7 ++++
 7 files changed, 42 insertions(+), 36 deletions(-)
 rename Documentation/devicetree/bindings/watchdog/{marvel.txt => orion-wdt.txt} (58%)
 create mode 100644 include/linux/time-orion.h

-- 
1.8.1.5

[PATCH 00/10] Orion Watchdog fixes

[Ezequiel Garcia]

This patchset introduces a bunch of fixes that removes the direct use of
the shared timer control register, and also removes the need to include
a mach-specific header.

With this changes the driver can be included in multiplatforms builds,
and in particular be used for the Armada 370/XP SoC.

I'm sending this early patchset mainly to discuss the possibility of
introducing this changes. As cleared stated in each patch, this series
breaks device-tree compatilibity by changing the meaning of the watchdog
node 'reg' property.

I'm aware of the implications of this, but I'm not sure how 'stable' the
Kirkwood DT is considered. So, given we're currently moving things around,
maybe there is still a chance to do this.

This patchset applies on top of Sebastian Hesselbarth's branch:

  git://github.com/shesselba/linux-dove.git orion-irqchip-for-v3.11_v4

Ezequiel Garcia (10):
  clocksource: orion: Add thread-safe API header
  watchdog: orion: Use thread-safe clocksource API
  watchdog: orion: Rename device-tree binding documentation
  watchdog: orion: Use the proper watchdog register
  watchdog: orion: Add a memory resource for RSTOUT register
  watchdog: orion: Update device-tree binding documentation
  watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
  watchdog: orion: Remove mach-specific unneeded header
  watchdog: orion: Use BIT()
  ARM: kirkwood: Fix the device-tree watchdog's node reg property

 .../watchdog/{marvel.txt => orion-wdt.txt}         |  8 ++--
 arch/arm/boot/dts/kirkwood.dtsi                    |  3 +-
 arch/arm/mach-kirkwood/include/mach/bridge-regs.h  |  2 +
 arch/arm/mach-orion5x/include/mach/bridge-regs.h   |  2 +
 arch/arm/plat-orion/common.c                       | 10 +++--
 drivers/watchdog/orion_wdt.c                       | 46 +++++++++-------------
 include/linux/time-orion.h                         |  7 ++++
 7 files changed, 42 insertions(+), 36 deletions(-)
 rename Documentation/devicetree/bindings/watchdog/{marvel.txt => orion-wdt.txt} (58%)
 create mode 100644 include/linux/time-orion.h

-- 
1.8.1.5

[PATCH 00/10] Orion Watchdog fixes

[Ezequiel Garcia]

This patchset introduces a bunch of fixes that removes the direct use of
the shared timer control register, and also removes the need to include
a mach-specific header.

With this changes the driver can be included in multiplatforms builds,
and in particular be used for the Armada 370/XP SoC.

I'm sending this early patchset mainly to discuss the possibility of
introducing this changes. As cleared stated in each patch, this series
breaks device-tree compatilibity by changing the meaning of the watchdog
node 'reg' property.

I'm aware of the implications of this, but I'm not sure how 'stable' the
Kirkwood DT is considered. So, given we're currently moving things around,
maybe there is still a chance to do this.

This patchset applies on top of Sebastian Hesselbarth's branch:

  git://github.com/shesselba/linux-dove.git orion-irqchip-for-v3.11_v4

Ezequiel Garcia (10):
  clocksource: orion: Add thread-safe API header
  watchdog: orion: Use thread-safe clocksource API
  watchdog: orion: Rename device-tree binding documentation
  watchdog: orion: Use the proper watchdog register
  watchdog: orion: Add a memory resource for RSTOUT register
  watchdog: orion: Update device-tree binding documentation
  watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
  watchdog: orion: Remove mach-specific unneeded header
  watchdog: orion: Use BIT()
  ARM: kirkwood: Fix the device-tree watchdog's node reg property

 .../watchdog/{marvel.txt => orion-wdt.txt}         |  8 ++--
 arch/arm/boot/dts/kirkwood.dtsi                    |  3 +-
 arch/arm/mach-kirkwood/include/mach/bridge-regs.h  |  2 +
 arch/arm/mach-orion5x/include/mach/bridge-regs.h   |  2 +
 arch/arm/plat-orion/common.c                       | 10 +++--
 drivers/watchdog/orion_wdt.c                       | 46 +++++++++-------------
 include/linux/time-orion.h                         |  7 ++++
 7 files changed, 42 insertions(+), 36 deletions(-)
 rename Documentation/devicetree/bindings/watchdog/{marvel.txt => orion-wdt.txt} (58%)
 create mode 100644 include/linux/time-orion.h

-- 
1.8.1.5

[PATCH 01/10] clocksource: orion: Add thread-safe API header

[Ezequiel Garcia]

Add a header declaration to allow drivers (such as watchdog)
to access this exported API.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 include/linux/time-orion.h | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 include/linux/time-orion.h

diff --git a/include/linux/time-orion.h b/include/linux/time-orion.h
new file mode 100644
index 0000000..c7e6458
--- /dev/null
+++ b/include/linux/time-orion.h
@@ -0,0 +1,7 @@
+
+#ifndef __TIME_ORION_H
+#define __TIME_ORION_H
+
+void orion_timer_ctrl_clrset(u32 clr, u32 set);
+
+#endif
-- 
1.8.1.5

[PATCH 01/10] clocksource: orion: Add thread-safe API header

[Ezequiel Garcia]

Add a header declaration to allow drivers (such as watchdog)
to access this exported API.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 include/linux/time-orion.h | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 include/linux/time-orion.h

diff --git a/include/linux/time-orion.h b/include/linux/time-orion.h
new file mode 100644
index 0000000..c7e6458
--- /dev/null
+++ b/include/linux/time-orion.h
@@ -0,0 +1,7 @@
+
+#ifndef __TIME_ORION_H
+#define __TIME_ORION_H
+
+void orion_timer_ctrl_clrset(u32 clr, u32 set);
+
+#endif
-- 
1.8.1.5

[PATCH 01/10] clocksource: orion: Add thread-safe API header

[Ezequiel Garcia]

Add a header declaration to allow drivers (such as watchdog)
to access this exported API.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 include/linux/time-orion.h | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 include/linux/time-orion.h

diff --git a/include/linux/time-orion.h b/include/linux/time-orion.h
new file mode 100644
index 0000000..c7e6458
--- /dev/null
+++ b/include/linux/time-orion.h
@@ -0,0 +1,7 @@
+
+#ifndef __TIME_ORION_H
+#define __TIME_ORION_H
+
+void orion_timer_ctrl_clrset(u32 clr, u32 set);
+
+#endif
-- 
1.8.1.5

[PATCH 02/10] watchdog: orion: Use thread-safe clocksource API

[Ezequiel Garcia]

The TIMER_CTRL register allows to control timer and watchdog counters,
so it's a register shared between the clocksource and the watchdog
drivers. In order to prevent race-conditions the clocksource driver
exposed a thread-safe API. Use the API.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/watchdog/orion_wdt.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index da57798..c0597f4 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -25,12 +25,12 @@
 #include <linux/clk.h>
 #include <linux/err.h>
 #include <linux/of.h>
+#include <linux/time-orion.h>
 #include <mach/bridge-regs.h>
 
 /*
  * Watchdog timer block registers.
  */
-#define TIMER_CTRL		0x0000
 #define WDT_EN			0x0010
 #define WDT_VAL			0x0024
 
@@ -72,9 +72,7 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
 	writel(reg, BRIDGE_CAUSE);
 
 	/* Enable watchdog timer */
-	reg = readl(wdt_reg + TIMER_CTRL);
-	reg |= WDT_EN;
-	writel(reg, wdt_reg + TIMER_CTRL);
+	orion_timer_ctrl_clrset(0, WDT_EN);
 
 	/* Enable reset on watchdog */
 	reg = readl(RSTOUTn_MASK);
@@ -97,9 +95,7 @@ static int orion_wdt_stop(struct watchdog_device *wdt_dev)
 	writel(reg, RSTOUTn_MASK);
 
 	/* Disable watchdog timer */
-	reg = readl(wdt_reg + TIMER_CTRL);
-	reg &= ~WDT_EN;
-	writel(reg, wdt_reg + TIMER_CTRL);
+	orion_timer_ctrl_clrset(WDT_EN, 0);
 
 	spin_unlock(&wdt_lock);
 	return 0;
-- 
1.8.1.5

[PATCH 02/10] watchdog: orion: Use thread-safe clocksource API

[Ezequiel Garcia]

The TIMER_CTRL register allows to control timer and watchdog counters,
so it's a register shared between the clocksource and the watchdog
drivers. In order to prevent race-conditions the clocksource driver
exposed a thread-safe API. Use the API.

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

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index da57798..c0597f4 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -25,12 +25,12 @@
 #include <linux/clk.h>
 #include <linux/err.h>
 #include <linux/of.h>
+#include <linux/time-orion.h>
 #include <mach/bridge-regs.h>
 
 /*
  * Watchdog timer block registers.
  */
-#define TIMER_CTRL		0x0000
 #define WDT_EN			0x0010
 #define WDT_VAL			0x0024
 
@@ -72,9 +72,7 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
 	writel(reg, BRIDGE_CAUSE);
 
 	/* Enable watchdog timer */
-	reg = readl(wdt_reg + TIMER_CTRL);
-	reg |= WDT_EN;
-	writel(reg, wdt_reg + TIMER_CTRL);
+	orion_timer_ctrl_clrset(0, WDT_EN);
 
 	/* Enable reset on watchdog */
 	reg = readl(RSTOUTn_MASK);
@@ -97,9 +95,7 @@ static int orion_wdt_stop(struct watchdog_device *wdt_dev)
 	writel(reg, RSTOUTn_MASK);
 
 	/* Disable watchdog timer */
-	reg = readl(wdt_reg + TIMER_CTRL);
-	reg &= ~WDT_EN;
-	writel(reg, wdt_reg + TIMER_CTRL);
+	orion_timer_ctrl_clrset(WDT_EN, 0);
 
 	spin_unlock(&wdt_lock);
 	return 0;
-- 
1.8.1.5

[PATCH 02/10] watchdog: orion: Use thread-safe clocksource API

[Ezequiel Garcia]

The TIMER_CTRL register allows to control timer and watchdog counters,
so it's a register shared between the clocksource and the watchdog
drivers. In order to prevent race-conditions the clocksource driver
exposed a thread-safe API. Use the API.

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

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index da57798..c0597f4 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -25,12 +25,12 @@
 #include <linux/clk.h>
 #include <linux/err.h>
 #include <linux/of.h>
+#include <linux/time-orion.h>
 #include <mach/bridge-regs.h>
 
 /*
  * Watchdog timer block registers.
  */
-#define TIMER_CTRL		0x0000
 #define WDT_EN			0x0010
 #define WDT_VAL			0x0024
 
@@ -72,9 +72,7 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
 	writel(reg, BRIDGE_CAUSE);
 
 	/* Enable watchdog timer */
-	reg = readl(wdt_reg + TIMER_CTRL);
-	reg |= WDT_EN;
-	writel(reg, wdt_reg + TIMER_CTRL);
+	orion_timer_ctrl_clrset(0, WDT_EN);
 
 	/* Enable reset on watchdog */
 	reg = readl(RSTOUTn_MASK);
@@ -97,9 +95,7 @@ static int orion_wdt_stop(struct watchdog_device *wdt_dev)
 	writel(reg, RSTOUTn_MASK);
 
 	/* Disable watchdog timer */
-	reg = readl(wdt_reg + TIMER_CTRL);
-	reg &= ~WDT_EN;
-	writel(reg, wdt_reg + TIMER_CTRL);
+	orion_timer_ctrl_clrset(WDT_EN, 0);
 
 	spin_unlock(&wdt_lock);
 	return 0;
-- 
1.8.1.5

[PATCH 03/10] watchdog: orion: Rename device-tree binding documentation

[Ezequiel Garcia]

Name this file to something a bit more judicious.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 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 03/10] watchdog: orion: Rename device-tree binding documentation

[Ezequiel Garcia]

Name this file to something a bit more judicious.

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 03/10] watchdog: orion: Rename device-tree binding documentation

[Ezequiel Garcia]

Name this file to something a bit more judicious.

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 04/10] watchdog: orion: Use the proper watchdog register

[Ezequiel Garcia]

Until now the watchdog driver was using the timer control register
to access the watchdog counter register. This is not appropriate,
given the timer control register should be controlled by the clocksource
driver alone.

Fix this by passing the correct register address to the driver and making
direct use of it. Note that this breaks the current device-tree binding
compatibility since it changes the meaning of the 'reg' property.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/mach-kirkwood/include/mach/bridge-regs.h | 1 +
 arch/arm/mach-orion5x/include/mach/bridge-regs.h  | 1 +
 arch/arm/plat-orion/common.c                      | 2 +-
 drivers/watchdog/orion_wdt.c                      | 7 +++----
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
index d4cbe5e..c3f361d 100644
--- a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
+++ b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
@@ -40,6 +40,7 @@
 
 #define TIMER_VIRT_BASE		(BRIDGE_VIRT_BASE + 0x0300)
 #define TIMER_PHYS_BASE		(BRIDGE_PHYS_BASE + 0x0300)
+#define WDT_PHYS_BASE		(BRIDGE_PHYS_BASE + 0x0324)
 
 #define L2_CONFIG_REG		(BRIDGE_VIRT_BASE + 0x0128)
 #define L2_WRITETHROUGH		0x00000010
diff --git a/arch/arm/mach-orion5x/include/mach/bridge-regs.h b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
index 461fd69..aa35de3 100644
--- a/arch/arm/mach-orion5x/include/mach/bridge-regs.h
+++ b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
@@ -36,4 +36,5 @@
 
 #define TIMER_VIRT_BASE		(ORION5X_BRIDGE_VIRT_BASE + 0x300)
 #define TIMER_PHYS_BASE		(ORION5X_BRIDGE_PHYS_BASE + 0x300)
+#define WDT_PHYS_BASE		(ORION5X_BRIDGE_PHYS_BASE + 0x324)
 #endif
diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
index c019b7a..77fac6c 100644
--- a/arch/arm/plat-orion/common.c
+++ b/arch/arm/plat-orion/common.c
@@ -595,7 +595,7 @@ void __init orion_spi_1_init(unsigned long mapbase)
  * Watchdog
  ****************************************************************************/
 static struct resource orion_wdt_resource =
-		DEFINE_RES_MEM(TIMER_PHYS_BASE, 0x28);
+		DEFINE_RES_MEM(WDT_PHYS_BASE, 0x04);
 
 static struct platform_device orion_wdt_device = {
 	.name		= "orion_wdt",
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index c0597f4..01bcf53 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -32,7 +32,6 @@
  * Watchdog timer block registers.
  */
 #define WDT_EN			0x0010
-#define WDT_VAL			0x0024
 
 #define WDT_MAX_CYCLE_COUNT	0xffffffff
 #define WDT_IN_USE		0
@@ -51,7 +50,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_reg);
 
 	spin_unlock(&wdt_lock);
 	return 0;
@@ -64,7 +63,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_reg);
 
 	/* Clear watchdog timer interrupt */
 	reg = readl(BRIDGE_CAUSE);
@@ -106,7 +105,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_reg) / wdt_tclk;
 	spin_unlock(&wdt_lock);
 
 	return time_left;
-- 
1.8.1.5

[PATCH 04/10] watchdog: orion: Use the proper watchdog register

[Ezequiel Garcia]

Until now the watchdog driver was using the timer control register
to access the watchdog counter register. This is not appropriate,
given the timer control register should be controlled by the clocksource
driver alone.

Fix this by passing the correct register address to the driver and making
direct use of it. Note that this breaks the current device-tree binding
compatibility since it changes the meaning of the 'reg' property.

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

diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
index d4cbe5e..c3f361d 100644
--- a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
+++ b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
@@ -40,6 +40,7 @@
 
 #define TIMER_VIRT_BASE		(BRIDGE_VIRT_BASE + 0x0300)
 #define TIMER_PHYS_BASE		(BRIDGE_PHYS_BASE + 0x0300)
+#define WDT_PHYS_BASE		(BRIDGE_PHYS_BASE + 0x0324)
 
 #define L2_CONFIG_REG		(BRIDGE_VIRT_BASE + 0x0128)
 #define L2_WRITETHROUGH		0x00000010
diff --git a/arch/arm/mach-orion5x/include/mach/bridge-regs.h b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
index 461fd69..aa35de3 100644
--- a/arch/arm/mach-orion5x/include/mach/bridge-regs.h
+++ b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
@@ -36,4 +36,5 @@
 
 #define TIMER_VIRT_BASE		(ORION5X_BRIDGE_VIRT_BASE + 0x300)
 #define TIMER_PHYS_BASE		(ORION5X_BRIDGE_PHYS_BASE + 0x300)
+#define WDT_PHYS_BASE		(ORION5X_BRIDGE_PHYS_BASE + 0x324)
 #endif
diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
index c019b7a..77fac6c 100644
--- a/arch/arm/plat-orion/common.c
+++ b/arch/arm/plat-orion/common.c
@@ -595,7 +595,7 @@ void __init orion_spi_1_init(unsigned long mapbase)
  * Watchdog
  ****************************************************************************/
 static struct resource orion_wdt_resource =
-		DEFINE_RES_MEM(TIMER_PHYS_BASE, 0x28);
+		DEFINE_RES_MEM(WDT_PHYS_BASE, 0x04);
 
 static struct platform_device orion_wdt_device = {
 	.name		= "orion_wdt",
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index c0597f4..01bcf53 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -32,7 +32,6 @@
  * Watchdog timer block registers.
  */
 #define WDT_EN			0x0010
-#define WDT_VAL			0x0024
 
 #define WDT_MAX_CYCLE_COUNT	0xffffffff
 #define WDT_IN_USE		0
@@ -51,7 +50,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_reg);
 
 	spin_unlock(&wdt_lock);
 	return 0;
@@ -64,7 +63,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_reg);
 
 	/* Clear watchdog timer interrupt */
 	reg = readl(BRIDGE_CAUSE);
@@ -106,7 +105,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_reg) / wdt_tclk;
 	spin_unlock(&wdt_lock);
 
 	return time_left;
-- 
1.8.1.5

[PATCH 04/10] watchdog: orion: Use the proper watchdog register

[Ezequiel Garcia]

Until now the watchdog driver was using the timer control register
to access the watchdog counter register. This is not appropriate,
given the timer control register should be controlled by the clocksource
driver alone.

Fix this by passing the correct register address to the driver and making
direct use of it. Note that this breaks the current device-tree binding
compatibility since it changes the meaning of the 'reg' property.

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

diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
index d4cbe5e..c3f361d 100644
--- a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
+++ b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
@@ -40,6 +40,7 @@
 
 #define TIMER_VIRT_BASE		(BRIDGE_VIRT_BASE + 0x0300)
 #define TIMER_PHYS_BASE		(BRIDGE_PHYS_BASE + 0x0300)
+#define WDT_PHYS_BASE		(BRIDGE_PHYS_BASE + 0x0324)
 
 #define L2_CONFIG_REG		(BRIDGE_VIRT_BASE + 0x0128)
 #define L2_WRITETHROUGH		0x00000010
diff --git a/arch/arm/mach-orion5x/include/mach/bridge-regs.h b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
index 461fd69..aa35de3 100644
--- a/arch/arm/mach-orion5x/include/mach/bridge-regs.h
+++ b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
@@ -36,4 +36,5 @@
 
 #define TIMER_VIRT_BASE		(ORION5X_BRIDGE_VIRT_BASE + 0x300)
 #define TIMER_PHYS_BASE		(ORION5X_BRIDGE_PHYS_BASE + 0x300)
+#define WDT_PHYS_BASE		(ORION5X_BRIDGE_PHYS_BASE + 0x324)
 #endif
diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
index c019b7a..77fac6c 100644
--- a/arch/arm/plat-orion/common.c
+++ b/arch/arm/plat-orion/common.c
@@ -595,7 +595,7 @@ void __init orion_spi_1_init(unsigned long mapbase)
  * Watchdog
  ****************************************************************************/
 static struct resource orion_wdt_resource =
-		DEFINE_RES_MEM(TIMER_PHYS_BASE, 0x28);
+		DEFINE_RES_MEM(WDT_PHYS_BASE, 0x04);
 
 static struct platform_device orion_wdt_device = {
 	.name		= "orion_wdt",
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index c0597f4..01bcf53 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -32,7 +32,6 @@
  * Watchdog timer block registers.
  */
 #define WDT_EN			0x0010
-#define WDT_VAL			0x0024
 
 #define WDT_MAX_CYCLE_COUNT	0xffffffff
 #define WDT_IN_USE		0
@@ -51,7 +50,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_reg);
 
 	spin_unlock(&wdt_lock);
 	return 0;
@@ -64,7 +63,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_reg);
 
 	/* Clear watchdog timer interrupt */
 	reg = readl(BRIDGE_CAUSE);
@@ -106,7 +105,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_reg) / wdt_tclk;
 	spin_unlock(&wdt_lock);
 
 	return time_left;
-- 
1.8.1.5

[PATCH 05/10] watchdog: orion: Add a memory resource for RSTOUT register

[Ezequiel Garcia]

Instead of accessing the RSTOUT register directly, this commit
adds a platform memory resource to map this register into the driver.

Note that by adding a required 2nd-cell for the reg property,
this change breaks the device-tree binding compatibility.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/mach-kirkwood/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                      | 18 ++++++++++--------
 4 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
index c3f361d..dead75a 100644
--- a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
+++ b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
@@ -21,6 +21,7 @@
 #define CPU_RESET		0x00000002
 
 #define RSTOUTn_MASK		(BRIDGE_VIRT_BASE + 0x0108)
+#define RSTOUT_PHYS_BASE	(BRIDGE_PHYS_BASE + 0x0108)
 #define WDT_RESET_OUT_EN	0x00000002
 #define SOFT_RESET_OUT_EN	0x00000004
 
diff --git a/arch/arm/mach-orion5x/include/mach/bridge-regs.h b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
index aa35de3..ccd91c9 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 RSTOUT_PHYS_BASE	(ORION5X_BRIDGE_PHYS_BASE + 0x108)
 #define WDT_RESET_OUT_EN	0x0002
 
 #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 77fac6c..37924f8 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(WDT_PHYS_BASE, 0x04);
+static struct resource orion_wdt_resource[] = {
+		DEFINE_RES_MEM(WDT_PHYS_BASE, 0x04),
+		DEFINE_RES_MEM(RSTOUT_PHYS_BASE, 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 01bcf53..3c9b3d2 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -43,6 +43,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 *rstout_reg;
 static DEFINE_SPINLOCK(wdt_lock);
 
 static int orion_wdt_ping(struct watchdog_device *wdt_dev)
@@ -74,9 +75,7 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
 	orion_timer_ctrl_clrset(0, WDT_EN);
 
 	/* Enable reset on watchdog */
-	reg = readl(RSTOUTn_MASK);
-	reg |= WDT_RESET_OUT_EN;
-	writel(reg, RSTOUTn_MASK);
+	writel(readl(rstout_reg) | WDT_RESET_OUT_EN, rstout_reg);
 
 	spin_unlock(&wdt_lock);
 	return 0;
@@ -84,14 +83,10 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
 
 static int orion_wdt_stop(struct watchdog_device *wdt_dev)
 {
-	u32 reg;
-
 	spin_lock(&wdt_lock);
 
 	/* Disable reset on watchdog */
-	reg = readl(RSTOUTn_MASK);
-	reg &= ~WDT_RESET_OUT_EN;
-	writel(reg, RSTOUTn_MASK);
+	writel(readl(rstout_reg) & ~WDT_RESET_OUT_EN, rstout_reg);
 
 	/* Disable watchdog timer */
 	orion_timer_ctrl_clrset(WDT_EN, 0);
@@ -158,6 +153,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;
+	rstout_reg = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+	if (!rstout_reg)
+		return -ENOMEM;
+
 	wdt_max_duration = WDT_MAX_CYCLE_COUNT / wdt_tclk;
 
 	orion_wdt.timeout = wdt_max_duration;
-- 
1.8.1.5

[PATCH 05/10] watchdog: orion: Add a memory resource for RSTOUT register

[Ezequiel Garcia]

Instead of accessing the RSTOUT register directly, this commit
adds a platform memory resource to map this register into the driver.

Note that by adding a required 2nd-cell for the reg property,
this change breaks the device-tree binding compatibility.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-kirkwood/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                      | 18 ++++++++++--------
 4 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
index c3f361d..dead75a 100644
--- a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
+++ b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
@@ -21,6 +21,7 @@
 #define CPU_RESET		0x00000002
 
 #define RSTOUTn_MASK		(BRIDGE_VIRT_BASE + 0x0108)
+#define RSTOUT_PHYS_BASE	(BRIDGE_PHYS_BASE + 0x0108)
 #define WDT_RESET_OUT_EN	0x00000002
 #define SOFT_RESET_OUT_EN	0x00000004
 
diff --git a/arch/arm/mach-orion5x/include/mach/bridge-regs.h b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
index aa35de3..ccd91c9 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 RSTOUT_PHYS_BASE	(ORION5X_BRIDGE_PHYS_BASE + 0x108)
 #define WDT_RESET_OUT_EN	0x0002
 
 #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 77fac6c..37924f8 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(WDT_PHYS_BASE, 0x04);
+static struct resource orion_wdt_resource[] = {
+		DEFINE_RES_MEM(WDT_PHYS_BASE, 0x04),
+		DEFINE_RES_MEM(RSTOUT_PHYS_BASE, 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 01bcf53..3c9b3d2 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -43,6 +43,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 *rstout_reg;
 static DEFINE_SPINLOCK(wdt_lock);
 
 static int orion_wdt_ping(struct watchdog_device *wdt_dev)
@@ -74,9 +75,7 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
 	orion_timer_ctrl_clrset(0, WDT_EN);
 
 	/* Enable reset on watchdog */
-	reg = readl(RSTOUTn_MASK);
-	reg |= WDT_RESET_OUT_EN;
-	writel(reg, RSTOUTn_MASK);
+	writel(readl(rstout_reg) | WDT_RESET_OUT_EN, rstout_reg);
 
 	spin_unlock(&wdt_lock);
 	return 0;
@@ -84,14 +83,10 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
 
 static int orion_wdt_stop(struct watchdog_device *wdt_dev)
 {
-	u32 reg;
-
 	spin_lock(&wdt_lock);
 
 	/* Disable reset on watchdog */
-	reg = readl(RSTOUTn_MASK);
-	reg &= ~WDT_RESET_OUT_EN;
-	writel(reg, RSTOUTn_MASK);
+	writel(readl(rstout_reg) & ~WDT_RESET_OUT_EN, rstout_reg);
 
 	/* Disable watchdog timer */
 	orion_timer_ctrl_clrset(WDT_EN, 0);
@@ -158,6 +153,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;
+	rstout_reg = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+	if (!rstout_reg)
+		return -ENOMEM;
+
 	wdt_max_duration = WDT_MAX_CYCLE_COUNT / wdt_tclk;
 
 	orion_wdt.timeout = wdt_max_duration;
-- 
1.8.1.5

[PATCH 05/10] watchdog: orion: Add a memory resource for RSTOUT register

[Ezequiel Garcia]

Instead of accessing the RSTOUT register directly, this commit
adds a platform memory resource to map this register into the driver.

Note that by adding a required 2nd-cell for the reg property,
this change breaks the device-tree binding compatibility.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-kirkwood/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                      | 18 ++++++++++--------
 4 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
index c3f361d..dead75a 100644
--- a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
+++ b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
@@ -21,6 +21,7 @@
 #define CPU_RESET		0x00000002
 
 #define RSTOUTn_MASK		(BRIDGE_VIRT_BASE + 0x0108)
+#define RSTOUT_PHYS_BASE	(BRIDGE_PHYS_BASE + 0x0108)
 #define WDT_RESET_OUT_EN	0x00000002
 #define SOFT_RESET_OUT_EN	0x00000004
 
diff --git a/arch/arm/mach-orion5x/include/mach/bridge-regs.h b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
index aa35de3..ccd91c9 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 RSTOUT_PHYS_BASE	(ORION5X_BRIDGE_PHYS_BASE + 0x108)
 #define WDT_RESET_OUT_EN	0x0002
 
 #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 77fac6c..37924f8 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(WDT_PHYS_BASE, 0x04);
+static struct resource orion_wdt_resource[] = {
+		DEFINE_RES_MEM(WDT_PHYS_BASE, 0x04),
+		DEFINE_RES_MEM(RSTOUT_PHYS_BASE, 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 01bcf53..3c9b3d2 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -43,6 +43,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 *rstout_reg;
 static DEFINE_SPINLOCK(wdt_lock);
 
 static int orion_wdt_ping(struct watchdog_device *wdt_dev)
@@ -74,9 +75,7 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
 	orion_timer_ctrl_clrset(0, WDT_EN);
 
 	/* Enable reset on watchdog */
-	reg = readl(RSTOUTn_MASK);
-	reg |= WDT_RESET_OUT_EN;
-	writel(reg, RSTOUTn_MASK);
+	writel(readl(rstout_reg) | WDT_RESET_OUT_EN, rstout_reg);
 
 	spin_unlock(&wdt_lock);
 	return 0;
@@ -84,14 +83,10 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
 
 static int orion_wdt_stop(struct watchdog_device *wdt_dev)
 {
-	u32 reg;
-
 	spin_lock(&wdt_lock);
 
 	/* Disable reset on watchdog */
-	reg = readl(RSTOUTn_MASK);
-	reg &= ~WDT_RESET_OUT_EN;
-	writel(reg, RSTOUTn_MASK);
+	writel(readl(rstout_reg) & ~WDT_RESET_OUT_EN, rstout_reg);
 
 	/* Disable watchdog timer */
 	orion_timer_ctrl_clrset(WDT_EN, 0);
@@ -158,6 +153,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;
+	rstout_reg = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+	if (!rstout_reg)
+		return -ENOMEM;
+
 	wdt_max_duration = WDT_MAX_CYCLE_COUNT / wdt_tclk;
 
 	orion_wdt.timeout = wdt_max_duration;
-- 
1.8.1.5

[PATCH 06/10] watchdog: orion: Update device-tree binding documentation

[Ezequiel Garcia]

Now that the 'reg' property meaning has been changed,
this commit updates the deivce-tree binding documentation.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 Documentation/devicetree/bindings/watchdog/orion-wdt.txt | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
index 5dc8d30..48a71fc 100644
--- a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
@@ -3,7 +3,8 @@
 Required Properties:
 
 - Compatibility : "marvell,orion-wdt"
-- reg		: Address of the timer registers
+- reg		: First cell contains the register for the watchdog counter.
+		  Second cell contains the register for the RSTOUT mask.
 
 Optional properties:
 
@@ -11,9 +12,10 @@ Optional properties:
 
 Example:
 
-	wdt@20300 {
+	wdt@20324 {
 		compatible = "marvell,orion-wdt";
-		reg = <0x20300 0x28>;
+		reg = <0x20324 0x4
+		       0x20108 0x4>;
 		timeout-sec = <10>;
 		status = "okay";
 	};
-- 
1.8.1.5

[PATCH 06/10] watchdog: orion: Update device-tree binding documentation

[Ezequiel Garcia]

Now that the 'reg' property meaning has been changed,
this commit updates the deivce-tree binding documentation.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 Documentation/devicetree/bindings/watchdog/orion-wdt.txt | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
index 5dc8d30..48a71fc 100644
--- a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
@@ -3,7 +3,8 @@
 Required Properties:
 
 - Compatibility : "marvell,orion-wdt"
-- reg		: Address of the timer registers
+- reg		: First cell contains the register for the watchdog counter.
+		  Second cell contains the register for the RSTOUT mask.
 
 Optional properties:
 
@@ -11,9 +12,10 @@ Optional properties:
 
 Example:
 
-	wdt@20300 {
+	wdt@20324 {
 		compatible = "marvell,orion-wdt";
-		reg = <0x20300 0x28>;
+		reg = <0x20324 0x4
+		       0x20108 0x4>;
 		timeout-sec = <10>;
 		status = "okay";
 	};
-- 
1.8.1.5

[PATCH 06/10] watchdog: orion: Update device-tree binding documentation

[Ezequiel Garcia]

Now that the 'reg' property meaning has been changed,
this commit updates the deivce-tree binding documentation.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 Documentation/devicetree/bindings/watchdog/orion-wdt.txt | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
index 5dc8d30..48a71fc 100644
--- a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
@@ -3,7 +3,8 @@
 Required Properties:
 
 - Compatibility : "marvell,orion-wdt"
-- reg		: Address of the timer registers
+- reg		: First cell contains the register for the watchdog counter.
+		  Second cell contains the register for the RSTOUT mask.
 
 Optional properties:
 
@@ -11,9 +12,10 @@ Optional properties:
 
 Example:
 
-	wdt at 20300 {
+	wdt at 20324 {
 		compatible = "marvell,orion-wdt";
-		reg = <0x20300 0x28>;
+		reg = <0x20324 0x4
+		       0x20108 0x4>;
 		timeout-sec = <10>;
 		status = "okay";
 	};
-- 
1.8.1.5

[PATCH 07/10] 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-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/watchdog/orion_wdt.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 3c9b3d2..4a0ab47 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -59,18 +59,11 @@ static int orion_wdt_ping(struct watchdog_device *wdt_dev)
 
 static int orion_wdt_start(struct watchdog_device *wdt_dev)
 {
-	u32 reg;
-
 	spin_lock(&wdt_lock);
 
 	/* Set watchdog duration */
 	writel(wdt_tclk * wdt_dev->timeout, wdt_reg);
 
-	/* Clear watchdog timer interrupt */
-	reg = readl(BRIDGE_CAUSE);
-	reg &= ~WDT_INT_REQ;
-	writel(reg, BRIDGE_CAUSE);
-
 	/* Enable watchdog timer */
 	orion_timer_ctrl_clrset(0, WDT_EN);
 
-- 
1.8.1.5

[PATCH 07/10] 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 | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 3c9b3d2..4a0ab47 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -59,18 +59,11 @@ static int orion_wdt_ping(struct watchdog_device *wdt_dev)
 
 static int orion_wdt_start(struct watchdog_device *wdt_dev)
 {
-	u32 reg;
-
 	spin_lock(&wdt_lock);
 
 	/* Set watchdog duration */
 	writel(wdt_tclk * wdt_dev->timeout, wdt_reg);
 
-	/* Clear watchdog timer interrupt */
-	reg = readl(BRIDGE_CAUSE);
-	reg &= ~WDT_INT_REQ;
-	writel(reg, BRIDGE_CAUSE);
-
 	/* Enable watchdog timer */
 	orion_timer_ctrl_clrset(0, WDT_EN);
 
-- 
1.8.1.5

[PATCH 07/10] 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 | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 3c9b3d2..4a0ab47 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -59,18 +59,11 @@ static int orion_wdt_ping(struct watchdog_device *wdt_dev)
 
 static int orion_wdt_start(struct watchdog_device *wdt_dev)
 {
-	u32 reg;
-
 	spin_lock(&wdt_lock);
 
 	/* Set watchdog duration */
 	writel(wdt_tclk * wdt_dev->timeout, wdt_reg);
 
-	/* Clear watchdog timer interrupt */
-	reg = readl(BRIDGE_CAUSE);
-	reg &= ~WDT_INT_REQ;
-	writel(reg, BRIDGE_CAUSE);
-
 	/* Enable watchdog timer */
 	orion_timer_ctrl_clrset(0, WDT_EN);
 
-- 
1.8.1.5

[PATCH 08/10] watchdog: orion: Remove mach-specific unneeded header

[Ezequiel Garcia]

The mach/bridge-regs.h header is not needed anymore, so we can remove it.
This commit allows to use this driver on multiplatforms builds.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/watchdog/orion_wdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 4a0ab47..4ed7a74 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -26,12 +26,12 @@
 #include <linux/err.h>
 #include <linux/of.h>
 #include <linux/time-orion.h>
-#include <mach/bridge-regs.h>
 
 /*
  * Watchdog timer block registers.
  */
 #define WDT_EN			0x0010
+#define WDT_RESET_OUT_EN	0x0002
 
 #define WDT_MAX_CYCLE_COUNT	0xffffffff
 #define WDT_IN_USE		0
-- 
1.8.1.5

[PATCH 08/10] watchdog: orion: Remove mach-specific unneeded header

[Ezequiel Garcia]

The mach/bridge-regs.h header is not needed anymore, so we can remove it.
This commit allows to use this driver on multiplatforms builds.

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

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 4a0ab47..4ed7a74 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -26,12 +26,12 @@
 #include <linux/err.h>
 #include <linux/of.h>
 #include <linux/time-orion.h>
-#include <mach/bridge-regs.h>
 
 /*
  * Watchdog timer block registers.
  */
 #define WDT_EN			0x0010
+#define WDT_RESET_OUT_EN	0x0002
 
 #define WDT_MAX_CYCLE_COUNT	0xffffffff
 #define WDT_IN_USE		0
-- 
1.8.1.5

[PATCH 08/10] watchdog: orion: Remove mach-specific unneeded header

[Ezequiel Garcia]

The mach/bridge-regs.h header is not needed anymore, so we can remove it.
This commit allows to use this driver on multiplatforms builds.

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

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 4a0ab47..4ed7a74 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -26,12 +26,12 @@
 #include <linux/err.h>
 #include <linux/of.h>
 #include <linux/time-orion.h>
-#include <mach/bridge-regs.h>
 
 /*
  * Watchdog timer block registers.
  */
 #define WDT_EN			0x0010
+#define WDT_RESET_OUT_EN	0x0002
 
 #define WDT_MAX_CYCLE_COUNT	0xffffffff
 #define WDT_IN_USE		0
-- 
1.8.1.5

[PATCH 09/10] watchdog: orion: Use BIT()

[Ezequiel Garcia]

This is a purely cosmetic commit: we replace hardcoded values that
representing bits by BIT(), which is slightly more readable.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/watchdog/orion_wdt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 4ed7a74..ae64cec 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -30,8 +30,8 @@
 /*
  * Watchdog timer block registers.
  */
-#define WDT_EN			0x0010
-#define WDT_RESET_OUT_EN	0x0002
+#define WDT_EN			BIT(4)
+#define WDT_RESET_OUT_EN	BIT(1)
 
 #define WDT_MAX_CYCLE_COUNT	0xffffffff
 #define WDT_IN_USE		0
-- 
1.8.1.5

[PATCH 09/10] watchdog: orion: Use BIT()

[Ezequiel Garcia]

This is a purely cosmetic commit: we replace hardcoded values that
representing bits by BIT(), which is slightly more readable.

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

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 4ed7a74..ae64cec 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -30,8 +30,8 @@
 /*
  * Watchdog timer block registers.
  */
-#define WDT_EN			0x0010
-#define WDT_RESET_OUT_EN	0x0002
+#define WDT_EN			BIT(4)
+#define WDT_RESET_OUT_EN	BIT(1)
 
 #define WDT_MAX_CYCLE_COUNT	0xffffffff
 #define WDT_IN_USE		0
-- 
1.8.1.5

[PATCH 09/10] watchdog: orion: Use BIT()

[Ezequiel Garcia]

This is a purely cosmetic commit: we replace hardcoded values that
representing bits by BIT(), which is slightly more readable.

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

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 4ed7a74..ae64cec 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -30,8 +30,8 @@
 /*
  * Watchdog timer block registers.
  */
-#define WDT_EN			0x0010
-#define WDT_RESET_OUT_EN	0x0002
+#define WDT_EN			BIT(4)
+#define WDT_RESET_OUT_EN	BIT(1)
 
 #define WDT_MAX_CYCLE_COUNT	0xffffffff
 #define WDT_IN_USE		0
-- 
1.8.1.5

[PATCH 10/10] ARM: kirkwood: Fix the device-tree watchdog's node reg property

[Ezequiel Garcia]

The watchdog driver now needs two 'reg' property cells. The first one
is for the register containing the watchdog counter, while the second
one is for the RSTOUT register.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 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 ca296c3..0d74ee9 100644
--- a/arch/arm/boot/dts/kirkwood.dtsi
+++ b/arch/arm/boot/dts/kirkwood.dtsi
@@ -116,7 +116,8 @@
 
 		wdt: watchdog-timer@20300 {
 			compatible = "marvell,orion-wdt";
-			reg = <0x20300 0x28>;
+			reg = <0x20324 0x04
+			       0x20108 0x04>;
 			interrupt-parent = <&bridge_intc>;
 			interrupts = <3>;
 			clocks = <&gate_clk 7>;
-- 
1.8.1.5

[PATCH 10/10] ARM: kirkwood: Fix the device-tree watchdog's node reg property

[Ezequiel Garcia]

The watchdog driver now needs two 'reg' property cells. The first one
is for the register containing the watchdog counter, while the second
one is for the RSTOUT register.

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 ca296c3..0d74ee9 100644
--- a/arch/arm/boot/dts/kirkwood.dtsi
+++ b/arch/arm/boot/dts/kirkwood.dtsi
@@ -116,7 +116,8 @@
 
 		wdt: watchdog-timer@20300 {
 			compatible = "marvell,orion-wdt";
-			reg = <0x20300 0x28>;
+			reg = <0x20324 0x04
+			       0x20108 0x04>;
 			interrupt-parent = <&bridge_intc>;
 			interrupts = <3>;
 			clocks = <&gate_clk 7>;
-- 
1.8.1.5

[PATCH 10/10] ARM: kirkwood: Fix the device-tree watchdog's node reg property

[Ezequiel Garcia]

The watchdog driver now needs two 'reg' property cells. The first one
is for the register containing the watchdog counter, while the second
one is for the RSTOUT register.

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 ca296c3..0d74ee9 100644
--- a/arch/arm/boot/dts/kirkwood.dtsi
+++ b/arch/arm/boot/dts/kirkwood.dtsi
@@ -116,7 +116,8 @@
 
 		wdt: watchdog-timer at 20300 {
 			compatible = "marvell,orion-wdt";
-			reg = <0x20300 0x28>;
+			reg = <0x20324 0x04
+			       0x20108 0x04>;
 			interrupt-parent = <&bridge_intc>;
 			interrupts = <3>;
 			clocks = <&gate_clk 7>;
-- 
1.8.1.5

Re: [PATCH 00/10] Orion Watchdog fixes

[Andrew Lunn]

On Mon, Jul 15, 2013 at 08:32:33PM -0300, Ezequiel Garcia wrote:
> This patchset introduces a bunch of fixes that removes the direct use of
> the shared timer control register, and also removes the need to include
> a mach-specific header.
> 
> With this changes the driver can be included in multiplatforms builds,
> and in particular be used for the Armada 370/XP SoC.

Hi Ezequiel

Maybe i'm missing something here. You are making use of
orion_timer_ctrl_clrset() from time-orion.c. How will this work on
370/XP which has a different clocksource driver?

Thanks
	Andrew

Re: [PATCH 00/10] Orion Watchdog fixes

[Andrew Lunn]

On Mon, Jul 15, 2013 at 08:32:33PM -0300, Ezequiel Garcia wrote:
> This patchset introduces a bunch of fixes that removes the direct use of
> the shared timer control register, and also removes the need to include
> a mach-specific header.
> 
> With this changes the driver can be included in multiplatforms builds,
> and in particular be used for the Armada 370/XP SoC.

Hi Ezequiel

Maybe i'm missing something here. You are making use of
orion_timer_ctrl_clrset() from time-orion.c. How will this work on
370/XP which has a different clocksource driver?

Thanks
	Andrew

[PATCH 00/10] Orion Watchdog fixes

[Andrew Lunn]

On Mon, Jul 15, 2013 at 08:32:33PM -0300, Ezequiel Garcia wrote:
> This patchset introduces a bunch of fixes that removes the direct use of
> the shared timer control register, and also removes the need to include
> a mach-specific header.
> 
> With this changes the driver can be included in multiplatforms builds,
> and in particular be used for the Armada 370/XP SoC.

Hi Ezequiel

Maybe i'm missing something here. You are making use of
orion_timer_ctrl_clrset() from time-orion.c. How will this work on
370/XP which has a different clocksource driver?

Thanks
	Andrew

Re: [PATCH 00/10] Orion Watchdog fixes

[Thomas Petazzoni]

Dear Andrew Lunn,

On Tue, 16 Jul 2013 08:59:52 +0200, Andrew Lunn wrote:

> Maybe i'm missing something here. You are making use of
> orion_timer_ctrl_clrset() from time-orion.c. How will this work on
> 370/XP which has a different clocksource driver?

I *think* the idea is that the Armada 370/XP driver will expose the
same function, so from the point of view of the watchdog driver, it
will just work. This set of patches is just some preparation patches on
the Orion watchdog driver, to later make it usable on Armada 370/XP.
The patches enabling its usage on Armada 370/XP will follow.

But Ezequiel will confirm all that, we had a discussion together about
this yesterday, so I guess what I said should be the plan, but it's
better if Ezequiel confirms, obviously :)

Thanks!

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

Re: [PATCH 00/10] Orion Watchdog fixes

[Thomas Petazzoni]

Dear Andrew Lunn,

On Tue, 16 Jul 2013 08:59:52 +0200, Andrew Lunn wrote:

> Maybe i'm missing something here. You are making use of
> orion_timer_ctrl_clrset() from time-orion.c. How will this work on
> 370/XP which has a different clocksource driver?

I *think* the idea is that the Armada 370/XP driver will expose the
same function, so from the point of view of the watchdog driver, it
will just work. This set of patches is just some preparation patches on
the Orion watchdog driver, to later make it usable on Armada 370/XP.
The patches enabling its usage on Armada 370/XP will follow.

But Ezequiel will confirm all that, we had a discussion together about
this yesterday, so I guess what I said should be the plan, but it's
better if Ezequiel confirms, obviously :)

Thanks!

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

[PATCH 00/10] Orion Watchdog fixes

[Thomas Petazzoni]

Dear Andrew Lunn,

On Tue, 16 Jul 2013 08:59:52 +0200, Andrew Lunn wrote:

> Maybe i'm missing something here. You are making use of
> orion_timer_ctrl_clrset() from time-orion.c. How will this work on
> 370/XP which has a different clocksource driver?

I *think* the idea is that the Armada 370/XP driver will expose the
same function, so from the point of view of the watchdog driver, it
will just work. This set of patches is just some preparation patches on
the Orion watchdog driver, to later make it usable on Armada 370/XP.
The patches enabling its usage on Armada 370/XP will follow.

But Ezequiel will confirm all that, we had a discussion together about
this yesterday, so I guess what I said should be the plan, but it's
better if Ezequiel confirms, obviously :)

Thanks!

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

Re: [PATCH 00/10] Orion Watchdog fixes

[Andrew Lunn]

On Tue, Jul 16, 2013 at 09:20:59AM +0200, Thomas Petazzoni wrote:
> Dear Andrew Lunn,
> 
> On Tue, 16 Jul 2013 08:59:52 +0200, Andrew Lunn wrote:
> 
> > Maybe i'm missing something here. You are making use of
> > orion_timer_ctrl_clrset() from time-orion.c. How will this work on
> > 370/XP which has a different clocksource driver?
> 
> I *think* the idea is that the Armada 370/XP driver will expose the
> same function, so from the point of view of the watchdog driver, it
> will just work.

Hi Thomas

That was what i was thinking would happen. And then i started to
wonder how well the kernel linker deals with multiple definitions of
the same symbol. Dove and 370/XP can end up in the same kernel. So we
need to have both orion-timer and the 370/XP timer in the same kernel,
so we end up with the same symbol in the kernel twice...

Lets wait for Ezequiel to answer.

   Andrew

Re: [PATCH 00/10] Orion Watchdog fixes

[Andrew Lunn]

On Tue, Jul 16, 2013 at 09:20:59AM +0200, Thomas Petazzoni wrote:
> Dear Andrew Lunn,
> 
> On Tue, 16 Jul 2013 08:59:52 +0200, Andrew Lunn wrote:
> 
> > Maybe i'm missing something here. You are making use of
> > orion_timer_ctrl_clrset() from time-orion.c. How will this work on
> > 370/XP which has a different clocksource driver?
> 
> I *think* the idea is that the Armada 370/XP driver will expose the
> same function, so from the point of view of the watchdog driver, it
> will just work.

Hi Thomas

That was what i was thinking would happen. And then i started to
wonder how well the kernel linker deals with multiple definitions of
the same symbol. Dove and 370/XP can end up in the same kernel. So we
need to have both orion-timer and the 370/XP timer in the same kernel,
so we end up with the same symbol in the kernel twice...

Lets wait for Ezequiel to answer.

   Andrew

[PATCH 00/10] Orion Watchdog fixes

[Andrew Lunn]

On Tue, Jul 16, 2013 at 09:20:59AM +0200, Thomas Petazzoni wrote:
> Dear Andrew Lunn,
> 
> On Tue, 16 Jul 2013 08:59:52 +0200, Andrew Lunn wrote:
> 
> > Maybe i'm missing something here. You are making use of
> > orion_timer_ctrl_clrset() from time-orion.c. How will this work on
> > 370/XP which has a different clocksource driver?
> 
> I *think* the idea is that the Armada 370/XP driver will expose the
> same function, so from the point of view of the watchdog driver, it
> will just work.

Hi Thomas

That was what i was thinking would happen. And then i started to
wonder how well the kernel linker deals with multiple definitions of
the same symbol. Dove and 370/XP can end up in the same kernel. So we
need to have both orion-timer and the 370/XP timer in the same kernel,
so we end up with the same symbol in the kernel twice...

Lets wait for Ezequiel to answer.

   Andrew

Re: [PATCH 00/10] Orion Watchdog fixes

[Sebastian Hesselbarth]

Andrew, Thomas,

In the discussion about orion clocksource Russell was proposing a generic 
thread-safe write. That puts a single lock around all those writes. Of 
course, it will also blocked by totally unrelated thread-safe register 
accesses but should prevent us from having dozens of locks and solves the 
symbol issue.

Sebastian



On July 16, 2013 9:31:01 AM Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:
> On Tue, Jul 16, 2013 at 09:20:59AM +0200, Thomas Petazzoni wrote:
> > Dear Andrew Lunn,
> > On Tue, 16 Jul 2013 08:59:52 +0200, Andrew Lunn wrote:
> > > Maybe i'm missing something here. You are making use of
> > > orion_timer_ctrl_clrset() from time-orion.c. How will this work on
> > > 370/XP which has a different clocksource driver?
> > I *think* the idea is that the Armada 370/XP driver will expose the
> > same function, so from the point of view of the watchdog driver, it
> > will just work.
>
> Hi Thomas
>
> That was what i was thinking would happen. And then i started to
> wonder how well the kernel linker deals with multiple definitions of
> the same symbol. Dove and 370/XP can end up in the same kernel. So we
> need to have both orion-timer and the 370/XP timer in the same kernel,
> so we end up with the same symbol in the kernel twice...
>
> Lets wait for Ezequiel to answer.
>
>    Andrew

Re: [PATCH 00/10] Orion Watchdog fixes

[Sebastian Hesselbarth]

Andrew, Thomas,

In the discussion about orion clocksource Russell was proposing a generic 
thread-safe write. That puts a single lock around all those writes. Of 
course, it will also blocked by totally unrelated thread-safe register 
accesses but should prevent us from having dozens of locks and solves the 
symbol issue.

Sebastian



On July 16, 2013 9:31:01 AM Andrew Lunn <andrew@lunn.ch> wrote:
> On Tue, Jul 16, 2013 at 09:20:59AM +0200, Thomas Petazzoni wrote:
> > Dear Andrew Lunn,
> > On Tue, 16 Jul 2013 08:59:52 +0200, Andrew Lunn wrote:
> > > Maybe i'm missing something here. You are making use of
> > > orion_timer_ctrl_clrset() from time-orion.c. How will this work on
> > > 370/XP which has a different clocksource driver?
> > I *think* the idea is that the Armada 370/XP driver will expose the
> > same function, so from the point of view of the watchdog driver, it
> > will just work.
>
> Hi Thomas
>
> That was what i was thinking would happen. And then i started to
> wonder how well the kernel linker deals with multiple definitions of
> the same symbol. Dove and 370/XP can end up in the same kernel. So we
> need to have both orion-timer and the 370/XP timer in the same kernel,
> so we end up with the same symbol in the kernel twice...
>
> Lets wait for Ezequiel to answer.
>
>    Andrew


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

[PATCH 00/10] Orion Watchdog fixes

[Sebastian Hesselbarth]

Andrew, Thomas,

In the discussion about orion clocksource Russell was proposing a generic 
thread-safe write. That puts a single lock around all those writes. Of 
course, it will also blocked by totally unrelated thread-safe register 
accesses but should prevent us from having dozens of locks and solves the 
symbol issue.

Sebastian



On July 16, 2013 9:31:01 AM Andrew Lunn <andrew@lunn.ch> wrote:
> On Tue, Jul 16, 2013 at 09:20:59AM +0200, Thomas Petazzoni wrote:
> > Dear Andrew Lunn,
> > On Tue, 16 Jul 2013 08:59:52 +0200, Andrew Lunn wrote:
> > > Maybe i'm missing something here. You are making use of
> > > orion_timer_ctrl_clrset() from time-orion.c. How will this work on
> > > 370/XP which has a different clocksource driver?
> > I *think* the idea is that the Armada 370/XP driver will expose the
> > same function, so from the point of view of the watchdog driver, it
> > will just work.
>
> Hi Thomas
>
> That was what i was thinking would happen. And then i started to
> wonder how well the kernel linker deals with multiple definitions of
> the same symbol. Dove and 370/XP can end up in the same kernel. So we
> need to have both orion-timer and the 370/XP timer in the same kernel,
> so we end up with the same symbol in the kernel twice...
>
> Lets wait for Ezequiel to answer.
>
>    Andrew

Re: [PATCH 00/10] Orion Watchdog fixes

[Ezequiel Garcia]

Hi Thomas, Andrew:

Thanks for looking at this!

On Tue, Jul 16, 2013 at 09:31:01AM +0200, Andrew Lunn wrote:
> On Tue, Jul 16, 2013 at 09:20:59AM +0200, Thomas Petazzoni wrote:
> > 
> > On Tue, 16 Jul 2013 08:59:52 +0200, Andrew Lunn wrote:
> > 
> > > Maybe i'm missing something here. You are making use of
> > > orion_timer_ctrl_clrset() from time-orion.c. How will this work on
> > > 370/XP which has a different clocksource driver?
> > 
> > I *think* the idea is that the Armada 370/XP driver will expose the
> > same function, so from the point of view of the watchdog driver, it
> > will just work.

Indeed that was one of the ideas. As Thomas said, this was just
preparation work.

> 
> That was what i was thinking would happen. And then i started to
> wonder how well the kernel linker deals with multiple definitions of
> the same symbol. Dove and 370/XP can end up in the same kernel. So we
> need to have both orion-timer and the 370/XP timer in the same kernel,
> so we end up with the same symbol in the kernel twice...
> 

Yeah, well... I wasn't sure about using the same name, so another approach
would be adding a new compatible to the driver and then make it use the
appropriate function in the 370/XP clocksource driver (with a different name).

And, yet another approach, is what Sebastian just said, although I'm
not sure I understood it :). In any case, we have already several solutions,
which is why I'm not too worried about this particular issue.

On the other side, I'm much interested in knowing if you are OK with
breaking the watchdog DT compatibility. If you NACK this, then I'll
start preparing a different watchdog driver for 370/XP, since I don't
want to extend a driver that is a bit dirty.

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 00/10] Orion Watchdog fixes

[Ezequiel Garcia]

Hi Thomas, Andrew:

Thanks for looking at this!

On Tue, Jul 16, 2013 at 09:31:01AM +0200, Andrew Lunn wrote:
> On Tue, Jul 16, 2013 at 09:20:59AM +0200, Thomas Petazzoni wrote:
> > 
> > On Tue, 16 Jul 2013 08:59:52 +0200, Andrew Lunn wrote:
> > 
> > > Maybe i'm missing something here. You are making use of
> > > orion_timer_ctrl_clrset() from time-orion.c. How will this work on
> > > 370/XP which has a different clocksource driver?
> > 
> > I *think* the idea is that the Armada 370/XP driver will expose the
> > same function, so from the point of view of the watchdog driver, it
> > will just work.

Indeed that was one of the ideas. As Thomas said, this was just
preparation work.

> 
> That was what i was thinking would happen. And then i started to
> wonder how well the kernel linker deals with multiple definitions of
> the same symbol. Dove and 370/XP can end up in the same kernel. So we
> need to have both orion-timer and the 370/XP timer in the same kernel,
> so we end up with the same symbol in the kernel twice...
> 

Yeah, well... I wasn't sure about using the same name, so another approach
would be adding a new compatible to the driver and then make it use the
appropriate function in the 370/XP clocksource driver (with a different name).

And, yet another approach, is what Sebastian just said, although I'm
not sure I understood it :). In any case, we have already several solutions,
which is why I'm not too worried about this particular issue.

On the other side, I'm much interested in knowing if you are OK with
breaking the watchdog DT compatibility. If you NACK this, then I'll
start preparing a different watchdog driver for 370/XP, since I don't
want to extend a driver that is a bit dirty.

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 00/10] Orion Watchdog fixes

[Ezequiel Garcia]

Hi Thomas, Andrew:

Thanks for looking at this!

On Tue, Jul 16, 2013 at 09:31:01AM +0200, Andrew Lunn wrote:
> On Tue, Jul 16, 2013 at 09:20:59AM +0200, Thomas Petazzoni wrote:
> > 
> > On Tue, 16 Jul 2013 08:59:52 +0200, Andrew Lunn wrote:
> > 
> > > Maybe i'm missing something here. You are making use of
> > > orion_timer_ctrl_clrset() from time-orion.c. How will this work on
> > > 370/XP which has a different clocksource driver?
> > 
> > I *think* the idea is that the Armada 370/XP driver will expose the
> > same function, so from the point of view of the watchdog driver, it
> > will just work.

Indeed that was one of the ideas. As Thomas said, this was just
preparation work.

> 
> That was what i was thinking would happen. And then i started to
> wonder how well the kernel linker deals with multiple definitions of
> the same symbol. Dove and 370/XP can end up in the same kernel. So we
> need to have both orion-timer and the 370/XP timer in the same kernel,
> so we end up with the same symbol in the kernel twice...
> 

Yeah, well... I wasn't sure about using the same name, so another approach
would be adding a new compatible to the driver and then make it use the
appropriate function in the 370/XP clocksource driver (with a different name).

And, yet another approach, is what Sebastian just said, although I'm
not sure I understood it :). In any case, we have already several solutions,
which is why I'm not too worried about this particular issue.

On the other side, I'm much interested in knowing if you are OK with
breaking the watchdog DT compatibility. If you NACK this, then I'll
start preparing a different watchdog driver for 370/XP, since I don't
want to extend a driver that is a bit dirty.

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

Re: [PATCH 00/10] Orion Watchdog fixes

[Ezequiel Garcia]

Hi Sebastian,

On Tue, Jul 16, 2013 at 09:48:56AM +0200, Sebastian Hesselbarth wrote:
> 
> In the discussion about orion clocksource Russell was proposing a generic 
> thread-safe write. That puts a single lock around all those writes. Of 
> course, it will also blocked by totally unrelated thread-safe register 
> accesses but should prevent us from having dozens of locks and solves the 
> symbol issue.
> 

Thanks for the insight! I'll try to dig this suggestion in the clocksource
discussion. Do you have any plans for posting the clocksource soon?
This way I can base this series on something that's in Jason's trees...

Thanks!
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 00/10] Orion Watchdog fixes

[Ezequiel Garcia]

Hi Sebastian,

On Tue, Jul 16, 2013 at 09:48:56AM +0200, Sebastian Hesselbarth wrote:
> 
> In the discussion about orion clocksource Russell was proposing a generic 
> thread-safe write. That puts a single lock around all those writes. Of 
> course, it will also blocked by totally unrelated thread-safe register 
> accesses but should prevent us from having dozens of locks and solves the 
> symbol issue.
> 

Thanks for the insight! I'll try to dig this suggestion in the clocksource
discussion. Do you have any plans for posting the clocksource soon?
This way I can base this series on something that's in Jason's trees...

Thanks!
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 00/10] Orion Watchdog fixes

[Ezequiel Garcia]

Hi Sebastian,

On Tue, Jul 16, 2013 at 09:48:56AM +0200, Sebastian Hesselbarth wrote:
> 
> In the discussion about orion clocksource Russell was proposing a generic 
> thread-safe write. That puts a single lock around all those writes. Of 
> course, it will also blocked by totally unrelated thread-safe register 
> accesses but should prevent us from having dozens of locks and solves the 
> symbol issue.
> 

Thanks for the insight! I'll try to dig this suggestion in the clocksource
discussion. Do you have any plans for posting the clocksource soon?
This way I can base this series on something that's in Jason's trees...

Thanks!
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH 06/10] watchdog: orion: Update device-tree binding documentation

[Jason Cooper]

On Mon, Jul 15, 2013 at 08:32:39PM -0300, Ezequiel Garcia wrote:
> Now that the 'reg' property meaning has been changed,
> this commit updates the deivce-tree binding documentation.

nit. s/deivce/device/

> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/watchdog/orion-wdt.txt | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> index 5dc8d30..48a71fc 100644
> --- a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> +++ b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> @@ -3,7 +3,8 @@
>  Required Properties:
>  
>  - Compatibility : "marvell,orion-wdt"
> -- reg		: Address of the timer registers
> +- reg		: First cell contains the register for the watchdog counter.
> +		  Second cell contains the register for the RSTOUT mask.
>  
>  Optional properties:
>  
> @@ -11,9 +12,10 @@ Optional properties:
>  
>  Example:
>  
> -	wdt@20300 {
> +	wdt@20324 {
>  		compatible = "marvell,orion-wdt";
> -		reg = <0x20300 0x28>;
> +		reg = <0x20324 0x4
> +		       0x20108 0x4>;
>  		timeout-sec = <10>;
>  		status = "okay";
>  	};
> -- 
> 1.8.1.5
> 

Re: [PATCH 06/10] watchdog: orion: Update device-tree binding documentation

[Jason Cooper]

On Mon, Jul 15, 2013 at 08:32:39PM -0300, Ezequiel Garcia wrote:
> Now that the 'reg' property meaning has been changed,
> this commit updates the deivce-tree binding documentation.

nit. s/deivce/device/

> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  Documentation/devicetree/bindings/watchdog/orion-wdt.txt | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> index 5dc8d30..48a71fc 100644
> --- a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> +++ b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> @@ -3,7 +3,8 @@
>  Required Properties:
>  
>  - Compatibility : "marvell,orion-wdt"
> -- reg		: Address of the timer registers
> +- reg		: First cell contains the register for the watchdog counter.
> +		  Second cell contains the register for the RSTOUT mask.
>  
>  Optional properties:
>  
> @@ -11,9 +12,10 @@ Optional properties:
>  
>  Example:
>  
> -	wdt@20300 {
> +	wdt@20324 {
>  		compatible = "marvell,orion-wdt";
> -		reg = <0x20300 0x28>;
> +		reg = <0x20324 0x4
> +		       0x20108 0x4>;
>  		timeout-sec = <10>;
>  		status = "okay";
>  	};
> -- 
> 1.8.1.5
> 

[PATCH 06/10] watchdog: orion: Update device-tree binding documentation

[Jason Cooper]

On Mon, Jul 15, 2013 at 08:32:39PM -0300, Ezequiel Garcia wrote:
> Now that the 'reg' property meaning has been changed,
> this commit updates the deivce-tree binding documentation.

nit. s/deivce/device/

> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  Documentation/devicetree/bindings/watchdog/orion-wdt.txt | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> index 5dc8d30..48a71fc 100644
> --- a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> +++ b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> @@ -3,7 +3,8 @@
>  Required Properties:
>  
>  - Compatibility : "marvell,orion-wdt"
> -- reg		: Address of the timer registers
> +- reg		: First cell contains the register for the watchdog counter.
> +		  Second cell contains the register for the RSTOUT mask.
>  
>  Optional properties:
>  
> @@ -11,9 +12,10 @@ Optional properties:
>  
>  Example:
>  
> -	wdt at 20300 {
> +	wdt at 20324 {
>  		compatible = "marvell,orion-wdt";
> -		reg = <0x20300 0x28>;
> +		reg = <0x20324 0x4
> +		       0x20108 0x4>;
>  		timeout-sec = <10>;
>  		status = "okay";
>  	};
> -- 
> 1.8.1.5
> 

Re: [PATCH 00/10] Orion Watchdog fixes

[Jason Cooper]

On Tue, Jul 16, 2013 at 09:14:33AM -0300, Ezequiel Garcia wrote:
> On the other side, I'm much interested in knowing if you are OK with
> breaking the watchdog DT compatibility. If you NACK this, then I'll
> start preparing a different watchdog driver for 370/XP, since I don't
> want to extend a driver that is a bit dirty.

Apparently there is some agreement that the bindings are still in flux
and that they need to be for a bit longer in order to hammer out
problems such as this.

Arnd and Olof both mentioned that something (a doc, and email?) is
forthcoming about marking some bindings as stable.  Whatever form that
takes, this one wouldn't get the stable marking yet. ;-)

Oh, and one more nit.  The work 'fix' triggers a whole bunch of "get on
this right away, does it need to go to stable?  Has anyone confirmed it?
Which commit caused the regression? etc."  Although I hate the word, I
think 'refactoring' is much more appropriate description for this series.

thx,

Jason.

Re: [PATCH 00/10] Orion Watchdog fixes

[Jason Cooper]

On Tue, Jul 16, 2013 at 09:14:33AM -0300, Ezequiel Garcia wrote:
> On the other side, I'm much interested in knowing if you are OK with
> breaking the watchdog DT compatibility. If you NACK this, then I'll
> start preparing a different watchdog driver for 370/XP, since I don't
> want to extend a driver that is a bit dirty.

Apparently there is some agreement that the bindings are still in flux
and that they need to be for a bit longer in order to hammer out
problems such as this.

Arnd and Olof both mentioned that something (a doc, and email?) is
forthcoming about marking some bindings as stable.  Whatever form that
takes, this one wouldn't get the stable marking yet. ;-)

Oh, and one more nit.  The work 'fix' triggers a whole bunch of "get on
this right away, does it need to go to stable?  Has anyone confirmed it?
Which commit caused the regression? etc."  Although I hate the word, I
think 'refactoring' is much more appropriate description for this series.

thx,

Jason.

[PATCH 00/10] Orion Watchdog fixes

[Jason Cooper]

On Tue, Jul 16, 2013 at 09:14:33AM -0300, Ezequiel Garcia wrote:
> On the other side, I'm much interested in knowing if you are OK with
> breaking the watchdog DT compatibility. If you NACK this, then I'll
> start preparing a different watchdog driver for 370/XP, since I don't
> want to extend a driver that is a bit dirty.

Apparently there is some agreement that the bindings are still in flux
and that they need to be for a bit longer in order to hammer out
problems such as this.

Arnd and Olof both mentioned that something (a doc, and email?) is
forthcoming about marking some bindings as stable.  Whatever form that
takes, this one wouldn't get the stable marking yet. ;-)

Oh, and one more nit.  The work 'fix' triggers a whole bunch of "get on
this right away, does it need to go to stable?  Has anyone confirmed it?
Which commit caused the regression? etc."  Although I hate the word, I
think 'refactoring' is much more appropriate description for this series.

thx,

Jason.

Re: [PATCH 05/10] watchdog: orion: Add a memory resource for RSTOUT register

[Andrew Lunn]

On Mon, Jul 15, 2013 at 08:32:38PM -0300, Ezequiel Garcia wrote:
> Instead of accessing the RSTOUT register directly, this commit
> adds a platform memory resource to map this register into the driver.

Hi Ezequiel

Have you looked at:

arch/arm/mach-mvebu/system-controller.c

It is also using this register. Are we going to have a similar problem
as the TIMER_CTRL register, which you refactered in an earlier patch?

"marvell,orion-system-controller" is not actually used yet, but once
kirkwood moves into mach-mvebu, it will start using it.

Andrew


> 
> Note that by adding a required 2nd-cell for the reg property,
> this change breaks the device-tree binding compatibility.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  arch/arm/mach-kirkwood/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                      | 18 ++++++++++--------
>  4 files changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
> index c3f361d..dead75a 100644
> --- a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
> +++ b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
> @@ -21,6 +21,7 @@
>  #define CPU_RESET		0x00000002
>  
>  #define RSTOUTn_MASK		(BRIDGE_VIRT_BASE + 0x0108)
> +#define RSTOUT_PHYS_BASE	(BRIDGE_PHYS_BASE + 0x0108)
>  #define WDT_RESET_OUT_EN	0x00000002
>  #define SOFT_RESET_OUT_EN	0x00000004
>  
> diff --git a/arch/arm/mach-orion5x/include/mach/bridge-regs.h b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
> index aa35de3..ccd91c9 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 RSTOUT_PHYS_BASE	(ORION5X_BRIDGE_PHYS_BASE + 0x108)
>  #define WDT_RESET_OUT_EN	0x0002
>  
>  #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 77fac6c..37924f8 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(WDT_PHYS_BASE, 0x04);
> +static struct resource orion_wdt_resource[] = {
> +		DEFINE_RES_MEM(WDT_PHYS_BASE, 0x04),
> +		DEFINE_RES_MEM(RSTOUT_PHYS_BASE, 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 01bcf53..3c9b3d2 100644
> --- a/drivers/watchdog/orion_wdt.c
> +++ b/drivers/watchdog/orion_wdt.c
> @@ -43,6 +43,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 *rstout_reg;
>  static DEFINE_SPINLOCK(wdt_lock);
>  
>  static int orion_wdt_ping(struct watchdog_device *wdt_dev)
> @@ -74,9 +75,7 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
>  	orion_timer_ctrl_clrset(0, WDT_EN);
>  
>  	/* Enable reset on watchdog */
> -	reg = readl(RSTOUTn_MASK);
> -	reg |= WDT_RESET_OUT_EN;
> -	writel(reg, RSTOUTn_MASK);
> +	writel(readl(rstout_reg) | WDT_RESET_OUT_EN, rstout_reg);
>  
>  	spin_unlock(&wdt_lock);
>  	return 0;
> @@ -84,14 +83,10 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
>  
>  static int orion_wdt_stop(struct watchdog_device *wdt_dev)
>  {
> -	u32 reg;
> -
>  	spin_lock(&wdt_lock);
>  
>  	/* Disable reset on watchdog */
> -	reg = readl(RSTOUTn_MASK);
> -	reg &= ~WDT_RESET_OUT_EN;
> -	writel(reg, RSTOUTn_MASK);
> +	writel(readl(rstout_reg) & ~WDT_RESET_OUT_EN, rstout_reg);
>  
>  	/* Disable watchdog timer */
>  	orion_timer_ctrl_clrset(WDT_EN, 0);
> @@ -158,6 +153,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;
> +	rstout_reg = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> +	if (!rstout_reg)
> +		return -ENOMEM;
> +
>  	wdt_max_duration = WDT_MAX_CYCLE_COUNT / wdt_tclk;
>  
>  	orion_wdt.timeout = wdt_max_duration;
> -- 
> 1.8.1.5
> 

Re: [PATCH 05/10] watchdog: orion: Add a memory resource for RSTOUT register

[Andrew Lunn]

On Mon, Jul 15, 2013 at 08:32:38PM -0300, Ezequiel Garcia wrote:
> Instead of accessing the RSTOUT register directly, this commit
> adds a platform memory resource to map this register into the driver.

Hi Ezequiel

Have you looked at:

arch/arm/mach-mvebu/system-controller.c

It is also using this register. Are we going to have a similar problem
as the TIMER_CTRL register, which you refactered in an earlier patch?

"marvell,orion-system-controller" is not actually used yet, but once
kirkwood moves into mach-mvebu, it will start using it.

Andrew


> 
> Note that by adding a required 2nd-cell for the reg property,
> this change breaks the device-tree binding compatibility.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  arch/arm/mach-kirkwood/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                      | 18 ++++++++++--------
>  4 files changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
> index c3f361d..dead75a 100644
> --- a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
> +++ b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
> @@ -21,6 +21,7 @@
>  #define CPU_RESET		0x00000002
>  
>  #define RSTOUTn_MASK		(BRIDGE_VIRT_BASE + 0x0108)
> +#define RSTOUT_PHYS_BASE	(BRIDGE_PHYS_BASE + 0x0108)
>  #define WDT_RESET_OUT_EN	0x00000002
>  #define SOFT_RESET_OUT_EN	0x00000004
>  
> diff --git a/arch/arm/mach-orion5x/include/mach/bridge-regs.h b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
> index aa35de3..ccd91c9 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 RSTOUT_PHYS_BASE	(ORION5X_BRIDGE_PHYS_BASE + 0x108)
>  #define WDT_RESET_OUT_EN	0x0002
>  
>  #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 77fac6c..37924f8 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(WDT_PHYS_BASE, 0x04);
> +static struct resource orion_wdt_resource[] = {
> +		DEFINE_RES_MEM(WDT_PHYS_BASE, 0x04),
> +		DEFINE_RES_MEM(RSTOUT_PHYS_BASE, 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 01bcf53..3c9b3d2 100644
> --- a/drivers/watchdog/orion_wdt.c
> +++ b/drivers/watchdog/orion_wdt.c
> @@ -43,6 +43,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 *rstout_reg;
>  static DEFINE_SPINLOCK(wdt_lock);
>  
>  static int orion_wdt_ping(struct watchdog_device *wdt_dev)
> @@ -74,9 +75,7 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
>  	orion_timer_ctrl_clrset(0, WDT_EN);
>  
>  	/* Enable reset on watchdog */
> -	reg = readl(RSTOUTn_MASK);
> -	reg |= WDT_RESET_OUT_EN;
> -	writel(reg, RSTOUTn_MASK);
> +	writel(readl(rstout_reg) | WDT_RESET_OUT_EN, rstout_reg);
>  
>  	spin_unlock(&wdt_lock);
>  	return 0;
> @@ -84,14 +83,10 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
>  
>  static int orion_wdt_stop(struct watchdog_device *wdt_dev)
>  {
> -	u32 reg;
> -
>  	spin_lock(&wdt_lock);
>  
>  	/* Disable reset on watchdog */
> -	reg = readl(RSTOUTn_MASK);
> -	reg &= ~WDT_RESET_OUT_EN;
> -	writel(reg, RSTOUTn_MASK);
> +	writel(readl(rstout_reg) & ~WDT_RESET_OUT_EN, rstout_reg);
>  
>  	/* Disable watchdog timer */
>  	orion_timer_ctrl_clrset(WDT_EN, 0);
> @@ -158,6 +153,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;
> +	rstout_reg = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> +	if (!rstout_reg)
> +		return -ENOMEM;
> +
>  	wdt_max_duration = WDT_MAX_CYCLE_COUNT / wdt_tclk;
>  
>  	orion_wdt.timeout = wdt_max_duration;
> -- 
> 1.8.1.5
> 

[PATCH 05/10] watchdog: orion: Add a memory resource for RSTOUT register

[Andrew Lunn]

On Mon, Jul 15, 2013 at 08:32:38PM -0300, Ezequiel Garcia wrote:
> Instead of accessing the RSTOUT register directly, this commit
> adds a platform memory resource to map this register into the driver.

Hi Ezequiel

Have you looked at:

arch/arm/mach-mvebu/system-controller.c

It is also using this register. Are we going to have a similar problem
as the TIMER_CTRL register, which you refactered in an earlier patch?

"marvell,orion-system-controller" is not actually used yet, but once
kirkwood moves into mach-mvebu, it will start using it.

Andrew


> 
> Note that by adding a required 2nd-cell for the reg property,
> this change breaks the device-tree binding compatibility.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  arch/arm/mach-kirkwood/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                      | 18 ++++++++++--------
>  4 files changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
> index c3f361d..dead75a 100644
> --- a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
> +++ b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
> @@ -21,6 +21,7 @@
>  #define CPU_RESET		0x00000002
>  
>  #define RSTOUTn_MASK		(BRIDGE_VIRT_BASE + 0x0108)
> +#define RSTOUT_PHYS_BASE	(BRIDGE_PHYS_BASE + 0x0108)
>  #define WDT_RESET_OUT_EN	0x00000002
>  #define SOFT_RESET_OUT_EN	0x00000004
>  
> diff --git a/arch/arm/mach-orion5x/include/mach/bridge-regs.h b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
> index aa35de3..ccd91c9 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 RSTOUT_PHYS_BASE	(ORION5X_BRIDGE_PHYS_BASE + 0x108)
>  #define WDT_RESET_OUT_EN	0x0002
>  
>  #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 77fac6c..37924f8 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(WDT_PHYS_BASE, 0x04);
> +static struct resource orion_wdt_resource[] = {
> +		DEFINE_RES_MEM(WDT_PHYS_BASE, 0x04),
> +		DEFINE_RES_MEM(RSTOUT_PHYS_BASE, 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 01bcf53..3c9b3d2 100644
> --- a/drivers/watchdog/orion_wdt.c
> +++ b/drivers/watchdog/orion_wdt.c
> @@ -43,6 +43,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 *rstout_reg;
>  static DEFINE_SPINLOCK(wdt_lock);
>  
>  static int orion_wdt_ping(struct watchdog_device *wdt_dev)
> @@ -74,9 +75,7 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
>  	orion_timer_ctrl_clrset(0, WDT_EN);
>  
>  	/* Enable reset on watchdog */
> -	reg = readl(RSTOUTn_MASK);
> -	reg |= WDT_RESET_OUT_EN;
> -	writel(reg, RSTOUTn_MASK);
> +	writel(readl(rstout_reg) | WDT_RESET_OUT_EN, rstout_reg);
>  
>  	spin_unlock(&wdt_lock);
>  	return 0;
> @@ -84,14 +83,10 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
>  
>  static int orion_wdt_stop(struct watchdog_device *wdt_dev)
>  {
> -	u32 reg;
> -
>  	spin_lock(&wdt_lock);
>  
>  	/* Disable reset on watchdog */
> -	reg = readl(RSTOUTn_MASK);
> -	reg &= ~WDT_RESET_OUT_EN;
> -	writel(reg, RSTOUTn_MASK);
> +	writel(readl(rstout_reg) & ~WDT_RESET_OUT_EN, rstout_reg);
>  
>  	/* Disable watchdog timer */
>  	orion_timer_ctrl_clrset(WDT_EN, 0);
> @@ -158,6 +153,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;
> +	rstout_reg = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> +	if (!rstout_reg)
> +		return -ENOMEM;
> +
>  	wdt_max_duration = WDT_MAX_CYCLE_COUNT / wdt_tclk;
>  
>  	orion_wdt.timeout = wdt_max_duration;
> -- 
> 1.8.1.5
> 

Re: [PATCH 00/10] Orion Watchdog fixes

[Ezequiel Garcia]

On Tue, Jul 16, 2013 at 09:44:22AM -0400, Jason Cooper wrote:
> On Tue, Jul 16, 2013 at 09:14:33AM -0300, Ezequiel Garcia wrote:
> > On the other side, I'm much interested in knowing if you are OK with
> > breaking the watchdog DT compatibility. If you NACK this, then I'll
> > start preparing a different watchdog driver for 370/XP, since I don't
> > want to extend a driver that is a bit dirty.
> 
> Apparently there is some agreement that the bindings are still in flux
> and that they need to be for a bit longer in order to hammer out
> problems such as this.
> 
> Arnd and Olof both mentioned that something (a doc, and email?) is
> forthcoming about marking some bindings as stable.  Whatever form that
> takes, this one wouldn't get the stable marking yet. ;-)
> 

Yup, that's my understanding as well. But on the other side, I don't
want to break possible users out there.

So, just to check, you say it's early enough to safely do such change?

In that case, I'll extend this patchset to include Armada 370/XP support
and post it as soon as Sebastian's clocksource stuff gets in.

> Oh, and one more nit.  The work 'fix' triggers a whole bunch of "get on
> this right away, does it need to go to stable?  Has anyone confirmed it?
> Which commit caused the regression? etc."  Although I hate the word, I
> think 'refactoring' is much more appropriate description for this series.
> 

Oh, good observation. I wrote the cover letter at 8 PM, after ten long
hours (*) of hacking and smashing this into something easy to review,
and that's the best title I could come up with. I'll change it on v2.

Thanks,

(*) yes, I have another pair of eyes, in case these wear out.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 00/10] Orion Watchdog fixes

[Ezequiel Garcia]

On Tue, Jul 16, 2013 at 09:44:22AM -0400, Jason Cooper wrote:
> On Tue, Jul 16, 2013 at 09:14:33AM -0300, Ezequiel Garcia wrote:
> > On the other side, I'm much interested in knowing if you are OK with
> > breaking the watchdog DT compatibility. If you NACK this, then I'll
> > start preparing a different watchdog driver for 370/XP, since I don't
> > want to extend a driver that is a bit dirty.
> 
> Apparently there is some agreement that the bindings are still in flux
> and that they need to be for a bit longer in order to hammer out
> problems such as this.
> 
> Arnd and Olof both mentioned that something (a doc, and email?) is
> forthcoming about marking some bindings as stable.  Whatever form that
> takes, this one wouldn't get the stable marking yet. ;-)
> 

Yup, that's my understanding as well. But on the other side, I don't
want to break possible users out there.

So, just to check, you say it's early enough to safely do such change?

In that case, I'll extend this patchset to include Armada 370/XP support
and post it as soon as Sebastian's clocksource stuff gets in.

> Oh, and one more nit.  The work 'fix' triggers a whole bunch of "get on
> this right away, does it need to go to stable?  Has anyone confirmed it?
> Which commit caused the regression? etc."  Although I hate the word, I
> think 'refactoring' is much more appropriate description for this series.
> 

Oh, good observation. I wrote the cover letter at 8 PM, after ten long
hours (*) of hacking and smashing this into something easy to review,
and that's the best title I could come up with. I'll change it on v2.

Thanks,

(*) yes, I have another pair of eyes, in case these wear out.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 00/10] Orion Watchdog fixes

[Ezequiel Garcia]

On Tue, Jul 16, 2013 at 09:44:22AM -0400, Jason Cooper wrote:
> On Tue, Jul 16, 2013 at 09:14:33AM -0300, Ezequiel Garcia wrote:
> > On the other side, I'm much interested in knowing if you are OK with
> > breaking the watchdog DT compatibility. If you NACK this, then I'll
> > start preparing a different watchdog driver for 370/XP, since I don't
> > want to extend a driver that is a bit dirty.
> 
> Apparently there is some agreement that the bindings are still in flux
> and that they need to be for a bit longer in order to hammer out
> problems such as this.
> 
> Arnd and Olof both mentioned that something (a doc, and email?) is
> forthcoming about marking some bindings as stable.  Whatever form that
> takes, this one wouldn't get the stable marking yet. ;-)
> 

Yup, that's my understanding as well. But on the other side, I don't
want to break possible users out there.

So, just to check, you say it's early enough to safely do such change?

In that case, I'll extend this patchset to include Armada 370/XP support
and post it as soon as Sebastian's clocksource stuff gets in.

> Oh, and one more nit.  The work 'fix' triggers a whole bunch of "get on
> this right away, does it need to go to stable?  Has anyone confirmed it?
> Which commit caused the regression? etc."  Although I hate the word, I
> think 'refactoring' is much more appropriate description for this series.
> 

Oh, good observation. I wrote the cover letter at 8 PM, after ten long
hours (*) of hacking and smashing this into something easy to review,
and that's the best title I could come up with. I'll change it on v2.

Thanks,

(*) yes, I have another pair of eyes, in case these wear out.
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH 05/10] watchdog: orion: Add a memory resource for RSTOUT register

[Ezequiel Garcia]

Andrew,

On Tue, Jul 16, 2013 at 04:04:15PM +0200, Andrew Lunn wrote:
> On Mon, Jul 15, 2013 at 08:32:38PM -0300, Ezequiel Garcia wrote:
> > Instead of accessing the RSTOUT register directly, this commit
> > adds a platform memory resource to map this register into the driver.
> 
> 
> Have you looked at:
> 
> arch/arm/mach-mvebu/system-controller.c
> 

Mmm... I saw the use of the RSTOUT register in kirkwood_restart()
but wasn't sure who should be the real 'owner' of this register.

> It is also using this register. Are we going to have a similar problem
> as the TIMER_CTRL register, which you refactered in an earlier patch?
> 

Probably.

> "marvell,orion-system-controller" is not actually used yet, but once
> kirkwood moves into mach-mvebu, it will start using it.
> 

I guess so. We should take that into account *now*. Let me think about
it and see if I can have something sane for v2.

Thanks!
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 05/10] watchdog: orion: Add a memory resource for RSTOUT register

[Ezequiel Garcia]

Andrew,

On Tue, Jul 16, 2013 at 04:04:15PM +0200, Andrew Lunn wrote:
> On Mon, Jul 15, 2013 at 08:32:38PM -0300, Ezequiel Garcia wrote:
> > Instead of accessing the RSTOUT register directly, this commit
> > adds a platform memory resource to map this register into the driver.
> 
> 
> Have you looked at:
> 
> arch/arm/mach-mvebu/system-controller.c
> 

Mmm... I saw the use of the RSTOUT register in kirkwood_restart()
but wasn't sure who should be the real 'owner' of this register.

> It is also using this register. Are we going to have a similar problem
> as the TIMER_CTRL register, which you refactered in an earlier patch?
> 

Probably.

> "marvell,orion-system-controller" is not actually used yet, but once
> kirkwood moves into mach-mvebu, it will start using it.
> 

I guess so. We should take that into account *now*. Let me think about
it and see if I can have something sane for v2.

Thanks!
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 05/10] watchdog: orion: Add a memory resource for RSTOUT register

[Ezequiel Garcia]

Andrew,

On Tue, Jul 16, 2013 at 04:04:15PM +0200, Andrew Lunn wrote:
> On Mon, Jul 15, 2013 at 08:32:38PM -0300, Ezequiel Garcia wrote:
> > Instead of accessing the RSTOUT register directly, this commit
> > adds a platform memory resource to map this register into the driver.
> 
> 
> Have you looked at:
> 
> arch/arm/mach-mvebu/system-controller.c
> 

Mmm... I saw the use of the RSTOUT register in kirkwood_restart()
but wasn't sure who should be the real 'owner' of this register.

> It is also using this register. Are we going to have a similar problem
> as the TIMER_CTRL register, which you refactered in an earlier patch?
> 

Probably.

> "marvell,orion-system-controller" is not actually used yet, but once
> kirkwood moves into mach-mvebu, it will start using it.
> 

I guess so. We should take that into account *now*. Let me think about
it and see if I can have something sane for v2.

Thanks!
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH 00/10] Orion Watchdog fixes

[Jason Cooper]

On Tue, Jul 16, 2013 at 11:04:36AM -0300, Ezequiel Garcia wrote:
> On Tue, Jul 16, 2013 at 09:44:22AM -0400, Jason Cooper wrote:
> > On Tue, Jul 16, 2013 at 09:14:33AM -0300, Ezequiel Garcia wrote:
> > > On the other side, I'm much interested in knowing if you are OK with
> > > breaking the watchdog DT compatibility. If you NACK this, then I'll
> > > start preparing a different watchdog driver for 370/XP, since I don't
> > > want to extend a driver that is a bit dirty.
> > 
> > Apparently there is some agreement that the bindings are still in flux
> > and that they need to be for a bit longer in order to hammer out
> > problems such as this.
> > 
> > Arnd and Olof both mentioned that something (a doc, and email?) is
> > forthcoming about marking some bindings as stable.  Whatever form that
> > takes, this one wouldn't get the stable marking yet. ;-)
> > 
> 
> Yup, that's my understanding as well. But on the other side, I don't
> want to break possible users out there.

I agree, but living with a sub-standard binding is worse imho.

> So, just to check, you say it's early enough to safely do such change?

Well, it's not *safe*.  But most consumers on mainline kernels are
currently developer/hackers/distro maintainers.  eg, folks who can
handle it.  It's better to do it now.

> In that case, I'll extend this patchset to include Armada 370/XP support
> and post it as soon as Sebastian's clocksource stuff gets in.

Cool.

thx,

Jason.

Re: [PATCH 00/10] Orion Watchdog fixes

[Jason Cooper]

On Tue, Jul 16, 2013 at 11:04:36AM -0300, Ezequiel Garcia wrote:
> On Tue, Jul 16, 2013 at 09:44:22AM -0400, Jason Cooper wrote:
> > On Tue, Jul 16, 2013 at 09:14:33AM -0300, Ezequiel Garcia wrote:
> > > On the other side, I'm much interested in knowing if you are OK with
> > > breaking the watchdog DT compatibility. If you NACK this, then I'll
> > > start preparing a different watchdog driver for 370/XP, since I don't
> > > want to extend a driver that is a bit dirty.
> > 
> > Apparently there is some agreement that the bindings are still in flux
> > and that they need to be for a bit longer in order to hammer out
> > problems such as this.
> > 
> > Arnd and Olof both mentioned that something (a doc, and email?) is
> > forthcoming about marking some bindings as stable.  Whatever form that
> > takes, this one wouldn't get the stable marking yet. ;-)
> > 
> 
> Yup, that's my understanding as well. But on the other side, I don't
> want to break possible users out there.

I agree, but living with a sub-standard binding is worse imho.

> So, just to check, you say it's early enough to safely do such change?

Well, it's not *safe*.  But most consumers on mainline kernels are
currently developer/hackers/distro maintainers.  eg, folks who can
handle it.  It's better to do it now.

> In that case, I'll extend this patchset to include Armada 370/XP support
> and post it as soon as Sebastian's clocksource stuff gets in.

Cool.

thx,

Jason.

[PATCH 00/10] Orion Watchdog fixes

[Jason Cooper]

On Tue, Jul 16, 2013 at 11:04:36AM -0300, Ezequiel Garcia wrote:
> On Tue, Jul 16, 2013 at 09:44:22AM -0400, Jason Cooper wrote:
> > On Tue, Jul 16, 2013 at 09:14:33AM -0300, Ezequiel Garcia wrote:
> > > On the other side, I'm much interested in knowing if you are OK with
> > > breaking the watchdog DT compatibility. If you NACK this, then I'll
> > > start preparing a different watchdog driver for 370/XP, since I don't
> > > want to extend a driver that is a bit dirty.
> > 
> > Apparently there is some agreement that the bindings are still in flux
> > and that they need to be for a bit longer in order to hammer out
> > problems such as this.
> > 
> > Arnd and Olof both mentioned that something (a doc, and email?) is
> > forthcoming about marking some bindings as stable.  Whatever form that
> > takes, this one wouldn't get the stable marking yet. ;-)
> > 
> 
> Yup, that's my understanding as well. But on the other side, I don't
> want to break possible users out there.

I agree, but living with a sub-standard binding is worse imho.

> So, just to check, you say it's early enough to safely do such change?

Well, it's not *safe*.  But most consumers on mainline kernels are
currently developer/hackers/distro maintainers.  eg, folks who can
handle it.  It's better to do it now.

> In that case, I'll extend this patchset to include Armada 370/XP support
> and post it as soon as Sebastian's clocksource stuff gets in.

Cool.

thx,

Jason.