From patchwork Wed Mar 27 09:50:25 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ulf Hansson X-Patchwork-Id: 15687 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 4B48C23E10 for ; Wed, 27 Mar 2013 09:50:54 +0000 (UTC) Received: from mail-ve0-f176.google.com (mail-ve0-f176.google.com [209.85.128.176]) by fiordland.canonical.com (Postfix) with ESMTP id D0DDAA18144 for ; Wed, 27 Mar 2013 09:50:53 +0000 (UTC) Received: by mail-ve0-f176.google.com with SMTP id ox1so2022532veb.21 for ; Wed, 27 Mar 2013 02:50:53 -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=uom9a38eYEZ/44Rj2UgYBAFOnUUGihrIafK4TOv2f+4=; b=UdYw3XBU1C40CP9G7AWiTh0DHIfTDM7qXUjDsrtu7eRDyv04J04YTTS90g7QJHbWQQ kOwYXyF0O/5Qo7uPlYzRWVfQOV+Q9NvST2Yalkr/wu3FycTvlZ5NnfJ0j5viTHevgiXk zT/hce035X8Z/o9GoH/g0kwtgIhpvPEMKN2AzgWi7RFgLa2dcW2i8SZ2k5Vmm4FMlxD0 Q7yd9rN9iR57gE2L4UNeHRPKIs6sB3OXeUz+5j6t/V1sbiDMV/rnLTtV+HIIel+u+oZg GaLCmIVLD3BhMtbbDP6Yi7avIN1x54HPcr7/CekM011JIK1Mas8r0E0eDXUcS/WbPRC/ /ziQ== X-Received: by 10.220.106.14 with SMTP id v14mr6120108vco.2.1364377853375; Wed, 27 Mar 2013 02:50:53 -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.59.4.204 with SMTP id cg12csp108189ved; Wed, 27 Mar 2013 02:50:52 -0700 (PDT) X-Received: by 10.14.216.2 with SMTP id f2mr54434560eep.44.1364377852200; Wed, 27 Mar 2013 02:50:52 -0700 (PDT) Received: from eu1sys200aog105.obsmtp.com (eu1sys200aog105.obsmtp.com [207.126.144.119]) by mx.google.com with SMTP id g3si25236214eev.94.2013.03.27.02.50.42 (version=TLSv1 cipher=RC4-SHA bits=128/128); Wed, 27 Mar 2013 02:50:52 -0700 (PDT) Received-SPF: neutral (google.com: 207.126.144.119 is neither permitted nor denied by best guess record for domain of ulf.hansson@stericsson.com) client-ip=207.126.144.119; Authentication-Results: mx.google.com; spf=neutral (google.com: 207.126.144.119 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-ap.st.com ([138.198.100.35]) (using TLSv1) by eu1sys200aob105.postini.com ([207.126.147.11]) with SMTP ID DSNKUVLA8oXGDpU8fJWbqdcpXPCXgKs6Zeur@postini.com; Wed, 27 Mar 2013 09:50:51 UTC Received: from zeta.dmz-ap.st.com (ns6.st.com [138.198.234.13]) by beta.dmz-ap.st.com (STMicroelectronics) with ESMTP id 51D0D119; Wed, 27 Mar 2013 09:42:32 +0000 (GMT) Received: from relay1.stm.gmessaging.net (unknown [10.230.100.17]) by zeta.dmz-ap.st.com (STMicroelectronics) with ESMTP id C17FDEC1; Wed, 27 Mar 2013 09:50:38 +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 CE0B524C07C; Wed, 27 Mar 2013 10:50:32 +0100 (CET) Received: from steludxu1397.lud.stericsson.com (10.230.100.153) by smtp.stericsson.com (10.230.100.2) with Microsoft SMTP Server (TLS) id 8.3.279.5; Wed, 27 Mar 2013 10:50:37 +0100 From: Ulf Hansson To: , Mike Turquette Cc: Linus Walleij , Par-Olof Hakansson , Ulf Hansson Subject: [PATCH V3 3/3] clk: Fixup locking issues for clk_set_parent Date: Wed, 27 Mar 2013 10:50:25 +0100 Message-ID: <1364377825-6171-4-git-send-email-ulf.hansson@stericsson.com> X-Mailer: git-send-email 1.7.10 In-Reply-To: <1364377825-6171-1-git-send-email-ulf.hansson@stericsson.com> References: <1364377825-6171-1-git-send-email-ulf.hansson@stericsson.com> MIME-Version: 1.0 X-Gm-Message-State: ALoCoQkIdd+cXVuckSyrgYYI+4vrlev3Y4isTyuTV/WN8BBs2w7xaaLs7e3ayjSoJxQnnOq4z9uS 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 | 67 +++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 52 insertions(+), 15 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 6fa301b..90dcbd5 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1300,31 +1300,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 */ 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 */ if (parent && clk->ops->set_parent) ret = clk->ops->set_parent(clk->hw, p_index); - /* clean up old prepare and enable */ - spin_lock_irqsave(&enable_lock, flags); - 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. + */ + spin_lock_irqsave(&enable_lock, flags); + + 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; } /** @@ -1387,14 +1427,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: mutex_unlock(&prepare_lock);