[PATCH v3 0/3] Add support for WDIOF_CARDRESET on TI AM65x

[PATCH v3 0/3] Add support for WDIOF_CARDRESET on TI AM65x

[huaqian.li]

From: Li Hua Qian <huaqian.li@siemens.com>

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.

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/20230711091713.1113010-1-huaqian.li@siemens.com

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/3137d87e56ef75ba0b8a923d407b2fecace6ccbd.camel@siemens.com/
  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         | 12 +++++
 .../boot/dts/ti/k3-am65-iot2050-common.dtsi   | 10 ++++
 drivers/watchdog/rti_wdt.c                    | 51 +++++++++++++++++++
 3 files changed, 73 insertions(+)

-- 
2.34.1

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

[huaqian.li]

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.

Signed-off-by: Li Hua Qian <huaqian.li@siemens.com>
---
 .../devicetree/bindings/watchdog/ti,rti-wdt.yaml     | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml b/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
index fc553211e42d..8c16fd3929ec 100644
--- a/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
+++ b/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
@@ -34,6 +34,18 @@ 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.
+
+
 required:
   - compatible
   - reg
-- 
2.34.1

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

[Krzysztof Kozlowski]

On 13/07/2023 11:51, 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.
> 
> Signed-off-by: Li Hua Qian <huaqian.li@siemens.com>
> ---
>  .../devicetree/bindings/watchdog/ti,rti-wdt.yaml     | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml b/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
> index fc553211e42d..8c16fd3929ec 100644
> --- a/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
> @@ -34,6 +34,18 @@ 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.
> +
> +

If there is going to be new version, only one blank line, not two.

In any case:
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH v3 2/3] arm64: dts: ti: Add reserved memory for watchdog

[huaqian.li]

From: Li Hua Qian <huaqian.li@siemens.com>

This patch adds a reserved memory for the TI AM65X platform watchdog to
reserve the specific info, triggering the watchdog reset in last boot,
to know if the board reboot is due to a watchdog reset.

Signed-off-by: Li Hua Qian <huaqian.li@siemens.com>
---
 arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
index e26bd988e522..4bb20d493651 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
@@ -63,6 +63,12 @@ rtos_ipc_memory_region: ipc-memories@a2000000 {
 			alignment = <0x1000>;
 			no-map;
 		};
+
+		/* To reserve the power-on(PON) reason for watchdog reset */
+		wdt_reset_memory_region: wdt-memory@a2200000 {
+			reg = <0x00 0xa2200000 0x00 0x1000>;
+			no-map;
+		};
 	};
 
 	leds {
@@ -718,3 +724,7 @@ &mcu_r5fss0_core1 {
 			<&mcu_r5fss0_core1_memory_region>;
 	mboxes = <&mailbox0_cluster1>, <&mbox_mcu_r5fss0_core1>;
 };
+
+&mcu_rti1 {
+	memory-region = <&wdt_reset_memory_region>;
+};
-- 
2.34.1

Re: [PATCH v3 2/3] arm64: dts: ti: Add reserved memory for watchdog

[Nishanth Menon]

On 17:51-20230713, huaqian.li@siemens.com wrote:
> From: Li Hua Qian <huaqian.li@siemens.com>

I guess I should be explicit about this: Lets keep this dts patch as
"DONOTMERGE" in subject line for driver subsystem maintainer (I don't
want a repeat of cpufreq maintainers picking up dts and associated
warnings that are now pending fixes), resubmit at next rc1 and I can
queue up the dts once the maintainers pick up the driver and bindings.

Ref: https://lore.kernel.org/all/20230714084725.27847-1-krzysztof.kozlowski@linaro.org/

> 
> This patch adds a reserved memory for the TI AM65X platform watchdog to
> reserve the specific info, triggering the watchdog reset in last boot,
> to know if the board reboot is due to a watchdog reset.
> 
> Signed-off-by: Li Hua Qian <huaqian.li@siemens.com>
> ---
>  arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
> index e26bd988e522..4bb20d493651 100644
> --- a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
> @@ -63,6 +63,12 @@ rtos_ipc_memory_region: ipc-memories@a2000000 {
>  			alignment = <0x1000>;
>  			no-map;
>  		};
> +
> +		/* To reserve the power-on(PON) reason for watchdog reset */
> +		wdt_reset_memory_region: wdt-memory@a2200000 {
> +			reg = <0x00 0xa2200000 0x00 0x1000>;
> +			no-map;
> +		};
>  	};
>  
>  	leds {
> @@ -718,3 +724,7 @@ &mcu_r5fss0_core1 {
>  			<&mcu_r5fss0_core1_memory_region>;
>  	mboxes = <&mailbox0_cluster1>, <&mbox_mcu_r5fss0_core1>;
>  };
> +
> +&mcu_rti1 {
> +	memory-region = <&wdt_reset_memory_region>;
> +};
> -- 
> 2.34.1
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH v3 2/3] arm64: dts: ti: Add reserved memory for watchdog

[Guenter Roeck]

On 7/14/23 15:52, Nishanth Menon wrote:
> On 17:51-20230713, huaqian.li@siemens.com wrote:
>> From: Li Hua Qian <huaqian.li@siemens.com>
> 
> I guess I should be explicit about this: Lets keep this dts patch as
> "DONOTMERGE" in subject line for driver subsystem maintainer (I don't
> want a repeat of cpufreq maintainers picking up dts and associated
> warnings that are now pending fixes), resubmit at next rc1 and I can
> queue up the dts once the maintainers pick up the driver and bindings.
> 

FWIW, I never pick up patches outside drivers/hwmon without explicit Ack
from responsible maintainers (and most definitely not dts patches unless
a maintainer responsible for the file(s) specifically asks me to do it
(which I think never happened).

Guenter

> Ref: https://lore.kernel.org/all/20230714084725.27847-1-krzysztof.kozlowski@linaro.org/
> 
>>
>> This patch adds a reserved memory for the TI AM65X platform watchdog to
>> reserve the specific info, triggering the watchdog reset in last boot,
>> to know if the board reboot is due to a watchdog reset.
>>
>> Signed-off-by: Li Hua Qian <huaqian.li@siemens.com>
>> ---
>>   arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
>> index e26bd988e522..4bb20d493651 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
>> +++ b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
>> @@ -63,6 +63,12 @@ rtos_ipc_memory_region: ipc-memories@a2000000 {
>>   			alignment = <0x1000>;
>>   			no-map;
>>   		};
>> +
>> +		/* To reserve the power-on(PON) reason for watchdog reset */
>> +		wdt_reset_memory_region: wdt-memory@a2200000 {
>> +			reg = <0x00 0xa2200000 0x00 0x1000>;
>> +			no-map;
>> +		};
>>   	};
>>   
>>   	leds {
>> @@ -718,3 +724,7 @@ &mcu_r5fss0_core1 {
>>   			<&mcu_r5fss0_core1_memory_region>;
>>   	mboxes = <&mailbox0_cluster1>, <&mbox_mcu_r5fss0_core1>;
>>   };
>> +
>> +&mcu_rti1 {
>> +	memory-region = <&wdt_reset_memory_region>;
>> +};
>> -- 
>> 2.34.1
>>
> 

Re: [PATCH v3 2/3] arm64: dts: ti: Add reserved memory for watchdog

[Nishanth Menon]

On 16:27-20230714, Guenter Roeck wrote:
> On 7/14/23 15:52, Nishanth Menon wrote:
> > On 17:51-20230713, huaqian.li@siemens.com wrote:
> > > From: Li Hua Qian <huaqian.li@siemens.com>
> > 
> > I guess I should be explicit about this: Lets keep this dts patch as
> > "DONOTMERGE" in subject line for driver subsystem maintainer (I don't
> > want a repeat of cpufreq maintainers picking up dts and associated
> > warnings that are now pending fixes), resubmit at next rc1 and I can
> > queue up the dts once the maintainers pick up the driver and bindings.
> > 
> 
> FWIW, I never pick up patches outside drivers/hwmon without explicit Ack
> from responsible maintainers (and most definitely not dts patches unless
> a maintainer responsible for the file(s) specifically asks me to do it
> (which I think never happened).

Oops Guenter, didn't mean to indicate your tree :( -> just wanted
to ensure that developers are clearly aware of what happens with
cross posting series involving multiple maintainers and level set
their expectations. Saves a bunch of private pings later on :)

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

RE: [PATCH v3 2/3] arm64: dts: ti: Add reserved memory for watchdog

[Li, Hua Qian]

Hi Nishanth,

The maintainers have picked up the driver and bindings, as follows:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=29057cc5bddc785ea0a11534d7ad2546fa0872d3

Do you have time to work on the "DONOTMERGE" dts patch?

-----Original Message-----
From: Nishanth Menon <nm@ti.com>
Sent: Saturday, July 15, 2023 6:53 AM
To: Li, Hua Qian (DI FA CTR IPC CN PRC4) <HuaQian.Li@siemens.com>
Cc: wim@linux-watchdog.org; linux@roeck-us.net; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; huaqianlee@gmail.com; vigneshr@ti.com; kristo@kernel.org; linux-watchdog@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Kiszka, Jan (T CED) <jan.kiszka@siemens.com>; Su, Bao Cheng (DI FA CTR IPC CN PRC4) <baocheng.su@siemens.com>
Subject: Re: [PATCH v3 2/3] arm64: dts: ti: Add reserved memory for watchdog

On 17:51-20230713, huaqian.li@siemens.com wrote:
> From: Li Hua Qian <huaqian.li@siemens.com>

I guess I should be explicit about this: Lets keep this dts patch as "DONOTMERGE" in subject line for driver subsystem maintainer (I don't want a repeat of cpufreq maintainers picking up dts and associated warnings that are now pending fixes), resubmit at next rc1 and I can queue up the dts once the maintainers pick up the driver and bindings.

Ref: https://lore.kernel.org/all/20230714084725.27847-1-krzysztof.kozlowski@linaro.org/

>
> This patch adds a reserved memory for the TI AM65X platform watchdog
> to reserve the specific info, triggering the watchdog reset in last
> boot, to know if the board reboot is due to a watchdog reset.
>
> Signed-off-by: Li Hua Qian <huaqian.li@siemens.com>
> ---
>  arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
> b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
> index e26bd988e522..4bb20d493651 100644
> --- a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
> @@ -63,6 +63,12 @@ rtos_ipc_memory_region: ipc-memories@a2000000 {
>                       alignment = <0x1000>;
>                       no-map;
>               };
> +
> +             /* To reserve the power-on(PON) reason for watchdog reset */
> +             wdt_reset_memory_region: wdt-memory@a2200000 {
> +                     reg = <0x00 0xa2200000 0x00 0x1000>;
> +                     no-map;
> +             };
>       };
>
>       leds {
> @@ -718,3 +724,7 @@ &mcu_r5fss0_core1 {
>                       <&mcu_r5fss0_core1_memory_region>;
>       mboxes = <&mailbox0_cluster1>, <&mbox_mcu_r5fss0_core1>;  };
> +
> +&mcu_rti1 {
> +     memory-region = <&wdt_reset_memory_region>; };
> --
> 2.34.1
>

--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH v3 2/3] arm64: dts: ti: Add reserved memory for watchdog

[Nishanth Menon]

On 08:16-20240108, Li, Hua Qian wrote:
> Hi Nishanth,
> 
> The maintainers have picked up the driver and bindings, as follows:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=29057cc5bddc785ea0a11534d7ad2546fa0872d3

Not sure what you want us to look at here.

> 
> Do you have time to work on the "DONOTMERGE" dts patch?

If the binding has hit master. please resubmit from the closest rc1 - at
this time, see if you can rebase on linux-next or wait till 6.8-rc1 is
tagged to rebase and re-submit.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

[PATCH v3 3/3] watchdog:rit_wdt: Add support for WDIOF_CARDRESET

[huaqian.li]

From: Li Hua Qian <huaqian.li@siemens.com>

This patch adds the WDIOF_CARDRESET support for the platform watchdog
whose hardware does not support this feature, to know if the board
reboot is due to a watchdog reset.

This is done via reserved memory(RAM), which indicates if specific
info saved, triggering the watchdog reset in last boot.

Signed-off-by: Li Hua Qian <huaqian.li@siemens.com>
---
 drivers/watchdog/rti_wdt.c | 51 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index ce8f18e93aa9..b9435b972cb9 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -18,6 +18,8 @@
 #include <linux/pm_runtime.h>
 #include <linux/types.h>
 #include <linux/watchdog.h>
+#include <linux/of_address.h>
+#include <linux/of.h>
 
 #define DEFAULT_HEARTBEAT 60
 
@@ -52,6 +54,11 @@
 
 #define DWDST			BIT(1)
 
+#define PON_REASON_SOF_NUM	0xBBBBCCCC
+#define PON_REASON_MAGIC_NUM	0xDDDDDDDD
+#define PON_REASON_EOF_NUM	0xCCCCBBBB
+#define RESERVED_MEM_MIN_SIZE	12
+
 static int heartbeat = DEFAULT_HEARTBEAT;
 
 /*
@@ -198,6 +205,11 @@ static int rti_wdt_probe(struct platform_device *pdev)
 	struct rti_wdt_device *wdt;
 	struct clk *clk;
 	u32 last_ping = 0;
+	struct device_node *node;
+	u32 reserved_mem_size;
+	struct resource res;
+	u32 *vaddr;
+	u64 paddr;
 
 	wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
 	if (!wdt)
@@ -284,6 +296,45 @@ static int rti_wdt_probe(struct platform_device *pdev)
 		}
 	}
 
+	node = of_parse_phandle(pdev->dev.of_node, "memory-region", 0);
+	if (!node) {
+		dev_dbg(dev, "No memory-region specified.\n");
+	} else {
+		ret = of_address_to_resource(node, 0, &res);
+		if (ret) {
+			dev_err(dev, "No memory address assigned to the region.\n");
+			goto err_iomap;
+		}
+
+		/*
+		 * If reserved memory is defined for watchdog reset cause.
+		 * Readout the Power-on(PON) reason and pass to bootstatus.
+		 */
+		paddr = res.start;
+		reserved_mem_size = res.end - (res.start - 1);
+		if (reserved_mem_size < RESERVED_MEM_MIN_SIZE) {
+			dev_err(dev, "The size of reserved memory is too small.\n");
+			ret = -EINVAL;
+			goto err_iomap;
+		}
+
+		vaddr = memremap(paddr, reserved_mem_size, MEMREMAP_WB);
+		if (vaddr == NULL) {
+			dev_err(dev, "Failed to map memory-region.\n");
+			ret = -ENOMEM;
+			goto err_iomap;
+		}
+
+		if (vaddr[0] == PON_REASON_SOF_NUM &&
+		    vaddr[1] == PON_REASON_MAGIC_NUM &&
+		    vaddr[2] == PON_REASON_EOF_NUM) {
+			dev_dbg(dev, "Watchdog reset cause detected.\n");
+			wdd->bootstatus |= WDIOF_CARDRESET;
+		}
+		memset(vaddr, 0, reserved_mem_size);
+		memunmap(vaddr);
+	}
+
 	watchdog_init_timeout(wdd, heartbeat, dev);
 
 	ret = watchdog_register_device(wdd);
-- 
2.34.1

Re: [PATCH v3 3/3] watchdog:rit_wdt: Add support for WDIOF_CARDRESET

[Guenter Roeck]

On 7/13/23 02:51, huaqian.li@siemens.com wrote:
> From: Li Hua Qian <huaqian.li@siemens.com>
> 
> This patch adds the WDIOF_CARDRESET support for the platform watchdog
> whose hardware does not support this feature, to know if the board
> reboot is due to a watchdog reset.
> 
> This is done via reserved memory(RAM), which indicates if specific
> info saved, triggering the watchdog reset in last boot.
> 
> Signed-off-by: Li Hua Qian <huaqian.li@siemens.com>
> ---
>   drivers/watchdog/rti_wdt.c | 51 ++++++++++++++++++++++++++++++++++++++
>   1 file changed, 51 insertions(+)
> 
> diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
> index ce8f18e93aa9..b9435b972cb9 100644
> --- a/drivers/watchdog/rti_wdt.c
> +++ b/drivers/watchdog/rti_wdt.c
> @@ -18,6 +18,8 @@
>   #include <linux/pm_runtime.h>
>   #include <linux/types.h>
>   #include <linux/watchdog.h>
> +#include <linux/of_address.h>
> +#include <linux/of.h>
>   

This driver so far managed to keep include files in alphabetic order.
Please keep it that way.

>   #define DEFAULT_HEARTBEAT 60
>   
> @@ -52,6 +54,11 @@
>   
>   #define DWDST			BIT(1)
>   
> +#define PON_REASON_SOF_NUM	0xBBBBCCCC
> +#define PON_REASON_MAGIC_NUM	0xDDDDDDDD
> +#define PON_REASON_EOF_NUM	0xCCCCBBBB
> +#define RESERVED_MEM_MIN_SIZE	12
> +
>   static int heartbeat = DEFAULT_HEARTBEAT;
>   
>   /*
> @@ -198,6 +205,11 @@ static int rti_wdt_probe(struct platform_device *pdev)
>   	struct rti_wdt_device *wdt;
>   	struct clk *clk;
>   	u32 last_ping = 0;
> +	struct device_node *node;
> +	u32 reserved_mem_size;
> +	struct resource res;
> +	u32 *vaddr;
> +	u64 paddr;
>   
>   	wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
>   	if (!wdt)
> @@ -284,6 +296,45 @@ static int rti_wdt_probe(struct platform_device *pdev)
>   		}
>   	}
>   
> +	node = of_parse_phandle(pdev->dev.of_node, "memory-region", 0);
> +	if (!node) {
> +		dev_dbg(dev, "No memory-region specified.\n");

If you really want that debug message, please keep the action part
first. I personally think it is just noise; the devicetree can always
be looked up if needed.

> +	} else {
> +		ret = of_address_to_resource(node, 0, &res);
> +		if (ret) {
> +			dev_err(dev, "No memory address assigned to the region.\n");
> +			goto err_iomap;
> +		}
> +
> +		/*
> +		 * If reserved memory is defined for watchdog reset cause.
> +		 * Readout the Power-on(PON) reason and pass to bootstatus.
> +		 */
> +		paddr = res.start;
> +		reserved_mem_size = res.end - (res.start - 1);

Please use resource_size().

> +		if (reserved_mem_size < RESERVED_MEM_MIN_SIZE) {
> +			dev_err(dev, "The size of reserved memory is too small.\n");
> +			ret = -EINVAL;
> +			goto err_iomap;
> +		}
> +
> +		vaddr = memremap(paddr, reserved_mem_size, MEMREMAP_WB);
> +		if (vaddr == NULL) {
> +			dev_err(dev, "Failed to map memory-region.\n");
> +			ret = -ENOMEM;
> +			goto err_iomap;
> +		}
> +
> +		if (vaddr[0] == PON_REASON_SOF_NUM &&
> +		    vaddr[1] == PON_REASON_MAGIC_NUM &&
> +		    vaddr[2] == PON_REASON_EOF_NUM) {
> +			dev_dbg(dev, "Watchdog reset cause detected.\n");

Isn't that a bit pointless ? That is obvious when reading the boot status.

> +			wdd->bootstatus |= WDIOF_CARDRESET;
> +		}
> +		memset(vaddr, 0, reserved_mem_size);
> +		memunmap(vaddr);
> +	}
> +
>   	watchdog_init_timeout(wdd, heartbeat, dev);
>   
>   	ret = watchdog_register_device(wdd);

Re: [PATCH v3 0/3] Add support for WDIOF_CARDRESET on TI AM65x

[Guenter Roeck]

On 7/13/23 02:51, huaqian.li@siemens.com wrote:
> From: Li Hua Qian <huaqian.li@siemens.com>
> 
> 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.
> 

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

> 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/20230711091713.1113010-1-huaqian.li@siemens.com
> 
> 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/3137d87e56ef75ba0b8a923d407b2fecace6ccbd.camel@siemens.com/
>    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         | 12 +++++
>   .../boot/dts/ti/k3-am65-iot2050-common.dtsi   | 10 ++++
>   drivers/watchdog/rti_wdt.c                    | 51 +++++++++++++++++++
>   3 files changed, 73 insertions(+)
> 

Re: [PATCH v3 0/3] Add support for WDIOF_CARDRESET on TI AM65x

[Li, Hua Qian]

On Thu, 2023-07-13 at 10:21 -0700, Guenter Roeck wrote:
> On 7/13/23 02:51, huaqian.li@siemens.com wrote:
> > From: Li Hua Qian <huaqian.li@siemens.com>
> > 
> > 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.
> > 
> 
> 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

Could you help to suggest how to handle it? 

I am not sure where is a good place to write the usage note. I am
thinking to add it in v4 to DT binding.

Best regards,
Li Hua Qian
> 
> > 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/20230711091713.1113010-1-huaqian.li@siemens.com
> > 
> > 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/3137d87e56ef75ba0b8a923d407b2fecace6ccbd.camel@siemens.com/
> >    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         | 12 +++++
> >   .../boot/dts/ti/k3-am65-iot2050-common.dtsi   | 10 ++++
> >   drivers/watchdog/rti_wdt.c                    | 51
> > +++++++++++++++++++
> >   3 files changed, 73 insertions(+)
> > 
>