diff mbox series

[v4,1/3] dt-bindings: watchdog: ti,rti-wdt: Add support for WDIOF_CARDRESET

Message ID 20230717040723.1306374-2-huaqian.li@siemens.com
State Superseded
Headers show
Series Add support for WDIOF_CARDRESET on TI AM65x | expand

Commit Message

Li, Hua Qian July 17, 2023, 4:07 a.m. UTC
From: Li Hua Qian <huaqian.li@siemens.com>

TI RTI (Real Time Interrupt) Watchdog doesn't support to record the
watchdog cause. Add a reserved memory to know the last reboot was caused
by the watchdog card. In the reserved memory, some specific info will be
saved to indicate whether the watchdog reset was triggered in last
boot.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Conor Dooley <conor@kernel.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Li Hua Qian <huaqian.li@siemens.com>
---
 .../bindings/watchdog/ti,rti-wdt.yaml         | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)

Comments

Krzysztof Kozlowski July 17, 2023, 6:13 a.m. UTC | #1
On 17/07/2023 06:07, huaqian.li@siemens.com wrote:
> From: Li Hua Qian <huaqian.li@siemens.com>
> 
> TI RTI (Real Time Interrupt) Watchdog doesn't support to record the
> watchdog cause. Add a reserved memory to know the last reboot was caused
> by the watchdog card. In the reserved memory, some specific info will be
> saved to indicate whether the watchdog reset was triggered in last
> boot.
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Reviewed-by: Conor Dooley <conor@kernel.org>
> Reviewed-by: Rob Herring <robh@kernel.org>

What? Where did these happened? Please provide links.

> Signed-off-by: Li Hua Qian <huaqian.li@siemens.com>
> ---
>  .../bindings/watchdog/ti,rti-wdt.yaml         | 41 +++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml b/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
> index fc553211e42d..4b66c4fcdf35 100644
> --- a/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
> @@ -34,6 +34,20 @@ properties:
>    power-domains:
>      maxItems: 1
>  
> +  memory-region:
> +    maxItems: 1
> +    description:
> +      Contains the watchdog reserved memory. It is optional.
> +      In the reserved memory, the specified values, which are
> +      PON_REASON_SOF_NUM(0xBBBBCCCC), PON_REASON_MAGIC_NUM(0xDDDDDDDD),
> +      and PON_REASON_EOF_NUM(0xCCCCBBBB), are pre-stored at the first
> +      3 * 4 bytes to tell that last boot was caused by watchdog reset.
> +      Once the PON reason is captured by driver(rti_wdt.c), the driver
> +      is supposed to wipe the whole memory region. Surely, if this
> +      property is set, at least 12 bytes reserved memory starting from
> +      specific memory address(0xa220000) should be set. More please
> +      refer to Example 2.
> +
>  required:
>    - compatible
>    - reg
> @@ -59,3 +73,30 @@ examples:
>          assigned-clocks = <&k3_clks 252 1>;
>          assigned-clock-parents = <&k3_clks 252 5>;
>      };
> +
> +  - |
> +    // Example 2 (Add reserved memory for watchdog reset cause):
> +    /*
> +     * RTI WDT in main domain on J721e SoC. Assigned clocks are used to
> +     * select the source clock for the watchdog, forcing it to tick with
> +     * a 32kHz clock in this case. Add a reserved memory to keep the
> +     * watchdog reset cause persistent, which was be written in 12 bytes
> +     * starting from 0xa2200000 by RTI Watchdog Firmware.
> +     *
> +     * Reserved memory should be defined as follows:
> +     * reserved-memory {
> +     *     wdt_reset_memory_region: wdt-memory@a2200000 {
> +     *         reg = <0x00 0xa2200000 0x00 0x1000>;
> +     *         no-map;
> +     *     };
> +     * }
> +     */

Integrate it with existing binding... there is really no need for new
example for one new property.

Best regards,
Krzysztof
Li, Hua Qian July 17, 2023, 7:13 a.m. UTC | #2
On Sun, 2023-07-16 at 22:15 -0700, Guenter Roeck wrote:
> On 7/16/23 21:07, huaqian.li@siemens.com wrote:
> > From: Li Hua Qian <huaqian.li@siemens.com>
> > 
> > TI RTI (Real Time Interrupt) Watchdog doesn't support to record the
> > watchdog cause. Add a reserved memory to know the last reboot was
> > caused
> > by the watchdog card. In the reserved memory, some specific info
> > will be
> > saved to indicate whether the watchdog reset was triggered in last
> > boot.
> > 
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > Reviewed-by: Conor Dooley <conor@kernel.org>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Signed-off-by: Li Hua Qian <huaqian.li@siemens.com>
> 
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Hi Guenter,

I'm going to integrate it with the existing binding as Krzysztof
suggested, could I leave you in `Reviewed-by`?

   59 examples:                                                       
   60   - |                                                           
   61     /*                                                          
   62      * RTI WDT in main domain on J721e SoC. Assigned clocks are
used to                                                               
   63      * select the source clock for the watchdog, forcing it to
tick with                                                             
~  64      * a 32kHz clock in this case. Add a reserved
memory(optional) to keep                                              
~_ 65      * the watchdog reset cause persistent, which was be written
in 12 bytes                                                           
   66      * starting from 0xa2200000 by RTI Watchdog Firmware.       
   67      *                                                          
   68      * Reserved memory should be defined as follows:            
   69      * reserved-memory {                                        
   70      *     wdt_reset_memory_region: wdt-memory@a2200000 {       
   71      *         reg = <0x00 0xa2200000 0x00 0x1000>;             
   72      *         no-map;                                          
   73      *     };                                                   
   74      * }                                                        
   75      */                                                         
~  76     #include <dt-bindings/soc/ti,sci_pm_domain.h>               
+  77                                                                 
+  78     watchdog@2200000 {                                          
   79         compatible = "ti,j7-rti-wdt";                           
~  80         reg = <0x2200000 0x100>;                                
~  81         clocks = <&k3_clks 252 1>;                              
~  82         power-domains = <&k3_pds 252 TI_SCI_PD_EXCLUSIVE>;      
~  83         assigned-clocks = <&k3_clks 252 1>;                     
~  84         assigned-clock-parents = <&k3_clks 252 5>;              
   85         memory-region = <&wdt_reset_memory_region>;             
   86     };                                                          

Best regards,
Li Hua Qian                                                           
> 
> > ---
> >   .../bindings/watchdog/ti,rti-wdt.yaml         | 41
> > +++++++++++++++++++
> >   1 file changed, 41 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/watchdog/ti,rti-
> > wdt.yaml b/Documentation/devicetree/bindings/watchdog/ti,rti-
> > wdt.yaml
> > index fc553211e42d..4b66c4fcdf35 100644
> > --- a/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
> > +++ b/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
> > @@ -34,6 +34,20 @@ properties:
> >     power-domains:
> >       maxItems: 1
> >   
> > +  memory-region:
> > +    maxItems: 1
> > +    description:
> > +      Contains the watchdog reserved memory. It is optional.
> > +      In the reserved memory, the specified values, which are
> > +      PON_REASON_SOF_NUM(0xBBBBCCCC),
> > PON_REASON_MAGIC_NUM(0xDDDDDDDD),
> > +      and PON_REASON_EOF_NUM(0xCCCCBBBB), are pre-stored at the
> > first
> > +      3 * 4 bytes to tell that last boot was caused by watchdog
> > reset.
> > +      Once the PON reason is captured by driver(rti_wdt.c), the
> > driver
> > +      is supposed to wipe the whole memory region. Surely, if this
> > +      property is set, at least 12 bytes reserved memory starting
> > from
> > +      specific memory address(0xa220000) should be set. More
> > please
> > +      refer to Example 2.
> > +
> >   required:
> >     - compatible
> >     - reg
> > @@ -59,3 +73,30 @@ examples:
> >           assigned-clocks = <&k3_clks 252 1>;
> >           assigned-clock-parents = <&k3_clks 252 5>;
> >       };
> > +
> > +  - |
> > +    // Example 2 (Add reserved memory for watchdog reset cause):
> > +    /*
> > +     * RTI WDT in main domain on J721e SoC. Assigned clocks are
> > used to
> > +     * select the source clock for the watchdog, forcing it to
> > tick with
> > +     * a 32kHz clock in this case. Add a reserved memory to keep
> > the
> > +     * watchdog reset cause persistent, which was be written in 12
> > bytes
> > +     * starting from 0xa2200000 by RTI Watchdog Firmware.
> > +     *
> > +     * Reserved memory should be defined as follows:
> > +     * reserved-memory {
> > +     *     wdt_reset_memory_region: wdt-memory@a2200000 {
> > +     *         reg = <0x00 0xa2200000 0x00 0x1000>;
> > +     *         no-map;
> > +     *     };
> > +     * }
> > +     */
> > +    watchdog@40610000 {
> > +        compatible = "ti,j7-rti-wdt";
> > +        reg = <0x40610000 0x100>;
> > +        clocks = <&k3_clks 135 1>;
> > +        power-domains = <&k3_pds 135 TI_SCI_PD_EXCLUSIVE>;
> > +        assigned-clocks = <&k3_clks 135 0>;
> > +        assigned-clock-parents = <&k3_clks 135 4>;
> > +        memory-region = <&wdt_reset_memory_region>;
> > +    };
>
Conor Dooley July 17, 2023, 7:17 a.m. UTC | #3
On Mon, Jul 17, 2023 at 12:07:21PM +0800, huaqian.li@siemens.com wrote:
> From: Li Hua Qian <huaqian.li@siemens.com>
> 
> TI RTI (Real Time Interrupt) Watchdog doesn't support to record the
> watchdog cause. Add a reserved memory to know the last reboot was caused
> by the watchdog card. In the reserved memory, some specific info will be
> saved to indicate whether the watchdog reset was triggered in last
> boot.
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

> Reviewed-by: Conor Dooley <conor@kernel.org>

I don't recall actually replying to the earlier revisions of this
patchset, let alone providing a review, but this is not the email
address I would have used, had I done so.

> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Li Hua Qian <huaqian.li@siemens.com>
Li, Hua Qian July 17, 2023, 7:23 a.m. UTC | #4
On Mon, 2023-07-17 at 08:17 +0100, Conor Dooley wrote:
> On Mon, Jul 17, 2023 at 12:07:21PM +0800,
> huaqian.li@siemens.com wrote:
> > From: Li Hua Qian <huaqian.li@siemens.com>
> > 
> > TI RTI (Real Time Interrupt) Watchdog doesn't support to record the
> > watchdog cause. Add a reserved memory to know the last reboot was
> > caused
> > by the watchdog card. In the reserved memory, some specific info
> > will be
> > saved to indicate whether the watchdog reset was triggered in last
> > boot.
> > 
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> > Reviewed-by: Conor Dooley <conor@kernel.org>
> 
> I don't recall actually replying to the earlier revisions of this
> patchset, let alone providing a review, but this is not the email
> address I would have used, had I done so.
> 
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Signed-off-by: Li Hua Qian <huaqian.li@siemens.com>
Because of my mistake in v4, I feel very sad and sorry. 

I was trying to fix it in v5, please ignore the v4 and jump to v5. Many
thanks!

Best regards,
Li Hua Qian
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml b/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
index fc553211e42d..4b66c4fcdf35 100644
--- a/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
+++ b/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
@@ -34,6 +34,20 @@  properties:
   power-domains:
     maxItems: 1
 
+  memory-region:
+    maxItems: 1
+    description:
+      Contains the watchdog reserved memory. It is optional.
+      In the reserved memory, the specified values, which are
+      PON_REASON_SOF_NUM(0xBBBBCCCC), PON_REASON_MAGIC_NUM(0xDDDDDDDD),
+      and PON_REASON_EOF_NUM(0xCCCCBBBB), are pre-stored at the first
+      3 * 4 bytes to tell that last boot was caused by watchdog reset.
+      Once the PON reason is captured by driver(rti_wdt.c), the driver
+      is supposed to wipe the whole memory region. Surely, if this
+      property is set, at least 12 bytes reserved memory starting from
+      specific memory address(0xa220000) should be set. More please
+      refer to Example 2.
+
 required:
   - compatible
   - reg
@@ -59,3 +73,30 @@  examples:
         assigned-clocks = <&k3_clks 252 1>;
         assigned-clock-parents = <&k3_clks 252 5>;
     };
+
+  - |
+    // Example 2 (Add reserved memory for watchdog reset cause):
+    /*
+     * RTI WDT in main domain on J721e SoC. Assigned clocks are used to
+     * select the source clock for the watchdog, forcing it to tick with
+     * a 32kHz clock in this case. Add a reserved memory to keep the
+     * watchdog reset cause persistent, which was be written in 12 bytes
+     * starting from 0xa2200000 by RTI Watchdog Firmware.
+     *
+     * Reserved memory should be defined as follows:
+     * reserved-memory {
+     *     wdt_reset_memory_region: wdt-memory@a2200000 {
+     *         reg = <0x00 0xa2200000 0x00 0x1000>;
+     *         no-map;
+     *     };
+     * }
+     */
+    watchdog@40610000 {
+        compatible = "ti,j7-rti-wdt";
+        reg = <0x40610000 0x100>;
+        clocks = <&k3_clks 135 1>;
+        power-domains = <&k3_pds 135 TI_SCI_PD_EXCLUSIVE>;
+        assigned-clocks = <&k3_clks 135 0>;
+        assigned-clock-parents = <&k3_clks 135 4>;
+        memory-region = <&wdt_reset_memory_region>;
+    };