| Message ID | 20240722055539.2594434-1-quic_varada@quicinc.com |
|---|---|
| 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
Enable icc-clk based interconnect driver for IPQ5332. This is similar to IPQ9574's icc-clk based driver. dt_bindings_check and dtbs_check passed. Ensured that icc_sync_state is called and relevant clocks are disabled. v3: Not taking Reviewed-By: Krzysztof, due to minor change in file Add 'clk: qcom: ipq5332: Register gcc_qdss_tsctr_clk_src' to fix gpll4_main's CLK_IGNORE_UNUSED issue. v2: Removed dependency as it is merged dt-bindings update to accommodate USB clock names change Use icc-clk for USB also v1: Dependency: [1] https://lore.kernel.org/linux-arm-msm/20240430064214.2030013-1-quic_varada@quicinc.com/ Varadarajan Narayanan (5): dt-bindings: interconnect: Add Qualcomm IPQ5332 support dt-bindings: usb: qcom,dwc3: Update ipq5332 clock details clk: qcom: ipq5332: Register gcc_qdss_tsctr_clk_src clk: qcom: ipq5332: Use icc-clk for enabling NoC related clocks arm64: dts: qcom: ipq5332: Add icc provider ability to gcc .../bindings/clock/qcom,ipq5332-gcc.yaml | 2 + .../devicetree/bindings/usb/qcom,dwc3.yaml | 17 ++++++- arch/arm64/boot/dts/qcom/ipq5332.dtsi | 7 ++- drivers/clk/qcom/gcc-ipq5332.c | 36 ++++++++++----- .../dt-bindings/interconnect/qcom,ipq5332.h | 46 +++++++++++++++++++ 5 files changed, 94 insertions(+), 14 deletions(-) create mode 100644 include/dt-bindings/interconnect/qcom,ipq5332.h