[PATCH v6 0/1] Update ASPEED WDT bootstatus

[PATCH v6 0/1] Update ASPEED WDT bootstatus

[Chin-Ting Kuo]

This patch series inherits the patch submitted by Peter.
https://patchwork.kernel.org/project/linux-watchdog/patch/20240430143114.1323686-2-peteryin.openbmc@gmail.com/
Bootstatus mechanism is reconstructed to the latest
architecture and for all existing ASPEED BMC platforms.

Changes in v2:
  - Support SW restart on AST2600 by default without
    adding any dts property.

Changes in v3:
  - Get watchdog controller index by dividing register
    base offset by register size.

Changes in v4:
  - Update the commit message for updating bootstatus
    handling patch.
  - Rename aspeed_wdt_config struct to aspeed_wdt_data.
  - Create restart callback function.

Changes in v5:
  - Remove SW reset mechanism since there is no consensus
    about bootstatus for SW reset currently.
  - Correct the method for clearing reset event flag on
    AST2400 and AST2500 legacy platforms.

Changes in v6:
  - Use resource_size() function to get WDT controller
    registers size.

Chin-Ting Kuo (1):
  watchdog: aspeed: Update bootstatus handling

 drivers/watchdog/aspeed_wdt.c | 81 ++++++++++++++++++++++++++++++++++-
 1 file changed, 79 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH v6 1/1] watchdog: aspeed: Update bootstatus handling

[Chin-Ting Kuo]

The boot status in the watchdog device struct is updated during
controller probe stage. Application layer can get the boot status
through the command, cat /sys/class/watchdog/watchdogX/bootstatus.
The bootstatus can be,
WDIOF_CARDRESET => the system is reset by WDT SoC reset.
Others          => other reset events, e.g., power on reset.

On ASPEED platform, the boot status is recorded in the SCU registers.
- AST2400: Only a bit represents for any WDT reset.
- AST2500/AST2600: The reset triggered by different WDT controllers
                   can be distinguished by different SCU bits.

Besides, on AST2400 and AST2500, since alternating boot event is
triggered by WDT SoC reset, it is classified as WDIOF_CARDRESET.

Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
---
 drivers/watchdog/aspeed_wdt.c | 81 ++++++++++++++++++++++++++++++++++-
 1 file changed, 79 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
index b4773a6aaf8c..369635b38ca0 100644
--- a/drivers/watchdog/aspeed_wdt.c
+++ b/drivers/watchdog/aspeed_wdt.c
@@ -11,21 +11,30 @@
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/kstrtox.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_irq.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/watchdog.h>
 
 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 aspeed_wdt_scu {
+	const char *compatible;
+	u32 reset_status_reg;
+	u32 wdt_reset_mask;
+	u32 wdt_reset_mask_shift;
+};
 
 struct aspeed_wdt_config {
 	u32 ext_pulse_width_mask;
 	u32 irq_shift;
 	u32 irq_mask;
+	struct aspeed_wdt_scu scu;
 };
 
 struct aspeed_wdt {
@@ -39,18 +48,36 @@ static const struct aspeed_wdt_config ast2400_config = {
 	.ext_pulse_width_mask = 0xff,
 	.irq_shift = 0,
 	.irq_mask = 0,
+	.scu = {
+		.compatible = "aspeed,ast2400-scu",
+		.reset_status_reg = 0x3c,
+		.wdt_reset_mask = 0x1,
+		.wdt_reset_mask_shift = 1,
+	},
 };
 
 static const struct aspeed_wdt_config ast2500_config = {
 	.ext_pulse_width_mask = 0xfffff,
 	.irq_shift = 12,
 	.irq_mask = GENMASK(31, 12),
+	.scu = {
+		.compatible = "aspeed,ast2500-scu",
+		.reset_status_reg = 0x3c,
+		.wdt_reset_mask = 0x1,
+		.wdt_reset_mask_shift = 2,
+	},
 };
 
 static const struct aspeed_wdt_config ast2600_config = {
 	.ext_pulse_width_mask = 0xfffff,
 	.irq_shift = 0,
 	.irq_mask = GENMASK(31, 10),
+	.scu = {
+		.compatible = "aspeed,ast2600-scu",
+		.reset_status_reg = 0x74,
+		.wdt_reset_mask = 0xf,
+		.wdt_reset_mask_shift = 16,
+	},
 };
 
 static const struct of_device_id aspeed_wdt_of_table[] = {
@@ -213,6 +240,56 @@ static int aspeed_wdt_restart(struct watchdog_device *wdd,
 	return 0;
 }
 
+static void aspeed_wdt_update_bootstatus(struct platform_device *pdev,
+					 struct aspeed_wdt *wdt)
+{
+	const struct resource *res;
+	struct aspeed_wdt_scu scu = wdt->cfg->scu;
+	struct regmap *scu_base;
+	u32 reset_mask_width;
+	u32 reset_mask_shift;
+	u32 idx = 0;
+	u32 status;
+	int ret;
+
+	if (!of_device_is_compatible(pdev->dev.of_node, "aspeed,ast2400-wdt")) {
+		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+		idx = ((intptr_t)wdt->base & 0x00000fff) / resource_size(res);
+	}
+
+	scu_base = syscon_regmap_lookup_by_compatible(scu.compatible);
+	if (IS_ERR(scu_base)) {
+		wdt->wdd.bootstatus = WDIOS_UNKNOWN;
+		return;
+	}
+
+	ret = regmap_read(scu_base, scu.reset_status_reg, &status);
+	if (ret) {
+		wdt->wdd.bootstatus = WDIOS_UNKNOWN;
+		return;
+	}
+
+	reset_mask_width = hweight32(scu.wdt_reset_mask);
+	reset_mask_shift = scu.wdt_reset_mask_shift +
+			   reset_mask_width * idx;
+
+	if (status & (scu.wdt_reset_mask << reset_mask_shift))
+		wdt->wdd.bootstatus = WDIOF_CARDRESET;
+
+	/* clear wdt reset event flag */
+	if (of_device_is_compatible(pdev->dev.of_node, "aspeed,ast2400-wdt") ||
+	    of_device_is_compatible(pdev->dev.of_node, "aspeed,ast2500-wdt")) {
+		ret = regmap_read(scu_base, scu.reset_status_reg, &status);
+		if (!ret) {
+			status &= ~(scu.wdt_reset_mask << reset_mask_shift);
+			regmap_write(scu_base, scu.reset_status_reg, status);
+		}
+	} else {
+		regmap_write(scu_base, scu.reset_status_reg,
+			     scu.wdt_reset_mask << reset_mask_shift);
+	}
+}
+
 /* access_cs0 shows if cs0 is accessible, hence the reverted bit */
 static ssize_t access_cs0_show(struct device *dev,
 			       struct device_attribute *attr, char *buf)
@@ -458,10 +535,10 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
 		writel(duration - 1, wdt->base + WDT_RESET_WIDTH);
 	}
 
+	aspeed_wdt_update_bootstatus(pdev, wdt);
+
 	status = readl(wdt->base + WDT_TIMEOUT_STATUS);
 	if (status & WDT_TIMEOUT_STATUS_BOOT_SECONDARY) {
-		wdt->wdd.bootstatus = WDIOF_CARDRESET;
-
 		if (of_device_is_compatible(np, "aspeed,ast2400-wdt") ||
 		    of_device_is_compatible(np, "aspeed,ast2500-wdt"))
 			wdt->wdd.groups = bswitch_groups;
-- 
2.34.1

Re: [PATCH v6 1/1] watchdog: aspeed: Update bootstatus handling

[Guenter Roeck]

On 1/12/25 00:12, Chin-Ting Kuo wrote:
> The boot status in the watchdog device struct is updated during
> controller probe stage. Application layer can get the boot status
> through the command, cat /sys/class/watchdog/watchdogX/bootstatus.
> The bootstatus can be,
> WDIOF_CARDRESET => the system is reset by WDT SoC reset.
> Others          => other reset events, e.g., power on reset.
> 
> On ASPEED platform, the boot status is recorded in the SCU registers.
> - AST2400: Only a bit represents for any WDT reset.
> - AST2500/AST2600: The reset triggered by different WDT controllers
>                     can be distinguished by different SCU bits.
> 
> Besides, on AST2400 and AST2500, since alternating boot event is
> triggered by WDT SoC reset, it is classified as WDIOF_CARDRESET.
> 
> Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>

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

Re: [PATCH v6 1/1] watchdog: aspeed: Update bootstatus handling

[Andrew Jeffery]

On Sun, 2025-01-12 at 16:12 +0800, Chin-Ting Kuo wrote:
> The boot status in the watchdog device struct is updated during
> controller probe stage. Application layer can get the boot status
> through the command, cat /sys/class/watchdog/watchdogX/bootstatus.
> The bootstatus can be,
> WDIOF_CARDRESET => the system is reset by WDT SoC reset.
> Others          => other reset events, e.g., power on reset.
> 
> On ASPEED platform, the boot status is recorded in the SCU registers.
> - AST2400: Only a bit represents for any WDT reset.
> - AST2500/AST2600: The reset triggered by different WDT controllers
>                    can be distinguished by different SCU bits.
> 
> Besides, on AST2400 and AST2500, since alternating boot event is
> triggered by WDT SoC reset, it is classified as WDIOF_CARDRESET.
> 
> Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
> ---
>  drivers/watchdog/aspeed_wdt.c | 81
> ++++++++++++++++++++++++++++++++++-
>  1 file changed, 79 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/watchdog/aspeed_wdt.c
> b/drivers/watchdog/aspeed_wdt.c
> index b4773a6aaf8c..369635b38ca0 100644
> --- a/drivers/watchdog/aspeed_wdt.c
> +++ b/drivers/watchdog/aspeed_wdt.c
> @@ -11,21 +11,30 @@
>  #include <linux/io.h>
>  #include <linux/kernel.h>
>  #include <linux/kstrtox.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_irq.h>
>  #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>  #include <linux/watchdog.h>
>  
>  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 aspeed_wdt_scu {
> +       const char *compatible;
> +       u32 reset_status_reg;
> +       u32 wdt_reset_mask;
> +       u32 wdt_reset_mask_shift;
> +};
>  
>  struct aspeed_wdt_config {
>         u32 ext_pulse_width_mask;
>         u32 irq_shift;
>         u32 irq_mask;
> +       struct aspeed_wdt_scu scu;
>  };
>  
>  struct aspeed_wdt {
> @@ -39,18 +48,36 @@ static const struct aspeed_wdt_config
> ast2400_config = {
>         .ext_pulse_width_mask = 0xff,
>         .irq_shift = 0,
>         .irq_mask = 0,
> +       .scu = {
> +               .compatible = "aspeed,ast2400-scu",
> +               .reset_status_reg = 0x3c,
> +               .wdt_reset_mask = 0x1,
> +               .wdt_reset_mask_shift = 1,
> +       },
>  };
>  
>  static const struct aspeed_wdt_config ast2500_config = {
>         .ext_pulse_width_mask = 0xfffff,
>         .irq_shift = 12,
>         .irq_mask = GENMASK(31, 12),
> +       .scu = {
> +               .compatible = "aspeed,ast2500-scu",
> +               .reset_status_reg = 0x3c,
> +               .wdt_reset_mask = 0x1,
> +               .wdt_reset_mask_shift = 2,
> +       },
>  };
>  
>  static const struct aspeed_wdt_config ast2600_config = {
>         .ext_pulse_width_mask = 0xfffff,
>         .irq_shift = 0,
>         .irq_mask = GENMASK(31, 10),
> +       .scu = {
> +               .compatible = "aspeed,ast2600-scu",
> +               .reset_status_reg = 0x74,
> +               .wdt_reset_mask = 0xf,
> +               .wdt_reset_mask_shift = 16,
> +       },
>  };
>  
>  static const struct of_device_id aspeed_wdt_of_table[] = {
> @@ -213,6 +240,56 @@ static int aspeed_wdt_restart(struct
> watchdog_device *wdd,
>         return 0;
>  }
>  
> +static void aspeed_wdt_update_bootstatus(struct platform_device
> *pdev,
> +                                        struct aspeed_wdt *wdt)
> +{
> +       const struct resource *res;
> +       struct aspeed_wdt_scu scu = wdt->cfg->scu;
> +       struct regmap *scu_base;
> +       u32 reset_mask_width;
> +       u32 reset_mask_shift;
> +       u32 idx = 0;
> +       u32 status;
> +       int ret;
> +
> +       if (!of_device_is_compatible(pdev->dev.of_node,
> "aspeed,ast2400-wdt")) {
> +               res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +               idx = ((intptr_t)wdt->base & 0x00000fff) /
> resource_size(res);
> +       }
> +
> +       scu_base =
> syscon_regmap_lookup_by_compatible(scu.compatible);
> +       if (IS_ERR(scu_base)) {
> +               wdt->wdd.bootstatus = WDIOS_UNKNOWN;
> +               return;
> +       }
> +
> +       ret = regmap_read(scu_base, scu.reset_status_reg, &status);
> +       if (ret) {
> +               wdt->wdd.bootstatus = WDIOS_UNKNOWN;
> +               return;
> +       }
> +
> +       reset_mask_width = hweight32(scu.wdt_reset_mask);
> +       reset_mask_shift = scu.wdt_reset_mask_shift +
> +                          reset_mask_width * idx;
> +
> +       if (status & (scu.wdt_reset_mask << reset_mask_shift))
> +               wdt->wdd.bootstatus = WDIOF_CARDRESET;

How precise is the wording in your commit message? The AST2600, for
instance, has 4 reset types:

1. "SoC"
2. "Full"
3. "ARM"
4. "Software"

In the commit message, you said:

> WDIOF_CARDRESET => the system is reset by WDT SoC reset.

However the logic here (with the way you've initialised the config
struct for the AST2600) will signal WDIOF_CARDRESET even if what
occurred was e.g. a (graceful?) software reset.

The only thing WDIOF_CARDRESET differentiates is a power-on reset from
any other kind of watchdog-driven reset.

Is that what's intended? I'm just wary of confusion for what appears to
be a generic use of "SoC" in the commit message vs the specific "SoC"
mode of the watchdog controller (should some documentation be
written/updated?)

Andrew

RE: [PATCH v6 1/1] watchdog: aspeed: Update bootstatus handling

[Chin-Ting Kuo]

Hi Andrew,

> -----Original Message-----
> From: Andrew Jeffery <andrew@codeconstruct.com.au>
> Sent: Monday, January 13, 2025 11:29 AM
> Subject: Re: [PATCH v6 1/1] watchdog: aspeed: Update bootstatus handling
> 
> On Sun, 2025-01-12 at 16:12 +0800, Chin-Ting Kuo wrote:
> > The boot status in the watchdog device struct is updated during
> > controller probe stage. Application layer can get the boot status
> > through the command, cat /sys/class/watchdog/watchdogX/bootstatus.
> > The bootstatus can be,
> > WDIOF_CARDRESET => the system is reset by WDT SoC reset.
> > Others          => other reset events, e.g., power on reset.
> >
> > On ASPEED platform, the boot status is recorded in the SCU registers.
> > - AST2400: Only a bit represents for any WDT reset.
> > - AST2500/AST2600: The reset triggered by different WDT controllers
> >                    can be distinguished by different SCU bits.
> >
> > Besides, on AST2400 and AST2500, since alternating boot event is
> > triggered by WDT SoC reset, it is classified as WDIOF_CARDRESET.
> >
> > Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
> > ---
> >  drivers/watchdog/aspeed_wdt.c | 81
> > ++++++++++++++++++++++++++++++++++-
> >  1 file changed, 79 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/watchdog/aspeed_wdt.c
> > b/drivers/watchdog/aspeed_wdt.c index b4773a6aaf8c..369635b38ca0
> > 100644
> > --- a/drivers/watchdog/aspeed_wdt.c
> > +++ b/drivers/watchdog/aspeed_wdt.c
> > @@ -11,21 +11,30 @@
> >  #include <linux/io.h>
> >  #include <linux/kernel.h>
> >  #include <linux/kstrtox.h>
> > +#include <linux/mfd/syscon.h>
> >  #include <linux/module.h>
> >  #include <linux/of.h>
> >  #include <linux/of_irq.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> >  #include <linux/watchdog.h>
> >
> >  static bool nowayout = WATCHDOG_NOWAYOUT;
> >  module_param(nowayout, bool, 0);
> >  MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once
> started
> > (default="
> >                                 __MODULE_STRING(WATC
> HDOG_NOWAYOUT)
> > ")");
> > +struct aspeed_wdt_scu {
> > +       const char *compatible;
> > +       u32 reset_status_reg;
> > +       u32 wdt_reset_mask;
> > +       u32 wdt_reset_mask_shift;
> > +};
> >
> >  struct aspeed_wdt_config {
> >         u32 ext_pulse_width_mask;
> >         u32 irq_shift;
> >         u32 irq_mask;
> > +       struct aspeed_wdt_scu scu;
> >  };
> >
> >  struct aspeed_wdt {
> > @@ -39,18 +48,36 @@ static const struct aspeed_wdt_config
> > ast2400_config = {
> >         .ext_pulse_width_mask = 0xff,
> >         .irq_shift = 0,
> >         .irq_mask = 0,
> > +       .scu = {
> > +               .compatible = "aspeed,ast2400-scu",
> > +               .reset_status_reg = 0x3c,
> > +               .wdt_reset_mask = 0x1,
> > +               .wdt_reset_mask_shift = 1,
> > +       },
> >  };
> >
> >  static const struct aspeed_wdt_config ast2500_config = {
> >         .ext_pulse_width_mask = 0xfffff,
> >         .irq_shift = 12,
> >         .irq_mask = GENMASK(31, 12),
> > +       .scu = {
> > +               .compatible = "aspeed,ast2500-scu",
> > +               .reset_status_reg = 0x3c,
> > +               .wdt_reset_mask = 0x1,
> > +               .wdt_reset_mask_shift = 2,
> > +       },
> >  };
> >
> >  static const struct aspeed_wdt_config ast2600_config = {
> >         .ext_pulse_width_mask = 0xfffff,
> >         .irq_shift = 0,
> >         .irq_mask = GENMASK(31, 10),
> > +       .scu = {
> > +               .compatible = "aspeed,ast2600-scu",
> > +               .reset_status_reg = 0x74,
> > +               .wdt_reset_mask = 0xf,
> > +               .wdt_reset_mask_shift = 16,
> > +       },
> >  };
> >
> >  static const struct of_device_id aspeed_wdt_of_table[] = { @@ -213,6
> > +240,56 @@ static int aspeed_wdt_restart(struct watchdog_device *wdd,
> >         return 0;
> >  }
> >
> > +static void aspeed_wdt_update_bootstatus(struct platform_device
> > *pdev,
> > +                                        struct
> aspeed_wdt *wdt) {
> > +       const struct resource *res;
> > +       struct aspeed_wdt_scu scu = wdt->cfg->scu;
> > +       struct regmap *scu_base;
> > +       u32 reset_mask_width;
> > +       u32 reset_mask_shift;
> > +       u32 idx = 0;
> > +       u32 status;
> > +       int ret;
> > +
> > +       if (!of_device_is_compatible(pdev->dev.of_node,
> > "aspeed,ast2400-wdt")) {
> > +               res = platform_get_resource(pdev,
> IORESOURCE_MEM, 0);
> > +               idx = ((intptr_t)wdt->base & 0x00000fff) /
> > resource_size(res);
> > +       }
> > +
> > +       scu_base =
> > syscon_regmap_lookup_by_compatible(scu.compatible);
> > +       if (IS_ERR(scu_base)) {
> > +               wdt->wdd.bootstatus = WDIOS_UNKNOWN;
> > +               return;
> > +       }
> > +
> > +       ret = regmap_read(scu_base, scu.reset_status_reg, &status);
> > +       if (ret) {
> > +               wdt->wdd.bootstatus = WDIOS_UNKNOWN;
> > +               return;
> > +       }
> > +
> > +       reset_mask_width = hweight32(scu.wdt_reset_mask);
> > +       reset_mask_shift = scu.wdt_reset_mask_shift +
> > +                          reset_mask_width * idx;
> > +
> > +       if (status & (scu.wdt_reset_mask << reset_mask_shift))
> > +               wdt->wdd.bootstatus = WDIOF_CARDRESET;
> 
> How precise is the wording in your commit message? The AST2600, for
> instance, has 4 reset types:
> 
> 1. "SoC"
> 2. "Full"
> 3. "ARM"
> 4. "Software"
> 
> In the commit message, you said:
> 
> > WDIOF_CARDRESET => the system is reset by WDT SoC reset.
> 

The commit message should be changed for more precisely.
"WDIOF_CARDRESET => the system is reset due to WDT timeout occurs."
This change can be updated with a new patch version.

> However the logic here (with the way you've initialised the config struct for the
> AST2600) will signal WDIOF_CARDRESET even if what occurred was e.g. a
> (graceful?) software reset.
> 

As the discussion in the previous patch series, there is no consensus for graceful
reset flag in WDT framework. Thus, this patch only distinguishes WDT reset from
other reset reasons.

> The only thing WDIOF_CARDRESET differentiates is a power-on reset from any
> other kind of watchdog-driven reset.
> 

Yes.

> Is that what's intended? I'm just wary of confusion for what appears to be a
> generic use of "SoC" in the commit message vs the specific "SoC"
> mode of the watchdog controller (should some documentation be
> written/updated?)
> 

The commit message can be updated.

Chin-Ting

Re: [PATCH v6 1/1] watchdog: aspeed: Update bootstatus handling

[Andrew Jeffery]

On Mon, 2025-01-13 at 03:41 +0000, Chin-Ting Kuo wrote:
> Hi Andrew,
> 
> > -----Original Message-----
> > From: Andrew Jeffery <andrew@codeconstruct.com.au>
> > Sent: Monday, January 13, 2025 11:29 AM
> > Subject: Re: [PATCH v6 1/1] watchdog: aspeed: Update bootstatus
> > handling
> > 
> > On Sun, 2025-01-12 at 16:12 +0800, Chin-Ting Kuo wrote:
> > > The boot status in the watchdog device struct is updated during
> > > controller probe stage. Application layer can get the boot status
> > > through the command, cat
> > > /sys/class/watchdog/watchdogX/bootstatus.
> > > The bootstatus can be,
> > > WDIOF_CARDRESET => the system is reset by WDT SoC reset.
> > > Others          => other reset events, e.g., power on reset.
> > > 
> > > On ASPEED platform, the boot status is recorded in the SCU
> > > registers.
> > > - AST2400: Only a bit represents for any WDT reset.
> > > - AST2500/AST2600: The reset triggered by different WDT
> > > controllers
> > >                    can be distinguished by different SCU bits.
> > > 
> > > Besides, on AST2400 and AST2500, since alternating boot event is
> > > triggered by WDT SoC reset, it is classified as WDIOF_CARDRESET.
> > > 
> > > Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
> > > ---
> > >  drivers/watchdog/aspeed_wdt.c | 81
> > > ++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 79 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/watchdog/aspeed_wdt.c
> > > b/drivers/watchdog/aspeed_wdt.c index b4773a6aaf8c..369635b38ca0
> > > 100644
> > > --- a/drivers/watchdog/aspeed_wdt.c
> > > +++ b/drivers/watchdog/aspeed_wdt.c
> > > @@ -11,21 +11,30 @@
> > >  #include <linux/io.h>
> > >  #include <linux/kernel.h>
> > >  #include <linux/kstrtox.h>
> > > +#include <linux/mfd/syscon.h>
> > >  #include <linux/module.h>
> > >  #include <linux/of.h>
> > >  #include <linux/of_irq.h>
> > >  #include <linux/platform_device.h>
> > > +#include <linux/regmap.h>
> > >  #include <linux/watchdog.h>
> > > 
> > >  static bool nowayout = WATCHDOG_NOWAYOUT;
> > >  module_param(nowayout, bool, 0);
> > >  MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once
> > started
> > > (default="
> > >                                 __MODULE_STRING(WATC
> > HDOG_NOWAYOUT)
> > > ")");
> > > +struct aspeed_wdt_scu {
> > > +       const char *compatible;
> > > +       u32 reset_status_reg;
> > > +       u32 wdt_reset_mask;
> > > +       u32 wdt_reset_mask_shift;
> > > +};
> > > 
> > >  struct aspeed_wdt_config {
> > >         u32 ext_pulse_width_mask;
> > >         u32 irq_shift;
> > >         u32 irq_mask;
> > > +       struct aspeed_wdt_scu scu;
> > >  };
> > > 
> > >  struct aspeed_wdt {
> > > @@ -39,18 +48,36 @@ static const struct aspeed_wdt_config
> > > ast2400_config = {
> > >         .ext_pulse_width_mask = 0xff,
> > >         .irq_shift = 0,
> > >         .irq_mask = 0,
> > > +       .scu = {
> > > +               .compatible = "aspeed,ast2400-scu",
> > > +               .reset_status_reg = 0x3c,
> > > +               .wdt_reset_mask = 0x1,
> > > +               .wdt_reset_mask_shift = 1,
> > > +       },
> > >  };
> > > 
> > >  static const struct aspeed_wdt_config ast2500_config = {
> > >         .ext_pulse_width_mask = 0xfffff,
> > >         .irq_shift = 12,
> > >         .irq_mask = GENMASK(31, 12),
> > > +       .scu = {
> > > +               .compatible = "aspeed,ast2500-scu",
> > > +               .reset_status_reg = 0x3c,
> > > +               .wdt_reset_mask = 0x1,
> > > +               .wdt_reset_mask_shift = 2,
> > > +       },
> > >  };
> > > 
> > >  static const struct aspeed_wdt_config ast2600_config = {
> > >         .ext_pulse_width_mask = 0xfffff,
> > >         .irq_shift = 0,
> > >         .irq_mask = GENMASK(31, 10),
> > > +       .scu = {
> > > +               .compatible = "aspeed,ast2600-scu",
> > > +               .reset_status_reg = 0x74,
> > > +               .wdt_reset_mask = 0xf,
> > > +               .wdt_reset_mask_shift = 16,
> > > +       },
> > >  };
> > > 
> > >  static const struct of_device_id aspeed_wdt_of_table[] = { @@ -
> > > 213,6
> > > +240,56 @@ static int aspeed_wdt_restart(struct watchdog_device
> > > *wdd,
> > >         return 0;
> > >  }
> > > 
> > > +static void aspeed_wdt_update_bootstatus(struct platform_device
> > > *pdev,
> > > +                                        struct
> > aspeed_wdt *wdt) {
> > > +       const struct resource *res;
> > > +       struct aspeed_wdt_scu scu = wdt->cfg->scu;
> > > +       struct regmap *scu_base;
> > > +       u32 reset_mask_width;
> > > +       u32 reset_mask_shift;
> > > +       u32 idx = 0;
> > > +       u32 status;
> > > +       int ret;
> > > +
> > > +       if (!of_device_is_compatible(pdev->dev.of_node,
> > > "aspeed,ast2400-wdt")) {
> > > +               res = platform_get_resource(pdev,
> > IORESOURCE_MEM, 0);
> > > +               idx = ((intptr_t)wdt->base & 0x00000fff) /
> > > resource_size(res);
> > > +       }
> > > +
> > > +       scu_base =
> > > syscon_regmap_lookup_by_compatible(scu.compatible);
> > > +       if (IS_ERR(scu_base)) {
> > > +               wdt->wdd.bootstatus = WDIOS_UNKNOWN;
> > > +               return;
> > > +       }
> > > +
> > > +       ret = regmap_read(scu_base, scu.reset_status_reg,
> > > &status);
> > > +       if (ret) {
> > > +               wdt->wdd.bootstatus = WDIOS_UNKNOWN;
> > > +               return;
> > > +       }
> > > +
> > > +       reset_mask_width = hweight32(scu.wdt_reset_mask);
> > > +       reset_mask_shift = scu.wdt_reset_mask_shift +
> > > +                          reset_mask_width * idx;
> > > +
> > > +       if (status & (scu.wdt_reset_mask << reset_mask_shift))
> > > +               wdt->wdd.bootstatus = WDIOF_CARDRESET;
> > 
> > How precise is the wording in your commit message? The AST2600, for
> > instance, has 4 reset types:
> > 
> > 1. "SoC"
> > 2. "Full"
> > 3. "ARM"
> > 4. "Software"
> > 
> > In the commit message, you said:
> > 
> > > WDIOF_CARDRESET => the system is reset by WDT SoC reset.
> > 
> 
> The commit message should be changed for more precisely.
> "WDIOF_CARDRESET => the system is reset due to WDT timeout occurs."
> This change can be updated with a new patch version.
> 
> > However the logic here (with the way you've initialised the config
> > struct for the
> > AST2600) will signal WDIOF_CARDRESET even if what occurred was e.g.
> > a
> > (graceful?) software reset.
> > 
> 
> As the discussion in the previous patch series, there is no consensus
> for graceful
> reset flag in WDT framework. Thus, this patch only distinguishes WDT
> reset from
> other reset reasons.
> 
> > The only thing WDIOF_CARDRESET differentiates is a power-on reset
> > from any
> > other kind of watchdog-driven reset.
> > 
> 
> Yes.
> 
> > Is that what's intended? I'm just wary of confusion for what
> > appears to be a
> > generic use of "SoC" in the commit message vs the specific "SoC"
> > mode of the watchdog controller (should some documentation be
> > written/updated?)
> > 
> 
> The commit message can be updated.

Okay, thanks.

Andrew