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

[Neil Armstrong]

On 23/07/2025 13:41, Luca Weiss wrote:
> Hi Neil,
> 
> On Fri Jun 20, 2025 at 11:07 AM CEST, Luca Weiss wrote:
>> 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.
> 
> Do you have any feedback how the clock drivers would be correct?

Nothing precise, but I guess we can fix this later on, it's not wrong as-is,
just not optimal.

Neil

> 
> Regards
> Luca