[PATCH v4 0/5] Add support to read the watchdog bootstatus from IMEM

[PATCH v4 0/5] Add support to read the watchdog bootstatus from IMEM

[Kathiravan Thirumoorthy]

In Qualcomm IPQ SoCs, if the system is rebooted due to the watchdog
timeout, there is no way to identify it. Current approach of checking
the EXPIRED_STATUS in WDT_STS is not working.

To achieve this, if the system is rebooted due to watchdog timeout, the
information is captured in the IMEM by the bootloader (along with other
reason codes as well).

This series attempts to address this by adding the support to read the
IMEM and populate the information via bootstatus sysfs file.

With the CONFIG_WATCHDOG_SYSFS enabled, user can extract the information
as below:

cat /sys/devices/platform/soc@0/f410000.watchdog/watchdog/watchdog0/bootstatus
32

Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
---
Changes in v4:
- Kept only the WDIOF_CARDRESET and dropped other codes (Guenter)
- Renamed qcom_wdt_get_restart_reason() to qcom_wdt_get_bootstatus()
- Dropped the device data and describe the required information in the DT (Konrad)
- Link to v3:
  https://lore.kernel.org/linux-arm-msm/20250502-wdt_reset_reason-v3-0-b2dc7ace38ca@oss.qualcomm.com/

Changes in v3:
- Picked up the relevant tags
- Dropped the fallback compatible handling
- Split the driver changes into 2. Introduce the device data in one and
  extend the same in another for the use case
- Link to v2:
  https://lore.kernel.org/linux-arm-msm/20250416-wdt_reset_reason-v2-0-c65bba312914@oss.qualcomm.com/

Changes in v2:
- Dropped the RFC tag
- Reworked the driver changes to use the syscon API
- Link to v1:
  https://lore.kernel.org/linux-arm-msm/20250408-wdt_reset_reason-v1-0-e6ec30c2c926@oss.qualcomm.com/

---
Kathiravan Thirumoorthy (5):
      dt-bindings: sram: qcom,imem: Document IPQ5424 compatible
      arm64: dts: qcom: ipq5424: Add the IMEM node
      dt-bindings: watchdog: qcom-wdt: Document qcom,imem property
      watchdog: qcom: add support to get the bootstatus from IMEM
      arm64: dts: qcom: ipq5424: add support to get watchdog bootstatus from IMEM

 .../devicetree/bindings/sram/qcom,imem.yaml        |  1 +
 .../devicetree/bindings/watchdog/qcom-wdt.yaml     | 20 +++++++++++
 arch/arm64/boot/dts/qcom/ipq5424.dtsi              | 10 ++++++
 drivers/watchdog/qcom-wdt.c                        | 42 ++++++++++++++++++++--
 4 files changed, 71 insertions(+), 2 deletions(-)
---
base-commit: 8566fc3b96539e3235909d6bdda198e1282beaed
change-id: 20250408-wdt_reset_reason-e12921963fa6

Best regards,
-- 
Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>

[PATCH v4 1/5] dt-bindings: sram: qcom,imem: Document IPQ5424 compatible

[Kathiravan Thirumoorthy]

Add compatible for Qualcomm's IPQ5424 IMEM.

Acked-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
---
Changes in v4:
	- No changes
Changes in v3:
	- Picked up the A-b tag
---
 Documentation/devicetree/bindings/sram/qcom,imem.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/sram/qcom,imem.yaml b/Documentation/devicetree/bindings/sram/qcom,imem.yaml
index 2711f90d9664b70fcd1e2f7e2dfd3386ed5c1952..dec1b1ee924cf1386f559eb262ea864f2788c165 100644
--- a/Documentation/devicetree/bindings/sram/qcom,imem.yaml
+++ b/Documentation/devicetree/bindings/sram/qcom,imem.yaml
@@ -18,6 +18,7 @@ properties:
     items:
       - enum:
           - qcom,apq8064-imem
+          - qcom,ipq5424-imem
           - qcom,msm8226-imem
           - qcom,msm8974-imem
           - qcom,msm8976-imem

-- 
2.34.1

[PATCH v4 2/5] arm64: dts: qcom: ipq5424: Add the IMEM node

[Kathiravan Thirumoorthy]

Add the IMEM node to the device tree to extract debugging information
like system restart reason, which is populated via IMEM. Define the
IMEM region to enable this functionality.

As described, overall IMEM region is 112KB but only initial 4KB is
accessible by all masters in the SoC.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
---
Changes in v4:
	- No changes
Changes in v3:
	- Picked up the R-b tag
Changes in v2:
	- Describe the entire IMEM region in the node
	- Explicitly call out that initial 4K only accessible by all
	  masters in the commit message
---
 arch/arm64/boot/dts/qcom/ipq5424.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq5424.dtsi b/arch/arm64/boot/dts/qcom/ipq5424.dtsi
index 5d6ed2172b1bb0a57c593f121f387ec917f42419..4f18ea79502738c2b9cb4b13e8eb4ac4ddd89adf 100644
--- a/arch/arm64/boot/dts/qcom/ipq5424.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5424.dtsi
@@ -486,6 +486,15 @@ ssphy_0: phy@7d000 {
 			status = "disabled";
 		};
 
+		sram@8600000 {
+			compatible = "qcom,ipq5424-imem", "syscon", "simple-mfd";
+			reg = <0 0x08600000 0 0x1c000>;
+			ranges = <0 0 0x08600000 0x1c000>;
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+		};
+
 		usb3: usb3@8a00000 {
 			compatible = "qcom,ipq5424-dwc3", "qcom,dwc3";
 			reg = <0 0x08af8800 0 0x400>;

-- 
2.34.1

[PATCH v4 3/5] dt-bindings: watchdog: qcom-wdt: Document qcom,imem property

[Kathiravan Thirumoorthy]

Document the "qcom,imem" property for the watchdog device on Qualcomm
IPQ platforms. Use this property to extract the restart reason from
IMEM, which is updated by XBL. Populate the watchdog's bootstatus sysFS
entry with this information, when the system reboots due to a watchdog
timeout.

Describe this property for the IPQ5424 watchdog device and extend support
to other targets subsequently.

Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
---
Changes in v4:
	- New patch
---
 .../devicetree/bindings/watchdog/qcom-wdt.yaml       | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
index 49e2b807db0bc9d3edfc93ec41ad0df0b74ed032..bbe9b68ff4c8b813744ffd86bb52303943366fa2 100644
--- a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
+++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
@@ -81,6 +81,16 @@ properties:
     minItems: 1
     maxItems: 5
 
+  qcom,imem:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description:
+      phandle to the IMEM syscon node that exposes the system restart reason
+    items:
+      - items:
+          - description: phandle of IMEM syscon
+          - description: offset of restart reason region
+          - description: value indicate that the watchdog timeout has occurred
+
 required:
   - compatible
   - reg
@@ -117,6 +127,16 @@ allOf:
       required:
         - clock-frequency
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,apss-wdt-ipq5424
+    then:
+      required:
+        - qcom,imem
+
 unevaluatedProperties: false
 
 examples:

-- 
2.34.1

[PATCH v4 4/5] watchdog: qcom: add support to get the bootstatus from IMEM

[Kathiravan Thirumoorthy]

When the system boots up after a watchdog reset, the EXPIRED_STATUS bit
in the WDT_STS register is cleared. To identify if the system was
restarted due to WDT expiry, XBL update the information in the IMEM region.
Update the driver to read the restart reason from IMEM and populate the
bootstatus accordingly.

With the CONFIG_WATCHDOG_SYSFS enabled, user can extract the information
as below:

cat /sys/devices/platform/soc@0/f410000.watchdog/watchdog/watchdog0/bootstatus
32

For backward compatibility, keep the EXPIRED_STATUS bit check. Add a new
function qcom_wdt_get_bootstatus() to read the restart reason from
IMEM.

Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
---
Changes in v4:
	- Kept only WDIOF_CARDRESET and dropped other codes
	- Renamed qcom_wdt_get_reason_reason() to qcom_wdt_get_bootstatus()
	- Moved the existing check inside qcom_wdt_get_bootstatus()
	- Dropped the device data and put all the details in the DT node
Changes in v3:
	- Split the introduction of device data into separate patch
	- s/bootloaders/XBL - for clarity of which bootloader is
	  involved
	- Mention the sysfs path on to extract this information
	- s/compatible/imem_compatible in the device data structure to
	  avoid the confusion / better naming
Changes in v2:
	- Use the syscon API to access the IMEM region
	- Handle the error cases returned by qcom_wdt_get_restart_reason
	- Define device specific data to retrieve the IMEM compatible,
	  offset and the value for non secure WDT, which allows to
	  extend the support for other SoCs
---
 drivers/watchdog/qcom-wdt.c | 42 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
index 006f9c61aa64fd2b4ee9db493aeb54c8fafac818..adbf21ae2ef15ae2faec57c6ec01c38924bc4955 100644
--- a/drivers/watchdog/qcom-wdt.c
+++ b/drivers/watchdog/qcom-wdt.c
@@ -7,9 +7,11 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/watchdog.h>
 
 enum wdt_reg {
@@ -187,6 +189,41 @@ static const struct qcom_wdt_match_data match_data_kpss = {
 	.max_tick_count = 0xFFFFFU,
 };
 
+static int qcom_wdt_get_bootstatus(struct device *dev, struct qcom_wdt *wdt)
+{
+	unsigned int args[2];
+	struct regmap *imem;
+	unsigned int val;
+	int ret;
+
+	imem = syscon_regmap_lookup_by_phandle_args(dev->of_node, "qcom,imem",
+						    ARRAY_SIZE(args), args);
+	if (IS_ERR(imem)) {
+		ret = PTR_ERR(imem);
+		if (ret != -ENOENT) {
+			dev_err(dev, "failed to lookup syscon: %d\n", ret);
+			return ret;
+		}
+
+		/* Fallback to the existing check */
+		if (readl(wdt_addr(wdt, WDT_STS)) & 1)
+			wdt->wdd.bootstatus = WDIOF_CARDRESET;
+
+		return 0;
+	}
+
+	ret = regmap_read(imem, args[0], &val);
+	if (ret) {
+		dev_err(dev, "failed to read the restart reason info\n");
+		return ret;
+	}
+
+	if (val == args[1])
+		wdt->wdd.bootstatus = WDIOF_CARDRESET;
+
+	return 0;
+}
+
 static int qcom_wdt_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -267,8 +304,9 @@ static int qcom_wdt_probe(struct platform_device *pdev)
 	wdt->wdd.parent = dev;
 	wdt->layout = data->offset;
 
-	if (readl(wdt_addr(wdt, WDT_STS)) & 1)
-		wdt->wdd.bootstatus = WDIOF_CARDRESET;
+	ret = qcom_wdt_get_bootstatus(dev, wdt);
+	if (ret)
+		return ret;
 
 	/*
 	 * If 'timeout-sec' unspecified in devicetree, assume a 30 second

-- 
2.34.1

[PATCH v4 5/5] arm64: dts: qcom: ipq5424: add support to get watchdog bootstatus from IMEM

[Kathiravan Thirumoorthy]

Add the "qcom,imem" property to the watchdog device node to enable
retrieval of the system restart reason from IMEM, populated by XBL.
Parse this information in the watchdog driver and update the bootstatus
sysFS if the restart was triggered by a watchdog timeout.

Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
---
Changes in v4:
	- New patch
---
 arch/arm64/boot/dts/qcom/ipq5424.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq5424.dtsi b/arch/arm64/boot/dts/qcom/ipq5424.dtsi
index 4f18ea79502738c2b9cb4b13e8eb4ac4ddd89adf..8952d769dad4ec4635759d6eae8344aaf1d8a79e 100644
--- a/arch/arm64/boot/dts/qcom/ipq5424.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5424.dtsi
@@ -380,6 +380,7 @@ watchdog@f410000 {
 			reg = <0 0x0f410000 0 0x1000>;
 			interrupts = <GIC_SPI 0 IRQ_TYPE_EDGE_RISING>;
 			clocks = <&sleep_clk>;
+			qcom,imem = <&imem 0x7b0 0x5>;
 		};
 
 		qusb_phy_1: phy@71000 {
@@ -486,7 +487,7 @@ ssphy_0: phy@7d000 {
 			status = "disabled";
 		};
 
-		sram@8600000 {
+		imem: sram@8600000 {
 			compatible = "qcom,ipq5424-imem", "syscon", "simple-mfd";
 			reg = <0 0x08600000 0 0x1c000>;
 			ranges = <0 0 0x08600000 0x1c000>;

-- 
2.34.1

Re: [PATCH v4 3/5] dt-bindings: watchdog: qcom-wdt: Document qcom,imem property

[Krzysztof Kozlowski]

On Mon, May 19, 2025 at 02:04:03PM GMT, Kathiravan Thirumoorthy wrote:
> Document the "qcom,imem" property for the watchdog device on Qualcomm
> IPQ platforms. Use this property to extract the restart reason from
> IMEM, which is updated by XBL. Populate the watchdog's bootstatus sysFS
> entry with this information, when the system reboots due to a watchdog
> timeout.
> 
> Describe this property for the IPQ5424 watchdog device and extend support
> to other targets subsequently.
> 
> Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
> ---
> Changes in v4:
> 	- New patch
> ---
>  .../devicetree/bindings/watchdog/qcom-wdt.yaml       | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
> index 49e2b807db0bc9d3edfc93ec41ad0df0b74ed032..bbe9b68ff4c8b813744ffd86bb52303943366fa2 100644
> --- a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
> @@ -81,6 +81,16 @@ properties:
>      minItems: 1
>      maxItems: 5
>  
> +  qcom,imem:

Shoouldn't this be existing 'sram' property? If IMEM is something
similar to OCMEM, then we already use sram for that.


> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description:
> +      phandle to the IMEM syscon node that exposes the system restart reason
> +    items:
> +      - items:
> +          - description: phandle of IMEM syscon
> +          - description: offset of restart reason region
> +          - description: value indicate that the watchdog timeout has occurred
> +
>  required:
>    - compatible
>    - reg
> @@ -117,6 +127,16 @@ allOf:
>        required:
>          - clock-frequency
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,apss-wdt-ipq5424
> +    then:
> +      required:
> +        - qcom,imem

No, drop this. That's an ABI break.

Best regards,
Krzysztof

Re: [PATCH v4 4/5] watchdog: qcom: add support to get the bootstatus from IMEM

[Krzysztof Kozlowski]

On Mon, May 19, 2025 at 02:04:04PM GMT, Kathiravan Thirumoorthy wrote:
> +static int qcom_wdt_get_bootstatus(struct device *dev, struct qcom_wdt *wdt)
> +{
> +	unsigned int args[2];
> +	struct regmap *imem;
> +	unsigned int val;
> +	int ret;
> +
> +	imem = syscon_regmap_lookup_by_phandle_args(dev->of_node, "qcom,imem",
> +						    ARRAY_SIZE(args), args);
> +	if (IS_ERR(imem)) {
> +		ret = PTR_ERR(imem);
> +		if (ret != -ENOENT) {
> +			dev_err(dev, "failed to lookup syscon: %d\n", ret);

Syntax is: return dev_err_probe().

> +			return ret;

Best regards,
Krzysztof

Re: [PATCH v4 3/5] dt-bindings: watchdog: qcom-wdt: Document qcom,imem property

[Konrad Dybcio]

On 5/20/25 9:25 AM, Krzysztof Kozlowski wrote:
> On Mon, May 19, 2025 at 02:04:03PM GMT, Kathiravan Thirumoorthy wrote:
>> Document the "qcom,imem" property for the watchdog device on Qualcomm
>> IPQ platforms. Use this property to extract the restart reason from
>> IMEM, which is updated by XBL. Populate the watchdog's bootstatus sysFS
>> entry with this information, when the system reboots due to a watchdog
>> timeout.
>>
>> Describe this property for the IPQ5424 watchdog device and extend support
>> to other targets subsequently.
>>
>> Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
>> ---
>> Changes in v4:
>> 	- New patch
>> ---
>>  .../devicetree/bindings/watchdog/qcom-wdt.yaml       | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
>> index 49e2b807db0bc9d3edfc93ec41ad0df0b74ed032..bbe9b68ff4c8b813744ffd86bb52303943366fa2 100644
>> --- a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
>> +++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
>> @@ -81,6 +81,16 @@ properties:
>>      minItems: 1
>>      maxItems: 5
>>  
>> +  qcom,imem:
> 
> Shoouldn't this be existing 'sram' property? If IMEM is something
> similar to OCMEM, then we already use sram for that.

We specifically want a handle to a predefined byte in IMEM, something akin
to qcom,4ln-config-sel in

Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml

Konrad

Re: [PATCH v4 3/5] dt-bindings: watchdog: qcom-wdt: Document qcom,imem property

[Krzysztof Kozlowski]

On 20/05/2025 18:00, Konrad Dybcio wrote:
> On 5/20/25 9:25 AM, Krzysztof Kozlowski wrote:
>> On Mon, May 19, 2025 at 02:04:03PM GMT, Kathiravan Thirumoorthy wrote:
>>> Document the "qcom,imem" property for the watchdog device on Qualcomm
>>> IPQ platforms. Use this property to extract the restart reason from
>>> IMEM, which is updated by XBL. Populate the watchdog's bootstatus sysFS
>>> entry with this information, when the system reboots due to a watchdog
>>> timeout.
>>>
>>> Describe this property for the IPQ5424 watchdog device and extend support
>>> to other targets subsequently.
>>>
>>> Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
>>> ---
>>> Changes in v4:
>>> 	- New patch
>>> ---
>>>  .../devicetree/bindings/watchdog/qcom-wdt.yaml       | 20 ++++++++++++++++++++
>>>  1 file changed, 20 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
>>> index 49e2b807db0bc9d3edfc93ec41ad0df0b74ed032..bbe9b68ff4c8b813744ffd86bb52303943366fa2 100644
>>> --- a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
>>> +++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
>>> @@ -81,6 +81,16 @@ properties:
>>>      minItems: 1
>>>      maxItems: 5
>>>  
>>> +  qcom,imem:
>>
>> Shoouldn't this be existing 'sram' property? If IMEM is something
>> similar to OCMEM, then we already use sram for that.
> 
> We specifically want a handle to a predefined byte in IMEM, something akin
> to qcom,4ln-config-sel in
> 
> Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml

Nothing stops that with sram. Above example is poor, because it mentions
syscon. There is no hardware as syscon. Does not exist. What is IMEM
here, what is this relationship?

Best regards,
Krzysztof

Re: [PATCH v4 3/5] dt-bindings: watchdog: qcom-wdt: Document qcom,imem property

[Konrad Dybcio]

On 5/21/25 8:53 AM, Krzysztof Kozlowski wrote:
> On 20/05/2025 18:00, Konrad Dybcio wrote:
>> On 5/20/25 9:25 AM, Krzysztof Kozlowski wrote:
>>> On Mon, May 19, 2025 at 02:04:03PM GMT, Kathiravan Thirumoorthy wrote:
>>>> Document the "qcom,imem" property for the watchdog device on Qualcomm
>>>> IPQ platforms. Use this property to extract the restart reason from
>>>> IMEM, which is updated by XBL. Populate the watchdog's bootstatus sysFS
>>>> entry with this information, when the system reboots due to a watchdog
>>>> timeout.
>>>>
>>>> Describe this property for the IPQ5424 watchdog device and extend support
>>>> to other targets subsequently.
>>>>
>>>> Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
>>>> ---
>>>> Changes in v4:
>>>> 	- New patch
>>>> ---
>>>>  .../devicetree/bindings/watchdog/qcom-wdt.yaml       | 20 ++++++++++++++++++++
>>>>  1 file changed, 20 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
>>>> index 49e2b807db0bc9d3edfc93ec41ad0df0b74ed032..bbe9b68ff4c8b813744ffd86bb52303943366fa2 100644
>>>> --- a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
>>>> +++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
>>>> @@ -81,6 +81,16 @@ properties:
>>>>      minItems: 1
>>>>      maxItems: 5
>>>>  
>>>> +  qcom,imem:
>>>
>>> Shoouldn't this be existing 'sram' property? If IMEM is something
>>> similar to OCMEM, then we already use sram for that.
>>
>> We specifically want a handle to a predefined byte in IMEM, something akin
>> to qcom,4ln-config-sel in
>>
>> Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
> 
> Nothing stops that with sram. Above example is poor, because it mentions
> syscon. There is no hardware as syscon. Does not exist. What is IMEM
> here, what is this relationship?

IMEM is indeed a small block of on-die SRAM. In this context, another subsystem
may write a magic value at a known offset that would correspond to the platform
having been rebooted by the watchdog. Now why the wdt register is cleared in the
first place, I have no clue.

Konrad

Re: [PATCH v4 3/5] dt-bindings: watchdog: qcom-wdt: Document qcom,imem property

[Kathiravan Thirumoorthy]

On 5/22/2025 9:15 PM, Konrad Dybcio wrote:
> On 5/21/25 8:53 AM, Krzysztof Kozlowski wrote:
>> On 20/05/2025 18:00, Konrad Dybcio wrote:
>>> On 5/20/25 9:25 AM, Krzysztof Kozlowski wrote:
>>>> On Mon, May 19, 2025 at 02:04:03PM GMT, Kathiravan Thirumoorthy wrote:
>>>>> Document the "qcom,imem" property for the watchdog device on Qualcomm
>>>>> IPQ platforms. Use this property to extract the restart reason from
>>>>> IMEM, which is updated by XBL. Populate the watchdog's bootstatus sysFS
>>>>> entry with this information, when the system reboots due to a watchdog
>>>>> timeout.
>>>>>
>>>>> Describe this property for the IPQ5424 watchdog device and extend support
>>>>> to other targets subsequently.
>>>>>
>>>>> Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
>>>>> ---
>>>>> Changes in v4:
>>>>> 	- New patch
>>>>> ---
>>>>>   .../devicetree/bindings/watchdog/qcom-wdt.yaml       | 20 ++++++++++++++++++++
>>>>>   1 file changed, 20 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
>>>>> index 49e2b807db0bc9d3edfc93ec41ad0df0b74ed032..bbe9b68ff4c8b813744ffd86bb52303943366fa2 100644
>>>>> --- a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
>>>>> +++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
>>>>> @@ -81,6 +81,16 @@ properties:
>>>>>       minItems: 1
>>>>>       maxItems: 5
>>>>>   
>>>>> +  qcom,imem:
>>>> Shoouldn't this be existing 'sram' property? If IMEM is something
>>>> similar to OCMEM, then we already use sram for that.
>>> We specifically want a handle to a predefined byte in IMEM, something akin
>>> to qcom,4ln-config-sel in
>>>
>>> Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
>> Nothing stops that with sram. Above example is poor, because it mentions
>> syscon. There is no hardware as syscon. Does not exist. What is IMEM
>> here, what is this relationship?
> IMEM is indeed a small block of on-die SRAM. In this context, another subsystem
> may write a magic value at a known offset that would correspond to the platform
> having been rebooted by the watchdog. Now why the wdt register is cleared in the
> first place, I have no clue.


Thanks, Konrad for chiming in and providing the background information. 
With respect to the WDT register, when the interrupt is triggered, I see 
the expire bit is set in the watchdog register. The bite interrupt is 
handled by TZ and TZ does the system reboot. After the system reboots, 
bit is cleared. I have cross checked with the design team and they 
confirmed that the behavior is expected one.

Krzysztof, Based on the discussions from the previous versions, I have 
made the changes. Can you help to guide me on how to handle this? Should 
I just name the property as "sram" and point to the sub block in the 
IMEM region like how it is done at [1][2], which is more or like similar 
to what I have submitted in V1 of this series[3] Or is the current 
approach acceptable? Or some other way to handle this?

[1] 
https://lore.kernel.org/linux-arm-msm/20250523-topic-ipa_imem-v1-1-b5d536291c7f@oss.qualcomm.com/T/#u

[2] 
https://lore.kernel.org/linux-arm-msm/20250523-topic-ipa_imem-v1-2-b5d536291c7f@oss.qualcomm.com/T/#u

[3] 
https://lore.kernel.org/linux-arm-msm/20250408-wdt_reset_reason-v1-0-e6ec30c2c926@oss.qualcomm.com/


>
> Konrad

Re: [PATCH v4 3/5] dt-bindings: watchdog: qcom-wdt: Document qcom,imem property

[Konrad Dybcio]

On 5/23/25 4:35 PM, Kathiravan Thirumoorthy wrote:
> 
> On 5/22/2025 9:15 PM, Konrad Dybcio wrote:
>> On 5/21/25 8:53 AM, Krzysztof Kozlowski wrote:
>>> On 20/05/2025 18:00, Konrad Dybcio wrote:
>>>> On 5/20/25 9:25 AM, Krzysztof Kozlowski wrote:
>>>>> On Mon, May 19, 2025 at 02:04:03PM GMT, Kathiravan Thirumoorthy wrote:
>>>>>> Document the "qcom,imem" property for the watchdog device on Qualcomm
>>>>>> IPQ platforms. Use this property to extract the restart reason from
>>>>>> IMEM, which is updated by XBL. Populate the watchdog's bootstatus sysFS
>>>>>> entry with this information, when the system reboots due to a watchdog
>>>>>> timeout.
>>>>>>
>>>>>> Describe this property for the IPQ5424 watchdog device and extend support
>>>>>> to other targets subsequently.
>>>>>>
>>>>>> Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
>>>>>> ---
>>>>>> Changes in v4:
>>>>>>     - New patch
>>>>>> ---
>>>>>>   .../devicetree/bindings/watchdog/qcom-wdt.yaml       | 20 ++++++++++++++++++++
>>>>>>   1 file changed, 20 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
>>>>>> index 49e2b807db0bc9d3edfc93ec41ad0df0b74ed032..bbe9b68ff4c8b813744ffd86bb52303943366fa2 100644
>>>>>> --- a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
>>>>>> @@ -81,6 +81,16 @@ properties:
>>>>>>       minItems: 1
>>>>>>       maxItems: 5
>>>>>>   +  qcom,imem:
>>>>> Shoouldn't this be existing 'sram' property? If IMEM is something
>>>>> similar to OCMEM, then we already use sram for that.
>>>> We specifically want a handle to a predefined byte in IMEM, something akin
>>>> to qcom,4ln-config-sel in
>>>>
>>>> Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
>>> Nothing stops that with sram. Above example is poor, because it mentions
>>> syscon. There is no hardware as syscon. Does not exist. What is IMEM
>>> here, what is this relationship?
>> IMEM is indeed a small block of on-die SRAM. In this context, another subsystem
>> may write a magic value at a known offset that would correspond to the platform
>> having been rebooted by the watchdog. Now why the wdt register is cleared in the
>> first place, I have no clue.
> 
> 
> Thanks, Konrad for chiming in and providing the background information. With respect to the WDT register, when the interrupt is triggered, I see the expire bit is set in the watchdog register. The bite interrupt is handled by TZ and TZ does the system reboot. After the system reboots, bit is cleared. I have cross checked with the design team and they confirmed that the behavior is expected one.
> 
> Krzysztof, Based on the discussions from the previous versions, I have made the changes. Can you help to guide me on how to handle this? Should I just name the property as "sram" and point to the sub block in the IMEM region like how it is done at [1][2], which is more or like similar to what I have submitted in V1 of this series[3] Or is the current approach acceptable? Or some other way to handle this?
> 
> [1] https://lore.kernel.org/linux-arm-msm/20250523-topic-ipa_imem-v1-1-b5d536291c7f@oss.qualcomm.com/T/#u
> 
> [2] https://lore.kernel.org/linux-arm-msm/20250523-topic-ipa_imem-v1-2-b5d536291c7f@oss.qualcomm.com/T/#u
> 
> [3] https://lore.kernel.org/linux-arm-msm/20250408-wdt_reset_reason-v1-0-e6ec30c2c926@oss.qualcomm.com/

Let's go with desired-value-in-dt here.. I don't trust the firmware
to never change. `sram` is prooobably fine, let's hear from Krzysztof

Konrad

Re: [PATCH v4 3/5] dt-bindings: watchdog: qcom-wdt: Document qcom,imem property

[Krzysztof Kozlowski]

On 28/05/2025 19:16, Konrad Dybcio wrote:
>> Krzysztof, Based on the discussions from the previous versions, I have made the changes. Can you help to guide me on how to handle this? Should I just name the property as "sram" and point to the sub block in the IMEM region like how it is done at [1][2], which is more or like similar to what I have submitted in V1 of this series[3] Or is the current approach acceptable? Or some other way to handle this?
>>
>> [1] https://lore.kernel.org/linux-arm-msm/20250523-topic-ipa_imem-v1-1-b5d536291c7f@oss.qualcomm.com/T/#u
>>
>> [2] https://lore.kernel.org/linux-arm-msm/20250523-topic-ipa_imem-v1-2-b5d536291c7f@oss.qualcomm.com/T/#u
>>
>> [3] https://lore.kernel.org/linux-arm-msm/20250408-wdt_reset_reason-v1-0-e6ec30c2c926@oss.qualcomm.com/
> 
> Let's go with desired-value-in-dt here.. I don't trust the firmware
> to never change. `sram` is prooobably fine, let's hear from Krzysztof
> 
I propose to go with 'sram' property.

Best regards,
Krzysztof

Re: [PATCH v4 3/5] dt-bindings: watchdog: qcom-wdt: Document qcom,imem property

[Kathiravan Thirumoorthy]

On 6/1/2025 9:21 PM, Krzysztof Kozlowski wrote:
> On 28/05/2025 19:16, Konrad Dybcio wrote:
>>> Krzysztof, Based on the discussions from the previous versions, I have made the changes. Can you help to guide me on how to handle this? Should I just name the property as "sram" and point to the sub block in the IMEM region like how it is done at [1][2], which is more or like similar to what I have submitted in V1 of this series[3] Or is the current approach acceptable? Or some other way to handle this?
>>>
>>> [1] https://lore.kernel.org/linux-arm-msm/20250523-topic-ipa_imem-v1-1-b5d536291c7f@oss.qualcomm.com/T/#u
>>>
>>> [2] https://lore.kernel.org/linux-arm-msm/20250523-topic-ipa_imem-v1-2-b5d536291c7f@oss.qualcomm.com/T/#u
>>>
>>> [3] https://lore.kernel.org/linux-arm-msm/20250408-wdt_reset_reason-v1-0-e6ec30c2c926@oss.qualcomm.com/
>> Let's go with desired-value-in-dt here.. I don't trust the firmware
>> to never change. `sram` is prooobably fine, let's hear from Krzysztof
>>
> I propose to go with 'sram' property.

Thanks Konrad and Krzysztof for the inputs. Let rename the property 
qcom,imem to sram and submit the next version.

Thanks,

Kathiravan T.

>
> Best regards,
> Krzysztof