From patchwork Tue Mar 12 19:20:50 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ulf Hansson X-Patchwork-Id: 15306 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id 5B8E823E2C for ; Tue, 12 Mar 2013 19:21:19 +0000 (UTC) Received: from mail-vb0-f45.google.com (mail-vb0-f45.google.com [209.85.212.45]) by fiordland.canonical.com (Postfix) with ESMTP id 0821CA18A2A for ; Tue, 12 Mar 2013 19:21:18 +0000 (UTC) Received: by mail-vb0-f45.google.com with SMTP id p1so72662vbi.4 for ; Tue, 12 Mar 2013 12:21:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:x-forwarded-to:x-forwarded-for:delivered-to:x-received :received-spf:from:to:cc:subject:date:message-id:x-mailer :in-reply-to:references:mime-version:content-type:x-gm-message-state; bh=A6U/4RKJcaOZZ/RqR4QmYQuk87Xjq05LQc343W6mx50=; b=lKcBldc7qE8vpaq7hwkTrDRNTb8smSTBNaWEaa2/Q8plWnepXT/A013uBY94qmjRrw xnMlWmgnTv2G81MIxulmk5UZ1uh0MF75oiEy1/6ceg5kct7CGKfFWkLSQVpX7Z8ixdug HlPbeRF71ulbrXLPMQIeLU5kl4rahqAqSp7WCHspdlKj5z1yOhTLzTa6KdrMeEYkPBGW 4ukk+CNB97NyJPG0TOt2BUgPnPNwqNU84E6Chj5/MOkpIZ4x99uAgFlO5lYnZXDdF2vk 5xQIzQIh6/w+eoYYGbl5E/b0qokgFmgmU7yPFTIxZdF2gtawiHlKm6xEpBKG2GOzOkXV rB+Q== X-Received: by 10.221.11.135 with SMTP id pe7mr6788900vcb.41.1363116078505; Tue, 12 Mar 2013 12:21:18 -0700 (PDT) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.58.127.98 with SMTP id nf2csp144398veb; Tue, 12 Mar 2013 12:21:17 -0700 (PDT) X-Received: by 10.14.111.72 with SMTP id v48mr875370eeg.11.1363116077415; Tue, 12 Mar 2013 12:21:17 -0700 (PDT) Received: from eu1sys200aog119.obsmtp.com (eu1sys200aog119.obsmtp.com [207.126.144.147]) by mx.google.com with SMTP id f9si37358608eep.101.2013.03.12.12.21.13 (version=TLSv1 cipher=RC4-SHA bits=128/128); Tue, 12 Mar 2013 12:21:17 -0700 (PDT) Received-SPF: neutral (google.com: 207.126.144.147 is neither permitted nor denied by best guess record for domain of ulf.hansson@stericsson.com) client-ip=207.126.144.147; Authentication-Results: mx.google.com; spf=neutral (google.com: 207.126.144.147 is neither permitted nor denied by best guess record for domain of ulf.hansson@stericsson.com) smtp.mail=ulf.hansson@stericsson.com Received: from beta.dmz-us.st.com ([167.4.1.35]) (using TLSv1) by eu1sys200aob119.postini.com ([207.126.147.11]) with SMTP ID DSNKUT+AKJaDJJy0IeLHw7rRFjzo9zzCTAJi@postini.com; Tue, 12 Mar 2013 19:21:17 UTC Received: from zeta.dmz-us.st.com (ns4.st.com [167.4.16.71]) by beta.dmz-us.st.com (STMicroelectronics) with ESMTP id E190F6D; Tue, 12 Mar 2013 19:20:19 +0000 (GMT) Received: from relay1.stm.gmessaging.net (unknown [10.230.100.17]) by zeta.dmz-us.st.com (STMicroelectronics) with ESMTP id A48824F2; Tue, 12 Mar 2013 13:01:08 +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 77D2624C07C; Tue, 12 Mar 2013 20:20:58 +0100 (CET) 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.83.0; Tue, 12 Mar 2013 20:21:07 +0100 From: Ulf Hansson To: , Mike Turquette Cc: Linus Walleij , Par-Olof Hakansson , Ulf Hansson Subject: [RESEND PATCH 3/3] clk: Fixup locking issues for clk_set_parent Date: Tue, 12 Mar 2013 20:20:50 +0100 Message-ID: <1363116050-3882-4-git-send-email-ulf.hansson@stericsson.com> X-Mailer: git-send-email 1.7.10 In-Reply-To: <1363116050-3882-1-git-send-email-ulf.hansson@stericsson.com> References: <1363116050-3882-1-git-send-email-ulf.hansson@stericsson.com> MIME-Version: 1.0 X-Gm-Message-State: ALoCoQm1U74GPyq3qhXodaEuJd8NNgWWprhf1JE0ZJg56OG3VMXCWMKaTdcpgo233IfbGZKhECa7 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);