| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAPDyKFontDwvpS97xBskNE-FLMo3HPDLthCEkviZv8rKdA8t=g@mail.gmail.com>
Date: Thu, 22 Mar 2018 10:31:01 +0100
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Viresh Kumar <viresh.kumar@...aro.org>
Cc: Kevin Hilman <khilman@...nel.org>,
"Rafael J. Wysocki" <rjw@...ysocki.net>,
Len Brown <len.brown@...el.com>, Pavel Machek <pavel@....cz>,
Linux PM <linux-pm@...r.kernel.org>,
Stephen Boyd <sboyd@...eaurora.org>,
Nishanth Menon <nm@...com>,
Vincent Guittot <vincent.guittot@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Rajendra Nayak <rnayak@...eaurora.org>,
Sudeep Holla <sudeep.holla@....com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/7] PM / Domain: Add support to parse domain's OPP table
On 22 December 2017 at 08:26, Viresh Kumar <viresh.kumar@...aro.org> wrote:
> Parse the OPP table for power domains if they have their
> set_performance_state() callback set.
Nitpick:
This is a bit too short, please elaborate a bit on the why and what
this change does.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@...aro.org>
> ---
> drivers/base/power/domain.c | 78 +++++++++++++++++++++++++++++++++++----------
> 1 file changed, 62 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 013c1e206167..1ad4ad0b0de0 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -10,6 +10,7 @@
> #include <linux/kernel.h>
> #include <linux/io.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_opp.h>
> #include <linux/pm_runtime.h>
> #include <linux/pm_domain.h>
> #include <linux/pm_qos.h>
> @@ -1900,15 +1901,33 @@ int of_genpd_add_provider_simple(struct device_node *np,
>
> mutex_lock(&gpd_list_lock);
>
> - if (genpd_present(genpd)) {
> - ret = genpd_add_provider(np, genpd_xlate_simple, genpd);
> - if (!ret) {
> - genpd->provider = &np->fwnode;
> - genpd->has_provider = true;
> - genpd->dev.of_node = np;
> + if (!genpd_present(genpd))
> + goto unlock;
> +
> + genpd->dev.of_node = np;
> +
> + /* Parse genpd OPP table */
> + if (genpd->set_performance_state) {
> + ret = dev_pm_opp_of_add_table(&genpd->dev);
> + if (ret) {
> + dev_err(&genpd->dev, "Failed to add OPP table: %d\n",
> + ret);
> + goto unlock;
> }
> }
>
> + ret = genpd_add_provider(np, genpd_xlate_simple, genpd);
> + if (ret) {
> + if (genpd->set_performance_state)
> + dev_pm_opp_of_remove_table(&genpd->dev);
> +
> + goto unlock;
> + }
> +
> + genpd->provider = &np->fwnode;
> + genpd->has_provider = true;
> +
> +unlock:
> mutex_unlock(&gpd_list_lock);
>
> return ret;
> @@ -1923,6 +1942,7 @@ EXPORT_SYMBOL_GPL(of_genpd_add_provider_simple);
> int of_genpd_add_provider_onecell(struct device_node *np,
> struct genpd_onecell_data *data)
> {
> + struct generic_pm_domain *genpd;
> unsigned int i;
> int ret = -EINVAL;
>
> @@ -1935,14 +1955,27 @@ int of_genpd_add_provider_onecell(struct device_node *np,
> data->xlate = genpd_xlate_onecell;
>
> for (i = 0; i < data->num_domains; i++) {
> - if (!data->domains[i])
> + genpd = data->domains[i];
> +
> + if (!genpd)
> continue;
> - if (!genpd_present(data->domains[i]))
> + if (!genpd_present(genpd))
> goto error;
>
> - data->domains[i]->provider = &np->fwnode;
> - data->domains[i]->has_provider = true;
> - data->domains[i]->dev.of_node = np;
> + genpd->dev.of_node = np;
> +
> + /* Parse genpd OPP table */
> + if (genpd->set_performance_state) {
> + ret = dev_pm_opp_of_add_table_indexed(&genpd->dev, i);
> + if (ret) {
> + dev_err(&genpd->dev, "Failed to add OPP table for index %d: %d\n",
> + i, ret);
> + goto error;
> + }
> + }
> +
> + genpd->provider = &np->fwnode;
> + genpd->has_provider = true;
> }
>
> ret = genpd_add_provider(np, data->xlate, data);
> @@ -1955,10 +1988,16 @@ int of_genpd_add_provider_onecell(struct device_node *np,
>
> error:
> while (i--) {
> - if (!data->domains[i])
> + genpd = data->domains[i];
> +
> + if (!genpd)
> continue;
> - data->domains[i]->provider = NULL;
> - data->domains[i]->has_provider = false;
> +
> + genpd->provider = NULL;
> + genpd->has_provider = false;
> +
> + if (genpd->set_performance_state)
> + dev_pm_opp_of_remove_table(&genpd->dev);
> }
>
> mutex_unlock(&gpd_list_lock);
> @@ -1985,10 +2024,17 @@ void of_genpd_del_provider(struct device_node *np)
> * provider, set the 'has_provider' to false
> * so that the PM domain can be safely removed.
> */
> - list_for_each_entry(gpd, &gpd_list, gpd_list_node)
> - if (gpd->provider == &np->fwnode)
> + list_for_each_entry(gpd, &gpd_list, gpd_list_node) {
> + if (gpd->provider == &np->fwnode) {
> gpd->has_provider = false;
>
> + if (!gpd->set_performance_state)
> + continue;
Nitpick, perhaps change this to:
if (gpd->set_performance_state)
dev_pm_opp_of_remove_table(&gpd->dev);
> +
> + dev_pm_opp_of_remove_table(&gpd->dev);
> + }
> + }
> +
> list_del(&cp->link);
> of_node_put(cp->node);
> kfree(cp);
> --
> 2.15.0.194.g9af6a3dea062
>
When fixed the nitpicks, feel free to add:
Acked-by: Ulf Hansson <ulf.hansson@...aro.org>
Kind regards
Uffe
Powered by blists - more mailing lists