[PATCH 03/10] watchdog: aspeed: Fix sequential control writes

Cédric Le Goater posted 10 patches 4 years, 6 months ago
Maintainers: Joel Stanley <joel@jms.id.au>, Peter Maydell <peter.maydell@linaro.org>, "Cédric Le Goater" <clg@kaod.org>, Andrew Jeffery <andrew@aj.id.au>
[PATCH 03/10] watchdog: aspeed: Fix sequential control writes
Posted by Cédric Le Goater 4 years, 6 months ago
From: Andrew Jeffery <andrew@aj.id.au>

The logic in the handling for the control register required toggling the
enable state for writes to stick. Rework the condition chain to allow
sequential writes that do not update the enable state.

Fixes: 854123bf8d4b ("wdt: Add Aspeed watchdog device model")
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Message-Id: <20210709053107.1829304-3-andrew@aj.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/watchdog/wdt_aspeed.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
index faa3d35fdf21..69c37af9a6e9 100644
--- a/hw/watchdog/wdt_aspeed.c
+++ b/hw/watchdog/wdt_aspeed.c
@@ -166,6 +166,8 @@ static void aspeed_wdt_write(void *opaque, hwaddr offset, uint64_t data,
         } else if (!enable && aspeed_wdt_is_enabled(s)) {
             s->regs[WDT_CTRL] = data;
             timer_del(s->timer);
+        } else {
+            s->regs[WDT_CTRL] = data;
         }
         break;
     case WDT_RESET_WIDTH:
-- 
2.31.1


Re: [PATCH 03/10] watchdog: aspeed: Fix sequential control writes
Posted by Philippe Mathieu-Daudé 4 years, 6 months ago
On 8/9/21 3:15 PM, Cédric Le Goater wrote:
> From: Andrew Jeffery <andrew@aj.id.au>
> 
> The logic in the handling for the control register required toggling the
> enable state for writes to stick. Rework the condition chain to allow
> sequential writes that do not update the enable state.
> 
> Fixes: 854123bf8d4b ("wdt: Add Aspeed watchdog device model")
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Message-Id: <20210709053107.1829304-3-andrew@aj.id.au>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/watchdog/wdt_aspeed.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
> index faa3d35fdf21..69c37af9a6e9 100644
> --- a/hw/watchdog/wdt_aspeed.c
> +++ b/hw/watchdog/wdt_aspeed.c
> @@ -166,6 +166,8 @@ static void aspeed_wdt_write(void *opaque, hwaddr offset, uint64_t data,
>          } else if (!enable && aspeed_wdt_is_enabled(s)) {
>              s->regs[WDT_CTRL] = data;
>              timer_del(s->timer);
> +        } else {
> +            s->regs[WDT_CTRL] = data;
>          }

Alternatively easier to review:

           } else {
               if (!enable && aspeed_wdt_is_enabled(s)) {
                   timer_del(s->timer);
               }
               s->regs[WDT_CTRL] = data;
           }

>          break;
>      case WDT_RESET_WIDTH:
>