Re: [PATCH v2] arm64: dts: qcom: nord: Add more clock controller nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] dts: qcom: nord: videocc clocks array missing sleep clock expected by driver
- [Medium] dts: qcom: nord: missing dt-bindings include for qcom,nord-gpu2cc
--

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

arm64: dts: qcom: nord: Add more clock controller nodes

This commit adds the GPU, camera, video, and display clock controller nodes 
for the Qualcomm Nord SoC. It also adds their required dt-bindings includes.

Link: https://lore.kernel.org/all/20260920081049.2183845-1-shengchao.guo@oss.qualcomm.com/

> diff --git a/arch/arm64/boot/dts/qcom/nord-embedded.dtsi b/arch/arm64/boot/dts/qcom/nord-embedded.dtsi
> index 568fe68cd1859..1ab7d9bc3cfed 100644
> --- a/arch/arm64/boot/dts/qcom/nord-embedded.dtsi
> +++ b/arch/arm64/boot/dts/qcom/nord-embedded.dtsi
> @@ -3,7 +3,11 @@
>   * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>   */
>  
> +#include <dt-bindings/clock/qcom,glymur-videocc.h>
> +#include <dt-bindings/clock/qcom,nord-camcc.h>
> +#include <dt-bindings/clock/qcom,nord-dispcc.h>
>  #include <dt-bindings/clock/qcom,nord-gcc.h>
> +#include <dt-bindings/clock/qcom,nord-gpucc.h>

[Severity: Medium]
Should there be an include for <dt-bindings/clock/qcom,nord-gpu2cc.h> in
this list?

The commit adds the gpu2cc node further down in the file, but without the
corresponding dt-bindings header, any future consumer nodes referencing the
gpu2cc clock identifiers will fail to compile.

[ ... ]
> @@ -814,6 +829,99 @@ nwgcc: clock-controller@8b00000 {
[ ... ]
> +	gpu2cc: clock-controller@9890000 {
> +		compatible = "qcom,nord-gpu2cc";
> +		reg = <0x0 0x09890000 0x0 0xa000>;
> +		clocks = <&bi_tcxo_div2>,
> +			 <&negcc NE_GCC_GPU_2_GPLL0_CLK_SRC>,
> +			 <&negcc NE_GCC_GPU_2_GPLL0_DIV_CLK_SRC>;
> +		#clock-cells = <1>;
> +		#reset-cells = <1>;
> +		#power-domain-cells = <1>;
> +	};
[ ... ]
> +	videocc: clock-controller@aaf0000 {
> +		compatible = "qcom,nord-videocc";
> +		reg = <0x0 0x0aaf0000 0x0 0x10000>;
> +		clocks = <&bi_tcxo_div2>,
> +			 <&nwgcc NW_GCC_VIDEO_AHB_CLK>;

[Severity: High]
Will the common clock framework fail to resolve the parent for the sleep clock
here?

The videocc-glymur.c driver defines DT_SLEEP_CLK as index 2 and uses it as
the parent for video_cc_sleep_clk_src. Because this device tree node only
provides two clocks, index 2 will be out of bounds, resulting in a parent
resolution failure (-EINVAL).

Also, mapping the AHB clock to index 1 appears to be incorrect, because the
driver maps index 1 to DT_BI_TCXO_AO, which isn't actively used as a parent
here.

> +		power-domains = <&rpmhpd RPMHPD_MMCX>,
> +				<&rpmhpd RPMHPD_MXC>;
> +		required-opps = <&rpmhpd_opp_low_svs>,
> +				<&rpmhpd_opp_low_svs>;
> +		#clock-cells = <1>;
> +		#reset-cells = <1>;
> +		#power-domain-cells = <1>;
> +	};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924094448.1030807-1-shengchao.guo@oss.qualcomm.com?part=1