[PATCH 1/8] clk/qcom: Add SM6350 clock driver

[Luca Weiss]

On Fri Jun 20, 2025 at 10:54 AM CEST, Neil Armstrong wrote:
> On 18/06/2025 16:25, Luca Weiss wrote:
>> Add Clock driver for the GCC block found in the SM6350 SoC.
>> 
>> Signed-off-by: Luca Weiss <luca.weiss at fairphone.com>
>> ---

<snip>

>> +static const struct freq_tbl ftbl_gcc_sdcc2_apps_clk_src[] = {
>> +	F(400000, CFG_CLK_SRC_CXO, 12, 1, 4),
>> +	F(9600000, CFG_CLK_SRC_CXO, 2, 0, 0),
>> +	F(19200000, CFG_CLK_SRC_CXO, 1, 0, 0),
>> +	F(25000000, CFG_CLK_SRC_GPLL0_ODD, 8, 0, 0),
>> +	F(50000000, CFG_CLK_SRC_GPLL0_ODD, 4, 0, 0),
>> +	F(100000000, CFG_CLK_SRC_GPLL0_ODD, 2, 0, 0),
>> +	F(202000000, CFG_CLK_SRC_GPLL7, 4, 0, 0),
>> +	{}
>> +};
>> +
>> +static struct pll_vote_clk gpll7_vote_clk = {
>> +	.status = APCS_GPLL7_STATUS,
>> +	.status_bit = BIT(31),
>> +	.ena_vote = APCS_GPLLX_ENA_REG,
>> +	.vote_bit = BIT(7),
>> +};
>> +
>> +static ulong sm6350_set_rate(struct clk *clk, ulong rate)
>> +{
>> +	struct msm_clk_priv *priv = dev_get_priv(clk->dev);
>> +	const struct freq_tbl *freq;
>> +
>> +	if (clk->id < priv->data->num_clks)
>> +		debug("%s: %s, requested rate=%ld\n", __func__,
>> +		      priv->data->clks[clk->id].name, rate);
>> +
>> +	switch (clk->id) {
>> +	case GCC_QUPV3_WRAP1_S3_CLK: /*UART9*/
>> +		freq = qcom_find_freq(ftbl_gcc_qupv3_wrap1_s3_clk_src, rate);
>> +		clk_rcg_set_rate_mnd(priv->base, GCC_SE12_UART_RCG_REG,
>> +				     freq->pre_div, freq->m, freq->n, freq->src,
>> +				     16);
>> +
>> +		return freq->freq;
>> +	case GCC_SDCC2_APPS_CLK:
>> +		/* Enable GPLL7 so that we can point SDCC2_APPS_CLK_SRC at it */
>> +		clk_enable_gpll0(priv->base, &gpll7_vote_clk);
>> +		freq = qcom_find_freq(ftbl_gcc_sdcc2_apps_clk_src, rate);
>> +		printf("%s: got freq %u\n", __func__, freq->freq);
>> +		WARN(freq->src != CFG_CLK_SRC_GPLL7,
>> +		     "SDCC2_APPS_CLK_SRC not set to GPLL7, requested rate %lu\n",
>> +		     rate);
>
> Why not enabling GPLL7 only if freq->src == CFG_CLK_SRC_GPLL7 ??

I honestly just copy-pasted this part, I didn't think too much about
this code. But also doesn't the code do the wrong thing when pointing to
any freq apart from 202 MHz? The warn and the clk_rcg_set_rate_mnd call
below will complain about/set the wrong parent?

Seems that construct is in clock drivers for qcm2290, sm6115 and sm8250.

Also other clk drivers like clock-sc7280.c don't have any set_rate
handling for this clock either and SD card supposedly works fine on the
Shiftphone 8.
 
>
> Apart that, looks good :-)
>
> Reviewed-by: Neil Armstrong <neil.armstrong at linaro.org>

Thanks!

Regards
Luca