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

Manikanta Mylavarapu posted 2 patches 1 year ago
There is a newer version of this series
[PATCH 1/2] dt-bindings: clock: qcom: gcc-ipq5424: remove apss_dbg clock macro
Posted by Manikanta Mylavarapu 1 year ago
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
Re: [PATCH 1/2] dt-bindings: clock: qcom: gcc-ipq5424: remove apss_dbg clock macro
Posted by Krzysztof Kozlowski 1 year ago
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
Posted by Manikanta Mylavarapu 1 year ago

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
Posted by Krzysztof Kozlowski 1 year ago
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
Posted by Manikanta Mylavarapu 1 year ago

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.