Re: [PATCH v3 7/7] arm64: dts: qcom: eliza: Add support for MM clock controllers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] The `camcc` and `videocc` nodes are missing their parent `power-domains` property (e.g., MMCX).
- [High] The `camcc` and `cambistmclkcc` nodes specify their `clocks` in the wrong order, causing a mismatch with the driver indices.
- [High] The `camcc` node is missing the `#power-domain-cells = <1>;` property.
--

commit d3feefa342664dd9a93d942a18197fa1fdbbcd37
Author: Taniya Das <taniya.das@oss.qualcomm.com>

arm64: dts: qcom: eliza: Add support for MM clock controllers

This commit adds the device tree nodes for the multimedia clock
controllers (cambistmclkcc, camcc, videocc, and gpucc) on the Qualcomm
Eliza SoC.

> diff --git a/arch/arm64/boot/dts/qcom/eliza.dtsi b/arch/arm64/boot/dts/qcom/eliza.dtsi
> index 4a7a0ac40ce62..07c871c1d4b0b 100644
> --- a/arch/arm64/boot/dts/qcom/eliza.dtsi
> +++ b/arch/arm64/boot/dts/qcom/eliza.dtsi

[ ... ]

> @@ -709,6 +713,18 @@ aggre2_noc: interconnect@1700000 {
>  			#interconnect-cells = <2>;
>  		};
>  
> +		cambistmclkcc: clock-controller@1760000 {
> +			compatible = "qcom,eliza-cambistmclkcc";
> +			reg = <0x0 0x01760000 0x0 0x6000>;
> +
> +			clocks = <&bi_tcxo_div2>,
> +				 <&gcc GCC_CAM_BIST_MCLK_AHB_CLK>,
> +				 <&sleep_clk>;

Are the AHB and sleep clocks specified in the correct order?

The clock drivers for these controllers typically resolve clocks using
specific index values, where the sleep clock is at index 1 and the AHB
clock is at index 2. Providing the AHB clock at index 1 and the sleep
clock at index 2 could cause the driver to use the AHB clock as the sleep
clock source and vice versa, leading to incorrect clock configurations.

> +
> +			#clock-cells = <1>;
> +			#reset-cells = <1>;
> +		};

[ ... ]

> @@ -883,6 +916,32 @@ lpass_lpicx_noc: interconnect@7420000 {
>  			#interconnect-cells = <2>;
>  		};
>  
> +		videocc: clock-controller@aaf0000 {
> +			compatible = "qcom,eliza-videocc";
> +			reg = <0x0 0xaaf0000 0x0 0x10000>;
> +
> +			clocks = <&bi_tcxo_div2>,
> +				 <&bi_tcxo_ao_div2>,
> +				 <&sleep_clk>,
> +				 <&gcc GCC_VIDEO_AHB_CLK>;

Is this node missing its parent power-domains property?

Multimedia clock controllers such as videocc typically reside within
hardware power domains (such as MMCX) which must be enabled before their
registers can be accessed. If the driver is probed without the parent power
domain enabled, accessing unpowered hardware registers can result in a
synchronous external abort and crash the system.

> +
> +			#clock-cells = <1>;
> +			#reset-cells = <1>;
> +			#power-domain-cells = <1>;
> +		};
> +
> +		camcc: clock-controller@ade0000 {
> +			compatible = "qcom,eliza-camcc";
> +			reg = <0x0 0x0ade0000 0x0 0x20000>;
> +
> +			clocks = <&bi_tcxo_div2>,
> +				 <&gcc GCC_CAMERA_AHB_CLK>,
> +				 <&sleep_clk>;

Similarly to cambistmclkcc, is the order of the AHB and sleep clocks
reversed here?

Also, is this node missing a power-domains property?

Like videocc, accessing this controller without its parent power domain
enabled could cause an external abort.

> +
> +			#clock-cells = <1>;
> +			#reset-cells = <1>;

Does this node also need to be declared as a power domain provider?

The camera clock controller driver typically exports global distributed
sync controllers which serve as power domains for the rest of the camera
subsystem.

Without the #power-domain-cells = <1> property, dependent camera device
nodes may fail to parse their power-domains phandles, preventing the
camera subsystem from powering up and initializing.

> +		};
> +
> +		pdc: interrupt-controller@b220000 {
> +			compatible = "qcom,eliza-pdc", "qcom,pdc";

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-eliza_mm_cc_v2-v3-0-b59c370dc281@oss.qualcomm.com?part=7