| Message ID | 20240722055539.2594434-4-quic_varada@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add interconnect driver for IPQ5332 SoC | expand |
On Mon, Jul 22, 2024 at 11:25:37AM GMT, Varadarajan Narayanan wrote: > gcc_qdss_tsctr_clk_src (enabled in the boot loaders and dependent > on gpll4_main) was not registered as one of the ipq5332 clocks. > Hence clk_disable_unused() disabled 'gpll4_main' assuming there > were no consumers for 'gpll4_main' resulting in system freeze or > reboots. > > After registering gcc_qdss_tsctr_clk_src, CLK_IGNORE_UNUSED can > be removed from gpll4_main. Commented below. > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> Fixes? > --- > drivers/clk/qcom/gcc-ipq5332.c | 12 +----------- > 1 file changed, 1 insertion(+), 11 deletions(-) > > diff --git a/drivers/clk/qcom/gcc-ipq5332.c b/drivers/clk/qcom/gcc-ipq5332.c > index f98591148a97..237b6a766179 100644 > --- a/drivers/clk/qcom/gcc-ipq5332.c > +++ b/drivers/clk/qcom/gcc-ipq5332.c > @@ -126,17 +126,6 @@ static struct clk_alpha_pll gpll4_main = { > .parent_data = &gcc_parent_data_xo, > .num_parents = 1, > .ops = &clk_alpha_pll_stromer_ops, > - /* > - * There are no consumers for this GPLL in kernel yet, > - * (will be added soon), so the clock framework > - * disables this source. But some of the clocks > - * initialized by boot loaders uses this source. So we > - * need to keep this clock ON. Add the > - * CLK_IGNORE_UNUSED flag so the clock will not be > - * disabled. Once the consumer in kernel is added, we > - * can get rid of this flag. > - */ > - .flags = CLK_IGNORE_UNUSED, You can't drop it in this patch, since GPLL4 still can get disabled if GCC_QDSS_TSCTR_CLK_SRC gets disabled. This chunk should go to the next patch (or you should reorder the patches). > }, > }, > }; > @@ -3388,6 +3377,7 @@ static struct clk_regmap *gcc_ipq5332_clocks[] = { > [GCC_QDSS_DAP_DIV_CLK_SRC] = &gcc_qdss_dap_div_clk_src.clkr, > [GCC_QDSS_ETR_USB_CLK] = &gcc_qdss_etr_usb_clk.clkr, > [GCC_QDSS_EUD_AT_CLK] = &gcc_qdss_eud_at_clk.clkr, > + [GCC_QDSS_TSCTR_CLK_SRC] = &gcc_qdss_tsctr_clk_src.clkr, > [GCC_QPIC_AHB_CLK] = &gcc_qpic_ahb_clk.clkr, > [GCC_QPIC_CLK] = &gcc_qpic_clk.clkr, > [GCC_QPIC_IO_MACRO_CLK] = &gcc_qpic_io_macro_clk.clkr, > -- > 2.34.1 >
On Mon, Jul 22, 2024 at 11:24:33AM +0300, Dmitry Baryshkov wrote: > On Mon, Jul 22, 2024 at 11:25:37AM GMT, Varadarajan Narayanan wrote: > > gcc_qdss_tsctr_clk_src (enabled in the boot loaders and dependent > > on gpll4_main) was not registered as one of the ipq5332 clocks. > > Hence clk_disable_unused() disabled 'gpll4_main' assuming there > > were no consumers for 'gpll4_main' resulting in system freeze or > > reboots. > > > > After registering gcc_qdss_tsctr_clk_src, CLK_IGNORE_UNUSED can > > be removed from gpll4_main. > > Commented below. > > > > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> > > Fixes? Will add that. Was hesitant to add as it would point to the approx 3800 line commit (3d89d52970fd) that introduced the gcc-ipq5332.c file. > > --- > > drivers/clk/qcom/gcc-ipq5332.c | 12 +----------- > > 1 file changed, 1 insertion(+), 11 deletions(-) > > > > diff --git a/drivers/clk/qcom/gcc-ipq5332.c b/drivers/clk/qcom/gcc-ipq5332.c > > index f98591148a97..237b6a766179 100644 > > --- a/drivers/clk/qcom/gcc-ipq5332.c > > +++ b/drivers/clk/qcom/gcc-ipq5332.c > > @@ -126,17 +126,6 @@ static struct clk_alpha_pll gpll4_main = { > > .parent_data = &gcc_parent_data_xo, > > .num_parents = 1, > > .ops = &clk_alpha_pll_stromer_ops, > > - /* > > - * There are no consumers for this GPLL in kernel yet, > > - * (will be added soon), so the clock framework > > - * disables this source. But some of the clocks > > - * initialized by boot loaders uses this source. So we > > - * need to keep this clock ON. Add the > > - * CLK_IGNORE_UNUSED flag so the clock will not be > > - * disabled. Once the consumer in kernel is added, we > > - * can get rid of this flag. > > - */ > > - .flags = CLK_IGNORE_UNUSED, > > You can't drop it in this patch, since GPLL4 still can get disabled if > GCC_QDSS_TSCTR_CLK_SRC gets disabled. This chunk should go to the next > patch (or you should reorder the patches). Ok, will move the above chunk to next patch. Thanks Varada > > }, > > }, > > }; > > @@ -3388,6 +3377,7 @@ static struct clk_regmap *gcc_ipq5332_clocks[] = { > > [GCC_QDSS_DAP_DIV_CLK_SRC] = &gcc_qdss_dap_div_clk_src.clkr, > > [GCC_QDSS_ETR_USB_CLK] = &gcc_qdss_etr_usb_clk.clkr, > > [GCC_QDSS_EUD_AT_CLK] = &gcc_qdss_eud_at_clk.clkr, > > + [GCC_QDSS_TSCTR_CLK_SRC] = &gcc_qdss_tsctr_clk_src.clkr, > > [GCC_QPIC_AHB_CLK] = &gcc_qpic_ahb_clk.clkr, > > [GCC_QPIC_CLK] = &gcc_qpic_clk.clkr, > > [GCC_QPIC_IO_MACRO_CLK] = &gcc_qpic_io_macro_clk.clkr, > > -- > > 2.34.1 > > > > -- > With best wishes > Dmitry
diff --git a/drivers/clk/qcom/gcc-ipq5332.c b/drivers/clk/qcom/gcc-ipq5332.c index f98591148a97..237b6a766179 100644 --- a/drivers/clk/qcom/gcc-ipq5332.c +++ b/drivers/clk/qcom/gcc-ipq5332.c @@ -126,17 +126,6 @@ static struct clk_alpha_pll gpll4_main = { .parent_data = &gcc_parent_data_xo, .num_parents = 1, .ops = &clk_alpha_pll_stromer_ops, - /* - * There are no consumers for this GPLL in kernel yet, - * (will be added soon), so the clock framework - * disables this source. But some of the clocks - * initialized by boot loaders uses this source. So we - * need to keep this clock ON. Add the - * CLK_IGNORE_UNUSED flag so the clock will not be - * disabled. Once the consumer in kernel is added, we - * can get rid of this flag. - */ - .flags = CLK_IGNORE_UNUSED, }, }, }; @@ -3388,6 +3377,7 @@ static struct clk_regmap *gcc_ipq5332_clocks[] = { [GCC_QDSS_DAP_DIV_CLK_SRC] = &gcc_qdss_dap_div_clk_src.clkr, [GCC_QDSS_ETR_USB_CLK] = &gcc_qdss_etr_usb_clk.clkr, [GCC_QDSS_EUD_AT_CLK] = &gcc_qdss_eud_at_clk.clkr, + [GCC_QDSS_TSCTR_CLK_SRC] = &gcc_qdss_tsctr_clk_src.clkr, [GCC_QPIC_AHB_CLK] = &gcc_qpic_ahb_clk.clkr, [GCC_QPIC_CLK] = &gcc_qpic_clk.clkr, [GCC_QPIC_IO_MACRO_CLK] = &gcc_qpic_io_macro_clk.clkr,
gcc_qdss_tsctr_clk_src (enabled in the boot loaders and dependent on gpll4_main) was not registered as one of the ipq5332 clocks. Hence clk_disable_unused() disabled 'gpll4_main' assuming there were no consumers for 'gpll4_main' resulting in system freeze or reboots. After registering gcc_qdss_tsctr_clk_src, CLK_IGNORE_UNUSED can be removed from gpll4_main. Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> --- drivers/clk/qcom/gcc-ipq5332.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-)