Re: [PATCH v25 12/15] 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
issues to consider:
- [High] arm64: dts: qcom: kaanapali: csiphy_mxc_opp_table uses invalid
  300 MHz OPP
- [Low] arm64: dts: qcom: kaanapali: unused phy.h include and
  opp-table-mxc order
--

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

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

This commit adds the CAMSS, CSIPHY0..5, and CSIPHY OPP table device tree nodes
for the Qualcomm Kaanapali SoC.

> diff --git a/arch/arm64/boot/dts/qcom/kaanapali.dtsi b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> index 9f04a5ad75f21..23406698370fb 100644
> --- a/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> +++ b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> @@ -20,6 +20,7 @@
>  #include <dt-bindings/interconnect/qcom,kaanapali-rpmh.h>
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include <dt-bindings/mailbox/qcom-ipcc.h>
> +#include <dt-bindings/phy/phy.h>
>  #include <dt-bindings/phy/phy-qcom-qmp.h>
>  #include <dt-bindings/power/qcom-rpmpd.h>
>  #include <dt-bindings/regulator/qcom,rpmh-regulator.h>

[ ... ]

> @@ -6566,6 +6943,28 @@ pdp_tx: scp-sram-section@100 {
>  		};
>  	};
>  
> +	csiphy_mxc_opp_table: opp-table-mxc {

[Severity: Low]
This isn't a bug, but dt-bindings/phy/phy.h is not used in this file, and
opp-table-mxc is placed after soc@0 instead of in alphabetical order by node
name between memory@a0000000 and pmu.

> +		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]
Should this lowest OPP in csiphy_mxc_opp_table be 266666667 Hz instead of
300000000 Hz?

The CSIPHY core clocks (CAM_CC_CSIPHY0_CLK..CAM_CC_CSIPHY5_CLK) parent to
cam_cc_cphy_rx_clk_src, which uses clk_rcg2_shared_ops (ceiling rate lookup)
and only supports 266.67 MHz, 400 MHz, and 480 MHz:

drivers/clk/qcom/camcc-kaanapali.c:
static const struct freq_tbl ftbl_cam_cc_cphy_rx_clk_src[] = {
	F(266666667, P_CAM_CC_PLL0_OUT_MAIN, 4.5, 0, 0),
	F(400000000, P_CAM_CC_PLL0_OUT_MAIN, 3, 0, 0),
	F(480000000, P_CAM_CC_PLL0_OUT_MAIN, 2.5, 0, 0),
	{ }
};

When a sensor streams with link_freq / 4 <= 300 MHz,
phy_qcom_mipi_csi2_set_clock_rates() selects opp-300000000, votes the MMCX and
MXC power domains to rpmhpd_opp_low_svs_d1, and calls dev_pm_opp_set_rate()
for 300 MHz.  Because 300 MHz exceeds 266666667 Hz, the RCG rounds the CSIPHY
core clock up to 400 MHz while leaving MMCX and MXC under-volted at
rpmhpd_opp_low_svs_d1 instead of rpmhpd_opp_low_svs, and the 266.67 MHz rate
can never be selected.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112-kaanapali-camss-v25-0-1d709f57f370@oss.qualcomm.com?part=12