[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPDyKFrTJbTrSMW30wN5Kbk4=yDAMF37HR2+9MKybkyDW0f8hQ@mail.gmail.com>
Date: Fri, 6 Nov 2020 11:25:27 +0100
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Viresh Kumar <viresh.kumar@...aro.org>
Cc: "Rafael J. Wysocki" <rjw@...ysocki.net>,
Kevin Hilman <khilman@...nel.org>,
Len Brown <len.brown@...el.com>, Pavel Machek <pavel@....cz>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Viresh Kumar <vireshk@...nel.org>, Nishanth Menon <nm@...com>,
Stephen Boyd <sboyd@...nel.org>,
Linux PM <linux-pm@...r.kernel.org>,
Vincent Guittot <vincent.guittot@...aro.org>,
Dmitry Osipenko <digetx@...il.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] opp: Don't create an OPP table from dev_pm_opp_get_opp_table()
On Fri, 6 Nov 2020 at 07:25, Viresh Kumar <viresh.kumar@...aro.org> wrote:
>
> It has been found that some users (like cpufreq-dt and others on LKML)
> have abused the helper dev_pm_opp_get_opp_table() to create the OPP
> table instead of just finding it, which is the wrong thing to do. This
> routine was meant for OPP core's internal working and exposed the whole
> functionality by mistake.
>
> Change the scope of dev_pm_opp_get_opp_table() to only finding the
> table. The internal helpers _opp_get_opp_table*() are thus renamed to
> _add_opp_table*(), dev_pm_opp_get_opp_table_indexed() is removed (as we
> don't need the index field for finding the OPP table) and so the only
> user, genpd, is updated.
>
> Note that the prototype of _add_opp_table() was already left in opp.h by
> mistake when it was removed earlier and so we weren't required to add it
> now.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@...aro.org>
Acked-by: Ulf Hansson <ulf.hansson@...aro.org>
Kind regards
Uffe
> ---
> drivers/base/power/domain.c | 2 +-
> drivers/opp/core.c | 27 +++++++++++++--------------
> drivers/opp/of.c | 4 ++--
> drivers/opp/opp.h | 1 +
> include/linux/pm_opp.h | 1 -
> 5 files changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 743268996336..92b750b865d5 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2249,7 +2249,7 @@ int of_genpd_add_provider_onecell(struct device_node *np,
> * Save table for faster processing while setting
> * performance state.
> */
> - genpd->opp_table = dev_pm_opp_get_opp_table_indexed(&genpd->dev, i);
> + genpd->opp_table = dev_pm_opp_get_opp_table(&genpd->dev);
> WARN_ON(IS_ERR(genpd->opp_table));
> }
>
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 9915e8487f0b..b24f685823ae 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -1138,7 +1138,7 @@ void _get_opp_table_kref(struct opp_table *opp_table)
> * uses the opp_tables_busy flag to indicate if another creator is in the middle
> * of adding an OPP table and others should wait for it to finish.
> */
> -static struct opp_table *_opp_get_opp_table(struct device *dev, int index)
> +struct opp_table *_add_opp_table_indexed(struct device *dev, int index)
> {
> struct opp_table *opp_table;
>
> @@ -1188,17 +1188,16 @@ static struct opp_table *_opp_get_opp_table(struct device *dev, int index)
> return opp_table;
> }
>
> -struct opp_table *dev_pm_opp_get_opp_table(struct device *dev)
> +struct opp_table *_add_opp_table(struct device *dev)
> {
> - return _opp_get_opp_table(dev, 0);
> + return _add_opp_table_indexed(dev, 0);
> }
> -EXPORT_SYMBOL_GPL(dev_pm_opp_get_opp_table);
>
> -struct opp_table *dev_pm_opp_get_opp_table_indexed(struct device *dev,
> - int index)
> +struct opp_table *dev_pm_opp_get_opp_table(struct device *dev)
> {
> - return _opp_get_opp_table(dev, index);
> + return _find_opp_table(dev);
> }
> +EXPORT_SYMBOL_GPL(dev_pm_opp_get_opp_table);
>
> static void _opp_table_kref_release(struct kref *kref)
> {
> @@ -1627,7 +1626,7 @@ struct opp_table *dev_pm_opp_set_supported_hw(struct device *dev,
> {
> struct opp_table *opp_table;
>
> - opp_table = dev_pm_opp_get_opp_table(dev);
> + opp_table = _add_opp_table(dev);
> if (IS_ERR(opp_table))
> return opp_table;
>
> @@ -1686,7 +1685,7 @@ struct opp_table *dev_pm_opp_set_prop_name(struct device *dev, const char *name)
> {
> struct opp_table *opp_table;
>
> - opp_table = dev_pm_opp_get_opp_table(dev);
> + opp_table = _add_opp_table(dev);
> if (IS_ERR(opp_table))
> return opp_table;
>
> @@ -1779,7 +1778,7 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev,
> struct regulator *reg;
> int ret, i;
>
> - opp_table = dev_pm_opp_get_opp_table(dev);
> + opp_table = _add_opp_table(dev);
> if (IS_ERR(opp_table))
> return opp_table;
>
> @@ -1887,7 +1886,7 @@ struct opp_table *dev_pm_opp_set_clkname(struct device *dev, const char *name)
> struct opp_table *opp_table;
> int ret;
>
> - opp_table = dev_pm_opp_get_opp_table(dev);
> + opp_table = _add_opp_table(dev);
> if (IS_ERR(opp_table))
> return opp_table;
>
> @@ -1955,7 +1954,7 @@ struct opp_table *dev_pm_opp_register_set_opp_helper(struct device *dev,
> if (!set_opp)
> return ERR_PTR(-EINVAL);
>
> - opp_table = dev_pm_opp_get_opp_table(dev);
> + opp_table = _add_opp_table(dev);
> if (IS_ERR(opp_table))
> return opp_table;
>
> @@ -2039,7 +2038,7 @@ struct opp_table *dev_pm_opp_attach_genpd(struct device *dev,
> int index = 0, ret = -EINVAL;
> const char **name = names;
>
> - opp_table = dev_pm_opp_get_opp_table(dev);
> + opp_table = _add_opp_table(dev);
> if (IS_ERR(opp_table))
> return opp_table;
>
> @@ -2204,7 +2203,7 @@ int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
> struct opp_table *opp_table;
> int ret;
>
> - opp_table = dev_pm_opp_get_opp_table(dev);
> + opp_table = _add_opp_table(dev);
> if (IS_ERR(opp_table))
> return PTR_ERR(opp_table);
>
> diff --git a/drivers/opp/of.c b/drivers/opp/of.c
> index 9faeb83e4b32..c718092757d9 100644
> --- a/drivers/opp/of.c
> +++ b/drivers/opp/of.c
> @@ -974,7 +974,7 @@ int dev_pm_opp_of_add_table(struct device *dev)
> struct opp_table *opp_table;
> int ret;
>
> - opp_table = dev_pm_opp_get_opp_table_indexed(dev, 0);
> + opp_table = _add_opp_table_indexed(dev, 0);
> if (IS_ERR(opp_table))
> return PTR_ERR(opp_table);
>
> @@ -1029,7 +1029,7 @@ int dev_pm_opp_of_add_table_indexed(struct device *dev, int index)
> index = 0;
> }
>
> - opp_table = dev_pm_opp_get_opp_table_indexed(dev, index);
> + opp_table = _add_opp_table_indexed(dev, index);
> if (IS_ERR(opp_table))
> return PTR_ERR(opp_table);
>
> diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
> index ebd930e0b3ca..4ced7ffa8158 100644
> --- a/drivers/opp/opp.h
> +++ b/drivers/opp/opp.h
> @@ -224,6 +224,7 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, struct opp_table *o
> 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, int last_cpu);
> struct opp_table *_add_opp_table(struct device *dev);
> +struct opp_table *_add_opp_table_indexed(struct device *dev, int index);
> void _put_opp_list_kref(struct opp_table *opp_table);
>
> #ifdef CONFIG_OF
> diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
> index dbb484524f82..1435c054016a 100644
> --- a/include/linux/pm_opp.h
> +++ b/include/linux/pm_opp.h
> @@ -90,7 +90,6 @@ struct dev_pm_set_opp_data {
> #if defined(CONFIG_PM_OPP)
>
> struct opp_table *dev_pm_opp_get_opp_table(struct device *dev);
> -struct opp_table *dev_pm_opp_get_opp_table_indexed(struct device *dev, int index);
> void dev_pm_opp_put_opp_table(struct opp_table *opp_table);
>
> unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp);
> --
> 2.25.0.rc1.19.g042ed3e048af
>
Powered by blists - more mailing lists