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: 14287 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 09C8623F02 for ; Fri, 25 Jan 2013 11:41:23 +0000 (UTC) Received: from mail-ve0-f179.google.com (mail-ve0-f179.google.com [209.85.128.179]) by fiordland.canonical.com (Postfix) with ESMTP id A29ABA182BC for ; Fri, 25 Jan 2013 11:41:22 +0000 (UTC) Received: by mail-ve0-f179.google.com with SMTP id da11so109769veb.24 for ; Fri, 25 Jan 2013 03:41:22 -0800 (PST) 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=bDqJBqRykD+ObC5CbHKggXGhaCd87TQFIdpdHDfnmsvVSRmMOcAezUuCICJnsxl0VT Spp5OG/agec8afbCtDe2rnG6EJn2aj12k5uDfv3Bbd/qAJ/+3nG0VrT1AR1tsF3Z+7ny 9Qb827Av2SK7AUaCb9K6VkRgE7B9JApeGt2ealfV/+YDY5LdNGnWPN6ZsBy83Qbr2G3p Bim/fDnxfWZ0Y+lB7/W+jxV/jC8KmtLB9yQhsHNRYZYQD0cTCuwD0caV9kFz6hHXFRLg PAVKQYnJNkqP8dZ1Wt9PL8SXk4g991adaoOqeJ9DuSTnE3LZ0VAVUiLoR4DYt1EgU/xb e/uw== X-Received: by 10.220.39.69 with SMTP id f5mr5570631vce.45.1359114082136; Fri, 25 Jan 2013 03:41:22 -0800 (PST) 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.145.101 with SMTP id st5csp111820veb; Fri, 25 Jan 2013 03:41:21 -0800 (PST) X-Received: by 10.194.78.162 with SMTP id c2mr8283759wjx.46.1359114079958; Fri, 25 Jan 2013 03:41:19 -0800 (PST) Received: from eu1sys200aog111.obsmtp.com (eu1sys200aog111.obsmtp.com [207.126.144.131]) by mx.google.com with SMTP id l1si1231779een.111.2013.01.25.03.41.16 (version=TLSv1 cipher=RC4-SHA bits=128/128); Fri, 25 Jan 2013 03:41:19 -0800 (PST) Received-SPF: neutral (google.com: 207.126.144.131 is neither permitted nor denied by best guess record for domain of ulf.hansson@stericsson.com) client-ip=207.126.144.131; Authentication-Results: mx.google.com; spf=neutral (google.com: 207.126.144.131 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 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 Cc: Linus Walleij , Philippe Begnic , Ulf Hansson 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-Gm-Message-State: ALoCoQnG0GleaY5UAaQKyI04phIWra6MhwUYvng2hXOSGCQ602ctuVYVwRCznXJfbdYdCYTBWCF2 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);