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

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

[Richa Bharti]

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>
---
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,
 		.ops = &clk_rcg2_floor_ops,
 	},
 };
-- 
2.39.5

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

[Taniya Das]

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>
> ---
> 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.

>  		.ops = &clk_rcg2_floor_ops,
>  	},
>  };

-- 
Thanks,
Taniya Das

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

[Bjorn Andersson]

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).

Regards,
Bjorn

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

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

[Taniya Das]

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

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

[Bjorn Andersson]

On Tue, Aug 12, 2025 at 12:34:02PM +0530, Taniya Das wrote:
> 
> 
> 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.
> 

Excellent, thanks for the explanation.

> Please let me know if you still have any queries.
> 

No concerns, this is exactly the kind of transparency I was looking for.

Many thanks for the quick reply,
Bjorn

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