Re: [PATCH 14/17] riscv: dts: spacemit: k3: add display nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Cyclic DT dependency between DP controllers, their child PHY nodes, and the APMU syscon will cause a fw_devlink warning and a potential -EPROBE_DEFER deadlock.
- [Medium] Fragile array override of `syscon_apmu` clocks creates a severe maintenance hazard.
--

--- Patch [14]: [PATCH 14/17] riscv: dts: spacemit: k3: add display nodes ---
commit c12c6ab932374f52bfc3ce3d04640d8193a9bfea
Author: Cody Kang <codykang.hk@gmail.com>

riscv: dts: spacemit: k3: add display nodes

Add the SoC-level display nodes: the two Saturn DPUs, the two DP/eDP
controllers with their PHY children.

> diff --git a/arch/riscv/boot/dts/spacemit/k3-display.dtsi b/arch/riscv/boot/dts/spacemit/k3-display.dtsi
> new file mode 100644
> index 0000000000000..4892ee6acda22
> --- /dev/null
> +++ b/arch/riscv/boot/dts/spacemit/k3-display.dtsi

[ ... ]

> +	edp0: edp@cac84000 {
> +		compatible = "spacemit,k3-inno-dp";
> +		reg = <0x0 0xcac84000 0x0 0x4000>;
> +		interrupts = <132 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&syscon_apmu CLK_APMU_EDP0_PXCLK>, <&edp0_phy>;

[Severity: High]
Does this create a cyclical dependency that could cause a fw_devlink warning
or a permanent probe deadlock?

edp0 depends on syscon_apmu here, while syscon_apmu depends on the edp0_phy
child node below. Since edp0_phy is a child of edp0, it won't be populated
until edp0 probes. If fw_devlink breaks the cycle by dropping the edp0 ->
syscon_apmu link, edp0 will attempt to probe first, fail to get its clock, and
defer forever because it will not proceed to populate the child PHY.

> +		clock-names = "pxclk", "pll";
> +		resets = <&syscon_apmu RESET_APMU_EDP0>;
> +		phys = <&edp0_phy>;
> +		phy-names = "dp";
> +		status = "disabled";
> +
> +		edp0_phy: phy {
> +			compatible = "spacemit,k3-inno-dp-phy";
> +			#phy-cells = <0>;
> +			#clock-cells = <0>;
> +			clock-output-names = "edp0_pll_pxclk";
> +		};

[ ... ]

> +/*
> + * Feed the PHY PLL pixel clocks back in as the APMU mux parents. This lives
> + * here, not in k3.dtsi: the PHYs only exist where this file is included.
> + */
> +&syscon_apmu {
> +	clocks = <&osc_32k>, <&vctcxo_1m>, <&vctcxo_3m>, <&vctcxo_24m>,
> +		 <&edp0_phy>, <&dp1_phy>;
> +	clock-names = "osc", "vctcxo_1m", "vctcxo_3m", "vctcxo_24m",
> +		      "edp0_pll_pxclk", "edp1_pll_pxclk";

[Severity: Medium]
Will overriding the clocks property completely drop any future additions made to
the syscon_apmu node in the base k3.dtsi?

Device tree doesn't support appending to array properties, so if any new clocks
are added to the base syscon_apmu node later, this file will silently override
them and break other subsystems relying on those clocks.

> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724-k3-display-v1-0-f22a10a52867@gmail.com?part=14