From patchwork Fri Jul 8 23:32:10 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Turquette X-Patchwork-Id: 9221907 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id D4E0860467 for ; Fri, 8 Jul 2016 23:34:26 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C4B5228513 for ; Fri, 8 Jul 2016 23:34:26 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B795328615; Fri, 8 Jul 2016 23:34:26 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 523CF28513 for ; Fri, 8 Jul 2016 23:34:26 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1bLfGC-0008IQ-BV; Fri, 08 Jul 2016 23:32:40 +0000 Received: from mail-pf0-x229.google.com ([2607:f8b0:400e:c00::229]) by bombadil.infradead.org with esmtps (Exim 4.85_2 #1 (Red Hat Linux)) id 1bLfG7-0008Gq-NU for linux-arm-kernel@lists.infradead.org; Fri, 08 Jul 2016 23:32:37 +0000 Received: by mail-pf0-x229.google.com with SMTP id h14so16840864pfe.1 for ; Fri, 08 Jul 2016 16:32:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20150623.gappssmtp.com; s=20150623; h=mime-version:content-transfer-encoding:to:from:in-reply-to:cc :references:message-id:user-agent:subject:date; bh=9N5QfjvGIQd+q06mF9Xt1+I49dHhC3eEguEN7GybR9k=; b=NU0WYCaoC1F7W+fU5fdTDk9qzp9aQmDAZtJ3Pgkt5tuM6Z0QgHGjb143u1xeoESyEJ fnIOYs6fCUnTEM2P/SBIevXl5tY1tigo31sZeIOa2jywIybVWsxWlDVvhFmW+MM3NI+3 O2+023XK6yUsmpNaUjAL/LJXycNDMyg9wfY654ULi6ku0uGj4yU+TFSx4ZbhM8k5NKgK RW8HfIE9NdqE/1sdLOQgLbjdsBhIu+PITMTpX25SkZMwDLkU6rqI5mxOoAWKJ/Sh1l2Q luSKcNNIbPwRYU1w/Tak1z4GKRsIuHda746xpb7p1zDUKra/jSt6ASZT34mlrL1e3EaV HNlA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:content-transfer-encoding:to:from :in-reply-to:cc:references:message-id:user-agent:subject:date; bh=9N5QfjvGIQd+q06mF9Xt1+I49dHhC3eEguEN7GybR9k=; b=B5llcLBgfk2zc/3eAcvudR27dOhMeXxzFDttgwyrVeGEA72GYTkHbBFjIqBnCF6a/6 cWkMl6DHMTchjfWtJ1El9BAosi86f253AJD/RCz6WqGy/tTDPTHQ/ji1fD9EzSr1Xuht 3SasYDjSLfB0THksuApYg3pJSzxyR2xV1I4z3+dE99MLaP4/LyRj1NzEfK6nAFNR4rjL gLOI2zIcA6HxtlSLsDTEeBO2Zn2/PaJduBOOzl69f29FSPKl8/x/qPf2Ji4UB9u4iwTi RLKi5Kh3Z2srlZEGHbS0uscHhlJurPUe2RVVE4X1/DR+EM64Ty17t04FLurE0dg1o/2E ZdJg== X-Gm-Message-State: ALyK8tI2nhh0NyNzMxk1CDRiqplQm3owYhBlNc7JiLAeu5+VD1bQbklHb7JP5gqkq/dcK5M9 X-Received: by 10.98.35.27 with SMTP id j27mr14131430pfj.10.1468020733983; Fri, 08 Jul 2016 16:32:13 -0700 (PDT) Received: from localhost (cpe-172-248-200-249.socal.res.rr.com. [172.248.200.249]) by smtp.gmail.com with ESMTPSA id c13sm5291260pfc.40.2016.07.08.16.32.12 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 08 Jul 2016 16:32:13 -0700 (PDT) MIME-Version: 1.0 To: James Liao , "Stephen Boyd" From: Michael Turquette In-Reply-To: <1467604308.26485.4.camel@mtksdaap41> References: <1466581229-2342-1-git-send-email-erin.lo@mediatek.com> <1466581229-2342-2-git-send-email-erin.lo@mediatek.com> <20160702012140.GB17071@codeaurora.org> <1467604308.26485.4.camel@mtksdaap41> Message-ID: <146802073038.73491.6675612765998057903@resonance> User-Agent: alot/0.3.7 Subject: Re: [PATCH v9 01/10] clk: fix initial state of critical clock's parents Date: Fri, 08 Jul 2016 16:32:10 -0700 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160708_163235_820083_ABC9ECF6 X-CRM114-Status: GOOD ( 28.35 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Philipp Zabel , Arnd Bergmann , devicetree@vger.kernel.org, Erin Lo , linux-kernel@vger.kernel.org, Daniel Kurtz , srv_heupstream@mediatek.com, linux-mediatek@lists.infradead.org, Sascha Hauer , Matthias Brugger , linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, John Crispin Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP Hi James, Quoting James Liao (2016-07-03 20:51:48) > On Fri, 2016-07-01 at 18:21 -0700, Stephen Boyd wrote: > > (Resending to everyone) > > > > On 06/22, Erin Lo wrote: > > > From: James Liao > > > > > > This patch fixed wrong state of parent clocks if they are registered > > > after critical clocks. > > > > > > Signed-off-by: James Liao > > > Signed-off-by: Erin Lo > > > > It would be nice if you included the information about the > > problem from James' previous mail. This says what it does, but > > doesn't explain what the problem is and how it is fixing it. > > > > > --- > > > drivers/clk/clk.c | 9 ++++++++- > > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > > > index d584004..e9f5f89 100644 > > > --- a/drivers/clk/clk.c > > > +++ b/drivers/clk/clk.c > > > @@ -2388,8 +2388,15 @@ static int __clk_core_init(struct clk_core *core) > > > hlist_for_each_entry_safe(orphan, tmp2, &clk_orphan_list, child_node) { > > > struct clk_core *parent = __clk_init_parent(orphan); > > > > > > - if (parent) > > > + if (parent) { > > > clk_core_reparent(orphan, parent); > > > + > > > + if (orphan->prepare_count) > > > + clk_core_prepare(parent); > > > + > > > + if (orphan->enable_count) > > > + clk_core_enable(parent); > > > + } > > > } > > > > I'm pretty sure I pointed this problem out to Mike when the > > critical clk patches were being pushed. I can't recall what the > > plan was though to fix the problem. I'm pretty sure he said that > > clk_core_reparent() would take care of it, but obviously it is > > not doing that. Or perhaps it was that clk handoff should figure > > out that the parents of a critical clk are also on and thus keep > > them on. > > Hi Mike > > Is there any other patch to fix this issue? Or did I misuse critical > clock flag? There is no fix yes. Your fix is basically correct. I was mistaken back when I told you and Stephen that the framework already took care of this. However, instead of "open coding" this solution, I would rather re-use the __clk_set_parent_{before,after} helpers instead. Can you review/test the following patch and let me know what you think? Thanks, Mike From c0163b3f719b1e219b28ad425f94f9ef54a25a8f Mon Sep 17 00:00:00 2001 From: Michael Turquette Date: Fri, 8 Jul 2016 16:05:22 -0700 Subject: [PATCH] clk: migrate ref counts when orphans are reunited It's always nice to see families reunited, and this is equally true when talking about parent clocks and their children. However, if the orphan clk had a positive prepare_count or enable_count, then we would not migrate those counts up the parent chain correctly. This has manifested with the recent critical clocks feature, which often enables clocks very early, before their parents have been registered. Fixed by replacing the call to clk_core_reparent with calls to __clk_set_parent_{before,after}. Cc: James Liao Cc: Erin Lo Signed-off-by: Michael Turquette --- drivers/clk/clk.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 820a939fb6bb..70efe4c4e0cc 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -2449,8 +2449,14 @@ static int __clk_core_init(struct clk_core *core) hlist_for_each_entry_safe(orphan, tmp2, &clk_orphan_list, child_node) { struct clk_core *parent = __clk_init_parent(orphan); - if (parent) - clk_core_reparent(orphan, parent); + /* + * we could call __clk_set_parent, but that would result in a + * reducant call to the .set_rate op, if it exists + */ + if (parent) { + __clk_set_parent_before(orphan, parent); + __clk_set_parent_after(orphan, parent, NULL); + } } /*