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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing post-divider clock definition for `cam_bist_mclk_cc_pll0_out_even` causes incorrect RCG frequency calculations.
--

--- Patch [9]: [PATCH v9 09/10] clk: qcom: camcc: Add support for camera clock controller for Eliza ---
commit 8a0c7a74e032d685c7765438594ec88ec68d1f8a
Author: Taniya Das <[email protected]>

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

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

> --- /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]
Is a post-divider definition missing here for the OUT_EVEN path?

Both the OUT_EVEN and OUT_MAIN paths currently point directly to the base
PLL (cam_bist_mclk_cc_pll0). Without a separate clk_alpha_pll_postdiv
definition for OUT_EVEN, the clock framework cannot read the hardware
post-divider and will assume the rate is the full 960 MHz instead of the
divided rate (e.g., 480 MHz).

[ ... ]
> +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 result in an incorrect output frequency for the 24 MHz MCLK?

Because the framework assumes the P_CAM_BIST_MCLK_CC_PLL0_OUT_EVEN input
is 960 MHz, it calculates a division factor of 40 (pre_div=10, n=4) to
achieve 24 MHz. If the actual hardware parent rate is 480 MHz, dividing by
40 would result in an MCLK frequency of 12 MHz instead, which could cause
camera sensor initialization or streaming to fail.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9