| Message ID | 20230717040723.1306374-2-huaqian.li@siemens.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Add support for WDIOF_CARDRESET on TI AM65x | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
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> > --- > .../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>; > + };
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
On 17/07/2023 08:13, Krzysztof Kozlowski wrote: > 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. To clarify: that's a NAK. Best regards, Krzysztof
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>; > > + }; >
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>
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 --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>; + };