clk: qcom: sc8180x: PM-related fixes (and refactoring)

[PATCH v2 10/11] clk: qcom: camcc-sc8180x: Add missing HW_CTRL GDSC flag

Posted by Val Packett 3 weeks, 5 days ago

In all other Qualcomm SoC camcc drivers, the BPS and IPE GDSCs use the
HW_CTRL flag, but it was missing on SC8180X.

Fix by setting it on all applicable GDSC entries.

Fixes: 691f3413baa4 ("clk: qcom: camcc-sc8180x: Add SC8180X camera clock controller driver")
Signed-off-by: Val Packett <val@packett.cool>
---
 drivers/clk/qcom/camcc-sc8180x.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/qcom/camcc-sc8180x.c b/drivers/clk/qcom/camcc-sc8180x.c
index cd4c40a81c28..67b2055bd212 100644
--- a/drivers/clk/qcom/camcc-sc8180x.c
+++ b/drivers/clk/qcom/camcc-sc8180x.c
@@ -2555,7 +2555,7 @@ static struct gdsc bps_gdsc = {
 	},
 	.pwrsts = PWRSTS_OFF_ON,
 	.parent = &titan_top_gdsc.pd,
-	.flags = POLL_CFG_GDSCR,
+	.flags = HW_CTRL | POLL_CFG_GDSCR,
 };
 
 static struct gdsc ife_0_gdsc = {
@@ -2620,7 +2620,7 @@ static struct gdsc ipe_0_gdsc = {
 	},
 	.pwrsts = PWRSTS_OFF_ON,
 	.parent = &titan_top_gdsc.pd,
-	.flags = POLL_CFG_GDSCR,
+	.flags = HW_CTRL | POLL_CFG_GDSCR,
 };
 
 static struct gdsc ipe_1_gdsc = {
@@ -2633,7 +2633,7 @@ static struct gdsc ipe_1_gdsc = {
 	},
 	.pwrsts = PWRSTS_OFF_ON,
 	.parent = &titan_top_gdsc.pd,
-	.flags = POLL_CFG_GDSCR,
+	.flags = HW_CTRL | POLL_CFG_GDSCR,
 };
 
 static struct clk_regmap *cam_cc_sc8180x_clocks[] = {
-- 
2.52.0

Re: [PATCH v2 10/11] clk: qcom: camcc-sc8180x: Add missing HW_CTRL GDSC flag

Posted by Konrad Dybcio 3 weeks, 4 days ago

On 3/12/26 12:12 PM, Val Packett wrote:
> In all other Qualcomm SoC camcc drivers, the BPS and IPE GDSCs use the
> HW_CTRL flag, but it was missing on SC8180X.
> 
> Fix by setting it on all applicable GDSC entries.
> 
> Fixes: 691f3413baa4 ("clk: qcom: camcc-sc8180x: Add SC8180X camera clock controller driver")
> Signed-off-by: Val Packett <val@packett.cool>
> ---
>  drivers/clk/qcom/camcc-sc8180x.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/qcom/camcc-sc8180x.c b/drivers/clk/qcom/camcc-sc8180x.c
> index cd4c40a81c28..67b2055bd212 100644
> --- a/drivers/clk/qcom/camcc-sc8180x.c
> +++ b/drivers/clk/qcom/camcc-sc8180x.c
> @@ -2555,7 +2555,7 @@ static struct gdsc bps_gdsc = {
>  	},
>  	.pwrsts = PWRSTS_OFF_ON,
>  	.parent = &titan_top_gdsc.pd,
> -	.flags = POLL_CFG_GDSCR,
> +	.flags = HW_CTRL | POLL_CFG_GDSCR,

FWIW downstream seems to use HW_MODE at runtime and SW_MODE+collapse on
suspend for both BPS and IPE, so perhaps HW_CTRL_TRIGGER (and a patch to
the driver) would be better suited

Konrad

Re: [PATCH v2 10/11] clk: qcom: camcc-sc8180x: Add missing HW_CTRL GDSC flag

Posted by Val Packett 3 weeks, 3 days ago

On 3/13/26 7:53 AM, Konrad Dybcio wrote:
> On 3/12/26 12:12 PM, Val Packett wrote:
>> In all other Qualcomm SoC camcc drivers, the BPS and IPE GDSCs use the
>> HW_CTRL flag, but it was missing on SC8180X.
>>
>> Fix by setting it on all applicable GDSC entries.
>>
>> Fixes: 691f3413baa4 ("clk: qcom: camcc-sc8180x: Add SC8180X camera clock controller driver")
>> Signed-off-by: Val Packett <val@packett.cool>
>> ---
>>   drivers/clk/qcom/camcc-sc8180x.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/camcc-sc8180x.c b/drivers/clk/qcom/camcc-sc8180x.c
>> index cd4c40a81c28..67b2055bd212 100644
>> --- a/drivers/clk/qcom/camcc-sc8180x.c
>> +++ b/drivers/clk/qcom/camcc-sc8180x.c
>> @@ -2555,7 +2555,7 @@ static struct gdsc bps_gdsc = {
>>   	},
>>   	.pwrsts = PWRSTS_OFF_ON,
>>   	.parent = &titan_top_gdsc.pd,
>> -	.flags = POLL_CFG_GDSCR,
>> +	.flags = HW_CTRL | POLL_CFG_GDSCR,
> FWIW downstream seems to use HW_MODE at runtime and SW_MODE+collapse on
> suspend for both BPS and IPE, so perhaps HW_CTRL_TRIGGER (and a patch to
> the driver) would be better suited

Hmm what exactly would that "patch to the driver" be? Not sure I 
understood that part…

~val

Re: [PATCH v2 10/11] clk: qcom: camcc-sc8180x: Add missing HW_CTRL GDSC flag

Posted by Konrad Dybcio 3 weeks, 1 day ago

On 3/14/26 1:13 AM, Val Packett wrote:
> 
> On 3/13/26 7:53 AM, Konrad Dybcio wrote:
>> On 3/12/26 12:12 PM, Val Packett wrote:
>>> In all other Qualcomm SoC camcc drivers, the BPS and IPE GDSCs use the
>>> HW_CTRL flag, but it was missing on SC8180X.
>>>
>>> Fix by setting it on all applicable GDSC entries.
>>>
>>> Fixes: 691f3413baa4 ("clk: qcom: camcc-sc8180x: Add SC8180X camera clock controller driver")
>>> Signed-off-by: Val Packett <val@packett.cool>
>>> ---
>>>   drivers/clk/qcom/camcc-sc8180x.c | 6 +++---
>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/clk/qcom/camcc-sc8180x.c b/drivers/clk/qcom/camcc-sc8180x.c
>>> index cd4c40a81c28..67b2055bd212 100644
>>> --- a/drivers/clk/qcom/camcc-sc8180x.c
>>> +++ b/drivers/clk/qcom/camcc-sc8180x.c
>>> @@ -2555,7 +2555,7 @@ static struct gdsc bps_gdsc = {
>>>       },
>>>       .pwrsts = PWRSTS_OFF_ON,
>>>       .parent = &titan_top_gdsc.pd,
>>> -    .flags = POLL_CFG_GDSCR,
>>> +    .flags = HW_CTRL | POLL_CFG_GDSCR,
>> FWIW downstream seems to use HW_MODE at runtime and SW_MODE+collapse on
>> suspend for both BPS and IPE, so perhaps HW_CTRL_TRIGGER (and a patch to
>> the driver) would be better suited
> 
> Hmm what exactly would that "patch to the driver" be? Not sure I understood that part…

There's now a dev_pm_genpd_set_hwmode() that on GDSCs translates to
toggling the HW_CTRL bit in the register at runtime

Konrad