From patchwork Wed Dec 7 04:42:03 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 86957 Delivered-To: patch@linaro.org Received: by 10.140.20.101 with SMTP id 92csp136832qgi; Tue, 6 Dec 2016 20:42:37 -0800 (PST) X-Received: by 10.98.159.67 with SMTP id g64mr65831251pfe.93.1481085757221; Tue, 06 Dec 2016 20:42:37 -0800 (PST) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id h67si22465114pfe.48.2016.12.06.20.42.37; Tue, 06 Dec 2016 20:42:37 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-pm-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org; spf=pass (google.com: best guess record for domain of linux-pm-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-pm-owner@vger.kernel.org; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751987AbcLGEmg (ORCPT + 13 others); Tue, 6 Dec 2016 23:42:36 -0500 Received: from mail-pf0-f180.google.com ([209.85.192.180]:34473 "EHLO mail-pf0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751618AbcLGEmf (ORCPT ); Tue, 6 Dec 2016 23:42:35 -0500 Received: by mail-pf0-f180.google.com with SMTP id c4so74722311pfb.1 for ; Tue, 06 Dec 2016 20:42:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :in-reply-to:references; bh=3PvhA+lWa9Gh1y5mJr77mSz14ZPGxEaRfUEXRhUJd5o=; b=jhuc7NO67CqMbm4uLzrEZgoohurti+WuYgP7dWeF9I9A7dCqgptYgXUP9Z3kNfAUBk SiEm9cgd9N1HimTkLdIXqQhOTQS1tPF0pwRLTLMPkbSoLDnwrxiHLKPKMFLIGqYS/7zZ Rbb6G4swSykG5dSKEjMK+bGYNN/4sQ02oUU7c= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:in-reply-to:references; bh=3PvhA+lWa9Gh1y5mJr77mSz14ZPGxEaRfUEXRhUJd5o=; b=fR1iD+n5inbdAPqMuwCNDwMY/U1FIv3j5orGGolfV4vM4IzGX1LUoiPBlYYGPZ3wWq 7T/KJoyjGwsAOHNiX2f/mYllkHg67C9w71dY6bygBMQJ+uKgr6yKH0HtLX+FFZ7mllOt DczRmIKYANVLqWvBaCcpRu0golLzPr9urdHJsNci5QQl/GztenmL6Z1+pCZIzDVhrlWq f5L3B6Qfqg5XEpCtumLhv8E1UD+JYqmI8EWChRiKoyWt5idU3LFRBAyr5fGs2jdjqKoM FVJSPwUatO9SCT5U92G2PQqLSX8CtzoWYYS77rw7n3bh5BJKH56upylgaeNKbJnf1Kk9 o/Zg== X-Gm-Message-State: AKaTC00bntgURucmOU0CWnFu3t4zF6WGZgk+Jt4jTX0a5U3OVkjF1pRLhEdT9wkDbMz/qx0a X-Received: by 10.84.213.130 with SMTP id g2mr142460371pli.43.1481085755041; Tue, 06 Dec 2016 20:42:35 -0800 (PST) Received: from localhost ([122.172.43.83]) by smtp.gmail.com with ESMTPSA id k25sm38210609pfg.84.2016.12.06.20.42.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 06 Dec 2016 20:42:34 -0800 (PST) From: Viresh Kumar To: Rafael Wysocki , Viresh Kumar , Nishanth Menon , Stephen Boyd Cc: linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org, Vincent Guittot , Viresh Kumar Subject: [PATCH V2 07/10] PM / OPP: Don't allocate OPP table from _opp_allocate() Date: Wed, 7 Dec 2016 10:12:03 +0530 Message-Id: X-Mailer: git-send-email 2.7.1.410.g6faf27b In-Reply-To: References: In-Reply-To: References: Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org There is no point in trying to find/allocate the table for every OPP that is added for a device. It would be far more efficient to allocate the table only once and pass its pointer to the routines that add the OPP entry. Locking is removed from _opp_add_static_v2() and _opp_add_v1() now as the callers call them with that lock already held. Call to _remove_opp_table() routine is also removed from _opp_free() now, as opp_table isn't allocated from within _opp_allocate(). This is handled by the routines which created the OPP table in the first place. Signed-off-by: Viresh Kumar --- drivers/base/power/opp/core.c | 66 +++++++++++++++++++------------------ drivers/base/power/opp/of.c | 75 ++++++++++++++++++++++--------------------- drivers/base/power/opp/opp.h | 9 ++++-- 3 files changed, 78 insertions(+), 72 deletions(-) -- 2.7.1.410.g6faf27b -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index 11496fbb2ee2..6f61809c5760 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -842,7 +842,7 @@ struct opp_device *_add_opp_dev(const struct device *dev, * * Return: valid opp_table pointer if success, else NULL. */ -static struct opp_table *_add_opp_table(struct device *dev) +struct opp_table *_add_opp_table(struct device *dev) { struct opp_table *opp_table; struct opp_device *opp_dev; @@ -942,10 +942,9 @@ static void _remove_opp_table(struct opp_table *opp_table) _kfree_device_rcu); } -void _opp_free(struct dev_pm_opp *opp, struct opp_table *opp_table) +void _opp_free(struct dev_pm_opp *opp) { kfree(opp); - _remove_opp_table(opp_table); } /** @@ -1030,15 +1029,10 @@ void dev_pm_opp_remove(struct device *dev, unsigned long freq) EXPORT_SYMBOL_GPL(dev_pm_opp_remove); struct dev_pm_opp *_opp_allocate(struct device *dev, - struct opp_table **opp_table) + struct opp_table *table) { struct dev_pm_opp *opp; int count, supply_size; - struct opp_table *table; - - table = _add_opp_table(dev); - if (!table) - return NULL; /* Allocate space for at least one supply */ count = table->regulator_count ? table->regulator_count : 1; @@ -1046,17 +1040,13 @@ struct dev_pm_opp *_opp_allocate(struct device *dev, /* allocate new OPP node and supplies structures */ opp = kzalloc(sizeof(*opp) + supply_size, GFP_KERNEL); - if (!opp) { - kfree(table); + if (!opp) return NULL; - } /* Put the supplies at the end of the OPP structure as an empty array */ opp->supplies = (struct dev_pm_opp_supply *)(opp + 1); INIT_LIST_HEAD(&opp->node); - *opp_table = table; - return opp; } @@ -1146,6 +1136,7 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, /** * _opp_add_v1() - Allocate a OPP based on v1 bindings. + * @opp_table: OPP table * @dev: device for which we do this operation * @freq: Frequency in Hz for this OPP * @u_volt: Voltage in uVolts for this OPP @@ -1171,22 +1162,18 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, * Duplicate OPPs (both freq and volt are same) and !opp->available * -ENOMEM Memory allocation failure */ -int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt, - bool dynamic) +int _opp_add_v1(struct opp_table *opp_table, struct device *dev, + unsigned long freq, long u_volt, bool dynamic) { - struct opp_table *opp_table; struct dev_pm_opp *new_opp; unsigned long tol; int ret; - /* Hold our table modification lock here */ - mutex_lock(&opp_table_lock); + opp_rcu_lockdep_assert(); - new_opp = _opp_allocate(dev, &opp_table); - if (!new_opp) { - ret = -ENOMEM; - goto unlock; - } + new_opp = _opp_allocate(dev, opp_table); + if (!new_opp) + return -ENOMEM; /* populate the opp table */ new_opp->rate = freq; @@ -1205,8 +1192,6 @@ int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt, goto free_opp; } - mutex_unlock(&opp_table_lock); - /* * Notify the changes in the availability of the operable * frequency/voltage list. @@ -1215,9 +1200,8 @@ int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt, return 0; free_opp: - _opp_free(new_opp, opp_table); -unlock: - mutex_unlock(&opp_table_lock); + _opp_free(new_opp); + return ret; } @@ -1735,7 +1719,25 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_register_put_opp_helper); */ int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt) { - return _opp_add_v1(dev, freq, u_volt, true); + struct opp_table *opp_table; + int ret; + + /* Hold our table modification lock here */ + mutex_lock(&opp_table_lock); + + opp_table = _add_opp_table(dev); + if (!opp_table) { + ret = -ENOMEM; + goto unlock; + } + + ret = _opp_add_v1(opp_table, dev, freq, u_volt, true); + if (ret) + _remove_opp_table(opp_table); + +unlock: + mutex_unlock(&opp_table_lock); + return ret; } EXPORT_SYMBOL_GPL(dev_pm_opp_add); @@ -1901,8 +1903,8 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_get_notifier); * Free OPPs either created using static entries present in DT or even the * dynamically added entries based on remove_all param. */ -static void _dev_pm_opp_remove_table(struct opp_table *opp_table, - struct device *dev, bool remove_all) +void _dev_pm_opp_remove_table(struct opp_table *opp_table, struct device *dev, + bool remove_all) { struct dev_pm_opp *opp, *tmp; diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c index 442fa46c4f5c..38efc14d829c 100644 --- a/drivers/base/power/opp/of.c +++ b/drivers/base/power/opp/of.c @@ -255,6 +255,7 @@ static struct device_node *_of_get_opp_desc_node(struct device *dev) /** * _opp_add_static_v2() - Allocate static OPPs (As per 'v2' DT bindings) + * @opp_table: OPP table * @dev: device for which we do this operation * @np: device node * @@ -276,22 +277,17 @@ static struct device_node *_of_get_opp_desc_node(struct device *dev) * -ENOMEM Memory allocation failure * -EINVAL Failed parsing the OPP node */ -static int _opp_add_static_v2(struct device *dev, struct device_node *np) +static int _opp_add_static_v2(struct opp_table *opp_table, struct device *dev, + struct device_node *np) { - struct opp_table *opp_table; struct dev_pm_opp *new_opp; u64 rate; u32 val; int ret; - /* Hold our table modification lock here */ - mutex_lock(&opp_table_lock); - - new_opp = _opp_allocate(dev, &opp_table); - if (!new_opp) { - ret = -ENOMEM; - goto unlock; - } + new_opp = _opp_allocate(dev, opp_table); + if (!new_opp) + return -ENOMEM; ret = of_property_read_u64(np, "opp-hz", &rate); if (ret < 0) { @@ -347,8 +343,6 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np) if (new_opp->clock_latency_ns > opp_table->clock_latency_ns_max) opp_table->clock_latency_ns_max = new_opp->clock_latency_ns; - mutex_unlock(&opp_table_lock); - pr_debug("%s: turbo:%d rate:%lu uv:%lu uvmin:%lu uvmax:%lu latency:%lu\n", __func__, new_opp->turbo, new_opp->rate, new_opp->supplies[0].u_volt, new_opp->supplies[0].u_volt_min, @@ -362,9 +356,8 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np) return 0; free_opp: - _opp_free(new_opp, opp_table); -unlock: - mutex_unlock(&opp_table_lock); + _opp_free(new_opp); + return ret; } @@ -382,16 +375,20 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np) /* OPPs are already managed */ if (!_add_opp_dev(dev, opp_table)) ret = -ENOMEM; - mutex_unlock(&opp_table_lock); - return ret; + goto unlock; + } + + opp_table = _add_opp_table(dev); + if (!opp_table) { + ret = -ENOMEM; + goto unlock; } - mutex_unlock(&opp_table_lock); /* We have opp-table node now, iterate over it and add OPPs */ for_each_available_child_of_node(opp_np, np) { count++; - ret = _opp_add_static_v2(dev, np); + ret = _opp_add_static_v2(opp_table, dev, np); if (ret) { dev_err(dev, "%s: Failed to add OPP, %d\n", __func__, ret); @@ -400,15 +397,8 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np) } /* There should be one of more OPP defined */ - if (WARN_ON(!count)) - return -ENOENT; - - mutex_lock(&opp_table_lock); - - opp_table = _find_opp_table(dev); - if (WARN_ON(IS_ERR(opp_table))) { - ret = PTR_ERR(opp_table); - mutex_unlock(&opp_table_lock); + if (WARN_ON(!count)) { + ret = -ENOENT; goto free_table; } @@ -418,12 +408,12 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np) else opp_table->shared_opp = OPP_TABLE_ACCESS_EXCLUSIVE; - mutex_unlock(&opp_table_lock); - - return 0; + goto unlock; free_table: - dev_pm_opp_of_remove_table(dev); + _dev_pm_opp_remove_table(opp_table, dev, false); +unlock: + mutex_unlock(&opp_table_lock); return ret; } @@ -431,9 +421,10 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np) /* Initializes OPP tables based on old-deprecated bindings */ static int _of_add_opp_table_v1(struct device *dev) { + struct opp_table *opp_table; const struct property *prop; const __be32 *val; - int nr, ret; + int nr, ret = 0; prop = of_find_property(dev->of_node, "operating-points", NULL); if (!prop) @@ -451,22 +442,32 @@ static int _of_add_opp_table_v1(struct device *dev) return -EINVAL; } + mutex_lock(&opp_table_lock); + + opp_table = _add_opp_table(dev); + if (!opp_table) { + ret = -ENOMEM; + goto unlock; + } + val = prop->value; while (nr) { unsigned long freq = be32_to_cpup(val++) * 1000; unsigned long volt = be32_to_cpup(val++); - ret = _opp_add_v1(dev, freq, volt, false); + ret = _opp_add_v1(opp_table, dev, freq, volt, false); if (ret) { dev_err(dev, "%s: Failed to add OPP %ld (%d)\n", __func__, freq, ret); - dev_pm_opp_of_remove_table(dev); - return ret; + _dev_pm_opp_remove_table(opp_table, dev, false); + break; } nr -= 2; } - return 0; +unlock: + mutex_unlock(&opp_table_lock); + return ret; } /** diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h index c4b539a8533a..e32dc80ddc12 100644 --- a/drivers/base/power/opp/opp.h +++ b/drivers/base/power/opp/opp.h @@ -191,13 +191,16 @@ struct opp_table { /* Routines internal to opp core */ struct opp_table *_find_opp_table(struct device *dev); +struct opp_table *_add_opp_table(struct device *dev); struct opp_device *_add_opp_dev(const struct device *dev, struct opp_table *opp_table); +void _dev_pm_opp_remove_table(struct opp_table *opp_table, struct device *dev, bool remove_all); void _dev_pm_opp_find_and_remove_table(struct device *dev, bool remove_all); -struct dev_pm_opp *_opp_allocate(struct device *dev, struct opp_table **opp_table); -void _opp_free(struct dev_pm_opp *opp, struct opp_table *opp_table); +struct dev_pm_opp *_opp_allocate(struct device *dev, struct opp_table *opp_table); +void _opp_free(struct dev_pm_opp *opp); int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, struct opp_table *opp_table); -int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt, bool dynamic); +int _opp_add_v1(struct opp_table *opp_table, struct device *dev, unsigned long freq, long u_volt, bool dynamic); void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, bool of); +struct opp_table *_add_opp_table(struct device *dev); #ifdef CONFIG_OF void _of_init_opp_table(struct opp_table *opp_table, struct device *dev);