Re: [PATCH v7 04/12] arm64: dts: qcom: x1e80100: Add CAMSS block definition

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] arm64: dts: qcom: redundant standalone csiphy nodes claim same resources as camss
- [Low] arm64: dts: qcom: code defines 3 CSID blocks but commit message claims 2
- [Low] arm64: dts: qcom: required-opps and power-domains mismatch in csiphy nodes
--

commit 74737ca93583e7d3d3b8f9b8c33d075ed4ae954e
Author: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

arm64: dts: qcom: x1e80100: Add CAMSS block definition

This commit adds a dtsi fragment to describe the x1e80100 CAMSS block,
defining the CSIPHY, TPG, CSID, IFE, and VFE components.

> arm64: dts: qcom: x1e80100: Add CAMSS block definition
> 
> Add dtsi to describe the x1e80100 CAMSS block
> 
> 4 x CSIPHY
> 3 x TPG
> 2 x CSID

[Severity: Low]
The commit message explicitly lists "2 x CSID", but the device tree code
declares three CSID blocks ("csid0", "csid1", "csid2"). Is this description
in the commit message accurate?

> diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> index 8ac3392b2d747..73bbaa49adf7d 100644
> --- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
> +++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> @@ -668,6 +668,50 @@ smem_mem: smem@ffe00000 {
>  		};
>  	};
>  
> +	csiphy_mxc_opp_table: opp-table-mxc {
> +		compatible = "operating-points-v2";
> +
> +		opp-300000000 {
> +			opp-hz = /bits/ 64 <300000000>;
> +			required-opps = <&rpmhpd_opp_low_svs_d1>,
> +					<&rpmhpd_opp_low_svs_d1>;
> +		};

[ ... ]

> +	csiphy_mxa_opp_table: opp-table-mxa {
> +		compatible = "operating-points-v2";
> +
> +		opp-300000000 {
> +			opp-hz = /bits/ 64 <300000000>;
> +			required-opps = <&rpmhpd_opp_low_svs_d1>,
> +					<&rpmhpd_opp_low_svs_d1>;
> +		};

[ ... ]

> +		camss: isp@acb7000 {
> +			compatible = "qcom,x1e80100-camss";
> +
> +			reg = <0 0x0acb7000 0 0x2000>,
> +			      <0 0x0acb9000 0 0x2000>,
> +			      <0 0x0acbb000 0 0x2000>,
> +			      <0 0x0acc6000 0 0x1000>,
> +			      <0 0x0acca000 0 0x1000>,
> +			      <0 0x0acb6000 0 0x1000>,
> +			      <0 0x0ace4000 0 0x2000>,
> +			      <0 0x0ace6000 0 0x2000>,
> +			      <0 0x0ace8000 0 0x2000>,
> +			      <0 0x0acec000 0 0x2000>,

[ ... ]

> +			reg-names = "csid0",
> +				    "csid1",
> +				    "csid2",
> +				    "csid_lite0",
> +				    "csid_lite1",
> +				    "csid_wrapper",
> +				    "csiphy0",
> +				    "csiphy1",
> +				    "csiphy2",
> +				    "csiphy4",

[ ... ]

> +		csiphy0: phy@ace4000 {
> +			compatible = "qcom,x1e80100-csi2-phy";
> +			reg = <0 0x0ace4000 0 0x2000>;

[Severity: Medium]
Will this lead to resource conflicts during device tree parsing? 

The standalone csiphy0 node natively claims the exact same memory region
(0x0ace4000) and interrupts as the monolithic camss node defined above.
Upstream Qualcomm CAMSS drivers map and manage the PHY intrinsically, so
does defining these separate phy nodes risk -EBUSY errors or structure
conflicts during boot?

> +
> +			clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
> +				 <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
> +				 <&camcc CAM_CC_CORE_AHB_CLK>;
> +			clock-names = "core",
> +				      "timer",
> +				      "ahb";
> +
> +			operating-points-v2 = <&csiphy_mxc_opp_table>;
> +
> +			interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
> +
> +			power-domains = <&camcc CAM_CC_TITAN_TOP_GDSC>,
> +					<&rpmhpd RPMHPD_MMCX>,
> +					<&rpmhpd RPMHPD_MXC>;

[Severity: Low]
Does this configuration map correctly in the OPP core? 

The csiphy_mxc_opp_table defines two required-opps handles, but three
power-domains are declared here. This mismatched configuration can cause
the OPP core to improperly attempt to apply the first RPMh performance
state to the CAM_CC_TITAN_TOP_GDSC domain.

[ ... ]

> +		csiphy4: phy@acec000 {
> +			compatible = "qcom,x1e80100-csi2-phy";
> +			reg = <0 0x0acec000 0 0x2000>;

[ ... ]

> +			operating-points-v2 = <&csiphy_mxa_opp_table>;
> +
> +			interrupts = <GIC_SPI 122 IRQ_TYPE_EDGE_RISING>;
> +
> +			power-domains = <&camcc CAM_CC_TITAN_TOP_GDSC>,
> +					<&rpmhpd RPMHPD_MMCX>,
> +					<&rpmhpd RPMHPD_MXC>;

[Severity: Low]
Is this the intended power domain mapping? The csiphy4 node is configured
to use the mxa OPP table (csiphy_mxa_opp_table), but it binds to RPMHPD_MXC
here instead of RPMHPD_MXA.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917-x1e-camss-csi2-phy-dtsi-v7-0-1a63eb35838b@linaro.org?part=4