Re: [PATCH] clk: qcom: gcc-sm8450: set OPS_PARENT_ENABLE on gcc_sdcc2_apps_clk_src

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 4/30/24 23:26, Stephen Boyd wrote:
Quoting Konrad Dybcio (2024-04-30 03:46:52)
On 30.04.2024 2:21 AM, Stephen Boyd wrote:
Quoting Konrad Dybcio (2024-04-27 05:01:07)
Similar to how it works on other SoCs, the top frequency of the SDHCI2
core clock is generated by a separate PLL (peculiar design choice) that
is not guaranteed to be enabled (why does the clock framework not handle
this by default?).

Add the CLK_OPS_PARENT_ENABLE flag to make sure we're not muxing the
RCG input to a dormant source.

The RCG2 hardware hasn't required the parent to be enabled for clk
operations besides for the glitch-free source switch. What scenario is
happening here that's requiring this flag? Is the RCG forcibly enabled
perhaps because the bootloader has left the root enable bit set
(CMD_ROOT_EN)? Or are we changing the parent while the clk framework
thinks the clk is off when it is actually on?

TL;DR: This is papering over a bigger bug.

Definitely.


Take a look at:

static const struct freq_tbl ftbl_gcc_sdcc2_apps_clk_src[] = {
         F(400000, P_BI_TCXO, 12, 1, 4),
         F(25000000, P_GCC_GPLL0_OUT_EVEN, 12, 0, 0),
         F(50000000, P_GCC_GPLL0_OUT_EVEN, 6, 0, 0),
         F(100000000, P_GCC_GPLL0_OUT_EVEN, 3, 0, 0),
         F(202000000, P_GCC_GPLL9_OUT_MAIN, 4, 0, 0),
         { }
};

XO and GPLL0 are more or less always on, but GPLL9 is described to only
be used for this specific clock for this specific frequency (perhaps it
feeds something else on the soc but that's besides the point).

Then, the parent input is changed during set_rate, but GPLL9 seems to
never be enabled:

Is the sdcc2 RCG enabled during the set_rate?

without PARENT_OPS_ENABLE:

[    3.326891] sdhci_msm 8804000.mmc: Got CD GPIO
[    3.336839] scsi host0: ufshcd
[    3.337105] gcc_sdcc2_apps_clk_src is DISABLED @ set_rate
[    3.346339] ------------[ cut here ]------------
[    3.351093] gcc_sdcc2_apps_clk_src: rcg didn't update its configuration.
[    3.351114] WARNING: CPU: 1 PID: 11 at drivers/clk/qcom/clk-rcg2.c:133 update_config+0xc8/0xd8

[...]

[    3.610523] gcc_sdcc2_apps_clk_src is ENABLED @ set_rate


with PARENT_OPS_ENABLE:

[    3.331419] sdhci_msm 8804000.mmc: Got CD GPIO
[    3.336569] gcc_sdcc2_apps_clk_src is DISABLED @ set_rate
[    3.344795] scsi host0: ufshcd
[    3.355122] qcrypto 1dfa000.crypto: Adding to iommu group 5
[    3.363567] remoteproc remoteproc0: 2400000.remoteproc is available
[    3.364729] gcc_sdcc2_apps_clk_src is ENABLED @ set_rate

after testing it both ways, I realized it wasn't supposed to make a
difference in this regard, but I suppose I can paste both results anyway..

Konrad




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux