[PATCH v8 12/12] ARM: dts: zte: Declare zx297520v3 CRM device nodes

[sashiko-bot at kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The lspcrm device defines a hardware reset dependency on matrixcrm (ZX297520V3_LSP_RESET), but the CRM drivers never request or deassert it, which can lead to a hardware hang when accessing the registers.
- [Low] Nodes under the `soc` bus are not sorted by their unit address.
- [Low] Non-generic node name `syscon-reboot`.
--

--- Patch [12]: [PATCH v8 12/12] ARM: dts: zte: Declare zx297520v3 CRM device nodes ---
commit ae42492b8ef6fce4731646f6907ba63dc94e9ffa
Author: Stefan Dösinger <stefandoesinger at gmail.com>

ARM: dts: zte: Declare zx297520v3 CRM device nodes

This commit wires up the zx297520v3 CRM device nodes, clock inputs,
and resets, getting rid of the placeholder uartclk node.

> diff --git a/arch/arm/boot/dts/zte/zx297520v3.dtsi b/arch/arm/boot/dts/zte/zx297520v3.dtsi
> --- a/arch/arm/boot/dts/zte/zx297520v3.dtsi
> +++ b/arch/arm/boot/dts/zte/zx297520v3.dtsi

[ ... ]

> @@ -70,13 +75,74 @@ gic: interrupt-controller at f2000000 {
>  			      <0xf2040000 0x20000>;
>  		};
>  
> +		topcrm: clock-controller at 13b000 {
> +			compatible = "zte,zx297520v3-topcrm", "syscon";
> +			reg = <0x0013b000 0x400>;

[Severity: Low]
Are these new nodes inserted out of order relative to their unit addresses?

The topcrm node at 13b000 is placed before matrixcrm at 1306000, and lspcrm
at 1400000 is placed before uart0 at 131000. It is customary to sort nodes
under a bus by their unit address to maintain readability.

[ ... ]

> +			#clock-cells = <1>;
> +			#phy-cells = <1>;
> +			#reset-cells = <1>;
> +
> +			syscon-reboot {
> +				compatible = "syscon-reboot";
> +				offset = <0x0>;
> +				value = <0x1>;
> +				mask = <0x1>;
> +			};

[Severity: Low]
Should this node use a generic name like reboot instead of syscon-reboot?

The Devicetree specification usually recommends node names to reflect the
function of the device rather than its programming model or driver name.

> +		};
> +
> +		matrixcrm: clock-controller at 1306000 {
> +			compatible = "zte,zx297520v3-matrixcrm";
> +			reg = <0x01306000 0x400>;

[ ... ]

> +		lspcrm: clock-controller at 1400000 {
> +			compatible = "zte,zx297520v3-lspcrm";
> +			reg = <0x01400000 0x100>;
> +			#clock-cells = <1>;
> +			#reset-cells = <1>;

[ ... ]

> +			clock-names = "mpll-d5", "mpll-d4", "mpll-d6", "mpll-d8", "mpll-d12",
> +				      "osc26m", "osc32k", "pclk", "tdm-wclk", "dpll-d4";
> +			resets = <&matrixcrm ZX297520V3_LSP_RESET>;
> +		};

[Severity: High]
Does this hardware reset dependency require explicit handling in the driver?

The DTS correctly defines the dependency, but looking at zx_clk_common_probe()
in drivers/clk/zte/clk-zx.c, the regmap is obtained using:

drivers/clk/zte/clk-zx.c:zx_clk_common_probe() {
    ...
    map = device_node_to_regmap(of_node);
    if (IS_ERR(map))
        return PTR_ERR(map);
    ...
}

Because the lspcrm node lacks the syscon compatible string, this bypasses the
generic syscon reset handling. As neither the MFD driver nor child drivers
manually deassert this reset, won't subsequent register accesses hit a reset
block and potentially hang the hardware?

> +
>  		uart0: serial at 131000 {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727-zx29clk-v8-0-7a107b00f1dd@gmail.com?part=12