From patchwork Wed Jul 26 07:37:39 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Boyd X-Patchwork-Id: 9864319 X-Patchwork-Delegate: sboyd@codeaurora.org Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 9078E60382 for ; Wed, 26 Jul 2017 07:37:44 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7E2A728632 for ; Wed, 26 Jul 2017 07:37:44 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 72B2F28738; Wed, 26 Jul 2017 07:37:44 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D40F528632 for ; Wed, 26 Jul 2017 07:37:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750883AbdGZHhn (ORCPT ); Wed, 26 Jul 2017 03:37:43 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:33506 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750826AbdGZHhm (ORCPT ); Wed, 26 Jul 2017 03:37:42 -0400 Received: by smtp.codeaurora.org (Postfix, from userid 1000) id E1EE66087E; Wed, 26 Jul 2017 07:37:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1501054661; bh=dOWVn6b+Yi9OsTtmKTRlX8ZffdukCgtkkFMrZ+5jFD4=; h=From:To:Cc:Subject:Date:From; b=cQaUeMfERzsOuZkQwUhM6wINn9PnIjLAtF2OWFTj/mfuMBTIiAsddK15BuLoCC6Dc 5Yi49XiE0NGcoZeYRyr4/rGsGeeiT7rYtMRVDcV0XNBMzz6bFlXBhBjXtNMiLJGxIv ZzmyQdzpROZW1GnasQ30cKzw9yd87YYLz0Y7b3C0= Received: from sboyd-linux.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: sboyd@smtp.codeaurora.org) by smtp.codeaurora.org (Postfix) with ESMTPSA id C68F9607C7; Wed, 26 Jul 2017 07:37:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1501054661; bh=dOWVn6b+Yi9OsTtmKTRlX8ZffdukCgtkkFMrZ+5jFD4=; h=From:To:Cc:Subject:Date:From; b=cQaUeMfERzsOuZkQwUhM6wINn9PnIjLAtF2OWFTj/mfuMBTIiAsddK15BuLoCC6Dc 5Yi49XiE0NGcoZeYRyr4/rGsGeeiT7rYtMRVDcV0XNBMzz6bFlXBhBjXtNMiLJGxIv ZzmyQdzpROZW1GnasQ30cKzw9yd87YYLz0Y7b3C0= DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org C68F9607C7 Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=sboyd@codeaurora.org From: Stephen Boyd To: Michael Turquette , Stephen Boyd Cc: linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org, Jerome Brunet Subject: [PATCH] clk: Remove recursion in clk_core_{prepare,enable}() Date: Wed, 26 Jul 2017 00:37:39 -0700 Message-Id: <20170726073739.16823-1-sboyd@codeaurora.org> X-Mailer: git-send-email 2.14.0.rc0 Sender: linux-clk-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-clk@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Enabling and preparing clocks can be written quite naturally with recursion. We start at some point in the tree and recurse up the tree to find the oldest parent clk that needs to be enabled or prepared. Then we enable/prepare and return to the caller, going back to the clk we started at and enabling/preparing along the way. The problem is recursion isn't great for kernel code where we have a limited stack size. Furthermore, we may be calling this code inside clk_set_rate() which also has recursion in it, so we're really not looking good if we encounter a tall clk tree. Let's create a stack instead by looping over the parent chain and collecting clks of interest. Then the enable/prepare becomes as simple as iterating over that list and calling enable. Cc: Jerome Brunet Signed-off-by: Stephen Boyd --- I have some vague fear that this may not work if a clk op is framework reentrant and attemps to call consumer clk APIs from within the clk ops. If the reentrant call tries to add a clk that's already in the list then we'll corrupt the list. Ugh. drivers/clk/clk.c | 91 ++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 59 insertions(+), 32 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index c8d83acda006..e6cb8390c518 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -67,6 +67,8 @@ struct clk_core { struct hlist_head children; struct hlist_node child_node; struct hlist_head clks; + struct list_head prepare_list; + struct list_head enable_list; unsigned int notifier_count; #ifdef CONFIG_DEBUG_FS struct dentry *dentry; @@ -523,33 +525,43 @@ EXPORT_SYMBOL_GPL(clk_unprepare); static int clk_core_prepare(struct clk_core *core) { int ret = 0; + struct clk_core *tmp, *parent; + LIST_HEAD(head); lockdep_assert_held(&prepare_lock); - if (!core) - return 0; + while (core) { + list_add(&core->prepare_list, &head); + /* Stop once we see a clk that is already prepared */ + if (core->prepare_count) + break; + core = core->parent; + } - if (core->prepare_count == 0) { - ret = clk_core_prepare(core->parent); - if (ret) - return ret; + list_for_each_entry_safe(core, tmp, &head, prepare_list) { + list_del_init(&core->prepare_list); - trace_clk_prepare(core); + if (core->prepare_count == 0) { + trace_clk_prepare(core); - if (core->ops->prepare) - ret = core->ops->prepare(core->hw); + if (core->ops->prepare) + ret = core->ops->prepare(core->hw); - trace_clk_prepare_complete(core); + trace_clk_prepare_complete(core); - if (ret) { - clk_core_unprepare(core->parent); - return ret; + if (ret) + goto err; } + core->prepare_count++; } - core->prepare_count++; - return 0; +err: + parent = core->parent; + list_for_each_entry_safe_continue(core, tmp, &head, prepare_list) + list_del_init(&core->prepare_list); + clk_core_unprepare(parent); + return ret; } static int clk_core_prepare_lock(struct clk_core *core) @@ -643,36 +655,49 @@ EXPORT_SYMBOL_GPL(clk_disable); static int clk_core_enable(struct clk_core *core) { int ret = 0; + struct clk_core *tmp, *parent, *prev; + LIST_HEAD(head); lockdep_assert_held(&enable_lock); - if (!core) - return 0; - - if (WARN_ON(core->prepare_count == 0)) - return -ESHUTDOWN; + while (core) { + list_add(&core->enable_list, &head); + /* Stop once we see a clk that is already enabled */ + if (core->enable_count) + break; + core = core->parent; + } - if (core->enable_count == 0) { - ret = clk_core_enable(core->parent); + list_for_each_entry_safe(core, tmp, &head, enable_list) { + list_del_init(&core->enable_list); - if (ret) - return ret; + if (WARN_ON(core->prepare_count == 0)) { + ret = -ESHUTDOWN; + goto err; + } - trace_clk_enable_rcuidle(core); + if (core->enable_count == 0) { + trace_clk_enable_rcuidle(core); - if (core->ops->enable) - ret = core->ops->enable(core->hw); + if (core->ops->enable) + ret = core->ops->enable(core->hw); - trace_clk_enable_complete_rcuidle(core); + trace_clk_enable_complete_rcuidle(core); - if (ret) { - clk_core_disable(core->parent); - return ret; + if (ret) + goto err; } + + core->enable_count++; } - core->enable_count++; return 0; +err: + parent = core->parent; + list_for_each_entry_safe_continue(core, tmp, &head, enable_list) + list_del_init(&core->enable_list); + clk_core_disable(parent); + return ret; } static int clk_core_enable_lock(struct clk_core *core) @@ -2590,6 +2615,8 @@ struct clk *clk_register(struct device *dev, struct clk_hw *hw) core->num_parents = hw->init->num_parents; core->min_rate = 0; core->max_rate = ULONG_MAX; + INIT_LIST_HEAD(&core->prepare_list); + INIT_LIST_HEAD(&core->enable_list); hw->core = core; /* allocate local copy in case parent_names is __initdata */