From patchwork Tue Mar 5 04:49:31 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Derek Basehore X-Patchwork-Id: 10838951 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A67731869 for ; Tue, 5 Mar 2019 04:50:23 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8B3362B78E for ; Tue, 5 Mar 2019 04:50:23 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 889292B77A; Tue, 5 Mar 2019 04:50:23 +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=-5.2 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id C5BEF2B78E for ; Tue, 5 Mar 2019 04:50:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=INK5VcAghK3Gt+hRkCrniikvQp70s6xVpTRmumTcddo=; b=mi4X2IAg8hVxYx OUzMdeM08RmOV+6jGdtP5R89mlmGJzZqrmJqgsIDdu+6GnOJsDUcVoDfQFh8oguHUvxTB+1s+D2W0 fUYbgNXLk+dUl+IFc2kDeyra7ehRRffI/KNYvlM7Tl/gg5TZHoXDz8U9ytM8bFIASl7HKWpuaqTxz AmpR8z3JYOIYhQcVswoQRNQ282qZ+Mz0387tYjvHEDKHXsvWY63fbllUqQbmrZwvmGPgGG38KrLnt XExFBu6NUhU4yfBVQSYiD4ud8TFqo4WfsLRT7oc5r74l17fOR1ghCrIg21GhX+6BC0zu90J5UcZu1 lKct70JlBTWcRxbzNP1A==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1h121y-0001ms-SZ; Tue, 05 Mar 2019 04:50:18 +0000 Received: from mail-pf1-x442.google.com ([2607:f8b0:4864:20::442]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1h121R-0008PK-3w for linux-rockchip@lists.infradead.org; Tue, 05 Mar 2019 04:49:48 +0000 Received: by mail-pf1-x442.google.com with SMTP id n125so4703240pfn.5 for ; Mon, 04 Mar 2019 20:49:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=8X01odutz3vPJQ0+JS1nE7ap7/RwHbCbl03cfhPHZdc=; b=FQn9dTUYle1V+FnEg9iKi095FYrzkrPaVn4b+M9ev4zJ0Os6HwUNy4YLwOZl+awmBO tjTiKkvncv+a3RGxOcvgmbjl+bQVvP5zE4FfPY/+zqY0B0vSOVfMh5W2J2zbYm/QPOuz riSAgIlHqkHuN0uwirpv6juFBb0ftEGi2XkOY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=8X01odutz3vPJQ0+JS1nE7ap7/RwHbCbl03cfhPHZdc=; b=buYlFOkuC0rxtxAaN3HcJnlmDm0JNiQwejFeRGtf/T4dOxPW1A9whXXKAydBv2AOHc xd3LvPlaDIxkR1CH+4oAW/8V3yAT0Oz1nxcoDFbUy4Oh1ShJITVdyjS9UZktvtPbjyOI eHHV2ni+EAozpvkt9cI3cebEzb+dMSQZSV3khwyI3Q1LwsYrwlLDwREMwaLFab6CLZFc xLdANQrECkiZbMJ/vG2Kpyn/HNK+RnjXfVY157EEzKWRI7FpinEeY9pYvq/SGfkNnTaI FMvOZKseXAFcUp0geFYyCQjFWk7Obh+eZVqC1XUi1H2ER7z8pMOx920bGSiHNBup3Ln0 OjJg== X-Gm-Message-State: APjAAAU7LmJGCdJbNLQuohqd7YYWYjrcu/qiV7F30iOiHXgI8gFTIsyV m4IJmonNzG3t4UWZr/rXg77A8Q== X-Google-Smtp-Source: APXvYqx3g9tNsNmso5eebQ6edaVVFjoNjzoEm1iKrF422AlmVyGZoOhDLoNDomTjxVcF4JG8OktGYA== X-Received: by 2002:a17:902:d214:: with SMTP id t20mr7541633ply.268.1551761383742; Mon, 04 Mar 2019 20:49:43 -0800 (PST) Received: from exogeni.mtv.corp.google.com ([2620:15c:202:1:db8d:8e3f:2514:5db8]) by smtp.gmail.com with ESMTPSA id z15sm15893883pgc.25.2019.03.04.20.49.42 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 04 Mar 2019 20:49:43 -0800 (PST) From: Derek Basehore To: linux-kernel@vger.kernel.org Subject: [PATCH v2 1/6] clk: Remove recursion in clk_core_{prepare,enable}() Date: Mon, 4 Mar 2019 20:49:31 -0800 Message-Id: <20190305044936.22267-2-dbasehore@chromium.org> X-Mailer: git-send-email 2.21.0.352.gf09ad66450-goog In-Reply-To: <20190305044936.22267-1-dbasehore@chromium.org> References: <20190305044936.22267-1-dbasehore@chromium.org> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190304_204945_193747_04F80D24 X-CRM114-Status: GOOD ( 22.64 ) X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: aisheng.dong@nxp.com, Derek Basehore , heiko@sntech.de, linux-doc@vger.kernel.org, sboyd@kernel.org, mturquette@baylibre.com, corbet@lwn.net, Stephen Boyd , linux-rockchip@lists.infradead.org, mchehab+samsung@kernel.org, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, jbrunet@baylibre.com Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+patchwork-linux-rockchip=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP From: Stephen Boyd 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. This also unroll the recursion in unprepare,disable which can just be done in the order of walking up the clk tree. 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. Modified verison of https://lore.kernel.org/patchwork/patch/814369/ -Fixed kernel warning -unrolled recursion in unprepare/disable too Cc: Jerome Brunet Signed-off-by: Stephen Boyd Signed-off-by: Derek Basehore --- drivers/clk/clk.c | 191 ++++++++++++++++++++++++++-------------------- 1 file changed, 107 insertions(+), 84 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index d2477a5058ac..94b3ac783d90 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -68,6 +68,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; @@ -677,34 +679,34 @@ static void clk_core_unprepare(struct clk_core *core) { lockdep_assert_held(&prepare_lock); - if (!core) - return; - - if (WARN(core->prepare_count == 0, - "%s already unprepared\n", core->name)) - return; - - if (WARN(core->prepare_count == 1 && core->flags & CLK_IS_CRITICAL, - "Unpreparing critical %s\n", core->name)) - return; + while (core) { + if (WARN(core->prepare_count == 0, + "%s already unprepared\n", core->name)) + return; - if (core->flags & CLK_SET_RATE_GATE) - clk_core_rate_unprotect(core); + if (WARN(core->prepare_count == 1 && + core->flags & CLK_IS_CRITICAL, + "Unpreparing critical %s\n", core->name)) + return; - if (--core->prepare_count > 0) - return; + if (core->flags & CLK_SET_RATE_GATE) + clk_core_rate_unprotect(core); - WARN(core->enable_count > 0, "Unpreparing enabled %s\n", core->name); + if (--core->prepare_count > 0) + return; - trace_clk_unprepare(core); + WARN(core->enable_count > 0, "Unpreparing enabled %s\n", + core->name); + trace_clk_unprepare(core); - if (core->ops->unprepare) - core->ops->unprepare(core->hw); + if (core->ops->unprepare) + core->ops->unprepare(core->hw); - clk_pm_runtime_put(core); + clk_pm_runtime_put(core); - trace_clk_unprepare_complete(core); - clk_core_unprepare(core->parent); + trace_clk_unprepare_complete(core); + core = core->parent; + } } static void clk_core_unprepare_lock(struct clk_core *core) @@ -737,49 +739,57 @@ EXPORT_SYMBOL_GPL(clk_unprepare); static int clk_core_prepare(struct clk_core *core) { int ret = 0; + 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. Adding a clk + * to the list with a non-zero prepare count (or reaching NULL) + * makes error handling work as implemented. + */ + if (core->prepare_count) + break; + core = core->parent; + } - if (core->prepare_count == 0) { - ret = clk_pm_runtime_get(core); - if (ret) - return ret; + /* First entry has either a prepare_count of 0 or a NULL parent. */ + list_for_each_entry(core, &head, prepare_list) { + if (core->prepare_count == 0) { + ret = clk_pm_runtime_get(core); + if (ret) + goto unprepare_parent; - ret = clk_core_prepare(core->parent); - if (ret) - goto runtime_put; + trace_clk_prepare(core); - 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) + goto runtime_put; + } + core->prepare_count++; - if (ret) - goto unprepare; + /* + * CLK_SET_RATE_GATE is a special case of clock protection + * Instead of a consumer claiming exclusive rate control, it is + * actually the provider which prevents any consumer from making + * any operation which could result in a rate change or rate + * glitch while the clock is prepared. + */ + if (core->flags & CLK_SET_RATE_GATE) + clk_core_rate_protect(core); } - core->prepare_count++; - - /* - * CLK_SET_RATE_GATE is a special case of clock protection - * Instead of a consumer claiming exclusive rate control, it is - * actually the provider which prevents any consumer from making any - * operation which could result in a rate change or rate glitch while - * the clock is prepared. - */ - if (core->flags & CLK_SET_RATE_GATE) - clk_core_rate_protect(core); - return 0; -unprepare: - clk_core_unprepare(core->parent); runtime_put: clk_pm_runtime_put(core); +unprepare_parent: + clk_core_unprepare(core->parent); return ret; } @@ -819,27 +829,27 @@ static void clk_core_disable(struct clk_core *core) { lockdep_assert_held(&enable_lock); - if (!core) - return; - - if (WARN(core->enable_count == 0, "%s already disabled\n", core->name)) - return; - - if (WARN(core->enable_count == 1 && core->flags & CLK_IS_CRITICAL, - "Disabling critical %s\n", core->name)) - return; + while (core) { + if (WARN(core->enable_count == 0, "%s already disabled\n", + core->name)) + return; - if (--core->enable_count > 0) - return; + if (--core->enable_count > 0) + return; - trace_clk_disable_rcuidle(core); + if (WARN(core->enable_count == 1 && + core->flags & CLK_IS_CRITICAL, + "Disabling critical %s\n", core->name)) + return; - if (core->ops->disable) - core->ops->disable(core->hw); + trace_clk_disable_rcuidle(core); - trace_clk_disable_complete_rcuidle(core); + if (core->ops->disable) + core->ops->disable(core->hw); - clk_core_disable(core->parent); + trace_clk_disable_complete_rcuidle(core); + core = core->parent; + } } static void clk_core_disable_lock(struct clk_core *core) @@ -875,37 +885,48 @@ EXPORT_SYMBOL_GPL(clk_disable); static int clk_core_enable(struct clk_core *core) { int ret = 0; + LIST_HEAD(head); lockdep_assert_held(&enable_lock); - if (!core) - return 0; + while (core) { + if (WARN(core->prepare_count == 0, + "Enabling unprepared %s\n", core->name)) + return -ESHUTDOWN; - if (WARN(core->prepare_count == 0, - "Enabling unprepared %s\n", core->name)) - return -ESHUTDOWN; - - if (core->enable_count == 0) { - ret = clk_core_enable(core->parent); + list_add(&core->enable_list, &head); + /* + * Stop once we see a clk that is already enabled. Adding a clk + * to the list with a non-zero prepare count (or reaching NULL) + * makes error handling work as implemented. + */ + if (core->enable_count) + break; - if (ret) - return ret; + core = core->parent; + } - trace_clk_enable_rcuidle(core); + /* First entry has either an enable_count of 0 or a NULL parent. */ + list_for_each_entry(core, &head, enable_list) { + 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: + clk_core_disable(core->parent); + return ret; } static int clk_core_enable_lock(struct clk_core *core) @@ -3288,6 +3309,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 */