Re: [PATCH v6 6/7] clk: qcom: camcc: Add support for camera clock controller for Eliza

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Frequency calculation mismatch due to missing post-divider model for OUT_EVEN in `cam_bist_mclk_cc_mclk0_clk_src`.
- [High] Missing `.use_rpm = true` in `cam_cc_eliza_desc` prevents power domain initialization.
--

commit fd46f39c4eb76b4e41aa55ab7c656fc72d38adcc
Author: Taniya Das <taniya.das@oss.qualcomm.com>

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

This commit adds support for the Camera Clock Controller (CAMCC) on the
Eliza platform, including both the primary camera clock controller and the
Camera BIST clock controller.

> diff --git a/drivers/clk/qcom/cambistmclkcc-eliza.c b/drivers/clk/qcom/cambistmclkcc-eliza.c
> --- /dev/null
> +++ b/drivers/clk/qcom/cambistmclkcc-eliza.c
[ ... ]
> +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 },
> +};
[ ... ]
> +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 a hardware clock rate of 12 MHz instead of 24 MHz?

In hardware, OUT_EVEN runs at half the main PLL rate (480 MHz instead of
960 MHz). Applying the pre-divider of 10 and m/n of 1/4 to 480 MHz yields
a physical clock of 12 MHz.

However, looking at cam_bist_mclk_cc_parent_data_0 above, OUT_EVEN is mapped
directly to the main PLL hardware without an intervening
clk_alpha_pll_postdiv instance. This causes the clock framework to calculate
the rate using the main 960 MHz PLL rate, falsely reporting 24 MHz in
software while actually running at 12 MHz.

Could this use P_CAM_BIST_MCLK_CC_PLL0_OUT_MAIN instead, or should OUT_EVEN
be properly modeled with a post-divider struct?

> diff --git a/drivers/clk/qcom/camcc-eliza.c b/drivers/clk/qcom/camcc-eliza.c
> --- /dev/null
> +++ b/drivers/clk/qcom/camcc-eliza.c
[ ... ]
> +static const struct qcom_cc_desc cam_cc_eliza_desc = {
> +	.config = &cam_cc_eliza_regmap_config,
> +	.clks = cam_cc_eliza_clocks,
> +	.num_clks = ARRAY_SIZE(cam_cc_eliza_clocks),
> +	.resets = cam_cc_eliza_resets,
> +	.num_resets = ARRAY_SIZE(cam_cc_eliza_resets),
> +	.gdscs = cam_cc_eliza_gdscs,
> +	.num_gdscs = ARRAY_SIZE(cam_cc_eliza_gdscs),
> +	.driver_data = &cam_cc_eliza_driver_data,
> +};

[Severity: High]
Can this missing .use_rpm = true initialization cause a boot panic?

In qcom_cc_really_probe(), the framework checks the .use_rpm flag to enable
runtime PM before accessing hardware registers. Without this flag, the
CAMCC power domain might not be brought online during probe.

Consequently, when the probe calls qcom_cc_clk_pll_configure() to
initialize the PLLs, writing to unpowered registers can cause a synchronous
external abort.

Should .use_rpm = true be added here to match the companion
cam_bist_mclk_cc_eliza_desc?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-b4-eliza_mm_cc_v6-v6-0-17df09e5940c@oss.qualcomm.com?part=6