Re: [PATCH v10 09/10] clk: qcom: camcc: Add support for camera clock controller for Eliza

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing `clk_alpha_pll_postdiv` for `P_CAM_BIST_MCLK_CC_PLL0_OUT_EVEN` causes incorrect clock rates
--

--- Patch [9]: [PATCH v10 09/10] clk: qcom: camcc: Add support for camera clock controller for Eliza ---
commit 87585fa44458a8d4bd37784a045b9c8108ed5a11
Author: Taniya Das <taniya.das@oss.qualcomm.com>

clk: qcom: camcc: Add support for camera clock controller for Eliza

Add support for the Camera Clock Controller (CAMCC) on the Eliza
platform. The CAMCC block on Eliza includes both the primary camera clock
controller and the Camera BIST clock controller, which provides the
functional MCLK required for camera operations.

> diff --git a/drivers/clk/qcom/cambistmclkcc-eliza.c b/drivers/clk/qcom/cambistmclkcc-eliza.c
> new file mode 100644
> index 0000000000000..5cb68b81095eb
> --- /dev/null
> +++ b/drivers/clk/qcom/cambistmclkcc-eliza.c
[ ... ]
> +static const struct parent_map cam_bist_mclk_cc_parent_map_0[] = {
> +	{ P_BI_TCXO, 0 },
> +	{ P_CAM_BIST_MCLK_CC_PLL0_OUT_EVEN, 3 },
> +	{ P_CAM_BIST_MCLK_CC_PLL0_OUT_MAIN, 5 },
> +};
> +
> +static const struct clk_parent_data cam_bist_mclk_cc_parent_data_0[] = {
> +	{ .index = DT_BI_TCXO },
> +	{ .hw = &cam_bist_mclk_cc_pll0.clkr.hw },
> +	{ .hw = &cam_bist_mclk_cc_pll0.clkr.hw },
> +};

[Severity: High]
Are both the EVEN and MAIN outputs mapped to the exact same hardware object
here?

Since there is no clk_alpha_pll_postdiv object defined for the EVEN output,
the Common Clock Framework will treat the EVEN output as 960MHz (the main PLL
rate) rather than 480MHz.

[ ... ]
> +static const struct freq_tbl ftbl_cam_bist_mclk_cc_mclk0_clk_src[] = {
> +	F(19200000, P_BI_TCXO, 1, 0, 0),
> +	F(24000000, P_CAM_BIST_MCLK_CC_PLL0_OUT_EVEN, 10, 1, 4),

[Severity: High]
Will this lead to an incorrect physical clock rate of 12MHz instead of 24MHz?

Because the CCF believes the EVEN source rate is 960MHz, it will program the
hardware with a divisor of 10. However, since the physical hardware EVEN
signal is actually 480MHz, dividing 480MHz by 10 and then by 4 (m=1, n=4)
produces 12MHz, which might cause camera operations to fail.

> +	F(68571429, P_CAM_BIST_MCLK_CC_PLL0_OUT_MAIN, 14, 0, 0),
> +	{ }
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806-eliza-mm-cc-v9-v10-0-6ba52dd14343@oss.qualcomm.com?part=9