Re: [PATCH] clk: qcom: gcc-qcs615: Fix gcc_sdcc2_apps_clk_src

[Taniya Das <taniya.das@oss.qualcomm.com>]

On 8/12/2025 9:20 AM, Bjorn Andersson wrote:
> On Tue, Jul 29, 2025 at 04:08:21PM +0530, Taniya Das wrote:
>>
>>
>> On 7/29/2025 3:19 PM, Richa Bharti wrote:
>>> On QCS615, we see the same issue as reported on SM8250 and SM6350:
>>> "gcc_sdcc2_apps_clk_src: rcg didn't update its configuration" during boot.
>>> This is due to GPLL7 not being enabled by default as a parent clock.
>>>
>>> Setting `.flags = CLK_OPS_PARENT_ENABLE` ensures that the parent (GPLL7)
>>> gets prepared and enabled when switching to it, fixing the warning.
>>>
>>> Fixes: 39d6dcf67fe9 ("clk: qcom: gcc: Add support for QCS615 GCC clocks")
>>> Signed-off-by: Richa Bharti <Richa.Bharti@siemens.com>
> 
> Thank you Richa for your patch!
> 
>>> ---
>>> This change is similar to upstream commits:
>>> - SM8250: 783cb693828c ("clk: qcom: gcc-sm8250: Fix
>>>   gcc_sdcc2_apps_clk_src")
>>> - SM6350: df04d166d1f3 ("clk: qcom: gcc-sm6350: Fix
>>>   gcc_sdcc2_apps_clk_src")
>>> ---
>>>  drivers/clk/qcom/gcc-qcs615.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/clk/qcom/gcc-qcs615.c b/drivers/clk/qcom/gcc-qcs615.c
>>> index 9695446bc2a3..b281f0dfe165 100644
>>> --- a/drivers/clk/qcom/gcc-qcs615.c
>>> +++ b/drivers/clk/qcom/gcc-qcs615.c
>>> @@ -830,6 +830,7 @@ static struct clk_rcg2 gcc_sdcc2_apps_clk_src = {
>>>  		.name = "gcc_sdcc2_apps_clk_src",
>>>  		.parent_data = gcc_parent_data_8,
>>>  		.num_parents = ARRAY_SIZE(gcc_parent_data_8),
>>> +		.flags = CLK_OPS_PARENT_ENABLE,
>>
>> This is not the correct way to fix it just because SM8250 and SM6350
>> uses this ops.
>>
>> We are testing the right fix internally and will be posting.
>>
> 
> Please use such opportunities to educate us, rather than just tell us to
> blindly wait for something (at least share your thoughts/hypothesis).
> 

Sure, Bjorn.

https://patchwork.kernel.org/project/linux-clk/patch/20250804-sdcc_rcg2_shared_ops-v1-1-41f989e8cbb1@oss.qualcomm.com/

The RCG configuration goes to a bad state because the current floor_ops
tries to update the rcg configuration even if the clock is not enabled.

The correct fix is to ensure we do that when the clock is actually
enabled which is brought it by the shared_floor_ops.

Please let me know if you still have any queries.

> Regards,
> Bjorn
> 
>>>  		.ops = &clk_rcg2_floor_ops,
>>>  	},
>>>  };
>>
>> -- 
>> Thanks,
>> Taniya Das
>>

-- 
Thanks,
Taniya Das