[PATCH v2] clk: qcom: gcc-ipq9574: Add BRANCH_HALT_VOTED flag

[PATCH v2] clk: qcom: gcc-ipq9574: Add BRANCH_HALT_VOTED flag

[Md Sadre Alam]

Add BRANCH_HALT_VOTED flag to inform clock framework
don't check for CLK_OFF bit.

CRYPTO_AHB_CLK_ENA and CRYPTO_AXI_CLK_ENA enable bit is
present in other VOTE registers also, like TZ.
If anyone else also enabled this clock, even if we turn
off in GCC_APCS_CLOCK_BRANCH_ENA_VOTE | 0x180B004, it won't
turn off.
Also changes the CRYPTO_AHB_CLK_ENA & CRYPTO_AXI_CLK_ENA
offset to 0xb004 from 0x16014.

Cc: stable@vger.kernel.org
Fixes: f6b2bd9cb29a ("clk: qcom: gcc-ipq9574: Enable crypto clocks")
Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---
Change in [v2]

* Added Fixes tag and stable kernel tag

* updated commit message about offset change

 drivers/clk/qcom/gcc-ipq9574.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/qcom/gcc-ipq9574.c b/drivers/clk/qcom/gcc-ipq9574.c
index 0a3f846695b8..f8b9a1e93bef 100644
--- a/drivers/clk/qcom/gcc-ipq9574.c
+++ b/drivers/clk/qcom/gcc-ipq9574.c
@@ -2140,9 +2140,10 @@ static struct clk_rcg2 pcnoc_bfdcd_clk_src = {
 
 static struct clk_branch gcc_crypto_axi_clk = {
 	.halt_reg = 0x16010,
+	.halt_check = BRANCH_HALT_VOTED,
 	.clkr = {
-		.enable_reg = 0x16010,
-		.enable_mask = BIT(0),
+		.enable_reg = 0xb004,
+		.enable_mask = BIT(15),
 		.hw.init = &(const struct clk_init_data) {
 			.name = "gcc_crypto_axi_clk",
 			.parent_hws = (const struct clk_hw *[]) {
@@ -2156,9 +2157,10 @@ static struct clk_branch gcc_crypto_axi_clk = {
 
 static struct clk_branch gcc_crypto_ahb_clk = {
 	.halt_reg = 0x16014,
+	.halt_check = BRANCH_HALT_VOTED,
 	.clkr = {
-		.enable_reg = 0x16014,
-		.enable_mask = BIT(0),
+		.enable_reg = 0xb004,
+		.enable_mask = BIT(16),
 		.hw.init = &(const struct clk_init_data) {
 			.name = "gcc_crypto_ahb_clk",
 			.parent_hws = (const struct clk_hw *[]) {
-- 
2.34.1

Re: [PATCH v2] clk: qcom: gcc-ipq9574: Add BRANCH_HALT_VOTED flag

[Stephen Boyd]

Quoting Md Sadre Alam (2024-05-05 23:37:51)
> Add BRANCH_HALT_VOTED flag to inform clock framework
> don't check for CLK_OFF bit.
> 
> CRYPTO_AHB_CLK_ENA and CRYPTO_AXI_CLK_ENA enable bit is
> present in other VOTE registers also, like TZ.
> If anyone else also enabled this clock, even if we turn
> off in GCC_APCS_CLOCK_BRANCH_ENA_VOTE | 0x180B004, it won't
> turn off.
> Also changes the CRYPTO_AHB_CLK_ENA & CRYPTO_AXI_CLK_ENA
> offset to 0xb004 from 0x16014.

How about this?

 The crypto_ahb and crypto_axi clks are hardware voteable. This means
 that the halt bit isn't reliable because some other voter in the
 system, e.g. TrustZone, could be keeping the clk enabled when the
 kernel turns it off from clk_disable(). Make these clks use voting mode
 by changing the halt check to BRANCH_HALT_VOTED and toggle the voting
 bit in the voting register instead of directly controlling the branch
 by writing to the branch register. This fixes stuck clk warnings seen
 on ipq9574 and saves power by actually turning the clk off.

> 
> Cc: stable@vger.kernel.org
> Fixes: f6b2bd9cb29a ("clk: qcom: gcc-ipq9574: Enable crypto clocks")
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>

Re: [PATCH v2] clk: qcom: gcc-ipq9574: Add BRANCH_HALT_VOTED flag

[Md Sadre Alam]

On 5/7/2024 5:34 AM, Stephen Boyd wrote:
> Quoting Md Sadre Alam (2024-05-05 23:37:51)
>> Add BRANCH_HALT_VOTED flag to inform clock framework
>> don't check for CLK_OFF bit.
>>
>> CRYPTO_AHB_CLK_ENA and CRYPTO_AXI_CLK_ENA enable bit is
>> present in other VOTE registers also, like TZ.
>> If anyone else also enabled this clock, even if we turn
>> off in GCC_APCS_CLOCK_BRANCH_ENA_VOTE | 0x180B004, it won't
>> turn off.
>> Also changes the CRYPTO_AHB_CLK_ENA & CRYPTO_AXI_CLK_ENA
>> offset to 0xb004 from 0x16014.
> 
> How about this?
> 
>   The crypto_ahb and crypto_axi clks are hardware voteable. This means
>   that the halt bit isn't reliable because some other voter in the
>   system, e.g. TrustZone, could be keeping the clk enabled when the
>   kernel turns it off from clk_disable(). Make these clks use voting mode
>   by changing the halt check to BRANCH_HALT_VOTED and toggle the voting
>   bit in the voting register instead of directly controlling the branch
>   by writing to the branch register. This fixes stuck clk warnings seen
>   on ipq9574 and saves power by actually turning the clk off.

  Ok , will update commit message in next patch.
> 
>>
>> Cc: stable@vger.kernel.org
>> Fixes: f6b2bd9cb29a ("clk: qcom: gcc-ipq9574: Enable crypto clocks")
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>