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

[PATCH 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.


Chris Brandt (3):
  power: reset: Add Renesas reset driver
  dt-bindings: power: reset: add document for renesas-reset driver
  ARM: dts: r7s72100: Add reset handler

 .../bindings/power/reset/renesas-reset.txt         |  15 +++
 arch/arm/boot/dts/r7s72100.dtsi                    |   5 +
 drivers/power/reset/Kconfig                        |   9 ++
 drivers/power/reset/Makefile                       |   1 +
 drivers/power/reset/renesas-reset.c                | 103 +++++++++++++++++++++
 5 files changed, 133 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/reset/renesas-reset.txt
 create mode 100644 drivers/power/reset/renesas-reset.c

-- 
2.10.1

[PATCH 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>
---
 drivers/power/reset/Kconfig         |   9 ++++
 drivers/power/reset/Makefile        |   1 +
 drivers/power/reset/renesas-reset.c | 103 ++++++++++++++++++++++++++++++++++++
 3 files changed, 113 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..dede029
--- /dev/null
+++ b/drivers/power/reset/renesas-reset.c
@@ -0,0 +1,103 @@
+/*
+ * 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/io.h>
+#include <linux/module.h>
+#include <linux/notifier.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/printk.h>
+#include <linux/reboot.h>
+
+/* Watchdog Timer Registers */
+#define WTCSR 0
+#define WTCNT 2
+#define WRCSR 4
+
+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) */
+	readw(base + WRCSR);
+
+	writew(0xA500, base + WRCSR);	/* Clear WOVF */
+	writew(0x5A5F, base + WRCSR);	/* Reset Enable */
+	writew(0x5A00, base + WTCNT);	/* Counter to 00 */
+	writew(0xA578, base + WTCSR);	/* Start timer */
+
+	/* Wait for WDT overflow */
+	while (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,wdt-reset", },
+	{ /* 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 2/3] dt-bindings: power: reset: add document for renesas-reset driver

[Chris Brandt]

Add device tree bindings document for renesas-reset driver.
This driver uses the WDT hardware to issue an immediate reset.

Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
---
 .../devicetree/bindings/power/reset/renesas-reset.txt     | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/reset/renesas-reset.txt

diff --git a/Documentation/devicetree/bindings/power/reset/renesas-reset.txt b/Documentation/devicetree/bindings/power/reset/renesas-reset.txt
new file mode 100644
index 0000000..241722d
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/renesas-reset.txt
@@ -0,0 +1,15 @@
+Device-Tree bindings for Renesas WDT reset functionality
+
+Required properties:
+  - compatible: must be one or more of the following:
+    - "renesas,r7s72100-reset" for the r7s72100 SoC
+    - "renesas,wdt-reset"
+                This is a fallback for the above renesas,*-reset entries
+
+  - reg: base address and length of the WDT register block
+
+Example node:
+	wdt: timer@fcfe0000 {
+		compatible = "renesas,r7s72100-reset", "renesas,wdt-reset";
+		reg = <0xfcfe0000 0x6>;
+	};
-- 
2.10.1


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

[PATCH 2/3] dt-bindings: power: reset: add document for renesas-reset driver

[Chris Brandt]

Add device tree bindings document for renesas-reset driver.
This driver uses the WDT hardware to issue an immediate reset.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
 .../devicetree/bindings/power/reset/renesas-reset.txt     | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/reset/renesas-reset.txt

diff --git a/Documentation/devicetree/bindings/power/reset/renesas-reset.txt b/Documentation/devicetree/bindings/power/reset/renesas-reset.txt
new file mode 100644
index 0000000..241722d
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/renesas-reset.txt
@@ -0,0 +1,15 @@
+Device-Tree bindings for Renesas WDT reset functionality
+
+Required properties:
+  - compatible: must be one or more of the following:
+    - "renesas,r7s72100-reset" for the r7s72100 SoC
+    - "renesas,wdt-reset"
+                This is a fallback for the above renesas,*-reset entries
+
+  - reg: base address and length of the WDT register block
+
+Example node:
+	wdt: timer@fcfe0000 {
+		compatible = "renesas,r7s72100-reset", "renesas,wdt-reset";
+		reg = <0xfcfe0000 0x6>;
+	};
-- 
2.10.1

[PATCH 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>
---
 arch/arm/boot/dts/r7s72100.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
index 614ba79..6fb4ef4 100644
--- a/arch/arm/boot/dts/r7s72100.dtsi
+++ b/arch/arm/boot/dts/r7s72100.dtsi
@@ -371,6 +371,11 @@
 			<0xe8202000 0x1000>;
 	};
 
+	wdt: timer@fcfe0000 {
+		compatible = "renesas,r7s72100-reset", "renesas,wdt-reset";
+		reg = <0xfcfe0000 0x6>;
+	};
+
 	i2c0: i2c@fcfee000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-- 
2.10.1

Re: [PATCH 2/3] dt-bindings: power: reset: add document for renesas-reset driver

[Geert Uytterhoeven]

Hi Chris,

On Mon, Feb 13, 2017 at 7:25 PM, Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> wrote:
> Add device tree bindings document for renesas-reset driver.
> This driver uses the WDT hardware to issue an immediate reset.
>
> Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> ---
>  .../devicetree/bindings/power/reset/renesas-reset.txt     | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/reset/renesas-reset.txt
>
> diff --git a/Documentation/devicetree/bindings/power/reset/renesas-reset.txt b/Documentation/devicetree/bindings/power/reset/renesas-reset.txt
> new file mode 100644
> index 0000000..241722d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/renesas-reset.txt
> @@ -0,0 +1,15 @@
> +Device-Tree bindings for Renesas WDT reset functionality

Please drop "reset", as this is a watchdog device.

> +Required properties:
> +  - compatible: must be one or more of the following:
> +    - "renesas,r7s72100-reset" for the r7s72100 SoC
> +    - "renesas,wdt-reset"
> +                This is a fallback for the above renesas,*-reset entries

Please use "renesas,r7s72100-wdt". DT describes the hardware (watchdog),
not the software policy (use it for reset only, as a max. timeout of 125 ms is
too short for a usable watchdog).

> +  - reg: base address and length of the WDT register block
> +
> +Example node:
> +       wdt: timer@fcfe0000 {
> +               compatible = "renesas,r7s72100-reset", "renesas,wdt-reset";
> +               reg = <0xfcfe0000 0x6>;
> +       };

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 2/3] dt-bindings: power: reset: add document for renesas-reset driver

[Geert Uytterhoeven]

Hi Chris,

On Mon, Feb 13, 2017 at 7:25 PM, Chris Brandt <chris.brandt@renesas.com> wrote:
> Add device tree bindings document for renesas-reset driver.
> This driver uses the WDT hardware to issue an immediate reset.
>
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
>  .../devicetree/bindings/power/reset/renesas-reset.txt     | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/reset/renesas-reset.txt
>
> diff --git a/Documentation/devicetree/bindings/power/reset/renesas-reset.txt b/Documentation/devicetree/bindings/power/reset/renesas-reset.txt
> new file mode 100644
> index 0000000..241722d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/renesas-reset.txt
> @@ -0,0 +1,15 @@
> +Device-Tree bindings for Renesas WDT reset functionality

Please drop "reset", as this is a watchdog device.

> +Required properties:
> +  - compatible: must be one or more of the following:
> +    - "renesas,r7s72100-reset" for the r7s72100 SoC
> +    - "renesas,wdt-reset"
> +                This is a fallback for the above renesas,*-reset entries

Please use "renesas,r7s72100-wdt". DT describes the hardware (watchdog),
not the software policy (use it for reset only, as a max. timeout of 125 ms is
too short for a usable watchdog).

> +  - reg: base address and length of the WDT register block
> +
> +Example node:
> +       wdt: timer@fcfe0000 {
> +               compatible = "renesas,r7s72100-reset", "renesas,wdt-reset";
> +               reg = <0xfcfe0000 0x6>;
> +       };

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 2/3] dt-bindings: power: reset: add document for renesas-reset driver

[Chris Brandt]

Hi Geert,

Thank you for your review!


Chris



On Tuesday, February 14, 2017, Geert Uytterhoeven wrote:
> On Mon, Feb 13, 2017 at 7:25 PM, Chris Brandt <chris.brandt@renesas.com>
> wrote:
> > Add device tree bindings document for renesas-reset driver.
> > This driver uses the WDT hardware to issue an immediate reset.
> >
> > Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> > ---
> >  .../devicetree/bindings/power/reset/renesas-reset.txt     | 15
> +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/power/reset/renesas-reset.txt
> >
> > diff --git
> > a/Documentation/devicetree/bindings/power/reset/renesas-reset.txt
> > b/Documentation/devicetree/bindings/power/reset/renesas-reset.txt
> > new file mode 100644
> > index 0000000..241722d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/reset/renesas-reset.txt
> > @@ -0,0 +1,15 @@
> > +Device-Tree bindings for Renesas WDT reset functionality
> 
> Please drop "reset", as this is a watchdog device.
> 
> > +Required properties:
> > +  - compatible: must be one or more of the following:
> > +    - "renesas,r7s72100-reset" for the r7s72100 SoC
> > +    - "renesas,wdt-reset"
> > +                This is a fallback for the above renesas,*-reset
> > +entries
> 
> Please use "renesas,r7s72100-wdt". DT describes the hardware (watchdog),
> not the software policy (use it for reset only, as a max. timeout of 125
> ms is too short for a usable watchdog).
> 
> > +  - reg: base address and length of the WDT register block
> > +
> > +Example node:
> > +       wdt: timer@fcfe0000 {
> > +               compatible = "renesas,r7s72100-reset", "renesas,wdt-
> reset";
> > +               reg = <0xfcfe0000 0x6>;
> > +       };
> 
> 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 1/3] power: reset: Add Renesas reset driver

[Geert Uytterhoeven]

Hi Chris,

On Mon, Feb 13, 2017 at 7:25 PM, Chris Brandt <chris.brandt@renesas.com> 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>

> --- /dev/null
> +++ b/drivers/power/reset/renesas-reset.c
> @@ -0,0 +1,103 @@

> +/* Watchdog Timer Registers */
> +#define WTCSR 0
> +#define WTCNT 2
> +#define WRCSR 4
> +
> +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) */
> +       readw(base + WRCSR);
> +
> +       writew(0xA500, base + WRCSR);   /* Clear WOVF */
> +       writew(0x5A5F, base + WRCSR);   /* Reset Enable */
> +       writew(0x5A00, base + WTCNT);   /* Counter to 00 */
> +       writew(0xA578, base + WTCSR);   /* Start timer */

Hardcoded register values? Please use #defines.
Yes, the 0xa5 and 0x5a are magic values, but the other bits aren't.

> +
> +       /* 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?

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 3/3] ARM: dts: r7s72100: Add reset handler

[Geert Uytterhoeven]

On Mon, Feb 13, 2017 at 7:25 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>
> ---
>  arch/arm/boot/dts/r7s72100.dtsi | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
> index 614ba79..6fb4ef4 100644
> --- a/arch/arm/boot/dts/r7s72100.dtsi
> +++ b/arch/arm/boot/dts/r7s72100.dtsi
> @@ -371,6 +371,11 @@
>                         <0xe8202000 0x1000>;
>         };
>
> +       wdt: timer@fcfe0000 {
> +               compatible = "renesas,r7s72100-reset", "renesas,wdt-reset";
> +               reg = <0xfcfe0000 0x6>;

No clocks property, pointing to p0_clk?
No interrupts property?
Yes, those should be documented in the bindings, too.

> +       };

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 2/3] dt-bindings: power: reset: add document for renesas-reset driver

[Chris Brandt]

Hi Geert,

On Tuesday, February 14, 2017, Geert Uytterhoeven wrote:
> > +Required properties:
> > +  - compatible: must be one or more of the following:
> > +    - "renesas,r7s72100-reset" for the r7s72100 SoC
> > +    - "renesas,wdt-reset"
> > +                This is a fallback for the above renesas,*-reset
> > +entries
> 
> Please use "renesas,r7s72100-wdt". DT describes the hardware (watchdog),
> not the software policy (use it for reset only, as a max. timeout of 125
> ms is too short for a usable watchdog).


I had a look at:

  Documentation/devicetree/bindings/watchdog/renesas-wdt.txt

Which has:

Required properties:
- compatible : Should be "renesas,<soctype>-wdt", and
	       "renesas,rcar-gen3-wdt" as fallback.
	       Examples with soctypes are:
	         - "renesas,r8a7795-wdt" (R-Car H3)
	         - "renesas,r8a7796-wdt" (R-Car M3-W)


So in my 'renesas-reset.txt' should I do:

A. "renesas,r7s72100-wdt", "renesas,rz-wdt"

  or just: 

B. "renesas,r7s72100-wdt"   (no fallback, change the driver to add new SoCs)


Thoughts????


Thank you,
Chris

Re: [PATCH 2/3] dt-bindings: power: reset: add document for renesas-reset driver

[Geert Uytterhoeven]

Hi Chris,

On Wed, Feb 15, 2017 at 6:33 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Tuesday, February 14, 2017, Geert Uytterhoeven wrote:
>> > +Required properties:
>> > +  - compatible: must be one or more of the following:
>> > +    - "renesas,r7s72100-reset" for the r7s72100 SoC
>> > +    - "renesas,wdt-reset"
>> > +                This is a fallback for the above renesas,*-reset
>> > +entries
>>
>> Please use "renesas,r7s72100-wdt". DT describes the hardware (watchdog),
>> not the software policy (use it for reset only, as a max. timeout of 125
>> ms is too short for a usable watchdog).
>
> I had a look at:
>
>   Documentation/devicetree/bindings/watchdog/renesas-wdt.txt
>
> Which has:
>
> Required properties:
> - compatible : Should be "renesas,<soctype>-wdt", and
>                "renesas,rcar-gen3-wdt" as fallback.
>                Examples with soctypes are:
>                  - "renesas,r8a7795-wdt" (R-Car H3)
>                  - "renesas,r8a7796-wdt" (R-Car M3-W)
>
>
> So in my 'renesas-reset.txt' should I do:

As "reset" is software policy, perhaps you should instead extend the existing
binding document Documentation/devicetree/bindings/watchdog/renesas-wdt.txt?

Nothing says you have to use the same Linux driver for R-Car Gen3 and
RZ/A1.

> A. "renesas,r7s72100-wdt", "renesas,rz-wdt"

Please don't use plain "rz", as RZ/A, RZ/G, and RZ/T are completely different
beasts.

>
>   or just:
>
> B. "renesas,r7s72100-wdt"   (no fallback, change the driver to add new SoCs)

Do you know if future RZ/A SoCs will use the exact same type of WDT?
If yes, you can document both "renesas,r7s72100-wdt" and "renesas,rza-wdt",
and let the driver match against the latter.
Else I'd just document and match against "renesas,r7s72100-wdt".

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 2/3] dt-bindings: power: reset: add document for renesas-reset driver

[Chris Brandt]

Hi Geert,

On Thursday, February 16, 2017, Geert Uytterhoeven wrote:
> On Wed, Feb 15, 2017 at 6:33 PM, Chris Brandt <Chris.Brandt@renesas.com>
> wrote:
> > On Tuesday, February 14, 2017, Geert Uytterhoeven wrote:
> >> > +Required properties:
> >> > +  - compatible: must be one or more of the following:
> >> > +    - "renesas,r7s72100-reset" for the r7s72100 SoC
> >> > +    - "renesas,wdt-reset"
> >> > +                This is a fallback for the above renesas,*-reset
> >> > +entries
> >>
> >> Please use "renesas,r7s72100-wdt". DT describes the hardware
> >> (watchdog), not the software policy (use it for reset only, as a max.
> >> timeout of 125 ms is too short for a usable watchdog).
> >
> > I had a look at:
> >
> >   Documentation/devicetree/bindings/watchdog/renesas-wdt.txt
> >
> > Which has:
> >
> > Required properties:
> > - compatible : Should be "renesas,<soctype>-wdt", and
> >                "renesas,rcar-gen3-wdt" as fallback.
> >                Examples with soctypes are:
> >                  - "renesas,r8a7795-wdt" (R-Car H3)
> >                  - "renesas,r8a7796-wdt" (R-Car M3-W)
> >
> >
> > So in my 'renesas-reset.txt' should I do:
> 
> As "reset" is software policy, perhaps you should instead extend the
> existing binding document
> Documentation/devicetree/bindings/watchdog/renesas-wdt.txt?
> 
> Nothing says you have to use the same Linux driver for R-Car Gen3 and
> RZ/A1.

That is what I was thinking as well. If I'm just supposed to document hardware,
I might as well group it in the same file...but then in SW use it for something
completely different (reset instead of its WDT functionality)

> > A. "renesas,r7s72100-wdt", "renesas,rz-wdt"
> 
> Please don't use plain "rz", as RZ/A, RZ/G, and RZ/T are completely
> different beasts.

Ya, I'm stuck on a name here. Internally, they don't really have defined IP block
name for the "8-bit WDT that can also be used as a simple timer". I know this
WDT was also used in SH4A devices like SH7757 and SH7758.

> >   or just:
> >
> > B. "renesas,r7s72100-wdt"   (no fallback, change the driver to add new
> SoCs)
> 
> Do you know if future RZ/A SoCs will use the exact same type of WDT?

Yes, RZ/A2 will for sure. I assume it would also be used for any RZ/A3+ because
it's easy to hook up from a HW perspective (if I remember correctly). Although,
I want them to extend the counter from 8-bit to 16-bit. 125ms is ridiculously
too short!

> If yes, you can document both "renesas,r7s72100-wdt" and "renesas,rza-wdt",
> and let the driver match against the latter.

I'm going to go with that.
If the timeout does ever get extended and it does become useful to be used
as an actual watchdog, I'll create a new driver or something.


As always, thank you for your replies!


Chris