diff mbox

[2/8] watchdog: s3c2410: Fix infinite interrupt in soft mode

Message ID 20170224151122.12142-3-krzk@kernel.org
State Not Applicable, archived
Headers show

Commit Message

Krzysztof Kozlowski Feb. 24, 2017, 3:11 p.m. UTC
In soft (no-reboot) mode, the driver self-pings watchdog upon expiration
of an interrupt.  However the interrupt itself was not cleared thus on
first hit, the system enters infinite interrupt handling loop.

On Odroid U3 (Exynos4412), when booted with s3c2410_wdt.soft_noboot=1
argument the console is flooded:
	# killall -9 watchdog
	[   60.523760] s3c2410-wdt 10060000.watchdog: watchdog timer expired (irq)
	[   60.536744] s3c2410-wdt 10060000.watchdog: watchdog timer expired (irq)

Fix this by writing something to the WTCLRINT register to clear the
interrupt.  The register WTCLRINT however appeared in S3C6410 so a new
watchdog quirk and flavor are needed.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

---

I was thinking about CC-ing stable, especially when I thought about
adding unconditional WTCLRINT clear.  However it appears that S3C2410
does not have the WTCLRINT register, this a new chip flavor has to be
added.  This makes the fix spread over driver and DTS changes.

Worth adding:
	Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
?
---
 .../devicetree/bindings/watchdog/samsung-wdt.txt    |  9 +++++----
 drivers/watchdog/s3c2410_wdt.c                      | 21 ++++++++++++++++++---
 2 files changed, 23 insertions(+), 7 deletions(-)

Comments

Guenter Roeck Feb. 24, 2017, 3:37 p.m. UTC | #1
On 02/24/2017 07:11 AM, Krzysztof Kozlowski wrote:
> In soft (no-reboot) mode, the driver self-pings watchdog upon expiration
> of an interrupt.  However the interrupt itself was not cleared thus on
> first hit, the system enters infinite interrupt handling loop.
>
> On Odroid U3 (Exynos4412), when booted with s3c2410_wdt.soft_noboot=1
> argument the console is flooded:
> 	# killall -9 watchdog
> 	[   60.523760] s3c2410-wdt 10060000.watchdog: watchdog timer expired (irq)
> 	[   60.536744] s3c2410-wdt 10060000.watchdog: watchdog timer expired (irq)
>
> Fix this by writing something to the WTCLRINT register to clear the
> interrupt.  The register WTCLRINT however appeared in S3C6410 so a new
> watchdog quirk and flavor are needed.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

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

>
> ---
>
> I was thinking about CC-ing stable, especially when I thought about
> adding unconditional WTCLRINT clear.  However it appears that S3C2410
> does not have the WTCLRINT register, this a new chip flavor has to be
> added.  This makes the fix spread over driver and DTS changes.
>
> Worth adding:
> 	Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> ?

How would that be different to Cc: stable ?

> ---
>  .../devicetree/bindings/watchdog/samsung-wdt.txt    |  9 +++++----
>  drivers/watchdog/s3c2410_wdt.c                      | 21 ++++++++++++++++++---
>  2 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
> index 8f3d96af81d7..1f6e101e299a 100644
> --- a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
> +++ b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
> @@ -6,10 +6,11 @@ occurred.
>
>  Required properties:
>  - compatible : should be one among the following
> -	(a) "samsung,s3c2410-wdt" for Exynos4 and previous SoCs
> -	(b) "samsung,exynos5250-wdt" for Exynos5250
> -	(c) "samsung,exynos5420-wdt" for Exynos5420
> -	(c) "samsung,exynos7-wdt" for Exynos7
> +	- "samsung,s3c2410-wdt" for S3C2410
> +	- "samsung,s3c6410-wdt" for S3C6410, S5PV210 and Exynos4
> +	- "samsung,exynos5250-wdt" for Exynos5250
> +	- "samsung,exynos5420-wdt" for Exynos5420
> +	- "samsung,exynos7-wdt" for Exynos7
>
>  - reg : base physical address of the controller and length of memory mapped
>  	region.
> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> index 3e216f54cb6f..7db7847a005c 100644
> --- a/drivers/watchdog/s3c2410_wdt.c
> +++ b/drivers/watchdog/s3c2410_wdt.c
> @@ -46,6 +46,7 @@
>  #define S3C2410_WTCON		0x00
>  #define S3C2410_WTDAT		0x04
>  #define S3C2410_WTCNT		0x08
> +#define S3C2410_WTCLRINT	0x0c
>
>  #define S3C2410_WTCNT_MAXCNT	0xffff
>
> @@ -72,6 +73,7 @@
>  #define EXYNOS5_WDT_MASK_RESET_REG_OFFSET	0x040c
>  #define QUIRK_HAS_PMU_CONFIG			(1 << 0)
>  #define QUIRK_HAS_RST_STAT			(1 << 1)
> +#define QUIRK_HAS_WTCLRINT_REG			(1 << 2)
>
>  /* These quirks require that we have a PMU register map */
>  #define QUIRKS_HAVE_PMUREG			(QUIRK_HAS_PMU_CONFIG | \
> @@ -143,13 +145,18 @@ static const struct s3c2410_wdt_variant drv_data_s3c2410 = {
>  };
>
>  #ifdef CONFIG_OF
> +static const struct s3c2410_wdt_variant drv_data_s3c6410 = {
> +	.quirks = QUIRK_HAS_WTCLRINT_REG,
> +};
> +
>  static const struct s3c2410_wdt_variant drv_data_exynos5250  = {
>  	.disable_reg = EXYNOS5_WDT_DISABLE_REG_OFFSET,
>  	.mask_reset_reg = EXYNOS5_WDT_MASK_RESET_REG_OFFSET,
>  	.mask_bit = 20,
>  	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
>  	.rst_stat_bit = 20,
> -	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT,
> +	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT \
> +		  | QUIRK_HAS_WTCLRINT_REG,
>  };
>
>  static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
> @@ -158,7 +165,8 @@ static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
>  	.mask_bit = 0,
>  	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
>  	.rst_stat_bit = 9,
> -	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT,
> +	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT \
> +		  | QUIRK_HAS_WTCLRINT_REG,
>  };
>
>  static const struct s3c2410_wdt_variant drv_data_exynos7 = {
> @@ -167,12 +175,15 @@ static const struct s3c2410_wdt_variant drv_data_exynos7 = {
>  	.mask_bit = 23,
>  	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
>  	.rst_stat_bit = 23,	/* A57 WDTRESET */
> -	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT,
> +	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT \
> +		  | QUIRK_HAS_WTCLRINT_REG,
>  };
>
>  static const struct of_device_id s3c2410_wdt_match[] = {
>  	{ .compatible = "samsung,s3c2410-wdt",
>  	  .data = &drv_data_s3c2410 },
> +	{ .compatible = "samsung,s3c6410-wdt",
> +	  .data = &drv_data_s3c6410 },
>  	{ .compatible = "samsung,exynos5250-wdt",
>  	  .data = &drv_data_exynos5250 },
>  	{ .compatible = "samsung,exynos5420-wdt",
> @@ -418,6 +429,10 @@ static irqreturn_t s3c2410wdt_irq(int irqno, void *param)
>  	dev_info(wdt->dev, "watchdog timer expired (irq)\n");
>
>  	s3c2410wdt_keepalive(&wdt->wdt_device);
> +
> +	if (wdt->drv_data->quirks & QUIRK_HAS_WTCLRINT_REG)
> +		writel(0x1, wdt->reg_base + S3C2410_WTCLRINT);
> +
>  	return IRQ_HANDLED;
>  }
>
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Kozlowski Feb. 24, 2017, 5:30 p.m. UTC | #2
On Fri, Feb 24, 2017 at 07:37:53AM -0800, Guenter Roeck wrote:
> On 02/24/2017 07:11 AM, Krzysztof Kozlowski wrote:
> > In soft (no-reboot) mode, the driver self-pings watchdog upon expiration
> > of an interrupt.  However the interrupt itself was not cleared thus on
> > first hit, the system enters infinite interrupt handling loop.
> > 
> > On Odroid U3 (Exynos4412), when booted with s3c2410_wdt.soft_noboot=1
> > argument the console is flooded:
> > 	# killall -9 watchdog
> > 	[   60.523760] s3c2410-wdt 10060000.watchdog: watchdog timer expired (irq)
> > 	[   60.536744] s3c2410-wdt 10060000.watchdog: watchdog timer expired (irq)
> > 
> > Fix this by writing something to the WTCLRINT register to clear the
> > interrupt.  The register WTCLRINT however appeared in S3C6410 so a new
> > watchdog quirk and flavor are needed.
> > 
> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> 
> > 
> > ---
> > 
> > I was thinking about CC-ing stable, especially when I thought about
> > adding unconditional WTCLRINT clear.  However it appears that S3C2410
> > does not have the WTCLRINT register, this a new chip flavor has to be
> > added.  This makes the fix spread over driver and DTS changes.
> > 
> > Worth adding:
> > 	Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > ?
> 
> How would that be different to Cc: stable ?

I guess not much... except for the metrics that some bug was fixed.

Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring (Arm) Feb. 28, 2017, 1:12 a.m. UTC | #3
On Fri, Feb 24, 2017 at 05:11:16PM +0200, Krzysztof Kozlowski wrote:
> In soft (no-reboot) mode, the driver self-pings watchdog upon expiration
> of an interrupt.  However the interrupt itself was not cleared thus on
> first hit, the system enters infinite interrupt handling loop.
> 
> On Odroid U3 (Exynos4412), when booted with s3c2410_wdt.soft_noboot=1
> argument the console is flooded:
> 	# killall -9 watchdog
> 	[   60.523760] s3c2410-wdt 10060000.watchdog: watchdog timer expired (irq)
> 	[   60.536744] s3c2410-wdt 10060000.watchdog: watchdog timer expired (irq)
> 
> Fix this by writing something to the WTCLRINT register to clear the
> interrupt.  The register WTCLRINT however appeared in S3C6410 so a new
> watchdog quirk and flavor are needed.
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> ---
> 
> I was thinking about CC-ing stable, especially when I thought about
> adding unconditional WTCLRINT clear.  However it appears that S3C2410
> does not have the WTCLRINT register, this a new chip flavor has to be
> added.  This makes the fix spread over driver and DTS changes.
> 
> Worth adding:
> 	Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> ?
> ---
>  .../devicetree/bindings/watchdog/samsung-wdt.txt    |  9 +++++----

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

>  drivers/watchdog/s3c2410_wdt.c                      | 21 ++++++++++++++++++---
>  2 files changed, 23 insertions(+), 7 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
index 8f3d96af81d7..1f6e101e299a 100644
--- a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
@@ -6,10 +6,11 @@  occurred.
 
 Required properties:
 - compatible : should be one among the following
-	(a) "samsung,s3c2410-wdt" for Exynos4 and previous SoCs
-	(b) "samsung,exynos5250-wdt" for Exynos5250
-	(c) "samsung,exynos5420-wdt" for Exynos5420
-	(c) "samsung,exynos7-wdt" for Exynos7
+	- "samsung,s3c2410-wdt" for S3C2410
+	- "samsung,s3c6410-wdt" for S3C6410, S5PV210 and Exynos4
+	- "samsung,exynos5250-wdt" for Exynos5250
+	- "samsung,exynos5420-wdt" for Exynos5420
+	- "samsung,exynos7-wdt" for Exynos7
 
 - reg : base physical address of the controller and length of memory mapped
 	region.
diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 3e216f54cb6f..7db7847a005c 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -46,6 +46,7 @@ 
 #define S3C2410_WTCON		0x00
 #define S3C2410_WTDAT		0x04
 #define S3C2410_WTCNT		0x08
+#define S3C2410_WTCLRINT	0x0c
 
 #define S3C2410_WTCNT_MAXCNT	0xffff
 
@@ -72,6 +73,7 @@ 
 #define EXYNOS5_WDT_MASK_RESET_REG_OFFSET	0x040c
 #define QUIRK_HAS_PMU_CONFIG			(1 << 0)
 #define QUIRK_HAS_RST_STAT			(1 << 1)
+#define QUIRK_HAS_WTCLRINT_REG			(1 << 2)
 
 /* These quirks require that we have a PMU register map */
 #define QUIRKS_HAVE_PMUREG			(QUIRK_HAS_PMU_CONFIG | \
@@ -143,13 +145,18 @@  static const struct s3c2410_wdt_variant drv_data_s3c2410 = {
 };
 
 #ifdef CONFIG_OF
+static const struct s3c2410_wdt_variant drv_data_s3c6410 = {
+	.quirks = QUIRK_HAS_WTCLRINT_REG,
+};
+
 static const struct s3c2410_wdt_variant drv_data_exynos5250  = {
 	.disable_reg = EXYNOS5_WDT_DISABLE_REG_OFFSET,
 	.mask_reset_reg = EXYNOS5_WDT_MASK_RESET_REG_OFFSET,
 	.mask_bit = 20,
 	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
 	.rst_stat_bit = 20,
-	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT,
+	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT \
+		  | QUIRK_HAS_WTCLRINT_REG,
 };
 
 static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
@@ -158,7 +165,8 @@  static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
 	.mask_bit = 0,
 	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
 	.rst_stat_bit = 9,
-	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT,
+	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT \
+		  | QUIRK_HAS_WTCLRINT_REG,
 };
 
 static const struct s3c2410_wdt_variant drv_data_exynos7 = {
@@ -167,12 +175,15 @@  static const struct s3c2410_wdt_variant drv_data_exynos7 = {
 	.mask_bit = 23,
 	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
 	.rst_stat_bit = 23,	/* A57 WDTRESET */
-	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT,
+	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT \
+		  | QUIRK_HAS_WTCLRINT_REG,
 };
 
 static const struct of_device_id s3c2410_wdt_match[] = {
 	{ .compatible = "samsung,s3c2410-wdt",
 	  .data = &drv_data_s3c2410 },
+	{ .compatible = "samsung,s3c6410-wdt",
+	  .data = &drv_data_s3c6410 },
 	{ .compatible = "samsung,exynos5250-wdt",
 	  .data = &drv_data_exynos5250 },
 	{ .compatible = "samsung,exynos5420-wdt",
@@ -418,6 +429,10 @@  static irqreturn_t s3c2410wdt_irq(int irqno, void *param)
 	dev_info(wdt->dev, "watchdog timer expired (irq)\n");
 
 	s3c2410wdt_keepalive(&wdt->wdt_device);
+
+	if (wdt->drv_data->quirks & QUIRK_HAS_WTCLRINT_REG)
+		writel(0x1, wdt->reg_base + S3C2410_WTCLRINT);
+
 	return IRQ_HANDLED;
 }