Skip to contentSkip to conversation

clk: qcom: ipq8074: fix NSS core PLL-s

PATCH Superseded Robert Marko 3 participants 10 messages Apr 25, 2022 – May 6, 2022

Subsystems (4)

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/7 clk: qcom: ipq8074: fix NSS core PLL-s

Like in IPQ6018 the NSS related Alpha PLL-s require initial configuration to work.

So, obtain the regmap that is required for the Alpha PLL configuration and thus utilize the qcom_cc_really_probe() as we already have the regmap. Then utilize the Alpha PLL configs from the downstream QCA 5.4 based kernel to configure them.

This fixes the UBI32 and NSS crypto PLL-s failing to get enabled by the kernel.

Fixes b8e7e519625f ("clk: qcom: ipq8074: add remaining PLL’s")
---
 drivers/clk/qcom/gcc-ipq8074.c | 39 +++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)
drivers/clk/qcom/gcc-ipq8074.c +38 -1
index e79c3329febd..2ebd1462db78 100644
@@ -4371,6 +4371,33 @@ static struct clk_branch gcc_pcie0_axi_s_bridge_clk = {
 	},
 };
 
+static const struct alpha_pll_config ubi32_pll_config = {
+	.l = 0x4e,
+	.config_ctl_val = 0x200d4aa8,
+	.config_ctl_hi_val = 0x3c2,
+	.main_output_mask = BIT(0),
+	.aux_output_mask = BIT(1),
+	.pre_div_val = 0x0,
+	.pre_div_mask = BIT(12),
+	.post_div_val = 0x0,
+	.post_div_mask = GENMASK(9, 8),
+};
+
+static const struct alpha_pll_config nss_crypto_pll_config = {
+	.l = 0x3e,
+	.alpha = 0x0,
+	.alpha_hi = 0x80,
+	.config_ctl_val = 0x4001055b,
+	.main_output_mask = BIT(0),
+	.pre_div_val = 0x0,
+	.pre_div_mask = GENMASK(14, 12),
+	.post_div_val = 0x1 << 8,
+	.post_div_mask = GENMASK(11, 8),
+	.vco_mask = GENMASK(21, 20),
+	.vco_val = 0x0,
+	.alpha_en_mask = BIT(24),
+};
+
 static struct clk_hw *gcc_ipq8074_hws[] = {
 	&gpll0_out_main_div2.hw,
 	&gpll6_out_main_div2.hw,
@@ -4773,7 +4800,17 @@ static const struct qcom_cc_desc gcc_ipq8074_desc = {
 
 static int gcc_ipq8074_probe(struct platform_device *pdev)
 {
-	return qcom_cc_probe(pdev, &gcc_ipq8074_desc);
+	struct regmap *regmap;
+
+	regmap = qcom_cc_map(pdev, &gcc_ipq8074_desc);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	clk_alpha_pll_configure(&ubi32_pll_main, regmap, &ubi32_pll_config);
+	clk_alpha_pll_configure(&nss_crypto_pll_main, regmap,
+				&nss_crypto_pll_config);
+
+	return qcom_cc_really_probe(pdev, &gcc_ipq8074_desc, regmap);
 }
 
 static struct platform_driver gcc_ipq8074_driver = {
Signature · 1 line
2.35.1
#1 lore raw

PATCH 2/7 clk: qcom: ipq8074: disable USB GDSC-s SW_COLLAPSE

Like in IPQ6018 Qualcomm intentionally disables the SW_COLLAPSE on the USB GDSC-s in the downstream 5.4 kernel.

This could potentially be better handled by utilizing the GDSC driver, but I am not familiar with it nor do I have datasheets.

---
 drivers/clk/qcom/gcc-ipq8074.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/clk/qcom/gcc-ipq8074.c +5 -0
index 2ebd1462db78..65249a03a672 100644
@@ -4806,6 +4806,11 @@ static int gcc_ipq8074_probe(struct platform_device *pdev)
 	if (IS_ERR(regmap))
 		return PTR_ERR(regmap);
 
+	/* Disable SW_COLLAPSE for USB0 GDSCR */
+	regmap_update_bits(regmap, 0x3e078, BIT(0), 0x0);
+	/* Disable SW_COLLAPSE for USB1 GDSCR */
+	regmap_update_bits(regmap, 0x3f078, BIT(0), 0x0);
+
 	clk_alpha_pll_configure(&ubi32_pll_main, regmap, &ubi32_pll_config);
 	clk_alpha_pll_configure(&nss_crypto_pll_main, regmap,
 				&nss_crypto_pll_config);
Signature · 1 line
2.35.1
#2 lore raw

Re: [PATCH 2/7] clk: qcom: ipq8074: disable USB GDSC-s SW_COLLAPSE

Quoted 5 lines · Robert Marko

Like in IPQ6018 Qualcomm intentionally disables the SW_COLLAPSE on the USB GDSC-s in the downstream 5.4 kernel.

This could potentially be better handled by utilizing the GDSC driver, but I am not familiar with it nor do I have datasheets.

Could you please give it a try before we pick this up?
Look at e.g. drivers/clk/qcom/gcc-sdm845.c how usb30_prim_gdsc and usb30_sec_gdsc are defined, the offsets in specified in .gdscr should be the same offsets you give below.

Then you specify an array of struct gdsc *, associating the two gdscs you have specified to some identifier (USB30_PRIM_GDSC and USB30_SEC_GDSC is used in sdm845) and reference this list as .gdscs and num_gdscs in the gcc_ipq8074_desc.

The last part is to tie the USB controllers to the two GDSCs, this is done by simply specifying:

	power-domains = <&gcc USB30_PRIM_GDSC>;

and USB30_SEC_GDSC, in the two USB nodes in DeviceTree. SW_COLLAPSE will be toggled by the PM state of the USB driver, like it's done on e.g. sdm845.

Regards,
Bjorn

Quoted 25 lines
---
 drivers/clk/qcom/gcc-ipq8074.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/clk/qcom/gcc-ipq8074.c +5 -0
index 2ebd1462db78..65249a03a672 100644
@@ -4806,6 +4806,11 @@ static int gcc_ipq8074_probe(struct platform_device *pdev)
 	if (IS_ERR(regmap))
 		return PTR_ERR(regmap);
 
+	/* Disable SW_COLLAPSE for USB0 GDSCR */
+	regmap_update_bits(regmap, 0x3e078, BIT(0), 0x0);
+	/* Disable SW_COLLAPSE for USB1 GDSCR */
+	regmap_update_bits(regmap, 0x3f078, BIT(0), 0x0);
+
 	clk_alpha_pll_configure(&ubi32_pll_main, regmap, &ubi32_pll_config);
 	clk_alpha_pll_configure(&nss_crypto_pll_main, regmap,
 				&nss_crypto_pll_config);
Signature · 1 line
2.35.1
#3 lore raw

Re: [PATCH 2/7] clk: qcom: ipq8074: disable USB GDSC-s SW_COLLAPSE

Quoted 19 lines · Bjorn Andersson
Quoted 5 lines · Robert Marko

Like in IPQ6018 Qualcomm intentionally disables the SW_COLLAPSE on the USB GDSC-s in the downstream 5.4 kernel.

This could potentially be better handled by utilizing the GDSC driver, but I am not familiar with it nor do I have datasheets.

Could you please give it a try before we pick this up?
Look at e.g. drivers/clk/qcom/gcc-sdm845.c how usb30_prim_gdsc and usb30_sec_gdsc are defined, the offsets in specified in .gdscr should be the same offsets you give below.

Then you specify an array of struct gdsc *, associating the two gdscs you have specified to some identifier (USB30_PRIM_GDSC and USB30_SEC_GDSC is used in sdm845) and reference this list as .gdscs and num_gdscs in the gcc_ipq8074_desc.

Quoted 8 lines

The last part is to tie the USB controllers to the two GDSCs, this is done by simply specifying:

        power-domains = <&gcc USB30_PRIM_GDSC>;

and USB30_SEC_GDSC, in the two USB nodes in DeviceTree. SW_COLLAPSE will be toggled by the PM state of the USB driver, like it's done on e.g. sdm845.

Hi Bjorn, thanks for the tips, it makes more sense now.
The only thing I am not sure about are the feature flags for these GDSCs, how to figure out which ones are correct as I dont have datasheets and QCA does not use GDSCs in the downstream kernel?
POLL_CFG_GDSCR will cause the GDSC not to get enabled, while VOTABLE seems to work.

Regards,
Robert

Quoted 29 lines

Regards,
Bjorn

Quoted 25 lines
---
 drivers/clk/qcom/gcc-ipq8074.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/clk/qcom/gcc-ipq8074.c +5 -1
index 2ebd1462db78..65249a03a672 100644
@@ -4806,6 +4806,11 @@ static int gcc_ipq8074_probe(struct platform_device *pdev)
      if (IS_ERR(regmap))
              return PTR_ERR(regmap);
 
+     /* Disable SW_COLLAPSE for USB0 GDSCR */
+     regmap_update_bits(regmap, 0x3e078, BIT(0), 0x0);
+     /* Disable SW_COLLAPSE for USB1 GDSCR */
+     regmap_update_bits(regmap, 0x3f078, BIT(0), 0x0);
+
      clk_alpha_pll_configure(&ubi32_pll_main, regmap, &ubi32_pll_config);
      clk_alpha_pll_configure(&nss_crypto_pll_main, regmap,
                              &nss_crypto_pll_config);
--

2.35.1

#4 lore raw

PATCH 3/7 clk: qcom: ipq8074: SW workaround for UBI32 PLL lock

In reply to Robert Marko

UBI32 Huayra PLL fails to lock in 5 us in some SoC silicon and thus it will cause the wait_for_pll() to timeout and thus return the error indicating that the PLL failed to lock.

This is bug in Huayra PLL HW for which SW workaround is to set bit 26 of TEST_CTL register.

This is ported from the QCA 5.4 based downstream kernel.

Fixes b8e7e519625f ("clk: qcom: ipq8074: add remaining PLL’s")
---
 drivers/clk/qcom/gcc-ipq8074.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/clk/qcom/gcc-ipq8074.c +3 -0
index 65249a03a672..969b38d4ba08 100644
@@ -4811,6 +4811,9 @@ static int gcc_ipq8074_probe(struct platform_device *pdev)
 	/* Disable SW_COLLAPSE for USB1 GDSCR */
 	regmap_update_bits(regmap, 0x3f078, BIT(0), 0x0);
 
+	/* SW Workaround for UBI32 Huayra PLL */
+	regmap_update_bits(regmap, 0x2501c, BIT(26), BIT(26));
+
 	clk_alpha_pll_configure(&ubi32_pll_main, regmap, &ubi32_pll_config);
 	clk_alpha_pll_configure(&nss_crypto_pll_main, regmap,
 				&nss_crypto_pll_config);
Signature · 1 line
2.35.1
#5 lore raw

PATCH 4/7 clk: qcom: ipq8074: fix NSS port frequency tables

In reply to Robert Marko

NSS port 5 and 6 frequency tables are currently broken and are causing a wide ranges of issue like 1G not working at all on port 6 or port 5 being clocked with 312 instead of 125 MHz as UNIPHY1 gets selected.

So, update the frequency tables with the ones from the downstream QCA 5.4 based kernel which has already fixed this.

Fixes 7117a51ed303 ("clk: qcom: ipq8074: add NSS ethernet port clocks")
---
 drivers/clk/qcom/gcc-ipq8074.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/clk/qcom/gcc-ipq8074.c +8 -0
index 969b38d4ba08..37af41d8b192 100644
@@ -1788,8 +1788,10 @@ static struct clk_regmap_div nss_port4_tx_div_clk_src = {
 static const struct freq_tbl ftbl_nss_port5_rx_clk_src[] = {
 	F(19200000, P_XO, 1, 0, 0),
 	F(25000000, P_UNIPHY1_RX, 12.5, 0, 0),
+	F(25000000, P_UNIPHY0_RX, 5, 0, 0),
 	F(78125000, P_UNIPHY1_RX, 4, 0, 0),
 	F(125000000, P_UNIPHY1_RX, 2.5, 0, 0),
+	F(125000000, P_UNIPHY0_RX, 1, 0, 0),
 	F(156250000, P_UNIPHY1_RX, 2, 0, 0),
 	F(312500000, P_UNIPHY1_RX, 1, 0, 0),
 	{ }
@@ -1828,8 +1830,10 @@ static struct clk_regmap_div nss_port5_rx_div_clk_src = {
 static const struct freq_tbl ftbl_nss_port5_tx_clk_src[] = {
 	F(19200000, P_XO, 1, 0, 0),
 	F(25000000, P_UNIPHY1_TX, 12.5, 0, 0),
+	F(25000000, P_UNIPHY0_TX, 5, 0, 0),
 	F(78125000, P_UNIPHY1_TX, 4, 0, 0),
 	F(125000000, P_UNIPHY1_TX, 2.5, 0, 0),
+	F(125000000, P_UNIPHY0_TX, 1, 0, 0),
 	F(156250000, P_UNIPHY1_TX, 2, 0, 0),
 	F(312500000, P_UNIPHY1_TX, 1, 0, 0),
 	{ }
@@ -1867,8 +1871,10 @@ static struct clk_regmap_div nss_port5_tx_div_clk_src = {
 
 static const struct freq_tbl ftbl_nss_port6_rx_clk_src[] = {
 	F(19200000, P_XO, 1, 0, 0),
+	F(25000000, P_UNIPHY2_RX, 5, 0, 0),
 	F(25000000, P_UNIPHY2_RX, 12.5, 0, 0),
 	F(78125000, P_UNIPHY2_RX, 4, 0, 0),
+	F(125000000, P_UNIPHY2_RX, 1, 0, 0),
 	F(125000000, P_UNIPHY2_RX, 2.5, 0, 0),
 	F(156250000, P_UNIPHY2_RX, 2, 0, 0),
 	F(312500000, P_UNIPHY2_RX, 1, 0, 0),
@@ -1907,8 +1913,10 @@ static struct clk_regmap_div nss_port6_rx_div_clk_src = {
 
 static const struct freq_tbl ftbl_nss_port6_tx_clk_src[] = {
 	F(19200000, P_XO, 1, 0, 0),
+	F(25000000, P_UNIPHY2_TX, 5, 0, 0),
 	F(25000000, P_UNIPHY2_TX, 12.5, 0, 0),
 	F(78125000, P_UNIPHY2_TX, 4, 0, 0),
+	F(125000000, P_UNIPHY2_TX, 1, 0, 0),
 	F(125000000, P_UNIPHY2_TX, 2.5, 0, 0),
 	F(156250000, P_UNIPHY2_TX, 2, 0, 0),
 	F(312500000, P_UNIPHY2_TX, 1, 0, 0),
Signature · 1 line
2.35.1
#6 lore raw

PATCH 5/7 clk: qcom: ipq8074: add PPE crypto clock

In reply to Robert Marko

The built-in PPE engine has a dedicated clock for the EIP-197 crypto engine.

So, since the required clock currently missing add support for it.

---
 drivers/clk/qcom/gcc-ipq8074.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
drivers/clk/qcom/gcc-ipq8074.c +19 -0
index 37af41d8b192..e6625b9fab35 100644
@@ -3182,6 +3182,24 @@ static struct clk_branch gcc_nss_ptp_ref_clk = {
 	},
 };
 
+static struct clk_branch gcc_crypto_ppe_clk = {
+	.halt_reg = 0x68310,
+	.halt_bit = 31,
+	.clkr = {
+		.enable_reg = 0x68310,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "gcc_crypto_ppe_clk",
+			.parent_names = (const char *[]){
+				"nss_ppe_clk_src"
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
 static struct clk_branch gcc_nssnoc_ce_apb_clk = {
 	.halt_reg = 0x6830c,
 	.clkr = {
@@ -4644,6 +4662,7 @@ static struct clk_regmap *gcc_ipq8074_clks[] = {
 	[GCC_PCIE0_RCHNG_CLK_SRC] = &pcie0_rchng_clk_src.clkr,
 	[GCC_PCIE0_RCHNG_CLK] = &gcc_pcie0_rchng_clk.clkr,
 	[GCC_PCIE0_AXI_S_BRIDGE_CLK] = &gcc_pcie0_axi_s_bridge_clk.clkr,
+	[GCC_CRYPTO_PPE_CLK] = &gcc_crypto_ppe_clk.clkr,
 };
 
 static const struct qcom_reset_map gcc_ipq8074_resets[] = {
Signature · 1 line
2.35.1
#7 lore raw

PATCH 6/7 dt-bindings: clock: qcom: ipq8074: add PPE crypto clock

In reply to Robert Marko

Add binding for the PPE crypto clock in IPQ8074.

---
 include/dt-bindings/clock/qcom,gcc-ipq8074.h | 1 +
 1 file changed, 1 insertion(+)
include/dt-bindings/clock/qcom,gcc-ipq8074.h +1 -0
index 9b1c42bc430c..2197185cb1a2 100644
@@ -233,6 +233,7 @@
 #define GCC_PCIE0_AXI_S_BRIDGE_CLK		224
 #define GCC_PCIE0_RCHNG_CLK_SRC			225
 #define GCC_PCIE0_RCHNG_CLK			226
+#define GCC_CRYPTO_PPE_CLK			227
 
 #define GCC_BLSP1_BCR				0
 #define GCC_BLSP1_QUP1_BCR			1
Signature · 1 line
2.35.1
#9 lore raw

PATCH 7/7 clk: qcom: ipq8074: set BRANCH_HALT_DELAY flag for UBI clocks

In reply to Robert Marko

Currently, attempting to enable the UBI clocks will cause the stuck at off warning to be printed and clk_enable will fail.

[   14.936694] gcc_ubi1_ahb_clk status stuck at 'off'

Downstream 5.4 QCA kernel has fixed this by seting the BRANCH_HALT_DELAY flag on UBI clocks, so lets do the same.

Fixes 5736294aef83 ("clk: qcom: ipq8074: add NSS clocks")
---
 drivers/clk/qcom/gcc-ipq8074.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/clk/qcom/gcc-ipq8074.c +10 -0
index e6625b9fab35..54f292ab1f0d 100644
@@ -3372,6 +3372,7 @@ static struct clk_branch gcc_nssnoc_ubi1_ahb_clk = {
 
 static struct clk_branch gcc_ubi0_ahb_clk = {
 	.halt_reg = 0x6820c,
+	.halt_check = BRANCH_HALT_DELAY,
 	.clkr = {
 		.enable_reg = 0x6820c,
 		.enable_mask = BIT(0),
@@ -3389,6 +3390,7 @@ static struct clk_branch gcc_ubi0_ahb_clk = {
 
 static struct clk_branch gcc_ubi0_axi_clk = {
 	.halt_reg = 0x68200,
+	.halt_check = BRANCH_HALT_DELAY,
 	.clkr = {
 		.enable_reg = 0x68200,
 		.enable_mask = BIT(0),
@@ -3406,6 +3408,7 @@ static struct clk_branch gcc_ubi0_axi_clk = {
 
 static struct clk_branch gcc_ubi0_nc_axi_clk = {
 	.halt_reg = 0x68204,
+	.halt_check = BRANCH_HALT_DELAY,
 	.clkr = {
 		.enable_reg = 0x68204,
 		.enable_mask = BIT(0),
@@ -3423,6 +3426,7 @@ static struct clk_branch gcc_ubi0_nc_axi_clk = {
 
 static struct clk_branch gcc_ubi0_core_clk = {
 	.halt_reg = 0x68210,
+	.halt_check = BRANCH_HALT_DELAY,
 	.clkr = {
 		.enable_reg = 0x68210,
 		.enable_mask = BIT(0),
@@ -3440,6 +3444,7 @@ static struct clk_branch gcc_ubi0_core_clk = {
 
 static struct clk_branch gcc_ubi0_mpt_clk = {
 	.halt_reg = 0x68208,
+	.halt_check = BRANCH_HALT_DELAY,
 	.clkr = {
 		.enable_reg = 0x68208,
 		.enable_mask = BIT(0),
@@ -3457,6 +3462,7 @@ static struct clk_branch gcc_ubi0_mpt_clk = {
 
 static struct clk_branch gcc_ubi1_ahb_clk = {
 	.halt_reg = 0x6822c,
+	.halt_check = BRANCH_HALT_DELAY,
 	.clkr = {
 		.enable_reg = 0x6822c,
 		.enable_mask = BIT(0),
@@ -3474,6 +3480,7 @@ static struct clk_branch gcc_ubi1_ahb_clk = {
 
 static struct clk_branch gcc_ubi1_axi_clk = {
 	.halt_reg = 0x68220,
+	.halt_check = BRANCH_HALT_DELAY,
 	.clkr = {
 		.enable_reg = 0x68220,
 		.enable_mask = BIT(0),
@@ -3491,6 +3498,7 @@ static struct clk_branch gcc_ubi1_axi_clk = {
 
 static struct clk_branch gcc_ubi1_nc_axi_clk = {
 	.halt_reg = 0x68224,
+	.halt_check = BRANCH_HALT_DELAY,
 	.clkr = {
 		.enable_reg = 0x68224,
 		.enable_mask = BIT(0),
@@ -3508,6 +3516,7 @@ static struct clk_branch gcc_ubi1_nc_axi_clk = {
 
 static struct clk_branch gcc_ubi1_core_clk = {
 	.halt_reg = 0x68230,
+	.halt_check = BRANCH_HALT_DELAY,
 	.clkr = {
 		.enable_reg = 0x68230,
 		.enable_mask = BIT(0),
@@ -3525,6 +3534,7 @@ static struct clk_branch gcc_ubi1_core_clk = {
 
 static struct clk_branch gcc_ubi1_mpt_clk = {
 	.halt_reg = 0x68228,
+	.halt_check = BRANCH_HALT_DELAY,
 	.clkr = {
 		.enable_reg = 0x68228,
 		.enable_mask = BIT(0),
Signature · 1 line
2.35.1