Re: [PATCH v24 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 issue(s) to consider:
- [High] arm64: dts: qcom: kaanapali: csiphy_mxc_opp_table uses invalid 300 MHz OPP
- [Low] arm64: dts: qcom: kaanapali: unused include of dt-bindings/phy/phy.h
--

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

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

This commit adds device tree nodes for the CAMSS camera subsystem, six CSIPHY
instances, and the CSIPHY OPP table on the Qualcomm Kaanapali SoC.

> diff --git a/arch/arm64/boot/dts/qcom/kaanapali.dtsi b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> index e48a179d07220..e107bcdf0f8ef 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>

[Severity: Low]
This isn't a bug, but is the dt-bindings/phy/phy.h include needed here? None
of the macros from that header appear to be used in kaanapali.dtsi.

[ ... ]

> @@ -6548,6 +6925,28 @@ pdp_tx: scp-sram-section@100 {
>  		};
>  	};
>  
> +	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]
As reported by Sashiko on v23 [1], should this lowest OPP in
csiphy_mxc_opp_table be 266666667 Hz (opp-266666667) instead of 300000000 Hz?

The CSIPHY core clocks (CAM_CC_CSIPHY0_CLK through CAM_CC_CSIPHY5_CLK) on
Kaanapali are sourced from cam_cc_cphy_rx_clk_src, which only supports
266666667 Hz, 400000000 Hz, and 480000000 Hz and uses clk_rcg2_shared_ops:

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 phy_qcom_mipi_csi2_power_on() -> phy_qcom_mipi_csi2_set_clock_rates() is
called with a link frequency where link_freq / 4 <= 300000000 Hz,
dev_pm_opp_find_freq_ceil() selects opp-300000000 and sets the RPMHPD_MMCX and
RPMHPD_MXC power domains to rpmhpd_opp_low_svs_d1:

drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c:phy_qcom_mipi_csi2_set_clock_rates() {
	...
	opp = dev_pm_opp_find_freq_ceil(dev, &opp_rate);
	...
	for (i = 0; i < csi2phy->pd_list->num_pds; i++) {
		...
		perf = dev_pm_opp_get_required_pstate(opp, pstate);
		pstate += 1;

		ret = dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], perf);
		...
	}
	dev_pm_opp_put(opp);

	ret = dev_pm_opp_set_rate(dev, opp_rate);
}

Because clk_rcg2_shared_ops rounds up via qcom_find_freq(..., CEIL), calling
dev_pm_opp_set_rate() with opp_rate = 300000000 skips 266666667 Hz and rounds
the CSIPHY core clock up to 400000000 Hz. Does this leave the 266.67 MHz rate
unreachable and run the CSIPHY core clock at 400 MHz while MMCX and MXC remain
at rpmhpd_opp_low_svs_d1 instead of rpmhpd_opp_low_svs?

[1] https://lore.kernel.org/r/20261028-kaanapali-camss-v23-12-95f250b18612@oss.qualcomm.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-kaanapali-camss-v24-0-d6b8f3f6f66f@oss.qualcomm.com?part=12

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy