From patchwork Fri Jan 25 11:40:57 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ulf Hansson X-Patchwork-Id: 2044341 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork1.kernel.org (Postfix) with ESMTP id 10ABC3FDD1 for ; Fri, 25 Jan 2013 11:44:03 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1Tyhf0-0002yk-GX; Fri, 25 Jan 2013 11:41:30 +0000 Received: from eu1sys200aog111.obsmtp.com ([207.126.144.131]) by merlin.infradead.org with smtps (Exim 4.76 #1 (Red Hat Linux)) id 1Tyhep-0002wb-WF for linux-arm-kernel@lists.infradead.org; Fri, 25 Jan 2013 11:41:22 +0000 Received: from beta.dmz-eu.st.com ([164.129.1.35]) (using TLSv1) by eu1sys200aob111.postini.com ([207.126.147.11]) with SMTP ID DSNKUQJvXL9n4nAdSddzOXxGKVbpQj2aSlqf@postini.com; Fri, 25 Jan 2013 11:41:19 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 26490F0; Fri, 25 Jan 2013 11:41:11 +0000 (GMT) Received: from relay1.stm.gmessaging.net (unknown [10.230.100.17]) by zeta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 80EC3488E; Fri, 25 Jan 2013 11:41:11 +0000 (GMT) Received: from exdcvycastm022.EQ1STM.local (alteon-source-exch [10.230.100.61]) (using TLSv1 with cipher RC4-MD5 (128/128 bits)) (Client CN "exdcvycastm022", Issuer "exdcvycastm022" (not verified)) by relay1.stm.gmessaging.net (Postfix) with ESMTPS id 469DB24C07C; Fri, 25 Jan 2013 12:41:05 +0100 (CET) Received: from steludxu1397.stericsson.com (10.230.100.153) by smtp.stericsson.com (10.230.100.30) with Microsoft SMTP Server (TLS) id 8.3.83.0; Fri, 25 Jan 2013 12:41:10 +0100 From: Ulf Hansson To: , Mike Turquette Subject: [PATCH 3/3] clk: Fixup locking issues for clk_set_parent Date: Fri, 25 Jan 2013 12:40:57 +0100 Message-ID: <1359114057-11681-4-git-send-email-ulf.hansson@stericsson.com> X-Mailer: git-send-email 1.7.10 In-Reply-To: <1359114057-11681-1-git-send-email-ulf.hansson@stericsson.com> References: <1359114057-11681-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-20130125_064120_358158_BA12F98B X-CRM114-Status: GOOD ( 18.99 ) 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.131 listed in list.dnswl.org] -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Philippe Begnic , Linus Walleij , Ulf Hansson X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org 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 --- drivers/clk/clk.c | 68 +++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 51 insertions(+), 17 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index a61cf6d..0c19420 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1258,30 +1258,69 @@ static int __clk_set_parent(struct clk *clk, struct clk *parent, u8 p_index) unsigned long flags; int ret; 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 */ spin_lock_irqsave(&enable_lock, flags); - 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); + spin_unlock_irqrestore(&enable_lock, flags); /* change clock input source */ ret = clk->ops->set_parent(clk->hw, p_index); + 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. + */ + spin_lock_irqsave(&enable_lock, flags); - /* clean up old prepare and enable */ - spin_lock_irqsave(&enable_lock, flags); - if (clk->enable_count) + 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); + } + + spin_unlock_irqrestore(&enable_lock, flags); + + if (clk->prepare_count) + __clk_unprepare(parent); + + return ret; + } + + /* clean up enable for old parent if migration was done */ + if (migrated_enable) { + spin_lock_irqsave(&enable_lock, flags); __clk_disable(old_parent); - spin_unlock_irqrestore(&enable_lock, flags); + spin_unlock_irqrestore(&enable_lock, 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; } /** @@ -1339,16 +1378,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); - clk_debug_reparent(clk, parent); - __clk_recalc_rates(clk, POST_RATE_CHANGE); + else + __clk_recalc_rates(clk, POST_RATE_CHANGE); out: mutex_unlock(&prepare_lock);