From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751899AbcBKVfu (ORCPT ); Thu, 11 Feb 2016 16:35:50 -0500 Received: from mail-pf0-f176.google.com ([209.85.192.176]:34001 "EHLO mail-pf0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751453AbcBKVfr convert rfc822-to-8bit (ORCPT ); Thu, 11 Feb 2016 16:35:47 -0500 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT To: James Liao , "Matthias Brugger" , "Stephen Boyd" , "Rob Herring" From: Michael Turquette In-Reply-To: <1454665050-37776-8-git-send-email-jamesjj.liao@mediatek.com> Cc: "John Crispin" , "Arnd Bergmann" , "Sascha Hauer" , "Daniel Kurtz" , "Philipp Zabel" , srv_heupstream@mediatek.com, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, linux-clk@vger.kernel.org, "James Liao" References: <1454665050-37776-1-git-send-email-jamesjj.liao@mediatek.com> <1454665050-37776-8-git-send-email-jamesjj.liao@mediatek.com> Message-ID: <20160211213542.26445.82983@quark.deferred.io> User-Agent: alot/0.3.6 Subject: Re: [PATCH v6 7/7] clk: mediatek: Enable critical clocks for MT2701 Date: Thu, 11 Feb 2016 13:35:42 -0800 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi James, Quoting James Liao (2016-02-05 01:37:30) > Some system clocks should be turned on by default on MT2701. > This patch enable these clocks when related clocks have > been registered. > > Signed-off-by: James Liao > --- > drivers/clk/mediatek/clk-mt2701.c | 23 +++++++++++++++++++++-- > 1 file changed, 21 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/mediatek/clk-mt2701.c b/drivers/clk/mediatek/clk-mt2701.c > index 01722e0..f7b4d52 100644 > --- a/drivers/clk/mediatek/clk-mt2701.c > +++ b/drivers/clk/mediatek/clk-mt2701.c > @@ -573,6 +573,21 @@ static const struct mtk_gate top_clks[] __initconst = { > GATE_TOP_AUD(CLK_TOP_AUD_I2S6_MCLK, "aud_i2s6_mclk", "aud_k6_src_div", 28), > }; > > +static struct clk_onecell_data *top_clk_data __initdata; > +static struct clk_onecell_data *pll_clk_data __initdata; > + > +static void __init mtk_clk_enable_critical(void) > +{ > + if (!top_clk_data || !pll_clk_data) > + return; > + > + clk_prepare_enable(pll_clk_data->clks[CLK_APMIXED_ARMPLL]); > + clk_prepare_enable(top_clk_data->clks[CLK_TOP_AXI_SEL]); > + clk_prepare_enable(top_clk_data->clks[CLK_TOP_MEM_SEL]); > + clk_prepare_enable(top_clk_data->clks[CLK_TOP_DDRPHYCFG_SEL]); > + clk_prepare_enable(top_clk_data->clks[CLK_TOP_RTC_SEL]); > +} I think we're close to having a better solution. Please see this patch series[0] and let me know if it provides a better way for you to enable these critical clocks instead of the open-coded solution. [0] http://lkml.kernel.org/r/<1455225554-13267-1-git-send-email-mturquette@baylibre.com> Regards, Mike > + > static void __init mtk_topckgen_init(struct device_node *node) > { > struct clk_onecell_data *clk_data; > @@ -585,7 +600,7 @@ static void __init mtk_topckgen_init(struct device_node *node) > return; > } > > - clk_data = mtk_alloc_clk_data(CLK_TOP_NR); > + top_clk_data = clk_data = mtk_alloc_clk_data(CLK_TOP_NR); > > mtk_clk_register_fixed_clks(top_fixed_clks, ARRAY_SIZE(top_fixed_clks), > clk_data); > @@ -606,6 +621,8 @@ static void __init mtk_topckgen_init(struct device_node *node) > if (r) > pr_err("%s(): could not register clock provider: %d\n", > __func__, r); > + > + mtk_clk_enable_critical(); So you call the function here, and ... > } > CLK_OF_DECLARE(mtk_topckgen, "mediatek,mt2701-topckgen", mtk_topckgen_init); > > @@ -1201,7 +1218,7 @@ static void __init mtk_apmixedsys_init(struct device_node *node) > struct clk_onecell_data *clk_data; > int r; > > - clk_data = mtk_alloc_clk_data(ARRAY_SIZE(apmixed_plls)); > + pll_clk_data = clk_data = mtk_alloc_clk_data(ARRAY_SIZE(apmixed_plls)); > if (!clk_data) > return; > > @@ -1212,6 +1229,8 @@ static void __init mtk_apmixedsys_init(struct device_node *node) > if (r) > pr_err("%s(): could not register clock provider: %d\n", > __func__, r); > + > + mtk_clk_enable_critical(); ... here as well. So the prepare_count and enable_count will be 2? I'm not sure that this makes sense. If you have different clocks that need to be enabled at different times (due to the split registration scheme that you employ in this driver) then why not pass an array of those clocks into mtk_clk_enable_critical? Anyways, like I mentioned above I think we're close to a solution to handle this in the framework. One question: do you ever want these critical clocks to be gated by a driver, or do you want them always on, forever? Regards, Mike > } > CLK_OF_DECLARE(mtk_apmixedsys, "mediatek,mt2701-apmixedsys", > mtk_apmixedsys_init); > -- > 1.9.1 >