Check on BMC Reboot Cause

Hi Tim Lee,
I checked your change at 
https://github.com/openbmc/phosphor-state-manager/commit/2bfb1efc4bc7e781224e19c05b51e6675f13a488 
to support BMC Reboot Cause feature. But I does not understand about why 
WDIOF_EXTERN1 is translated to watchdog reboot and WDIOF_CARDRESET is 
translated to Power ON reboot. Can you help explain about it?

I checked on 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/watchdog/watchdog-api.rst 
but the explanation seems not clear.

Note that we are trying to make Aspeed watchdog support this as below 
but I still not clear if the use of WDIOF_CARDRESET and WDIOF_EXTERN1 
are correct:

         return devm_watchdog_register_device(dev, &wdt->wdd);

Thanks,
Thang Q. Nguyen -

Hi Thang,
Sorry for late reply your message.

In my opinion, usually BMC play rule of card and be pulgged in server motherboard.
However, according watchdog.h the definition WDIOF_CARDRESET is more reasonable
for power on reboot BMC card then the other definitions.

In NPCM watchdog driver, we will provide dts flag for customer to design their watchodog
system. For example, we provide card-reset-type, ext1-reset-type and ext2-reset-type.
Customer can define their own reset type according their server motherboard design with BMC card.

card-reset-type = Power ON Reset
ext1-reset-type = Watchdog Reset 0, 1, 2 (depends on your SOC support)
ext2-reset-type = Software Reset 1, 2, 3 (depends on your SOC support)

Then according your SOC support to assign bootstatus as example:
if (rstval & wdt->card_reset)
        wdt->wdd.bootstatus |= WDIOF_CARDRESET;
if (rstval & wdt->ext1_reset)
        wdt->wdd.bootstatus |= WDIOF_EXTERN1;
if (rstval & wdt->ext2_reset)
        wdt->wdd.bootstatus |= WDIOF_EXTERN2;

Sincerely,
Tim

-----Original Message-----
From: openbmc <openbmc-bounces+chli30=nuvoton.com@lists.ozlabs.org> On Behalf Of Thang Nguyen
Sent: Monday, November 15, 2021 6:46 PM
To: OpenBMC Maillist <openbmc@lists.ozlabs.org>; , Tim Lee <timlee660101@gmail.com>
Subject: Check on BMC Reboot Cause

Hi Tim Lee,
I checked your change at
https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fopenbmc%2Fphosphor-state-manager%2Fcommit%2F2bfb1efc4bc7e781224e19c05b51e6675f13a488&amp;data=04%7C01%7Cchli30%40nuvoton.com%7C5e09ba4542d844db827108d9a82559f0%7Ca3f24931d4034b4a94f17d83ac638e07%7C0%7C0%7C637726413598056663%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=7nPq4H3s1hhZVqSfLwVHas%2F%2FVm8eWZ9ceexUzZ8bvR0%3D&amp;reserved=0
to support BMC Reboot Cause feature. But I does not understand about why
WDIOF_EXTERN1 is translated to watchdog reboot and WDIOF_CARDRESET is translated to Power ON reboot. Can you help explain about it?

I checked on
https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2FDocumentation%2Fwatchdog%2Fwatchdog-api.rst&amp;data=04%7C01%7Cchli30%40nuvoton.com%7C5e09ba4542d844db827108d9a82559f0%7Ca3f24931d4034b4a94f17d83ac638e07%7C0%7C0%7C637726413598056663%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=GRpNzdv9PYTVsFWB%2FAE2h0X1lFRXoX4LHOIOOSJb5s4%3D&amp;reserved=0
but the explanation seems not clear.

Note that we are trying to make Aspeed watchdog support this as below but I still not clear if the use of WDIOF_CARDRESET and WDIOF_EXTERN1 are correct:

diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c index 436571b6fc79..c5c3f80dfc48 100644
--- a/drivers/watchdog/aspeed_wdt.c
+++ b/drivers/watchdog/aspeed_wdt.c
@@ -54,6 +54,7 @@ MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table);
  #define   WDT_CTRL_ENABLE              BIT(0)
  #define WDT_TIMEOUT_STATUS     0x10
  #define   WDT_TIMEOUT_STATUS_BOOT_SECONDARY    BIT(1)
+#define   WDT_EVENT_COUNTER_MASK       (0xFF << 8)
  #define WDT_CLEAR_TIMEOUT_STATUS       0x14
  #define   WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION    BIT(0)

@@ -369,13 +370,19 @@ static int aspeed_wdt_probe(struct platform_device
*pdev)

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

+       if(status & WDT_EVENT_COUNTER_MASK) {
+               // Reset cause by WatchDog
+               wdt->wdd.bootstatus |= WDIOF_EXTERN1;
+       } else {
+               // Reset cause by Power On Reset
+               wdt->wdd.bootstatus |= WDIOF_CARDRESET;
+       }
+
         dev_set_drvdata(dev, wdt);

         return devm_watchdog_register_device(dev, &wdt->wdd);

Thanks,
Thang Q. Nguyen -

Thanks Tim for your response.
I have one more question on this topic. With BMC reboot cause supported, 
is the information used to check if BMC is rebooted or powered ON to 
skip chassis policy (always-on, always-off, previous) when BMC is 
rebooted? The reason is that the Host status should not be changed when 
BMC is rebooted.

On 25/11/2021 09:10, CS20 CHLi30 wrote:
> Hi Thang,
> Sorry for late reply your message.
> 
> In my opinion, usually BMC play rule of card and be pulgged in server motherboard.
> However, according watchdog.h the definition WDIOF_CARDRESET is more reasonable
> for power on reboot BMC card then the other definitions.
> 
> In NPCM watchdog driver, we will provide dts flag for customer to design their watchodog
> system. For example, we provide card-reset-type, ext1-reset-type and ext2-reset-type.
> Customer can define their own reset type according their server motherboard design with BMC card.
> 
> card-reset-type = Power ON Reset
> ext1-reset-type = Watchdog Reset 0, 1, 2 (depends on your SOC support)
> ext2-reset-type = Software Reset 1, 2, 3 (depends on your SOC support)
> 
> Then according your SOC support to assign bootstatus as example:
> if (rstval & wdt->card_reset)
>          wdt->wdd.bootstatus |= WDIOF_CARDRESET;
> if (rstval & wdt->ext1_reset)
>          wdt->wdd.bootstatus |= WDIOF_EXTERN1;
> if (rstval & wdt->ext2_reset)
>          wdt->wdd.bootstatus |= WDIOF_EXTERN2;
> 
> Sincerely,
> Tim
> 
> -----Original Message-----
> From: openbmc <openbmc-bounces+chli30=nuvoton.com@lists.ozlabs.org> On Behalf Of Thang Nguyen
> Sent: Monday, November 15, 2021 6:46 PM
> To: OpenBMC Maillist <openbmc@lists.ozlabs.org>; , Tim Lee <timlee660101@gmail.com>
> Subject: Check on BMC Reboot Cause
> 
> Hi Tim Lee,
> I checked your change at
> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fopenbmc%2Fphosphor-state-manager%2Fcommit%2F2bfb1efc4bc7e781224e19c05b51e6675f13a488&amp;data=04%7C01%7Cchli30%40nuvoton.com%7C5e09ba4542d844db827108d9a82559f0%7Ca3f24931d4034b4a94f17d83ac638e07%7C0%7C0%7C637726413598056663%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=7nPq4H3s1hhZVqSfLwVHas%2F%2FVm8eWZ9ceexUzZ8bvR0%3D&amp;reserved=0
> to support BMC Reboot Cause feature. But I does not understand about why
> WDIOF_EXTERN1 is translated to watchdog reboot and WDIOF_CARDRESET is translated to Power ON reboot. Can you help explain about it?
> 
> I checked on
> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2FDocumentation%2Fwatchdog%2Fwatchdog-api.rst&amp;data=04%7C01%7Cchli30%40nuvoton.com%7C5e09ba4542d844db827108d9a82559f0%7Ca3f24931d4034b4a94f17d83ac638e07%7C0%7C0%7C637726413598056663%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=GRpNzdv9PYTVsFWB%2FAE2h0X1lFRXoX4LHOIOOSJb5s4%3D&amp;reserved=0
> but the explanation seems not clear.
> 
> Note that we are trying to make Aspeed watchdog support this as below but I still not clear if the use of WDIOF_CARDRESET and WDIOF_EXTERN1 are correct:
> 
> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c index 436571b6fc79..c5c3f80dfc48 100644
> --- a/drivers/watchdog/aspeed_wdt.c
> +++ b/drivers/watchdog/aspeed_wdt.c
> @@ -54,6 +54,7 @@ MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table);
>    #define   WDT_CTRL_ENABLE              BIT(0)
>    #define WDT_TIMEOUT_STATUS     0x10
>    #define   WDT_TIMEOUT_STATUS_BOOT_SECONDARY    BIT(1)
> +#define   WDT_EVENT_COUNTER_MASK       (0xFF << 8)
>    #define WDT_CLEAR_TIMEOUT_STATUS       0x14
>    #define   WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION    BIT(0)
> 
> @@ -369,13 +370,19 @@ static int aspeed_wdt_probe(struct platform_device
> *pdev)
> 
>           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;
>           }
> 
> +       if(status & WDT_EVENT_COUNTER_MASK) {
> +               // Reset cause by WatchDog
> +               wdt->wdd.bootstatus |= WDIOF_EXTERN1;
> +       } else {
> +               // Reset cause by Power On Reset
> +               wdt->wdd.bootstatus |= WDIOF_CARDRESET;
> +       }
> +
>           dev_set_drvdata(dev, wdt);
> 
>           return devm_watchdog_register_device(dev, &wdt->wdd);
> 
> Thanks,
> Thang Q. Nguyen -
> ________________________________
> ________________________________
>   The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email. If you are not the addressee indicated in this email or are not responsible for delivery of the email to such a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately. Your cooperation is highly appreciated. It is advised that any unauthorized use of confidential information of Nuvoton is strictly prohibited; and any information in this email irrelevant to the official business of Nuvoton shall be deemed as neither given nor endorsed by Nuvoton.
>

Thanks your response for this topic.
This information is NOT used to check if BMC is rebooted or others.
Thus, the Host status will not be changed when BMC is rebooted.

-----Original Message-----
From: Thang Nguyen <thang@amperemail.onmicrosoft.com>
Sent: Friday, November 26, 2021 9:29 AM
To: CS20 CHLi30 <CHLI30@nuvoton.com>; OpenBMC Maillist <openbmc@lists.ozlabs.org>; Tim Lee <timlee660101@gmail.com>
Subject: Re: Check on BMC Reboot Cause

Thanks Tim for your response.
I have one more question on this topic. With BMC reboot cause supported, is the information used to check if BMC is rebooted or powered ON to skip chassis policy (always-on, always-off, previous) when BMC is rebooted? The reason is that the Host status should not be changed when BMC is rebooted.

On 25/11/2021 09:10, CS20 CHLi30 wrote:
> Hi Thang,
> Sorry for late reply your message.
>
> In my opinion, usually BMC play rule of card and be pulgged in server motherboard.
> However, according watchdog.h the definition WDIOF_CARDRESET is more
> reasonable for power on reboot BMC card then the other definitions.
>
> In NPCM watchdog driver, we will provide dts flag for customer to
> design their watchodog system. For example, we provide card-reset-type, ext1-reset-type and ext2-reset-type.
> Customer can define their own reset type according their server motherboard design with BMC card.
>
> card-reset-type = Power ON Reset
> ext1-reset-type = Watchdog Reset 0, 1, 2 (depends on your SOC support)
> ext2-reset-type = Software Reset 1, 2, 3 (depends on your SOC support)
>
> Then according your SOC support to assign bootstatus as example:
> if (rstval & wdt->card_reset)
>          wdt->wdd.bootstatus |= WDIOF_CARDRESET; if (rstval &
> wdt->ext1_reset)
>          wdt->wdd.bootstatus |= WDIOF_EXTERN1; if (rstval &
> wdt->ext2_reset)
>          wdt->wdd.bootstatus |= WDIOF_EXTERN2;
>
> Sincerely,
> Tim
>
> -----Original Message-----
> From: openbmc <openbmc-bounces+chli30=nuvoton.com@lists.ozlabs.org> On
> Behalf Of Thang Nguyen
> Sent: Monday, November 15, 2021 6:46 PM
> To: OpenBMC Maillist <openbmc@lists.ozlabs.org>; , Tim Lee
> <timlee660101@gmail.com>
> Subject: Check on BMC Reboot Cause
>
> Hi Tim Lee,
> I checked your change at
> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Fopenbmc%2Fphosphor-state-manager%2Fcommit%2F2bfb1efc4bc7e7812
> 24e19c05b51e6675f13a488&amp;data=04%7C01%7CCHLI30%40nuvoton.com%7C75ce
> 6806e6974058580108d9b07c32bd%7Ca3f24931d4034b4a94f17d83ac638e07%7C0%7C
> 0%7C637734872896751423%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJ
> QIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=eU0eIoLtq
> VBXEZzth4op9q%2BqGqdpHxc1JTi45ZDKSz4%3D&amp;reserved=0
> to support BMC Reboot Cause feature. But I does not understand about
> why
> WDIOF_EXTERN1 is translated to watchdog reboot and WDIOF_CARDRESET is translated to Power ON reboot. Can you help explain about it?
>
> I checked on
> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.
> kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2
> Ftree%2FDocumentation%2Fwatchdog%2Fwatchdog-api.rst&amp;data=04%7C01%7
> CCHLI30%40nuvoton.com%7C75ce6806e6974058580108d9b07c32bd%7Ca3f24931d40
> 34b4a94f17d83ac638e07%7C0%7C0%7C637734872896761421%7CUnknown%7CTWFpbGZ
> sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> D%7C3000&amp;sdata=vwvVKtx8NhNoxTYCuR9pYigYZ4fI68D1l5BeB2X96pQ%3D&amp;
> reserved=0
> but the explanation seems not clear.
>
> Note that we are trying to make Aspeed watchdog support this as below but I still not clear if the use of WDIOF_CARDRESET and WDIOF_EXTERN1 are correct:
>
> diff --git a/drivers/watchdog/aspeed_wdt.c
> b/drivers/watchdog/aspeed_wdt.c index 436571b6fc79..c5c3f80dfc48
> 100644
> --- a/drivers/watchdog/aspeed_wdt.c
> +++ b/drivers/watchdog/aspeed_wdt.c
> @@ -54,6 +54,7 @@ MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table);
>    #define   WDT_CTRL_ENABLE              BIT(0)
>    #define WDT_TIMEOUT_STATUS     0x10
>    #define   WDT_TIMEOUT_STATUS_BOOT_SECONDARY    BIT(1)
> +#define   WDT_EVENT_COUNTER_MASK       (0xFF << 8)
>    #define WDT_CLEAR_TIMEOUT_STATUS       0x14
>    #define   WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION    BIT(0)
>
> @@ -369,13 +370,19 @@ static int aspeed_wdt_probe(struct
> platform_device
> *pdev)
>
>           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;
>           }
>
> +       if(status & WDT_EVENT_COUNTER_MASK) {
> +               // Reset cause by WatchDog
> +               wdt->wdd.bootstatus |= WDIOF_EXTERN1;
> +       } else {
> +               // Reset cause by Power On Reset
> +               wdt->wdd.bootstatus |= WDIOF_CARDRESET;
> +       }
> +
>           dev_set_drvdata(dev, wdt);
>
>           return devm_watchdog_register_device(dev, &wdt->wdd);
>
> Thanks,
> Thang Q. Nguyen -
> ________________________________
> ________________________________
>   The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email. If you are not the addressee indicated in this email or are not responsible for delivery of the email to such a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately. Your cooperation is highly appreciated. It is advised that any unauthorized use of confidential information of Nuvoton is strictly prohibited; and any information in this email irrelevant to the official business of Nuvoton shall be deemed as neither given nor endorsed by Nuvoton.
>