diff mbox series

[04/13] clk: qcom: gpucc-sa8775p: Remove the CLK_IS_CRITICAL and ALWAYS_ON flags

Message ID 20240531090249.10293-5-quic_tdas@quicinc.com (mailing list archive)
State Superseded
Headers show
Series Add support for SA8775P Multimedia clock controllers | expand

Commit Message

Taniya Das May 31, 2024, 9:02 a.m. UTC
The gpu clocks and GDSC have been marked critical from the clock driver
which is not desired for functionality. Hence remove the CLK_IS_CRITICAL
and ALWAYS_ON flags.

Fixes: 0afa16afc36d ("clk: qcom: add the GPUCC driver for sa8775p")
Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
---
 drivers/clk/qcom/gpucc-sa8775p.c | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

Comments

Krzysztof Kozlowski May 31, 2024, 9:59 a.m. UTC | #1
On 31/05/2024 11:02, Taniya Das wrote:
> The gpu clocks and GDSC have been marked critical from the clock driver
> which is not desired for functionality. Hence remove the CLK_IS_CRITICAL
> and ALWAYS_ON flags.
> 
> Fixes: 0afa16afc36d ("clk: qcom: add the GPUCC driver for sa8775p")
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> ---
>  drivers/clk/qcom/gpucc-sa8775p.c | 27 +++++++++++----------------
>  1 file changed, 11 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/clk/qcom/gpucc-sa8775p.c b/drivers/clk/qcom/gpucc-sa8775p.c
> index 1167c42da39d..f965babf4330 100644
> --- a/drivers/clk/qcom/gpucc-sa8775p.c
> +++ b/drivers/clk/qcom/gpucc-sa8775p.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
> - * Copyright (c) 2021-2022, Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2021-2022, 2024, Qualcomm Innovation Center, Inc. All rights reserved.

That's not a fix.

>   * Copyright (c) 2023, Linaro Limited
>   */
>  
> @@ -280,7 +280,7 @@ static struct clk_branch gpu_cc_ahb_clk = {
>  				&gpu_cc_hub_ahb_div_clk_src.clkr.hw,
>  			},
>  			.num_parents = 1,
> -			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
> +			.flags = CLK_SET_RATE_PARENT,

I fail to see why this is a fix. They were marked as critical on
purpose. It was needed, wasn't it?

Provide jsutification for commits, not just sprinkle Fixes tag all around.


Best regards,
Krzysztof
Trilok Soni May 31, 2024, 5:46 p.m. UTC | #2
On 5/31/2024 2:59 AM, Krzysztof Kozlowski wrote:
> On 31/05/2024 11:02, Taniya Das wrote:
>> The gpu clocks and GDSC have been marked critical from the clock driver
>> which is not desired for functionality. Hence remove the CLK_IS_CRITICAL
>> and ALWAYS_ON flags.
>>
>> Fixes: 0afa16afc36d ("clk: qcom: add the GPUCC driver for sa8775p")
>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>> ---
>>  drivers/clk/qcom/gpucc-sa8775p.c | 27 +++++++++++----------------
>>  1 file changed, 11 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/gpucc-sa8775p.c b/drivers/clk/qcom/gpucc-sa8775p.c
>> index 1167c42da39d..f965babf4330 100644
>> --- a/drivers/clk/qcom/gpucc-sa8775p.c
>> +++ b/drivers/clk/qcom/gpucc-sa8775p.c
>> @@ -1,6 +1,6 @@
>>  // SPDX-License-Identifier: GPL-2.0-only
>>  /*
>> - * Copyright (c) 2021-2022, Qualcomm Innovation Center, Inc. All rights reserved.
>> + * Copyright (c) 2021-2022, 2024, Qualcomm Innovation Center, Inc. All rights reserved.
> 
> That's not a fix.
> 
>>   * Copyright (c) 2023, Linaro Limited
>>   */
>>  
>> @@ -280,7 +280,7 @@ static struct clk_branch gpu_cc_ahb_clk = {
>>  				&gpu_cc_hub_ahb_div_clk_src.clkr.hw,
>>  			},
>>  			.num_parents = 1,
>> -			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
>> +			.flags = CLK_SET_RATE_PARENT,
> 
> I fail to see why this is a fix. They were marked as critical on
> purpose. It was needed, wasn't it?
> 
> Provide jsutification for commits, not just sprinkle Fixes tag all around.

Taniya - please separate fixes into another series?
Bjorn Andersson June 2, 2024, 4:08 a.m. UTC | #3
On Fri, May 31, 2024 at 10:46:39AM GMT, Trilok Soni wrote:
> On 5/31/2024 2:59 AM, Krzysztof Kozlowski wrote:
> > On 31/05/2024 11:02, Taniya Das wrote:
> >> The gpu clocks and GDSC have been marked critical from the clock driver
> >> which is not desired for functionality. Hence remove the CLK_IS_CRITICAL
> >> and ALWAYS_ON flags.
> >>
> >> Fixes: 0afa16afc36d ("clk: qcom: add the GPUCC driver for sa8775p")
> >> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> >> ---
> >>  drivers/clk/qcom/gpucc-sa8775p.c | 27 +++++++++++----------------
> >>  1 file changed, 11 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/drivers/clk/qcom/gpucc-sa8775p.c b/drivers/clk/qcom/gpucc-sa8775p.c
> >> index 1167c42da39d..f965babf4330 100644
> >> --- a/drivers/clk/qcom/gpucc-sa8775p.c
> >> +++ b/drivers/clk/qcom/gpucc-sa8775p.c
> >> @@ -1,6 +1,6 @@
> >>  // SPDX-License-Identifier: GPL-2.0-only
> >>  /*
> >> - * Copyright (c) 2021-2022, Qualcomm Innovation Center, Inc. All rights reserved.
> >> + * Copyright (c) 2021-2022, 2024, Qualcomm Innovation Center, Inc. All rights reserved.
> > 
> > That's not a fix.
> > 
> >>   * Copyright (c) 2023, Linaro Limited
> >>   */
> >>  
> >> @@ -280,7 +280,7 @@ static struct clk_branch gpu_cc_ahb_clk = {
> >>  				&gpu_cc_hub_ahb_div_clk_src.clkr.hw,
> >>  			},
> >>  			.num_parents = 1,
> >> -			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
> >> +			.flags = CLK_SET_RATE_PARENT,
> > 
> > I fail to see why this is a fix. They were marked as critical on
> > purpose. It was needed, wasn't it?
> > 
> > Provide jsutification for commits, not just sprinkle Fixes tag all around.
> 
> Taniya - please separate fixes into another series? 
> 

There's no problem including fixes in a series with other changes, but
keeping them at the beginning of the series is generally preferred - as
this both enable them being picked for some -fixes branch and avoid
potential issues when backporting.

Regards,
Bjorn
Bjorn Andersson June 2, 2024, 4:12 a.m. UTC | #4
On Fri, May 31, 2024 at 11:59:04AM GMT, Krzysztof Kozlowski wrote:
> On 31/05/2024 11:02, Taniya Das wrote:
> > The gpu clocks and GDSC have been marked critical from the clock driver
> > which is not desired for functionality. Hence remove the CLK_IS_CRITICAL
> > and ALWAYS_ON flags.
> > 
> > Fixes: 0afa16afc36d ("clk: qcom: add the GPUCC driver for sa8775p")
> > Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> > ---
> >  drivers/clk/qcom/gpucc-sa8775p.c | 27 +++++++++++----------------
> >  1 file changed, 11 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/clk/qcom/gpucc-sa8775p.c b/drivers/clk/qcom/gpucc-sa8775p.c
> > index 1167c42da39d..f965babf4330 100644
> > --- a/drivers/clk/qcom/gpucc-sa8775p.c
> > +++ b/drivers/clk/qcom/gpucc-sa8775p.c
> > @@ -1,6 +1,6 @@
> >  // SPDX-License-Identifier: GPL-2.0-only
> >  /*
> > - * Copyright (c) 2021-2022, Qualcomm Innovation Center, Inc. All rights reserved.
> > + * Copyright (c) 2021-2022, 2024, Qualcomm Innovation Center, Inc. All rights reserved.
> 
> That's not a fix.
> 
> >   * Copyright (c) 2023, Linaro Limited
> >   */
> >  
> > @@ -280,7 +280,7 @@ static struct clk_branch gpu_cc_ahb_clk = {
> >  				&gpu_cc_hub_ahb_div_clk_src.clkr.hw,
> >  			},
> >  			.num_parents = 1,
> > -			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
> > +			.flags = CLK_SET_RATE_PARENT,
> 
> I fail to see why this is a fix. They were marked as critical on
> purpose. It was needed, wasn't it?
> 
> Provide jsutification for commits, not just sprinkle Fixes tag all around.
> 

This is indeed a fix, as marking clocks CLK_IS_CRITICAL prevents any
power-domain associated with the clock controller from suspending. In
other words, the current behavior is broken.

@Taniya, "not desired for functionality" does not carry any useful
information explaining why this change is made. Please update the commit
message.

Regards,
Bjorn

> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski June 2, 2024, 3:28 p.m. UTC | #5
On 02/06/2024 06:12, Bjorn Andersson wrote:
> On Fri, May 31, 2024 at 11:59:04AM GMT, Krzysztof Kozlowski wrote:
>> On 31/05/2024 11:02, Taniya Das wrote:
>>> The gpu clocks and GDSC have been marked critical from the clock driver
>>> which is not desired for functionality. Hence remove the CLK_IS_CRITICAL
>>> and ALWAYS_ON flags.
>>>
>>> Fixes: 0afa16afc36d ("clk: qcom: add the GPUCC driver for sa8775p")
>>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>>> ---
>>>  drivers/clk/qcom/gpucc-sa8775p.c | 27 +++++++++++----------------
>>>  1 file changed, 11 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/clk/qcom/gpucc-sa8775p.c b/drivers/clk/qcom/gpucc-sa8775p.c
>>> index 1167c42da39d..f965babf4330 100644
>>> --- a/drivers/clk/qcom/gpucc-sa8775p.c
>>> +++ b/drivers/clk/qcom/gpucc-sa8775p.c
>>> @@ -1,6 +1,6 @@
>>>  // SPDX-License-Identifier: GPL-2.0-only
>>>  /*
>>> - * Copyright (c) 2021-2022, Qualcomm Innovation Center, Inc. All rights reserved.
>>> + * Copyright (c) 2021-2022, 2024, Qualcomm Innovation Center, Inc. All rights reserved.
>>
>> That's not a fix.
>>
>>>   * Copyright (c) 2023, Linaro Limited
>>>   */
>>>  
>>> @@ -280,7 +280,7 @@ static struct clk_branch gpu_cc_ahb_clk = {
>>>  				&gpu_cc_hub_ahb_div_clk_src.clkr.hw,
>>>  			},
>>>  			.num_parents = 1,
>>> -			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
>>> +			.flags = CLK_SET_RATE_PARENT,
>>
>> I fail to see why this is a fix. They were marked as critical on
>> purpose. It was needed, wasn't it?
>>
>> Provide jsutification for commits, not just sprinkle Fixes tag all around.
>>
> 
> This is indeed a fix, as marking clocks CLK_IS_CRITICAL prevents any
> power-domain associated with the clock controller from suspending. In
> other words, the current behavior is broken.
> 
> @Taniya, "not desired for functionality" does not carry any useful
> information explaining why this change is made. Please update the commit
> message.

Then please provide some sort of bug explanation in the commit msg. I
assume the clocks were marked critical to solve some particular problem,
e.g. missing parents, so marking this as fix sounds like both incorrect
and error-prone when backported. Maybe that's not the case, so this is
why there is commit msg to explain such details...

Best regards,
Krzysztof
Trilok Soni June 2, 2024, 5:58 p.m. UTC | #6
On 6/1/2024 9:08 PM, Bjorn Andersson wrote:
>>> I fail to see why this is a fix. They were marked as critical on
>>> purpose. It was needed, wasn't it?
>>>
>>> Provide jsutification for commits, not just sprinkle Fixes tag all around.
>> Taniya - please separate fixes into another series? 
>>
> There's no problem including fixes in a series with other changes, but
> keeping them at the beginning of the series is generally preferred - as
> this both enable them being picked for some -fixes branch and avoid
> potential issues when backporting.

Sure, but I prefer that these fixes are not blocked by open items
and feedback to other drivers in the series. Once the fixes are reviewed
and picked up independently of the drivers later in the series then I am fine.
Taniya Das June 10, 2024, 9:06 a.m. UTC | #7
On 6/2/2024 9:42 AM, Bjorn Andersson wrote:
> On Fri, May 31, 2024 at 11:59:04AM GMT, Krzysztof Kozlowski wrote:
>> On 31/05/2024 11:02, Taniya Das wrote:
>>> The gpu clocks and GDSC have been marked critical from the clock driver
>>> which is not desired for functionality. Hence remove the CLK_IS_CRITICAL
>>> and ALWAYS_ON flags.
>>>
>>> Fixes: 0afa16afc36d ("clk: qcom: add the GPUCC driver for sa8775p")
>>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>>> ---
>>>   drivers/clk/qcom/gpucc-sa8775p.c | 27 +++++++++++----------------
>>>   1 file changed, 11 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/clk/qcom/gpucc-sa8775p.c b/drivers/clk/qcom/gpucc-sa8775p.c
>>> index 1167c42da39d..f965babf4330 100644
>>> --- a/drivers/clk/qcom/gpucc-sa8775p.c
>>> +++ b/drivers/clk/qcom/gpucc-sa8775p.c
>>> @@ -1,6 +1,6 @@
>>>   // SPDX-License-Identifier: GPL-2.0-only
>>>   /*
>>> - * Copyright (c) 2021-2022, Qualcomm Innovation Center, Inc. All rights reserved.
>>> + * Copyright (c) 2021-2022, 2024, Qualcomm Innovation Center, Inc. All rights reserved.
>>
>> That's not a fix.
>>
>>>    * Copyright (c) 2023, Linaro Limited
>>>    */
>>>   
>>> @@ -280,7 +280,7 @@ static struct clk_branch gpu_cc_ahb_clk = {
>>>   				&gpu_cc_hub_ahb_div_clk_src.clkr.hw,
>>>   			},
>>>   			.num_parents = 1,
>>> -			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
>>> +			.flags = CLK_SET_RATE_PARENT,
>>
>> I fail to see why this is a fix. They were marked as critical on
>> purpose. It was needed, wasn't it?
>>
>> Provide jsutification for commits, not just sprinkle Fixes tag all around.
>>
> 
> This is indeed a fix, as marking clocks CLK_IS_CRITICAL prevents any
> power-domain associated with the clock controller from suspending. In
> other words, the current behavior is broken.
> 
> @Taniya, "not desired for functionality" does not carry any useful
> information explaining why this change is made. Please update the commit
> message.
> 

Sure, Bjorn, will update the commit in the next series.

The GPU clocks/GDSCs have been marked critical from the clock driver
but the GPU driver votes on these resources as per the HW requirement.
We don't require these clocks and GDSC's to be kept always ON which 
would have power impact and also GPU stability/corruptions.
Hence remove the CLK_IS_CRITICAL and ALWAYS_ON flags.

> Regards,
> Bjorn
> 
>>
>> Best regards,
>> Krzysztof
>>
Taniya Das June 10, 2024, 9:10 a.m. UTC | #8
On 6/2/2024 8:58 PM, Krzysztof Kozlowski wrote:
> On 02/06/2024 06:12, Bjorn Andersson wrote:
>> On Fri, May 31, 2024 at 11:59:04AM GMT, Krzysztof Kozlowski wrote:
>>> On 31/05/2024 11:02, Taniya Das wrote:
>>>> The gpu clocks and GDSC have been marked critical from the clock driver
>>>> which is not desired for functionality. Hence remove the CLK_IS_CRITICAL
>>>> and ALWAYS_ON flags.
>>>>
>>>> Fixes: 0afa16afc36d ("clk: qcom: add the GPUCC driver for sa8775p")
>>>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>>>> ---
>>>>   drivers/clk/qcom/gpucc-sa8775p.c | 27 +++++++++++----------------
>>>>   1 file changed, 11 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/qcom/gpucc-sa8775p.c b/drivers/clk/qcom/gpucc-sa8775p.c
>>>> index 1167c42da39d..f965babf4330 100644
>>>> --- a/drivers/clk/qcom/gpucc-sa8775p.c
>>>> +++ b/drivers/clk/qcom/gpucc-sa8775p.c
>>>> @@ -1,6 +1,6 @@
>>>>   // SPDX-License-Identifier: GPL-2.0-only
>>>>   /*
>>>> - * Copyright (c) 2021-2022, Qualcomm Innovation Center, Inc. All rights reserved.
>>>> + * Copyright (c) 2021-2022, 2024, Qualcomm Innovation Center, Inc. All rights reserved.
>>>
>>> That's not a fix.
>>>
>>>>    * Copyright (c) 2023, Linaro Limited
>>>>    */
>>>>   
>>>> @@ -280,7 +280,7 @@ static struct clk_branch gpu_cc_ahb_clk = {
>>>>   				&gpu_cc_hub_ahb_div_clk_src.clkr.hw,
>>>>   			},
>>>>   			.num_parents = 1,
>>>> -			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
>>>> +			.flags = CLK_SET_RATE_PARENT,
>>>
>>> I fail to see why this is a fix. They were marked as critical on
>>> purpose. It was needed, wasn't it?
>>>
>>> Provide jsutification for commits, not just sprinkle Fixes tag all around.
>>>
>>
>> This is indeed a fix, as marking clocks CLK_IS_CRITICAL prevents any
>> power-domain associated with the clock controller from suspending. In
>> other words, the current behavior is broken.
>>
>> @Taniya, "not desired for functionality" does not carry any useful
>> information explaining why this change is made. Please update the commit
>> message.
> 
> Then please provide some sort of bug explanation in the commit msg. I
> assume the clocks were marked critical to solve some particular problem,
> e.g. missing parents, so marking this as fix sounds like both incorrect
> and error-prone when backported. Maybe that's not the case, so this is
> why there is commit msg to explain such details...
> 

Thanks for your review. Sure, I will update the next series with the 
below details.

The GPU clocks/GDSCs have been marked critical from the clock driver
but the GPU driver votes on these resources as per the HW requirement.
We don't require these clocks and GDSC's to be kept always ON which 
would have power impact and also GPU stability/corruptions.
Hence remove the CLK_IS_CRITICAL and ALWAYS_ON flags.

But I am not sure why the original author left the clocks/GDSCs always ON.


> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/drivers/clk/qcom/gpucc-sa8775p.c b/drivers/clk/qcom/gpucc-sa8775p.c
index 1167c42da39d..f965babf4330 100644
--- a/drivers/clk/qcom/gpucc-sa8775p.c
+++ b/drivers/clk/qcom/gpucc-sa8775p.c
@@ -1,6 +1,6 @@ 
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Copyright (c) 2021-2022, Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2021-2022, 2024, Qualcomm Innovation Center, Inc. All rights reserved.
  * Copyright (c) 2023, Linaro Limited
  */
 
@@ -280,7 +280,7 @@  static struct clk_branch gpu_cc_ahb_clk = {
 				&gpu_cc_hub_ahb_div_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
-			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
+			.flags = CLK_SET_RATE_PARENT,
 			.ops = &clk_branch2_ops,
 		},
 	},
@@ -294,7 +294,6 @@  static struct clk_branch gpu_cc_cb_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(const struct clk_init_data){
 			.name = "gpu_cc_cb_clk",
-			.flags = CLK_IS_CRITICAL,
 			.ops = &clk_branch2_ops,
 		},
 	},
@@ -312,7 +311,7 @@  static struct clk_branch gpu_cc_crc_ahb_clk = {
 				&gpu_cc_hub_ahb_div_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
-			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
+			.flags = CLK_SET_RATE_PARENT,
 			.ops = &clk_branch2_ops,
 		},
 	},
@@ -330,7 +329,7 @@  static struct clk_branch gpu_cc_cx_ff_clk = {
 				&gpu_cc_ff_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
-			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
+			.flags = CLK_SET_RATE_PARENT,
 			.ops = &clk_branch2_ops,
 		},
 	},
@@ -348,7 +347,7 @@  static struct clk_branch gpu_cc_cx_gmu_clk = {
 				&gpu_cc_gmu_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
-			.flags =  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
+			.flags =  CLK_SET_RATE_PARENT,
 			.ops = &clk_branch2_aon_ops,
 		},
 	},
@@ -362,7 +361,6 @@  static struct clk_branch gpu_cc_cx_snoc_dvm_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(const struct clk_init_data){
 			.name = "gpu_cc_cx_snoc_dvm_clk",
-			.flags = CLK_IS_CRITICAL,
 			.ops = &clk_branch2_ops,
 		},
 	},
@@ -380,7 +378,7 @@  static struct clk_branch gpu_cc_cxo_aon_clk = {
 				&gpu_cc_xo_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
-			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
+			.flags = CLK_SET_RATE_PARENT,
 			.ops = &clk_branch2_ops,
 		},
 	},
@@ -398,7 +396,7 @@  static struct clk_branch gpu_cc_cxo_clk = {
 				&gpu_cc_xo_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
-			.flags =  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
+			.flags =  CLK_SET_RATE_PARENT,
 			.ops = &clk_branch2_ops,
 		},
 	},
@@ -416,7 +414,7 @@  static struct clk_branch gpu_cc_demet_clk = {
 				&gpu_cc_demet_div_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
-			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
+			.flags = CLK_SET_RATE_PARENT,
 			.ops = &clk_branch2_aon_ops,
 		},
 	},
@@ -430,7 +428,6 @@  static struct clk_branch gpu_cc_hlos1_vote_gpu_smmu_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(const struct clk_init_data){
 			.name = "gpu_cc_hlos1_vote_gpu_smmu_clk",
-			.flags = CLK_IS_CRITICAL,
 			.ops = &clk_branch2_ops,
 		},
 	},
@@ -448,7 +445,7 @@  static struct clk_branch gpu_cc_hub_aon_clk = {
 				&gpu_cc_hub_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
-			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
+			.flags = CLK_SET_RATE_PARENT,
 			.ops = &clk_branch2_aon_ops,
 		},
 	},
@@ -466,7 +463,7 @@  static struct clk_branch gpu_cc_hub_cx_int_clk = {
 				&gpu_cc_hub_cx_int_div_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
-			.flags =  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
+			.flags =  CLK_SET_RATE_PARENT,
 			.ops = &clk_branch2_aon_ops,
 		},
 	},
@@ -480,7 +477,6 @@  static struct clk_branch gpu_cc_memnoc_gfx_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(const struct clk_init_data){
 			.name = "gpu_cc_memnoc_gfx_clk",
-			.flags = CLK_IS_CRITICAL,
 			.ops = &clk_branch2_ops,
 		},
 	},
@@ -494,7 +490,6 @@  static struct clk_branch gpu_cc_sleep_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(const struct clk_init_data){
 			.name = "gpu_cc_sleep_clk",
-			.flags = CLK_IS_CRITICAL,
 			.ops = &clk_branch2_ops,
 		},
 	},
@@ -533,7 +528,7 @@  static struct gdsc cx_gdsc = {
 		.name = "cx_gdsc",
 	},
 	.pwrsts = PWRSTS_OFF_ON,
-	.flags = VOTABLE | RETAIN_FF_ENABLE | ALWAYS_ON,
+	.flags = VOTABLE | RETAIN_FF_ENABLE,
 };
 
 static struct gdsc gx_gdsc = {