From patchwork Thu Mar 21 13:48:13 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ulf Hansson X-Patchwork-Id: 15471 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 5B80B23E3E for ; Thu, 21 Mar 2013 13:48:36 +0000 (UTC) Received: from mail-vc0-f181.google.com (mail-vc0-f181.google.com [209.85.220.181]) by fiordland.canonical.com (Postfix) with ESMTP id 0333CA18434 for ; Thu, 21 Mar 2013 13:48:35 +0000 (UTC) Received: by mail-vc0-f181.google.com with SMTP id hv10so2311018vcb.12 for ; Thu, 21 Mar 2013 06:48:35 -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=ie6R2CBvYA/GzmE+iO6D8+fSp3UJ1JhbaHa1tlj8NiY=; b=EdJTmjRWjMSZO8aAjj5MHflpP7vEyB0SDDzVPSvJk/J3jNXTGUkw3YKK7feCAX2Onr +7KbPhj37ot++nBeR+QmOeqeAN3tVXCttm/ez3MtKz+p9F6VnDGGqy9a+FqkkDcN5rz7 f1VguGFvVKE0NGzEH7Z8af2OTuqyru6/F/UTO5mK8O2BbMNpJ6VoMavfkJDVXfOOGgHN MTFnYQG3cK+1uB4TXjvNvnr5TcQwMv0ijk1vlnKpqbq/uRRyKPUbj3uX2s1Anp/OQaNE pO/t4ZzF6s8Ab2rG54FFFUAQEy3pfuw0RPyz3HcNdUirW4eRpNhITnFaebRNWRpWo0Wz EpDA== X-Received: by 10.220.150.74 with SMTP id x10mr13463964vcv.68.1363873715504; Thu, 21 Mar 2013 06:48:35 -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.233.198 with SMTP id ty6csp61165vec; Thu, 21 Mar 2013 06:48:34 -0700 (PDT) X-Received: by 10.194.82.34 with SMTP id f2mr17412387wjy.25.1363873713686; Thu, 21 Mar 2013 06:48:33 -0700 (PDT) Received: from psmtp.com (eu1sys200aog123.obsmtp.com [207.126.144.155]) by mx.google.com with SMTP id t43si8895473eeg.86.2013.03.21.06.48.29 (version=TLSv1 cipher=RC4-SHA bits=128/128); Thu, 21 Mar 2013 06:48:33 -0700 (PDT) Received-SPF: neutral (google.com: 207.126.144.155 is neither permitted nor denied by best guess record for domain of ulf.hansson@stericsson.com) client-ip=207.126.144.155; Authentication-Results: mx.google.com; spf=neutral (google.com: 207.126.144.155 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-eu.st.com ([164.129.1.35]) (using TLSv1) by eu1sys200aob123.postini.com ([207.126.147.11]) with SMTP ID DSNKUUsPrTHg5LtIE9CASi12ynn4MS7tTwN2@postini.com; Thu, 21 Mar 2013 13:48:33 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 25950ED; Thu, 21 Mar 2013 13:48:29 +0000 (GMT) Received: from relay2.stm.gmessaging.net (unknown [10.230.100.18]) by zeta.dmz-eu.st.com (STMicroelectronics) with ESMTP id B5CC75079; Thu, 21 Mar 2013 13:48:28 +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 relay2.stm.gmessaging.net (Postfix) with ESMTPS id 2EB98A8074; Thu, 21 Mar 2013 14:48:23 +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.279.5; Thu, 21 Mar 2013 14:48:28 +0100 From: Ulf Hansson To: , Mike Turquette Cc: Linus Walleij , Par-Olof Hakansson , Ulf Hansson Subject: [PATCH V2 3/3] clk: Fixup locking issues for clk_set_parent Date: Thu, 21 Mar 2013 14:48:13 +0100 Message-ID: <1363873693-30902-4-git-send-email-ulf.hansson@stericsson.com> X-Mailer: git-send-email 1.7.10 In-Reply-To: <1363873693-30902-1-git-send-email-ulf.hansson@stericsson.com> References: <1363873693-30902-1-git-send-email-ulf.hansson@stericsson.com> MIME-Version: 1.0 X-Gm-Message-State: ALoCoQnowLrd7ZadeBz+tIIe9X381GLwZFIgdhHDSMsjyKvBksQNDTHN7/00D1CKhmS6HBYRdlVt 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 c7eb0d7..fee7628 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1300,30 +1300,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; } /** @@ -1381,16 +1420,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);