[PATCH 1/2] ARM: BCM2835: Drop the init_irq() hook.

[PATCH 1/2] ARM: BCM2835: Drop the init_irq() hook.

[Eric Anholt]

This is the default function that gets called if the hook is NULL.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 arch/arm/mach-bcm/board_bcm2835.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/mach-bcm/board_bcm2835.c b/arch/arm/mach-bcm/board_bcm2835.c
index 9851ee8..49dd5b0 100644
--- a/arch/arm/mach-bcm/board_bcm2835.c
+++ b/arch/arm/mach-bcm/board_bcm2835.c
@@ -113,7 +113,6 @@ static const char * const bcm2835_compat[] = {
 };
 
 DT_MACHINE_START(BCM2835, "BCM2835")
-	.init_irq = irqchip_init,
 	.init_machine = bcm2835_init,
 	.restart = bcm2835_restart,
 	.dt_compat = bcm2835_compat
-- 
2.1.4

[PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Eric Anholt]

Since the WDT is what's used to drive restart and power off, it makes
more sense to keep it there, where the regs are already mapped and
definitions for them provided.  Note that this means you may need to
add CONFIG_BCM2835_WDT to retain functionality of your kernel.

Signed-off-by: Eric Anholt <eric@anholt.net>
Cc: linux-watchdog@vger.kernel.org
---

Note that power off has never worked for me, and just reboots as well.
So I can't say that I've *really* tested the power off code.

 arch/arm/mach-bcm/board_bcm2835.c | 73 ---------------------------------------
 drivers/watchdog/bcm2835_wdt.c    | 62 +++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 73 deletions(-)

diff --git a/arch/arm/mach-bcm/board_bcm2835.c b/arch/arm/mach-bcm/board_bcm2835.c
index 49dd5b0..0f7b9ea 100644
--- a/arch/arm/mach-bcm/board_bcm2835.c
+++ b/arch/arm/mach-bcm/board_bcm2835.c
@@ -12,7 +12,6 @@
  * GNU General Public License for more details.
  */
 
-#include <linux/delay.h>
 #include <linux/init.h>
 #include <linux/irqchip.h>
 #include <linux/of_address.h>
@@ -22,81 +21,10 @@
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
 
-#define PM_RSTC				0x1c
-#define PM_RSTS				0x20
-#define PM_WDOG				0x24
-
-#define PM_PASSWORD			0x5a000000
-#define PM_RSTC_WRCFG_MASK		0x00000030
-#define PM_RSTC_WRCFG_FULL_RESET	0x00000020
-#define PM_RSTS_HADWRH_SET		0x00000040
-
-static void __iomem *wdt_regs;
-
-/*
- * The machine restart method can be called from an atomic context so we won't
- * be able to ioremap the regs then.
- */
-static void bcm2835_setup_restart(void)
-{
-	struct device_node *np = of_find_compatible_node(NULL, NULL,
-						"brcm,bcm2835-pm-wdt");
-	if (WARN(!np, "unable to setup watchdog restart"))
-		return;
-
-	wdt_regs = of_iomap(np, 0);
-	WARN(!wdt_regs, "failed to remap watchdog regs");
-}
-
-static void bcm2835_restart(enum reboot_mode mode, const char *cmd)
-{
-	u32 val;
-
-	if (!wdt_regs)
-		return;
-
-	/* use a timeout of 10 ticks (~150us) */
-	writel_relaxed(10 | PM_PASSWORD, wdt_regs + PM_WDOG);
-	val = readl_relaxed(wdt_regs + PM_RSTC);
-	val &= ~PM_RSTC_WRCFG_MASK;
-	val |= PM_PASSWORD | PM_RSTC_WRCFG_FULL_RESET;
-	writel_relaxed(val, wdt_regs + PM_RSTC);
-
-	/* No sleeping, possibly atomic. */
-	mdelay(1);
-}
-
-/*
- * We can't really power off, but if we do the normal reset scheme, and
- * indicate to bootcode.bin not to reboot, then most of the chip will be
- * powered off.
- */
-static void bcm2835_power_off(void)
-{
-	u32 val;
-
-	/*
-	 * We set the watchdog hard reset bit here to distinguish this reset
-	 * from the normal (full) reset. bootcode.bin will not reboot after a
-	 * hard reset.
-	 */
-	val = readl_relaxed(wdt_regs + PM_RSTS);
-	val &= ~PM_RSTC_WRCFG_MASK;
-	val |= PM_PASSWORD | PM_RSTS_HADWRH_SET;
-	writel_relaxed(val, wdt_regs + PM_RSTS);
-
-	/* Continue with normal reset mechanism */
-	bcm2835_restart(REBOOT_HARD, "");
-}
-
 static void __init bcm2835_init(void)
 {
 	int ret;
 
-	bcm2835_setup_restart();
-	if (wdt_regs)
-		pm_power_off = bcm2835_power_off;
-
 	bcm2835_init_clocks();
 
 	ret = of_platform_populate(NULL, of_default_bus_match_table, NULL,
@@ -114,6 +42,5 @@ static const char * const bcm2835_compat[] = {
 
 DT_MACHINE_START(BCM2835, "BCM2835")
 	.init_machine = bcm2835_init,
-	.restart = bcm2835_restart,
 	.dt_compat = bcm2835_compat
 MACHINE_END
diff --git a/drivers/watchdog/bcm2835_wdt.c b/drivers/watchdog/bcm2835_wdt.c
index 2b5a9bb..7116968 100644
--- a/drivers/watchdog/bcm2835_wdt.c
+++ b/drivers/watchdog/bcm2835_wdt.c
@@ -13,20 +13,25 @@
  * option) any later version.
  */
 
+#include <linux/delay.h>
+#include <linux/reboot.h>
 #include <linux/types.h>
 #include <linux/module.h>
 #include <linux/io.h>
 #include <linux/watchdog.h>
 #include <linux/platform_device.h>
 #include <linux/of_address.h>
+#include <linux/of_platform.h>
 
 #define PM_RSTC				0x1c
+#define PM_RSTS				0x20
 #define PM_WDOG				0x24
 
 #define PM_PASSWORD			0x5a000000
 
 #define PM_WDOG_TIME_SET		0x000fffff
 #define PM_RSTC_WRCFG_CLR		0xffffffcf
+#define PM_RSTS_HADWRH_SET		0x00000040
 #define PM_RSTC_WRCFG_SET		0x00000030
 #define PM_RSTC_WRCFG_FULL_RESET	0x00000020
 #define PM_RSTC_RESET			0x00000102
@@ -37,6 +42,7 @@
 struct bcm2835_wdt {
 	void __iomem		*base;
 	spinlock_t		lock;
+	struct notifier_block	restart_handler;
 };
 
 static unsigned int heartbeat;
@@ -106,6 +112,53 @@ static struct watchdog_device bcm2835_wdt_wdd = {
 	.timeout =	WDOG_TICKS_TO_SECS(PM_WDOG_TIME_SET),
 };
 
+static int
+bcm2835_restart(struct notifier_block *this, unsigned long mode, void *cmd)
+{
+	struct bcm2835_wdt *wdt = container_of(this, struct bcm2835_wdt,
+					       restart_handler);
+	u32 val;
+
+	/* use a timeout of 10 ticks (~150us) */
+	writel_relaxed(10 | PM_PASSWORD, wdt->base + PM_WDOG);
+	val = readl_relaxed(wdt->base + PM_RSTC);
+	val &= PM_RSTC_WRCFG_CLR;
+	val |= PM_PASSWORD | PM_RSTC_WRCFG_FULL_RESET;
+	writel_relaxed(val, wdt->base + PM_RSTC);
+
+	/* No sleeping, possibly atomic. */
+	mdelay(1);
+
+	return 0;
+}
+
+/*
+ * We can't really power off, but if we do the normal reset scheme, and
+ * indicate to bootcode.bin not to reboot, then most of the chip will be
+ * powered off.
+ */
+static void bcm2835_power_off(void)
+{
+	struct device_node *np =
+		of_find_compatible_node(NULL, NULL, "brcm,bcm2835-pm-wdt");
+	struct platform_device *pdev = of_find_device_by_node(np);
+	struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
+	u32 val;
+
+	/*
+	 * We set the watchdog hard reset bit here to distinguish this reset
+	 * from the normal (full) reset. bootcode.bin will not reboot after a
+	 * hard reset.
+	 */
+	val = readl_relaxed(wdt->base + PM_RSTS);
+	val &= PM_RSTC_WRCFG_CLR;
+	val |= PM_PASSWORD | PM_RSTS_HADWRH_SET;
+	writel_relaxed(val, wdt->base + PM_RSTS);
+
+	/* Continue with normal reset mechanism */
+	bcm2835_restart(&wdt->restart_handler, REBOOT_HARD, NULL);
+}
+
 static int bcm2835_wdt_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -136,6 +189,12 @@ static int bcm2835_wdt_probe(struct platform_device *pdev)
 		return err;
 	}
 
+	wdt->restart_handler.notifier_call = bcm2835_restart;
+	wdt->restart_handler.priority = 128;
+	register_restart_handler(&wdt->restart_handler);
+	if (pm_power_off == NULL)
+		pm_power_off = bcm2835_power_off;
+
 	dev_info(dev, "Broadcom BCM2835 watchdog timer");
 	return 0;
 }
@@ -144,6 +203,9 @@ static int bcm2835_wdt_remove(struct platform_device *pdev)
 {
 	struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
 
+	unregister_restart_handler(&wdt->restart_handler);
+	if (pm_power_off == bcm2835_power_off)
+		pm_power_off = NULL;
 	watchdog_unregister_device(&bcm2835_wdt_wdd);
 	iounmap(wdt->base);
 
-- 
2.1.4

[PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Eric Anholt]

Since the WDT is what's used to drive restart and power off, it makes
more sense to keep it there, where the regs are already mapped and
definitions for them provided.  Note that this means you may need to
add CONFIG_BCM2835_WDT to retain functionality of your kernel.

Signed-off-by: Eric Anholt <eric@anholt.net>
Cc: linux-watchdog at vger.kernel.org
---

Note that power off has never worked for me, and just reboots as well.
So I can't say that I've *really* tested the power off code.

 arch/arm/mach-bcm/board_bcm2835.c | 73 ---------------------------------------
 drivers/watchdog/bcm2835_wdt.c    | 62 +++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 73 deletions(-)

diff --git a/arch/arm/mach-bcm/board_bcm2835.c b/arch/arm/mach-bcm/board_bcm2835.c
index 49dd5b0..0f7b9ea 100644
--- a/arch/arm/mach-bcm/board_bcm2835.c
+++ b/arch/arm/mach-bcm/board_bcm2835.c
@@ -12,7 +12,6 @@
  * GNU General Public License for more details.
  */
 
-#include <linux/delay.h>
 #include <linux/init.h>
 #include <linux/irqchip.h>
 #include <linux/of_address.h>
@@ -22,81 +21,10 @@
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
 
-#define PM_RSTC				0x1c
-#define PM_RSTS				0x20
-#define PM_WDOG				0x24
-
-#define PM_PASSWORD			0x5a000000
-#define PM_RSTC_WRCFG_MASK		0x00000030
-#define PM_RSTC_WRCFG_FULL_RESET	0x00000020
-#define PM_RSTS_HADWRH_SET		0x00000040
-
-static void __iomem *wdt_regs;
-
-/*
- * The machine restart method can be called from an atomic context so we won't
- * be able to ioremap the regs then.
- */
-static void bcm2835_setup_restart(void)
-{
-	struct device_node *np = of_find_compatible_node(NULL, NULL,
-						"brcm,bcm2835-pm-wdt");
-	if (WARN(!np, "unable to setup watchdog restart"))
-		return;
-
-	wdt_regs = of_iomap(np, 0);
-	WARN(!wdt_regs, "failed to remap watchdog regs");
-}
-
-static void bcm2835_restart(enum reboot_mode mode, const char *cmd)
-{
-	u32 val;
-
-	if (!wdt_regs)
-		return;
-
-	/* use a timeout of 10 ticks (~150us) */
-	writel_relaxed(10 | PM_PASSWORD, wdt_regs + PM_WDOG);
-	val = readl_relaxed(wdt_regs + PM_RSTC);
-	val &= ~PM_RSTC_WRCFG_MASK;
-	val |= PM_PASSWORD | PM_RSTC_WRCFG_FULL_RESET;
-	writel_relaxed(val, wdt_regs + PM_RSTC);
-
-	/* No sleeping, possibly atomic. */
-	mdelay(1);
-}
-
-/*
- * We can't really power off, but if we do the normal reset scheme, and
- * indicate to bootcode.bin not to reboot, then most of the chip will be
- * powered off.
- */
-static void bcm2835_power_off(void)
-{
-	u32 val;
-
-	/*
-	 * We set the watchdog hard reset bit here to distinguish this reset
-	 * from the normal (full) reset. bootcode.bin will not reboot after a
-	 * hard reset.
-	 */
-	val = readl_relaxed(wdt_regs + PM_RSTS);
-	val &= ~PM_RSTC_WRCFG_MASK;
-	val |= PM_PASSWORD | PM_RSTS_HADWRH_SET;
-	writel_relaxed(val, wdt_regs + PM_RSTS);
-
-	/* Continue with normal reset mechanism */
-	bcm2835_restart(REBOOT_HARD, "");
-}
-
 static void __init bcm2835_init(void)
 {
 	int ret;
 
-	bcm2835_setup_restart();
-	if (wdt_regs)
-		pm_power_off = bcm2835_power_off;
-
 	bcm2835_init_clocks();
 
 	ret = of_platform_populate(NULL, of_default_bus_match_table, NULL,
@@ -114,6 +42,5 @@ static const char * const bcm2835_compat[] = {
 
 DT_MACHINE_START(BCM2835, "BCM2835")
 	.init_machine = bcm2835_init,
-	.restart = bcm2835_restart,
 	.dt_compat = bcm2835_compat
 MACHINE_END
diff --git a/drivers/watchdog/bcm2835_wdt.c b/drivers/watchdog/bcm2835_wdt.c
index 2b5a9bb..7116968 100644
--- a/drivers/watchdog/bcm2835_wdt.c
+++ b/drivers/watchdog/bcm2835_wdt.c
@@ -13,20 +13,25 @@
  * option) any later version.
  */
 
+#include <linux/delay.h>
+#include <linux/reboot.h>
 #include <linux/types.h>
 #include <linux/module.h>
 #include <linux/io.h>
 #include <linux/watchdog.h>
 #include <linux/platform_device.h>
 #include <linux/of_address.h>
+#include <linux/of_platform.h>
 
 #define PM_RSTC				0x1c
+#define PM_RSTS				0x20
 #define PM_WDOG				0x24
 
 #define PM_PASSWORD			0x5a000000
 
 #define PM_WDOG_TIME_SET		0x000fffff
 #define PM_RSTC_WRCFG_CLR		0xffffffcf
+#define PM_RSTS_HADWRH_SET		0x00000040
 #define PM_RSTC_WRCFG_SET		0x00000030
 #define PM_RSTC_WRCFG_FULL_RESET	0x00000020
 #define PM_RSTC_RESET			0x00000102
@@ -37,6 +42,7 @@
 struct bcm2835_wdt {
 	void __iomem		*base;
 	spinlock_t		lock;
+	struct notifier_block	restart_handler;
 };
 
 static unsigned int heartbeat;
@@ -106,6 +112,53 @@ static struct watchdog_device bcm2835_wdt_wdd = {
 	.timeout =	WDOG_TICKS_TO_SECS(PM_WDOG_TIME_SET),
 };
 
+static int
+bcm2835_restart(struct notifier_block *this, unsigned long mode, void *cmd)
+{
+	struct bcm2835_wdt *wdt = container_of(this, struct bcm2835_wdt,
+					       restart_handler);
+	u32 val;
+
+	/* use a timeout of 10 ticks (~150us) */
+	writel_relaxed(10 | PM_PASSWORD, wdt->base + PM_WDOG);
+	val = readl_relaxed(wdt->base + PM_RSTC);
+	val &= PM_RSTC_WRCFG_CLR;
+	val |= PM_PASSWORD | PM_RSTC_WRCFG_FULL_RESET;
+	writel_relaxed(val, wdt->base + PM_RSTC);
+
+	/* No sleeping, possibly atomic. */
+	mdelay(1);
+
+	return 0;
+}
+
+/*
+ * We can't really power off, but if we do the normal reset scheme, and
+ * indicate to bootcode.bin not to reboot, then most of the chip will be
+ * powered off.
+ */
+static void bcm2835_power_off(void)
+{
+	struct device_node *np =
+		of_find_compatible_node(NULL, NULL, "brcm,bcm2835-pm-wdt");
+	struct platform_device *pdev = of_find_device_by_node(np);
+	struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
+	u32 val;
+
+	/*
+	 * We set the watchdog hard reset bit here to distinguish this reset
+	 * from the normal (full) reset. bootcode.bin will not reboot after a
+	 * hard reset.
+	 */
+	val = readl_relaxed(wdt->base + PM_RSTS);
+	val &= PM_RSTC_WRCFG_CLR;
+	val |= PM_PASSWORD | PM_RSTS_HADWRH_SET;
+	writel_relaxed(val, wdt->base + PM_RSTS);
+
+	/* Continue with normal reset mechanism */
+	bcm2835_restart(&wdt->restart_handler, REBOOT_HARD, NULL);
+}
+
 static int bcm2835_wdt_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -136,6 +189,12 @@ static int bcm2835_wdt_probe(struct platform_device *pdev)
 		return err;
 	}
 
+	wdt->restart_handler.notifier_call = bcm2835_restart;
+	wdt->restart_handler.priority = 128;
+	register_restart_handler(&wdt->restart_handler);
+	if (pm_power_off == NULL)
+		pm_power_off = bcm2835_power_off;
+
 	dev_info(dev, "Broadcom BCM2835 watchdog timer");
 	return 0;
 }
@@ -144,6 +203,9 @@ static int bcm2835_wdt_remove(struct platform_device *pdev)
 {
 	struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
 
+	unregister_restart_handler(&wdt->restart_handler);
+	if (pm_power_off == bcm2835_power_off)
+		pm_power_off = NULL;
 	watchdog_unregister_device(&bcm2835_wdt_wdd);
 	iounmap(wdt->base);
 
-- 
2.1.4

Re: [PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Pranith Kumar]

On Fri, Apr 24, 2015 at 3:08 PM, Eric Anholt <eric@anholt.net> wrote:
> Since the WDT is what's used to drive restart and power off, it makes
> more sense to keep it there, where the regs are already mapped and
> definitions for them provided.  Note that this means you may need to
> add CONFIG_BCM2835_WDT to retain functionality of your kernel.
>
> Signed-off-by: Eric Anholt <eric@anholt.net>
> Cc: linux-watchdog@vger.kernel.org


Any reason not to select BCM2835_WDT in the config by default then?

Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
---
diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
index 8b11f44..dd4856d 100644
--- a/arch/arm/mach-bcm/Kconfig
+++ b/arch/arm/mach-bcm/Kconfig
@@ -116,6 +116,7 @@ config ARCH_BCM2835
        select CLKSRC_OF
        select PINCTRL
        select PINCTRL_BCM2835
+       select BCM2835_WDT
        help
          This enables support for the Broadcom BCM2835 SoC. This SoC is
          used in the Raspberry Pi and Roku 2 devices.



-- 
Pranith

[PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Pranith Kumar]

On Fri, Apr 24, 2015 at 3:08 PM, Eric Anholt <eric@anholt.net> wrote:
> Since the WDT is what's used to drive restart and power off, it makes
> more sense to keep it there, where the regs are already mapped and
> definitions for them provided.  Note that this means you may need to
> add CONFIG_BCM2835_WDT to retain functionality of your kernel.
>
> Signed-off-by: Eric Anholt <eric@anholt.net>
> Cc: linux-watchdog at vger.kernel.org


Any reason not to select BCM2835_WDT in the config by default then?

Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
---
diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
index 8b11f44..dd4856d 100644
--- a/arch/arm/mach-bcm/Kconfig
+++ b/arch/arm/mach-bcm/Kconfig
@@ -116,6 +116,7 @@ config ARCH_BCM2835
        select CLKSRC_OF
        select PINCTRL
        select PINCTRL_BCM2835
+       select BCM2835_WDT
        help
          This enables support for the Broadcom BCM2835 SoC. This SoC is
          used in the Raspberry Pi and Roku 2 devices.



-- 
Pranith

Re: [PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Stephen Warren]

On 04/24/2015 01:08 PM, Eric Anholt wrote:
> Since the WDT is what's used to drive restart and power off, it makes
> more sense to keep it there, where the regs are already mapped and
> definitions for them provided.  Note that this means you may need to
> add CONFIG_BCM2835_WDT to retain functionality of your kernel.

The series,
Acked-by: Stephen Warren <swarren@wwwdotorg.org>
Tested-by: Stephen Warren <swarren@wwwdotorg.org>

> Note that power off has never worked for me, and just reboots as well.
> So I can't say that I've *really* tested the power off code.

The RPi can't actually power itself off, but it used to be the case that
if you rebooted it after setting up a certain register configuration,
the firmware would put the device into a low-power state. This did work
when it was first upstreamed. However, it no longer works. I believe
this was due to a change in the firmware, which is why I don't always
trust the firmware. I should really check what the downstream kernel
does for power off now; I assume it must have changed since the code was
upstreamed.

[PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Stephen Warren]

On 04/24/2015 01:08 PM, Eric Anholt wrote:
> Since the WDT is what's used to drive restart and power off, it makes
> more sense to keep it there, where the regs are already mapped and
> definitions for them provided.  Note that this means you may need to
> add CONFIG_BCM2835_WDT to retain functionality of your kernel.

The series,
Acked-by: Stephen Warren <swarren@wwwdotorg.org>
Tested-by: Stephen Warren <swarren@wwwdotorg.org>

> Note that power off has never worked for me, and just reboots as well.
> So I can't say that I've *really* tested the power off code.

The RPi can't actually power itself off, but it used to be the case that
if you rebooted it after setting up a certain register configuration,
the firmware would put the device into a low-power state. This did work
when it was first upstreamed. However, it no longer works. I believe
this was due to a change in the firmware, which is why I don't always
trust the firmware. I should really check what the downstream kernel
does for power off now; I assume it must have changed since the code was
upstreamed.

Re: [PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Arnd Bergmann]

On Friday 24 April 2015 12:08:54 Eric Anholt wrote:
> +/*
> + * We can't really power off, but if we do the normal reset scheme, and
> + * indicate to bootcode.bin not to reboot, then most of the chip will be
> + * powered off.
> + */
> +static void bcm2835_power_off(void)
> +{
> +       struct device_node *np =
> +               of_find_compatible_node(NULL, NULL, "brcm,bcm2835-pm-wdt");
> +       struct platform_device *pdev = of_find_device_by_node(np);
> +       struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
> +       u32 val;
> +

Instead of doing the lookup again here, I'd suggest using a static variable
in the driver to store the device pointer for the device used on power_off.

Make sure that the device remove callback assigns it back to NULL though
and that the function checks for NULL pointer.

Aside from this, the patch looks great.

	Arnd

[PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Arnd Bergmann]

On Friday 24 April 2015 12:08:54 Eric Anholt wrote:
> +/*
> + * We can't really power off, but if we do the normal reset scheme, and
> + * indicate to bootcode.bin not to reboot, then most of the chip will be
> + * powered off.
> + */
> +static void bcm2835_power_off(void)
> +{
> +       struct device_node *np =
> +               of_find_compatible_node(NULL, NULL, "brcm,bcm2835-pm-wdt");
> +       struct platform_device *pdev = of_find_device_by_node(np);
> +       struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
> +       u32 val;
> +

Instead of doing the lookup again here, I'd suggest using a static variable
in the driver to store the device pointer for the device used on power_off.

Make sure that the device remove callback assigns it back to NULL though
and that the function checks for NULL pointer.

Aside from this, the patch looks great.

	Arnd

Re: [PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Guenter Roeck]

On 04/25/2015 01:11 PM, Arnd Bergmann wrote:
> On Friday 24 April 2015 12:08:54 Eric Anholt wrote:
>> +/*
>> + * We can't really power off, but if we do the normal reset scheme, and
>> + * indicate to bootcode.bin not to reboot, then most of the chip will be
>> + * powered off.
>> + */
>> +static void bcm2835_power_off(void)
>> +{
>> +       struct device_node *np =
>> +               of_find_compatible_node(NULL, NULL, "brcm,bcm2835-pm-wdt");
>> +       struct platform_device *pdev = of_find_device_by_node(np);
>> +       struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
>> +       u32 val;
>> +
>
> Instead of doing the lookup again here, I'd suggest using a static variable
> in the driver to store the device pointer for the device used on power_off.
>
> Make sure that the device remove callback assigns it back to NULL though
> and that the function checks for NULL pointer.
>

Why would that be needed ?

Guenter

[PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Guenter Roeck]

On 04/25/2015 01:11 PM, Arnd Bergmann wrote:
> On Friday 24 April 2015 12:08:54 Eric Anholt wrote:
>> +/*
>> + * We can't really power off, but if we do the normal reset scheme, and
>> + * indicate to bootcode.bin not to reboot, then most of the chip will be
>> + * powered off.
>> + */
>> +static void bcm2835_power_off(void)
>> +{
>> +       struct device_node *np =
>> +               of_find_compatible_node(NULL, NULL, "brcm,bcm2835-pm-wdt");
>> +       struct platform_device *pdev = of_find_device_by_node(np);
>> +       struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
>> +       u32 val;
>> +
>
> Instead of doing the lookup again here, I'd suggest using a static variable
> in the driver to store the device pointer for the device used on power_off.
>
> Make sure that the device remove callback assigns it back to NULL though
> and that the function checks for NULL pointer.
>

Why would that be needed ?

Guenter

Re: [PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Arnd Bergmann]

On Sunday 26 April 2015 08:35:57 Guenter Roeck wrote:
> On 04/25/2015 01:11 PM, Arnd Bergmann wrote:
> > On Friday 24 April 2015 12:08:54 Eric Anholt wrote:
> >> +/*
> >> + * We can't really power off, but if we do the normal reset scheme, and
> >> + * indicate to bootcode.bin not to reboot, then most of the chip will be
> >> + * powered off.
> >> + */
> >> +static void bcm2835_power_off(void)
> >> +{
> >> +       struct device_node *np =
> >> +               of_find_compatible_node(NULL, NULL, "brcm,bcm2835-pm-wdt");
> >> +       struct platform_device *pdev = of_find_device_by_node(np);
> >> +       struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
> >> +       u32 val;
> >> +
> >
> > Instead of doing the lookup again here, I'd suggest using a static variable
> > in the driver to store the device pointer for the device used on power_off.
> >
> > Make sure that the device remove callback assigns it back to NULL though
> > and that the function checks for NULL pointer.
> >
> 
> Why would that be needed ?

devices can be unbound from drivers through sysfs, or using dynamic DT
updates. If one does that, the watchdog driver will correctly destroy
all information it has about the device, so if the power_off function
still uses that pointer, it will crash.

	Arnd

[PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Arnd Bergmann]

On Sunday 26 April 2015 08:35:57 Guenter Roeck wrote:
> On 04/25/2015 01:11 PM, Arnd Bergmann wrote:
> > On Friday 24 April 2015 12:08:54 Eric Anholt wrote:
> >> +/*
> >> + * We can't really power off, but if we do the normal reset scheme, and
> >> + * indicate to bootcode.bin not to reboot, then most of the chip will be
> >> + * powered off.
> >> + */
> >> +static void bcm2835_power_off(void)
> >> +{
> >> +       struct device_node *np =
> >> +               of_find_compatible_node(NULL, NULL, "brcm,bcm2835-pm-wdt");
> >> +       struct platform_device *pdev = of_find_device_by_node(np);
> >> +       struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
> >> +       u32 val;
> >> +
> >
> > Instead of doing the lookup again here, I'd suggest using a static variable
> > in the driver to store the device pointer for the device used on power_off.
> >
> > Make sure that the device remove callback assigns it back to NULL though
> > and that the function checks for NULL pointer.
> >
> 
> Why would that be needed ?

devices can be unbound from drivers through sysfs, or using dynamic DT
updates. If one does that, the watchdog driver will correctly destroy
all information it has about the device, so if the power_off function
still uses that pointer, it will crash.

	Arnd

Re: [PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Guenter Roeck]

On 04/27/2015 02:18 AM, Arnd Bergmann wrote:
> On Sunday 26 April 2015 08:35:57 Guenter Roeck wrote:
>> On 04/25/2015 01:11 PM, Arnd Bergmann wrote:
>>> On Friday 24 April 2015 12:08:54 Eric Anholt wrote:
>>>> +/*
>>>> + * We can't really power off, but if we do the normal reset scheme, and
>>>> + * indicate to bootcode.bin not to reboot, then most of the chip will be
>>>> + * powered off.
>>>> + */
>>>> +static void bcm2835_power_off(void)
>>>> +{
>>>> +       struct device_node *np =
>>>> +               of_find_compatible_node(NULL, NULL, "brcm,bcm2835-pm-wdt");
>>>> +       struct platform_device *pdev = of_find_device_by_node(np);
>>>> +       struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
>>>> +       u32 val;
>>>> +
>>>
>>> Instead of doing the lookup again here, I'd suggest using a static variable
>>> in the driver to store the device pointer for the device used on power_off.
>>>
>>> Make sure that the device remove callback assigns it back to NULL though
>>> and that the function checks for NULL pointer.
>>>
>>
>> Why would that be needed ?
>
> devices can be unbound from drivers through sysfs, or using dynamic DT
> updates. If one does that, the watchdog driver will correctly destroy
> all information it has about the device, so if the power_off function
> still uses that pointer, it will crash.
>
Without calling its remove function ? That sounds odd. Lots of drivers
would break if that was really the case.

Guenter

[PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Guenter Roeck]

On 04/27/2015 02:18 AM, Arnd Bergmann wrote:
> On Sunday 26 April 2015 08:35:57 Guenter Roeck wrote:
>> On 04/25/2015 01:11 PM, Arnd Bergmann wrote:
>>> On Friday 24 April 2015 12:08:54 Eric Anholt wrote:
>>>> +/*
>>>> + * We can't really power off, but if we do the normal reset scheme, and
>>>> + * indicate to bootcode.bin not to reboot, then most of the chip will be
>>>> + * powered off.
>>>> + */
>>>> +static void bcm2835_power_off(void)
>>>> +{
>>>> +       struct device_node *np =
>>>> +               of_find_compatible_node(NULL, NULL, "brcm,bcm2835-pm-wdt");
>>>> +       struct platform_device *pdev = of_find_device_by_node(np);
>>>> +       struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
>>>> +       u32 val;
>>>> +
>>>
>>> Instead of doing the lookup again here, I'd suggest using a static variable
>>> in the driver to store the device pointer for the device used on power_off.
>>>
>>> Make sure that the device remove callback assigns it back to NULL though
>>> and that the function checks for NULL pointer.
>>>
>>
>> Why would that be needed ?
>
> devices can be unbound from drivers through sysfs, or using dynamic DT
> updates. If one does that, the watchdog driver will correctly destroy
> all information it has about the device, so if the power_off function
> still uses that pointer, it will crash.
>
Without calling its remove function ? That sounds odd. Lots of drivers
would break if that was really the case.

Guenter

Re: [PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Arnd Bergmann]

On Monday 27 April 2015 05:44:14 Guenter Roeck wrote:
> On 04/27/2015 02:18 AM, Arnd Bergmann wrote:
> > On Sunday 26 April 2015 08:35:57 Guenter Roeck wrote:
> >> On 04/25/2015 01:11 PM, Arnd Bergmann wrote:
> >>> On Friday 24 April 2015 12:08:54 Eric Anholt wrote:
> >>>> +/*
> >>>> + * We can't really power off, but if we do the normal reset scheme, and
> >>>> + * indicate to bootcode.bin not to reboot, then most of the chip will be
> >>>> + * powered off.
> >>>> + */
> >>>> +static void bcm2835_power_off(void)
> >>>> +{
> >>>> +       struct device_node *np =
> >>>> +               of_find_compatible_node(NULL, NULL, "brcm,bcm2835-pm-wdt");
> >>>> +       struct platform_device *pdev = of_find_device_by_node(np);
> >>>> +       struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
> >>>> +       u32 val;
> >>>> +
> >>>
> >>> Instead of doing the lookup again here, I'd suggest using a static variable
> >>> in the driver to store the device pointer for the device used on power_off.
> >>>
> >>> Make sure that the device remove callback assigns it back to NULL though
> >>> and that the function checks for NULL pointer.
> >>>
> >>
> >> Why would that be needed ?
> >
> > devices can be unbound from drivers through sysfs, or using dynamic DT
> > updates. If one does that, the watchdog driver will correctly destroy
> > all information it has about the device, so if the power_off function
> > still uses that pointer, it will crash.
> >
> Without calling its remove function ? That sounds odd. Lots of drivers
> would break if that was really the case.

I'm not sure what you mean now. The remove function gets called to
do the final cleanup, and afterwards the device is deleted, so the
remove function should make sure that no global variables point to
the device any more.

This is not a concern for most other drivers, because they do not
keep global pointers to device structures.

	Arnd

[PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Arnd Bergmann]

On Monday 27 April 2015 05:44:14 Guenter Roeck wrote:
> On 04/27/2015 02:18 AM, Arnd Bergmann wrote:
> > On Sunday 26 April 2015 08:35:57 Guenter Roeck wrote:
> >> On 04/25/2015 01:11 PM, Arnd Bergmann wrote:
> >>> On Friday 24 April 2015 12:08:54 Eric Anholt wrote:
> >>>> +/*
> >>>> + * We can't really power off, but if we do the normal reset scheme, and
> >>>> + * indicate to bootcode.bin not to reboot, then most of the chip will be
> >>>> + * powered off.
> >>>> + */
> >>>> +static void bcm2835_power_off(void)
> >>>> +{
> >>>> +       struct device_node *np =
> >>>> +               of_find_compatible_node(NULL, NULL, "brcm,bcm2835-pm-wdt");
> >>>> +       struct platform_device *pdev = of_find_device_by_node(np);
> >>>> +       struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
> >>>> +       u32 val;
> >>>> +
> >>>
> >>> Instead of doing the lookup again here, I'd suggest using a static variable
> >>> in the driver to store the device pointer for the device used on power_off.
> >>>
> >>> Make sure that the device remove callback assigns it back to NULL though
> >>> and that the function checks for NULL pointer.
> >>>
> >>
> >> Why would that be needed ?
> >
> > devices can be unbound from drivers through sysfs, or using dynamic DT
> > updates. If one does that, the watchdog driver will correctly destroy
> > all information it has about the device, so if the power_off function
> > still uses that pointer, it will crash.
> >
> Without calling its remove function ? That sounds odd. Lots of drivers
> would break if that was really the case.

I'm not sure what you mean now. The remove function gets called to
do the final cleanup, and afterwards the device is deleted, so the
remove function should make sure that no global variables point to
the device any more.

This is not a concern for most other drivers, because they do not
keep global pointers to device structures.

	Arnd

Re: [PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Lubomir Rintel]

On Fri, 2015-04-24 at 12:08 -0700, Eric Anholt wrote:
> Since the WDT is what's used to drive restart and power off, it makes
> more sense to keep it there, where the regs are already mapped and
> definitions for them provided.  Note that this means you may need to
> add CONFIG_BCM2835_WDT to retain functionality of your kernel.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> Cc: linux-watchdog@vger.kernel.org

Acked-by: Lubomir Rintel <lkundrak@v3.sk>

[PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Lubomir Rintel]

On Fri, 2015-04-24 at 12:08 -0700, Eric Anholt wrote:
> Since the WDT is what's used to drive restart and power off, it makes
> more sense to keep it there, where the regs are already mapped and
> definitions for them provided.  Note that this means you may need to
> add CONFIG_BCM2835_WDT to retain functionality of your kernel.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> Cc: linux-watchdog at vger.kernel.org

Acked-by: Lubomir Rintel <lkundrak@v3.sk>

Re: [PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Guenter Roeck]

On 04/27/2015 05:48 AM, Arnd Bergmann wrote:
> On Monday 27 April 2015 05:44:14 Guenter Roeck wrote:
>> On 04/27/2015 02:18 AM, Arnd Bergmann wrote:
>>> On Sunday 26 April 2015 08:35:57 Guenter Roeck wrote:
>>>> On 04/25/2015 01:11 PM, Arnd Bergmann wrote:
>>>>> On Friday 24 April 2015 12:08:54 Eric Anholt wrote:
>>>>>> +/*
>>>>>> + * We can't really power off, but if we do the normal reset scheme, and
>>>>>> + * indicate to bootcode.bin not to reboot, then most of the chip will be
>>>>>> + * powered off.
>>>>>> + */
>>>>>> +static void bcm2835_power_off(void)
>>>>>> +{
>>>>>> +       struct device_node *np =
>>>>>> +               of_find_compatible_node(NULL, NULL, "brcm,bcm2835-pm-wdt");
>>>>>> +       struct platform_device *pdev = of_find_device_by_node(np);
>>>>>> +       struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
>>>>>> +       u32 val;
>>>>>> +
>>>>>
>>>>> Instead of doing the lookup again here, I'd suggest using a static variable
>>>>> in the driver to store the device pointer for the device used on power_off.
>>>>>
>>>>> Make sure that the device remove callback assigns it back to NULL though
>>>>> and that the function checks for NULL pointer.
>>>>>
>>>>
>>>> Why would that be needed ?
>>>
>>> devices can be unbound from drivers through sysfs, or using dynamic DT
>>> updates. If one does that, the watchdog driver will correctly destroy
>>> all information it has about the device, so if the power_off function
>>> still uses that pointer, it will crash.
>>>
>> Without calling its remove function ? That sounds odd. Lots of drivers
>> would break if that was really the case.
>
> I'm not sure what you mean now. The remove function gets called to
> do the final cleanup, and afterwards the device is deleted, so the
> remove function should make sure that no global variables point to
> the device any more.
>
> This is not a concern for most other drivers, because they do not
> keep global pointers to device structures.
>

The pointer to the power-off handler is removed in the device remove callback,
and thus won't be called after the device is unbound.

Ultimately this is similar the the current code, which assumes that the
call to of_find_device_by_node(np) never fails.

Guenter

[PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Guenter Roeck]

On 04/27/2015 05:48 AM, Arnd Bergmann wrote:
> On Monday 27 April 2015 05:44:14 Guenter Roeck wrote:
>> On 04/27/2015 02:18 AM, Arnd Bergmann wrote:
>>> On Sunday 26 April 2015 08:35:57 Guenter Roeck wrote:
>>>> On 04/25/2015 01:11 PM, Arnd Bergmann wrote:
>>>>> On Friday 24 April 2015 12:08:54 Eric Anholt wrote:
>>>>>> +/*
>>>>>> + * We can't really power off, but if we do the normal reset scheme, and
>>>>>> + * indicate to bootcode.bin not to reboot, then most of the chip will be
>>>>>> + * powered off.
>>>>>> + */
>>>>>> +static void bcm2835_power_off(void)
>>>>>> +{
>>>>>> +       struct device_node *np =
>>>>>> +               of_find_compatible_node(NULL, NULL, "brcm,bcm2835-pm-wdt");
>>>>>> +       struct platform_device *pdev = of_find_device_by_node(np);
>>>>>> +       struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
>>>>>> +       u32 val;
>>>>>> +
>>>>>
>>>>> Instead of doing the lookup again here, I'd suggest using a static variable
>>>>> in the driver to store the device pointer for the device used on power_off.
>>>>>
>>>>> Make sure that the device remove callback assigns it back to NULL though
>>>>> and that the function checks for NULL pointer.
>>>>>
>>>>
>>>> Why would that be needed ?
>>>
>>> devices can be unbound from drivers through sysfs, or using dynamic DT
>>> updates. If one does that, the watchdog driver will correctly destroy
>>> all information it has about the device, so if the power_off function
>>> still uses that pointer, it will crash.
>>>
>> Without calling its remove function ? That sounds odd. Lots of drivers
>> would break if that was really the case.
>
> I'm not sure what you mean now. The remove function gets called to
> do the final cleanup, and afterwards the device is deleted, so the
> remove function should make sure that no global variables point to
> the device any more.
>
> This is not a concern for most other drivers, because they do not
> keep global pointers to device structures.
>

The pointer to the power-off handler is removed in the device remove callback,
and thus won't be called after the device is unbound.

Ultimately this is similar the the current code, which assumes that the
call to of_find_device_by_node(np) never fails.

Guenter

Re: [PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Arnd Bergmann]

On Monday 27 April 2015 06:28:46 Guenter Roeck wrote:
> >
> 
> The pointer to the power-off handler is removed in the device remove callback,
> and thus won't be called after the device is unbound.
> 
> Ultimately this is similar the the current code, which assumes that the
> call to of_find_device_by_node(np) never fails.
> 

Ok, I missed that part. Yes, that works just as well.

	Arnd

[PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Arnd Bergmann]

On Monday 27 April 2015 06:28:46 Guenter Roeck wrote:
> >
> 
> The pointer to the power-off handler is removed in the device remove callback,
> and thus won't be called after the device is unbound.
> 
> Ultimately this is similar the the current code, which assumes that the
> call to of_find_device_by_node(np) never fails.
> 

Ok, I missed that part. Yes, that works just as well.

	Arnd

Re: [PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Guenter Roeck]

On Fri, Apr 24, 2015 at 12:08:54PM -0700, Eric Anholt wrote:
> Since the WDT is what's used to drive restart and power off, it makes
> more sense to keep it there, where the regs are already mapped and
> definitions for them provided.  Note that this means you may need to
> add CONFIG_BCM2835_WDT to retain functionality of your kernel.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> Cc: linux-watchdog@vger.kernel.org

The patch will require a rebase to v4.0-rc1.

Thanks,
Guenter

[PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Guenter Roeck]

On Fri, Apr 24, 2015 at 12:08:54PM -0700, Eric Anholt wrote:
> Since the WDT is what's used to drive restart and power off, it makes
> more sense to keep it there, where the regs are already mapped and
> definitions for them provided.  Note that this means you may need to
> add CONFIG_BCM2835_WDT to retain functionality of your kernel.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> Cc: linux-watchdog at vger.kernel.org

The patch will require a rebase to v4.0-rc1.

Thanks,
Guenter

Re: [PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Guenter Roeck]

On Mon, Apr 27, 2015 at 09:04:32AM -0700, Guenter Roeck wrote:
> On Fri, Apr 24, 2015 at 12:08:54PM -0700, Eric Anholt wrote:
> > Since the WDT is what's used to drive restart and power off, it makes
> > more sense to keep it there, where the regs are already mapped and
> > definitions for them provided.  Note that this means you may need to
> > add CONFIG_BCM2835_WDT to retain functionality of your kernel.
> > 
> > Signed-off-by: Eric Anholt <eric@anholt.net>
> > Cc: linux-watchdog@vger.kernel.org
> 
> The patch will require a rebase to v4.0-rc1.
> 
s/4.0/4.1/

Guenter

[PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Guenter Roeck]

On Mon, Apr 27, 2015 at 09:04:32AM -0700, Guenter Roeck wrote:
> On Fri, Apr 24, 2015 at 12:08:54PM -0700, Eric Anholt wrote:
> > Since the WDT is what's used to drive restart and power off, it makes
> > more sense to keep it there, where the regs are already mapped and
> > definitions for them provided.  Note that this means you may need to
> > add CONFIG_BCM2835_WDT to retain functionality of your kernel.
> > 
> > Signed-off-by: Eric Anholt <eric@anholt.net>
> > Cc: linux-watchdog at vger.kernel.org
> 
> The patch will require a rebase to v4.0-rc1.
> 
s/4.0/4.1/

Guenter

Re: [PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Lee Jones]

On Fri, 24 Apr 2015, Eric Anholt wrote:
> Since the WDT is what's used to drive restart and power off, it makes
> more sense to keep it there, where the regs are already mapped and
> definitions for them provided.  Note that this means you may need to
> add CONFIG_BCM2835_WDT to retain functionality of your kernel.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> Cc: linux-watchdog@vger.kernel.org
> ---
> 
> Note that power off has never worked for me, and just reboots as well.
> So I can't say that I've *really* tested the power off code.
> 
>  arch/arm/mach-bcm/board_bcm2835.c | 73 ---------------------------------------
>  drivers/watchdog/bcm2835_wdt.c    | 62 +++++++++++++++++++++++++++++++++
>  2 files changed, 62 insertions(+), 73 deletions(-)

Please rebase and resend, just this patch, as requested by Guenter.

I'm going to take the other patch.

> diff --git a/arch/arm/mach-bcm/board_bcm2835.c b/arch/arm/mach-bcm/board_bcm2835.c
> index 49dd5b0..0f7b9ea 100644
> --- a/arch/arm/mach-bcm/board_bcm2835.c
> +++ b/arch/arm/mach-bcm/board_bcm2835.c
> @@ -12,7 +12,6 @@
>   * GNU General Public License for more details.
>   */
>  
> -#include <linux/delay.h>
>  #include <linux/init.h>
>  #include <linux/irqchip.h>
>  #include <linux/of_address.h>
> @@ -22,81 +21,10 @@
>  #include <asm/mach/arch.h>
>  #include <asm/mach/map.h>
>  
> -#define PM_RSTC				0x1c
> -#define PM_RSTS				0x20
> -#define PM_WDOG				0x24
> -
> -#define PM_PASSWORD			0x5a000000
> -#define PM_RSTC_WRCFG_MASK		0x00000030
> -#define PM_RSTC_WRCFG_FULL_RESET	0x00000020
> -#define PM_RSTS_HADWRH_SET		0x00000040
> -
> -static void __iomem *wdt_regs;
> -
> -/*
> - * The machine restart method can be called from an atomic context so we won't
> - * be able to ioremap the regs then.
> - */
> -static void bcm2835_setup_restart(void)
> -{
> -	struct device_node *np = of_find_compatible_node(NULL, NULL,
> -						"brcm,bcm2835-pm-wdt");
> -	if (WARN(!np, "unable to setup watchdog restart"))
> -		return;
> -
> -	wdt_regs = of_iomap(np, 0);
> -	WARN(!wdt_regs, "failed to remap watchdog regs");
> -}
> -
> -static void bcm2835_restart(enum reboot_mode mode, const char *cmd)
> -{
> -	u32 val;
> -
> -	if (!wdt_regs)
> -		return;
> -
> -	/* use a timeout of 10 ticks (~150us) */
> -	writel_relaxed(10 | PM_PASSWORD, wdt_regs + PM_WDOG);
> -	val = readl_relaxed(wdt_regs + PM_RSTC);
> -	val &= ~PM_RSTC_WRCFG_MASK;
> -	val |= PM_PASSWORD | PM_RSTC_WRCFG_FULL_RESET;
> -	writel_relaxed(val, wdt_regs + PM_RSTC);
> -
> -	/* No sleeping, possibly atomic. */
> -	mdelay(1);
> -}
> -
> -/*
> - * We can't really power off, but if we do the normal reset scheme, and
> - * indicate to bootcode.bin not to reboot, then most of the chip will be
> - * powered off.
> - */
> -static void bcm2835_power_off(void)
> -{
> -	u32 val;
> -
> -	/*
> -	 * We set the watchdog hard reset bit here to distinguish this reset
> -	 * from the normal (full) reset. bootcode.bin will not reboot after a
> -	 * hard reset.
> -	 */
> -	val = readl_relaxed(wdt_regs + PM_RSTS);
> -	val &= ~PM_RSTC_WRCFG_MASK;
> -	val |= PM_PASSWORD | PM_RSTS_HADWRH_SET;
> -	writel_relaxed(val, wdt_regs + PM_RSTS);
> -
> -	/* Continue with normal reset mechanism */
> -	bcm2835_restart(REBOOT_HARD, "");
> -}
> -
>  static void __init bcm2835_init(void)
>  {
>  	int ret;
>  
> -	bcm2835_setup_restart();
> -	if (wdt_regs)
> -		pm_power_off = bcm2835_power_off;
> -
>  	bcm2835_init_clocks();
>  
>  	ret = of_platform_populate(NULL, of_default_bus_match_table, NULL,
> @@ -114,6 +42,5 @@ static const char * const bcm2835_compat[] = {
>  
>  DT_MACHINE_START(BCM2835, "BCM2835")
>  	.init_machine = bcm2835_init,
> -	.restart = bcm2835_restart,
>  	.dt_compat = bcm2835_compat
>  MACHINE_END
> diff --git a/drivers/watchdog/bcm2835_wdt.c b/drivers/watchdog/bcm2835_wdt.c
> index 2b5a9bb..7116968 100644
> --- a/drivers/watchdog/bcm2835_wdt.c
> +++ b/drivers/watchdog/bcm2835_wdt.c
> @@ -13,20 +13,25 @@
>   * option) any later version.
>   */
>  
> +#include <linux/delay.h>
> +#include <linux/reboot.h>
>  #include <linux/types.h>
>  #include <linux/module.h>
>  #include <linux/io.h>
>  #include <linux/watchdog.h>
>  #include <linux/platform_device.h>
>  #include <linux/of_address.h>
> +#include <linux/of_platform.h>
>  
>  #define PM_RSTC				0x1c
> +#define PM_RSTS				0x20
>  #define PM_WDOG				0x24
>  
>  #define PM_PASSWORD			0x5a000000
>  
>  #define PM_WDOG_TIME_SET		0x000fffff
>  #define PM_RSTC_WRCFG_CLR		0xffffffcf
> +#define PM_RSTS_HADWRH_SET		0x00000040
>  #define PM_RSTC_WRCFG_SET		0x00000030
>  #define PM_RSTC_WRCFG_FULL_RESET	0x00000020
>  #define PM_RSTC_RESET			0x00000102
> @@ -37,6 +42,7 @@
>  struct bcm2835_wdt {
>  	void __iomem		*base;
>  	spinlock_t		lock;
> +	struct notifier_block	restart_handler;
>  };
>  
>  static unsigned int heartbeat;
> @@ -106,6 +112,53 @@ static struct watchdog_device bcm2835_wdt_wdd = {
>  	.timeout =	WDOG_TICKS_TO_SECS(PM_WDOG_TIME_SET),
>  };
>  
> +static int
> +bcm2835_restart(struct notifier_block *this, unsigned long mode, void *cmd)
> +{
> +	struct bcm2835_wdt *wdt = container_of(this, struct bcm2835_wdt,
> +					       restart_handler);
> +	u32 val;
> +
> +	/* use a timeout of 10 ticks (~150us) */
> +	writel_relaxed(10 | PM_PASSWORD, wdt->base + PM_WDOG);
> +	val = readl_relaxed(wdt->base + PM_RSTC);
> +	val &= PM_RSTC_WRCFG_CLR;
> +	val |= PM_PASSWORD | PM_RSTC_WRCFG_FULL_RESET;
> +	writel_relaxed(val, wdt->base + PM_RSTC);
> +
> +	/* No sleeping, possibly atomic. */
> +	mdelay(1);
> +
> +	return 0;
> +}
> +
> +/*
> + * We can't really power off, but if we do the normal reset scheme, and
> + * indicate to bootcode.bin not to reboot, then most of the chip will be
> + * powered off.
> + */
> +static void bcm2835_power_off(void)
> +{
> +	struct device_node *np =
> +		of_find_compatible_node(NULL, NULL, "brcm,bcm2835-pm-wdt");
> +	struct platform_device *pdev = of_find_device_by_node(np);
> +	struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
> +	u32 val;
> +
> +	/*
> +	 * We set the watchdog hard reset bit here to distinguish this reset
> +	 * from the normal (full) reset. bootcode.bin will not reboot after a
> +	 * hard reset.
> +	 */
> +	val = readl_relaxed(wdt->base + PM_RSTS);
> +	val &= PM_RSTC_WRCFG_CLR;
> +	val |= PM_PASSWORD | PM_RSTS_HADWRH_SET;
> +	writel_relaxed(val, wdt->base + PM_RSTS);
> +
> +	/* Continue with normal reset mechanism */
> +	bcm2835_restart(&wdt->restart_handler, REBOOT_HARD, NULL);
> +}
> +
>  static int bcm2835_wdt_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -136,6 +189,12 @@ static int bcm2835_wdt_probe(struct platform_device *pdev)
>  		return err;
>  	}
>  
> +	wdt->restart_handler.notifier_call = bcm2835_restart;
> +	wdt->restart_handler.priority = 128;
> +	register_restart_handler(&wdt->restart_handler);
> +	if (pm_power_off == NULL)
> +		pm_power_off = bcm2835_power_off;
> +
>  	dev_info(dev, "Broadcom BCM2835 watchdog timer");
>  	return 0;
>  }
> @@ -144,6 +203,9 @@ static int bcm2835_wdt_remove(struct platform_device *pdev)
>  {
>  	struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
>  
> +	unregister_restart_handler(&wdt->restart_handler);
> +	if (pm_power_off == bcm2835_power_off)
> +		pm_power_off = NULL;
>  	watchdog_unregister_device(&bcm2835_wdt_wdd);
>  	iounmap(wdt->base);
>  

[PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Lee Jones]

On Fri, 24 Apr 2015, Eric Anholt wrote:
> Since the WDT is what's used to drive restart and power off, it makes
> more sense to keep it there, where the regs are already mapped and
> definitions for them provided.  Note that this means you may need to
> add CONFIG_BCM2835_WDT to retain functionality of your kernel.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> Cc: linux-watchdog at vger.kernel.org
> ---
> 
> Note that power off has never worked for me, and just reboots as well.
> So I can't say that I've *really* tested the power off code.
> 
>  arch/arm/mach-bcm/board_bcm2835.c | 73 ---------------------------------------
>  drivers/watchdog/bcm2835_wdt.c    | 62 +++++++++++++++++++++++++++++++++
>  2 files changed, 62 insertions(+), 73 deletions(-)

Please rebase and resend, just this patch, as requested by Guenter.

I'm going to take the other patch.

> diff --git a/arch/arm/mach-bcm/board_bcm2835.c b/arch/arm/mach-bcm/board_bcm2835.c
> index 49dd5b0..0f7b9ea 100644
> --- a/arch/arm/mach-bcm/board_bcm2835.c
> +++ b/arch/arm/mach-bcm/board_bcm2835.c
> @@ -12,7 +12,6 @@
>   * GNU General Public License for more details.
>   */
>  
> -#include <linux/delay.h>
>  #include <linux/init.h>
>  #include <linux/irqchip.h>
>  #include <linux/of_address.h>
> @@ -22,81 +21,10 @@
>  #include <asm/mach/arch.h>
>  #include <asm/mach/map.h>
>  
> -#define PM_RSTC				0x1c
> -#define PM_RSTS				0x20
> -#define PM_WDOG				0x24
> -
> -#define PM_PASSWORD			0x5a000000
> -#define PM_RSTC_WRCFG_MASK		0x00000030
> -#define PM_RSTC_WRCFG_FULL_RESET	0x00000020
> -#define PM_RSTS_HADWRH_SET		0x00000040
> -
> -static void __iomem *wdt_regs;
> -
> -/*
> - * The machine restart method can be called from an atomic context so we won't
> - * be able to ioremap the regs then.
> - */
> -static void bcm2835_setup_restart(void)
> -{
> -	struct device_node *np = of_find_compatible_node(NULL, NULL,
> -						"brcm,bcm2835-pm-wdt");
> -	if (WARN(!np, "unable to setup watchdog restart"))
> -		return;
> -
> -	wdt_regs = of_iomap(np, 0);
> -	WARN(!wdt_regs, "failed to remap watchdog regs");
> -}
> -
> -static void bcm2835_restart(enum reboot_mode mode, const char *cmd)
> -{
> -	u32 val;
> -
> -	if (!wdt_regs)
> -		return;
> -
> -	/* use a timeout of 10 ticks (~150us) */
> -	writel_relaxed(10 | PM_PASSWORD, wdt_regs + PM_WDOG);
> -	val = readl_relaxed(wdt_regs + PM_RSTC);
> -	val &= ~PM_RSTC_WRCFG_MASK;
> -	val |= PM_PASSWORD | PM_RSTC_WRCFG_FULL_RESET;
> -	writel_relaxed(val, wdt_regs + PM_RSTC);
> -
> -	/* No sleeping, possibly atomic. */
> -	mdelay(1);
> -}
> -
> -/*
> - * We can't really power off, but if we do the normal reset scheme, and
> - * indicate to bootcode.bin not to reboot, then most of the chip will be
> - * powered off.
> - */
> -static void bcm2835_power_off(void)
> -{
> -	u32 val;
> -
> -	/*
> -	 * We set the watchdog hard reset bit here to distinguish this reset
> -	 * from the normal (full) reset. bootcode.bin will not reboot after a
> -	 * hard reset.
> -	 */
> -	val = readl_relaxed(wdt_regs + PM_RSTS);
> -	val &= ~PM_RSTC_WRCFG_MASK;
> -	val |= PM_PASSWORD | PM_RSTS_HADWRH_SET;
> -	writel_relaxed(val, wdt_regs + PM_RSTS);
> -
> -	/* Continue with normal reset mechanism */
> -	bcm2835_restart(REBOOT_HARD, "");
> -}
> -
>  static void __init bcm2835_init(void)
>  {
>  	int ret;
>  
> -	bcm2835_setup_restart();
> -	if (wdt_regs)
> -		pm_power_off = bcm2835_power_off;
> -
>  	bcm2835_init_clocks();
>  
>  	ret = of_platform_populate(NULL, of_default_bus_match_table, NULL,
> @@ -114,6 +42,5 @@ static const char * const bcm2835_compat[] = {
>  
>  DT_MACHINE_START(BCM2835, "BCM2835")
>  	.init_machine = bcm2835_init,
> -	.restart = bcm2835_restart,
>  	.dt_compat = bcm2835_compat
>  MACHINE_END
> diff --git a/drivers/watchdog/bcm2835_wdt.c b/drivers/watchdog/bcm2835_wdt.c
> index 2b5a9bb..7116968 100644
> --- a/drivers/watchdog/bcm2835_wdt.c
> +++ b/drivers/watchdog/bcm2835_wdt.c
> @@ -13,20 +13,25 @@
>   * option) any later version.
>   */
>  
> +#include <linux/delay.h>
> +#include <linux/reboot.h>
>  #include <linux/types.h>
>  #include <linux/module.h>
>  #include <linux/io.h>
>  #include <linux/watchdog.h>
>  #include <linux/platform_device.h>
>  #include <linux/of_address.h>
> +#include <linux/of_platform.h>
>  
>  #define PM_RSTC				0x1c
> +#define PM_RSTS				0x20
>  #define PM_WDOG				0x24
>  
>  #define PM_PASSWORD			0x5a000000
>  
>  #define PM_WDOG_TIME_SET		0x000fffff
>  #define PM_RSTC_WRCFG_CLR		0xffffffcf
> +#define PM_RSTS_HADWRH_SET		0x00000040
>  #define PM_RSTC_WRCFG_SET		0x00000030
>  #define PM_RSTC_WRCFG_FULL_RESET	0x00000020
>  #define PM_RSTC_RESET			0x00000102
> @@ -37,6 +42,7 @@
>  struct bcm2835_wdt {
>  	void __iomem		*base;
>  	spinlock_t		lock;
> +	struct notifier_block	restart_handler;
>  };
>  
>  static unsigned int heartbeat;
> @@ -106,6 +112,53 @@ static struct watchdog_device bcm2835_wdt_wdd = {
>  	.timeout =	WDOG_TICKS_TO_SECS(PM_WDOG_TIME_SET),
>  };
>  
> +static int
> +bcm2835_restart(struct notifier_block *this, unsigned long mode, void *cmd)
> +{
> +	struct bcm2835_wdt *wdt = container_of(this, struct bcm2835_wdt,
> +					       restart_handler);
> +	u32 val;
> +
> +	/* use a timeout of 10 ticks (~150us) */
> +	writel_relaxed(10 | PM_PASSWORD, wdt->base + PM_WDOG);
> +	val = readl_relaxed(wdt->base + PM_RSTC);
> +	val &= PM_RSTC_WRCFG_CLR;
> +	val |= PM_PASSWORD | PM_RSTC_WRCFG_FULL_RESET;
> +	writel_relaxed(val, wdt->base + PM_RSTC);
> +
> +	/* No sleeping, possibly atomic. */
> +	mdelay(1);
> +
> +	return 0;
> +}
> +
> +/*
> + * We can't really power off, but if we do the normal reset scheme, and
> + * indicate to bootcode.bin not to reboot, then most of the chip will be
> + * powered off.
> + */
> +static void bcm2835_power_off(void)
> +{
> +	struct device_node *np =
> +		of_find_compatible_node(NULL, NULL, "brcm,bcm2835-pm-wdt");
> +	struct platform_device *pdev = of_find_device_by_node(np);
> +	struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
> +	u32 val;
> +
> +	/*
> +	 * We set the watchdog hard reset bit here to distinguish this reset
> +	 * from the normal (full) reset. bootcode.bin will not reboot after a
> +	 * hard reset.
> +	 */
> +	val = readl_relaxed(wdt->base + PM_RSTS);
> +	val &= PM_RSTC_WRCFG_CLR;
> +	val |= PM_PASSWORD | PM_RSTS_HADWRH_SET;
> +	writel_relaxed(val, wdt->base + PM_RSTS);
> +
> +	/* Continue with normal reset mechanism */
> +	bcm2835_restart(&wdt->restart_handler, REBOOT_HARD, NULL);
> +}
> +
>  static int bcm2835_wdt_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -136,6 +189,12 @@ static int bcm2835_wdt_probe(struct platform_device *pdev)
>  		return err;
>  	}
>  
> +	wdt->restart_handler.notifier_call = bcm2835_restart;
> +	wdt->restart_handler.priority = 128;
> +	register_restart_handler(&wdt->restart_handler);
> +	if (pm_power_off == NULL)
> +		pm_power_off = bcm2835_power_off;
> +
>  	dev_info(dev, "Broadcom BCM2835 watchdog timer");
>  	return 0;
>  }
> @@ -144,6 +203,9 @@ static int bcm2835_wdt_remove(struct platform_device *pdev)
>  {
>  	struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
>  
> +	unregister_restart_handler(&wdt->restart_handler);
> +	if (pm_power_off == bcm2835_power_off)
> +		pm_power_off = NULL;
>  	watchdog_unregister_device(&bcm2835_wdt_wdd);
>  	iounmap(wdt->base);
>  

[PATCH 1/2] ARM: BCM2835: Drop the init_irq() hook.

[Lee Jones]

On Fri, 24 Apr 2015, Eric Anholt wrote:

> This is the default function that gets called if the hook is NULL.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
>  arch/arm/mach-bcm/board_bcm2835.c | 1 -
>  1 file changed, 1 deletion(-)

Applied with Lubomir and Stephen's Ack.

> diff --git a/arch/arm/mach-bcm/board_bcm2835.c b/arch/arm/mach-bcm/board_bcm2835.c
> index 9851ee8..49dd5b0 100644
> --- a/arch/arm/mach-bcm/board_bcm2835.c
> +++ b/arch/arm/mach-bcm/board_bcm2835.c
> @@ -113,7 +113,6 @@ static const char * const bcm2835_compat[] = {
>  };
>  
>  DT_MACHINE_START(BCM2835, "BCM2835")
> -	.init_irq = irqchip_init,
>  	.init_machine = bcm2835_init,
>  	.restart = bcm2835_restart,
>  	.dt_compat = bcm2835_compat

Re: [PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 761 bytes --]

Guenter Roeck <linux@roeck-us.net> writes:

> On Mon, Apr 27, 2015 at 09:04:32AM -0700, Guenter Roeck wrote:
>> On Fri, Apr 24, 2015 at 12:08:54PM -0700, Eric Anholt wrote:
>> > Since the WDT is what's used to drive restart and power off, it makes
>> > more sense to keep it there, where the regs are already mapped and
>> > definitions for them provided.  Note that this means you may need to
>> > add CONFIG_BCM2835_WDT to retain functionality of your kernel.
>> > 
>> > Signed-off-by: Eric Anholt <eric@anholt.net>
>> > Cc: linux-watchdog@vger.kernel.org
>> 
>> The patch will require a rebase to v4.0-rc1.
>> 
> s/4.0/4.1/

It will?  It seems to work fine, and the diff's the same.  Is there some
new thing you were expecting in a rebase?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

[PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Eric Anholt]

Guenter Roeck <linux@roeck-us.net> writes:

> On Mon, Apr 27, 2015 at 09:04:32AM -0700, Guenter Roeck wrote:
>> On Fri, Apr 24, 2015 at 12:08:54PM -0700, Eric Anholt wrote:
>> > Since the WDT is what's used to drive restart and power off, it makes
>> > more sense to keep it there, where the regs are already mapped and
>> > definitions for them provided.  Note that this means you may need to
>> > add CONFIG_BCM2835_WDT to retain functionality of your kernel.
>> > 
>> > Signed-off-by: Eric Anholt <eric@anholt.net>
>> > Cc: linux-watchdog at vger.kernel.org
>> 
>> The patch will require a rebase to v4.0-rc1.
>> 
> s/4.0/4.1/

It will?  It seems to work fine, and the diff's the same.  Is there some
new thing you were expecting in a rebase?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150427/16bb2623/attachment.sig>

Re: [PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Guenter Roeck]

On 04/27/2015 04:12 PM, Eric Anholt wrote:
> Guenter Roeck <linux@roeck-us.net> writes:
>
>> On Mon, Apr 27, 2015 at 09:04:32AM -0700, Guenter Roeck wrote:
>>> On Fri, Apr 24, 2015 at 12:08:54PM -0700, Eric Anholt wrote:
>>>> Since the WDT is what's used to drive restart and power off, it makes
>>>> more sense to keep it there, where the regs are already mapped and
>>>> definitions for them provided.  Note that this means you may need to
>>>> add CONFIG_BCM2835_WDT to retain functionality of your kernel.
>>>>
>>>> Signed-off-by: Eric Anholt <eric@anholt.net>
>>>> Cc: linux-watchdog@vger.kernel.org
>>>
>>> The patch will require a rebase to v4.0-rc1.
>>>
>> s/4.0/4.1/
>
> It will?  It seems to work fine, and the diff's the same.  Is there some
> new thing you were expecting in a rebase?
>
All I can say is that it didn't apply for me. Maybe someone else has more luck.

Guenter

[PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Guenter Roeck]

On 04/27/2015 04:12 PM, Eric Anholt wrote:
> Guenter Roeck <linux@roeck-us.net> writes:
>
>> On Mon, Apr 27, 2015 at 09:04:32AM -0700, Guenter Roeck wrote:
>>> On Fri, Apr 24, 2015 at 12:08:54PM -0700, Eric Anholt wrote:
>>>> Since the WDT is what's used to drive restart and power off, it makes
>>>> more sense to keep it there, where the regs are already mapped and
>>>> definitions for them provided.  Note that this means you may need to
>>>> add CONFIG_BCM2835_WDT to retain functionality of your kernel.
>>>>
>>>> Signed-off-by: Eric Anholt <eric@anholt.net>
>>>> Cc: linux-watchdog at vger.kernel.org
>>>
>>> The patch will require a rebase to v4.0-rc1.
>>>
>> s/4.0/4.1/
>
> It will?  It seems to work fine, and the diff's the same.  Is there some
> new thing you were expecting in a rebase?
>
All I can say is that it didn't apply for me. Maybe someone else has more luck.

Guenter

Re: [PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Lee Jones]

On Mon, 27 Apr 2015, Guenter Roeck wrote:

> On 04/27/2015 04:12 PM, Eric Anholt wrote:
> >Guenter Roeck <linux@roeck-us.net> writes:
> >
> >>On Mon, Apr 27, 2015 at 09:04:32AM -0700, Guenter Roeck wrote:
> >>>On Fri, Apr 24, 2015 at 12:08:54PM -0700, Eric Anholt wrote:
> >>>>Since the WDT is what's used to drive restart and power off, it makes
> >>>>more sense to keep it there, where the regs are already mapped and
> >>>>definitions for them provided.  Note that this means you may need to
> >>>>add CONFIG_BCM2835_WDT to retain functionality of your kernel.
> >>>>
> >>>>Signed-off-by: Eric Anholt <eric@anholt.net>
> >>>>Cc: linux-watchdog@vger.kernel.org
> >>>
> >>>The patch will require a rebase to v4.0-rc1.
> >>>
> >>s/4.0/4.1/
> >
> >It will?  It seems to work fine, and the diff's the same.  Is there some
> >new thing you were expecting in a rebase?
> >
> All I can say is that it didn't apply for me. Maybe someone else has more luck.

I won't even try without a WDT Ack. ;)

[PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Lee Jones]

On Mon, 27 Apr 2015, Guenter Roeck wrote:

> On 04/27/2015 04:12 PM, Eric Anholt wrote:
> >Guenter Roeck <linux@roeck-us.net> writes:
> >
> >>On Mon, Apr 27, 2015 at 09:04:32AM -0700, Guenter Roeck wrote:
> >>>On Fri, Apr 24, 2015 at 12:08:54PM -0700, Eric Anholt wrote:
> >>>>Since the WDT is what's used to drive restart and power off, it makes
> >>>>more sense to keep it there, where the regs are already mapped and
> >>>>definitions for them provided.  Note that this means you may need to
> >>>>add CONFIG_BCM2835_WDT to retain functionality of your kernel.
> >>>>
> >>>>Signed-off-by: Eric Anholt <eric@anholt.net>
> >>>>Cc: linux-watchdog at vger.kernel.org
> >>>
> >>>The patch will require a rebase to v4.0-rc1.
> >>>
> >>s/4.0/4.1/
> >
> >It will?  It seems to work fine, and the diff's the same.  Is there some
> >new thing you were expecting in a rebase?
> >
> All I can say is that it didn't apply for me. Maybe someone else has more luck.

I won't even try without a WDT Ack. ;)

Re: [PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 1229 bytes --]

Guenter Roeck <linux@roeck-us.net> writes:

> On 04/27/2015 04:12 PM, Eric Anholt wrote:
>> Guenter Roeck <linux@roeck-us.net> writes:
>>
>>> On Mon, Apr 27, 2015 at 09:04:32AM -0700, Guenter Roeck wrote:
>>>> On Fri, Apr 24, 2015 at 12:08:54PM -0700, Eric Anholt wrote:
>>>>> Since the WDT is what's used to drive restart and power off, it makes
>>>>> more sense to keep it there, where the regs are already mapped and
>>>>> definitions for them provided.  Note that this means you may need to
>>>>> add CONFIG_BCM2835_WDT to retain functionality of your kernel.
>>>>>
>>>>> Signed-off-by: Eric Anholt <eric@anholt.net>
>>>>> Cc: linux-watchdog@vger.kernel.org
>>>>
>>>> The patch will require a rebase to v4.0-rc1.
>>>>
>>> s/4.0/4.1/
>>
>> It will?  It seems to work fine, and the diff's the same.  Is there some
>> new thing you were expecting in a rebase?
>>
> All I can say is that it didn't apply for me. Maybe someone else has more luck.

If you were trying to apply it to stock 4.1-rc1, it would fail.  It was
based on the for-rpi-next tree, since that's where I assume it would go
in through.

If you'd like to take it through your tree I could send you that one,
but it'll mean conflicts for someone to resolve later.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

[PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Eric Anholt]

Guenter Roeck <linux@roeck-us.net> writes:

> On 04/27/2015 04:12 PM, Eric Anholt wrote:
>> Guenter Roeck <linux@roeck-us.net> writes:
>>
>>> On Mon, Apr 27, 2015 at 09:04:32AM -0700, Guenter Roeck wrote:
>>>> On Fri, Apr 24, 2015 at 12:08:54PM -0700, Eric Anholt wrote:
>>>>> Since the WDT is what's used to drive restart and power off, it makes
>>>>> more sense to keep it there, where the regs are already mapped and
>>>>> definitions for them provided.  Note that this means you may need to
>>>>> add CONFIG_BCM2835_WDT to retain functionality of your kernel.
>>>>>
>>>>> Signed-off-by: Eric Anholt <eric@anholt.net>
>>>>> Cc: linux-watchdog at vger.kernel.org
>>>>
>>>> The patch will require a rebase to v4.0-rc1.
>>>>
>>> s/4.0/4.1/
>>
>> It will?  It seems to work fine, and the diff's the same.  Is there some
>> new thing you were expecting in a rebase?
>>
> All I can say is that it didn't apply for me. Maybe someone else has more luck.

If you were trying to apply it to stock 4.1-rc1, it would fail.  It was
based on the for-rpi-next tree, since that's where I assume it would go
in through.

If you'd like to take it through your tree I could send you that one,
but it'll mean conflicts for someone to resolve later.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150428/ed9d0a7d/attachment.sig>

Re: [2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Guenter Roeck]

On Fri, Apr 24, 2015 at 12:08:54PM -0700, Eric Anholt wrote:
> Since the WDT is what's used to drive restart and power off, it makes
> more sense to keep it there, where the regs are already mapped and
> definitions for them provided.  Note that this means you may need to
> add CONFIG_BCM2835_WDT to retain functionality of your kernel.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> Cc: linux-watchdog@vger.kernel.org
> Acked-by: Stephen Warren <swarren@wwwdotorg.org>
> Tested-by: Stephen Warren <swarren@wwwdotorg.org>
> Acked-by: Lubomir Rintel <lkundrak@v3.sk>

Acked-by: Guenter Roeck <linux@roeck-us.net>

[2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Guenter Roeck]

On Fri, Apr 24, 2015 at 12:08:54PM -0700, Eric Anholt wrote:
> Since the WDT is what's used to drive restart and power off, it makes
> more sense to keep it there, where the regs are already mapped and
> definitions for them provided.  Note that this means you may need to
> add CONFIG_BCM2835_WDT to retain functionality of your kernel.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> Cc: linux-watchdog at vger.kernel.org
> Acked-by: Stephen Warren <swarren@wwwdotorg.org>
> Tested-by: Stephen Warren <swarren@wwwdotorg.org>
> Acked-by: Lubomir Rintel <lkundrak@v3.sk>

Acked-by: Guenter Roeck <linux@roeck-us.net>

Re: [PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Lee Jones]

On Fri, 24 Apr 2015, Eric Anholt wrote:

> Since the WDT is what's used to drive restart and power off, it makes
> more sense to keep it there, where the regs are already mapped and
> definitions for them provided.  Note that this means you may need to
> add CONFIG_BCM2835_WDT to retain functionality of your kernel.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> Cc: linux-watchdog@vger.kernel.org
> ---
> 
> Note that power off has never worked for me, and just reboots as well.
> So I can't say that I've *really* tested the power off code.
> 
>  arch/arm/mach-bcm/board_bcm2835.c | 73 ---------------------------------------
>  drivers/watchdog/bcm2835_wdt.c    | 62 +++++++++++++++++++++++++++++++++
>  2 files changed, 62 insertions(+), 73 deletions(-)

Applied with Guenter's Ack.

> diff --git a/arch/arm/mach-bcm/board_bcm2835.c b/arch/arm/mach-bcm/board_bcm2835.c
> index 49dd5b0..0f7b9ea 100644
> --- a/arch/arm/mach-bcm/board_bcm2835.c
> +++ b/arch/arm/mach-bcm/board_bcm2835.c
> @@ -12,7 +12,6 @@
>   * GNU General Public License for more details.
>   */
>  
> -#include <linux/delay.h>
>  #include <linux/init.h>
>  #include <linux/irqchip.h>
>  #include <linux/of_address.h>
> @@ -22,81 +21,10 @@
>  #include <asm/mach/arch.h>
>  #include <asm/mach/map.h>
>  
> -#define PM_RSTC				0x1c
> -#define PM_RSTS				0x20
> -#define PM_WDOG				0x24
> -
> -#define PM_PASSWORD			0x5a000000
> -#define PM_RSTC_WRCFG_MASK		0x00000030
> -#define PM_RSTC_WRCFG_FULL_RESET	0x00000020
> -#define PM_RSTS_HADWRH_SET		0x00000040
> -
> -static void __iomem *wdt_regs;
> -
> -/*
> - * The machine restart method can be called from an atomic context so we won't
> - * be able to ioremap the regs then.
> - */
> -static void bcm2835_setup_restart(void)
> -{
> -	struct device_node *np = of_find_compatible_node(NULL, NULL,
> -						"brcm,bcm2835-pm-wdt");
> -	if (WARN(!np, "unable to setup watchdog restart"))
> -		return;
> -
> -	wdt_regs = of_iomap(np, 0);
> -	WARN(!wdt_regs, "failed to remap watchdog regs");
> -}
> -
> -static void bcm2835_restart(enum reboot_mode mode, const char *cmd)
> -{
> -	u32 val;
> -
> -	if (!wdt_regs)
> -		return;
> -
> -	/* use a timeout of 10 ticks (~150us) */
> -	writel_relaxed(10 | PM_PASSWORD, wdt_regs + PM_WDOG);
> -	val = readl_relaxed(wdt_regs + PM_RSTC);
> -	val &= ~PM_RSTC_WRCFG_MASK;
> -	val |= PM_PASSWORD | PM_RSTC_WRCFG_FULL_RESET;
> -	writel_relaxed(val, wdt_regs + PM_RSTC);
> -
> -	/* No sleeping, possibly atomic. */
> -	mdelay(1);
> -}
> -
> -/*
> - * We can't really power off, but if we do the normal reset scheme, and
> - * indicate to bootcode.bin not to reboot, then most of the chip will be
> - * powered off.
> - */
> -static void bcm2835_power_off(void)
> -{
> -	u32 val;
> -
> -	/*
> -	 * We set the watchdog hard reset bit here to distinguish this reset
> -	 * from the normal (full) reset. bootcode.bin will not reboot after a
> -	 * hard reset.
> -	 */
> -	val = readl_relaxed(wdt_regs + PM_RSTS);
> -	val &= ~PM_RSTC_WRCFG_MASK;
> -	val |= PM_PASSWORD | PM_RSTS_HADWRH_SET;
> -	writel_relaxed(val, wdt_regs + PM_RSTS);
> -
> -	/* Continue with normal reset mechanism */
> -	bcm2835_restart(REBOOT_HARD, "");
> -}
> -
>  static void __init bcm2835_init(void)
>  {
>  	int ret;
>  
> -	bcm2835_setup_restart();
> -	if (wdt_regs)
> -		pm_power_off = bcm2835_power_off;
> -
>  	bcm2835_init_clocks();
>  
>  	ret = of_platform_populate(NULL, of_default_bus_match_table, NULL,
> @@ -114,6 +42,5 @@ static const char * const bcm2835_compat[] = {
>  
>  DT_MACHINE_START(BCM2835, "BCM2835")
>  	.init_machine = bcm2835_init,
> -	.restart = bcm2835_restart,
>  	.dt_compat = bcm2835_compat
>  MACHINE_END
> diff --git a/drivers/watchdog/bcm2835_wdt.c b/drivers/watchdog/bcm2835_wdt.c
> index 2b5a9bb..7116968 100644
> --- a/drivers/watchdog/bcm2835_wdt.c
> +++ b/drivers/watchdog/bcm2835_wdt.c
> @@ -13,20 +13,25 @@
>   * option) any later version.
>   */
>  
> +#include <linux/delay.h>
> +#include <linux/reboot.h>
>  #include <linux/types.h>
>  #include <linux/module.h>
>  #include <linux/io.h>
>  #include <linux/watchdog.h>
>  #include <linux/platform_device.h>
>  #include <linux/of_address.h>
> +#include <linux/of_platform.h>
>  
>  #define PM_RSTC				0x1c
> +#define PM_RSTS				0x20
>  #define PM_WDOG				0x24
>  
>  #define PM_PASSWORD			0x5a000000
>  
>  #define PM_WDOG_TIME_SET		0x000fffff
>  #define PM_RSTC_WRCFG_CLR		0xffffffcf
> +#define PM_RSTS_HADWRH_SET		0x00000040
>  #define PM_RSTC_WRCFG_SET		0x00000030
>  #define PM_RSTC_WRCFG_FULL_RESET	0x00000020
>  #define PM_RSTC_RESET			0x00000102
> @@ -37,6 +42,7 @@
>  struct bcm2835_wdt {
>  	void __iomem		*base;
>  	spinlock_t		lock;
> +	struct notifier_block	restart_handler;
>  };
>  
>  static unsigned int heartbeat;
> @@ -106,6 +112,53 @@ static struct watchdog_device bcm2835_wdt_wdd = {
>  	.timeout =	WDOG_TICKS_TO_SECS(PM_WDOG_TIME_SET),
>  };
>  
> +static int
> +bcm2835_restart(struct notifier_block *this, unsigned long mode, void *cmd)
> +{
> +	struct bcm2835_wdt *wdt = container_of(this, struct bcm2835_wdt,
> +					       restart_handler);
> +	u32 val;
> +
> +	/* use a timeout of 10 ticks (~150us) */
> +	writel_relaxed(10 | PM_PASSWORD, wdt->base + PM_WDOG);
> +	val = readl_relaxed(wdt->base + PM_RSTC);
> +	val &= PM_RSTC_WRCFG_CLR;
> +	val |= PM_PASSWORD | PM_RSTC_WRCFG_FULL_RESET;
> +	writel_relaxed(val, wdt->base + PM_RSTC);
> +
> +	/* No sleeping, possibly atomic. */
> +	mdelay(1);
> +
> +	return 0;
> +}
> +
> +/*
> + * We can't really power off, but if we do the normal reset scheme, and
> + * indicate to bootcode.bin not to reboot, then most of the chip will be
> + * powered off.
> + */
> +static void bcm2835_power_off(void)
> +{
> +	struct device_node *np =
> +		of_find_compatible_node(NULL, NULL, "brcm,bcm2835-pm-wdt");
> +	struct platform_device *pdev = of_find_device_by_node(np);
> +	struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
> +	u32 val;
> +
> +	/*
> +	 * We set the watchdog hard reset bit here to distinguish this reset
> +	 * from the normal (full) reset. bootcode.bin will not reboot after a
> +	 * hard reset.
> +	 */
> +	val = readl_relaxed(wdt->base + PM_RSTS);
> +	val &= PM_RSTC_WRCFG_CLR;
> +	val |= PM_PASSWORD | PM_RSTS_HADWRH_SET;
> +	writel_relaxed(val, wdt->base + PM_RSTS);
> +
> +	/* Continue with normal reset mechanism */
> +	bcm2835_restart(&wdt->restart_handler, REBOOT_HARD, NULL);
> +}
> +
>  static int bcm2835_wdt_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -136,6 +189,12 @@ static int bcm2835_wdt_probe(struct platform_device *pdev)
>  		return err;
>  	}
>  
> +	wdt->restart_handler.notifier_call = bcm2835_restart;
> +	wdt->restart_handler.priority = 128;
> +	register_restart_handler(&wdt->restart_handler);
> +	if (pm_power_off == NULL)
> +		pm_power_off = bcm2835_power_off;
> +
>  	dev_info(dev, "Broadcom BCM2835 watchdog timer");
>  	return 0;
>  }
> @@ -144,6 +203,9 @@ static int bcm2835_wdt_remove(struct platform_device *pdev)
>  {
>  	struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
>  
> +	unregister_restart_handler(&wdt->restart_handler);
> +	if (pm_power_off == bcm2835_power_off)
> +		pm_power_off = NULL;
>  	watchdog_unregister_device(&bcm2835_wdt_wdd);
>  	iounmap(wdt->base);
>  

[PATCH 2/2] ARM: BCM2835: Move the restart/power_off handling to the WDT driver.

[Lee Jones]

On Fri, 24 Apr 2015, Eric Anholt wrote:

> Since the WDT is what's used to drive restart and power off, it makes
> more sense to keep it there, where the regs are already mapped and
> definitions for them provided.  Note that this means you may need to
> add CONFIG_BCM2835_WDT to retain functionality of your kernel.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> Cc: linux-watchdog at vger.kernel.org
> ---
> 
> Note that power off has never worked for me, and just reboots as well.
> So I can't say that I've *really* tested the power off code.
> 
>  arch/arm/mach-bcm/board_bcm2835.c | 73 ---------------------------------------
>  drivers/watchdog/bcm2835_wdt.c    | 62 +++++++++++++++++++++++++++++++++
>  2 files changed, 62 insertions(+), 73 deletions(-)

Applied with Guenter's Ack.

> diff --git a/arch/arm/mach-bcm/board_bcm2835.c b/arch/arm/mach-bcm/board_bcm2835.c
> index 49dd5b0..0f7b9ea 100644
> --- a/arch/arm/mach-bcm/board_bcm2835.c
> +++ b/arch/arm/mach-bcm/board_bcm2835.c
> @@ -12,7 +12,6 @@
>   * GNU General Public License for more details.
>   */
>  
> -#include <linux/delay.h>
>  #include <linux/init.h>
>  #include <linux/irqchip.h>
>  #include <linux/of_address.h>
> @@ -22,81 +21,10 @@
>  #include <asm/mach/arch.h>
>  #include <asm/mach/map.h>
>  
> -#define PM_RSTC				0x1c
> -#define PM_RSTS				0x20
> -#define PM_WDOG				0x24
> -
> -#define PM_PASSWORD			0x5a000000
> -#define PM_RSTC_WRCFG_MASK		0x00000030
> -#define PM_RSTC_WRCFG_FULL_RESET	0x00000020
> -#define PM_RSTS_HADWRH_SET		0x00000040
> -
> -static void __iomem *wdt_regs;
> -
> -/*
> - * The machine restart method can be called from an atomic context so we won't
> - * be able to ioremap the regs then.
> - */
> -static void bcm2835_setup_restart(void)
> -{
> -	struct device_node *np = of_find_compatible_node(NULL, NULL,
> -						"brcm,bcm2835-pm-wdt");
> -	if (WARN(!np, "unable to setup watchdog restart"))
> -		return;
> -
> -	wdt_regs = of_iomap(np, 0);
> -	WARN(!wdt_regs, "failed to remap watchdog regs");
> -}
> -
> -static void bcm2835_restart(enum reboot_mode mode, const char *cmd)
> -{
> -	u32 val;
> -
> -	if (!wdt_regs)
> -		return;
> -
> -	/* use a timeout of 10 ticks (~150us) */
> -	writel_relaxed(10 | PM_PASSWORD, wdt_regs + PM_WDOG);
> -	val = readl_relaxed(wdt_regs + PM_RSTC);
> -	val &= ~PM_RSTC_WRCFG_MASK;
> -	val |= PM_PASSWORD | PM_RSTC_WRCFG_FULL_RESET;
> -	writel_relaxed(val, wdt_regs + PM_RSTC);
> -
> -	/* No sleeping, possibly atomic. */
> -	mdelay(1);
> -}
> -
> -/*
> - * We can't really power off, but if we do the normal reset scheme, and
> - * indicate to bootcode.bin not to reboot, then most of the chip will be
> - * powered off.
> - */
> -static void bcm2835_power_off(void)
> -{
> -	u32 val;
> -
> -	/*
> -	 * We set the watchdog hard reset bit here to distinguish this reset
> -	 * from the normal (full) reset. bootcode.bin will not reboot after a
> -	 * hard reset.
> -	 */
> -	val = readl_relaxed(wdt_regs + PM_RSTS);
> -	val &= ~PM_RSTC_WRCFG_MASK;
> -	val |= PM_PASSWORD | PM_RSTS_HADWRH_SET;
> -	writel_relaxed(val, wdt_regs + PM_RSTS);
> -
> -	/* Continue with normal reset mechanism */
> -	bcm2835_restart(REBOOT_HARD, "");
> -}
> -
>  static void __init bcm2835_init(void)
>  {
>  	int ret;
>  
> -	bcm2835_setup_restart();
> -	if (wdt_regs)
> -		pm_power_off = bcm2835_power_off;
> -
>  	bcm2835_init_clocks();
>  
>  	ret = of_platform_populate(NULL, of_default_bus_match_table, NULL,
> @@ -114,6 +42,5 @@ static const char * const bcm2835_compat[] = {
>  
>  DT_MACHINE_START(BCM2835, "BCM2835")
>  	.init_machine = bcm2835_init,
> -	.restart = bcm2835_restart,
>  	.dt_compat = bcm2835_compat
>  MACHINE_END
> diff --git a/drivers/watchdog/bcm2835_wdt.c b/drivers/watchdog/bcm2835_wdt.c
> index 2b5a9bb..7116968 100644
> --- a/drivers/watchdog/bcm2835_wdt.c
> +++ b/drivers/watchdog/bcm2835_wdt.c
> @@ -13,20 +13,25 @@
>   * option) any later version.
>   */
>  
> +#include <linux/delay.h>
> +#include <linux/reboot.h>
>  #include <linux/types.h>
>  #include <linux/module.h>
>  #include <linux/io.h>
>  #include <linux/watchdog.h>
>  #include <linux/platform_device.h>
>  #include <linux/of_address.h>
> +#include <linux/of_platform.h>
>  
>  #define PM_RSTC				0x1c
> +#define PM_RSTS				0x20
>  #define PM_WDOG				0x24
>  
>  #define PM_PASSWORD			0x5a000000
>  
>  #define PM_WDOG_TIME_SET		0x000fffff
>  #define PM_RSTC_WRCFG_CLR		0xffffffcf
> +#define PM_RSTS_HADWRH_SET		0x00000040
>  #define PM_RSTC_WRCFG_SET		0x00000030
>  #define PM_RSTC_WRCFG_FULL_RESET	0x00000020
>  #define PM_RSTC_RESET			0x00000102
> @@ -37,6 +42,7 @@
>  struct bcm2835_wdt {
>  	void __iomem		*base;
>  	spinlock_t		lock;
> +	struct notifier_block	restart_handler;
>  };
>  
>  static unsigned int heartbeat;
> @@ -106,6 +112,53 @@ static struct watchdog_device bcm2835_wdt_wdd = {
>  	.timeout =	WDOG_TICKS_TO_SECS(PM_WDOG_TIME_SET),
>  };
>  
> +static int
> +bcm2835_restart(struct notifier_block *this, unsigned long mode, void *cmd)
> +{
> +	struct bcm2835_wdt *wdt = container_of(this, struct bcm2835_wdt,
> +					       restart_handler);
> +	u32 val;
> +
> +	/* use a timeout of 10 ticks (~150us) */
> +	writel_relaxed(10 | PM_PASSWORD, wdt->base + PM_WDOG);
> +	val = readl_relaxed(wdt->base + PM_RSTC);
> +	val &= PM_RSTC_WRCFG_CLR;
> +	val |= PM_PASSWORD | PM_RSTC_WRCFG_FULL_RESET;
> +	writel_relaxed(val, wdt->base + PM_RSTC);
> +
> +	/* No sleeping, possibly atomic. */
> +	mdelay(1);
> +
> +	return 0;
> +}
> +
> +/*
> + * We can't really power off, but if we do the normal reset scheme, and
> + * indicate to bootcode.bin not to reboot, then most of the chip will be
> + * powered off.
> + */
> +static void bcm2835_power_off(void)
> +{
> +	struct device_node *np =
> +		of_find_compatible_node(NULL, NULL, "brcm,bcm2835-pm-wdt");
> +	struct platform_device *pdev = of_find_device_by_node(np);
> +	struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
> +	u32 val;
> +
> +	/*
> +	 * We set the watchdog hard reset bit here to distinguish this reset
> +	 * from the normal (full) reset. bootcode.bin will not reboot after a
> +	 * hard reset.
> +	 */
> +	val = readl_relaxed(wdt->base + PM_RSTS);
> +	val &= PM_RSTC_WRCFG_CLR;
> +	val |= PM_PASSWORD | PM_RSTS_HADWRH_SET;
> +	writel_relaxed(val, wdt->base + PM_RSTS);
> +
> +	/* Continue with normal reset mechanism */
> +	bcm2835_restart(&wdt->restart_handler, REBOOT_HARD, NULL);
> +}
> +
>  static int bcm2835_wdt_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -136,6 +189,12 @@ static int bcm2835_wdt_probe(struct platform_device *pdev)
>  		return err;
>  	}
>  
> +	wdt->restart_handler.notifier_call = bcm2835_restart;
> +	wdt->restart_handler.priority = 128;
> +	register_restart_handler(&wdt->restart_handler);
> +	if (pm_power_off == NULL)
> +		pm_power_off = bcm2835_power_off;
> +
>  	dev_info(dev, "Broadcom BCM2835 watchdog timer");
>  	return 0;
>  }
> @@ -144,6 +203,9 @@ static int bcm2835_wdt_remove(struct platform_device *pdev)
>  {
>  	struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
>  
> +	unregister_restart_handler(&wdt->restart_handler);
> +	if (pm_power_off == bcm2835_power_off)
> +		pm_power_off = NULL;
>  	watchdog_unregister_device(&bcm2835_wdt_wdd);
>  	iounmap(wdt->base);
>