[PATCH v8 0/3] Add Gen2 support to rwdt driver

[PATCH v8 0/3] Add Gen2 support to rwdt driver

[Fabrizio Castro]

Dear All,

this new version of the series is to fold the following patch from Geert:
"watchdog: renesas_wdt: Blacklist early R-Car Gen2 SoCs"

into patch:
"watchdog: renesas_wdt: Add R-Car Gen2 support"

Thanks,
Fab

Fabrizio Castro (3):
  watchdog: renesas_wdt: Add suspend/resume support
  watchdog: renesas_wdt: Add R-Car Gen2 support
  watchdog: renesas_wdt: Add restart handler

 drivers/watchdog/renesas_wdt.c | 87 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 82 insertions(+), 5 deletions(-)

-- 
2.7.4

[PATCH v8 1/3] watchdog: renesas_wdt: Add suspend/resume support

[Fabrizio Castro]

On R-Car Gen2 and RZ/G1 the watchdog IP clock needs to be always ON,
on R-Car Gen3 we power the IP down during suspend.

This commit adds suspend/resume support, so that the watchdog counting
"pauses" during suspend on all of the SoCs compatible with this driver
and on those we are now adding support for (R-Car Gen2 and RZ/G1).

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>

---
v7->v8:
* no change

 drivers/watchdog/renesas_wdt.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/watchdog/renesas_wdt.c b/drivers/watchdog/renesas_wdt.c
index 831ef83..024d54e 100644
--- a/drivers/watchdog/renesas_wdt.c
+++ b/drivers/watchdog/renesas_wdt.c
@@ -49,6 +49,7 @@ struct rwdt_priv {
 	void __iomem *base;
 	struct watchdog_device wdev;
 	unsigned long clk_rate;
+	u16 time_left;
 	u8 cks;
 };
 
@@ -203,6 +204,30 @@ static int rwdt_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int __maybe_unused rwdt_suspend(struct device *dev)
+{
+	struct rwdt_priv *priv = dev_get_drvdata(dev);
+
+	if (watchdog_active(&priv->wdev)) {
+		priv->time_left = readw(priv->base + RWTCNT);
+		rwdt_stop(&priv->wdev);
+	}
+	return 0;
+}
+
+static int __maybe_unused rwdt_resume(struct device *dev)
+{
+	struct rwdt_priv *priv = dev_get_drvdata(dev);
+
+	if (watchdog_active(&priv->wdev)) {
+		rwdt_start(&priv->wdev);
+		rwdt_write(priv, priv->time_left, RWTCNT);
+	}
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(rwdt_pm_ops, rwdt_suspend, rwdt_resume);
+
 /*
  * 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
@@ -218,6 +243,7 @@ static struct platform_driver rwdt_driver = {
 	.driver = {
 		.name = "renesas_wdt",
 		.of_match_table = rwdt_ids,
+		.pm = &rwdt_pm_ops,
 	},
 	.probe = rwdt_probe,
 	.remove = rwdt_remove,
-- 
2.7.4

Re: [PATCH v8 1/3] watchdog: renesas_wdt: Add suspend/resume support

[Geert Uytterhoeven]

On Mon, Mar 5, 2018 at 4:30 PM, Fabrizio Castro
<fabrizio.castro@bp.renesas.com> wrote:
> On R-Car Gen2 and RZ/G1 the watchdog IP clock needs to be always ON,
> on R-Car Gen3 we power the IP down during suspend.
>
> This commit adds suspend/resume support, so that the watchdog counting
> "pauses" during suspend on all of the SoCs compatible with this driver
> and on those we are now adding support for (R-Car Gen2 and RZ/G1).
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>

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

[PATCH v8 2/3] watchdog: renesas_wdt: Add R-Car Gen2 support

[Fabrizio Castro]

Due to commits:
* "ARM: shmobile: Add watchdog support",
* "ARM: shmobile: rcar-gen2: Add watchdog support", and
* "soc: renesas: rcar-rst: Enable watchdog as reset trigger for Gen2",
we now have everything we needed for the watchdog to work on Gen2 and
RZ/G1.

However, on early revisions of some R-Car Gen2 SoCs, and depending on SMP
configuration, the system may fail to restart on watchdog time-out, and
lock up instead.

Specifically:
  - On R-Car H2 ES1.0 and M2-W ES1.0, watchdog restart fails unless
    only the first CPU core is in use (using e.g. the "maxcpus=1" kernel
    commandline option).
  - On R-Car V2H ES1.1, watchdog restart fails unless SMP is disabled
    completely (using CONFIG_SMP=n during build configuration, or using
    the "nosmp" or "maxcpus=0" kernel commandline options).

This commit adds "renesas,rcar-gen2-wdt" as compatible string for R-Car
Gen2 and RZ/G1, but also prevents the system from using the watchdog
driver in cases where the system would fail to restart by blacklisting
the affected SoCs, using the minimum known working revisions (ES2.0 on R-Car
H2, and ES3.0 on M2-W), and taking the actual SMP software configuration
into account.

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
[Geert: blacklisting logic]
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v7->v8:
* folded patch "watchdog: renesas_wdt: Blacklist early R-Car Gen2 SoCs" from
  Geert into this patch.

Since Simon only reviewed Geert's patch, and Wolfram and Guenter only reviewed
earlier versions of this patch, I am not including their "Reviewed-by" as they
may disagree with the final result.

Thanks,
Fab

 drivers/watchdog/renesas_wdt.c | 49 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 44 insertions(+), 5 deletions(-)

diff --git a/drivers/watchdog/renesas_wdt.c b/drivers/watchdog/renesas_wdt.c
index 024d54e..0dede5b 100644
--- a/drivers/watchdog/renesas_wdt.c
+++ b/drivers/watchdog/renesas_wdt.c
@@ -16,6 +16,8 @@
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/smp.h>
+#include <linux/sys_soc.h>
 #include <linux/watchdog.h>
 
 #define RWTCNT		0
@@ -121,6 +123,44 @@ static const struct watchdog_ops rwdt_ops = {
 	.get_timeleft = rwdt_get_timeleft,
 };
 
+#if defined(CONFIG_ARCH_RCAR_GEN2) && defined(CONFIG_SMP)
+/*
+ * Watchdog-reset integration is broken on early revisions of R-Car Gen2 SoCs
+ */
+static const struct soc_device_attribute rwdt_quirks_match[] = {
+	{
+		.soc_id = "r8a7790",
+		.revision = "ES1.*",
+		.data = (void *)1,	/* needs single CPU */
+	}, {
+		.soc_id = "r8a7791",
+		.revision = "ES[12].*",
+		.data = (void *)1,	/* needs single CPU */
+	}, {
+		.soc_id = "r8a7792",
+		.revision = "*",
+		.data = (void *)0,	/* needs SMP disabled */
+	},
+	{ /* sentinel */ }
+};
+
+static bool rwdt_blacklisted(struct device *dev)
+{
+	const struct soc_device_attribute *attr;
+
+	attr = soc_device_match(rwdt_quirks_match);
+	if (attr && setup_max_cpus > (uintptr_t)attr->data) {
+		dev_info(dev, "Watchdog blacklisted on %s %s\n", attr->soc_id,
+			 attr->revision);
+		return true;
+	}
+
+	return false;
+}
+#else /* !CONFIG_ARCH_RCAR_GEN2 || !CONFIG_SMP */
+static inline bool rwdt_blacklisted(struct device *dev) { return false; }
+#endif /* !CONFIG_ARCH_RCAR_GEN2 || !CONFIG_SMP */
+
 static int rwdt_probe(struct platform_device *pdev)
 {
 	struct rwdt_priv *priv;
@@ -129,6 +169,9 @@ static int rwdt_probe(struct platform_device *pdev)
 	unsigned long clks_per_sec;
 	int ret, i;
 
+	if (rwdt_blacklisted(&pdev->dev))
+		return -ENODEV;
+
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
@@ -228,12 +271,8 @@ static int __maybe_unused rwdt_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(rwdt_pm_ops, rwdt_suspend, rwdt_resume);
 
-/*
- * 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-gen2-wdt", },
 	{ .compatible = "renesas,rcar-gen3-wdt", },
 	{ /* sentinel */ }
 };
-- 
2.7.4

Re: [PATCH v8 2/3] watchdog: renesas_wdt: Add R-Car Gen2 support

[Wolfram Sang]

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


> Since Simon only reviewed Geert's patch, and Wolfram and Guenter only reviewed
> earlier versions of this patch, I am not including their "Reviewed-by" as they
> may disagree with the final result.

I don't :)

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


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

Re: [PATCH v8 2/3] watchdog: renesas_wdt: Add R-Car Gen2 support

[Guenter Roeck]

On Mon, Mar 05, 2018 at 03:30:25PM +0000, Fabrizio Castro wrote:
> Due to commits:
> * "ARM: shmobile: Add watchdog support",
> * "ARM: shmobile: rcar-gen2: Add watchdog support", and
> * "soc: renesas: rcar-rst: Enable watchdog as reset trigger for Gen2",
> we now have everything we needed for the watchdog to work on Gen2 and
> RZ/G1.
> 
> However, on early revisions of some R-Car Gen2 SoCs, and depending on SMP
> configuration, the system may fail to restart on watchdog time-out, and
> lock up instead.
> 
> Specifically:
>   - On R-Car H2 ES1.0 and M2-W ES1.0, watchdog restart fails unless
>     only the first CPU core is in use (using e.g. the "maxcpus=1" kernel
>     commandline option).
>   - On R-Car V2H ES1.1, watchdog restart fails unless SMP is disabled
>     completely (using CONFIG_SMP=n during build configuration, or using
>     the "nosmp" or "maxcpus=0" kernel commandline options).
> 
> This commit adds "renesas,rcar-gen2-wdt" as compatible string for R-Car
> Gen2 and RZ/G1, but also prevents the system from using the watchdog
> driver in cases where the system would fail to restart by blacklisting
> the affected SoCs, using the minimum known working revisions (ES2.0 on R-Car
> H2, and ES3.0 on M2-W), and taking the actual SMP software configuration
> into account.
> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
> [Geert: blacklisting logic]
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v7->v8:
> * folded patch "watchdog: renesas_wdt: Blacklist early R-Car Gen2 SoCs" from
>   Geert into this patch.
> 
> Since Simon only reviewed Geert's patch, and Wolfram and Guenter only reviewed
> earlier versions of this patch, I am not including their "Reviewed-by" as they
> may disagree with the final result.
> 

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

> Thanks,
> Fab
> 
>  drivers/watchdog/renesas_wdt.c | 49 +++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 44 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/watchdog/renesas_wdt.c b/drivers/watchdog/renesas_wdt.c
> index 024d54e..0dede5b 100644
> --- a/drivers/watchdog/renesas_wdt.c
> +++ b/drivers/watchdog/renesas_wdt.c
> @@ -16,6 +16,8 @@
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/smp.h>
> +#include <linux/sys_soc.h>
>  #include <linux/watchdog.h>
>  
>  #define RWTCNT		0
> @@ -121,6 +123,44 @@ static const struct watchdog_ops rwdt_ops = {
>  	.get_timeleft = rwdt_get_timeleft,
>  };
>  
> +#if defined(CONFIG_ARCH_RCAR_GEN2) && defined(CONFIG_SMP)
> +/*
> + * Watchdog-reset integration is broken on early revisions of R-Car Gen2 SoCs
> + */
> +static const struct soc_device_attribute rwdt_quirks_match[] = {
> +	{
> +		.soc_id = "r8a7790",
> +		.revision = "ES1.*",
> +		.data = (void *)1,	/* needs single CPU */
> +	}, {
> +		.soc_id = "r8a7791",
> +		.revision = "ES[12].*",
> +		.data = (void *)1,	/* needs single CPU */
> +	}, {
> +		.soc_id = "r8a7792",
> +		.revision = "*",
> +		.data = (void *)0,	/* needs SMP disabled */
> +	},
> +	{ /* sentinel */ }
> +};
> +
> +static bool rwdt_blacklisted(struct device *dev)
> +{
> +	const struct soc_device_attribute *attr;
> +
> +	attr = soc_device_match(rwdt_quirks_match);
> +	if (attr && setup_max_cpus > (uintptr_t)attr->data) {
> +		dev_info(dev, "Watchdog blacklisted on %s %s\n", attr->soc_id,
> +			 attr->revision);
> +		return true;
> +	}
> +
> +	return false;
> +}
> +#else /* !CONFIG_ARCH_RCAR_GEN2 || !CONFIG_SMP */
> +static inline bool rwdt_blacklisted(struct device *dev) { return false; }
> +#endif /* !CONFIG_ARCH_RCAR_GEN2 || !CONFIG_SMP */
> +
>  static int rwdt_probe(struct platform_device *pdev)
>  {
>  	struct rwdt_priv *priv;
> @@ -129,6 +169,9 @@ static int rwdt_probe(struct platform_device *pdev)
>  	unsigned long clks_per_sec;
>  	int ret, i;
>  
> +	if (rwdt_blacklisted(&pdev->dev))
> +		return -ENODEV;
> +
>  	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>  	if (!priv)
>  		return -ENOMEM;
> @@ -228,12 +271,8 @@ static int __maybe_unused rwdt_resume(struct device *dev)
>  
>  static SIMPLE_DEV_PM_OPS(rwdt_pm_ops, rwdt_suspend, rwdt_resume);
>  
> -/*
> - * 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-gen2-wdt", },
>  	{ .compatible = "renesas,rcar-gen3-wdt", },
>  	{ /* sentinel */ }
>  };
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v8 3/3] watchdog: renesas_wdt: Add restart handler

[Fabrizio Castro]

On iWave's boards iwg20d and iwg22d the only way to reboot the system is
by means of the watchdog.
This patch adds a restart handler to rwdt_ops, and also makes sure we
keep its priority to the lowest level, in order to not override other
more effective handlers.

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
v7->v8:
* no change

 drivers/watchdog/renesas_wdt.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/watchdog/renesas_wdt.c b/drivers/watchdog/renesas_wdt.c
index 0dede5b..6b8c6dd 100644
--- a/drivers/watchdog/renesas_wdt.c
+++ b/drivers/watchdog/renesas_wdt.c
@@ -110,6 +110,16 @@ static unsigned int rwdt_get_timeleft(struct watchdog_device *wdev)
 	return DIV_BY_CLKS_PER_SEC(priv, 65536 - val);
 }
 
+static int rwdt_restart(struct watchdog_device *wdev, unsigned long action,
+			void *data)
+{
+	struct rwdt_priv *priv = watchdog_get_drvdata(wdev);
+
+	rwdt_start(wdev);
+	rwdt_write(priv, 0xffff, RWTCNT);
+	return 0;
+}
+
 static const struct watchdog_info rwdt_ident = {
 	.options = WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT,
 	.identity = "Renesas WDT Watchdog",
@@ -121,6 +131,7 @@ static const struct watchdog_ops rwdt_ops = {
 	.stop = rwdt_stop,
 	.ping = rwdt_init_timeout,
 	.get_timeleft = rwdt_get_timeleft,
+	.restart = rwdt_restart,
 };
 
 #if defined(CONFIG_ARCH_RCAR_GEN2) && defined(CONFIG_SMP)
@@ -220,6 +231,7 @@ static int rwdt_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, priv);
 	watchdog_set_drvdata(&priv->wdev, priv);
 	watchdog_set_nowayout(&priv->wdev, nowayout);
+	watchdog_set_restart_priority(&priv->wdev, 0);
 
 	/* This overrides the default timeout only if DT configuration was found */
 	ret = watchdog_init_timeout(&priv->wdev, 0, &pdev->dev);
-- 
2.7.4

Re: [PATCH v8 3/3] watchdog: renesas_wdt: Add restart handler

[Geert Uytterhoeven]

On Mon, Mar 5, 2018 at 4:30 PM, Fabrizio Castro
<fabrizio.castro@bp.renesas.com> wrote:
> On iWave's boards iwg20d and iwg22d the only way to reboot the system is
> by means of the watchdog.
> This patch adds a restart handler to rwdt_ops, and also makes sure we
> keep its priority to the lowest level, in order to not override other
> more effective handlers.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> Reviewed-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