From: Li Hua Qian <[email protected]>
The watchdog hardware of TI AM65X platform does not support
WDIOF_CARDRESET feature, add a reserved memory to save the watchdog
reset cause, to know if the board reboot is due to a watchdog reset.
Signed-off-by: Li Hua Qian <[email protected]>
---
Changes in v4:
- Fix the coding style.
- Add usage note for the reserved memory.
- Link to v3:
https://lore.kernel.org/linux-watchdog/[email protected]
Changes in v3:
- Add memory-region back for the reserved memory, and remove reserved
memory from the watchdog IO address space.
- Add changelog.
- Link to v2:
https://lore.kernel.org/linux-watchdog/[email protected]
Changes in v2:
- Remove memory-region and memory-size properties, and bind the reserved
memory to watchdog IO address space.
- Remove the unnecessary rti_wdt_ioctl.
- Fix the mail list
- Link to v1:
https://lore.kernel.org/all/[email protected]
v1 had a wrong mail list at the beginning, and the mail thread was
messed up.
Li Hua Qian (3):
dt-bindings: watchdog: ti,rti-wdt: Add support for WDIOF_CARDRESET
arm64: dts: ti: Add reserved memory for watchdog
watchdog:rit_wdt: Add support for WDIOF_CARDRESET
.../bindings/watchdog/ti,rti-wdt.yaml | 41 ++++++++++++++++
.../boot/dts/ti/k3-am65-iot2050-common.dtsi | 10 ++++
drivers/watchdog/rti_wdt.c | 48 +++++++++++++++++++
3 files changed, 99 insertions(+)
--
2.34.1
From: Li Hua Qian <[email protected]>
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 <[email protected]>
Reviewed-by: Conor Dooley <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Signed-off-by: Li Hua Qian <[email protected]>
---
.../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>;
+ };
--
2.34.1
On 7/16/23 21:07, [email protected] wrote:
> From: Li Hua Qian <[email protected]>
>
> 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 <[email protected]>
> Reviewed-by: Conor Dooley <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
> Signed-off-by: Li Hua Qian <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> .../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, [email protected] wrote:
> From: Li Hua Qian <[email protected]>
>
> The watchdog hardware of TI AM65X platform does not support
> WDIOF_CARDRESET feature, add a reserved memory to save the watchdog
> reset cause, to know if the board reboot is due to a watchdog reset.
>
> Signed-off-by: Li Hua Qian <[email protected]>
> ---
> Changes in v4:
> - Fix the coding style.
> - Add usage note for the reserved memory.
> - Link to v3:
> https://lore.kernel.org/linux-watchdog/[email protected]
Much more changed. You added example in the bindings which no one asked
for. Then you added multiple fake review tags to all the patches.
Best regards,
Krzysztof
On 17/07/2023 06:07, [email protected] wrote:
> From: Li Hua Qian <[email protected]>
>
> 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 <[email protected]>
> Reviewed-by: Conor Dooley <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
What? Where did these happened? Please provide links.
> Signed-off-by: Li Hua Qian <[email protected]>
> ---
> .../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, [email protected] wrote:
>> From: Li Hua Qian <[email protected]>
>>
>> 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 <[email protected]>
>> Reviewed-by: Conor Dooley <[email protected]>
>> Reviewed-by: Rob Herring <[email protected]>
>
> What? Where did these happened? Please provide links.
To clarify: that's a NAK.
Best regards,
Krzysztof
On Mon, 2023-07-17 at 08:16 +0200, Krzysztof Kozlowski wrote:
> On 17/07/2023 06:07, [email protected] wrote:
> > From: Li Hua Qian <[email protected]>
> >
> > The watchdog hardware of TI AM65X platform does not support
> > WDIOF_CARDRESET feature, add a reserved memory to save the watchdog
> > reset cause, to know if the board reboot is due to a watchdog
> > reset.
> >
> > Signed-off-by: Li Hua Qian <[email protected]>
> > ---
> > Changes in v4:
> > - Fix the coding style.
> > - Add usage note for the reserved memory.
> > - Link to v3:
> >
> > https://lore.kernel.org/linux-watchdog/[email protected]
>
> Much more changed. You added example in the bindings which no one
> asked
> for. Then you added multiple fake review tags to all the patches.
>
> Best regards,
> Krzysztof
>
Hi,
Sorry for the wrong statement. I missed some key information and
missunderstood `Reviewed-by`, I treated `Reviewed-by` as `Who
has reviewed`.
Best regards,
Li Hua Qian
On 17/07/2023 08:24, Li, Hua Qian wrote:
> On Mon, 2023-07-17 at 08:16 +0200, Krzysztof Kozlowski wrote:
>> On 17/07/2023 06:07, [email protected] wrote:
>>> From: Li Hua Qian <[email protected]>
>>>
>>> The watchdog hardware of TI AM65X platform does not support
>>> WDIOF_CARDRESET feature, add a reserved memory to save the watchdog
>>> reset cause, to know if the board reboot is due to a watchdog
>>> reset.
>>>
>>> Signed-off-by: Li Hua Qian <[email protected]>
>>> ---
>>> Changes in v4:
>>> - Fix the coding style.
>>> - Add usage note for the reserved memory.
>>> - Link to v3:
>>>
>>> https://lore.kernel.org/linux-watchdog/[email protected]
>>
>> Much more changed. You added example in the bindings which no one
>> asked
>> for. Then you added multiple fake review tags to all the patches.
>>
>> Best regards,
>> Krzysztof
>>
> Hi,
>
> Sorry for the wrong statement. I missed some key information and
> missunderstood `Reviewed-by`, I treated `Reviewed-by` as `Who
> has reviewed`.
But you don't have even that information who has reviewed! Where do you
see any reviews coming from me for patch #2? Where do you see reviews
from Rob for patch #3?
Best regards,
Krzysztof
On Mon, 2023-07-17 at 08:27 +0200, Krzysztof Kozlowski wrote:
> On 17/07/2023 08:24, Li, Hua Qian wrote:
> > On Mon, 2023-07-17 at 08:16 +0200, Krzysztof Kozlowski wrote:
> > > On 17/07/2023 06:07, [email protected] wrote:
> > > > From: Li Hua Qian <[email protected]>
> > > >
> > > > The watchdog hardware of TI AM65X platform does not support
> > > > WDIOF_CARDRESET feature, add a reserved memory to save the
> > > > watchdog
> > > > reset cause, to know if the board reboot is due to a watchdog
> > > > reset.
> > > >
> > > > Signed-off-by: Li Hua Qian <[email protected]>
> > > > ---
> > > > Changes in v4:
> > > > - Fix the coding style.
> > > > - Add usage note for the reserved memory.
> > > > - Link to v3:
> > > >
> > > > https://lore.kernel.org/linux-watchdog/[email protected]
> > >
> > > Much more changed. You added example in the bindings which no one
> > > asked
> > > for. Then you added multiple fake review tags to all the patches.
> > >
> > > Best regards,
> > > Krzysztof
> > >
> > Hi,
> >
> > Sorry for the wrong statement. I missed some key information and
> > missunderstood `Reviewed-by`, I treated `Reviewed-by` as `Who
> > has reviewed`.
>
> But you don't have even that information who has reviewed! Where do
> you
> see any reviews coming from me for patch #2? Where do you see reviews
> from Rob for patch #3?
>
> Best regards,
> Krzysztof
>
I got these information from my email thread. Anyway I made a stupid
mistake, sorry for wasting your time.
By the way, when you wrote the following in '[PATCH v3 1/3] dt-
bindings: watchdog: ti,rti-wdt: Add support for WDIOF_CARDRESET', you
were kind of saying that it looks good to you if I remove the extra
empty line, right?
In any case:
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Li Hua Qian
On 17/07/2023 08:34, Li, Hua Qian wrote:
> On Mon, 2023-07-17 at 08:27 +0200, Krzysztof Kozlowski wrote:
>> On 17/07/2023 08:24, Li, Hua Qian wrote:
>>> On Mon, 2023-07-17 at 08:16 +0200, Krzysztof Kozlowski wrote:
>>>> On 17/07/2023 06:07, [email protected] wrote:
>>>>> From: Li Hua Qian <[email protected]>
>>>>>
>>>>> The watchdog hardware of TI AM65X platform does not support
>>>>> WDIOF_CARDRESET feature, add a reserved memory to save the
>>>>> watchdog
>>>>> reset cause, to know if the board reboot is due to a watchdog
>>>>> reset.
>>>>>
>>>>> Signed-off-by: Li Hua Qian <[email protected]>
>>>>> ---
>>>>> Changes in v4:
>>>>> - Fix the coding style.
>>>>> - Add usage note for the reserved memory.
>>>>> - Link to v3:
>>>>>
>>>>> https://lore.kernel.org/linux-watchdog/[email protected]
>>>>
>>>> Much more changed. You added example in the bindings which no one
>>>> asked
>>>> for. Then you added multiple fake review tags to all the patches.
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>>
>>> Hi,
>>>
>>> Sorry for the wrong statement. I missed some key information and
>>> missunderstood `Reviewed-by`, I treated `Reviewed-by` as `Who
>>> has reviewed`.
>>
>> But you don't have even that information who has reviewed! Where do
>> you
>> see any reviews coming from me for patch #2? Where do you see reviews
>> from Rob for patch #3?
>>
>> Best regards,
>> Krzysztof
>>
> I got these information from my email thread. Anyway I made a stupid
> mistake, sorry for wasting your time.
>
> By the way, when you wrote the following in '[PATCH v3 1/3] dt-
> bindings: watchdog: ti,rti-wdt: Add support for WDIOF_CARDRESET', you
> were kind of saying that it looks good to you if I remove the extra
> empty line, right?
>
> In any case:
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
This was patch 1. But you added my review to patch 2 also. Why then not
adding to patch 3? What logic is driving this?
Best regards,
Krzysztof
On Mon, 2023-07-17 at 08:43 +0200, Krzysztof Kozlowski wrote:
> On 17/07/2023 08:34, Li, Hua Qian wrote:
> > On Mon, 2023-07-17 at 08:27 +0200, Krzysztof Kozlowski wrote:
> > > On 17/07/2023 08:24, Li, Hua Qian wrote:
> > > > On Mon, 2023-07-17 at 08:16 +0200, Krzysztof Kozlowski wrote:
> > > > > On 17/07/2023 06:07, [email protected] wrote:
> > > > > > From: Li Hua Qian <[email protected]>
> > > > > >
> > > > > > The watchdog hardware of TI AM65X platform does not support
> > > > > > WDIOF_CARDRESET feature, add a reserved memory to save the
> > > > > > watchdog
> > > > > > reset cause, to know if the board reboot is due to a
> > > > > > watchdog
> > > > > > reset.
> > > > > >
> > > > > > Signed-off-by: Li Hua Qian <[email protected]>
> > > > > > ---
> > > > > > Changes in v4:
> > > > > > - Fix the coding style.
> > > > > > - Add usage note for the reserved memory.
> > > > > > - Link to v3:
> > > > > >
> > > > > > https://lore.kernel.org/linux-watchdog/[email protected]
> > > > >
> > > > > Much more changed. You added example in the bindings which no
> > > > > one
> > > > > asked
> > > > > for. Then you added multiple fake review tags to all the
> > > > > patches.
> > > > >
> > > > > Best regards,
> > > > > Krzysztof
> > > > >
> > > > Hi,
> > > >
> > > > Sorry for the wrong statement. I missed some key information
> > > > and
> > > > missunderstood `Reviewed-by`, I treated `Reviewed-by` as `Who
> > > > has reviewed`.
> > >
> > > But you don't have even that information who has reviewed! Where
> > > do
> > > you
> > > see any reviews coming from me for patch #2? Where do you see
> > > reviews
> > > from Rob for patch #3?
> > >
> > > Best regards,
> > > Krzysztof
> > >
> > I got these information from my email thread. Anyway I made a
> > stupid
> > mistake, sorry for wasting your time.
> >
> > By the way, when you wrote the following in '[PATCH v3 1/3] dt-
> > bindings: watchdog: ti,rti-wdt: Add support for WDIOF_CARDRESET',
> > you
> > were kind of saying that it looks good to you if I remove the extra
> > empty line, right?
> >
> > In any case:
> > Reviewed-by: Krzysztof Kozlowski <[email protected]>
>
> This was patch 1. But you added my review to patch 2 also. Why then
> not
> adding to patch 3? What logic is driving this?
>
> Best regards,
> Krzysztof
>
Much sorry again, please forget the wrong info in v4. I wrongly added
you to patch 2 only because I found you commented on patch 2 once.
Best regards,
Li Hua Qian
On Mon, 2023-07-17 at 14:23 +0800, Hua Qian Li wrote:
> On Mon, 2023-07-17 at 08:16 +0200, Krzysztof Kozlowski wrote:
> > On 17/07/2023 06:07, [email protected] wrote:
> > > From: Li Hua Qian <[email protected]>
> > >
> > > The watchdog hardware of TI AM65X platform does not support
> > > WDIOF_CARDRESET feature, add a reserved memory to save the
> > > watchdog
> > > reset cause, to know if the board reboot is due to a watchdog
> > > reset.
> > >
> > > Signed-off-by: Li Hua Qian <[email protected]>
> > > ---
> > > Changes in v4:
> > > - Fix the coding style.
> > > - Add usage note for the reserved memory.
> > > - Link to v3:
> > >
> > > https://lore.kernel.org/linux-watchdog/[email protected]
> >
> > Much more changed. You added example in the bindings which no one
> > asked
> > for. Then you added multiple fake review tags to all the patches.
> >
> > Best regards,
> > Krzysztof
> >
> Hi,
>
> Sorry for the wrong statement. I missed some key information and
> missunderstood `Reviewed-by`, I treated `Reviewed-by` as `Who
> has reviewed`.
>
> Best regards,
> Li Hua Qian
By the way, `Much more changes` came from the following message which
was commented on [PATCH v3 0/3] Add support for WDIOF_CARDRESET on TI
AM65x:
One thing I keep wondering about: What prevents the Linux kernel from
treating the special memory area like normal memory ? I would have
expected
some usage note, such as that the memory area must be reported as
reserved
to the kernel, but I don't see anything like that.
Guenter
On Mon, Jul 17, 2023 at 12:07:21PM +0800, [email protected] wrote:
> From: Li Hua Qian <[email protected]>
>
> 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 <[email protected]>
> Reviewed-by: Conor Dooley <[email protected]>
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 <[email protected]>
> Signed-off-by: Li Hua Qian <[email protected]>
On Sun, 2023-07-16 at 22:15 -0700, Guenter Roeck wrote:
> On 7/16/23 21:07, [email protected] wrote:
> > From: Li Hua Qian <[email protected]>
> >
> > 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 <[email protected]>
> > Reviewed-by: Conor Dooley <[email protected]>
> > Reviewed-by: Rob Herring <[email protected]>
> > Signed-off-by: Li Hua Qian <[email protected]>
>
> Reviewed-by: Guenter Roeck <[email protected]>
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, 2023-07-17 at 08:17 +0100, Conor Dooley wrote:
> On Mon, Jul 17, 2023 at 12:07:21PM +0800,
> [email protected] wrote:
> > From: Li Hua Qian <[email protected]>
> >
> > 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 <[email protected]>
>
> > Reviewed-by: Conor Dooley <[email protected]>
>
> 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 <[email protected]>
> > Signed-off-by: Li Hua Qian <[email protected]>
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