[PATCH V2] clk: qcom: camcc-sc7180: Add parent dependency to all camera GDSCs

[PATCH V2] clk: qcom: camcc-sc7180: Add parent dependency to all camera GDSCs

[Taniya Das]

Camera titan top GDSC is a parent supply to all other camera GDSCs. Titan
top GDSC is required to be enabled before enabling any other camera GDSCs
and it should be disabled only after all other camera GDSCs are disabled.
Ensure this behavior by marking titan top GDSC as parent of all other
camera GDSCs.

Fixes: 15d09e830bbc ("clk: qcom: camcc: Add camera clock controller driver for SC7180")
Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
---
Changes since v1:
 - Expanded the commit text and added fixes tag.

 drivers/clk/qcom/camcc-sc7180.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/qcom/camcc-sc7180.c b/drivers/clk/qcom/camcc-sc7180.c
index e2b4804695f3..8a4ba7a19ed1 100644
--- a/drivers/clk/qcom/camcc-sc7180.c
+++ b/drivers/clk/qcom/camcc-sc7180.c
@@ -1480,12 +1480,21 @@ static struct clk_branch cam_cc_sys_tmr_clk = {
 	},
 };

+static struct gdsc titan_top_gdsc = {
+	.gdscr = 0xb134,
+	.pd = {
+		.name = "titan_top_gdsc",
+	},
+	.pwrsts = PWRSTS_OFF_ON,
+};
+
 static struct gdsc bps_gdsc = {
 	.gdscr = 0x6004,
 	.pd = {
 		.name = "bps_gdsc",
 	},
 	.pwrsts = PWRSTS_OFF_ON,
+	.parent = &titan_top_gdsc.pd,
 	.flags = HW_CTRL,
 };

@@ -1495,6 +1504,7 @@ static struct gdsc ife_0_gdsc = {
 		.name = "ife_0_gdsc",
 	},
 	.pwrsts = PWRSTS_OFF_ON,
+	.parent = &titan_top_gdsc.pd,
 };

 static struct gdsc ife_1_gdsc = {
@@ -1503,6 +1513,7 @@ static struct gdsc ife_1_gdsc = {
 		.name = "ife_1_gdsc",
 	},
 	.pwrsts = PWRSTS_OFF_ON,
+	.parent = &titan_top_gdsc.pd,
 };

 static struct gdsc ipe_0_gdsc = {
@@ -1512,15 +1523,9 @@ static struct gdsc ipe_0_gdsc = {
 	},
 	.pwrsts = PWRSTS_OFF_ON,
 	.flags = HW_CTRL,
+	.parent = &titan_top_gdsc.pd,
 };

-static struct gdsc titan_top_gdsc = {
-	.gdscr = 0xb134,
-	.pd = {
-		.name = "titan_top_gdsc",
-	},
-	.pwrsts = PWRSTS_OFF_ON,
-};

 static struct clk_hw *cam_cc_sc7180_hws[] = {
 	[CAM_CC_PLL2_OUT_EARLY] = &cam_cc_pll2_out_early.hw,
--
2.17.1

Re: [PATCH V2] clk: qcom: camcc-sc7180: Add parent dependency to all camera GDSCs

[Bryan O'Donoghue]

On 01/05/2023 15:29, Taniya Das wrote:
> Camera titan top GDSC is a parent supply to all other camera GDSCs. Titan
> top GDSC is required to be enabled before enabling any other camera GDSCs
> and it should be disabled only after all other camera GDSCs are disabled.
> Ensure this behavior by marking titan top GDSC as parent of all other
> camera GDSCs.
> 
> Fixes: 15d09e830bbc ("clk: qcom: camcc: Add camera clock controller driver for SC7180")
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>


Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Re: [PATCH V2] clk: qcom: camcc-sc7180: Add parent dependency to all camera GDSCs

[Stephen Boyd]

Quoting Taniya Das (2023-05-01 07:29:32)
> Camera titan top GDSC is a parent supply to all other camera GDSCs. Titan
> top GDSC is required to be enabled before enabling any other camera GDSCs
> and it should be disabled only after all other camera GDSCs are disabled.
> Ensure this behavior by marking titan top GDSC as parent of all other
> camera GDSCs.
> 
> Fixes: 15d09e830bbc ("clk: qcom: camcc: Add camera clock controller driver for SC7180")
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> ---

Acked-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH V2] clk: qcom: camcc-sc7180: Add parent dependency to all camera GDSCs

[Stephen Boyd]

Quoting Taniya Das (2023-05-01 07:29:32)
> Camera titan top GDSC is a parent supply to all other camera GDSCs. Titan
> top GDSC is required to be enabled before enabling any other camera GDSCs
> and it should be disabled only after all other camera GDSCs are disabled.
> Ensure this behavior by marking titan top GDSC as parent of all other
> camera GDSCs.
> 
> Fixes: 15d09e830bbc ("clk: qcom: camcc: Add camera clock controller driver for SC7180")
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>

Is something broken right now? The commit text doesn't tell me if we
need to backport this to stable kernels or merge it as soon as possible.
What's the priority of this fix?

Re: [PATCH V2] clk: qcom: camcc-sc7180: Add parent dependency to all camera GDSCs

[Satya Priya Kakitapalli (Temp)]

Hi Stephen,

On 5/11/2023 1:02 AM, Stephen Boyd wrote:
> Quoting Taniya Das (2023-05-01 07:29:32)
>> Camera titan top GDSC is a parent supply to all other camera GDSCs. Titan
>> top GDSC is required to be enabled before enabling any other camera GDSCs
>> and it should be disabled only after all other camera GDSCs are disabled.
>> Ensure this behavior by marking titan top GDSC as parent of all other
>> camera GDSCs.
>>
>> Fixes: 15d09e830bbc ("clk: qcom: camcc: Add camera clock controller driver for SC7180")
>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> Is something broken right now? The commit text doesn't tell me if we
> need to backport this to stable kernels or merge it as soon as possible.
> What's the priority of this fix?


Modularization of camx driver triggered this issue now, but there could 
be some other scenarios which could trigger the same issue on stable 
kernels.  Hence it needs to be back ported to stable kernels.

Re: [PATCH V2] clk: qcom: camcc-sc7180: Add parent dependency to all camera GDSCs

[Bryan O'Donoghue]

On 26/05/2023 12:10, Satya Priya Kakitapalli (Temp) wrote:
> Hi Stephen,
> 
> On 5/11/2023 1:02 AM, Stephen Boyd wrote:
>> Quoting Taniya Das (2023-05-01 07:29:32)
>>> Camera titan top GDSC is a parent supply to all other camera GDSCs. 
>>> Titan
>>> top GDSC is required to be enabled before enabling any other camera 
>>> GDSCs
>>> and it should be disabled only after all other camera GDSCs are 
>>> disabled.
>>> Ensure this behavior by marking titan top GDSC as parent of all other
>>> camera GDSCs.
>>>
>>> Fixes: 15d09e830bbc ("clk: qcom: camcc: Add camera clock controller 
>>> driver for SC7180")
>>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>> Is something broken right now? The commit text doesn't tell me if we
>> need to backport this to stable kernels or merge it as soon as possible.
>> What's the priority of this fix?
> 
> 
> Modularization of camx driver triggered this issue now, but there could 
> be some other scenarios which could trigger the same issue on stable 
> kernels.  Hence it needs to be back ported to stable kernels.
> 

So ... I'm 99% sure we need to do this for all titan IP blocks, not just 
for IFE but BPS, IPE...

I guess I'll spin a series for this.

---
bod

Re: [PATCH V2] clk: qcom: camcc-sc7180: Add parent dependency to all camera GDSCs

[Bjorn Andersson]

On Mon, 1 May 2023 19:59:32 +0530, Taniya Das wrote:
> Camera titan top GDSC is a parent supply to all other camera GDSCs. Titan
> top GDSC is required to be enabled before enabling any other camera GDSCs
> and it should be disabled only after all other camera GDSCs are disabled.
> Ensure this behavior by marking titan top GDSC as parent of all other
> camera GDSCs.
> 
> 
> [...]

Applied, thanks!

[1/1] clk: qcom: camcc-sc7180: Add parent dependency to all camera GDSCs
      commit: 3e4d179532423f299554cd0dedabdd9d2fdd238d

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>