Re: [PATCH v3 3/3] arm64: dts: qcom: ipq5332: add support to get watchdog bootstatus from IMEM

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] arm64: dts: qcom: ipq5332: sram@8600000 size 0x14000 mismatches IMEM bounds
--

commit 353fa282f26f76cd09b02c6b70d1605ceadf060b
Author: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>

arm64: dts: qcom: ipq5332: add support to get watchdog bootstatus from IMEM

This commit adds the sram@8600000 IMEM node and links the restart_reason
subnode to the watchdog node via the sram property on IPQ5332 so the
watchdog driver can read the system restart reason populated by XBL.

> diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> index 74d9de8d7641d..c1b0836246964 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> @@ -493,6 +493,21 @@ qpic_nand: spi@79b0000 {
>  			status = "disabled";
>  		};
>  
> +		sram@8600000 {
> +			compatible = "qcom,ipq5332-imem", "mmio-sram";
> +			reg = <0x08600000 0x14000>;
> +			ranges = <0 0x08600000 0x14000>;

[Severity: Medium]
Should the reg and ranges size in sram@8600000 be 0x1000 (or 0x8000 with
the inaccessible portion reserved) rather than 0x14000?

The commit message notes:

    The overall IMEM region is 32KB but only initial 4KB is accessible by
    all masters in the SoC.

However, sram@8600000 in arch/arm64/boot/dts/qcom/ipq5332.dtsi specifies
0x14000 (80KB), which matches neither the 32KB (0x8000) total IMEM size
nor the 4KB (0x1000) accessible window.

When the mmio-sram driver probes sram@8600000 via
sram_probe()->sram_reserve_regions() in drivers/misc/sram.c, it maps the
entire reg range and adds all unreserved chunks ([0x0, 0x7a4) and
[0x7a8, 0x14000)) into the general-purpose SRAM allocation pool:

drivers/misc/sram.c:sram_reserve_regions() {
    ...
		if (sram->pool) {
			dev_dbg(sram->dev, "adding chunk 0x%lx-0x%lx\n",
				cur_start, cur_start + cur_size);

			ret = gen_pool_add_virt(sram->pool,
					(unsigned long)sram->virt_base + cur_start,
					res->start + cur_start, cur_size, -1);
    ...
}

If the hardware IMEM is only 32KB or access-protected beyond the initial
4KB window, won't exposing offsets [0x1000, 0x14000) as allocatable SRAM
risk bus aborts or access violations if any consumer allocates from the
pool?

> +
> +			no-memory-wc;
> +
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			restart_reason: restartreason-sram@7a4 {
> +				reg = <0x7a4 0x4>;
> +			};
> +		};
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019-imem_restart_reason-v3-0-e6b1ff5de841@oss.qualcomm.com?part=3