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

Guenter Roeck
2017-02-24T15:38:03Z

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