Re: [PATCH v1] arm: dts: imx: Add watchdog support for early boot stages

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Hardware IRQ conflict: wdog4 and ddr-pmu share the same interrupt in imx94.dtsi.
- [Medium] Missing bootph-all property on the newly added wdog2 node in imx7ulp.dtsi.
- [Medium] Undocumented addition of multiple new watchdog hardware nodes.
--

commit 566f5af2df66e555eb4881057449fc8955a46103
Author: Alice Guo <alice.guo@nxp.com>

arm: dts: imx: Add watchdog support for early boot stages

This commit adds the bootph-all property to watchdog nodes across several
i.MX device trees to make them available during U-Boot's early boot phases.
It also introduces several new watchdog nodes to the device trees.

> diff --git a/arch/arm/boot/dts/nxp/imx/imx7ulp.dtsi b/arch/arm/boot/dts/nxp/imx/imx7ulp.dtsi
> index 880b9a4f32b08..5e3e562225e58 100644
> --- a/arch/arm/boot/dts/nxp/imx/imx7ulp.dtsi
> +++ b/arch/arm/boot/dts/nxp/imx/imx7ulp.dtsi
> @@ -297,6 +297,17 @@ smc1: clock-controller@40410000 {
>  			clock-names = "divcore", "hsrun_divcore";
>  		};
>  
> +		wdog2: watchdog@40430000 {
> +			compatible = "fsl,imx7ulp-wdt";
> +			reg = <0x40430000 0x10000>;
> +			interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&pcc2 IMX7ULP_CLK_WDG2>;
> +			assigned-clocks = <&pcc2 IMX7ULP_CLK_WDG2>;
> +			assigned-clock-parents = <&scg1 IMX7ULP_CLK_FIRC_BUS_CLK>;
> +			timeout-sec = <40>;
> +			status = "disabled";
> +		};

Was the bootph-all property intentionally omitted from this node?

The commit message explicitly states the purpose of the patch is to add
the bootph-all property to watchdog nodes, but it appears to be missing
from this newly added wdog2 node.

Additionally, is adding completely new hardware nodes expected within
this patch?

The commit message notes that the patch adds the bootph-all property
to watchdog nodes, but the diff reveals the patch actually introduces
entirely new watchdog nodes (such as wdog2 in imx7ulp.dtsi, and wdog4
in imx94.dtsi, imx95.dtsi, and imx952.dtsi).

Should the commit message be updated to explicitly document the
addition of these new hardware nodes, or should these additions be split
into a separate preparatory patch?

[ ... ]

> diff --git a/arch/arm64/boot/dts/freescale/imx94.dtsi b/arch/arm64/boot/dts/freescale/imx94.dtsi
> index d2f31c8caf6eb..be366af8ac912 100644
> --- a/arch/arm64/boot/dts/freescale/imx94.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx94.dtsi
> @@ -1220,6 +1220,18 @@ wdog3: watchdog@49220000 {
>  				timeout-sec = <40>;
>  				fsl,ext-reset-output;
>  				status = "disabled";
> +				bootph-all;
> +			};
> +
> +			wdog4: watchdog@49230000 {
> +				compatible = "fsl,imx94-wdt", "fsl,imx93-wdt";
> +				reg = <0x49230000 0x10000>;
> +				interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;

Could this interrupt assignment conflict with existing hardware?

Looking at the same imx94.dtsi file, GIC_SPI 91 is already assigned to the
ddr-pmu node:

		ddr-pmu@4e090dc0 {
			compatible = "fsl,imx94-ddr-pmu", "fsl,imx93-ddr-pmu";
			reg = <0x0 0x4e090dc0 0x0 0x200>;
			interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
		};

Sharing a GIC SPI interrupt between independent hardware blocks like a
watchdog and a PMU is an unusual hardware description unless they are
explicitly designed to share IRQ handling.

Does this need to be updated to the correct hardware interrupt to prevent
issues if the watchdog driver is updated to request this IRQ?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512024850.904551-1-alice.guo@oss.nxp.com?part=1