* [PATCH 0/4] [PATCH 0/4] Update ASPEED WDT bootstatus @ 2024-10-07 6:34 ` Chin-Ting Kuo 0 siblings, 0 replies; 26+ messages in thread From: Chin-Ting Kuo @ 2024-10-07 6:34 UTC (permalink / raw) To: linux-aspeed This patch series inherits the patch submitted by Peter. https://patchwork.kernel.org/project/linux-watchdog/patch/20240430143114.1323686-2-peteryin.openbmc at gmail.com/ Besides, the boot status modififed in the WDT driver obeys the rules proposed in the OpenBMC. https://github.com/openbmc/docs/blob/master/designs/bmc-reboot-cause-update.md#proposed-design Moreover, WDT SW reset is supported since AST2600 platform and is also included in this patch series. Chin-Ting Kuo (4): dt-bindings: watchdog: aspeed: Add property for WDT SW reset ARM: dts: aspeed: Add WDT controller into alias field watchdog: aspeed: Update bootstatus handling watchdog: aspeed: Add support for SW restart .../bindings/watchdog/aspeed,ast2400-wdt.yaml | 11 ++ arch/arm/boot/dts/aspeed/aspeed-g4.dtsi | 2 + arch/arm/boot/dts/aspeed/aspeed-g5.dtsi | 3 + arch/arm/boot/dts/aspeed/aspeed-g6.dtsi | 4 + drivers/watchdog/aspeed_wdt.c | 149 ++++++++++++++++-- 5 files changed, 160 insertions(+), 9 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 0/4] [PATCH 0/4] Update ASPEED WDT bootstatus @ 2024-10-07 6:34 ` Chin-Ting Kuo 0 siblings, 0 replies; 26+ messages in thread From: Chin-Ting Kuo @ 2024-10-07 6:34 UTC (permalink / raw) To: patrick, wim, linux, robh, krzk+dt, conor+dt, joel, andrew, linux-watchdog, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel Cc: Peter.Yin, Patrick_NC_Lin, Bonnie_Lo, DELPHINE_CHIU, BMC-SW This patch series inherits the patch submitted by Peter. https://patchwork.kernel.org/project/linux-watchdog/patch/20240430143114.1323686-2-peteryin.openbmc@gmail.com/ Besides, the boot status modififed in the WDT driver obeys the rules proposed in the OpenBMC. https://github.com/openbmc/docs/blob/master/designs/bmc-reboot-cause-update.md#proposed-design Moreover, WDT SW reset is supported since AST2600 platform and is also included in this patch series. Chin-Ting Kuo (4): dt-bindings: watchdog: aspeed: Add property for WDT SW reset ARM: dts: aspeed: Add WDT controller into alias field watchdog: aspeed: Update bootstatus handling watchdog: aspeed: Add support for SW restart .../bindings/watchdog/aspeed,ast2400-wdt.yaml | 11 ++ arch/arm/boot/dts/aspeed/aspeed-g4.dtsi | 2 + arch/arm/boot/dts/aspeed/aspeed-g5.dtsi | 3 + arch/arm/boot/dts/aspeed/aspeed-g6.dtsi | 4 + drivers/watchdog/aspeed_wdt.c | 149 ++++++++++++++++-- 5 files changed, 160 insertions(+), 9 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT SW reset 2024-10-07 6:34 ` Chin-Ting Kuo @ 2024-10-07 6:34 ` Chin-Ting Kuo -1 siblings, 0 replies; 26+ messages in thread From: Chin-Ting Kuo @ 2024-10-07 6:34 UTC (permalink / raw) To: linux-aspeed Add "aspeed,restart-sw" property to distinguish normal WDT reset from system restart triggered by SW consciously. Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> --- .../bindings/watchdog/aspeed,ast2400-wdt.yaml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml index be78a9865584..6cc3604c295a 100644 --- a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml +++ b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml @@ -95,6 +95,17 @@ properties: array with the first word defined using the AST2600_WDT_RESET1_* macros, and the second word defined using the AST2600_WDT_RESET2_* macros. + aspeed,restart-sw: + $ref: /schemas/types.yaml#/definitions/flag + description: > + Normally, ASPEED WDT reset may occur when system hangs or reboot + triggered by SW consciously. However, system doesn't know whether the + restart is triggered by SW consciously since the reset event flag is + the same as normal WDT timeout reset. With this property, SW can + restart the system immediately and directly without wait for WDT + timeout occurs. The reset event flag is also different from the normal + WDT reset. This property is only supported since AST2600 platform. + required: - compatible - reg -- 2.34.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT SW reset @ 2024-10-07 6:34 ` Chin-Ting Kuo 0 siblings, 0 replies; 26+ messages in thread From: Chin-Ting Kuo @ 2024-10-07 6:34 UTC (permalink / raw) To: patrick, wim, linux, robh, krzk+dt, conor+dt, joel, andrew, linux-watchdog, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel Cc: Peter.Yin, Patrick_NC_Lin, Bonnie_Lo, DELPHINE_CHIU, BMC-SW Add "aspeed,restart-sw" property to distinguish normal WDT reset from system restart triggered by SW consciously. Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> --- .../bindings/watchdog/aspeed,ast2400-wdt.yaml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml index be78a9865584..6cc3604c295a 100644 --- a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml +++ b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml @@ -95,6 +95,17 @@ properties: array with the first word defined using the AST2600_WDT_RESET1_* macros, and the second word defined using the AST2600_WDT_RESET2_* macros. + aspeed,restart-sw: + $ref: /schemas/types.yaml#/definitions/flag + description: > + Normally, ASPEED WDT reset may occur when system hangs or reboot + triggered by SW consciously. However, system doesn't know whether the + restart is triggered by SW consciously since the reset event flag is + the same as normal WDT timeout reset. With this property, SW can + restart the system immediately and directly without wait for WDT + timeout occurs. The reset event flag is also different from the normal + WDT reset. This property is only supported since AST2600 platform. + required: - compatible - reg -- 2.34.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT SW reset 2024-10-07 6:34 ` Chin-Ting Kuo @ 2024-10-07 6:58 ` Krzysztof Kozlowski -1 siblings, 0 replies; 26+ messages in thread From: Krzysztof Kozlowski @ 2024-10-07 6:58 UTC (permalink / raw) To: linux-aspeed On 07/10/2024 08:34, Chin-Ting Kuo wrote: > Add "aspeed,restart-sw" property to distinguish normal WDT > reset from system restart triggered by SW consciously. > > Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> > --- > .../bindings/watchdog/aspeed,ast2400-wdt.yaml | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > index be78a9865584..6cc3604c295a 100644 > --- a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > +++ b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > @@ -95,6 +95,17 @@ properties: > array with the first word defined using the AST2600_WDT_RESET1_* macros, > and the second word defined using the AST2600_WDT_RESET2_* macros. > > + aspeed,restart-sw: > + $ref: /schemas/types.yaml#/definitions/flag > + description: > Why >? > + Normally, ASPEED WDT reset may occur when system hangs or reboot > + triggered by SW consciously. However, system doesn't know whether the > + restart is triggered by SW consciously since the reset event flag is > + the same as normal WDT timeout reset. With this property, SW can So DTS has this property and watchdog bites (timeout) but you will ignore it and claim that it was software choice? This does not make much sense to me, at least based on this explanation > + restart the system immediately and directly without wait for WDT > + timeout occurs. The reset event flag is also different from the normal > + WDT reset. This property is only supported since AST2600 platform. Supported as drivers? How is this related? Or you mean hardware? Then property should be restricted there. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT SW reset @ 2024-10-07 6:58 ` Krzysztof Kozlowski 0 siblings, 0 replies; 26+ messages in thread From: Krzysztof Kozlowski @ 2024-10-07 6:58 UTC (permalink / raw) To: Chin-Ting Kuo, patrick, wim, linux, robh, krzk+dt, conor+dt, joel, andrew, linux-watchdog, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel Cc: Peter.Yin, Patrick_NC_Lin, Bonnie_Lo, DELPHINE_CHIU, BMC-SW On 07/10/2024 08:34, Chin-Ting Kuo wrote: > Add "aspeed,restart-sw" property to distinguish normal WDT > reset from system restart triggered by SW consciously. > > Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> > --- > .../bindings/watchdog/aspeed,ast2400-wdt.yaml | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > index be78a9865584..6cc3604c295a 100644 > --- a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > +++ b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > @@ -95,6 +95,17 @@ properties: > array with the first word defined using the AST2600_WDT_RESET1_* macros, > and the second word defined using the AST2600_WDT_RESET2_* macros. > > + aspeed,restart-sw: > + $ref: /schemas/types.yaml#/definitions/flag > + description: > Why >? > + Normally, ASPEED WDT reset may occur when system hangs or reboot > + triggered by SW consciously. However, system doesn't know whether the > + restart is triggered by SW consciously since the reset event flag is > + the same as normal WDT timeout reset. With this property, SW can So DTS has this property and watchdog bites (timeout) but you will ignore it and claim that it was software choice? This does not make much sense to me, at least based on this explanation > + restart the system immediately and directly without wait for WDT > + timeout occurs. The reset event flag is also different from the normal > + WDT reset. This property is only supported since AST2600 platform. Supported as drivers? How is this related? Or you mean hardware? Then property should be restricted there. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT SW reset 2024-10-07 6:58 ` Krzysztof Kozlowski @ 2024-10-14 2:07 ` Chin-Ting Kuo -1 siblings, 0 replies; 26+ messages in thread From: Chin-Ting Kuo @ 2024-10-14 2:07 UTC (permalink / raw) To: linux-aspeed Hi Krzysztof, Thanks for the review. > -----Original Message----- > From: Krzysztof Kozlowski <krzk@kernel.org> > Sent: Monday, October 7, 2024 2:58 PM > Subject: Re: [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT > SW reset > > On 07/10/2024 08:34, Chin-Ting Kuo wrote: > > Add "aspeed,restart-sw" property to distinguish normal WDT reset from > > system restart triggered by SW consciously. > > > > Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> > > --- > > .../bindings/watchdog/aspeed,ast2400-wdt.yaml | 11 > +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > > b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > > index be78a9865584..6cc3604c295a 100644 > > --- > > a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > > +++ b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.ya > > +++ ml > > @@ -95,6 +95,17 @@ properties: > > array with the first word defined using the AST2600_WDT_RESET1_* > macros, > > and the second word defined using the AST2600_WDT_RESET2_* > macros. > > > > + aspeed,restart-sw: > > + $ref: /schemas/types.yaml#/definitions/flag > > + description: > > > Why >? > ">" will be removed in the next patch series and the description content will be concatenated after the colon, ":". > > + Normally, ASPEED WDT reset may occur when system hangs or > reboot > > + triggered by SW consciously. However, system doesn't know whether > the > > + restart is triggered by SW consciously since the reset event flag is > > + the same as normal WDT timeout reset. With this property, SW > > + can > > So DTS has this property and watchdog bites (timeout) but you will ignore it > and claim that it was software choice? > No. Normally, when WDT is enabled, a counter is also be enabled. When the counter is equal to an expected value, timeout event occurs. AST2600 hardware supports a SW mode, when a magic number is filled into a specific register, WDT reset is triggered immediately without controlling the counter and the counter is not counted. Thus, WDT timeout doesn't occur. > This does not make much sense to me, at least based on this explanation > > > + restart the system immediately and directly without wait for WDT > > + timeout occurs. The reset event flag is also different from the > normal > > + WDT reset. This property is only supported since AST2600 platform. > > Supported as drivers? How is this related? Or you mean hardware? Then > property should be restricted there. > It is a hardware supported function on AST2600. For platform compatibility, without this property, all behaviors are the same as the previous generation platform, AST2500. This property may be removed in the next patch series with referring to Rob suggestion in the other reply. After checking with the major users, it is feasible to remove this property and using SW reset by default when the restart operation is triggered by SW deliberately on AST2600 platform. > Best regards, > Krzysztof Best Wishes, Chin-Ting ^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT SW reset @ 2024-10-14 2:07 ` Chin-Ting Kuo 0 siblings, 0 replies; 26+ messages in thread From: Chin-Ting Kuo @ 2024-10-14 2:07 UTC (permalink / raw) To: Krzysztof Kozlowski, patrick@stwcx.xyz, wim@linux-watchdog.org, linux@roeck-us.net, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, joel@jms.id.au, andrew@codeconstruct.com.au, linux-watchdog@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org Cc: Peter.Yin@quantatw.com, Patrick_NC_Lin@wiwynn.com, Bonnie_Lo@wiwynn.com, DELPHINE_CHIU@wiwynn.com, BMC-SW, Aaron Lee Hi Krzysztof, Thanks for the review. > -----Original Message----- > From: Krzysztof Kozlowski <krzk@kernel.org> > Sent: Monday, October 7, 2024 2:58 PM > Subject: Re: [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT > SW reset > > On 07/10/2024 08:34, Chin-Ting Kuo wrote: > > Add "aspeed,restart-sw" property to distinguish normal WDT reset from > > system restart triggered by SW consciously. > > > > Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> > > --- > > .../bindings/watchdog/aspeed,ast2400-wdt.yaml | 11 > +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > > b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > > index be78a9865584..6cc3604c295a 100644 > > --- > > a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > > +++ b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.ya > > +++ ml > > @@ -95,6 +95,17 @@ properties: > > array with the first word defined using the AST2600_WDT_RESET1_* > macros, > > and the second word defined using the AST2600_WDT_RESET2_* > macros. > > > > + aspeed,restart-sw: > > + $ref: /schemas/types.yaml#/definitions/flag > > + description: > > > Why >? > ">" will be removed in the next patch series and the description content will be concatenated after the colon, ":". > > + Normally, ASPEED WDT reset may occur when system hangs or > reboot > > + triggered by SW consciously. However, system doesn't know whether > the > > + restart is triggered by SW consciously since the reset event flag is > > + the same as normal WDT timeout reset. With this property, SW > > + can > > So DTS has this property and watchdog bites (timeout) but you will ignore it > and claim that it was software choice? > No. Normally, when WDT is enabled, a counter is also be enabled. When the counter is equal to an expected value, timeout event occurs. AST2600 hardware supports a SW mode, when a magic number is filled into a specific register, WDT reset is triggered immediately without controlling the counter and the counter is not counted. Thus, WDT timeout doesn't occur. > This does not make much sense to me, at least based on this explanation > > > + restart the system immediately and directly without wait for WDT > > + timeout occurs. The reset event flag is also different from the > normal > > + WDT reset. This property is only supported since AST2600 platform. > > Supported as drivers? How is this related? Or you mean hardware? Then > property should be restricted there. > It is a hardware supported function on AST2600. For platform compatibility, without this property, all behaviors are the same as the previous generation platform, AST2500. This property may be removed in the next patch series with referring to Rob suggestion in the other reply. After checking with the major users, it is feasible to remove this property and using SW reset by default when the restart operation is triggered by SW deliberately on AST2600 platform. > Best regards, > Krzysztof Best Wishes, Chin-Ting ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT SW reset 2024-10-14 2:07 ` Chin-Ting Kuo @ 2024-10-14 6:53 ` Krzysztof Kozlowski -1 siblings, 0 replies; 26+ messages in thread From: Krzysztof Kozlowski @ 2024-10-14 6:53 UTC (permalink / raw) To: linux-aspeed On 14/10/2024 04:07, Chin-Ting Kuo wrote: > Hi Krzysztof, > > Thanks for the review. > >> -----Original Message----- >> From: Krzysztof Kozlowski <krzk@kernel.org> >> Sent: Monday, October 7, 2024 2:58 PM >> Subject: Re: [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT >> SW reset >> >> On 07/10/2024 08:34, Chin-Ting Kuo wrote: >>> Add "aspeed,restart-sw" property to distinguish normal WDT reset from >>> system restart triggered by SW consciously. >>> >>> Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> >>> --- >>> .../bindings/watchdog/aspeed,ast2400-wdt.yaml | 11 >> +++++++++++ >>> 1 file changed, 11 insertions(+) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml >>> b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml >>> index be78a9865584..6cc3604c295a 100644 >>> --- >>> a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml >>> +++ b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.ya >>> +++ ml >>> @@ -95,6 +95,17 @@ properties: >>> array with the first word defined using the AST2600_WDT_RESET1_* >> macros, >>> and the second word defined using the AST2600_WDT_RESET2_* >> macros. >>> >>> + aspeed,restart-sw: >>> + $ref: /schemas/types.yaml#/definitions/flag >>> + description: > >> >> Why >? >> > > ">" will be removed in the next patch series and the description content will be > concatenated after the colon, ":". > >>> + Normally, ASPEED WDT reset may occur when system hangs or >> reboot >>> + triggered by SW consciously. However, system doesn't know whether >> the >>> + restart is triggered by SW consciously since the reset event flag is >>> + the same as normal WDT timeout reset. With this property, SW >>> + can >> >> So DTS has this property and watchdog bites (timeout) but you will ignore it >> and claim that it was software choice? >> > > No. Normally, when WDT is enabled, a counter is also be enabled. When the counter > is equal to an expected value, timeout event occurs. AST2600 hardware supports a SW > mode, when a magic number is filled into a specific register, WDT reset is triggered > immediately without controlling the counter and the counter is not counted. > Thus, WDT timeout doesn't occur. How is this a no? > >> This does not make much sense to me, at least based on this explanation >> >>> + restart the system immediately and directly without wait for WDT >>> + timeout occurs. The reset event flag is also different from the >> normal >>> + WDT reset. This property is only supported since AST2600 platform. >> >> Supported as drivers? How is this related? Or you mean hardware? Then >> property should be restricted there. >> > > It is a hardware supported function on AST2600. For platform compatibility, without > this property, all behaviors are the same as the previous generation platform, AST2500. > > This property may be removed in the next patch series with referring to Rob suggestion s/may/will/ > in the other reply. After checking with the major users, it is feasible to remove this > property and using SW reset by default when the restart operation is triggered by SW > deliberately on AST2600 platform. > Best regards, Krzysztof ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT SW reset @ 2024-10-14 6:53 ` Krzysztof Kozlowski 0 siblings, 0 replies; 26+ messages in thread From: Krzysztof Kozlowski @ 2024-10-14 6:53 UTC (permalink / raw) To: Chin-Ting Kuo, patrick@stwcx.xyz, wim@linux-watchdog.org, linux@roeck-us.net, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, joel@jms.id.au, andrew@codeconstruct.com.au, linux-watchdog@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org Cc: Peter.Yin@quantatw.com, Patrick_NC_Lin@wiwynn.com, Bonnie_Lo@wiwynn.com, DELPHINE_CHIU@wiwynn.com, BMC-SW, Aaron Lee On 14/10/2024 04:07, Chin-Ting Kuo wrote: > Hi Krzysztof, > > Thanks for the review. > >> -----Original Message----- >> From: Krzysztof Kozlowski <krzk@kernel.org> >> Sent: Monday, October 7, 2024 2:58 PM >> Subject: Re: [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT >> SW reset >> >> On 07/10/2024 08:34, Chin-Ting Kuo wrote: >>> Add "aspeed,restart-sw" property to distinguish normal WDT reset from >>> system restart triggered by SW consciously. >>> >>> Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> >>> --- >>> .../bindings/watchdog/aspeed,ast2400-wdt.yaml | 11 >> +++++++++++ >>> 1 file changed, 11 insertions(+) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml >>> b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml >>> index be78a9865584..6cc3604c295a 100644 >>> --- >>> a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml >>> +++ b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.ya >>> +++ ml >>> @@ -95,6 +95,17 @@ properties: >>> array with the first word defined using the AST2600_WDT_RESET1_* >> macros, >>> and the second word defined using the AST2600_WDT_RESET2_* >> macros. >>> >>> + aspeed,restart-sw: >>> + $ref: /schemas/types.yaml#/definitions/flag >>> + description: > >> >> Why >? >> > > ">" will be removed in the next patch series and the description content will be > concatenated after the colon, ":". > >>> + Normally, ASPEED WDT reset may occur when system hangs or >> reboot >>> + triggered by SW consciously. However, system doesn't know whether >> the >>> + restart is triggered by SW consciously since the reset event flag is >>> + the same as normal WDT timeout reset. With this property, SW >>> + can >> >> So DTS has this property and watchdog bites (timeout) but you will ignore it >> and claim that it was software choice? >> > > No. Normally, when WDT is enabled, a counter is also be enabled. When the counter > is equal to an expected value, timeout event occurs. AST2600 hardware supports a SW > mode, when a magic number is filled into a specific register, WDT reset is triggered > immediately without controlling the counter and the counter is not counted. > Thus, WDT timeout doesn't occur. How is this a no? > >> This does not make much sense to me, at least based on this explanation >> >>> + restart the system immediately and directly without wait for WDT >>> + timeout occurs. The reset event flag is also different from the >> normal >>> + WDT reset. This property is only supported since AST2600 platform. >> >> Supported as drivers? How is this related? Or you mean hardware? Then >> property should be restricted there. >> > > It is a hardware supported function on AST2600. For platform compatibility, without > this property, all behaviors are the same as the previous generation platform, AST2500. > > This property may be removed in the next patch series with referring to Rob suggestion s/may/will/ > in the other reply. After checking with the major users, it is feasible to remove this > property and using SW reset by default when the restart operation is triggered by SW > deliberately on AST2600 platform. > Best regards, Krzysztof ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT SW reset 2024-10-14 6:53 ` Krzysztof Kozlowski @ 2024-10-14 9:58 ` Chin-Ting Kuo -1 siblings, 0 replies; 26+ messages in thread From: Chin-Ting Kuo @ 2024-10-14 9:58 UTC (permalink / raw) To: linux-aspeed Hi Krzysztof, > -----Original Message----- > From: Krzysztof Kozlowski <krzk@kernel.org> > Sent: Monday, October 14, 2024 2:53 PM > Subject: Re: [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT > SW reset > > On 14/10/2024 04:07, Chin-Ting Kuo wrote: > > Hi Krzysztof, > > > > Thanks for the review. > > > >> -----Original Message----- > >> From: Krzysztof Kozlowski <krzk@kernel.org> > >> Sent: Monday, October 7, 2024 2:58 PM > >> Subject: Re: [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property > >> for WDT SW reset > >> > >> On 07/10/2024 08:34, Chin-Ting Kuo wrote: > >>> Add "aspeed,restart-sw" property to distinguish normal WDT reset > >>> from system restart triggered by SW consciously. > >>> > >>> Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> > >>> --- > >>> .../bindings/watchdog/aspeed,ast2400-wdt.yaml | 11 > >> +++++++++++ > >>> 1 file changed, 11 insertions(+) > >>> > >>> diff --git > >>> > a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > >>> > b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > >>> index be78a9865584..6cc3604c295a 100644 > >>> --- > >>> > a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > >>> +++ b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt. > >>> +++ ya > >>> +++ ml > >>> @@ -95,6 +95,17 @@ properties: > >>> array with the first word defined using the > >>> AST2600_WDT_RESET1_* > >> macros, > >>> and the second word defined using the AST2600_WDT_RESET2_* > >> macros. > >>> > >>> + aspeed,restart-sw: > >>> + $ref: /schemas/types.yaml#/definitions/flag > >>> + description: > > >> > >> Why >? > >> > > > > ">" will be removed in the next patch series and the description > > content will be concatenated after the colon, ":". > > > >>> + Normally, ASPEED WDT reset may occur when system hangs or > >> reboot > >>> + triggered by SW consciously. However, system doesn't know > >>> + whether > >> the > >>> + restart is triggered by SW consciously since the reset event flag is > >>> + the same as normal WDT timeout reset. With this property, SW > >>> + can > >> > >> So DTS has this property and watchdog bites (timeout) but you will > >> ignore it and claim that it was software choice? > >> > > > > No. Normally, when WDT is enabled, a counter is also be enabled. When > > the counter is equal to an expected value, timeout event occurs. > > AST2600 hardware supports a SW mode, when a magic number is filled > > into a specific register, WDT reset is triggered immediately without > controlling the counter and the counter is not counted. > > Thus, WDT timeout doesn't occur. > > How is this a no? > It is used to emphasize that the driver doesn?t ignore the timeout event because the counter is not counted when SW mode is used and thus, no timeout occurs. > > > >> This does not make much sense to me, at least based on this > >> explanation > >> > >>> + restart the system immediately and directly without wait for WDT > >>> + timeout occurs. The reset event flag is also different from > >>> + the > >> normal > >>> + WDT reset. This property is only supported since AST2600 > platform. > >> > >> Supported as drivers? How is this related? Or you mean hardware? Then > >> property should be restricted there. > >> > > > > It is a hardware supported function on AST2600. For platform > > compatibility, without this property, all behaviors are the same as the > previous generation platform, AST2500. > > > > This property may be removed in the next patch series with referring > > to Rob suggestion > > s/may/will/ > This property will be removed in the next patch series. > > in the other reply. After checking with the major users, it is > > feasible to remove this property and using SW reset by default when > > the restart operation is triggered by SW deliberately on AST2600 platform. > > > > Best Wishes, Chin-Ting ^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT SW reset @ 2024-10-14 9:58 ` Chin-Ting Kuo 0 siblings, 0 replies; 26+ messages in thread From: Chin-Ting Kuo @ 2024-10-14 9:58 UTC (permalink / raw) To: Krzysztof Kozlowski, patrick@stwcx.xyz, wim@linux-watchdog.org, linux@roeck-us.net, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, joel@jms.id.au, andrew@codeconstruct.com.au, linux-watchdog@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org Cc: Peter.Yin@quantatw.com, Patrick_NC_Lin@wiwynn.com, Bonnie_Lo@wiwynn.com, DELPHINE_CHIU@wiwynn.com, BMC-SW, Aaron Lee Hi Krzysztof, > -----Original Message----- > From: Krzysztof Kozlowski <krzk@kernel.org> > Sent: Monday, October 14, 2024 2:53 PM > Subject: Re: [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT > SW reset > > On 14/10/2024 04:07, Chin-Ting Kuo wrote: > > Hi Krzysztof, > > > > Thanks for the review. > > > >> -----Original Message----- > >> From: Krzysztof Kozlowski <krzk@kernel.org> > >> Sent: Monday, October 7, 2024 2:58 PM > >> Subject: Re: [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property > >> for WDT SW reset > >> > >> On 07/10/2024 08:34, Chin-Ting Kuo wrote: > >>> Add "aspeed,restart-sw" property to distinguish normal WDT reset > >>> from system restart triggered by SW consciously. > >>> > >>> Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> > >>> --- > >>> .../bindings/watchdog/aspeed,ast2400-wdt.yaml | 11 > >> +++++++++++ > >>> 1 file changed, 11 insertions(+) > >>> > >>> diff --git > >>> > a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > >>> > b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > >>> index be78a9865584..6cc3604c295a 100644 > >>> --- > >>> > a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > >>> +++ b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt. > >>> +++ ya > >>> +++ ml > >>> @@ -95,6 +95,17 @@ properties: > >>> array with the first word defined using the > >>> AST2600_WDT_RESET1_* > >> macros, > >>> and the second word defined using the AST2600_WDT_RESET2_* > >> macros. > >>> > >>> + aspeed,restart-sw: > >>> + $ref: /schemas/types.yaml#/definitions/flag > >>> + description: > > >> > >> Why >? > >> > > > > ">" will be removed in the next patch series and the description > > content will be concatenated after the colon, ":". > > > >>> + Normally, ASPEED WDT reset may occur when system hangs or > >> reboot > >>> + triggered by SW consciously. However, system doesn't know > >>> + whether > >> the > >>> + restart is triggered by SW consciously since the reset event flag is > >>> + the same as normal WDT timeout reset. With this property, SW > >>> + can > >> > >> So DTS has this property and watchdog bites (timeout) but you will > >> ignore it and claim that it was software choice? > >> > > > > No. Normally, when WDT is enabled, a counter is also be enabled. When > > the counter is equal to an expected value, timeout event occurs. > > AST2600 hardware supports a SW mode, when a magic number is filled > > into a specific register, WDT reset is triggered immediately without > controlling the counter and the counter is not counted. > > Thus, WDT timeout doesn't occur. > > How is this a no? > It is used to emphasize that the driver doesn’t ignore the timeout event because the counter is not counted when SW mode is used and thus, no timeout occurs. > > > >> This does not make much sense to me, at least based on this > >> explanation > >> > >>> + restart the system immediately and directly without wait for WDT > >>> + timeout occurs. The reset event flag is also different from > >>> + the > >> normal > >>> + WDT reset. This property is only supported since AST2600 > platform. > >> > >> Supported as drivers? How is this related? Or you mean hardware? Then > >> property should be restricted there. > >> > > > > It is a hardware supported function on AST2600. For platform > > compatibility, without this property, all behaviors are the same as the > previous generation platform, AST2500. > > > > This property may be removed in the next patch series with referring > > to Rob suggestion > > s/may/will/ > This property will be removed in the next patch series. > > in the other reply. After checking with the major users, it is > > feasible to remove this property and using SW reset by default when > > the restart operation is triggered by SW deliberately on AST2600 platform. > > > > Best Wishes, Chin-Ting ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT SW reset 2024-10-07 6:34 ` Chin-Ting Kuo @ 2024-10-07 17:59 ` Rob Herring -1 siblings, 0 replies; 26+ messages in thread From: Rob Herring @ 2024-10-07 17:59 UTC (permalink / raw) To: linux-aspeed On Mon, Oct 07, 2024 at 02:34:05PM +0800, Chin-Ting Kuo wrote: > Add "aspeed,restart-sw" property to distinguish normal WDT > reset from system restart triggered by SW consciously. > > Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> > --- > .../bindings/watchdog/aspeed,ast2400-wdt.yaml | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > index be78a9865584..6cc3604c295a 100644 > --- a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > +++ b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > @@ -95,6 +95,17 @@ properties: > array with the first word defined using the AST2600_WDT_RESET1_* macros, > and the second word defined using the AST2600_WDT_RESET2_* macros. > > + aspeed,restart-sw: > + $ref: /schemas/types.yaml#/definitions/flag > + description: > > + Normally, ASPEED WDT reset may occur when system hangs or reboot > + triggered by SW consciously. However, system doesn't know whether the > + restart is triggered by SW consciously since the reset event flag is > + the same as normal WDT timeout reset. With this property, SW can > + restart the system immediately and directly without wait for WDT > + timeout occurs. The reset event flag is also different from the normal > + WDT reset. This property is only supported since AST2600 platform. Why can't this be implicit based on the ast2600 compatible string? Rob ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT SW reset @ 2024-10-07 17:59 ` Rob Herring 0 siblings, 0 replies; 26+ messages in thread From: Rob Herring @ 2024-10-07 17:59 UTC (permalink / raw) To: Chin-Ting Kuo Cc: patrick, wim, linux, krzk+dt, conor+dt, joel, andrew, linux-watchdog, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, Peter.Yin, Patrick_NC_Lin, Bonnie_Lo, DELPHINE_CHIU, BMC-SW On Mon, Oct 07, 2024 at 02:34:05PM +0800, Chin-Ting Kuo wrote: > Add "aspeed,restart-sw" property to distinguish normal WDT > reset from system restart triggered by SW consciously. > > Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> > --- > .../bindings/watchdog/aspeed,ast2400-wdt.yaml | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > index be78a9865584..6cc3604c295a 100644 > --- a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > +++ b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > @@ -95,6 +95,17 @@ properties: > array with the first word defined using the AST2600_WDT_RESET1_* macros, > and the second word defined using the AST2600_WDT_RESET2_* macros. > > + aspeed,restart-sw: > + $ref: /schemas/types.yaml#/definitions/flag > + description: > > + Normally, ASPEED WDT reset may occur when system hangs or reboot > + triggered by SW consciously. However, system doesn't know whether the > + restart is triggered by SW consciously since the reset event flag is > + the same as normal WDT timeout reset. With this property, SW can > + restart the system immediately and directly without wait for WDT > + timeout occurs. The reset event flag is also different from the normal > + WDT reset. This property is only supported since AST2600 platform. Why can't this be implicit based on the ast2600 compatible string? Rob ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT SW reset 2024-10-07 17:59 ` Rob Herring @ 2024-10-07 19:54 ` Guenter Roeck -1 siblings, 0 replies; 26+ messages in thread From: Guenter Roeck @ 2024-10-07 19:54 UTC (permalink / raw) To: linux-aspeed On 10/7/24 10:59, Rob Herring wrote: > On Mon, Oct 07, 2024 at 02:34:05PM +0800, Chin-Ting Kuo wrote: >> Add "aspeed,restart-sw" property to distinguish normal WDT >> reset from system restart triggered by SW consciously. >> >> Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> >> --- >> .../bindings/watchdog/aspeed,ast2400-wdt.yaml | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml >> index be78a9865584..6cc3604c295a 100644 >> --- a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml >> +++ b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml >> @@ -95,6 +95,17 @@ properties: >> array with the first word defined using the AST2600_WDT_RESET1_* macros, >> and the second word defined using the AST2600_WDT_RESET2_* macros. >> >> + aspeed,restart-sw: >> + $ref: /schemas/types.yaml#/definitions/flag >> + description: > >> + Normally, ASPEED WDT reset may occur when system hangs or reboot >> + triggered by SW consciously. However, system doesn't know whether the >> + restart is triggered by SW consciously since the reset event flag is >> + the same as normal WDT timeout reset. With this property, SW can >> + restart the system immediately and directly without wait for WDT >> + timeout occurs. The reset event flag is also different from the normal >> + WDT reset. This property is only supported since AST2600 platform. > > Why can't this be implicit based on the ast2600 compatible string? > Same question here. Guenter ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT SW reset @ 2024-10-07 19:54 ` Guenter Roeck 0 siblings, 0 replies; 26+ messages in thread From: Guenter Roeck @ 2024-10-07 19:54 UTC (permalink / raw) To: Rob Herring, Chin-Ting Kuo Cc: patrick, wim, krzk+dt, conor+dt, joel, andrew, linux-watchdog, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel, Peter.Yin, Patrick_NC_Lin, Bonnie_Lo, DELPHINE_CHIU, BMC-SW On 10/7/24 10:59, Rob Herring wrote: > On Mon, Oct 07, 2024 at 02:34:05PM +0800, Chin-Ting Kuo wrote: >> Add "aspeed,restart-sw" property to distinguish normal WDT >> reset from system restart triggered by SW consciously. >> >> Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> >> --- >> .../bindings/watchdog/aspeed,ast2400-wdt.yaml | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml >> index be78a9865584..6cc3604c295a 100644 >> --- a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml >> +++ b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml >> @@ -95,6 +95,17 @@ properties: >> array with the first word defined using the AST2600_WDT_RESET1_* macros, >> and the second word defined using the AST2600_WDT_RESET2_* macros. >> >> + aspeed,restart-sw: >> + $ref: /schemas/types.yaml#/definitions/flag >> + description: > >> + Normally, ASPEED WDT reset may occur when system hangs or reboot >> + triggered by SW consciously. However, system doesn't know whether the >> + restart is triggered by SW consciously since the reset event flag is >> + the same as normal WDT timeout reset. With this property, SW can >> + restart the system immediately and directly without wait for WDT >> + timeout occurs. The reset event flag is also different from the normal >> + WDT reset. This property is only supported since AST2600 platform. > > Why can't this be implicit based on the ast2600 compatible string? > Same question here. Guenter ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT SW reset 2024-10-07 19:54 ` Guenter Roeck @ 2024-10-14 2:08 ` Chin-Ting Kuo -1 siblings, 0 replies; 26+ messages in thread From: Chin-Ting Kuo @ 2024-10-14 2:08 UTC (permalink / raw) To: linux-aspeed Hi Guenter, Thanks for the review. > -----Original Message----- > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck > Sent: Tuesday, October 8, 2024 3:55 AM > Subject: Re: [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT > SW reset > > On 10/7/24 10:59, Rob Herring wrote: > > On Mon, Oct 07, 2024 at 02:34:05PM +0800, Chin-Ting Kuo wrote: > >> Add "aspeed,restart-sw" property to distinguish normal WDT reset from > >> system restart triggered by SW consciously. > >> > >> Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> > >> --- > >> .../bindings/watchdog/aspeed,ast2400-wdt.yaml | 11 > +++++++++++ > >> 1 file changed, 11 insertions(+) > >> > >> diff --git > >> a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > >> b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > >> index be78a9865584..6cc3604c295a 100644 > >> --- > >> a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > >> +++ b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.y > >> +++ aml > >> @@ -95,6 +95,17 @@ properties: > >> array with the first word defined using the > AST2600_WDT_RESET1_* macros, > >> and the second word defined using the AST2600_WDT_RESET2_* > macros. > >> > >> + aspeed,restart-sw: > >> + $ref: /schemas/types.yaml#/definitions/flag > >> + description: > > >> + Normally, ASPEED WDT reset may occur when system hangs or > reboot > >> + triggered by SW consciously. However, system doesn't know > whether the > >> + restart is triggered by SW consciously since the reset event flag is > >> + the same as normal WDT timeout reset. With this property, SW can > >> + restart the system immediately and directly without wait for WDT > >> + timeout occurs. The reset event flag is also different from the > normal > >> + WDT reset. This property is only supported since AST2600 platform. > > > > Why can't this be implicit based on the ast2600 compatible string? > > > > Same question here. > Yes, this property will be implicit based on the ast2600 compatible string in the next patch series. > Guenter Chin-Ting ^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT SW reset @ 2024-10-14 2:08 ` Chin-Ting Kuo 0 siblings, 0 replies; 26+ messages in thread From: Chin-Ting Kuo @ 2024-10-14 2:08 UTC (permalink / raw) To: Guenter Roeck, Rob Herring Cc: patrick@stwcx.xyz, wim@linux-watchdog.org, krzk+dt@kernel.org, conor+dt@kernel.org, joel@jms.id.au, andrew@codeconstruct.com.au, linux-watchdog@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org, Peter.Yin@quantatw.com, Patrick_NC_Lin@wiwynn.com, Bonnie_Lo@wiwynn.com, DELPHINE_CHIU@wiwynn.com, BMC-SW Hi Guenter, Thanks for the review. > -----Original Message----- > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck > Sent: Tuesday, October 8, 2024 3:55 AM > Subject: Re: [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT > SW reset > > On 10/7/24 10:59, Rob Herring wrote: > > On Mon, Oct 07, 2024 at 02:34:05PM +0800, Chin-Ting Kuo wrote: > >> Add "aspeed,restart-sw" property to distinguish normal WDT reset from > >> system restart triggered by SW consciously. > >> > >> Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> > >> --- > >> .../bindings/watchdog/aspeed,ast2400-wdt.yaml | 11 > +++++++++++ > >> 1 file changed, 11 insertions(+) > >> > >> diff --git > >> a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > >> b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > >> index be78a9865584..6cc3604c295a 100644 > >> --- > >> a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > >> +++ b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.y > >> +++ aml > >> @@ -95,6 +95,17 @@ properties: > >> array with the first word defined using the > AST2600_WDT_RESET1_* macros, > >> and the second word defined using the AST2600_WDT_RESET2_* > macros. > >> > >> + aspeed,restart-sw: > >> + $ref: /schemas/types.yaml#/definitions/flag > >> + description: > > >> + Normally, ASPEED WDT reset may occur when system hangs or > reboot > >> + triggered by SW consciously. However, system doesn't know > whether the > >> + restart is triggered by SW consciously since the reset event flag is > >> + the same as normal WDT timeout reset. With this property, SW can > >> + restart the system immediately and directly without wait for WDT > >> + timeout occurs. The reset event flag is also different from the > normal > >> + WDT reset. This property is only supported since AST2600 platform. > > > > Why can't this be implicit based on the ast2600 compatible string? > > > > Same question here. > Yes, this property will be implicit based on the ast2600 compatible string in the next patch series. > Guenter Chin-Ting ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT SW reset 2024-10-07 17:59 ` Rob Herring @ 2024-10-14 2:07 ` Chin-Ting Kuo -1 siblings, 0 replies; 26+ messages in thread From: Chin-Ting Kuo @ 2024-10-14 2:07 UTC (permalink / raw) To: linux-aspeed Hi Rob, Thanks for the review. > -----Original Message----- > From: Rob Herring <robh@kernel.org> > Sent: Tuesday, October 8, 2024 2:00 AM > Subject: Re: [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT > SW reset > > On Mon, Oct 07, 2024 at 02:34:05PM +0800, Chin-Ting Kuo wrote: > > Add "aspeed,restart-sw" property to distinguish normal WDT reset from > > system restart triggered by SW consciously. > > > > Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> > > --- > > .../bindings/watchdog/aspeed,ast2400-wdt.yaml | 11 > +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > > b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > > index be78a9865584..6cc3604c295a 100644 > > --- > > a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > > +++ b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.ya > > +++ ml > > @@ -95,6 +95,17 @@ properties: > > array with the first word defined using the AST2600_WDT_RESET1_* > macros, > > and the second word defined using the AST2600_WDT_RESET2_* > macros. > > > > + aspeed,restart-sw: > > + $ref: /schemas/types.yaml#/definitions/flag > > + description: > > > + Normally, ASPEED WDT reset may occur when system hangs or > reboot > > + triggered by SW consciously. However, system doesn't know whether > the > > + restart is triggered by SW consciously since the reset event flag is > > + the same as normal WDT timeout reset. With this property, SW can > > + restart the system immediately and directly without wait for WDT > > + timeout occurs. The reset event flag is also different from the > normal > > + WDT reset. This property is only supported since AST2600 platform. > > Why can't this be implicit based on the ast2600 compatible string? > Yes, this property will be implicit based on the ast2600 compatible string in the next patch series. > Rob Chin-Ting ^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT SW reset @ 2024-10-14 2:07 ` Chin-Ting Kuo 0 siblings, 0 replies; 26+ messages in thread From: Chin-Ting Kuo @ 2024-10-14 2:07 UTC (permalink / raw) To: Rob Herring Cc: patrick@stwcx.xyz, wim@linux-watchdog.org, linux@roeck-us.net, krzk+dt@kernel.org, conor+dt@kernel.org, joel@jms.id.au, andrew@codeconstruct.com.au, linux-watchdog@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org, Peter.Yin@quantatw.com, Patrick_NC_Lin@wiwynn.com, Bonnie_Lo@wiwynn.com, DELPHINE_CHIU@wiwynn.com, BMC-SW Hi Rob, Thanks for the review. > -----Original Message----- > From: Rob Herring <robh@kernel.org> > Sent: Tuesday, October 8, 2024 2:00 AM > Subject: Re: [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT > SW reset > > On Mon, Oct 07, 2024 at 02:34:05PM +0800, Chin-Ting Kuo wrote: > > Add "aspeed,restart-sw" property to distinguish normal WDT reset from > > system restart triggered by SW consciously. > > > > Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> > > --- > > .../bindings/watchdog/aspeed,ast2400-wdt.yaml | 11 > +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > > b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > > index be78a9865584..6cc3604c295a 100644 > > --- > > a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml > > +++ b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.ya > > +++ ml > > @@ -95,6 +95,17 @@ properties: > > array with the first word defined using the AST2600_WDT_RESET1_* > macros, > > and the second word defined using the AST2600_WDT_RESET2_* > macros. > > > > + aspeed,restart-sw: > > + $ref: /schemas/types.yaml#/definitions/flag > > + description: > > > + Normally, ASPEED WDT reset may occur when system hangs or > reboot > > + triggered by SW consciously. However, system doesn't know whether > the > > + restart is triggered by SW consciously since the reset event flag is > > + the same as normal WDT timeout reset. With this property, SW can > > + restart the system immediately and directly without wait for WDT > > + timeout occurs. The reset event flag is also different from the > normal > > + WDT reset. This property is only supported since AST2600 platform. > > Why can't this be implicit based on the ast2600 compatible string? > Yes, this property will be implicit based on the ast2600 compatible string in the next patch series. > Rob Chin-Ting ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 2/4] ARM: dts: aspeed: Add WDT controller into alias field 2024-10-07 6:34 ` Chin-Ting Kuo @ 2024-10-07 6:34 ` Chin-Ting Kuo -1 siblings, 0 replies; 26+ messages in thread From: Chin-Ting Kuo @ 2024-10-07 6:34 UTC (permalink / raw) To: linux-aspeed Add WDT controller into alias field. After that, WDT index, used to distinguish different WDT controllers in the driver, can be gotten by using of_alias_get_id dts API. Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> --- arch/arm/boot/dts/aspeed/aspeed-g4.dtsi | 2 ++ arch/arm/boot/dts/aspeed/aspeed-g5.dtsi | 3 +++ arch/arm/boot/dts/aspeed/aspeed-g6.dtsi | 4 ++++ 3 files changed, 9 insertions(+) diff --git a/arch/arm/boot/dts/aspeed/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed/aspeed-g4.dtsi index 78c967812492..d8b4136d0ca0 100644 --- a/arch/arm/boot/dts/aspeed/aspeed-g4.dtsi +++ b/arch/arm/boot/dts/aspeed/aspeed-g4.dtsi @@ -29,6 +29,8 @@ aliases { serial3 = &uart4; serial4 = &uart5; serial5 = &vuart; + watchdog0 = &wdt1; + watchdog1 = &wdt2; }; cpus { diff --git a/arch/arm/boot/dts/aspeed/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed/aspeed-g5.dtsi index 57a699a7c149..4dd220bca617 100644 --- a/arch/arm/boot/dts/aspeed/aspeed-g5.dtsi +++ b/arch/arm/boot/dts/aspeed/aspeed-g5.dtsi @@ -30,6 +30,9 @@ aliases { serial3 = &uart4; serial4 = &uart5; serial5 = &vuart; + watchdog0 = &wdt1; + watchdog1 = &wdt2; + watchdog2 = &wdt3; }; cpus { diff --git a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi index 8ed715bd53aa..c0a47c795fff 100644 --- a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi +++ b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi @@ -40,6 +40,10 @@ aliases { mdio1 = &mdio1; mdio2 = &mdio2; mdio3 = &mdio3; + watchdog0 = &wdt1; + watchdog1 = &wdt2; + watchdog2 = &wdt3; + watchdog3 = &wdt4; }; -- 2.34.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 2/4] ARM: dts: aspeed: Add WDT controller into alias field @ 2024-10-07 6:34 ` Chin-Ting Kuo 0 siblings, 0 replies; 26+ messages in thread From: Chin-Ting Kuo @ 2024-10-07 6:34 UTC (permalink / raw) To: patrick, wim, linux, robh, krzk+dt, conor+dt, joel, andrew, linux-watchdog, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel Cc: Peter.Yin, Patrick_NC_Lin, Bonnie_Lo, DELPHINE_CHIU, BMC-SW Add WDT controller into alias field. After that, WDT index, used to distinguish different WDT controllers in the driver, can be gotten by using of_alias_get_id dts API. Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> --- arch/arm/boot/dts/aspeed/aspeed-g4.dtsi | 2 ++ arch/arm/boot/dts/aspeed/aspeed-g5.dtsi | 3 +++ arch/arm/boot/dts/aspeed/aspeed-g6.dtsi | 4 ++++ 3 files changed, 9 insertions(+) diff --git a/arch/arm/boot/dts/aspeed/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed/aspeed-g4.dtsi index 78c967812492..d8b4136d0ca0 100644 --- a/arch/arm/boot/dts/aspeed/aspeed-g4.dtsi +++ b/arch/arm/boot/dts/aspeed/aspeed-g4.dtsi @@ -29,6 +29,8 @@ aliases { serial3 = &uart4; serial4 = &uart5; serial5 = &vuart; + watchdog0 = &wdt1; + watchdog1 = &wdt2; }; cpus { diff --git a/arch/arm/boot/dts/aspeed/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed/aspeed-g5.dtsi index 57a699a7c149..4dd220bca617 100644 --- a/arch/arm/boot/dts/aspeed/aspeed-g5.dtsi +++ b/arch/arm/boot/dts/aspeed/aspeed-g5.dtsi @@ -30,6 +30,9 @@ aliases { serial3 = &uart4; serial4 = &uart5; serial5 = &vuart; + watchdog0 = &wdt1; + watchdog1 = &wdt2; + watchdog2 = &wdt3; }; cpus { diff --git a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi index 8ed715bd53aa..c0a47c795fff 100644 --- a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi +++ b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi @@ -40,6 +40,10 @@ aliases { mdio1 = &mdio1; mdio2 = &mdio2; mdio3 = &mdio3; + watchdog0 = &wdt1; + watchdog1 = &wdt2; + watchdog2 = &wdt3; + watchdog3 = &wdt4; }; -- 2.34.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 3/4] watchdog: aspeed: Update bootstatus handling 2024-10-07 6:34 ` Chin-Ting Kuo @ 2024-10-07 6:34 ` Chin-Ting Kuo -1 siblings, 0 replies; 26+ messages in thread From: Chin-Ting Kuo @ 2024-10-07 6:34 UTC (permalink / raw) To: linux-aspeed Update the bootstatus according to the latest design guide from the OpenBMC shown as below. https://github.com/openbmc/docs/blob/master/designs/bmc-reboot-cause-update.md#proposed-design In short, - WDIOF_EXTERN1 => system is reset by Software - WDIOF_CARDRESET => system is reset by WDT - Others => other reset events, e.g., power on reset. On AST2400 platform, only a bit, SCU3C[1], represents that the system is reset by WDT1 or WDT2. On AST2500 platform, SCU3C[4:2] are WDT reset flags. SCU3C[4]: system is reset by WDT3. SCU3C[3]: system is reset by WDT2. SCU3C[2]: system is reset by WDT1. On AST2600 platform, SCU074[31:16] are WDT reset flags. SCU074[31:28]: system is reset by WDT4 SCU074[31]: system is reset by WDT4 software reset. SCU074[27:24]: system is reset by WDT3 SCU074[27]: system is reset by WDT3 software reset. SCU074[23:20]: system is reset by WDT2 SCU074[23]: system is reset by WDT2 software reset. SCU074[19:16]: system is reset by WDT1 SCU074[19]: system is reset by WDT1 software reset. Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> --- drivers/watchdog/aspeed_wdt.c | 109 +++++++++++++++++++++++++++++++--- 1 file changed, 101 insertions(+), 8 deletions(-) diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c index b4773a6aaf8c..68eaada8a564 100644 --- a/drivers/watchdog/aspeed_wdt.c +++ b/drivers/watchdog/aspeed_wdt.c @@ -11,10 +11,12 @@ #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; @@ -22,15 +24,41 @@ module_param(nowayout, bool, 0); MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); + +/* AST SCU Register for System Reset Event Log Register Set + * ast2600 is scu074 ast2400/2500 is scu03c + */ +#define AST2400_SCU_SYS_RESET_STATUS 0x3c +#define AST2400_SCU_SYS_RESET_WDT_MASK 0x1 +#define AST2400_SCU_SYS_RESET_WDT_MASK_SHIFT 1 + +#define AST2500_SCU_SYS_RESET_WDT_MASK 0x1 +#define AST2500_SCU_SYS_RESET_WDT_MASK_SHIFT 2 + +#define AST2600_SCU_SYS_RESET_STATUS 0x74 +#define AST2600_SCU_SYS_RESET_WDT_MASK 0xf +#define AST2600_SCU_SYS_RESET_WDT_SW_MASK 0x8 +#define AST2600_SCU_SYS_RESET_WDT_MASK_SHIFT 16 + +struct aspeed_wdt_scu { + const char *compatible; + u32 reset_status_reg; + u32 wdt_reset_mask; + u32 wdt_sw_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 { struct watchdog_device wdd; void __iomem *base; + int idx; u32 ctrl; const struct aspeed_wdt_config *cfg; }; @@ -39,18 +67,39 @@ 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 = AST2400_SCU_SYS_RESET_STATUS, + .wdt_reset_mask = AST2400_SCU_SYS_RESET_WDT_MASK, + .wdt_sw_reset_mask = 0, + .wdt_reset_mask_shift = AST2400_SCU_SYS_RESET_WDT_MASK_SHIFT, + }, }; 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 = AST2400_SCU_SYS_RESET_STATUS, + .wdt_reset_mask = AST2500_SCU_SYS_RESET_WDT_MASK, + .wdt_sw_reset_mask = 0, + .wdt_reset_mask_shift = AST2500_SCU_SYS_RESET_WDT_MASK_SHIFT, + }, }; 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 = AST2600_SCU_SYS_RESET_STATUS, + .wdt_reset_mask = AST2600_SCU_SYS_RESET_WDT_MASK, + .wdt_sw_reset_mask = AST2600_SCU_SYS_RESET_WDT_SW_MASK, + .wdt_reset_mask_shift = AST2600_SCU_SYS_RESET_WDT_MASK_SHIFT, + }, }; static const struct of_device_id aspeed_wdt_of_table[] = { @@ -213,6 +262,52 @@ static int aspeed_wdt_restart(struct watchdog_device *wdd, return 0; } +static int aspeed_wdt_get_bootstatus(struct device *dev, + struct aspeed_wdt *wdt) +{ + struct device_node *np = dev->of_node; + struct aspeed_wdt_scu scu = wdt->cfg->scu; + struct regmap *scu_base; + u32 reset_mask_width; + u32 reset_mask_shift; + u32 status; + int ret; + + wdt->idx = of_alias_get_id(np, "watchdog"); + if (wdt->idx < 0) + wdt->idx = 0; + + scu_base = syscon_regmap_lookup_by_compatible(scu.compatible); + if (IS_ERR(scu_base)) + return PTR_ERR(scu_base); + + ret = regmap_read(scu_base, scu.reset_status_reg, &status); + if (ret) + return ret; + + /* On AST2400, only a bit used to represent WDT reset */ + if (of_device_is_compatible(np, "aspeed,ast2400-wdt")) + wdt->idx = 0; + + reset_mask_width = hweight32(scu.wdt_reset_mask); + reset_mask_shift = scu.wdt_reset_mask_shift + + reset_mask_width * wdt->idx; + + if (status & (scu.wdt_sw_reset_mask << reset_mask_shift)) + wdt->wdd.bootstatus = WDIOF_EXTERN1; + else if (status & (scu.wdt_reset_mask << reset_mask_shift)) + wdt->wdd.bootstatus = WDIOF_CARDRESET; + else + wdt->wdd.bootstatus = WDIOF_UNKNOWN; + + ret = regmap_write(scu_base, scu.reset_status_reg, + scu.wdt_reset_mask << reset_mask_shift); + if (ret) + return ret; + + return 0; +} + /* 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) @@ -312,7 +407,6 @@ static int aspeed_wdt_probe(struct platform_device *pdev) struct device_node *np; const char *reset_type; u32 duration; - u32 status; int ret; wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL); @@ -458,14 +552,13 @@ static int aspeed_wdt_probe(struct platform_device *pdev) writel(duration - 1, wdt->base + WDT_RESET_WIDTH); } - status = readl(wdt->base + WDT_TIMEOUT_STATUS); - if (status & WDT_TIMEOUT_STATUS_BOOT_SECONDARY) { - wdt->wdd.bootstatus = WDIOF_CARDRESET; + ret = aspeed_wdt_get_bootstatus(dev, wdt); + if (ret) + return ret; - if (of_device_is_compatible(np, "aspeed,ast2400-wdt") || - of_device_is_compatible(np, "aspeed,ast2500-wdt")) - wdt->wdd.groups = bswitch_groups; - } + if (of_device_is_compatible(np, "aspeed,ast2400-wdt") || + of_device_is_compatible(np, "aspeed,ast2500-wdt")) + wdt->wdd.groups = bswitch_groups; dev_set_drvdata(dev, wdt); -- 2.34.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 3/4] watchdog: aspeed: Update bootstatus handling @ 2024-10-07 6:34 ` Chin-Ting Kuo 0 siblings, 0 replies; 26+ messages in thread From: Chin-Ting Kuo @ 2024-10-07 6:34 UTC (permalink / raw) To: patrick, wim, linux, robh, krzk+dt, conor+dt, joel, andrew, linux-watchdog, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel Cc: Peter.Yin, Patrick_NC_Lin, Bonnie_Lo, DELPHINE_CHIU, BMC-SW Update the bootstatus according to the latest design guide from the OpenBMC shown as below. https://github.com/openbmc/docs/blob/master/designs/bmc-reboot-cause-update.md#proposed-design In short, - WDIOF_EXTERN1 => system is reset by Software - WDIOF_CARDRESET => system is reset by WDT - Others => other reset events, e.g., power on reset. On AST2400 platform, only a bit, SCU3C[1], represents that the system is reset by WDT1 or WDT2. On AST2500 platform, SCU3C[4:2] are WDT reset flags. SCU3C[4]: system is reset by WDT3. SCU3C[3]: system is reset by WDT2. SCU3C[2]: system is reset by WDT1. On AST2600 platform, SCU074[31:16] are WDT reset flags. SCU074[31:28]: system is reset by WDT4 SCU074[31]: system is reset by WDT4 software reset. SCU074[27:24]: system is reset by WDT3 SCU074[27]: system is reset by WDT3 software reset. SCU074[23:20]: system is reset by WDT2 SCU074[23]: system is reset by WDT2 software reset. SCU074[19:16]: system is reset by WDT1 SCU074[19]: system is reset by WDT1 software reset. Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> --- drivers/watchdog/aspeed_wdt.c | 109 +++++++++++++++++++++++++++++++--- 1 file changed, 101 insertions(+), 8 deletions(-) diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c index b4773a6aaf8c..68eaada8a564 100644 --- a/drivers/watchdog/aspeed_wdt.c +++ b/drivers/watchdog/aspeed_wdt.c @@ -11,10 +11,12 @@ #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; @@ -22,15 +24,41 @@ module_param(nowayout, bool, 0); MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); + +/* AST SCU Register for System Reset Event Log Register Set + * ast2600 is scu074 ast2400/2500 is scu03c + */ +#define AST2400_SCU_SYS_RESET_STATUS 0x3c +#define AST2400_SCU_SYS_RESET_WDT_MASK 0x1 +#define AST2400_SCU_SYS_RESET_WDT_MASK_SHIFT 1 + +#define AST2500_SCU_SYS_RESET_WDT_MASK 0x1 +#define AST2500_SCU_SYS_RESET_WDT_MASK_SHIFT 2 + +#define AST2600_SCU_SYS_RESET_STATUS 0x74 +#define AST2600_SCU_SYS_RESET_WDT_MASK 0xf +#define AST2600_SCU_SYS_RESET_WDT_SW_MASK 0x8 +#define AST2600_SCU_SYS_RESET_WDT_MASK_SHIFT 16 + +struct aspeed_wdt_scu { + const char *compatible; + u32 reset_status_reg; + u32 wdt_reset_mask; + u32 wdt_sw_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 { struct watchdog_device wdd; void __iomem *base; + int idx; u32 ctrl; const struct aspeed_wdt_config *cfg; }; @@ -39,18 +67,39 @@ 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 = AST2400_SCU_SYS_RESET_STATUS, + .wdt_reset_mask = AST2400_SCU_SYS_RESET_WDT_MASK, + .wdt_sw_reset_mask = 0, + .wdt_reset_mask_shift = AST2400_SCU_SYS_RESET_WDT_MASK_SHIFT, + }, }; 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 = AST2400_SCU_SYS_RESET_STATUS, + .wdt_reset_mask = AST2500_SCU_SYS_RESET_WDT_MASK, + .wdt_sw_reset_mask = 0, + .wdt_reset_mask_shift = AST2500_SCU_SYS_RESET_WDT_MASK_SHIFT, + }, }; 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 = AST2600_SCU_SYS_RESET_STATUS, + .wdt_reset_mask = AST2600_SCU_SYS_RESET_WDT_MASK, + .wdt_sw_reset_mask = AST2600_SCU_SYS_RESET_WDT_SW_MASK, + .wdt_reset_mask_shift = AST2600_SCU_SYS_RESET_WDT_MASK_SHIFT, + }, }; static const struct of_device_id aspeed_wdt_of_table[] = { @@ -213,6 +262,52 @@ static int aspeed_wdt_restart(struct watchdog_device *wdd, return 0; } +static int aspeed_wdt_get_bootstatus(struct device *dev, + struct aspeed_wdt *wdt) +{ + struct device_node *np = dev->of_node; + struct aspeed_wdt_scu scu = wdt->cfg->scu; + struct regmap *scu_base; + u32 reset_mask_width; + u32 reset_mask_shift; + u32 status; + int ret; + + wdt->idx = of_alias_get_id(np, "watchdog"); + if (wdt->idx < 0) + wdt->idx = 0; + + scu_base = syscon_regmap_lookup_by_compatible(scu.compatible); + if (IS_ERR(scu_base)) + return PTR_ERR(scu_base); + + ret = regmap_read(scu_base, scu.reset_status_reg, &status); + if (ret) + return ret; + + /* On AST2400, only a bit used to represent WDT reset */ + if (of_device_is_compatible(np, "aspeed,ast2400-wdt")) + wdt->idx = 0; + + reset_mask_width = hweight32(scu.wdt_reset_mask); + reset_mask_shift = scu.wdt_reset_mask_shift + + reset_mask_width * wdt->idx; + + if (status & (scu.wdt_sw_reset_mask << reset_mask_shift)) + wdt->wdd.bootstatus = WDIOF_EXTERN1; + else if (status & (scu.wdt_reset_mask << reset_mask_shift)) + wdt->wdd.bootstatus = WDIOF_CARDRESET; + else + wdt->wdd.bootstatus = WDIOF_UNKNOWN; + + ret = regmap_write(scu_base, scu.reset_status_reg, + scu.wdt_reset_mask << reset_mask_shift); + if (ret) + return ret; + + return 0; +} + /* 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) @@ -312,7 +407,6 @@ static int aspeed_wdt_probe(struct platform_device *pdev) struct device_node *np; const char *reset_type; u32 duration; - u32 status; int ret; wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL); @@ -458,14 +552,13 @@ static int aspeed_wdt_probe(struct platform_device *pdev) writel(duration - 1, wdt->base + WDT_RESET_WIDTH); } - status = readl(wdt->base + WDT_TIMEOUT_STATUS); - if (status & WDT_TIMEOUT_STATUS_BOOT_SECONDARY) { - wdt->wdd.bootstatus = WDIOF_CARDRESET; + ret = aspeed_wdt_get_bootstatus(dev, wdt); + if (ret) + return ret; - if (of_device_is_compatible(np, "aspeed,ast2400-wdt") || - of_device_is_compatible(np, "aspeed,ast2500-wdt")) - wdt->wdd.groups = bswitch_groups; - } + if (of_device_is_compatible(np, "aspeed,ast2400-wdt") || + of_device_is_compatible(np, "aspeed,ast2500-wdt")) + wdt->wdd.groups = bswitch_groups; dev_set_drvdata(dev, wdt); -- 2.34.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 4/4] watchdog: aspeed: Add support for SW restart 2024-10-07 6:34 ` Chin-Ting Kuo @ 2024-10-07 6:34 ` Chin-Ting Kuo -1 siblings, 0 replies; 26+ messages in thread From: Chin-Ting Kuo @ 2024-10-07 6:34 UTC (permalink / raw) To: linux-aspeed WDT reset can be triggered when system hangs or a deliberate SW restart scenario. Originally, system can only know it is reset by WDT through a reset flag. However, since AST2600, a SW reset mechanism is created, SW can trigger the reset event consciously and directly without wait for WDT timeout. This function can be achieved by adding "aspeed,restart-sw" property in dts. After that, an independent reset event flag will be set after system reset by SW. Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> --- drivers/watchdog/aspeed_wdt.c | 40 ++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c index 68eaada8a564..eefca972dfa4 100644 --- a/drivers/watchdog/aspeed_wdt.c +++ b/drivers/watchdog/aspeed_wdt.c @@ -61,6 +61,7 @@ struct aspeed_wdt { int idx; u32 ctrl; const struct aspeed_wdt_config *cfg; + u32 flags; }; static const struct aspeed_wdt_config ast2400_config = { @@ -130,6 +131,11 @@ MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table); #define WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION BIT(0) #define WDT_RESET_MASK1 0x1c #define WDT_RESET_MASK2 0x20 +#define WDT_SW_RESET_CTRL 0x24 +#define WDT_SW_RESET_COUNT_CLEAR 0xDEADDEAD +#define WDT_SW_RESET_ENABLE 0xAEEDF123 +#define WDT_SW_RESET_MASK1 0x28 +#define WDT_SW_RESET_MASK2 0x2c /* * WDT_RESET_WIDTH controls the characteristics of the external pulse (if @@ -170,6 +176,9 @@ MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table); #define WDT_DEFAULT_TIMEOUT 30 #define WDT_RATE_1MHZ 1000000 +/* WDT behavior control flag */ +#define WDT_RESTART_SYSTEM_SW 0x00000001 + static struct aspeed_wdt *to_aspeed_wdt(struct watchdog_device *wdd) { return container_of(wdd, struct aspeed_wdt, wdd); @@ -249,11 +258,31 @@ static int aspeed_wdt_set_pretimeout(struct watchdog_device *wdd, return 0; } +static void aspeed_wdt_sw_reset(struct watchdog_device *wdd) +{ + struct aspeed_wdt *wdt = to_aspeed_wdt(wdd); + u32 ctrl = WDT_CTRL_RESET_MODE_SOC | + WDT_CTRL_RESET_SYSTEM; + + writel(ctrl, wdt->base + WDT_CTRL); + writel(WDT_SW_RESET_COUNT_CLEAR, + wdt->base + WDT_SW_RESET_CTRL); + writel(WDT_SW_RESET_ENABLE, wdt->base + WDT_SW_RESET_CTRL); + + /* system must be reset immediately */ + mdelay(1000); +} + static int aspeed_wdt_restart(struct watchdog_device *wdd, unsigned long action, void *data) { struct aspeed_wdt *wdt = to_aspeed_wdt(wdd); + if (wdt->flags & WDT_RESTART_SYSTEM_SW) { + aspeed_wdt_sw_reset(wdd); + return 0; + } + wdt->ctrl &= ~WDT_CTRL_BOOT_SECONDARY; aspeed_wdt_enable(wdt, 128 * WDT_RATE_1MHZ / 1000); @@ -521,8 +550,11 @@ static int aspeed_wdt_probe(struct platform_device *pdev) ret = of_property_read_u32_array(np, "aspeed,reset-mask", reset_mask, nrstmask); if (!ret) { writel(reset_mask[0], wdt->base + WDT_RESET_MASK1); - if (nrstmask > 1) + writel(reset_mask[0], wdt->base + WDT_SW_RESET_MASK1); + if (nrstmask > 1) { writel(reset_mask[1], wdt->base + WDT_RESET_MASK2); + writel(reset_mask[1], wdt->base + WDT_SW_RESET_MASK2); + } } } @@ -552,6 +584,12 @@ static int aspeed_wdt_probe(struct platform_device *pdev) writel(duration - 1, wdt->base + WDT_RESET_WIDTH); } + wdt->flags = 0; + if (!of_device_is_compatible(np, "aspeed,ast2400-wdt") && + !of_device_is_compatible(np, "aspeed,ast2500-wdt") && + of_property_read_bool(np, "aspeed,restart-sw")) + wdt->flags |= WDT_RESTART_SYSTEM_SW; + ret = aspeed_wdt_get_bootstatus(dev, wdt); if (ret) return ret; -- 2.34.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 4/4] watchdog: aspeed: Add support for SW restart @ 2024-10-07 6:34 ` Chin-Ting Kuo 0 siblings, 0 replies; 26+ messages in thread From: Chin-Ting Kuo @ 2024-10-07 6:34 UTC (permalink / raw) To: patrick, wim, linux, robh, krzk+dt, conor+dt, joel, andrew, linux-watchdog, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel Cc: Peter.Yin, Patrick_NC_Lin, Bonnie_Lo, DELPHINE_CHIU, BMC-SW WDT reset can be triggered when system hangs or a deliberate SW restart scenario. Originally, system can only know it is reset by WDT through a reset flag. However, since AST2600, a SW reset mechanism is created, SW can trigger the reset event consciously and directly without wait for WDT timeout. This function can be achieved by adding "aspeed,restart-sw" property in dts. After that, an independent reset event flag will be set after system reset by SW. Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> --- drivers/watchdog/aspeed_wdt.c | 40 ++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c index 68eaada8a564..eefca972dfa4 100644 --- a/drivers/watchdog/aspeed_wdt.c +++ b/drivers/watchdog/aspeed_wdt.c @@ -61,6 +61,7 @@ struct aspeed_wdt { int idx; u32 ctrl; const struct aspeed_wdt_config *cfg; + u32 flags; }; static const struct aspeed_wdt_config ast2400_config = { @@ -130,6 +131,11 @@ MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table); #define WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION BIT(0) #define WDT_RESET_MASK1 0x1c #define WDT_RESET_MASK2 0x20 +#define WDT_SW_RESET_CTRL 0x24 +#define WDT_SW_RESET_COUNT_CLEAR 0xDEADDEAD +#define WDT_SW_RESET_ENABLE 0xAEEDF123 +#define WDT_SW_RESET_MASK1 0x28 +#define WDT_SW_RESET_MASK2 0x2c /* * WDT_RESET_WIDTH controls the characteristics of the external pulse (if @@ -170,6 +176,9 @@ MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table); #define WDT_DEFAULT_TIMEOUT 30 #define WDT_RATE_1MHZ 1000000 +/* WDT behavior control flag */ +#define WDT_RESTART_SYSTEM_SW 0x00000001 + static struct aspeed_wdt *to_aspeed_wdt(struct watchdog_device *wdd) { return container_of(wdd, struct aspeed_wdt, wdd); @@ -249,11 +258,31 @@ static int aspeed_wdt_set_pretimeout(struct watchdog_device *wdd, return 0; } +static void aspeed_wdt_sw_reset(struct watchdog_device *wdd) +{ + struct aspeed_wdt *wdt = to_aspeed_wdt(wdd); + u32 ctrl = WDT_CTRL_RESET_MODE_SOC | + WDT_CTRL_RESET_SYSTEM; + + writel(ctrl, wdt->base + WDT_CTRL); + writel(WDT_SW_RESET_COUNT_CLEAR, + wdt->base + WDT_SW_RESET_CTRL); + writel(WDT_SW_RESET_ENABLE, wdt->base + WDT_SW_RESET_CTRL); + + /* system must be reset immediately */ + mdelay(1000); +} + static int aspeed_wdt_restart(struct watchdog_device *wdd, unsigned long action, void *data) { struct aspeed_wdt *wdt = to_aspeed_wdt(wdd); + if (wdt->flags & WDT_RESTART_SYSTEM_SW) { + aspeed_wdt_sw_reset(wdd); + return 0; + } + wdt->ctrl &= ~WDT_CTRL_BOOT_SECONDARY; aspeed_wdt_enable(wdt, 128 * WDT_RATE_1MHZ / 1000); @@ -521,8 +550,11 @@ static int aspeed_wdt_probe(struct platform_device *pdev) ret = of_property_read_u32_array(np, "aspeed,reset-mask", reset_mask, nrstmask); if (!ret) { writel(reset_mask[0], wdt->base + WDT_RESET_MASK1); - if (nrstmask > 1) + writel(reset_mask[0], wdt->base + WDT_SW_RESET_MASK1); + if (nrstmask > 1) { writel(reset_mask[1], wdt->base + WDT_RESET_MASK2); + writel(reset_mask[1], wdt->base + WDT_SW_RESET_MASK2); + } } } @@ -552,6 +584,12 @@ static int aspeed_wdt_probe(struct platform_device *pdev) writel(duration - 1, wdt->base + WDT_RESET_WIDTH); } + wdt->flags = 0; + if (!of_device_is_compatible(np, "aspeed,ast2400-wdt") && + !of_device_is_compatible(np, "aspeed,ast2500-wdt") && + of_property_read_bool(np, "aspeed,restart-sw")) + wdt->flags |= WDT_RESTART_SYSTEM_SW; + ret = aspeed_wdt_get_bootstatus(dev, wdt); if (ret) return ret; -- 2.34.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
end of thread, other threads:[~2024-10-14 9:58 UTC | newest] Thread overview: 26+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-10-07 6:34 [PATCH 0/4] [PATCH 0/4] Update ASPEED WDT bootstatus Chin-Ting Kuo 2024-10-07 6:34 ` Chin-Ting Kuo 2024-10-07 6:34 ` [PATCH 1/4] dt-bindings: watchdog: aspeed: Add property for WDT SW reset Chin-Ting Kuo 2024-10-07 6:34 ` Chin-Ting Kuo 2024-10-07 6:58 ` Krzysztof Kozlowski 2024-10-07 6:58 ` Krzysztof Kozlowski 2024-10-14 2:07 ` Chin-Ting Kuo 2024-10-14 2:07 ` Chin-Ting Kuo 2024-10-14 6:53 ` Krzysztof Kozlowski 2024-10-14 6:53 ` Krzysztof Kozlowski 2024-10-14 9:58 ` Chin-Ting Kuo 2024-10-14 9:58 ` Chin-Ting Kuo 2024-10-07 17:59 ` Rob Herring 2024-10-07 17:59 ` Rob Herring 2024-10-07 19:54 ` Guenter Roeck 2024-10-07 19:54 ` Guenter Roeck 2024-10-14 2:08 ` Chin-Ting Kuo 2024-10-14 2:08 ` Chin-Ting Kuo 2024-10-14 2:07 ` Chin-Ting Kuo 2024-10-14 2:07 ` Chin-Ting Kuo 2024-10-07 6:34 ` [PATCH 2/4] ARM: dts: aspeed: Add WDT controller into alias field Chin-Ting Kuo 2024-10-07 6:34 ` Chin-Ting Kuo 2024-10-07 6:34 ` [PATCH 3/4] watchdog: aspeed: Update bootstatus handling Chin-Ting Kuo 2024-10-07 6:34 ` Chin-Ting Kuo 2024-10-07 6:34 ` [PATCH 4/4] watchdog: aspeed: Add support for SW restart Chin-Ting Kuo 2024-10-07 6:34 ` Chin-Ting Kuo
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.