Re: [PATCH 3/4] clk: qcom: videocc-nord: Add video clock controller driver for Nord

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The VIDEO_CC_SLEEP_CLK_SRC clock is exported in the device tree bindings but is missing from the video_cc_nord_clocks array.
--

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

clk: qcom: videocc-nord: Add video clock controller driver for Nord

This commit adds support for the video clock controller for video clients to
be able to request videocc clocks on the Nord platform.

> diff --git a/drivers/clk/qcom/videocc-nord.c b/drivers/clk/qcom/videocc-nord.c
> new file mode 100644
> index 0000000000000..ee73e89a01daa
> --- /dev/null
> +++ b/drivers/clk/qcom/videocc-nord.c

[ ... ]

> +enum {
> +	P_BI_TCXO,
> +	P_SLEEP_CLK,
> +	P_VIDEO_CC_PLL0_OUT_MAIN,
> +};

[Severity: Medium]
Is P_SLEEP_CLK intended to be used in any parent_map? It is defined in this
enum but does not appear to be referenced anywhere else in the driver.

[ ... ]

> +static struct clk_regmap *video_cc_nord_clocks[] = {
> +	[VIDEO_CC_AHB_CLK_SRC] = &video_cc_ahb_clk_src.clkr,
> +	[VIDEO_CC_MVS0_CLK] = &video_cc_mvs0_clk.clkr,

[ ... ]

> +	[VIDEO_CC_MVS1_FREERUN_CLK] = &video_cc_mvs1_freerun_clk.clkr,
> +	[VIDEO_CC_MVS1_SHIFT_CLK] = &video_cc_mvs1_shift_clk.clkr,
> +	[VIDEO_CC_PLL0] = &video_cc_pll0.clkr,
> +	[VIDEO_CC_XO_CLK_SRC] = &video_cc_xo_clk_src.clkr,
> +};

[Severity: Medium]
Should VIDEO_CC_SLEEP_CLK_SRC be included in this array? 

It appears to be exported in the device tree bindings but is missing from
the clocks array. If a device tree node requests this clock (e.g. 
clocks = <&videocc VIDEO_CC_SLEEP_CLK_SRC>), will it receive a missing clock
and fail to probe?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706-nord_videocc_camcc-v1-0-bae3be9e9770@oss.qualcomm.com?part=3