[PATCH v2 0/3] power: reset: add reset for renesas r7s72100

[PATCH v2 0/3] power: reset: add reset for renesas r7s72100

[Chris Brandt]

Some Renesas SoCs do not have a reset register and the only way to do a SW
controlled reset is to use the watchdog timer.

Additionally, since all the WDT timeout options are so quick, a system
reset is about the only thing it's good for.
For example, the longest WDT overflow you can get with a RZ/A1 (R7S72100)
with its 8-bit wide counter is 125ms.

v2:
* added to renesas-wdt.txt instead of creating a new file
* changed "renesas,r7s72100-reset" to "renesas,r7s72100-wdt"
* changed "renesas,wdt-reset" to "renesas,rza-wdt"
* added "renesas,rza-wdt" as a fallback
* added interupt property (even though it is not used)
* added clocks property
* changed hard coded register values to defines
* added msleep to while(1) loop
* removed unnecessary #include files
* added Reviewed-by: Geert Uytterhoeven for renesas-reset.c

Chris Brandt (3):
  power: reset: Add Renesas reset driver
  watchdog: renesas-wdt: add support for rza
  ARM: dts: r7s72100: Add reset handler

 .../devicetree/bindings/watchdog/renesas-wdt.txt   |   4 +-
 arch/arm/boot/dts/r7s72100.dtsi                    |   7 ++
 drivers/power/reset/Kconfig                        |   9 ++
 drivers/power/reset/Makefile                       |   1 +
 drivers/power/reset/renesas-reset.c                | 112 +++++++++++++++++++++
 5 files changed, 132 insertions(+), 1 deletion(-)
 create mode 100644 drivers/power/reset/renesas-reset.c

-- 
2.10.1

[PATCH v2 1/3] power: reset: Add Renesas reset driver

[Chris Brandt]

Some Renesas SoCs do not have a reset register and the only way to do a SW
controlled reset is to use the watchdog timer. Additionally, since all the
WDT timeout options are so quick, a system reset is about the only thing
it's good for.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
* changed DT bindings from 'wdt-reset' to 'rza-wdt'
* changed hard coded register values to defines
* added msleep to while(1) loop
* removed unnecessary #include files
* added Reviewed-by: Geert Uytterhoeven
---
 drivers/power/reset/Kconfig         |   9 +++
 drivers/power/reset/Makefile        |   1 +
 drivers/power/reset/renesas-reset.c | 112 ++++++++++++++++++++++++++++++++++++
 3 files changed, 122 insertions(+)
 create mode 100644 drivers/power/reset/renesas-reset.c

diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index b8caccc..e3100c9 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -130,6 +130,15 @@ config POWER_RESET_QNAP
 
 	  Say Y if you have a QNAP NAS.
 
+config POWER_RESET_RENESAS
+	tristate "Renesas WDT reset driver"
+	depends on ARCH_RENESAS || COMPILE_TEST
+	depends on HAS_IOMEM
+	help
+	  Reboot support for Renesas SoCs with WDT reset.
+	  Some Renesas SoCs do not have a reset register and the only way
+	  to do a SW controlled reset is to use the watchdog timer.
+
 config POWER_RESET_RESTART
 	bool "Restart power-off driver"
 	help
diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
index 11dae3b..a78a56c 100644
--- a/drivers/power/reset/Makefile
+++ b/drivers/power/reset/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_POWER_RESET_MSM) += msm-poweroff.o
 obj-$(CONFIG_POWER_RESET_PIIX4_POWEROFF) += piix4-poweroff.o
 obj-$(CONFIG_POWER_RESET_LTC2952) += ltc2952-poweroff.o
 obj-$(CONFIG_POWER_RESET_QNAP) += qnap-poweroff.o
+obj-$(CONFIG_POWER_RESET_RENESAS) += renesas-reset.o
 obj-$(CONFIG_POWER_RESET_RESTART) += restart-poweroff.o
 obj-$(CONFIG_POWER_RESET_ST) += st-poweroff.o
 obj-$(CONFIG_POWER_RESET_VERSATILE) += arm-versatile-reboot.o
diff --git a/drivers/power/reset/renesas-reset.c b/drivers/power/reset/renesas-reset.c
new file mode 100644
index 0000000..812cee0
--- /dev/null
+++ b/drivers/power/reset/renesas-reset.c
@@ -0,0 +1,112 @@
+/*
+ * Renesas WDT Reset Driver
+ *
+ * Copyright (C) 2017 Renesas Electronics America, Inc.
+ * Copyright (C) 2017 Chris Brandt
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * based on rmobile-reset.c
+ *
+ */
+
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/reboot.h>
+#include <linux/delay.h>
+
+/* Watchdog Timer Registers */
+#define WTCSR 0
+#define WTCSR_MAGIC 0xA500
+#define WTSCR_WT (1<<6)
+#define WTSCR_TME (1<<5)
+
+#define WTCNT 2
+#define WTCNT_MAGIC 0x5A00
+
+#define WRCSR 4
+#define WRCSR_MAGIC 0x5A00
+#define WRCSR_RSTE (1<<6)
+#define WRCSR_CLEAR_WOVF 0xA500 /* special value */
+
+static void __iomem *base;
+
+static int wdt_reset_handler(struct notifier_block *this,
+			     unsigned long mode, void *cmd)
+{
+	pr_debug("%s %lu\n", __func__, mode);
+
+	/* Dummy read (must read WRCSR:WOVF at least once before clearing) */
+	readb(base + WRCSR);
+
+	writew(WRCSR_CLEAR_WOVF, base + WRCSR);		/* Clear WOVF */
+	writew(WRCSR_MAGIC | WRCSR_RSTE, base + WRCSR);	/* Reset Enable */
+	writew(WTCNT_MAGIC, base + WTCNT);		/* Counter to 00 */
+
+	/* Start timer */
+	writew(WTCSR_MAGIC | WTSCR_WT | WTSCR_TME, base + WTCSR);
+
+	/* Wait for WDT overflow (reset) */
+	while (1)
+		msleep(1);
+
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block wdt_reset_nb = {
+	.notifier_call = wdt_reset_handler,
+	.priority = 192,
+};
+
+static int wdt_reset_probe(struct platform_device *pdev)
+{
+	int error;
+
+	base = of_iomap(pdev->dev.of_node, 0);
+	if (!base)
+		return -ENODEV;
+
+	error = register_restart_handler(&wdt_reset_nb);
+	if (error) {
+		dev_err(&pdev->dev,
+			"cannot register restart handler (err=%d)\n", error);
+		goto fail_unmap;
+	}
+
+	return 0;
+
+fail_unmap:
+	iounmap(base);
+	return error;
+}
+
+static int wdt_reset_remove(struct platform_device *pdev)
+{
+	unregister_restart_handler(&wdt_reset_nb);
+	iounmap(base);
+	return 0;
+}
+
+static const struct of_device_id wdt_reset_of_match[] = {
+	{ .compatible = "renesas,rza-wdt", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, wdt_reset_of_match);
+
+static struct platform_driver wdt_reset_driver = {
+	.probe = wdt_reset_probe,
+	.remove = wdt_reset_remove,
+	.driver = {
+		.name = "wdt_reset",
+		.of_match_table = wdt_reset_of_match,
+	},
+};
+
+module_platform_driver(wdt_reset_driver);
+
+MODULE_DESCRIPTION("Renesas WDT Reset Driver");
+MODULE_AUTHOR("Chris Brandt <chris.brandt@renesas.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.10.1

[PATCH v2 2/3] watchdog: renesas-wdt: add support for rza

[Chris Brandt]

Describe the WDT hardware in the RZ/A series.

Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
---
v2:
* added to renesas-wdt.txt instead of creating a new file
* changed commit title
* added "renesas,rza-wdt" as a fallback
* added interrupts property
---
 Documentation/devicetree/bindings/watchdog/renesas-wdt.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/watchdog/renesas-wdt.txt b/Documentation/devicetree/bindings/watchdog/renesas-wdt.txt
index da24e31..9e306af 100644
--- a/Documentation/devicetree/bindings/watchdog/renesas-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/renesas-wdt.txt
@@ -2,10 +2,11 @@ Renesas Watchdog Timer (WDT) Controller
 
 Required properties:
 - compatible : Should be "renesas,<soctype>-wdt", and
-	       "renesas,rcar-gen3-wdt" as fallback.
+	       "renesas,rcar-gen3-wdt" or "renesas,rza-wdt" as fallback.
 	       Examples with soctypes are:
 	         - "renesas,r8a7795-wdt" (R-Car H3)
 	         - "renesas,r8a7796-wdt" (R-Car M3-W)
+	         - "renesas,r7s72100-wdt" (RZ/A1)
 
   When compatible with the generic version, nodes must list the SoC-specific
   version corresponding to the platform first, followed by the generic
@@ -17,6 +18,7 @@ Required properties:
 Optional properties:
 - timeout-sec : Contains the watchdog timeout in seconds
 - power-domains : the power domain the WDT belongs to
+- interrupts: Some WDTs have an interrupt when used in interval timer mode
 
 Examples:
 
-- 
2.10.1


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

[PATCH v2 2/3] watchdog: renesas-wdt: add support for rza

[Chris Brandt]

Describe the WDT hardware in the RZ/A series.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
v2:
* added to renesas-wdt.txt instead of creating a new file
* changed commit title
* added "renesas,rza-wdt" as a fallback
* added interrupts property
---
 Documentation/devicetree/bindings/watchdog/renesas-wdt.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/watchdog/renesas-wdt.txt b/Documentation/devicetree/bindings/watchdog/renesas-wdt.txt
index da24e31..9e306af 100644
--- a/Documentation/devicetree/bindings/watchdog/renesas-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/renesas-wdt.txt
@@ -2,10 +2,11 @@ Renesas Watchdog Timer (WDT) Controller
 
 Required properties:
 - compatible : Should be "renesas,<soctype>-wdt", and
-	       "renesas,rcar-gen3-wdt" as fallback.
+	       "renesas,rcar-gen3-wdt" or "renesas,rza-wdt" as fallback.
 	       Examples with soctypes are:
 	         - "renesas,r8a7795-wdt" (R-Car H3)
 	         - "renesas,r8a7796-wdt" (R-Car M3-W)
+	         - "renesas,r7s72100-wdt" (RZ/A1)
 
   When compatible with the generic version, nodes must list the SoC-specific
   version corresponding to the platform first, followed by the generic
@@ -17,6 +18,7 @@ Required properties:
 Optional properties:
 - timeout-sec : Contains the watchdog timeout in seconds
 - power-domains : the power domain the WDT belongs to
+- interrupts: Some WDTs have an interrupt when used in interval timer mode
 
 Examples:
 
-- 
2.10.1

[PATCH v2 3/3] ARM: dts: r7s72100: Add reset handler

[Chris Brandt]

For the RZ/A1, the only way to do a reset is to overflow the WDT.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
v2:
* changed "renesas,r7s72100-reset" to "renesas,r7s72100-wdt"
* changed "renesas,wdt-reset" to "renesas,rza-wdt"
* added interupt property (even though it is not used)
* added clocks property
---
 arch/arm/boot/dts/r7s72100.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
index 614ba79..88d9840 100644
--- a/arch/arm/boot/dts/r7s72100.dtsi
+++ b/arch/arm/boot/dts/r7s72100.dtsi
@@ -371,6 +371,13 @@
 			<0xe8202000 0x1000>;
 	};
 
+	wdt: timer@fcfe0000 {
+		compatible = "renesas,r7s72100-wdt", "renesas,rza-wdt";
+		reg = <0xfcfe0000 0x6>;
+		interrupts = <GIC_SPI 106 IRQ_TYPE_EDGE_RISING>;
+		clocks = <&p0_clk>;
+	};
+
 	i2c0: i2c@fcfee000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-- 
2.10.1

Re: [PATCH v2 1/3] power: reset: Add Renesas reset driver

[Guenter Roeck]

On Thu, Feb 16, 2017 at 12:23:18PM -0500, Chris Brandt wrote:
> Some Renesas SoCs do not have a reset register and the only way to do a SW
> controlled reset is to use the watchdog timer. Additionally, since all the
> WDT timeout options are so quick, a system reset is about the only thing
> it's good for.
> 
> Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>

Wondering - why not use the watchdog driver and the infrastructure
provided by the watchdog core (ie the restart callback) instead ?

> ---
> v2:
> * changed DT bindings from 'wdt-reset' to 'rza-wdt'
> * changed hard coded register values to defines
> * added msleep to while(1) loop

Sure you can sleep here ?

Guenter

> * removed unnecessary #include files
> * added Reviewed-by: Geert Uytterhoeven
> ---
>  drivers/power/reset/Kconfig         |   9 +++
>  drivers/power/reset/Makefile        |   1 +
>  drivers/power/reset/renesas-reset.c | 112 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 122 insertions(+)
>  create mode 100644 drivers/power/reset/renesas-reset.c
> 
> diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
> index b8caccc..e3100c9 100644
> --- a/drivers/power/reset/Kconfig
> +++ b/drivers/power/reset/Kconfig
> @@ -130,6 +130,15 @@ config POWER_RESET_QNAP
>  
>  	  Say Y if you have a QNAP NAS.
>  
> +config POWER_RESET_RENESAS
> +	tristate "Renesas WDT reset driver"
> +	depends on ARCH_RENESAS || COMPILE_TEST
> +	depends on HAS_IOMEM
> +	help
> +	  Reboot support for Renesas SoCs with WDT reset.
> +	  Some Renesas SoCs do not have a reset register and the only way
> +	  to do a SW controlled reset is to use the watchdog timer.
> +
>  config POWER_RESET_RESTART
>  	bool "Restart power-off driver"
>  	help
> diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
> index 11dae3b..a78a56c 100644
> --- a/drivers/power/reset/Makefile
> +++ b/drivers/power/reset/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_POWER_RESET_MSM) += msm-poweroff.o
>  obj-$(CONFIG_POWER_RESET_PIIX4_POWEROFF) += piix4-poweroff.o
>  obj-$(CONFIG_POWER_RESET_LTC2952) += ltc2952-poweroff.o
>  obj-$(CONFIG_POWER_RESET_QNAP) += qnap-poweroff.o
> +obj-$(CONFIG_POWER_RESET_RENESAS) += renesas-reset.o
>  obj-$(CONFIG_POWER_RESET_RESTART) += restart-poweroff.o
>  obj-$(CONFIG_POWER_RESET_ST) += st-poweroff.o
>  obj-$(CONFIG_POWER_RESET_VERSATILE) += arm-versatile-reboot.o
> diff --git a/drivers/power/reset/renesas-reset.c b/drivers/power/reset/renesas-reset.c
> new file mode 100644
> index 0000000..812cee0
> --- /dev/null
> +++ b/drivers/power/reset/renesas-reset.c
> @@ -0,0 +1,112 @@
> +/*
> + * Renesas WDT Reset Driver
> + *
> + * Copyright (C) 2017 Renesas Electronics America, Inc.
> + * Copyright (C) 2017 Chris Brandt
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + *
> + * based on rmobile-reset.c
> + *
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/reboot.h>
> +#include <linux/delay.h>
> +
> +/* Watchdog Timer Registers */
> +#define WTCSR 0
> +#define WTCSR_MAGIC 0xA500
> +#define WTSCR_WT (1<<6)
> +#define WTSCR_TME (1<<5)
> +
> +#define WTCNT 2
> +#define WTCNT_MAGIC 0x5A00
> +
> +#define WRCSR 4
> +#define WRCSR_MAGIC 0x5A00
> +#define WRCSR_RSTE (1<<6)
> +#define WRCSR_CLEAR_WOVF 0xA500 /* special value */
> +
> +static void __iomem *base;
> +
> +static int wdt_reset_handler(struct notifier_block *this,
> +			     unsigned long mode, void *cmd)
> +{
> +	pr_debug("%s %lu\n", __func__, mode);
> +
> +	/* Dummy read (must read WRCSR:WOVF at least once before clearing) */
> +	readb(base + WRCSR);
> +
> +	writew(WRCSR_CLEAR_WOVF, base + WRCSR);		/* Clear WOVF */
> +	writew(WRCSR_MAGIC | WRCSR_RSTE, base + WRCSR);	/* Reset Enable */
> +	writew(WTCNT_MAGIC, base + WTCNT);		/* Counter to 00 */
> +
> +	/* Start timer */
> +	writew(WTCSR_MAGIC | WTSCR_WT | WTSCR_TME, base + WTCSR);
> +
> +	/* Wait for WDT overflow (reset) */
> +	while (1)
> +		msleep(1);
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block wdt_reset_nb = {
> +	.notifier_call = wdt_reset_handler,
> +	.priority = 192,
> +};
> +
> +static int wdt_reset_probe(struct platform_device *pdev)
> +{
> +	int error;
> +
> +	base = of_iomap(pdev->dev.of_node, 0);
> +	if (!base)
> +		return -ENODEV;
> +
> +	error = register_restart_handler(&wdt_reset_nb);
> +	if (error) {
> +		dev_err(&pdev->dev,
> +			"cannot register restart handler (err=%d)\n", error);
> +		goto fail_unmap;
> +	}
> +
> +	return 0;
> +
> +fail_unmap:
> +	iounmap(base);
> +	return error;
> +}
> +
> +static int wdt_reset_remove(struct platform_device *pdev)
> +{
> +	unregister_restart_handler(&wdt_reset_nb);
> +	iounmap(base);
> +	return 0;
> +}
> +
> +static const struct of_device_id wdt_reset_of_match[] = {
> +	{ .compatible = "renesas,rza-wdt", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, wdt_reset_of_match);
> +
> +static struct platform_driver wdt_reset_driver = {
> +	.probe = wdt_reset_probe,
> +	.remove = wdt_reset_remove,
> +	.driver = {
> +		.name = "wdt_reset",
> +		.of_match_table = wdt_reset_of_match,
> +	},
> +};
> +
> +module_platform_driver(wdt_reset_driver);
> +
> +MODULE_DESCRIPTION("Renesas WDT Reset Driver");
> +MODULE_AUTHOR("Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.10.1
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] power: reset: Add Renesas reset driver

[Guenter Roeck]

On Thu, Feb 16, 2017 at 12:23:18PM -0500, Chris Brandt wrote:
> Some Renesas SoCs do not have a reset register and the only way to do a SW
> controlled reset is to use the watchdog timer. Additionally, since all the
> WDT timeout options are so quick, a system reset is about the only thing
> it's good for.
> 
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Wondering - why not use the watchdog driver and the infrastructure
provided by the watchdog core (ie the restart callback) instead ?

> ---
> v2:
> * changed DT bindings from 'wdt-reset' to 'rza-wdt'
> * changed hard coded register values to defines
> * added msleep to while(1) loop

Sure you can sleep here ?

Guenter

> * removed unnecessary #include files
> * added Reviewed-by: Geert Uytterhoeven
> ---
>  drivers/power/reset/Kconfig         |   9 +++
>  drivers/power/reset/Makefile        |   1 +
>  drivers/power/reset/renesas-reset.c | 112 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 122 insertions(+)
>  create mode 100644 drivers/power/reset/renesas-reset.c
> 
> diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
> index b8caccc..e3100c9 100644
> --- a/drivers/power/reset/Kconfig
> +++ b/drivers/power/reset/Kconfig
> @@ -130,6 +130,15 @@ config POWER_RESET_QNAP
>  
>  	  Say Y if you have a QNAP NAS.
>  
> +config POWER_RESET_RENESAS
> +	tristate "Renesas WDT reset driver"
> +	depends on ARCH_RENESAS || COMPILE_TEST
> +	depends on HAS_IOMEM
> +	help
> +	  Reboot support for Renesas SoCs with WDT reset.
> +	  Some Renesas SoCs do not have a reset register and the only way
> +	  to do a SW controlled reset is to use the watchdog timer.
> +
>  config POWER_RESET_RESTART
>  	bool "Restart power-off driver"
>  	help
> diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
> index 11dae3b..a78a56c 100644
> --- a/drivers/power/reset/Makefile
> +++ b/drivers/power/reset/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_POWER_RESET_MSM) += msm-poweroff.o
>  obj-$(CONFIG_POWER_RESET_PIIX4_POWEROFF) += piix4-poweroff.o
>  obj-$(CONFIG_POWER_RESET_LTC2952) += ltc2952-poweroff.o
>  obj-$(CONFIG_POWER_RESET_QNAP) += qnap-poweroff.o
> +obj-$(CONFIG_POWER_RESET_RENESAS) += renesas-reset.o
>  obj-$(CONFIG_POWER_RESET_RESTART) += restart-poweroff.o
>  obj-$(CONFIG_POWER_RESET_ST) += st-poweroff.o
>  obj-$(CONFIG_POWER_RESET_VERSATILE) += arm-versatile-reboot.o
> diff --git a/drivers/power/reset/renesas-reset.c b/drivers/power/reset/renesas-reset.c
> new file mode 100644
> index 0000000..812cee0
> --- /dev/null
> +++ b/drivers/power/reset/renesas-reset.c
> @@ -0,0 +1,112 @@
> +/*
> + * Renesas WDT Reset Driver
> + *
> + * Copyright (C) 2017 Renesas Electronics America, Inc.
> + * Copyright (C) 2017 Chris Brandt
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + *
> + * based on rmobile-reset.c
> + *
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/reboot.h>
> +#include <linux/delay.h>
> +
> +/* Watchdog Timer Registers */
> +#define WTCSR 0
> +#define WTCSR_MAGIC 0xA500
> +#define WTSCR_WT (1<<6)
> +#define WTSCR_TME (1<<5)
> +
> +#define WTCNT 2
> +#define WTCNT_MAGIC 0x5A00
> +
> +#define WRCSR 4
> +#define WRCSR_MAGIC 0x5A00
> +#define WRCSR_RSTE (1<<6)
> +#define WRCSR_CLEAR_WOVF 0xA500 /* special value */
> +
> +static void __iomem *base;
> +
> +static int wdt_reset_handler(struct notifier_block *this,
> +			     unsigned long mode, void *cmd)
> +{
> +	pr_debug("%s %lu\n", __func__, mode);
> +
> +	/* Dummy read (must read WRCSR:WOVF at least once before clearing) */
> +	readb(base + WRCSR);
> +
> +	writew(WRCSR_CLEAR_WOVF, base + WRCSR);		/* Clear WOVF */
> +	writew(WRCSR_MAGIC | WRCSR_RSTE, base + WRCSR);	/* Reset Enable */
> +	writew(WTCNT_MAGIC, base + WTCNT);		/* Counter to 00 */
> +
> +	/* Start timer */
> +	writew(WTCSR_MAGIC | WTSCR_WT | WTSCR_TME, base + WTCSR);
> +
> +	/* Wait for WDT overflow (reset) */
> +	while (1)
> +		msleep(1);
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block wdt_reset_nb = {
> +	.notifier_call = wdt_reset_handler,
> +	.priority = 192,
> +};
> +
> +static int wdt_reset_probe(struct platform_device *pdev)
> +{
> +	int error;
> +
> +	base = of_iomap(pdev->dev.of_node, 0);
> +	if (!base)
> +		return -ENODEV;
> +
> +	error = register_restart_handler(&wdt_reset_nb);
> +	if (error) {
> +		dev_err(&pdev->dev,
> +			"cannot register restart handler (err=%d)\n", error);
> +		goto fail_unmap;
> +	}
> +
> +	return 0;
> +
> +fail_unmap:
> +	iounmap(base);
> +	return error;
> +}
> +
> +static int wdt_reset_remove(struct platform_device *pdev)
> +{
> +	unregister_restart_handler(&wdt_reset_nb);
> +	iounmap(base);
> +	return 0;
> +}
> +
> +static const struct of_device_id wdt_reset_of_match[] = {
> +	{ .compatible = "renesas,rza-wdt", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, wdt_reset_of_match);
> +
> +static struct platform_driver wdt_reset_driver = {
> +	.probe = wdt_reset_probe,
> +	.remove = wdt_reset_remove,
> +	.driver = {
> +		.name = "wdt_reset",
> +		.of_match_table = wdt_reset_of_match,
> +	},
> +};
> +
> +module_platform_driver(wdt_reset_driver);
> +
> +MODULE_DESCRIPTION("Renesas WDT Reset Driver");
> +MODULE_AUTHOR("Chris Brandt <chris.brandt@renesas.com>");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.10.1
> 
> 

RE: [PATCH v2 1/3] power: reset: Add Renesas reset driver

[Chris Brandt]

On Thursday, February 16, 2017, Guenter Roeck wrote:
> On Thu, Feb 16, 2017 at 12:23:18PM -0500, Chris Brandt wrote:
> > Some Renesas SoCs do not have a reset register and the only way to do
> > a SW controlled reset is to use the watchdog timer. Additionally,
> > since all the WDT timeout options are so quick, a system reset is
> > about the only thing it's good for.
> >
> > Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Wondering - why not use the watchdog driver and the infrastructure
> provided by the watchdog core (ie the restart callback) instead ?

That was Geert's first suggestion, but then he said:

On Friday, February 10, 2017, Geert Uytterhoeven wrote:
> On Fri, Feb 10, 2017 at 3:59 PM, Chris Brandt <Chris.Brandt@renesas.com>
> wrote:
> > On Friday, February 10, 2017, Geert Uytterhoeven wrote:
> >> >  static const char *const r7s72100_boards_compat_dt[] __initconst = {
> >> >         "renesas,r7s72100",
> >> >         NULL,
> >> > @@ -29,4 +58,5 @@ DT_MACHINE_START(R7S72100_DT, "Generic R7S72100
> >> (Flattened Device Tree)")
> >> >         .init_early     = shmobile_init_delay,
> >> >         .init_late      = shmobile_init_late,
> >> >         .dt_compat      = r7s72100_boards_compat_dt,
> >> > +       .restart        = r7s72100_restart,
> >> >  MACHINE_END
> >>
> >> Perhaps unsurprisingly, I'd recommend writing a watchdog driver instead.
> >> drivers/watchdog/renesas_wdt.c (currently supporting R-Car Gen3 only)
> >> may serve as an example.
> >
> > Why do you guys always make things more difficult for me... ;)
> >
> > To be clear, you are recommending I make a WDT timer driver (and not
> > use .restart) and that will reset the system?
> >
> > Or, make a WDT driver just so I can steal the base address?
> 
> I meant a WDT timer driver. If the watchdog driver provides a .restart
> callback, it will be used for system reset (hmm, renesas_wdt.c no longer
> has the .restart callback, as per arm64 "system reset must be implemented
> using PSCI"-policy).



> > v2:
> > * changed DT bindings from 'wdt-reset' to 'rza-wdt'
> > * changed hard coded register values to defines
> > * added msleep to while(1) loop
> 
> Sure you can sleep here ?

As per Geert's review:

On Wednesday, February 15, 2017, Geert Uytterhoeven wrote:
> > +
> > +       /* Wait for WDT overflow */
> > +       while (1)
> > +               ;
> 
> This burns CPU, and thus power, albeit for a very short time.
> Perhaps add an msleep() to the loop body?

Note that you only have 7.7us before the restart occurs, so probably
not much sleeping will be going on.


Chris

Re: [PATCH v2 1/3] power: reset: Add Renesas reset driver

[Guenter Roeck]

On Thu, Feb 16, 2017 at 06:40:05PM +0000, Chris Brandt wrote:
> On Thursday, February 16, 2017, Guenter Roeck wrote:
> > On Thu, Feb 16, 2017 at 12:23:18PM -0500, Chris Brandt wrote:
> > > Some Renesas SoCs do not have a reset register and the only way to do a SW
> > > controlled reset is to use the watchdog timer. Additionally, since all the
> > > WDT timeout options are so quick, a system reset is about the only thing
> > > it's good for.
> > >
> > > Signed-off-by: Chris Brandt <chris.brandt@renesas.com> Reviewed-by: Geert
> > > Uytterhoeven <geert+renesas@glider.be>
> > 
> > Wondering - why not use the watchdog driver and the infrastructure provided
> > by the watchdog core (ie the restart callback) instead ?
> 
> That was Geert's first suggestion, but then he said:
> 
> On Friday, February 10, 2017, Geert Uytterhoeven wrote:
> > On Fri, Feb 10, 2017 at 3:59 PM, Chris Brandt <Chris.Brandt@renesas.com>
> > wrote:
> > > On Friday, February 10, 2017, Geert Uytterhoeven wrote:
> > >> >  static const char *const r7s72100_boards_compat_dt[] __initconst = {
> > >> >  "renesas,r7s72100", NULL, @@ -29,4 +58,5 @@
> > >> >  DT_MACHINE_START(R7S72100_DT, "Generic R7S72100
> > >> (Flattened Device Tree)")
> > >> >         .init_early     = shmobile_init_delay, .init_late      =
> > >> >         shmobile_init_late, .dt_compat      =
> > >> >         r7s72100_boards_compat_dt, +       .restart        =
> > >> >         r7s72100_restart, MACHINE_END
> > >>
> > >> Perhaps unsurprisingly, I'd recommend writing a watchdog driver instead.
> > >> drivers/watchdog/renesas_wdt.c (currently supporting R-Car Gen3 only) may
> > >> serve as an example.
> > >
> > > Why do you guys always make things more difficult for me... ;)
> > >
> > > To be clear, you are recommending I make a WDT timer driver (and not use
> > > .restart) and that will reset the system?
> > >
> > > Or, make a WDT driver just so I can steal the base address?
> > 
> > I meant a WDT timer driver. If the watchdog driver provides a .restart
> > callback, it will be used for system reset (hmm, renesas_wdt.c no longer has
> > the .restart callback, as per arm64 "system reset must be implemented using
> > PSCI"-policy).
> 

Hmm, ok. Guess I don't have to understand that you can not use the watchdog
driver because of the above, but implementing exactly the same functionality
in a separate driver is ok.

[ I am sure I am missing something here, so just ignore what I am saying ]

> 
> 
> > > v2: * changed DT bindings from 'wdt-reset' to 'rza-wdt' * changed hard
> > > coded register values to defines * added msleep to while(1) loop
> > 
> > Sure you can sleep here ?
> 
> As per Geert's review:
> 
> On Wednesday, February 15, 2017, Geert Uytterhoeven wrote:
> > > + +       /* Wait for WDT overflow */ +       while (1) +               ;
> > 
> > This burns CPU, and thus power, albeit for a very short time.  Perhaps add
> > an msleep() to the loop body?
> 
> Note that you only have 7.7us before the restart occurs, so probably not much
> sleeping will be going on.
> 
Let me rephrase my question. Is it guaranteed that you _can_ sleep in this
function, or in other words that it is guaranteed that interrupts are enabled ?

Thanks,
Guenter

RE: [PATCH v2 1/3] power: reset: Add Renesas reset driver

[Chris Brandt]

On Thursday, February 16, 2017, Guenter Roeck wrote:
> Hmm, ok. Guess I don't have to understand that you can not use the
> watchdog driver because of the above, but implementing exactly the same
> functionality in a separate driver is ok.
> 
> [ I am sure I am missing something here, so just ignore what I am saying ]

Honestly, I don't have a handle on all the latest 'policies and procedures'
for all the various subsystems. All I know is that I want to figure out how
to execute my 5 lines of code to reset the system when the user types "reboot"
on the command line.

If this WDT had a timeout longer than 125ms, I would make a real watchdog driver
out of it. But at the moment, it just serves as the only way to reset the chip.
Historically, this was the only way to reset a SH4 device...and we just lived
with that fact. When Renesas moved from SH4 to ARM, a lot of the design teams
just kept the same philosophy (and copied the HW blocks they knew already worked)


> > > > v2: * changed DT bindings from 'wdt-reset' to 'rza-wdt' * changed
> > > > hard coded register values to defines * added msleep to while(1)
> > > > loop
> > >
> > > Sure you can sleep here ?
> >
> > As per Geert's review:
> >
> > On Wednesday, February 15, 2017, Geert Uytterhoeven wrote:
> > > > + +       /* Wait for WDT overflow */ +       while (1)
> +               ;
> > >
> > > This burns CPU, and thus power, albeit for a very short time.
> > > Perhaps add an msleep() to the loop body?
> >
> > Note that you only have 7.7us before the restart occurs, so probably
> > not much sleeping will be going on.
> >
> Let me rephrase my question. Is it guaranteed that you _can_ sleep in this
> function, or in other words that it is guaranteed that interrupts are
> enabled ?

Hmm, I'm not sure if will actually 'sleep' or not. Regardless, interrupts or not,
in 7.7us, the internal reset circuit is going to get triggered and the whole system
is going to reboot not matter what is going on.

I know Geert's suggestion was in reference to saving power...but in reality it's
probably negligible when we are talking about 7.7us. The reboot is going to
automatically shut off all the peripherals clocks as well.


Chris

Re: [PATCH v2 1/3] power: reset: Add Renesas reset driver

[Guenter Roeck]

On 02/16/2017 06:00 PM, Chris Brandt wrote:
> On Thursday, February 16, 2017, Guenter Roeck wrote:
>> Hmm, ok. Guess I don't have to understand that you can not use the
>> watchdog driver because of the above, but implementing exactly the same
>> functionality in a separate driver is ok.
>>
>> [ I am sure I am missing something here, so just ignore what I am saying ]
>
> Honestly, I don't have a handle on all the latest 'policies and procedures'
> for all the various subsystems. All I know is that I want to figure out how
> to execute my 5 lines of code to reset the system when the user types "reboot"
> on the command line.
>
> If this WDT had a timeout longer than 125ms, I would make a real watchdog driver
> out of it. But at the moment, it just serves as the only way to reset the chip.
> Historically, this was the only way to reset a SH4 device...and we just lived
> with that fact. When Renesas moved from SH4 to ARM, a lot of the design teams
> just kept the same philosophy (and copied the HW blocks they knew already worked)
>

FWIW, the watchdog subsystem should support that easily, even with 125 ms hardware
timeout. We added that capability for that very purpose. That would only fail if
the system is stuck with interrupts disabled for more than 125 ms, which seems
unlikely. I think the gpio watchdog on some systems has a similar low hardware
timeout.

>
>>>>> v2: * changed DT bindings from 'wdt-reset' to 'rza-wdt' * changed
>>>>> hard coded register values to defines * added msleep to while(1)
>>>>> loop
>>>>
>>>> Sure you can sleep here ?
>>>
>>> As per Geert's review:
>>>
>>> On Wednesday, February 15, 2017, Geert Uytterhoeven wrote:
>>>>> + +       /* Wait for WDT overflow */ +       while (1)
>> +               ;
>>>>
>>>> This burns CPU, and thus power, albeit for a very short time.
>>>> Perhaps add an msleep() to the loop body?
>>>
>>> Note that you only have 7.7us before the restart occurs, so probably
>>> not much sleeping will be going on.
>>>
>> Let me rephrase my question. Is it guaranteed that you _can_ sleep in this
>> function, or in other words that it is guaranteed that interrupts are
>> enabled ?
>
> Hmm, I'm not sure if will actually 'sleep' or not. Regardless, interrupts or not,
> in 7.7us, the internal reset circuit is going to get triggered and the whole system
> is going to reboot not matter what is going on.
>

That isn't the point, really. You might get a WARN blob if msleep() is called
with interrupts disabled, but of course you won't really see that because it can
not be displayed in 7.7 us.

> I know Geert's suggestion was in reference to saving power...but in reality it's
> probably negligible when we are talking about 7.7us. The reboot is going to
> automatically shut off all the peripherals clocks as well.
>

Maybe udelay(10); would have been more acceptable (and appropriate).

Guenter

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

Re: [PATCH v2 1/3] power: reset: Add Renesas reset driver

[Guenter Roeck]

On 02/16/2017 06:00 PM, Chris Brandt wrote:
> On Thursday, February 16, 2017, Guenter Roeck wrote:
>> Hmm, ok. Guess I don't have to understand that you can not use the
>> watchdog driver because of the above, but implementing exactly the same
>> functionality in a separate driver is ok.
>>
>> [ I am sure I am missing something here, so just ignore what I am saying ]
>
> Honestly, I don't have a handle on all the latest 'policies and procedures'
> for all the various subsystems. All I know is that I want to figure out how
> to execute my 5 lines of code to reset the system when the user types "reboot"
> on the command line.
>
> If this WDT had a timeout longer than 125ms, I would make a real watchdog driver
> out of it. But at the moment, it just serves as the only way to reset the chip.
> Historically, this was the only way to reset a SH4 device...and we just lived
> with that fact. When Renesas moved from SH4 to ARM, a lot of the design teams
> just kept the same philosophy (and copied the HW blocks they knew already worked)
>

FWIW, the watchdog subsystem should support that easily, even with 125 ms hardware
timeout. We added that capability for that very purpose. That would only fail if
the system is stuck with interrupts disabled for more than 125 ms, which seems
unlikely. I think the gpio watchdog on some systems has a similar low hardware
timeout.

>
>>>>> v2: * changed DT bindings from 'wdt-reset' to 'rza-wdt' * changed
>>>>> hard coded register values to defines * added msleep to while(1)
>>>>> loop
>>>>
>>>> Sure you can sleep here ?
>>>
>>> As per Geert's review:
>>>
>>> On Wednesday, February 15, 2017, Geert Uytterhoeven wrote:
>>>>> + +       /* Wait for WDT overflow */ +       while (1)
>> +               ;
>>>>
>>>> This burns CPU, and thus power, albeit for a very short time.
>>>> Perhaps add an msleep() to the loop body?
>>>
>>> Note that you only have 7.7us before the restart occurs, so probably
>>> not much sleeping will be going on.
>>>
>> Let me rephrase my question. Is it guaranteed that you _can_ sleep in this
>> function, or in other words that it is guaranteed that interrupts are
>> enabled ?
>
> Hmm, I'm not sure if will actually 'sleep' or not. Regardless, interrupts or not,
> in 7.7us, the internal reset circuit is going to get triggered and the whole system
> is going to reboot not matter what is going on.
>

That isn't the point, really. You might get a WARN blob if msleep() is called
with interrupts disabled, but of course you won't really see that because it can
not be displayed in 7.7 us.

> I know Geert's suggestion was in reference to saving power...but in reality it's
> probably negligible when we are talking about 7.7us. The reboot is going to
> automatically shut off all the peripherals clocks as well.
>

Maybe udelay(10); would have been more acceptable (and appropriate).

Guenter

Re: [PATCH v2 1/3] power: reset: Add Renesas reset driver

[Geert Uytterhoeven]

Hi Günter,

On Fri, Feb 17, 2017 at 5:45 AM, Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org> wrote:
> On 02/16/2017 06:00 PM, Chris Brandt wrote:
>> On Thursday, February 16, 2017, Guenter Roeck wrote:
>> If this WDT had a timeout longer than 125ms, I would make a real watchdog
>> driver
>> out of it. But at the moment, it just serves as the only way to reset the
>> chip.
>> Historically, this was the only way to reset a SH4 device...and we just
>> lived
>> with that fact. When Renesas moved from SH4 to ARM, a lot of the design
>> teams
>> just kept the same philosophy (and copied the HW blocks they knew already
>> worked)
>
> FWIW, the watchdog subsystem should support that easily, even with 125 ms
> hardware
> timeout. We added that capability for that very purpose. That would only
> fail if
> the system is stuck with interrupts disabled for more than 125 ms, which
> seems
> unlikely. I think the gpio watchdog on some systems has a similar low
> hardware
> timeout.

I also thought 125ms was a bit short, but I'm happy to be proven wrong!
Let's make a real watchdog driver instead ;-)

>>>>>> v2: * changed DT bindings from 'wdt-reset' to 'rza-wdt' * changed
>>>>>> hard coded register values to defines * added msleep to while(1)
>>>>>> loop
>>>>>
>>>>> Sure you can sleep here ?
>>>>
>>>> As per Geert's review:
>>>>
>>>> On Wednesday, February 15, 2017, Geert Uytterhoeven wrote:
>>>>>>
>>>>>> + +       /* Wait for WDT overflow */ +       while (1)
>>>
>>> +               ;
>>>>>
>>>>> This burns CPU, and thus power, albeit for a very short time.
>>>>> Perhaps add an msleep() to the loop body?
>>>>
>>>> Note that you only have 7.7us before the restart occurs, so probably
>>>> not much sleeping will be going on.
>>>>
>>> Let me rephrase my question. Is it guaranteed that you _can_ sleep in
>>> this
>>> function, or in other words that it is guaranteed that interrupts are
>>> enabled ?
>>
>> Hmm, I'm not sure if will actually 'sleep' or not. Regardless, interrupts
>> or not,
>> in 7.7us, the internal reset circuit is going to get triggered and the
>> whole system
>> is going to reboot not matter what is going on.
>>
>
> That isn't the point, really. You might get a WARN blob if msleep() is
> called
> with interrupts disabled, but of course you won't really see that because it
> can
> not be displayed in 7.7 us.

If it's not 100% guaranteed that we cannot sleep, we should not use msleep(),
and stick to busy waiting.
On ARM, we could also use wfi().

>> I know Geert's suggestion was in reference to saving power...but in
>> reality it's
>> probably negligible when we are talking about 7.7us. The reboot is going
>> to
>> automatically shut off all the peripherals clocks as well.
>
> Maybe udelay(10); would have been more acceptable (and appropriate).

Inside the while (1) loop? That's the same as a plain "while (1) ;" ;-)
Or just udelay(10) and return, with the latter never happening if everything
goes well? Then the next restart handler will be tried, if available.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] power: reset: Add Renesas reset driver

[Geert Uytterhoeven]

Hi Günter,

On Fri, Feb 17, 2017 at 5:45 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> On 02/16/2017 06:00 PM, Chris Brandt wrote:
>> On Thursday, February 16, 2017, Guenter Roeck wrote:
>> If this WDT had a timeout longer than 125ms, I would make a real watchdog
>> driver
>> out of it. But at the moment, it just serves as the only way to reset the
>> chip.
>> Historically, this was the only way to reset a SH4 device...and we just
>> lived
>> with that fact. When Renesas moved from SH4 to ARM, a lot of the design
>> teams
>> just kept the same philosophy (and copied the HW blocks they knew already
>> worked)
>
> FWIW, the watchdog subsystem should support that easily, even with 125 ms
> hardware
> timeout. We added that capability for that very purpose. That would only
> fail if
> the system is stuck with interrupts disabled for more than 125 ms, which
> seems
> unlikely. I think the gpio watchdog on some systems has a similar low
> hardware
> timeout.

I also thought 125ms was a bit short, but I'm happy to be proven wrong!
Let's make a real watchdog driver instead ;-)

>>>>>> v2: * changed DT bindings from 'wdt-reset' to 'rza-wdt' * changed
>>>>>> hard coded register values to defines * added msleep to while(1)
>>>>>> loop
>>>>>
>>>>> Sure you can sleep here ?
>>>>
>>>> As per Geert's review:
>>>>
>>>> On Wednesday, February 15, 2017, Geert Uytterhoeven wrote:
>>>>>>
>>>>>> + +       /* Wait for WDT overflow */ +       while (1)
>>>
>>> +               ;
>>>>>
>>>>> This burns CPU, and thus power, albeit for a very short time.
>>>>> Perhaps add an msleep() to the loop body?
>>>>
>>>> Note that you only have 7.7us before the restart occurs, so probably
>>>> not much sleeping will be going on.
>>>>
>>> Let me rephrase my question. Is it guaranteed that you _can_ sleep in
>>> this
>>> function, or in other words that it is guaranteed that interrupts are
>>> enabled ?
>>
>> Hmm, I'm not sure if will actually 'sleep' or not. Regardless, interrupts
>> or not,
>> in 7.7us, the internal reset circuit is going to get triggered and the
>> whole system
>> is going to reboot not matter what is going on.
>>
>
> That isn't the point, really. You might get a WARN blob if msleep() is
> called
> with interrupts disabled, but of course you won't really see that because it
> can
> not be displayed in 7.7 us.

If it's not 100% guaranteed that we cannot sleep, we should not use msleep(),
and stick to busy waiting.
On ARM, we could also use wfi().

>> I know Geert's suggestion was in reference to saving power...but in
>> reality it's
>> probably negligible when we are talking about 7.7us. The reboot is going
>> to
>> automatically shut off all the peripherals clocks as well.
>
> Maybe udelay(10); would have been more acceptable (and appropriate).

Inside the while (1) loop? That's the same as a plain "while (1) ;" ;-)
Or just udelay(10) and return, with the latter never happening if everything
goes well? Then the next restart handler will be tried, if available.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 1/3] power: reset: Add Renesas reset driver

[Guenter Roeck]

On 02/17/2017 12:09 AM, Geert Uytterhoeven wrote:
> Hi Günter,
>
> On Fri, Feb 17, 2017 at 5:45 AM, Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org> wrote:
>> On 02/16/2017 06:00 PM, Chris Brandt wrote:
>>> On Thursday, February 16, 2017, Guenter Roeck wrote:
>>> If this WDT had a timeout longer than 125ms, I would make a real watchdog
>>> driver
>>> out of it. But at the moment, it just serves as the only way to reset the
>>> chip.
>>> Historically, this was the only way to reset a SH4 device...and we just
>>> lived
>>> with that fact. When Renesas moved from SH4 to ARM, a lot of the design
>>> teams
>>> just kept the same philosophy (and copied the HW blocks they knew already
>>> worked)
>>
>> FWIW, the watchdog subsystem should support that easily, even with 125 ms
>> hardware
>> timeout. We added that capability for that very purpose. That would only
>> fail if
>> the system is stuck with interrupts disabled for more than 125 ms, which
>> seems
>> unlikely. I think the gpio watchdog on some systems has a similar low
>> hardware
>> timeout.
>
> I also thought 125ms was a bit short, but I'm happy to be proven wrong!
> Let's make a real watchdog driver instead ;-)
>
>>>>>>> v2: * changed DT bindings from 'wdt-reset' to 'rza-wdt' * changed
>>>>>>> hard coded register values to defines * added msleep to while(1)
>>>>>>> loop
>>>>>>
>>>>>> Sure you can sleep here ?
>>>>>
>>>>> As per Geert's review:
>>>>>
>>>>> On Wednesday, February 15, 2017, Geert Uytterhoeven wrote:
>>>>>>>
>>>>>>> + +       /* Wait for WDT overflow */ +       while (1)
>>>>
>>>> +               ;
>>>>>>
>>>>>> This burns CPU, and thus power, albeit for a very short time.
>>>>>> Perhaps add an msleep() to the loop body?
>>>>>
>>>>> Note that you only have 7.7us before the restart occurs, so probably
>>>>> not much sleeping will be going on.
>>>>>
>>>> Let me rephrase my question. Is it guaranteed that you _can_ sleep in
>>>> this
>>>> function, or in other words that it is guaranteed that interrupts are
>>>> enabled ?
>>>
>>> Hmm, I'm not sure if will actually 'sleep' or not. Regardless, interrupts
>>> or not,
>>> in 7.7us, the internal reset circuit is going to get triggered and the
>>> whole system
>>> is going to reboot not matter what is going on.
>>>
>>
>> That isn't the point, really. You might get a WARN blob if msleep() is
>> called
>> with interrupts disabled, but of course you won't really see that because it
>> can
>> not be displayed in 7.7 us.
>
> If it's not 100% guaranteed that we cannot sleep, we should not use msleep(),
> and stick to busy waiting.
> On ARM, we could also use wfi().
>
>>> I know Geert's suggestion was in reference to saving power...but in
>>> reality it's
>>> probably negligible when we are talking about 7.7us. The reboot is going
>>> to
>>> automatically shut off all the peripherals clocks as well.
>>
>> Maybe udelay(10); would have been more acceptable (and appropriate).
>
> Inside the while (1) loop? That's the same as a plain "while (1) ;" ;-)
> Or just udelay(10) and return, with the latter never happening if everything
> goes well? Then the next restart handler will be tried, if available.
>

That is what I meant. Or use udelay(20) to be on the safe side.

Guenter

> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

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

Re: [PATCH v2 1/3] power: reset: Add Renesas reset driver

[Guenter Roeck]

On 02/17/2017 12:09 AM, Geert Uytterhoeven wrote:
> Hi Günter,
>
> On Fri, Feb 17, 2017 at 5:45 AM, Guenter Roeck <linux@roeck-us.net> wrote:
>> On 02/16/2017 06:00 PM, Chris Brandt wrote:
>>> On Thursday, February 16, 2017, Guenter Roeck wrote:
>>> If this WDT had a timeout longer than 125ms, I would make a real watchdog
>>> driver
>>> out of it. But at the moment, it just serves as the only way to reset the
>>> chip.
>>> Historically, this was the only way to reset a SH4 device...and we just
>>> lived
>>> with that fact. When Renesas moved from SH4 to ARM, a lot of the design
>>> teams
>>> just kept the same philosophy (and copied the HW blocks they knew already
>>> worked)
>>
>> FWIW, the watchdog subsystem should support that easily, even with 125 ms
>> hardware
>> timeout. We added that capability for that very purpose. That would only
>> fail if
>> the system is stuck with interrupts disabled for more than 125 ms, which
>> seems
>> unlikely. I think the gpio watchdog on some systems has a similar low
>> hardware
>> timeout.
>
> I also thought 125ms was a bit short, but I'm happy to be proven wrong!
> Let's make a real watchdog driver instead ;-)
>
>>>>>>> v2: * changed DT bindings from 'wdt-reset' to 'rza-wdt' * changed
>>>>>>> hard coded register values to defines * added msleep to while(1)
>>>>>>> loop
>>>>>>
>>>>>> Sure you can sleep here ?
>>>>>
>>>>> As per Geert's review:
>>>>>
>>>>> On Wednesday, February 15, 2017, Geert Uytterhoeven wrote:
>>>>>>>
>>>>>>> + +       /* Wait for WDT overflow */ +       while (1)
>>>>
>>>> +               ;
>>>>>>
>>>>>> This burns CPU, and thus power, albeit for a very short time.
>>>>>> Perhaps add an msleep() to the loop body?
>>>>>
>>>>> Note that you only have 7.7us before the restart occurs, so probably
>>>>> not much sleeping will be going on.
>>>>>
>>>> Let me rephrase my question. Is it guaranteed that you _can_ sleep in
>>>> this
>>>> function, or in other words that it is guaranteed that interrupts are
>>>> enabled ?
>>>
>>> Hmm, I'm not sure if will actually 'sleep' or not. Regardless, interrupts
>>> or not,
>>> in 7.7us, the internal reset circuit is going to get triggered and the
>>> whole system
>>> is going to reboot not matter what is going on.
>>>
>>
>> That isn't the point, really. You might get a WARN blob if msleep() is
>> called
>> with interrupts disabled, but of course you won't really see that because it
>> can
>> not be displayed in 7.7 us.
>
> If it's not 100% guaranteed that we cannot sleep, we should not use msleep(),
> and stick to busy waiting.
> On ARM, we could also use wfi().
>
>>> I know Geert's suggestion was in reference to saving power...but in
>>> reality it's
>>> probably negligible when we are talking about 7.7us. The reboot is going
>>> to
>>> automatically shut off all the peripherals clocks as well.
>>
>> Maybe udelay(10); would have been more acceptable (and appropriate).
>
> Inside the while (1) loop? That's the same as a plain "while (1) ;" ;-)
> Or just udelay(10) and return, with the latter never happening if everything
> goes well? Then the next restart handler will be tried, if available.
>

That is what I meant. Or use udelay(20) to be on the safe side.

Guenter

> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> --
> 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
>

--
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

Re: [PATCH v2 1/3] power: reset: Add Renesas reset driver

[Guenter Roeck]

On 02/17/2017 12:09 AM, Geert Uytterhoeven wrote:
> Hi Günter,
>
> On Fri, Feb 17, 2017 at 5:45 AM, Guenter Roeck <linux@roeck-us.net> wrote:
>> On 02/16/2017 06:00 PM, Chris Brandt wrote:
>>> On Thursday, February 16, 2017, Guenter Roeck wrote:
>>> If this WDT had a timeout longer than 125ms, I would make a real watchdog
>>> driver
>>> out of it. But at the moment, it just serves as the only way to reset the
>>> chip.
>>> Historically, this was the only way to reset a SH4 device...and we just
>>> lived
>>> with that fact. When Renesas moved from SH4 to ARM, a lot of the design
>>> teams
>>> just kept the same philosophy (and copied the HW blocks they knew already
>>> worked)
>>
>> FWIW, the watchdog subsystem should support that easily, even with 125 ms
>> hardware
>> timeout. We added that capability for that very purpose. That would only
>> fail if
>> the system is stuck with interrupts disabled for more than 125 ms, which
>> seems
>> unlikely. I think the gpio watchdog on some systems has a similar low
>> hardware
>> timeout.
>
> I also thought 125ms was a bit short, but I'm happy to be proven wrong!
> Let's make a real watchdog driver instead ;-)
>
>>>>>>> v2: * changed DT bindings from 'wdt-reset' to 'rza-wdt' * changed
>>>>>>> hard coded register values to defines * added msleep to while(1)
>>>>>>> loop
>>>>>>
>>>>>> Sure you can sleep here ?
>>>>>
>>>>> As per Geert's review:
>>>>>
>>>>> On Wednesday, February 15, 2017, Geert Uytterhoeven wrote:
>>>>>>>
>>>>>>> + +       /* Wait for WDT overflow */ +       while (1)
>>>>
>>>> +               ;
>>>>>>
>>>>>> This burns CPU, and thus power, albeit for a very short time.
>>>>>> Perhaps add an msleep() to the loop body?
>>>>>
>>>>> Note that you only have 7.7us before the restart occurs, so probably
>>>>> not much sleeping will be going on.
>>>>>
>>>> Let me rephrase my question. Is it guaranteed that you _can_ sleep in
>>>> this
>>>> function, or in other words that it is guaranteed that interrupts are
>>>> enabled ?
>>>
>>> Hmm, I'm not sure if will actually 'sleep' or not. Regardless, interrupts
>>> or not,
>>> in 7.7us, the internal reset circuit is going to get triggered and the
>>> whole system
>>> is going to reboot not matter what is going on.
>>>
>>
>> That isn't the point, really. You might get a WARN blob if msleep() is
>> called
>> with interrupts disabled, but of course you won't really see that because it
>> can
>> not be displayed in 7.7 us.
>
> If it's not 100% guaranteed that we cannot sleep, we should not use msleep(),
> and stick to busy waiting.
> On ARM, we could also use wfi().
>
>>> I know Geert's suggestion was in reference to saving power...but in
>>> reality it's
>>> probably negligible when we are talking about 7.7us. The reboot is going
>>> to
>>> automatically shut off all the peripherals clocks as well.
>>
>> Maybe udelay(10); would have been more acceptable (and appropriate).
>
> Inside the while (1) loop? That's the same as a plain "while (1) ;" ;-)
> Or just udelay(10) and return, with the latter never happening if everything
> goes well? Then the next restart handler will be tried, if available.
>

That is what I meant. Or use udelay(20) to be on the safe side.

Guenter

> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> --
> 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
>

Re: [PATCH v2 2/3] watchdog: renesas-wdt: add support for rza

[Geert Uytterhoeven]

On Thu, Feb 16, 2017 at 6:23 PM, Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> wrote:
> Describe the WDT hardware in the RZ/A series.
>
> Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/3] watchdog: renesas-wdt: add support for rza

[Geert Uytterhoeven]

On Thu, Feb 16, 2017 at 6:23 PM, Chris Brandt <chris.brandt@renesas.com> wrote:
> Describe the WDT hardware in the RZ/A series.
>
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 3/3] ARM: dts: r7s72100: Add reset handler

[Geert Uytterhoeven]

On Thu, Feb 16, 2017 at 6:23 PM, Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> wrote:
> For the RZ/A1, the only way to do a reset is to overflow the WDT.
>
> Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/3] ARM: dts: r7s72100: Add reset handler

[Geert Uytterhoeven]

On Thu, Feb 16, 2017 at 6:23 PM, Chris Brandt <chris.brandt@renesas.com> wrote:
> For the RZ/A1, the only way to do a reset is to overflow the WDT.
>
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH v2 1/3] power: reset: Add Renesas reset driver

[Chris Brandt]

Hi Günter and Geert,

Catching up on the subject (because I was sleeping here in the US...)


On Friday, February 17, 2017, Geert Uytterhoeven wrote:
> > FWIW, the watchdog subsystem should support that easily, even with 125
> > ms hardware timeout. We added that capability for that very purpose.
> > That would only fail if the system is stuck with interrupts disabled
> > for more than 125 ms, which seems unlikely. I think the gpio watchdog
> > on some systems has a similar low hardware timeout.
> 
> I also thought 125ms was a bit short, but I'm happy to be proven wrong!
> Let's make a real watchdog driver instead ;-)

OK, I'll see about switching over to a real watchdog driver. As long as I
can get my reset handler, I'll be happy.

Besides, if I get the 8-bit counter register changed to 16-bit (a useful
WDT), then I already have a WDT made.

The good thing is that the DT binding will stay exactly the same, I just
need a new driver.

FWIW, if I was making a real product I would always have a WDT running (but
with a 1-3 second timeout just to make sure I was completely dead).

# side note, I tell people to set up a DMA with a timer trigger and
automatically feed the WDT that way, then they can set the DMA count to
whatever failsafe timeout they want. Not sure if I could submit that kind
of hackery in an upstream driver.


> >>> I know Geert's suggestion was in reference to saving power...but in
> >>> reality it's probably negligible when we are talking about 7.7us.
> >>> The reboot is going to automatically shut off all the peripherals
> >>> clocks as well.
> >>
> >> Maybe udelay(10); would have been more acceptable (and appropriate).
> >
> > Inside the while (1) loop? That's the same as a plain "while (1) ;"
> > ;-) Or just udelay(10) and return, with the latter never happening if
> > everything goes well? Then the next restart handler will be tried, if
> available.
> >
> 
> That is what I meant. Or use udelay(20) to be on the safe side.

OK. Sounds like we agree on:

	/* Start timer */
	writew(WTCSR_MAGIC | WTSCR_WT | WTSCR_TME, base + WTCSR);

	/* Wait for WDT overflow (reset) */
	Udelay(20);

	return NOTIFY_DONE;
}


FYI, I'll be at ELC next week, so I might not get to this until after then.

Thanks for the discussion.

Cheers

Chris

RE: [PATCH v2 1/3] power: reset: Add Renesas reset driver

[Chris Brandt]

Hi Günter and Geert,

Catching up on the subject (because I was sleeping here in the US...)


On Friday, February 17, 2017, Geert Uytterhoeven wrote:
> > FWIW, the watchdog subsystem should support that easily, even with 125
> > ms hardware timeout. We added that capability for that very purpose.
> > That would only fail if the system is stuck with interrupts disabled
> > for more than 125 ms, which seems unlikely. I think the gpio watchdog
> > on some systems has a similar low hardware timeout.
> 
> I also thought 125ms was a bit short, but I'm happy to be proven wrong!
> Let's make a real watchdog driver instead ;-)

OK, I'll see about switching over to a real watchdog driver. As long as I
can get my reset handler, I'll be happy.

Besides, if I get the 8-bit counter register changed to 16-bit (a useful
WDT), then I already have a WDT made.

The good thing is that the DT binding will stay exactly the same, I just
need a new driver.

FWIW, if I was making a real product I would always have a WDT running (but
with a 1-3 second timeout just to make sure I was completely dead).

# side note, I tell people to set up a DMA with a timer trigger and
automatically feed the WDT that way, then they can set the DMA count to
whatever failsafe timeout they want. Not sure if I could submit that kind
of hackery in an upstream driver.


> >>> I know Geert's suggestion was in reference to saving power...but in
> >>> reality it's probably negligible when we are talking about 7.7us.
> >>> The reboot is going to automatically shut off all the peripherals
> >>> clocks as well.
> >>
> >> Maybe udelay(10); would have been more acceptable (and appropriate).
> >
> > Inside the while (1) loop? That's the same as a plain "while (1) ;"
> > ;-) Or just udelay(10) and return, with the latter never happening if
> > everything goes well? Then the next restart handler will be tried, if
> available.
> >
> 
> That is what I meant. Or use udelay(20) to be on the safe side.

OK. Sounds like we agree on:

	/* Start timer */
	writew(WTCSR_MAGIC | WTSCR_WT | WTSCR_TME, base + WTCSR);

	/* Wait for WDT overflow (reset) */
	Udelay(20);

	return NOTIFY_DONE;
}


FYI, I'll be at ELC next week, so I might not get to this until after then.

Thanks for the discussion.

Cheers

Chris

RE: [PATCH v2 1/3] power: reset: Add Renesas reset driver

[Chris Brandt]

Hello Geert and Guenter,

On Thursday, February 16, 2017, Guenter Roeck wrote:
> FWIW, the watchdog subsystem should support that easily, even with 125 ms
> hardware timeout. We added that capability for that very purpose. That
> would only fail if the system is stuck with interrupts disabled for more
> than 125 ms, which seems unlikely. I think the gpio watchdog on some
> systems has a similar low hardware timeout.


While I'm going to try that for the RZ/A1, I have a question for you guys
(or anyone else that has an opinion about end applications)


If I were going to make a request to the chip designers to make the timeout longer
for the next RZ/A chip, what would be a good max timeout for common Linux applications?

Looking through the drivers in the watchdog directly, I see default timeouts of 20,
30, 60, and 120 seconds.

Thank you,
Chris

Re: [PATCH v2 1/3] power: reset: Add Renesas reset driver

[Guenter Roeck]

On Wed, Feb 22, 2017 at 01:35:12PM +0000, Chris Brandt wrote:
> Hello Geert and Guenter,
> 
> On Thursday, February 16, 2017, Guenter Roeck wrote:
> > FWIW, the watchdog subsystem should support that easily, even with 125 ms
> > hardware timeout. We added that capability for that very purpose. That
> > would only fail if the system is stuck with interrupts disabled for more
> > than 125 ms, which seems unlikely. I think the gpio watchdog on some
> > systems has a similar low hardware timeout.
> 
> 
> While I'm going to try that for the RZ/A1, I have a question for you guys
> (or anyone else that has an opinion about end applications)
> 
> 
> If I were going to make a request to the chip designers to make the timeout longer
> for the next RZ/A chip, what would be a good max timeout for common Linux applications?
> 
> Looking through the drivers in the watchdog directly, I see default timeouts of 20,
> 30, 60, and 120 seconds.
> 
30 and 60 are pretty common for default timeouts, though I personally think
they are a bit long. If you want direct HW support, a maximum of 120 seconds
should be sufficient though not really necessary anymore since the core
supports virtual timeouts. A maximum of at least 30 seconds would be needed
if the watchdog is supposed to run at boot time (ie if it is enabled by
ROMMON/BIOS and kept running by the kernel).

Hope this helps,
Guenter

RE: [PATCH v2 1/3] power: reset: Add Renesas reset driver

[Chris Brandt]

Hello Guenter,

On Wednesday, February 22, 2017, Guenter Roeck wrote:
> > Looking through the drivers in the watchdog directly, I see default
> > timeouts of 20, 30, 60, and 120 seconds.
> >
> 30 and 60 are pretty common for default timeouts, though I personally
> think they are a bit long. If you want direct HW support, a maximum of 120
> seconds should be sufficient though not really necessary anymore since the
> core supports virtual timeouts. A maximum of at least 30 seconds would be
> needed if the watchdog is supposed to run at boot time (ie if it is
> enabled by ROMMON/BIOS and kept running by the kernel).


Thank you!
Good point about the boot time.

Chris

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

RE: [PATCH v2 1/3] power: reset: Add Renesas reset driver

[Chris Brandt]

Hello Guenter,

On Wednesday, February 22, 2017, Guenter Roeck wrote:
> > Looking through the drivers in the watchdog directly, I see default
> > timeouts of 20, 30, 60, and 120 seconds.
> >
> 30 and 60 are pretty common for default timeouts, though I personally
> think they are a bit long. If you want direct HW support, a maximum of 120
> seconds should be sufficient though not really necessary anymore since the
> core supports virtual timeouts. A maximum of at least 30 seconds would be
> needed if the watchdog is supposed to run at boot time (ie if it is
> enabled by ROMMON/BIOS and kept running by the kernel).


Thank you!
Good point about the boot time.

Chris

Re: [PATCH v2 2/3] watchdog: renesas-wdt: add support for rza

[Rob Herring]

On Thu, Feb 16, 2017 at 12:23:19PM -0500, Chris Brandt wrote:
> Describe the WDT hardware in the RZ/A series.
> 
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
> v2:
> * added to renesas-wdt.txt instead of creating a new file
> * changed commit title
> * added "renesas,rza-wdt" as a fallback
> * added interrupts property
> ---
>  Documentation/devicetree/bindings/watchdog/renesas-wdt.txt | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

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

Re: [PATCH v2 3/3] ARM: dts: r7s72100: Add reset handler

[Simon Horman]

On Fri, Feb 17, 2017 at 11:29:25AM +0100, Geert Uytterhoeven wrote:
> On Thu, Feb 16, 2017 at 6:23 PM, Chris Brandt <chris.brandt@renesas.com> wrote:
> > For the RZ/A1, the only way to do a reset is to overflow the WDT.
> >
> > Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Hi Chris,

while going through old patches I noticed that I seem to have missed this
one. It looks like it needs a rebase. If it is still applicable could you
please consider doing so and reposting.

Thanks

RE: [PATCH v2 3/3] ARM: dts: r7s72100: Add reset handler

[Chris Brandt]

Hi Simon,

On Wednesday, August 30, 2017, Simon Horman wrote:
> On Fri, Feb 17, 2017 at 11:29:25AM +0100, Geert Uytterhoeven wrote:
> > On Thu, Feb 16, 2017 at 6:23 PM, Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> wrote:
> > > For the RZ/A1, the only way to do a reset is to overflow the WDT.
> > >
> > > Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> >
> > Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
> 
> Hi Chris,
> 
> while going through old patches I noticed that I seem to have missed this
> one. It looks like it needs a rebase. If it is still applicable could you
> please consider doing so and reposting.


It's in -next already:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/arch/arm/boot/dts/r7s72100.dtsi?h=next-20170829&id=69ed50de582eff6307fd3fa050fdc505731f0a2d

Doesn't that mean it's queued up for 4.14?

Or, somehow it didn't make it into the pull request for Arnd's tree?


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

RE: [PATCH v2 3/3] ARM: dts: r7s72100: Add reset handler

[Chris Brandt]

Hi Simon,

On Wednesday, August 30, 2017, Simon Horman wrote:
> On Fri, Feb 17, 2017 at 11:29:25AM +0100, Geert Uytterhoeven wrote:
> > On Thu, Feb 16, 2017 at 6:23 PM, Chris Brandt <chris.brandt@renesas.com>
> wrote:
> > > For the RZ/A1, the only way to do a reset is to overflow the WDT.
> > >
> > > Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> >
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Hi Chris,
> 
> while going through old patches I noticed that I seem to have missed this
> one. It looks like it needs a rebase. If it is still applicable could you
> please consider doing so and reposting.


It's in -next already:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/arch/arm/boot/dts/r7s72100.dtsi?h=next-20170829&id=69ed50de582eff6307fd3fa050fdc505731f0a2d

Doesn't that mean it's queued up for 4.14?

Or, somehow it didn't make it into the pull request for Arnd's tree?


Chris

Re: [PATCH v2 3/3] ARM: dts: r7s72100: Add reset handler

[Simon Horman]

On Wed, Aug 30, 2017 at 01:22:32PM +0000, Chris Brandt wrote:
> Hi Simon,
> 
> On Wednesday, August 30, 2017, Simon Horman wrote:
> > On Fri, Feb 17, 2017 at 11:29:25AM +0100, Geert Uytterhoeven wrote:
> > > On Thu, Feb 16, 2017 at 6:23 PM, Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> > wrote:
> > > > For the RZ/A1, the only way to do a reset is to overflow the WDT.
> > > >
> > > > Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> > >
> > > Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
> > 
> > Hi Chris,
> > 
> > while going through old patches I noticed that I seem to have missed this
> > one. It looks like it needs a rebase. If it is still applicable could you
> > please consider doing so and reposting.
> 
> 
> It's in -next already:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/arch/arm/boot/dts/r7s72100.dtsi?h=next-20170829&id=69ed50de582eff6307fd3fa050fdc505731f0a2d
> 
> Doesn't that mean it's queued up for 4.14?
> 
> Or, somehow it didn't make it into the pull request for Arnd's tree?

Thanks, I now see that it is included in v4.12.
Sorry for the noise.
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/3] ARM: dts: r7s72100: Add reset handler

[Simon Horman]

On Wed, Aug 30, 2017 at 01:22:32PM +0000, Chris Brandt wrote:
> Hi Simon,
> 
> On Wednesday, August 30, 2017, Simon Horman wrote:
> > On Fri, Feb 17, 2017 at 11:29:25AM +0100, Geert Uytterhoeven wrote:
> > > On Thu, Feb 16, 2017 at 6:23 PM, Chris Brandt <chris.brandt@renesas.com>
> > wrote:
> > > > For the RZ/A1, the only way to do a reset is to overflow the WDT.
> > > >
> > > > Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> > >
> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > 
> > Hi Chris,
> > 
> > while going through old patches I noticed that I seem to have missed this
> > one. It looks like it needs a rebase. If it is still applicable could you
> > please consider doing so and reposting.
> 
> 
> It's in -next already:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/arch/arm/boot/dts/r7s72100.dtsi?h=next-20170829&id=69ed50de582eff6307fd3fa050fdc505731f0a2d
> 
> Doesn't that mean it's queued up for 4.14?
> 
> Or, somehow it didn't make it into the pull request for Arnd's tree?

Thanks, I now see that it is included in v4.12.
Sorry for the noise.