From patchwork Tue Apr 2 21:09:39 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ulf Hansson X-Patchwork-Id: 2380411 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork2.kernel.org (Postfix) with ESMTP id 031BDDF2A1 for ; Tue, 2 Apr 2013 21:12:41 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1UN8TR-0007HJ-0C; Tue, 02 Apr 2013 21:10:33 +0000 Received: from eu1sys200aog107.obsmtp.com ([207.126.144.123]) by merlin.infradead.org with smtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1UN8T0-0007Bt-GX for linux-arm-kernel@lists.infradead.org; Tue, 02 Apr 2013 21:10:09 +0000 Received: from beta.dmz-eu.st.com ([164.129.1.35]) (using TLSv1) by eu1sys200aob107.postini.com ([207.126.147.11]) with SMTP ID DSNKUVtJKyeRcQPvArcdZiffWMjMQxdf9M93@postini.com; Tue, 02 Apr 2013 21:10:06 UTC Received: from zeta.dmz-eu.st.com (zeta.dmz-eu.st.com [164.129.230.9]) by beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 03ADEE8; Tue, 2 Apr 2013 21:09:57 +0000 (GMT) Received: from relay1.stm.gmessaging.net (unknown [10.230.100.17]) by zeta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 951DCA151; Tue, 2 Apr 2013 21:09:57 +0000 (GMT) Received: from exdcvycastm004.EQ1STM.local (alteon-source-exch [10.230.100.61]) (using TLSv1 with cipher RC4-MD5 (128/128 bits)) (Client CN "exdcvycastm004", Issuer "exdcvycastm004" (not verified)) by relay1.stm.gmessaging.net (Postfix) with ESMTPS id 976D524C07D; Tue, 2 Apr 2013 23:09:46 +0200 (CEST) Received: from steludxu1397.stericsson.com (10.230.100.153) by smtp.stericsson.com (10.230.100.2) with Microsoft SMTP Server (TLS) id 8.3.279.5; Tue, 2 Apr 2013 23:09:56 +0200 From: Ulf Hansson To: , Mike Turquette Subject: [PATCH V4 3/3] clk: Fixup locking issues for clk_set_parent Date: Tue, 2 Apr 2013 23:09:39 +0200 Message-ID: <1364936979-20805-4-git-send-email-ulf.hansson@stericsson.com> X-Mailer: git-send-email 1.7.10 In-Reply-To: <1364936979-20805-1-git-send-email-ulf.hansson@stericsson.com> References: <1364936979-20805-1-git-send-email-ulf.hansson@stericsson.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130402_171006_946469_E20D28B6 X-CRM114-Status: GOOD ( 19.02 ) X-Spam-Score: -4.2 (----) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-4.2 points) pts rule name description ---- ---------------------- -------------------------------------------------- -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/, medium trust [207.126.144.123 listed in list.dnswl.org] -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Rajagopal Venkat , Linus Walleij , Ulf Hansson , Par-Olof Hakansson X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 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 From: Ulf Hansson Updating the clock tree topology must be protected with the spinlock when doing clk_set_parent, otherwise we can not handle the migration of the enable_count in a safe manner. While issuing the .set_parent callback to make the clk-hw perform the switch to the new parent, we can not hold the spinlock since it is must be allowed to be slow path. This complicates error handling, but is still possible to achieve. Signed-off-by: Ulf Hansson Cc: Rajagopal Venkat --- drivers/clk/clk.c | 67 +++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 52 insertions(+), 15 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index c83e8e5..de6b459 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1363,31 +1363,71 @@ static int __clk_set_parent(struct clk *clk, struct clk *parent, u8 p_index) unsigned long flags; int ret = 0; struct clk *old_parent = clk->parent; + bool migrated_enable = false; - /* migrate prepare and enable */ + /* migrate prepare */ if (clk->prepare_count) __clk_prepare(parent); - /* FIXME replace with clk_is_enabled(clk) someday */ flags = clk_enable_lock(); - if (clk->enable_count) + + /* migrate enable */ + if (clk->enable_count) { __clk_enable(parent); + migrated_enable = true; + } + + /* update the clk tree topology */ + clk_reparent(clk, parent); + clk_enable_unlock(flags); /* change clock input source */ if (parent && clk->ops->set_parent) ret = clk->ops->set_parent(clk->hw, p_index); - /* clean up old prepare and enable */ - flags = clk_enable_lock(); - if (clk->enable_count) + if (ret) { + /* + * The error handling is tricky due to that we need to release + * the spinlock while issuing the .set_parent callback. This + * means the new parent might have been enabled/disabled in + * between, which must be considered when doing rollback. + */ + flags = clk_enable_lock(); + + clk_reparent(clk, old_parent); + + if (migrated_enable && clk->enable_count) { + __clk_disable(parent); + } else if (migrated_enable && (clk->enable_count == 0)) { + __clk_disable(old_parent); + } else if (!migrated_enable && clk->enable_count) { + __clk_disable(parent); + __clk_enable(old_parent); + } + + clk_enable_unlock(flags); + + if (clk->prepare_count) + __clk_unprepare(parent); + + return ret; + } + + /* clean up enable for old parent if migration was done */ + if (migrated_enable) { + flags = clk_enable_lock(); __clk_disable(old_parent); - clk_enable_unlock(flags); + clk_enable_unlock(flags); + } + /* clean up prepare for old parent if migration was done */ if (clk->prepare_count) __clk_unprepare(old_parent); - return ret; + /* update debugfs with new clk tree topology */ + clk_debug_reparent(clk, parent); + return 0; } /** @@ -1450,14 +1490,11 @@ int clk_set_parent(struct clk *clk, struct clk *parent) /* do the re-parent */ ret = __clk_set_parent(clk, parent, p_index); - /* propagate ABORT_RATE_CHANGE if .set_parent failed */ - if (ret) { + /* propagate rate recalculation accordingly */ + if (ret) __clk_recalc_rates(clk, ABORT_RATE_CHANGE); - goto out; - } - - /* propagate rate recalculation downstream */ - __clk_reparent(clk, parent); + else + __clk_recalc_rates(clk, POST_RATE_CHANGE); out: clk_prepare_unlock();