Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752514AbbHTNXZ (ORCPT ); Thu, 20 Aug 2015 09:23:25 -0400 Received: from mail-la0-f48.google.com ([209.85.215.48]:36449 "EHLO mail-la0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751571AbbHTNXX (ORCPT ); Thu, 20 Aug 2015 09:23:23 -0400 MIME-Version: 1.0 In-Reply-To: <20150817074244.GA3239@x1> References: <1437570255-21049-1-git-send-email-lee.jones@linaro.org> <1437570255-21049-5-git-send-email-lee.jones@linaro.org> <20150817074244.GA3239@x1> From: Barry Song <21cnbao@gmail.com> Date: Thu, 20 Aug 2015 21:23:02 +0800 Message-ID: Subject: Re: [PATCH v7 4/5] clk: Provide critical clock support To: Lee Jones Cc: "linux-arm-kernel@lists.infradead.org" , LKML , "devicetree@vger.kernel.org" , Mike Turquette , kernel@stlinux.com, Sascha Hauer , Stephen Boyd , Geert Uytterhoeven , Maxime Ripard , shengtao.sheng@csr.com Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5734 Lines: 161 2015-08-17 15:42 GMT+08:00 Lee Jones : > On Mon, 17 Aug 2015, Barry Song wrote: > >> 2015-07-22 21:04 GMT+08:00 Lee Jones : >> > Lots of platforms contain clocks which if turned off would prove fatal. >> > The only way to recover from these catastrophic failures is to restart >> > the board(s). Now, when a clock provider is registered with the >> > framework it is possible for a list of critical clocks to be supplied >> > which must be kept ungated. Each clock mentioned in the newly >> > introduced 'critical-clock' property will be clk_prepare_enable()d >> > where the normal references will be taken. This will prevent the >> > common clk framework from attempting to gate them during the normal >> > clk_disable_unused() and disable_clock() procedures. >> > >> > Note that it will still be possible for knowledgeable drivers to turn >> > these clocks off using clk_{enable,disable}_critical() calls. >> > >> > Signed-off-by: Lee Jones >> >> hi Lee, >> i have another email about this. i am wondering whether >> CLK_IGNORE_UNUSE is still useful after your patch. another solution >> for your patch might be extending the current CLK_IGNORE_UNUSE to >> runtime? >> >> >> copy the mail here: >> currently we can set a CLK_IGNORE_UNUSE flag to a clock to stop >> clk_disable_unused() from disabling it at the boot stage: >> >> static void clk_disable_unused_subtree(struct clk_core *core) >> { >> ... >> >> if (core->flags & CLK_IGNORE_UNUSED) >> goto unlock_out; >> } >> >> static int clk_disable_unused(void) >> { >> ... >> >> clk_unprepare_unused_subtree(core); >> ... >> return 0; >> } >> >> late_initcall_sync(clk_disable_unused); >> >> so if there are two clocks A and B, A is the parent of B, and A is >> marked as CLK_IGNORE_UNUSED. >> >> in boot stage if there is nobody using A and B, Linux will disable B >> due to clk_disable_unused() , but keep A being enabled since A has >> CLK_IGNORE_UNUSED. >> >> but in Linux runtime, we might frequently disable /enable B in runtime >> power management, this will cause A disabled since Linux will not >> check CLK_IGNORE_UNUSED for runtime disabling clk . >> >> so this makes CLK_IGNORE_UNUSE only work if we don't disable its >> sub-clock at runtime. this looks making no sense. >> >> i am thinking whether we should do some changes to make it have side >> affect for runtime clk disable. otherwise, why do we want to stop it >> from being disabled during boot stage? > > This is one of this problems, along with some others that this set > aims to solve. > > Extending CLK_IGNORE_UNUSED is not a good idea. In fact, if we can > phase it out completely, that will be a good thing. i would agree it is better we can drop CLK_IGNORE_UNUSED since it is confusing... > > Since this set Mike has submitted an alternitive solution. > > Please see: https://groups.google.com/forum/#!msg/linux.kernel/kX_nWSsWRxU/IZSjhG5Ed4oJ > >> I am not sure whether i missed something in clk core level support. >> >> -barry >> >> > --- >> > drivers/clk/clk-conf.c | 45 ++++++++++++++++++++++++++++++++++++++++++++- >> > 1 file changed, 44 insertions(+), 1 deletion(-) >> > >> > diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c >> > index aad4796..f83c1c2 100644 >> > --- a/drivers/clk/clk-conf.c >> > +++ b/drivers/clk/clk-conf.c >> > @@ -116,6 +116,45 @@ static int __set_clk_rates(struct device_node *node, bool clk_supplier) >> > return 0; >> > } >> > >> > +static int __set_critical_clocks(struct device_node *node, bool clk_supplier) >> > +{ >> > + struct of_phandle_args clkspec; >> > + struct clk *clk; >> > + struct property *prop; >> > + const __be32 *cur; >> > + uint32_t index; >> > + int ret; >> > + >> > + if (!clk_supplier) >> > + return 0; >> > + >> > + of_property_for_each_u32(node, "critical-clock", prop, cur, index) { >> > + clkspec.np = node; >> > + clkspec.args_count = 1; >> > + clkspec.args[0] = index; >> > + >> > + clk = of_clk_get_from_provider(&clkspec); >> > + if (IS_ERR(clk)) { >> > + pr_err("clk: couldn't get clock %u for %s\n", >> > + index, node->full_name); >> > + return PTR_ERR(clk); >> > + } >> > + >> > + clk_init_critical(clk); >> > + >> > + ret = clk_prepare_enable(clk); >> > + if (ret) { >> > + pr_err("Failed to enable clock %u for %s: %d\n", >> > + index, node->full_name, ret); >> > + return ret; >> > + } >> > + >> > + pr_debug("Setting clock as critical %pC\n", clk); >> > + } >> > + >> > + return 0; >> > +} >> > + >> > /** >> > * of_clk_set_defaults() - parse and set assigned clocks configuration >> > * @node: device node to apply clock settings for >> > @@ -139,6 +178,10 @@ int of_clk_set_defaults(struct device_node *node, bool clk_supplier) >> > if (rc < 0) >> > return rc; >> > >> > - return __set_clk_rates(node, clk_supplier); >> > + rc = __set_clk_rates(node, clk_supplier); >> > + if (rc < 0) >> > + return rc; >> > + >> > + return __set_critical_clocks(node, clk_supplier); >> > } >> > EXPORT_SYMBOL_GPL(of_clk_set_defaults); > > -- -barry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/