| Message ID | 20211230023101.1122588-1-konrad.dybcio@somainline.org (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | clk: qcom: gcc-msm8994: Remove NoC clocks | expand |
On Thu, 30 Dec 2021 at 05:31, Konrad Dybcio <konrad.dybcio@somainline.org> wrote: > > Just like in commit 05cf3ec00d460b50088d421fb878a0f83f57e262 > ("clk: qcom: gcc-msm8996: Drop (again) gcc_aggre1_pnoc_ahb_clk") > adding NoC clocks turned out to be a huge mistake, as they cause a lot of > issues at little benefit (basically only letting Linux know about their > children's frequencies), especially when mishandled or misconfigured. I'm not against this patch, but it manifests another question to me: should the NoC driver set these frequencies (as demanded), or are they set by the hardware/RPM/etc and so are read-only to us? > > Adding these ones broke SDCC approx 99 out of 100 times, but that somehow > went unnoticed. To prevent further issues like this one, remove them. > > This commit is effectively a revert of 74a33fac3aab77558ca0d80c9935 > ("clk: qcom: gcc-msm8994: Add missing NoC clocks") with ABI preservation. > > Fixes: 74a33fac3aab ("clk: qcom: gcc-msm8994: Add missing NoC clocks") > Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org> > --- > drivers/clk/qcom/gcc-msm8994.c | 100 +-------------------------------- > 1 file changed, 3 insertions(+), 97 deletions(-) > > diff --git a/drivers/clk/qcom/gcc-msm8994.c b/drivers/clk/qcom/gcc-msm8994.c > index 702a9bdc0559..18044e8dc732 100644 > --- a/drivers/clk/qcom/gcc-msm8994.c > +++ b/drivers/clk/qcom/gcc-msm8994.c > @@ -107,42 +107,6 @@ static const struct clk_parent_data gcc_xo_gpll0_gpll4[] = { > { .hw = &gpll4.clkr.hw }, > }; > > -static struct clk_rcg2 system_noc_clk_src = { > - .cmd_rcgr = 0x0120, > - .hid_width = 5, > - .parent_map = gcc_xo_gpll0_map, > - .clkr.hw.init = &(struct clk_init_data){ > - .name = "system_noc_clk_src", > - .parent_data = gcc_xo_gpll0, > - .num_parents = ARRAY_SIZE(gcc_xo_gpll0), > - .ops = &clk_rcg2_ops, > - }, > -}; > - > -static struct clk_rcg2 config_noc_clk_src = { > - .cmd_rcgr = 0x0150, > - .hid_width = 5, > - .parent_map = gcc_xo_gpll0_map, > - .clkr.hw.init = &(struct clk_init_data){ > - .name = "config_noc_clk_src", > - .parent_data = gcc_xo_gpll0, > - .num_parents = ARRAY_SIZE(gcc_xo_gpll0), > - .ops = &clk_rcg2_ops, > - }, > -}; > - > -static struct clk_rcg2 periph_noc_clk_src = { > - .cmd_rcgr = 0x0190, > - .hid_width = 5, > - .parent_map = gcc_xo_gpll0_map, > - .clkr.hw.init = &(struct clk_init_data){ > - .name = "periph_noc_clk_src", > - .parent_data = gcc_xo_gpll0, > - .num_parents = ARRAY_SIZE(gcc_xo_gpll0), > - .ops = &clk_rcg2_ops, > - }, > -}; > - > static struct freq_tbl ftbl_ufs_axi_clk_src[] = { > F(50000000, P_GPLL0, 12, 0, 0), > F(100000000, P_GPLL0, 6, 0, 0), > @@ -1149,8 +1113,6 @@ static struct clk_branch gcc_blsp1_ahb_clk = { > .enable_mask = BIT(17), > .hw.init = &(struct clk_init_data){ > .name = "gcc_blsp1_ahb_clk", > - .parent_hws = (const struct clk_hw *[]){ &periph_noc_clk_src.clkr.hw }, > - .num_parents = 1, > .ops = &clk_branch2_ops, > }, > }, > @@ -1434,8 +1396,6 @@ static struct clk_branch gcc_blsp2_ahb_clk = { > .enable_mask = BIT(15), > .hw.init = &(struct clk_init_data){ > .name = "gcc_blsp2_ahb_clk", > - .parent_hws = (const struct clk_hw *[]){ &periph_noc_clk_src.clkr.hw }, > - .num_parents = 1, > .ops = &clk_branch2_ops, > }, > }, > @@ -1763,8 +1723,6 @@ static struct clk_branch gcc_lpass_q6_axi_clk = { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_lpass_q6_axi_clk", > - .parent_hws = (const struct clk_hw *[]){ &system_noc_clk_src.clkr.hw }, > - .num_parents = 1, > .ops = &clk_branch2_ops, > }, > }, > @@ -1777,8 +1735,6 @@ static struct clk_branch gcc_mss_q6_bimc_axi_clk = { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_mss_q6_bimc_axi_clk", > - .parent_hws = (const struct clk_hw *[]){ &system_noc_clk_src.clkr.hw }, > - .num_parents = 1, > .ops = &clk_branch2_ops, > }, > }, > @@ -1806,9 +1762,6 @@ static struct clk_branch gcc_pcie_0_cfg_ahb_clk = { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_pcie_0_cfg_ahb_clk", > - .parent_hws = (const struct clk_hw *[]){ &config_noc_clk_src.clkr.hw }, > - .num_parents = 1, > - .flags = CLK_SET_RATE_PARENT, > .ops = &clk_branch2_ops, > }, > }, > @@ -1821,9 +1774,6 @@ static struct clk_branch gcc_pcie_0_mstr_axi_clk = { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_pcie_0_mstr_axi_clk", > - .parent_hws = (const struct clk_hw *[]){ &system_noc_clk_src.clkr.hw }, > - .num_parents = 1, > - .flags = CLK_SET_RATE_PARENT, > .ops = &clk_branch2_ops, > }, > }, > @@ -1853,9 +1803,6 @@ static struct clk_branch gcc_pcie_0_slv_axi_clk = { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_pcie_0_slv_axi_clk", > - .parent_hws = (const struct clk_hw *[]){ &system_noc_clk_src.clkr.hw }, > - .num_parents = 1, > - .flags = CLK_SET_RATE_PARENT, > .ops = &clk_branch2_ops, > }, > }, > @@ -1883,9 +1830,6 @@ static struct clk_branch gcc_pcie_1_cfg_ahb_clk = { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_pcie_1_cfg_ahb_clk", > - .parent_hws = (const struct clk_hw *[]){ &config_noc_clk_src.clkr.hw }, > - .num_parents = 1, > - .flags = CLK_SET_RATE_PARENT, > .ops = &clk_branch2_ops, > }, > }, > @@ -1898,9 +1842,6 @@ static struct clk_branch gcc_pcie_1_mstr_axi_clk = { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_pcie_1_mstr_axi_clk", > - .parent_hws = (const struct clk_hw *[]){ &system_noc_clk_src.clkr.hw }, > - .num_parents = 1, > - .flags = CLK_SET_RATE_PARENT, > .ops = &clk_branch2_ops, > }, > }, > @@ -1929,9 +1870,6 @@ static struct clk_branch gcc_pcie_1_slv_axi_clk = { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_pcie_1_slv_axi_clk", > - .parent_hws = (const struct clk_hw *[]){ &system_noc_clk_src.clkr.hw }, > - .num_parents = 1, > - .flags = CLK_SET_RATE_PARENT, > .ops = &clk_branch2_ops, > }, > }, > @@ -1959,8 +1897,6 @@ static struct clk_branch gcc_pdm_ahb_clk = { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_pdm_ahb_clk", > - .parent_hws = (const struct clk_hw *[]){ &periph_noc_clk_src.clkr.hw }, > - .num_parents = 1, > .ops = &clk_branch2_ops, > }, > }, > @@ -1988,9 +1924,6 @@ static struct clk_branch gcc_sdcc1_ahb_clk = { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_sdcc1_ahb_clk", > - .parent_hws = (const struct clk_hw *[]){ &periph_noc_clk_src.clkr.hw }, > - .num_parents = 1, > - .flags = CLK_SET_RATE_PARENT, > .ops = &clk_branch2_ops, > }, > }, > @@ -2003,9 +1936,6 @@ static struct clk_branch gcc_sdcc2_ahb_clk = { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_sdcc2_ahb_clk", > - .parent_hws = (const struct clk_hw *[]){ &periph_noc_clk_src.clkr.hw }, > - .num_parents = 1, > - .flags = CLK_SET_RATE_PARENT, > .ops = &clk_branch2_ops, > }, > }, > @@ -2033,9 +1963,6 @@ static struct clk_branch gcc_sdcc3_ahb_clk = { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_sdcc3_ahb_clk", > - .parent_hws = (const struct clk_hw *[]){ &periph_noc_clk_src.clkr.hw }, > - .num_parents = 1, > - .flags = CLK_SET_RATE_PARENT, > .ops = &clk_branch2_ops, > }, > }, > @@ -2063,9 +1990,6 @@ static struct clk_branch gcc_sdcc4_ahb_clk = { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_sdcc4_ahb_clk", > - .parent_hws = (const struct clk_hw *[]){ &periph_noc_clk_src.clkr.hw }, > - .num_parents = 1, > - .flags = CLK_SET_RATE_PARENT, > .ops = &clk_branch2_ops, > }, > }, > @@ -2123,8 +2047,6 @@ static struct clk_branch gcc_tsif_ahb_clk = { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_tsif_ahb_clk", > - .parent_hws = (const struct clk_hw *[]){ &periph_noc_clk_src.clkr.hw }, > - .num_parents = 1, > .ops = &clk_branch2_ops, > }, > }, > @@ -2152,8 +2074,6 @@ static struct clk_branch gcc_ufs_ahb_clk = { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_ufs_ahb_clk", > - .parent_hws = (const struct clk_hw *[]){ &config_noc_clk_src.clkr.hw }, > - .num_parents = 1, > .ops = &clk_branch2_ops, > }, > }, > @@ -2197,8 +2117,6 @@ static struct clk_branch gcc_ufs_rx_symbol_0_clk = { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_ufs_rx_symbol_0_clk", > - .parent_hws = (const struct clk_hw *[]){ &system_noc_clk_src.clkr.hw }, > - .num_parents = 1, > .ops = &clk_branch2_ops, > }, > }, > @@ -2212,8 +2130,6 @@ static struct clk_branch gcc_ufs_rx_symbol_1_clk = { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_ufs_rx_symbol_1_clk", > - .parent_hws = (const struct clk_hw *[]){ &system_noc_clk_src.clkr.hw }, > - .num_parents = 1, > .ops = &clk_branch2_ops, > }, > }, > @@ -2242,8 +2158,6 @@ static struct clk_branch gcc_ufs_tx_symbol_0_clk = { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_ufs_tx_symbol_0_clk", > - .parent_hws = (const struct clk_hw *[]){ &system_noc_clk_src.clkr.hw }, > - .num_parents = 1, > .ops = &clk_branch2_ops, > }, > }, > @@ -2257,8 +2171,6 @@ static struct clk_branch gcc_ufs_tx_symbol_1_clk = { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_ufs_tx_symbol_1_clk", > - .parent_hws = (const struct clk_hw *[]){ &system_noc_clk_src.clkr.hw }, > - .num_parents = 1, > .ops = &clk_branch2_ops, > }, > }, > @@ -2363,8 +2275,6 @@ static struct clk_branch gcc_usb_hs_ahb_clk = { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_usb_hs_ahb_clk", > - .parent_hws = (const struct clk_hw *[]){ &periph_noc_clk_src.clkr.hw }, > - .num_parents = 1, > .ops = &clk_branch2_ops, > }, > }, > @@ -2487,8 +2397,6 @@ static struct clk_branch gcc_boot_rom_ahb_clk = { > .enable_mask = BIT(10), > .hw.init = &(struct clk_init_data){ > .name = "gcc_boot_rom_ahb_clk", > - .parent_hws = (const struct clk_hw *[]){ &config_noc_clk_src.clkr.hw }, > - .num_parents = 1, > .ops = &clk_branch2_ops, > }, > }, > @@ -2502,8 +2410,6 @@ static struct clk_branch gcc_prng_ahb_clk = { > .enable_mask = BIT(13), > .hw.init = &(struct clk_init_data){ > .name = "gcc_prng_ahb_clk", > - .parent_hws = (const struct clk_hw *[]){ &periph_noc_clk_src.clkr.hw }, > - .num_parents = 1, > .ops = &clk_branch2_ops, > }, > }, > @@ -2546,9 +2452,6 @@ static struct clk_regmap *gcc_msm8994_clocks[] = { > [GPLL0] = &gpll0.clkr, > [GPLL4_EARLY] = &gpll4_early.clkr, > [GPLL4] = &gpll4.clkr, > - [CONFIG_NOC_CLK_SRC] = &config_noc_clk_src.clkr, > - [PERIPH_NOC_CLK_SRC] = &periph_noc_clk_src.clkr, > - [SYSTEM_NOC_CLK_SRC] = &system_noc_clk_src.clkr, > [UFS_AXI_CLK_SRC] = &ufs_axi_clk_src.clkr, > [USB30_MASTER_CLK_SRC] = &usb30_master_clk_src.clkr, > [BLSP1_QUP1_I2C_APPS_CLK_SRC] = &blsp1_qup1_i2c_apps_clk_src.clkr, > @@ -2695,6 +2598,9 @@ static struct clk_regmap *gcc_msm8994_clocks[] = { > [USB_SS_PHY_LDO] = &usb_ss_phy_ldo.clkr, > [GCC_BOOT_ROM_AHB_CLK] = &gcc_boot_rom_ahb_clk.clkr, > [GCC_PRNG_AHB_CLK] = &gcc_prng_ahb_clk.clkr, > + [CONFIG_NOC_CLK_SRC] = NULL, > + [PERIPH_NOC_CLK_SRC] = NULL, > + [SYSTEM_NOC_CLK_SRC] = NULL, You don't have to init array entries with NULL values (if it's not for the documentation purposes). Uninitialized entries will get NULL value anyway.
On 30.12.2021 15:06, Dmitry Baryshkov wrote: > On Thu, 30 Dec 2021 at 05:31, Konrad Dybcio > <konrad.dybcio@somainline.org> wrote: >> Just like in commit 05cf3ec00d460b50088d421fb878a0f83f57e262 >> ("clk: qcom: gcc-msm8996: Drop (again) gcc_aggre1_pnoc_ahb_clk") >> adding NoC clocks turned out to be a huge mistake, as they cause a lot of >> issues at little benefit (basically only letting Linux know about their >> children's frequencies), especially when mishandled or misconfigured. > I'm not against this patch, but it manifests another question to me: > should the NoC driver set these frequencies (as demanded), or are they > set by the hardware/RPM/etc and so are read-only to us? The downstream driver [1] only seems to vote for 19.2 MHz on p(c)noc_keepalive_a_clk and 40MHz on mmssnoc_ahb_a_clk and not really care much about them otherwise in the (msm_)clk framework. Interestingly, the voting-at-probe also seems to be true for 8916 [2], and even more so for 8974 [3] which votes for CXO too, and I don't think we handle it upstream.. Is it unnecessary, or did things always work by miracle? Should we perhaps set it with assigned-clocks under rpmcc in DT? Otherwise, they seem to be handled by msm_bus's voter clocks, so in our case that'll be interconnect's job. I had an old WIP driver somewhere, but it had issues with some (well, many) paths.. I'll rebase it and try debugging that. Decoding ancient msm-3.10 code is not for the faint of heart, but I don't think 8994 or 8974 (which are similar in many ways) ever got a newer kernel release.. [..] Konrad [1] https://github.com/sonyxperiadev/kernel/blob/aosp/LA.BR.1.3.3_rb2.14/drivers/clk/qcom/clock-rpm-8994.c#L292 [2] https://github.com/sonyxperiadev/kernel/blob/aosp/LA.BR.1.3.3_rb2.14/drivers/clk/qcom/clock-rpm-8916.c#L168 [3] https://github.com/sonyxperiadev/kernel/blob/aosp/LA.BR.1.3.3_rb2.14/arch/arm/mach-msm/clock-rpm-8974.c
> You don't have to init array entries with NULL values (if it's not for > the documentation purposes). Uninitialized entries will get NULL value > anyway. > Forgot to address this in my previous message, but I think it may be a good indicator for the next person that decides this would be a good idea that they should probably stay away from it.. If it's really bad, then sure, I can remove it.. Konrad
On Thu, 30 Dec 2021 at 17:29, Konrad Dybcio <konrad.dybcio@somainline.org> wrote: > > > > You don't have to init array entries with NULL values (if it's not for > > the documentation purposes). Uninitialized entries will get NULL value > > anyway. > > > Forgot to address this in my previous message, but I think it may be a > > good indicator for the next person that decides this would be a good idea > > that they should probably stay away from it.. If it's really bad, then sure, I > > can remove it.. You can add a comment on top of those assignments telling this, so that the next person doing the semi-auto cleanup wouldn't delete the assignments.
diff --git a/drivers/clk/qcom/gcc-msm8994.c b/drivers/clk/qcom/gcc-msm8994.c index 702a9bdc0559..18044e8dc732 100644 --- a/drivers/clk/qcom/gcc-msm8994.c +++ b/drivers/clk/qcom/gcc-msm8994.c @@ -107,42 +107,6 @@ static const struct clk_parent_data gcc_xo_gpll0_gpll4[] = { { .hw = &gpll4.clkr.hw }, }; -static struct clk_rcg2 system_noc_clk_src = { - .cmd_rcgr = 0x0120, - .hid_width = 5, - .parent_map = gcc_xo_gpll0_map, - .clkr.hw.init = &(struct clk_init_data){ - .name = "system_noc_clk_src", - .parent_data = gcc_xo_gpll0, - .num_parents = ARRAY_SIZE(gcc_xo_gpll0), - .ops = &clk_rcg2_ops, - }, -}; - -static struct clk_rcg2 config_noc_clk_src = { - .cmd_rcgr = 0x0150, - .hid_width = 5, - .parent_map = gcc_xo_gpll0_map, - .clkr.hw.init = &(struct clk_init_data){ - .name = "config_noc_clk_src", - .parent_data = gcc_xo_gpll0, - .num_parents = ARRAY_SIZE(gcc_xo_gpll0), - .ops = &clk_rcg2_ops, - }, -}; - -static struct clk_rcg2 periph_noc_clk_src = { - .cmd_rcgr = 0x0190, - .hid_width = 5, - .parent_map = gcc_xo_gpll0_map, - .clkr.hw.init = &(struct clk_init_data){ - .name = "periph_noc_clk_src", - .parent_data = gcc_xo_gpll0, - .num_parents = ARRAY_SIZE(gcc_xo_gpll0), - .ops = &clk_rcg2_ops, - }, -}; - static struct freq_tbl ftbl_ufs_axi_clk_src[] = { F(50000000, P_GPLL0, 12, 0, 0), F(100000000, P_GPLL0, 6, 0, 0), @@ -1149,8 +1113,6 @@ static struct clk_branch gcc_blsp1_ahb_clk = { .enable_mask = BIT(17), .hw.init = &(struct clk_init_data){ .name = "gcc_blsp1_ahb_clk", - .parent_hws = (const struct clk_hw *[]){ &periph_noc_clk_src.clkr.hw }, - .num_parents = 1, .ops = &clk_branch2_ops, }, }, @@ -1434,8 +1396,6 @@ static struct clk_branch gcc_blsp2_ahb_clk = { .enable_mask = BIT(15), .hw.init = &(struct clk_init_data){ .name = "gcc_blsp2_ahb_clk", - .parent_hws = (const struct clk_hw *[]){ &periph_noc_clk_src.clkr.hw }, - .num_parents = 1, .ops = &clk_branch2_ops, }, }, @@ -1763,8 +1723,6 @@ static struct clk_branch gcc_lpass_q6_axi_clk = { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_lpass_q6_axi_clk", - .parent_hws = (const struct clk_hw *[]){ &system_noc_clk_src.clkr.hw }, - .num_parents = 1, .ops = &clk_branch2_ops, }, }, @@ -1777,8 +1735,6 @@ static struct clk_branch gcc_mss_q6_bimc_axi_clk = { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_mss_q6_bimc_axi_clk", - .parent_hws = (const struct clk_hw *[]){ &system_noc_clk_src.clkr.hw }, - .num_parents = 1, .ops = &clk_branch2_ops, }, }, @@ -1806,9 +1762,6 @@ static struct clk_branch gcc_pcie_0_cfg_ahb_clk = { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_pcie_0_cfg_ahb_clk", - .parent_hws = (const struct clk_hw *[]){ &config_noc_clk_src.clkr.hw }, - .num_parents = 1, - .flags = CLK_SET_RATE_PARENT, .ops = &clk_branch2_ops, }, }, @@ -1821,9 +1774,6 @@ static struct clk_branch gcc_pcie_0_mstr_axi_clk = { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_pcie_0_mstr_axi_clk", - .parent_hws = (const struct clk_hw *[]){ &system_noc_clk_src.clkr.hw }, - .num_parents = 1, - .flags = CLK_SET_RATE_PARENT, .ops = &clk_branch2_ops, }, }, @@ -1853,9 +1803,6 @@ static struct clk_branch gcc_pcie_0_slv_axi_clk = { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_pcie_0_slv_axi_clk", - .parent_hws = (const struct clk_hw *[]){ &system_noc_clk_src.clkr.hw }, - .num_parents = 1, - .flags = CLK_SET_RATE_PARENT, .ops = &clk_branch2_ops, }, }, @@ -1883,9 +1830,6 @@ static struct clk_branch gcc_pcie_1_cfg_ahb_clk = { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_pcie_1_cfg_ahb_clk", - .parent_hws = (const struct clk_hw *[]){ &config_noc_clk_src.clkr.hw }, - .num_parents = 1, - .flags = CLK_SET_RATE_PARENT, .ops = &clk_branch2_ops, }, }, @@ -1898,9 +1842,6 @@ static struct clk_branch gcc_pcie_1_mstr_axi_clk = { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_pcie_1_mstr_axi_clk", - .parent_hws = (const struct clk_hw *[]){ &system_noc_clk_src.clkr.hw }, - .num_parents = 1, - .flags = CLK_SET_RATE_PARENT, .ops = &clk_branch2_ops, }, }, @@ -1929,9 +1870,6 @@ static struct clk_branch gcc_pcie_1_slv_axi_clk = { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_pcie_1_slv_axi_clk", - .parent_hws = (const struct clk_hw *[]){ &system_noc_clk_src.clkr.hw }, - .num_parents = 1, - .flags = CLK_SET_RATE_PARENT, .ops = &clk_branch2_ops, }, }, @@ -1959,8 +1897,6 @@ static struct clk_branch gcc_pdm_ahb_clk = { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_pdm_ahb_clk", - .parent_hws = (const struct clk_hw *[]){ &periph_noc_clk_src.clkr.hw }, - .num_parents = 1, .ops = &clk_branch2_ops, }, }, @@ -1988,9 +1924,6 @@ static struct clk_branch gcc_sdcc1_ahb_clk = { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_sdcc1_ahb_clk", - .parent_hws = (const struct clk_hw *[]){ &periph_noc_clk_src.clkr.hw }, - .num_parents = 1, - .flags = CLK_SET_RATE_PARENT, .ops = &clk_branch2_ops, }, }, @@ -2003,9 +1936,6 @@ static struct clk_branch gcc_sdcc2_ahb_clk = { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_sdcc2_ahb_clk", - .parent_hws = (const struct clk_hw *[]){ &periph_noc_clk_src.clkr.hw }, - .num_parents = 1, - .flags = CLK_SET_RATE_PARENT, .ops = &clk_branch2_ops, }, }, @@ -2033,9 +1963,6 @@ static struct clk_branch gcc_sdcc3_ahb_clk = { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_sdcc3_ahb_clk", - .parent_hws = (const struct clk_hw *[]){ &periph_noc_clk_src.clkr.hw }, - .num_parents = 1, - .flags = CLK_SET_RATE_PARENT, .ops = &clk_branch2_ops, }, }, @@ -2063,9 +1990,6 @@ static struct clk_branch gcc_sdcc4_ahb_clk = { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_sdcc4_ahb_clk", - .parent_hws = (const struct clk_hw *[]){ &periph_noc_clk_src.clkr.hw }, - .num_parents = 1, - .flags = CLK_SET_RATE_PARENT, .ops = &clk_branch2_ops, }, }, @@ -2123,8 +2047,6 @@ static struct clk_branch gcc_tsif_ahb_clk = { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_tsif_ahb_clk", - .parent_hws = (const struct clk_hw *[]){ &periph_noc_clk_src.clkr.hw }, - .num_parents = 1, .ops = &clk_branch2_ops, }, }, @@ -2152,8 +2074,6 @@ static struct clk_branch gcc_ufs_ahb_clk = { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_ufs_ahb_clk", - .parent_hws = (const struct clk_hw *[]){ &config_noc_clk_src.clkr.hw }, - .num_parents = 1, .ops = &clk_branch2_ops, }, }, @@ -2197,8 +2117,6 @@ static struct clk_branch gcc_ufs_rx_symbol_0_clk = { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_ufs_rx_symbol_0_clk", - .parent_hws = (const struct clk_hw *[]){ &system_noc_clk_src.clkr.hw }, - .num_parents = 1, .ops = &clk_branch2_ops, }, }, @@ -2212,8 +2130,6 @@ static struct clk_branch gcc_ufs_rx_symbol_1_clk = { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_ufs_rx_symbol_1_clk", - .parent_hws = (const struct clk_hw *[]){ &system_noc_clk_src.clkr.hw }, - .num_parents = 1, .ops = &clk_branch2_ops, }, }, @@ -2242,8 +2158,6 @@ static struct clk_branch gcc_ufs_tx_symbol_0_clk = { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_ufs_tx_symbol_0_clk", - .parent_hws = (const struct clk_hw *[]){ &system_noc_clk_src.clkr.hw }, - .num_parents = 1, .ops = &clk_branch2_ops, }, }, @@ -2257,8 +2171,6 @@ static struct clk_branch gcc_ufs_tx_symbol_1_clk = { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_ufs_tx_symbol_1_clk", - .parent_hws = (const struct clk_hw *[]){ &system_noc_clk_src.clkr.hw }, - .num_parents = 1, .ops = &clk_branch2_ops, }, }, @@ -2363,8 +2275,6 @@ static struct clk_branch gcc_usb_hs_ahb_clk = { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_usb_hs_ahb_clk", - .parent_hws = (const struct clk_hw *[]){ &periph_noc_clk_src.clkr.hw }, - .num_parents = 1, .ops = &clk_branch2_ops, }, }, @@ -2487,8 +2397,6 @@ static struct clk_branch gcc_boot_rom_ahb_clk = { .enable_mask = BIT(10), .hw.init = &(struct clk_init_data){ .name = "gcc_boot_rom_ahb_clk", - .parent_hws = (const struct clk_hw *[]){ &config_noc_clk_src.clkr.hw }, - .num_parents = 1, .ops = &clk_branch2_ops, }, }, @@ -2502,8 +2410,6 @@ static struct clk_branch gcc_prng_ahb_clk = { .enable_mask = BIT(13), .hw.init = &(struct clk_init_data){ .name = "gcc_prng_ahb_clk", - .parent_hws = (const struct clk_hw *[]){ &periph_noc_clk_src.clkr.hw }, - .num_parents = 1, .ops = &clk_branch2_ops, }, }, @@ -2546,9 +2452,6 @@ static struct clk_regmap *gcc_msm8994_clocks[] = { [GPLL0] = &gpll0.clkr, [GPLL4_EARLY] = &gpll4_early.clkr, [GPLL4] = &gpll4.clkr, - [CONFIG_NOC_CLK_SRC] = &config_noc_clk_src.clkr, - [PERIPH_NOC_CLK_SRC] = &periph_noc_clk_src.clkr, - [SYSTEM_NOC_CLK_SRC] = &system_noc_clk_src.clkr, [UFS_AXI_CLK_SRC] = &ufs_axi_clk_src.clkr, [USB30_MASTER_CLK_SRC] = &usb30_master_clk_src.clkr, [BLSP1_QUP1_I2C_APPS_CLK_SRC] = &blsp1_qup1_i2c_apps_clk_src.clkr, @@ -2695,6 +2598,9 @@ static struct clk_regmap *gcc_msm8994_clocks[] = { [USB_SS_PHY_LDO] = &usb_ss_phy_ldo.clkr, [GCC_BOOT_ROM_AHB_CLK] = &gcc_boot_rom_ahb_clk.clkr, [GCC_PRNG_AHB_CLK] = &gcc_prng_ahb_clk.clkr, + [CONFIG_NOC_CLK_SRC] = NULL, + [PERIPH_NOC_CLK_SRC] = NULL, + [SYSTEM_NOC_CLK_SRC] = NULL, }; static struct gdsc *gcc_msm8994_gdscs[] = {
Just like in commit 05cf3ec00d460b50088d421fb878a0f83f57e262 ("clk: qcom: gcc-msm8996: Drop (again) gcc_aggre1_pnoc_ahb_clk") adding NoC clocks turned out to be a huge mistake, as they cause a lot of issues at little benefit (basically only letting Linux know about their children's frequencies), especially when mishandled or misconfigured. Adding these ones broke SDCC approx 99 out of 100 times, but that somehow went unnoticed. To prevent further issues like this one, remove them. This commit is effectively a revert of 74a33fac3aab77558ca0d80c9935 ("clk: qcom: gcc-msm8994: Add missing NoC clocks") with ABI preservation. Fixes: 74a33fac3aab ("clk: qcom: gcc-msm8994: Add missing NoC clocks") Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org> --- drivers/clk/qcom/gcc-msm8994.c | 100 +-------------------------------- 1 file changed, 3 insertions(+), 97 deletions(-)