[PATCH v2 0/4] watchdog: add driver for Renesas Gen3 WDT watchdogs

[PATCH v2 0/4] watchdog: add driver for Renesas Gen3 WDT watchdogs

[Wolfram Sang]

Here is V2 of this series with the following changes:

* include bitops.h
* s/unsigned/unsigned int/
* drop set_timeout() because core does it already
* add get_timeleft()
* don't make "nowayout" a readable parameter
* use "watchdog" instead of "wdt" in the dt bindings

The dts and defconfig changes should go via Simon's tree once the driver is
accepted.

To use this driver, you need a recent firmware/uboot which initializes the
WDTRSTCR register which can only be changed in secure mode. 'md 0xe6160054 4'
should give you 0x8002 or something else with bit 15 set and bit 0 cleared.

Also, you need the clock enablement patches which just have been posted in
their newest version to the renesas-soc mailing list and are also in
close-to-be-applied state.

A complete branch to test can be found here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/v8-wdt

Please review, test, comment...

Thanks,

   Wolfram


Wolfram Sang (4):
  watchdog: renesas-wdt: add driver
  arm64: dts: r8a7795: Add RWDT node
  arm64: dts: salvator-x: Enable watchdog timer
  arm64: defconfig: enable Renesas Watchdog Timer

 .../devicetree/bindings/watchdog/renesas-wdt.txt   |  25 +++
 arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts |   5 +
 arch/arm64/boot/dts/renesas/r8a7795.dtsi           |   8 +
 arch/arm64/configs/defconfig                       |   2 +
 drivers/watchdog/Kconfig                           |   8 +
 drivers/watchdog/Makefile                          |   1 +
 drivers/watchdog/renesas_wdt.c                     | 213 +++++++++++++++++++++
 7 files changed, 262 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/renesas-wdt.txt
 create mode 100644 drivers/watchdog/renesas_wdt.c

-- 
2.7.0

[PATCH v2 1/4] watchdog: renesas-wdt: add driver

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Add support for watchdogs (RWDT and SWDT) found on RCar Gen3 based SoCs
from Renesas.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 .../devicetree/bindings/watchdog/renesas-wdt.txt   |  25 +++
 drivers/watchdog/Kconfig                           |   8 +
 drivers/watchdog/Makefile                          |   1 +
 drivers/watchdog/renesas_wdt.c                     | 213 +++++++++++++++++++++
 4 files changed, 247 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/renesas-wdt.txt
 create mode 100644 drivers/watchdog/renesas_wdt.c

diff --git a/Documentation/devicetree/bindings/watchdog/renesas-wdt.txt b/Documentation/devicetree/bindings/watchdog/renesas-wdt.txt
new file mode 100644
index 00000000000000..b9512f1eb80a02
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/renesas-wdt.txt
@@ -0,0 +1,25 @@
+Renesas Watchdog Timer (WDT) Controller
+
+Required properties:
+- compatible : Should be "renesas,r8a7795-wdt", or "renesas,rcar-gen3-wdt"
+
+  When compatible with the generic version, nodes must list the SoC-specific
+  version corresponding to the platform first, followed by the generic
+  version.
+
+- reg : Should contain WDT registers location and length
+- clocks : the clock feeding the watchdog timer.
+
+Optional properties:
+- timeout-sec : Contains the watchdog timeout in seconds
+- power-domains : the power domain the WDT belongs to
+
+Examples:
+
+	wdt0: watchdog@e6020000 {
+		compatible = "renesas,r8a7795-wdt", "renesas,rcar-gen3-wdt";
+		reg = <0 0xe6020000 0 0x0c>;
+		clocks = <&cpg CPG_MOD 402>;
+		power-domains = <&cpg>;
+		timeout-sec = <60>;
+	};
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index fb947655badd02..3902c9ca7f099d 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -661,6 +661,14 @@ config ATLAS7_WATCHDOG
 	  To compile this driver as a module, choose M here: the
 	  module will be called atlas7_wdt.
 
+config RENESAS_WDT
+	tristate "Renesas WDT Watchdog"
+	depends on ARCH_RENESAS || COMPILE_TEST
+	select WATCHDOG_CORE
+	help
+	  This driver adds watchdog support for the integrated watchdogs in the
+	  Renesas R-Car and other SH-Mobile SoCs (usually named RWDT or SWDT).
+
 # AVR32 Architecture
 
 config AT32AP700X_WDT
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index feb6270fdbde5f..2cbc9709852d0e 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -73,6 +73,7 @@ obj-$(CONFIG_DIGICOLOR_WATCHDOG) += digicolor_wdt.o
 obj-$(CONFIG_LPC18XX_WATCHDOG) += lpc18xx_wdt.o
 obj-$(CONFIG_BCM7038_WDT) += bcm7038_wdt.o
 obj-$(CONFIG_ATLAS7_WATCHDOG) += atlas7_wdt.o
+obj-$(CONFIG_RENESAS_WDT) += renesas_wdt.o
 
 # AVR32 Architecture
 obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
diff --git a/drivers/watchdog/renesas_wdt.c b/drivers/watchdog/renesas_wdt.c
new file mode 100644
index 00000000000000..cf61c92f7ecd63
--- /dev/null
+++ b/drivers/watchdog/renesas_wdt.c
@@ -0,0 +1,213 @@
+/*
+ * Watchdog driver for Renesas WDT watchdog
+ *
+ * Copyright (C) 2015-16 Wolfram Sang, Sang Engineering <wsa@sang-engineering.com>
+ * Copyright (C) 2015-16 Renesas Electronics Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/watchdog.h>
+
+#define RWTCNT		0
+#define RWTCSRA		4
+#define RWTCSRA_WOVF	BIT(4)
+#define RWTCSRA_WRFLG	BIT(5)
+#define RWTCSRA_TME	BIT(7)
+
+#define RWDT_DEFAULT_TIMEOUT 60U
+
+static const unsigned int clk_divs[] = { 1, 4, 16, 32, 64, 128, 1024 };
+
+static bool nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, bool, 0);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
+				__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+
+struct rwdt_priv {
+	void __iomem *base;
+	struct watchdog_device wdev;
+	struct clk *clk;
+	unsigned int clks_per_sec;
+	u8 cks;
+};
+
+static void rwdt_write(struct rwdt_priv *priv, u32 val, unsigned int reg)
+{
+	if (reg == RWTCNT)
+		val |= 0x5a5a0000;
+	else
+		val |= 0xa5a5a500;
+
+	writel_relaxed(val, priv->base + reg);
+}
+
+static int rwdt_init_timeout(struct watchdog_device *wdev)
+{
+	struct rwdt_priv *priv = watchdog_get_drvdata(wdev);
+
+	rwdt_write(priv, 65536 - wdev->timeout * priv->clks_per_sec, RWTCNT);
+
+	return 0;
+}
+
+static int rwdt_start(struct watchdog_device *wdev)
+{
+	struct rwdt_priv *priv = watchdog_get_drvdata(wdev);
+
+	clk_prepare_enable(priv->clk);
+
+	rwdt_write(priv, priv->cks, RWTCSRA);
+	rwdt_init_timeout(wdev);
+
+	while (readb_relaxed(priv->base + RWTCSRA) & RWTCSRA_WRFLG)
+		cpu_relax();
+
+	rwdt_write(priv, priv->cks | RWTCSRA_TME, RWTCSRA);
+
+	return 0;
+}
+
+static int rwdt_stop(struct watchdog_device *wdev)
+{
+	struct rwdt_priv *priv = watchdog_get_drvdata(wdev);
+
+	rwdt_write(priv, priv->cks, RWTCSRA);
+	clk_disable_unprepare(priv->clk);
+
+	return 0;
+}
+
+static unsigned int rwdt_get_timeleft(struct watchdog_device *wdev)
+{
+	struct rwdt_priv *priv = watchdog_get_drvdata(wdev);
+	u16 val = readw_relaxed(priv->base + RWTCNT);
+
+	return DIV_ROUND_CLOSEST(65536 - val, priv->clks_per_sec);
+}
+
+static const struct watchdog_info rwdt_ident = {
+	.options = WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT,
+	.identity = "Renesas WDT Watchdog",
+};
+
+static const struct watchdog_ops rwdt_ops = {
+	.owner = THIS_MODULE,
+	.start = rwdt_start,
+	.stop = rwdt_stop,
+	.ping = rwdt_init_timeout,
+	.get_timeleft = rwdt_get_timeleft,
+};
+
+static int rwdt_probe(struct platform_device *pdev)
+{
+	struct rwdt_priv *priv;
+	struct resource *res;
+	unsigned long rate;
+	unsigned int clks_per_sec;
+	int ret, i;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	priv->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(priv->clk))
+		return PTR_ERR(priv->clk);
+
+	rate = clk_get_rate(priv->clk);
+	if (!rate)
+		return -ENOENT;
+
+	for (i = ARRAY_SIZE(clk_divs) - 1; i >= 0; i--) {
+		clks_per_sec = DIV_ROUND_UP(rate, clk_divs[i]);
+		if (clks_per_sec) {
+			priv->clks_per_sec = clks_per_sec;
+			priv->cks = i;
+			break;
+		}
+	}
+
+	if (!clks_per_sec) {
+		dev_err(&pdev->dev, "Can't find suitable clock divider\n");
+		return -ERANGE;
+	}
+
+	pm_runtime_enable(&pdev->dev);
+	pm_runtime_get_sync(&pdev->dev);
+
+	priv->wdev.info = &rwdt_ident,
+	priv->wdev.ops = &rwdt_ops,
+	priv->wdev.parent = &pdev->dev;
+	priv->wdev.min_timeout = 1;
+	priv->wdev.max_timeout = 65536 / clks_per_sec;
+	priv->wdev.timeout = min(priv->wdev.max_timeout, RWDT_DEFAULT_TIMEOUT);
+
+	platform_set_drvdata(pdev, priv);
+	watchdog_set_drvdata(&priv->wdev, priv);
+	watchdog_set_nowayout(&priv->wdev, nowayout);
+
+	/* This overrides the default timeout only if DT configuration was found */
+	ret = watchdog_init_timeout(&priv->wdev, 0, &pdev->dev);
+	if (ret)
+		dev_warn(&pdev->dev, "Specified timeout value invalid, using default\n");
+
+	ret = watchdog_register_device(&priv->wdev);
+	if (ret < 0) {
+		pm_runtime_put(&pdev->dev);
+		pm_runtime_disable(&pdev->dev);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int rwdt_remove(struct platform_device *pdev)
+{
+	struct rwdt_priv *priv = platform_get_drvdata(pdev);
+
+	watchdog_unregister_device(&priv->wdev);
+	pm_runtime_put(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+
+	return 0;
+}
+
+/*
+ * This driver does also fit for R-Car Gen2 (r8a779[0-4]) WDT. However, for SMP
+ * to work there, one also needs a RESET (RST) driver which does not exist yet
+ * due to HW issues. This needs to be solved before adding compatibles here.
+ */
+static const struct of_device_id rwdt_ids[] = {
+	{ .compatible = "renesas,rcar-gen3-wdt", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, rwdt_ids);
+
+static struct platform_driver rwdt_driver = {
+	.driver = {
+		.name = "renesas_wdt",
+		.of_match_table = rwdt_ids,
+	},
+	.probe = rwdt_probe,
+	.remove = rwdt_remove,
+};
+module_platform_driver(rwdt_driver);
+
+MODULE_DESCRIPTION("Renesas WDT Watchdog Driver");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Wolfram Sang <wsa@sang-engineering.com>");
-- 
2.7.0

Re: [PATCH v2 1/4] watchdog: renesas-wdt: add driver

[Wolfram Sang]

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


> we are using it. So it should be implemented in this driver as well,
> if it is not supported by HW, then we will need to use second timer or
> watchdog for pretimeout interrupt.

As I said, I have no task like this assigned (and no personal interest,
too). So, you'd need to do it yourself, hire me, or request this feature
from Renesas (and hope that they pass the task to me ;)).

> Please correct me if i'm wrong - module parameter is a way to ignore
> kernel config. For same purpose, to disable wdt at runtime "magic 'V'"
> should be used.

I have to admit that I don't have a specific use-case, I just did
general support as requested. So, I followed the style that basically
every other watchdog driver has this parameter.


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

Re: [PATCH v2 1/4] watchdog: renesas-wdt: add driver

[Guenter Roeck]

On Mon, Apr 04, 2016 at 02:36:54PM +0200, Wolfram Sang wrote:
> 
> > we are using it. So it should be implemented in this driver as well,
> > if it is not supported by HW, then we will need to use second timer or
> > watchdog for pretimeout interrupt.
> 
> As I said, I have no task like this assigned (and no personal interest,
> too). So, you'd need to do it yourself, hire me, or request this feature
> from Renesas (and hope that they pass the task to me ;)).
> 
Sorry, I lost the context here. 

> > Please correct me if i'm wrong - module parameter is a way to ignore
> > kernel config. For same purpose, to disable wdt at runtime "magic 'V'"
> > should be used.
> 
> I have to admit that I don't have a specific use-case, I just did
> general support as requested. So, I followed the style that basically
> every other watchdog driver has this parameter.
> 
I don't understand this one. Unless I am missing something, the module
parameter is standard, and magic close by writing 'V' is supported.
What is the problem ?

>From my perspective, for v2 of the patch,

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

Thanks,
Guenter

Re: [PATCH v2 1/4] watchdog: renesas-wdt: add driver

[Guenter Roeck]

On Tue, Apr 05, 2016 at 08:25:46AM +0200, fixed-term.Oleksij.Rempel wrote:
> 
> 
> On 04.04.2016 16:08, Guenter Roeck wrote:
> > On Mon, Apr 04, 2016 at 02:36:54PM +0200, Wolfram Sang wrote:
> >>
> >>> we are using it. So it should be implemented in this driver as well,
> >>> if it is not supported by HW, then we will need to use second timer or
> >>> watchdog for pretimeout interrupt.
> >>
> >> As I said, I have no task like this assigned (and no personal interest,
> >> too). So, you'd need to do it yourself, hire me, or request this feature
> >> from Renesas (and hope that they pass the task to me ;)).
> >>
> > Sorry, I lost the context here. 
> > 
> >>> Please correct me if i'm wrong - module parameter is a way to ignore
> >>> kernel config. For same purpose, to disable wdt at runtime "magic 'V'"
> >>> should be used.
> >>
> >> I have to admit that I don't have a specific use-case, I just did
> >> general support as requested. So, I followed the style that basically
> >> every other watchdog driver has this parameter.
> >>
> > I don't understand this one. Unless I am missing something, the module
> > parameter is standard, and magic close by writing 'V' is supported.
> > What is the problem ?
> 
> Sorry it was more about our internal requirements, which are fallowing:
> - WDT is always on. there should be no option to disable it.
> - WDTs can fail, it is proven fact. In our case, worst scenario, failed
> WDT will kill a car battery. This is why we use many watchdogs in one
> system. So, using differently configured WDTs of one SoC is valid use
> case. From this point, module options are kind of sweet but useless or
> harmful.

No one _has_ to use the module parameter. That doesn't mean that it must
not be there - someone else may want it for some reason.

> - pretimout interrupt is not optional - not any more. Some issues are
> wary hard or impossible to reproduce. Pretimout interrupt helped us to
> debug some of them. If WDT hardware can't provide one, we will need to
> mix two watchdogs (one for reset and other for interrupt) or wdt with
> some other hw timer.

Adding pretimeout support should be easy with a follow-up patch.

> 
> May be this requirements can be passed to upstream as well :)

Keep in mind that a specific use case does not and should not mandate
driver implementation details. The driver can support both module parameter
and pretimeout, but none of those has to be used in a specific application.
Important is that the kernel _supports_ your use case, which should be no
problem.

Thanks,
Guenter

Re: [PATCH v2 1/4] watchdog: renesas-wdt: add driver

[Guenter Roeck]

On Wed, Apr 06, 2016 at 07:39:24AM +0200, fixed-term.Oleksij.Rempel wrote:
> 
> 
> On 05.04.2016 20:47, Guenter Roeck wrote:
> > On Tue, Apr 05, 2016 at 08:25:46AM +0200, fixed-term.Oleksij.Rempel wrote:
> >>
> >>
> >> On 04.04.2016 16:08, Guenter Roeck wrote:
> >>> On Mon, Apr 04, 2016 at 02:36:54PM +0200, Wolfram Sang wrote:
> >>>>
> >>>>> we are using it. So it should be implemented in this driver as well,
> >>>>> if it is not supported by HW, then we will need to use second timer or
> >>>>> watchdog for pretimeout interrupt.
> >>>>
> >>>> As I said, I have no task like this assigned (and no personal interest,
> >>>> too). So, you'd need to do it yourself, hire me, or request this feature
> >>>> from Renesas (and hope that they pass the task to me ;)).
> >>>>
> >>> Sorry, I lost the context here. 
> >>>
> >>>>> Please correct me if i'm wrong - module parameter is a way to ignore
> >>>>> kernel config. For same purpose, to disable wdt at runtime "magic 'V'"
> >>>>> should be used.
> >>>>
> >>>> I have to admit that I don't have a specific use-case, I just did
> >>>> general support as requested. So, I followed the style that basically
> >>>> every other watchdog driver has this parameter.
> >>>>
> >>> I don't understand this one. Unless I am missing something, the module
> >>> parameter is standard, and magic close by writing 'V' is supported.
> >>> What is the problem ?
> >>
> >> Sorry it was more about our internal requirements, which are fallowing:
> >> - WDT is always on. there should be no option to disable it.
> >> - WDTs can fail, it is proven fact. In our case, worst scenario, failed
> >> WDT will kill a car battery. This is why we use many watchdogs in one
> >> system. So, using differently configured WDTs of one SoC is valid use
> >> case. From this point, module options are kind of sweet but useless or
> >> harmful.
> > 
> > No one _has_ to use the module parameter. That doesn't mean that it must
> > not be there - someone else may want it for some reason.
> > 
> >> - pretimout interrupt is not optional - not any more. Some issues are
> >> wary hard or impossible to reproduce. Pretimout interrupt helped us to
> >> debug some of them. If WDT hardware can't provide one, we will need to
> >> mix two watchdogs (one for reset and other for interrupt) or wdt with
> >> some other hw timer.
> > 
> > Adding pretimeout support should be easy with a follow-up patch.
> > 
> >>
> >> May be this requirements can be passed to upstream as well :)
> > 
> > Keep in mind that a specific use case does not and should not mandate
> > driver implementation details. The driver can support both module parameter
> > and pretimeout, but none of those has to be used in a specific application.
> > Important is that the kernel _supports_ your use case, which should be no
> > problem.
> 
> Ok,
> thank you for your answer.
> 
> Some of patches was send for almost 5 months, suddenly i don't see them
> in any git repo:
> https://www.spinics.net/lists/linux-watchdog/msg08086.html
> 

Different subject ?

> Are there some doubts about it?

No, pretty much waiting for a rebase to the latest watchdog core code
to have another look.

There have been several proposals for adding pretimeout support into the
watchdog core. Some are more complex, some are less complex. The difficulty
is to find a balance between complexity and functionality.

Guenter

[PATCH v2 2/4] arm64: dts: r8a7795: Add RWDT node

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

This patch adds the RWDT device node for r8a7795.

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 arch/arm64/boot/dts/renesas/r8a7795.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
index bccd52e0462b92..ab0d20b7cb6f7b 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
@@ -167,6 +167,14 @@
 					(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
 		};
 
+		wdt0: watchdog@e6020000 {
+			compatible = "renesas,r8a7795-wdt", "renesas,rcar-gen3-wdt";
+			reg = <0 0xe6020000 0 0x0c>;
+			clocks = <&cpg CPG_MOD 402>;
+			power-domains = <&cpg>;
+			status = "disabled";
+		};
+
 		gpio0: gpio@e6050000 {
 			compatible = "renesas,gpio-r8a7795",
 				     "renesas,gpio-rcar";
-- 
2.7.0

Re: [PATCH v2 2/4] arm64: dts: r8a7795: Add RWDT node

[Geert Uytterhoeven]

On Fri, Apr 1, 2016 at 1:56 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> This patch adds the RWDT device node for r8a7795.
>
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.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 2/4] arm64: dts: r8a7795: Add RWDT node

[Guenter Roeck]

On Fri, Apr 01, 2016 at 01:56:24PM +0200, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> This patch adds the RWDT device node for r8a7795.
> 
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

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

I assume the dts patches will go through the an arm64 tree.
Is this correct ?

Thanks,
Guenter

> ---
>  arch/arm64/boot/dts/renesas/r8a7795.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> index bccd52e0462b92..ab0d20b7cb6f7b 100644
> --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> @@ -167,6 +167,14 @@
>  					(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
>  		};
>  
> +		wdt0: watchdog@e6020000 {
> +			compatible = "renesas,r8a7795-wdt", "renesas,rcar-gen3-wdt";
> +			reg = <0 0xe6020000 0 0x0c>;
> +			clocks = <&cpg CPG_MOD 402>;
> +			power-domains = <&cpg>;
> +			status = "disabled";
> +		};
> +
>  		gpio0: gpio@e6050000 {
>  			compatible = "renesas,gpio-r8a7795",
>  				     "renesas,gpio-rcar";
> -- 
> 2.7.0
> 
> --
> 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/4] arm64: dts: r8a7795: Add RWDT node

[Wolfram Sang]

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

On Mon, Apr 04, 2016 at 07:00:57AM -0700, Guenter Roeck wrote:
> On Fri, Apr 01, 2016 at 01:56:24PM +0200, Wolfram Sang wrote:
> > From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > 
> > This patch adds the RWDT device node for r8a7795.
> > 
> > Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> Acked-by: Guenter Roeck <linux@roeck-us.net>
> 
> I assume the dts patches will go through the an arm64 tree.
> Is this correct ?

Yes, via Simon's Renesas Tree to arm64.


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

Re: [PATCH v2 2/4] arm64: dts: r8a7795: Add RWDT node

[Wolfram Sang]

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

On Mon, Apr 04, 2016 at 07:00:57AM -0700, Guenter Roeck wrote:
> On Fri, Apr 01, 2016 at 01:56:24PM +0200, Wolfram Sang wrote:
> > From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > 
> > This patch adds the RWDT device node for r8a7795.
> > 
> > Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> Acked-by: Guenter Roeck <linux@roeck-us.net>
> 
> I assume the dts patches will go through the an arm64 tree.
> Is this correct ?

Yes. Simon: ping :)


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

Re: [PATCH v2 2/4] arm64: dts: r8a7795: Add RWDT node

[Simon Horman]

On Wed, May 18, 2016 at 11:38:32AM +0200, Wolfram Sang wrote:
> On Mon, Apr 04, 2016 at 07:00:57AM -0700, Guenter Roeck wrote:
> > On Fri, Apr 01, 2016 at 01:56:24PM +0200, Wolfram Sang wrote:
> > > From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > 
> > > This patch adds the RWDT device node for r8a7795.
> > > 
> > > Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > 
> > Acked-by: Guenter Roeck <linux@roeck-us.net>
> > 
> > I assume the dts patches will go through the an arm64 tree.
> > Is this correct ?
> 
> Yes. Simon: ping :)

Thanks, I have queued this up.

[PATCH v2 3/4] arm64: dts: salvator-x: Enable watchdog timer

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

This patch enables watchdog timer for Salvator-X board.

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
index c3c25409224a22..dbea5078fec367 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
@@ -507,6 +507,11 @@
 	shared-pin;
 };
 
+&wdt0 {
+	timeout-sec = <60>;
+	status = "okay";
+};
+
 &audio_clk_a {
 	clock-frequency = <22579200>;
 };
-- 
2.7.0

Re: [PATCH v2 3/4] arm64: dts: salvator-x: Enable watchdog timer

[Geert Uytterhoeven]

On Fri, Apr 1, 2016 at 1:56 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> This patch enables watchdog timer for Salvator-X board.
>
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Acked-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/4] arm64: dts: salvator-x: Enable watchdog timer

[Guenter Roeck]

On Fri, Apr 01, 2016 at 01:56:25PM +0200, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> This patch enables watchdog timer for Salvator-X board.
> 
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Acked-by: Guenter Roeck <linux2roeck-us.net>

> ---
>  arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
> index c3c25409224a22..dbea5078fec367 100644
> --- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
> @@ -507,6 +507,11 @@
>  	shared-pin;
>  };
>  
> +&wdt0 {
> +	timeout-sec = <60>;
> +	status = "okay";
> +};
> +
>  &audio_clk_a {
>  	clock-frequency = <22579200>;
>  };
> -- 
> 2.7.0
> 
> --
> 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 3/4] arm64: dts: salvator-x: Enable watchdog timer

[Wolfram Sang]

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

On Mon, Apr 04, 2016 at 07:01:31AM -0700, Guenter Roeck wrote:
> On Fri, Apr 01, 2016 at 01:56:25PM +0200, Wolfram Sang wrote:
> > From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > 
> > This patch enables watchdog timer for Salvator-X board.
> > 
> > Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> Acked-by: Guenter Roeck <linux2roeck-us.net>

Thanks. s/2/@/, I assume ;) Simon: ping


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

Re: [PATCH v2 3/4] arm64: dts: salvator-x: Enable watchdog timer

[Simon Horman]

On Wed, May 18, 2016 at 11:39:54AM +0200, Wolfram Sang wrote:
> On Mon, Apr 04, 2016 at 07:01:31AM -0700, Guenter Roeck wrote:
> > On Fri, Apr 01, 2016 at 01:56:25PM +0200, Wolfram Sang wrote:
> > > From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > 
> > > This patch enables watchdog timer for Salvator-X board.
> > > 
> > > Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > 
> > Acked-by: Guenter Roeck <linux2roeck-us.net>
> 
> Thanks. s/2/@/, I assume ;) Simon: ping

Thanks, I have have queued this up.

[PATCH v2 4/4] arm64: defconfig: enable Renesas Watchdog Timer

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 arch/arm64/configs/defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index f78360c08c1eca..795d40e810ff95 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -168,6 +168,8 @@ CONFIG_POWER_RESET_SYSCON=y
 CONFIG_THERMAL=y
 CONFIG_THERMAL_EMULATION=y
 CONFIG_EXYNOS_THERMAL=y
+CONFIG_WATCHDOG=y
+CONFIG_RENESAS_WDT=y
 CONFIG_MFD_SPMI_PMIC=y
 CONFIG_MFD_SEC_CORE=y
 CONFIG_REGULATOR=y
-- 
2.7.0

Re: [PATCH v2 4/4] arm64: defconfig: enable Renesas Watchdog Timer

[Geert Uytterhoeven]

On Fri, Apr 1, 2016 at 1:56 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.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 4/4] arm64: defconfig: enable Renesas Watchdog Timer

[Guenter Roeck]

On Fri, Apr 01, 2016 at 01:56:26PM +0200, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

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

> ---
>  arch/arm64/configs/defconfig | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index f78360c08c1eca..795d40e810ff95 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -168,6 +168,8 @@ CONFIG_POWER_RESET_SYSCON=y
>  CONFIG_THERMAL=y
>  CONFIG_THERMAL_EMULATION=y
>  CONFIG_EXYNOS_THERMAL=y
> +CONFIG_WATCHDOG=y
> +CONFIG_RENESAS_WDT=y
>  CONFIG_MFD_SPMI_PMIC=y
>  CONFIG_MFD_SEC_CORE=y
>  CONFIG_REGULATOR=y
> -- 
> 2.7.0
> 
> --
> 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 0/4] watchdog: add driver for Renesas Gen3 WDT watchdogs

[Geert Uytterhoeven]

Hi Wolfram,

On Fri, Apr 1, 2016 at 1:56 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> Here is V2 of this series with the following changes:
>
> * include bitops.h
> * s/unsigned/unsigned int/
> * drop set_timeout() because core does it already
> * add get_timeleft()
> * don't make "nowayout" a readable parameter
> * use "watchdog" instead of "wdt" in the dt bindings
>
> The dts and defconfig changes should go via Simon's tree once the driver is
> accepted.
>
> To use this driver, you need a recent firmware/uboot which initializes the
> WDTRSTCR register which can only be changed in secure mode. 'md 0xe6160054 4'
> should give you 0x8002 or something else with bit 15 set and bit 0 cleared.

Thanks a lot!

My Salvator-X reboots after timeout from "cat > /dev/watchdog0", but
it doesn't reboot through "reboot" or "reboot -f"?

Am I missing something?

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 0/4] watchdog: add driver for Renesas Gen3 WDT watchdogs

[Wolfram Sang]

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


> My Salvator-X reboots after timeout from "cat > /dev/watchdog0", but
> it doesn't reboot through "reboot" or "reboot -f"?
> 

That sadly doesn't work on Gen3. From the RFC v5 cover letter:

===

* drop restart_handler since ARM64 uses PSCI firmware resets which do
  not call restart handlers

The last point was quite a bummer to me because plain reboot was the
reason I wrote this driver ;) Well, so is life...

===

   Wolfram


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

Re: [PATCH v2 0/4] watchdog: add driver for Renesas Gen3 WDT watchdogs

[Geert Uytterhoeven]

Hi Wolfram,

On Mon, Apr 4, 2016 at 4:59 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> My Salvator-X reboots after timeout from "cat > /dev/watchdog0", but
>> it doesn't reboot through "reboot" or "reboot -f"?
>
> That sadly doesn't work on Gen3. From the RFC v5 cover letter:

> ===
>
> * drop restart_handler since ARM64 uses PSCI firmware resets which do
>   not call restart handlers
>
> The last point was quite a bummer to me because plain reboot was the
> reason I wrote this driver ;) Well, so is life...

That's indeed silly. Can't we have it as a low-priority restart handler, to
restart the system if PSCI fails to do so?

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 0/4] watchdog: add driver for Renesas Gen3 WDT watchdogs

[Wolfram Sang]

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


> That's indeed silly. Can't we have it as a low-priority restart handler, to
> restart the system if PSCI fails to do so?

+1. But my PSCI knowledge is 0. Unlike my task list :/


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

Re: [PATCH v2 0/4] watchdog: add driver for Renesas Gen3 WDT watchdogs

[Guenter Roeck]

On Mon, Apr 04, 2016 at 05:21:44PM +0200, Geert Uytterhoeven wrote:
> Hi Wolfram,
> 
> On Mon, Apr 4, 2016 at 4:59 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> >> My Salvator-X reboots after timeout from "cat > /dev/watchdog0", but
> >> it doesn't reboot through "reboot" or "reboot -f"?
> >
> > That sadly doesn't work on Gen3. From the RFC v5 cover letter:
> 
> > ===
> >
> > * drop restart_handler since ARM64 uses PSCI firmware resets which do
> >   not call restart handlers
> >
> > The last point was quite a bummer to me because plain reboot was the
> > reason I wrote this driver ;) Well, so is life...
> 
> That's indeed silly. Can't we have it as a low-priority restart handler, to

Yes, it is. It defeats the purpose of restart handlers. PSCI reset should have
been implemented as a high priority restart handler.

Guenter

> restart the system if PSCI fails to do so?
> 
> 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 0/4] watchdog: add driver for Renesas Gen3 WDT watchdogs

[Geert Uytterhoeven]

Hi Günter,

On Mon, Apr 4, 2016 at 5:25 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> On Mon, Apr 04, 2016 at 05:21:44PM +0200, Geert Uytterhoeven wrote:
>> On Mon, Apr 4, 2016 at 4:59 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> >> My Salvator-X reboots after timeout from "cat > /dev/watchdog0", but
>> >> it doesn't reboot through "reboot" or "reboot -f"?
>> >
>> > That sadly doesn't work on Gen3. From the RFC v5 cover letter:
>>
>> > ===
>> >
>> > * drop restart_handler since ARM64 uses PSCI firmware resets which do
>> >   not call restart handlers
>> >
>> > The last point was quite a bummer to me because plain reboot was the
>> > reason I wrote this driver ;) Well, so is life...
>>
>> That's indeed silly. Can't we have it as a low-priority restart handler, to
>
> Yes, it is. It defeats the purpose of restart handlers. PSCI reset should have
> been implemented as a high priority restart handler.

Unfortunately that won't work: psci_sys_reset() doesn't return on failure.

We can still clear arm_pm_restart in platform code, though ;-)

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 0/4] watchdog: add driver for Renesas Gen3 WDT watchdogs

[Guenter Roeck]

On Mon, Apr 04, 2016 at 07:02:45PM +0200, Geert Uytterhoeven wrote:
> Hi Günter,
> 
> On Mon, Apr 4, 2016 at 5:25 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> > On Mon, Apr 04, 2016 at 05:21:44PM +0200, Geert Uytterhoeven wrote:
> >> On Mon, Apr 4, 2016 at 4:59 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> >> >> My Salvator-X reboots after timeout from "cat > /dev/watchdog0", but
> >> >> it doesn't reboot through "reboot" or "reboot -f"?
> >> >
> >> > That sadly doesn't work on Gen3. From the RFC v5 cover letter:
> >>
> >> > ===
> >> >
> >> > * drop restart_handler since ARM64 uses PSCI firmware resets which do
> >> >   not call restart handlers
> >> >
> >> > The last point was quite a bummer to me because plain reboot was the
> >> > reason I wrote this driver ;) Well, so is life...
> >>
> >> That's indeed silly. Can't we have it as a low-priority restart handler, to
> >
> > Yes, it is. It defeats the purpose of restart handlers. PSCI reset should have
> > been implemented as a high priority restart handler.
> 
> Unfortunately that won't work: psci_sys_reset() doesn't return on failure.
> 
You mean it just hangs ? That is bad. If that is the case, it is not reliable
and thus should be a low priority (or at best medium priority) restart handler
(which can be replaced with a working higher priority one).

> We can still clear arm_pm_restart in platform code, though ;-)
> 
I had originally planned to replace arm_pm_restart() completely with restart
handlers. Maybe I should revive the effort ?

Guenter

Re: [PATCH v2 0/4] watchdog: add driver for Renesas Gen3 WDT watchdogs

[Geert Uytterhoeven]

Hi Günter,

On Mon, Apr 4, 2016 at 7:52 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> On Mon, Apr 04, 2016 at 07:02:45PM +0200, Geert Uytterhoeven wrote:
>> On Mon, Apr 4, 2016 at 5:25 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>> > On Mon, Apr 04, 2016 at 05:21:44PM +0200, Geert Uytterhoeven wrote:
>> >> On Mon, Apr 4, 2016 at 4:59 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> >> >> My Salvator-X reboots after timeout from "cat > /dev/watchdog0", but
>> >> >> it doesn't reboot through "reboot" or "reboot -f"?
>> >> >
>> >> > That sadly doesn't work on Gen3. From the RFC v5 cover letter:
>> >>
>> >> > ===
>> >> >
>> >> > * drop restart_handler since ARM64 uses PSCI firmware resets which do
>> >> >   not call restart handlers
>> >> >
>> >> > The last point was quite a bummer to me because plain reboot was the
>> >> > reason I wrote this driver ;) Well, so is life...
>> >>
>> >> That's indeed silly. Can't we have it as a low-priority restart handler, to
>> >
>> > Yes, it is. It defeats the purpose of restart handlers. PSCI reset should have
>> > been implemented as a high priority restart handler.
>>
>> Unfortunately that won't work: psci_sys_reset() doesn't return on failure.
>>
> You mean it just hangs ? That is bad. If that is the case, it is not reliable
> and thus should be a low priority (or at best medium priority) restart handler
> (which can be replaced with a working higher priority one).

On my Salvator-X board, it hangs. The call into PSCI doesn't return.
As this is firmware, it may depend on the version of the firmware.

>> We can still clear arm_pm_restart in platform code, though ;-)
>>
> I had originally planned to replace arm_pm_restart() completely with restart
> handlers. Maybe I should revive the effort ?

Perhaps. As I remember, it was not such a pretty experience?

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 0/4] watchdog: add driver for Renesas Gen3 WDT watchdogs

[Guenter Roeck]

On Mon, Apr 04, 2016 at 08:14:02PM +0200, Geert Uytterhoeven wrote:
> Hi Günter,
> 
> On Mon, Apr 4, 2016 at 7:52 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> > On Mon, Apr 04, 2016 at 07:02:45PM +0200, Geert Uytterhoeven wrote:
> >> On Mon, Apr 4, 2016 at 5:25 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> >> > On Mon, Apr 04, 2016 at 05:21:44PM +0200, Geert Uytterhoeven wrote:
> >> >> On Mon, Apr 4, 2016 at 4:59 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> >> >> >> My Salvator-X reboots after timeout from "cat > /dev/watchdog0", but
> >> >> >> it doesn't reboot through "reboot" or "reboot -f"?
> >> >> >
> >> >> > That sadly doesn't work on Gen3. From the RFC v5 cover letter:
> >> >>
> >> >> > ===
> >> >> >
> >> >> > * drop restart_handler since ARM64 uses PSCI firmware resets which do
> >> >> >   not call restart handlers
> >> >> >
> >> >> > The last point was quite a bummer to me because plain reboot was the
> >> >> > reason I wrote this driver ;) Well, so is life...
> >> >>
> >> >> That's indeed silly. Can't we have it as a low-priority restart handler, to
> >> >
> >> > Yes, it is. It defeats the purpose of restart handlers. PSCI reset should have
> >> > been implemented as a high priority restart handler.
> >>
> >> Unfortunately that won't work: psci_sys_reset() doesn't return on failure.
> >>
> > You mean it just hangs ? That is bad. If that is the case, it is not reliable
> > and thus should be a low priority (or at best medium priority) restart handler
> > (which can be replaced with a working higher priority one).
> 
> On my Salvator-X board, it hangs. The call into PSCI doesn't return.
> As this is firmware, it may depend on the version of the firmware.
> 
> >> We can still clear arm_pm_restart in platform code, though ;-)
> >>
> > I had originally planned to replace arm_pm_restart() completely with restart
> > handlers. Maybe I should revive the effort ?
> 
> Perhaps. As I remember, it was not such a pretty experience?
> 
That was the attempt to introduce the same mechanism for power-off handling.
Yes, that bad experience got me to abandon the attempt to finish the restart
handler cleanup as well, but it may still worth at least a final attempt.

Guenter

Re: [PATCH v2 0/4] watchdog: add driver for Renesas Gen3 WDT watchdogs

[Wolfram Sang]

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


> > You mean it just hangs ? That is bad. If that is the case, it is not reliable
> > and thus should be a low priority (or at best medium priority) restart handler
> > (which can be replaced with a working higher priority one).
> 
> On my Salvator-X board, it hangs. The call into PSCI doesn't return.
> As this is firmware, it may depend on the version of the firmware.

Yes, this is also where I stopped looking into it. I dunno what is
expected behaviour from PSCI and such...


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

Re: [PATCH v2 0/4] watchdog: add driver for Renesas Gen3 WDT watchdogs

[Wim Van Sebroeck]

Hi Wolfram,

> Here is V2 of this series with the following changes:
> 
> * include bitops.h
> * s/unsigned/unsigned int/
> * drop set_timeout() because core does it already
> * add get_timeleft()
> * don't make "nowayout" a readable parameter
> * use "watchdog" instead of "wdt" in the dt bindings
> 
> The dts and defconfig changes should go via Simon's tree once the driver is
> accepted.
> 
> To use this driver, you need a recent firmware/uboot which initializes the
> WDTRSTCR register which can only be changed in secure mode. 'md 0xe6160054 4'
> should give you 0x8002 or something else with bit 15 set and bit 0 cleared.
> 
> Also, you need the clock enablement patches which just have been posted in
> their newest version to the renesas-soc mailing list and are also in
> close-to-be-applied state.
> 
> A complete branch to test can be found here:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/v8-wdt
> 
> Please review, test, comment...
> 
> Thanks,
> 
>    Wolfram
> 
> 
> Wolfram Sang (4):
>   watchdog: renesas-wdt: add driver
>   arm64: dts: r8a7795: Add RWDT node
>   arm64: dts: salvator-x: Enable watchdog timer
>   arm64: defconfig: enable Renesas Watchdog Timer
> 
>  .../devicetree/bindings/watchdog/renesas-wdt.txt   |  25 +++
>  arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts |   5 +
>  arch/arm64/boot/dts/renesas/r8a7795.dtsi           |   8 +
>  arch/arm64/configs/defconfig                       |   2 +
>  drivers/watchdog/Kconfig                           |   8 +
>  drivers/watchdog/Makefile                          |   1 +
>  drivers/watchdog/renesas_wdt.c                     | 213 +++++++++++++++++++++
>  7 files changed, 262 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/renesas-wdt.txt
>  create mode 100644 drivers/watchdog/renesas_wdt.c
> 
> -- 
> 2.7.0
>

Patch 1 and 4 have been added to linux-watchdog-next.

Kind regards,
Wim.