[PATCH 0/2] Remove apss_dbg clock from IPQ5424 gcc driver

[PATCH 0/2] Remove apss_dbg clock from IPQ5424 gcc driver

[Manikanta Mylavarapu]

Since gcc_apss_dbg_clk has no consumers, the linux kernel will turn it
off. This causes a kernel crash because this clock is access protected
by trust zone. Therefore remove this clock from the gcc driver and dt
bindings.

Manikanta Mylavarapu (2):
  dt-bindings: clock: qcom: gcc-ipq5424: remove apss_dbg clock macro
  clk: qcom: ipq5424: remove apss_dbg clock

 drivers/clk/qcom/gcc-ipq5424.c               | 19 -------------------
 include/dt-bindings/clock/qcom,ipq5424-gcc.h |  1 -
 2 files changed, 20 deletions(-)

-- 
2.34.1

[PATCH 1/2] dt-bindings: clock: qcom: gcc-ipq5424: remove apss_dbg clock macro

[Manikanta Mylavarapu]

Since gcc_apss_dbg_clk has no consumers, the linux kernel will turn it
off. This causes a kernel crash because this clock is access protected
by trust zone. Therefore remove the gcc_apss_dbg_clk macro.

Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@quicinc.com>
---
 include/dt-bindings/clock/qcom,ipq5424-gcc.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/dt-bindings/clock/qcom,ipq5424-gcc.h b/include/dt-bindings/clock/qcom,ipq5424-gcc.h
index 755ce7a71c7c..9f14680052a0 100644
--- a/include/dt-bindings/clock/qcom,ipq5424-gcc.h
+++ b/include/dt-bindings/clock/qcom,ipq5424-gcc.h
@@ -12,7 +12,6 @@
 #define GPLL2					2
 #define GPLL2_OUT_MAIN                          3
 #define GCC_SLEEP_CLK_SRC			4
-#define GCC_APSS_DBG_CLK                        5
 #define GCC_USB0_EUD_AT_CLK			6
 #define GCC_PCIE0_AXI_M_CLK_SRC			7
 #define GCC_PCIE0_AXI_M_CLK			8
-- 
2.34.1

[PATCH 2/2] clk: qcom: ipq5424: remove apss_dbg clock

[Manikanta Mylavarapu]

Since gcc_apss_dbg_clk has no consumers, the linux kernel will turn it
off. This causes a kernel crash because this clock is access protected
by trust zone. Therefore remove gcc_apss_dbg_clk from the gcc driver.

Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@quicinc.com>
---
 drivers/clk/qcom/gcc-ipq5424.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/drivers/clk/qcom/gcc-ipq5424.c b/drivers/clk/qcom/gcc-ipq5424.c
index 88a7d5b2e751..5f42ac6c8cdc 100644
--- a/drivers/clk/qcom/gcc-ipq5424.c
+++ b/drivers/clk/qcom/gcc-ipq5424.c
@@ -1097,24 +1097,6 @@ static struct clk_branch gcc_adss_pwm_clk = {
 	},
 };
 
-static struct clk_branch gcc_apss_dbg_clk = {
-	.halt_reg = 0x2402c,
-	.halt_check = BRANCH_HALT_VOTED,
-	.clkr = {
-		.enable_reg = 0x2402c,
-		.enable_mask = BIT(0),
-		.hw.init = &(const struct clk_init_data) {
-			.name = "gcc_apss_dbg_clk",
-			.parent_hws = (const struct clk_hw*[]) {
-				&gcc_qdss_dap_sync_clk_src.hw
-			},
-			.num_parents = 1,
-			.flags = CLK_SET_RATE_PARENT,
-			.ops = &clk_branch2_ops,
-		},
-	},
-};
-
 static struct clk_branch gcc_cnoc_pcie0_1lane_s_clk = {
 	.halt_reg = 0x31088,
 	.halt_check = BRANCH_HALT,
@@ -2785,7 +2767,6 @@ static struct clk_branch gcc_pcie3_rchng_clk = {
 static struct clk_regmap *gcc_ipq5424_clocks[] = {
 	[GCC_ADSS_PWM_CLK] = &gcc_adss_pwm_clk.clkr,
 	[GCC_ADSS_PWM_CLK_SRC] = &gcc_adss_pwm_clk_src.clkr,
-	[GCC_APSS_DBG_CLK] = &gcc_apss_dbg_clk.clkr,
 	[GCC_CNOC_PCIE0_1LANE_S_CLK] = &gcc_cnoc_pcie0_1lane_s_clk.clkr,
 	[GCC_CNOC_PCIE1_1LANE_S_CLK] = &gcc_cnoc_pcie1_1lane_s_clk.clkr,
 	[GCC_CNOC_PCIE2_2LANE_S_CLK] = &gcc_cnoc_pcie2_2lane_s_clk.clkr,
-- 
2.34.1

Re: [PATCH 1/2] dt-bindings: clock: qcom: gcc-ipq5424: remove apss_dbg clock macro

[Krzysztof Kozlowski]

On Thu, Dec 05, 2024 at 12:10:36PM +0530, Manikanta Mylavarapu wrote:
> Since gcc_apss_dbg_clk has no consumers, the linux kernel will turn it

That's not a reason to drop a binding.

> off. This causes a kernel crash because this clock is access protected

This could be. Please rephrase stating that this clock should not be
exposed to any user because trust zone handles it and any access would
trigger faults.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: clock: qcom: gcc-ipq5424: remove apss_dbg clock macro

[Krzysztof Kozlowski]

On Thu, Dec 05, 2024 at 12:10:36PM +0530, Manikanta Mylavarapu wrote:
> Since gcc_apss_dbg_clk has no consumers, the linux kernel will turn it
> off. This causes a kernel crash because this clock is access protected
> by trust zone. Therefore remove the gcc_apss_dbg_clk macro.
> 
> Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@quicinc.com>
> ---
>  include/dt-bindings/clock/qcom,ipq5424-gcc.h | 1 -
>  1 file changed, 1 deletion(-)
>

You did not even build your patches... This fails to compile.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: clock: qcom: gcc-ipq5424: remove apss_dbg clock macro

[Manikanta Mylavarapu]

On 12/5/2024 3:56 PM, Krzysztof Kozlowski wrote:
> On Thu, Dec 05, 2024 at 12:10:36PM +0530, Manikanta Mylavarapu wrote:
>> Since gcc_apss_dbg_clk has no consumers, the linux kernel will turn it
> 
> That's not a reason to drop a binding.
> 

Thank you for reviewing the patch.
I will drop this statement.

>> off. This causes a kernel crash because this clock is access protected
> 
> This could be. Please rephrase stating that this clock should not be
> exposed to any user because trust zone handles it and any access would
> trigger faults.
> 

I will rephrase it and include the changes in the next version.

Thanks & Regards,
Manikanta.

> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 1/2] dt-bindings: clock: qcom: gcc-ipq5424: remove apss_dbg clock macro

[Manikanta Mylavarapu]

On 12/5/2024 3:57 PM, Krzysztof Kozlowski wrote:
> On Thu, Dec 05, 2024 at 12:10:36PM +0530, Manikanta Mylavarapu wrote:
>> Since gcc_apss_dbg_clk has no consumers, the linux kernel will turn it
>> off. This causes a kernel crash because this clock is access protected
>> by trust zone. Therefore remove the gcc_apss_dbg_clk macro.
>>
>> Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@quicinc.com>
>> ---
>>  include/dt-bindings/clock/qcom,ipq5424-gcc.h | 1 -
>>  1 file changed, 1 deletion(-)
>>
> 
> You did not even build your patches... This fails to compile.
> 

Thank you for reviewing the patch and pointing out the issue.
I apologize for the oversight. I build the entire series, but missed building this patch individually.
Moving forward, I will ensure each patch is built separately to prevent this issue.

Thanks & Regards,
Manikanta.