Re: [PATCH v18 09/11] arm64: dts: qcom: kaanapali: Add CAMSS and CSIPHY block definitions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] dt-bindings: pm: mismatch between power-domains and required-opps in csiphy nodes
- [Medium] dt-bindings: media: missing internal SoC OF graph links between camss and csiphy
--

commit 14fbcf150a21f3b840f99ac0a615d5f91a887f84
Author: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>

arm64: dts: qcom: kaanapali: Add CAMSS and CSIPHY block definitions

Describe the CAMSS and CSIPHY blocks on Kaanapali so that camera pipelines
can be enabled by board device trees.

> diff --git a/arch/arm64/boot/dts/qcom/kaanapali.dtsi b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> index e48a179d07220..7e5106865e7ee 100644
> --- a/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> +++ b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
[ ... ]
> +		camss: isp@9253000 {
> +			compatible = "qcom,kaanapali-camss";
[ ... ]
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				port@0 {
> +					reg = <0>;
> +				};
> +
> +				port@1 {
> +					reg = <1>;
> +				};

[Severity: Medium]
Will the camera subsystem data path break without endpoint nodes connecting
these CAMSS ports to the respective CSIPHY outputs?

The CAMSS driver checks for endpoints to discover non-legacy PHY nodes, so
missing them here could cause the CSIPHYs to fail to initialize during
probe. Should endpoints be added here to complete the internal SoC OF graph
links?

[ ... ]
> +		csiphy0: phy@9523000 {
> +			compatible = "qcom,kaanapali-csi2-phy";
> +			reg = <0x0 0x09523000 0x0 0x2000>;
> +
> +			clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
> +				 <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
> +				 <&camcc CAM_CC_CORE_AHB_CLK>;
> +			clock-names = "core",
> +				      "timer",
> +				      "ahb";
> +
> +			interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
> +
> +			operating-points-v2 = <&csiphy_mxc_opp_table>;
> +
> +			power-domains = <&camcc CAM_CC_TITAN_TOP_GDSC>,
> +					<&rpmhpd RPMHPD_MMCX>,
> +					<&rpmhpd RPMHPD_MXC>;

[Severity: High]
Since this node declares 3 power domains but relies on an OPP table with only
2 required-opps, does this break index alignment?

The genpd framework maps required-opps positionally to the PM domains
specified in power-domains. This mismatch might cause incorrect performance
states to be applied to the wrong power domain (e.g. voting on a GDSC
instead of MMCX), leading to PM framework errors or hardware hangs.

Also, shouldn't there be a ports definition in this CSIPHY node to link
back to the CAMSS controller?

> +			power-domain-names = "top",
> +					     "mmcx",
> +					     "mx";
> +
> +			status = "disabled";
> +		};
[ ... ]
> +	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>;

[Severity: High]
Does this required-opps array need to be padded to align with the 3 power
domains specified in the CSIPHY nodes above?

To correctly skip the GDSC at index 0, should it be padded like:
<0>, <&rpmhpd_opp_low_svs_d1>, <&rpmhpd_opp_low_svs_d1> ?

> +		};
> +
> +		opp-400000000 {
> +			opp-hz = /bits/ 64 <400000000>;
> +			required-opps = <&rpmhpd_opp_low_svs>,
> +					<&rpmhpd_opp_low_svs>;
> +		};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261003-kaanapali-camss-v18-0-87ba4ebe0db1@oss.qualcomm.com?part=9