From patchwork Wed Apr 1 01:13:02 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Turquette X-Patchwork-Id: 6137091 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id BF7DBBF4A6 for ; Wed, 1 Apr 2015 01:19:10 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id C4F34201FE for ; Wed, 1 Apr 2015 01:19:09 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C98EE201FA for ; Wed, 1 Apr 2015 01:19:08 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1Yd7E4-0004vq-Rz; Wed, 01 Apr 2015 01:13:48 +0000 Received: from mail-ig0-f179.google.com ([209.85.213.179]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Yd7Dx-0004od-Co for linux-arm-kernel@lists.infradead.org; Wed, 01 Apr 2015 01:13:43 +0000 Received: by igbud6 with SMTP id ud6so35119426igb.1 for ; Tue, 31 Mar 2015 18:13:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:content-type:mime-version :content-transfer-encoding:to:from:in-reply-to:cc:references :message-id:user-agent:subject:date; bh=NcFhDugHtKZ8NVJPEvQaAHIcaxCDom5F3/rqCbGTNTk=; b=iLCXg/OA1U8z5tEkCxdeNHeZsLK/zScAYR/6StCarnCU61vUpLn4jq7+FlYawP+8Mf /saKXhBRpGq9xwQ1a8hhf29LwbENJWGoZauhtM0KECC3I9b3m8qh0X7OV3ChxeXPJGQy +pi0HotE+IcwPTSOwczsCvmJ6l6BmD3NZPBXndLM+8/cPqemnW3I5SxbdGzk8oN6klJW 7MsFWvHyBq06QepI9C5yhMwfp4OOKhyqen/nsziVJxZCVv0SWqmMsnKGsw1BaHKJGKjF e+pzLeRkcaQTT2L+BO0X4SQ5t9adai5Pp9ND6HipfbIiAIap1agJTW6UM1fjWHGyDJy0 1NmQ== X-Gm-Message-State: ALoCoQkF4omFvv/YwldQpf41fSotTitINCIGfqWbsQMa2G0rw8UC4gBRnNyrGNyfHv+v1jkfNa/r X-Received: by 10.42.193.69 with SMTP id dt5mr39950886icb.42.1427850795233; Tue, 31 Mar 2015 18:13:15 -0700 (PDT) Received: from localhost (pool-71-119-96-202.lsanca.fios.verizon.net. [71.119.96.202]) by mx.google.com with ESMTPSA id m191sm244456ioe.23.2015.03.31.18.13.13 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Tue, 31 Mar 2015 18:13:14 -0700 (PDT) MIME-Version: 1.0 To: Lee Jones , "Maxime Coquelin" From: Michael Turquette In-Reply-To: <20150302081606.GC31325@x1> References: <1425071674-16995-1-git-send-email-lee.jones@linaro.org> <1425071674-16995-4-git-send-email-lee.jones@linaro.org> <54F173B7.9030305@st.com> <20150302081606.GC31325@x1> Message-ID: <20150401011302.25195.55588@quantum> User-Agent: alot/0.3.5 Subject: Re: [STLinux Kernel] [PATCH 3/4] clk: Provide always-on clock support Date: Tue, 31 Mar 2015 18:13:02 -0700 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20150331_181341_529733_972B56F4 X-CRM114-Status: GOOD ( 25.26 ) X-Spam-Score: -0.7 (/) Cc: devicetree@vger.kernel.org, sboyd@codeaurora.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kernel@stlinux.com X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Quoting Lee Jones (2015-03-02 00:16:06) > On Sat, 28 Feb 2015, Maxime Coquelin wrote: > > > Hi Lee, > > > > On 02/27/2015 10:14 PM, Lee Jones wrote: > > >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 is registered with the framework it is > > >compared against a list of provided always-on clock names which must be > > >kept ungated. If it matches, we enable the existing CLK_IGNORE_UNUSED > > >flag, which will prevent the common clk framework from attempting to > > >gate it during the clk_disable_unused() procedure. > > > > Please correct me if I'm wrong, but your patch does not fix the > > issue you had initially. > > Let's take an example: > > A clock is critical for the system, and should never be gated, so > > you add the CLK_IGNORE_UNUSED > > flag so that it is not disabled by clk_disable_unused() procedure. > > The same clock is also used by other IPs, for example spi 0 instance. > > When starting a spi transfer, clk_enable() is called on this clock, > > so its usecount becomes 1. > > Once transfer done, clk_disable() is called, usecount becomes 0 and > > the clock gets disabled: system freeze. > > You're right. I also need to extend clk_core_disable() to take notice > of CLK_IGNORE_UNUSED. Hi Lee, I'd like to combine elements of your V3 and this series (V4?). Firstly let's stay away from modifying the semantics around CLK_IGNORE_UNUSED. There was discussion around adding a new flag which I'd like to avoid if possible. If we're leaving clocks enabled then it would be best to reflect that reality with the prepare/enable refcounting in the core. The best way to do this is with clk_prepare_enable(my_alwon_clk). Drivers already do this today by hacking calls to clk_prepare_enable into their driver's probe function, and we can support it more explicitly in DT. Since we already have assigned-clock-rate and assigned-clock-parent in drivers/clk/clk-conf.c I propose that we add a third property here, assigned-clock-alwon. I don't really care about the name, so feel free to change it, so long as your suggestion is Measurably Better. This property can only be added to clock providers (not consumers), because if we have a clock consumer then that consumer should just call clk_prepare_enable. See the pseudo-code below: Amazing, I know. Is there any aspect of this approach which does not solve your use case? We should mark this new property (assigned-clk-alwon) as unstable since it can (and likely will) grow into more stuff in the future (e.g. Stephen Boyd mentioned a "hand-off" which tells the core to keep clocks enabled until the correct driver claims them). Regards, Mike diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c index 48a65b2..19dce89 100644 --- a/drivers/clk/clk-conf.c +++ b/drivers/clk/clk-conf.c @@ -115,6 +115,14 @@ static int __set_clk_rates(struct device_node *node, bool clk_supplier) return 0; } +static int __set_clk_alwon(struct device_node *node, bool clk_supplier) +{ + if (!clk_supplier) + return -EINVAL; + + clk_prepare_enable(...); +} + /** * of_clk_set_defaults() - parse and set assigned clocks configuration * @node: device node to apply clock settings for @@ -138,6 +146,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_clk_alwon(node, clk_supplier); } EXPORT_SYMBOL_GPL(of_clk_set_defaults);