Re: [PATCH v11 2/5] clk: qcom: Add Global Clock Controller (GCC) driver for SM7250

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] clk: qcom: Incorrect source enum in freq_tbl causes cpuss_ahb_clk_src rate setting failure
--

commit 457d324324dea631af01284e949d55a40199a579
Author: Sreeshankar K <sreeshankar0910@gmail.com>

clk: qcom: Add Global Clock Controller (GCC) driver for SM7250

Add support for the global clock controller found on SM7250
based devices.

> diff --git a/drivers/clk/qcom/gcc-sm7250.c b/drivers/clk/qcom/gcc-sm7250.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/clk/qcom/gcc-sm7250.c
[ ... ]
> +static const struct freq_tbl ftbl_gcc_cpuss_ahb_clk_src[] = {
> +	F(19200000, DT_BI_TCXO_AO, 1, 0, 0),
> +	{ }
> +};
> +
> +static struct clk_rcg2 gcc_cpuss_ahb_clk_src = {
> +	.cmd_rcgr = 0x48010,
> +	.mnd_width = 0,
> +	.hid_width = 5,
> +	.parent_map = gcc_parent_map_0,
> +	.freq_tbl = ftbl_gcc_cpuss_ahb_clk_src,
> +	.clkr.hw.init = &(const struct clk_init_data){
> +		.name = "gcc_cpuss_ahb_clk_src",
> +		.parent_data = gcc_parent_data_0_ao,
> +		.num_parents = ARRAY_SIZE(gcc_parent_data_0_ao),
> +		.flags = CLK_SET_RATE_PARENT,
> +		.ops = &clk_rcg2_ops,
> +	},
> +};

[Severity: High]
Will this cause a rate setting failure for gcc_cpuss_ahb_clk_src?

The frequency table ftbl_gcc_cpuss_ahb_clk_src uses the device tree index 
DT_BI_TCXO_AO (which evaluates to 1) instead of the internal hardware parent 
macro P_BI_TCXO (which evaluates to 0).

The associated clock gcc_cpuss_ahb_clk_src restricts its num_parents to 1 
via gcc_parent_data_0_ao, and uses gcc_parent_map_0 as its parent map. 
During a rate change, the common clock framework invokes qcom_find_src_index() 
which iterates up to num_parents (1) and checks if the requested source 
matches map[0].src (P_BI_TCXO = 0).

Since 1 != 0, the lookup fails and returns -ENOENT, preventing rate 
configuration on the CPUSS AHB clock. Should the frequency table use 
P_BI_TCXO instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016042432.824-1-sreeshankar0910@gmail.com?part=2