Hello,
We have too many configuration specific APIs currently, six of them already,
like dev_pm_opp_set_regulators(). This makes it complex/messy for both the OPP
core and its users to manage. There is also code redundancy in these APIs, in
the way they add/manage the OPP table specific stuff.
This patch series is an attempt to simplify these interfaces by adding a new
interface, dev_pm_opp_set_config(), which is now used by all the existing ones.
This series also migrates few users to the new API, where multiple
configurations were required and rest are updated for API interface changes.
This is pushed here:
git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git opp/linux-next
This was earlier tested by various folks, I have tested it again on hikey board,
it will get further tested on linux-next in the coming days. Build test is
already done by Linaro's bot for enough platform though.
The entire patchset shall get merged via the OPP tree in 5.20-rc1, please do not
merge individual patches.
V2->V3:
- Merged two patchsets:
[PATCH V2 00/30] OPP: Add new configuration interface: dev_pm_opp_set_config()
[PATCH V2 0/5] OPP: Replace custom set_opp() with config_regulators()
- The existing APIs aren't removed anymore, but are made to use the new core API
to set various configurations (Greg KH).
- clk-names and regulator-names are NULL terminated arrays now (Greg KH).
- New interface added: dev_pm_opp_set_config_regulators().
V1->V2:
- dev_pm_opp_set_config() doesn't return the OPP table anymore, but a token
allocated with xa_alloc(). The same needs to be passed to clear-config API.
- Updated all users according to that as well.
- The clk_names interface is updated to allow multiple clocks.
- Converted few // comments to /* */.
- Added tags by few people.
- Dropped the last patch to rearrange stuff, not required anymore.
Thanks.
--
Viresh
Viresh Kumar (20):
OPP: Track if clock name is configured by platform
OPP: Make dev_pm_opp_set_regulators() accept NULL terminated list
OPP: Add dev_pm_opp_set_config() and friends
cpufreq: qcom-nvmem: Migrate to dev_pm_opp_set_config()
cpufreq: sti: Migrate to dev_pm_opp_set_config()
cpufreq: ti: Migrate to dev_pm_opp_set_config()
drm/lima: Migrate to dev_pm_opp_set_config()
soc/tegra: Add comment over devm_pm_opp_set_clkname()
soc/tegra: Migrate to dev_pm_opp_set_config()
OPP: Migrate set-regulators API to use set-config helpers
OPP: Migrate set-supported-hw API to use set-config helpers
OPP: Migrate set-clk-name API to use set-config helpers
OPP: Migrate set-opp-helper API to use set-config helpers
OPP: Migrate attach-genpd API to use set-config helpers
OPP: Migrate set-prop-name helper API to use set-config helpers
OPP: Add support for config_regulators() helper
OPP: Make _generic_set_opp_regulator() a config_regulators() interface
OPP: Add dev_pm_opp_get_supplies()
OPP: ti: Migrate to dev_pm_opp_set_config_regulators()
OPP: Remove custom OPP helper support
drivers/cpufreq/cpufreq-dt.c | 19 +-
drivers/cpufreq/imx-cpufreq-dt.c | 12 +-
drivers/cpufreq/qcom-cpufreq-nvmem.c | 109 +--
drivers/cpufreq/sti-cpufreq.c | 27 +-
drivers/cpufreq/sun50i-cpufreq-nvmem.c | 31 +-
drivers/cpufreq/tegra20-cpufreq.c | 12 +-
drivers/cpufreq/ti-cpufreq.c | 42 +-
drivers/devfreq/exynos-bus.c | 21 +-
drivers/gpu/drm/lima/lima_devfreq.c | 12 +-
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 4 +-
drivers/memory/tegra/tegra124-emc.c | 11 +-
drivers/opp/core.c | 821 +++++++++-----------
drivers/opp/opp.h | 32 +-
drivers/opp/ti-opp-supply.c | 77 +-
drivers/soc/tegra/common.c | 49 +-
drivers/soc/tegra/pmc.c | 4 +-
include/linux/pm_opp.h | 295 ++++---
17 files changed, 750 insertions(+), 828 deletions(-)
--
2.31.1.272.g89b43f80a514
Now that we have a central API to handle all OPP table configurations,
migrate the set-clk-name family of helpers to use the new
infrastructure.
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/opp/core.c | 142 +++++++++++++----------------------------
include/linux/pm_opp.h | 41 +++++++-----
2 files changed, 69 insertions(+), 114 deletions(-)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 8dbdfff38973..0a82ca7ae453 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -2164,104 +2164,71 @@ static void _opp_put_regulators(struct opp_table *opp_table)
}
/**
- * dev_pm_opp_set_clkname() - Set clk name for the device
- * @dev: Device for which clk name is being set.
- * @name: Clk name.
- *
- * In order to support OPP switching, OPP layer needs to get pointer to the
- * clock for the device. Simple cases work fine without using this routine (i.e.
- * by passing connection-id as NULL), but for a device with multiple clocks
- * available, the OPP core needs to know the exact name of the clk to use.
+ * _opp_set_clknames() - Set clk names for the device
+ * @dev: Device for which clk names is being set.
+ * @names: Clk names.
+ *
+ * In order to support OPP switching, OPP layer needs to get pointers to the
+ * clocks for the device. Simple cases work fine without using this routine
+ * (i.e. by passing connection-id as NULL), but for a device with multiple
+ * clocks available, the OPP core needs to know the exact names of the clks to
+ * use.
*
* This must be called before any OPPs are initialized for the device.
*/
-struct opp_table *dev_pm_opp_set_clkname(struct device *dev, const char *name)
+static int _opp_set_clknames(struct opp_table *opp_table, struct device *dev,
+ const char * const names[])
{
- struct opp_table *opp_table;
- int ret;
+ const char * const *temp = names;
+ int count = 0;
- opp_table = _add_opp_table(dev, false);
- if (IS_ERR(opp_table))
- return opp_table;
+ /* Count number of clks */
+ while (*temp++)
+ count++;
- /* This should be called before OPPs are initialized */
- if (WARN_ON(!list_empty(&opp_table->opp_list))) {
- ret = -EBUSY;
- goto err;
- }
+ /*
+ * This is a special case where we have a single clock, whose connection
+ * id name is NULL, i.e. first two entries are NULL in the array.
+ */
+ if (!count && !names[1])
+ count = 1;
+
+ /* We support only one clock name for now */
+ if (count != 1)
+ return -EINVAL;
/* Another CPU that shares the OPP table has set the clkname ? */
if (opp_table->clk_configured)
- return opp_table;
+ return 0;
/* clk shouldn't be initialized at this point */
- if (WARN_ON(opp_table->clk)) {
- ret = -EBUSY;
- goto err;
- }
+ if (WARN_ON(opp_table->clk))
+ return -EBUSY;
/* Find clk for the device */
- opp_table->clk = clk_get(dev, name);
+ opp_table->clk = clk_get(dev, names[0]);
if (IS_ERR(opp_table->clk)) {
- ret = dev_err_probe(dev, PTR_ERR(opp_table->clk),
+ return dev_err_probe(dev, PTR_ERR(opp_table->clk),
"%s: Couldn't find clock\n", __func__);
- goto err;
}
opp_table->clk_configured = true;
- return opp_table;
-
-err:
- dev_pm_opp_put_opp_table(opp_table);
-
- return ERR_PTR(ret);
-}
-EXPORT_SYMBOL_GPL(dev_pm_opp_set_clkname);
-
-/**
- * dev_pm_opp_put_clkname() - Releases resources blocked for clk.
- * @opp_table: OPP table returned from dev_pm_opp_set_clkname().
- */
-void dev_pm_opp_put_clkname(struct opp_table *opp_table)
-{
- if (unlikely(!opp_table))
- return;
-
- clk_put(opp_table->clk);
- opp_table->clk = ERR_PTR(-EINVAL);
- opp_table->clk_configured = false;
-
- dev_pm_opp_put_opp_table(opp_table);
-}
-EXPORT_SYMBOL_GPL(dev_pm_opp_put_clkname);
-
-static void devm_pm_opp_clkname_release(void *data)
-{
- dev_pm_opp_put_clkname(data);
+ return 0;
}
/**
- * devm_pm_opp_set_clkname() - Set clk name for the device
- * @dev: Device for which clk name is being set.
- * @name: Clk name.
- *
- * This is a resource-managed variant of dev_pm_opp_set_clkname().
- *
- * Return: 0 on success and errorno otherwise.
+ * _opp_put_clknames() - Releases resources blocked for clks.
+ * @opp_table: OPP table returned from _opp_set_clknames().
*/
-int devm_pm_opp_set_clkname(struct device *dev, const char *name)
+static void _opp_put_clknames(struct opp_table *opp_table)
{
- struct opp_table *opp_table;
-
- opp_table = dev_pm_opp_set_clkname(dev, name);
- if (IS_ERR(opp_table))
- return PTR_ERR(opp_table);
-
- return devm_add_action_or_reset(dev, devm_pm_opp_clkname_release,
- opp_table);
+ if (opp_table->clk_configured) {
+ clk_put(opp_table->clk);
+ opp_table->clk = ERR_PTR(-EINVAL);
+ opp_table->clk_configured = false;
+ }
}
-EXPORT_SYMBOL_GPL(devm_pm_opp_set_clkname);
/**
* dev_pm_opp_register_set_opp_helper() - Register custom set OPP helper
@@ -2544,7 +2511,7 @@ static void _opp_clear_config(struct opp_config_data *data)
if (data->flags & OPP_CONFIG_PROP_NAME)
dev_pm_opp_put_prop_name(data->opp_table);
if (data->flags & OPP_CONFIG_CLK)
- dev_pm_opp_put_clkname(data->opp_table);
+ _opp_put_clknames(data->opp_table);
dev_pm_opp_put_opp_table(data->opp_table);
kfree(data);
@@ -2595,32 +2562,9 @@ int dev_pm_opp_set_config(struct device *dev, struct dev_pm_opp_config *config)
/* Configure clocks */
if (config->clk_names) {
- const char * const *temp = config->clk_names;
- int count = 0;
-
- /* Count number of clks */
- while (*temp++)
- count++;
-
- /*
- * This is a special case where we have a single clock, whose
- * connection id name is NULL, i.e. first two entries are NULL
- * in the array.
- */
- if (!count && !config->clk_names[1])
- count = 1;
-
- /* We support only one clock name for now */
- if (count != 1) {
- ret = -EINVAL;
- goto err;
- }
-
- err = dev_pm_opp_set_clkname(dev, config->clk_names[0]);
- if (IS_ERR(err)) {
- ret = PTR_ERR(err);
+ ret = _opp_set_clknames(opp_table, dev, config->clk_names);
+ if (ret)
goto err;
- }
data->flags |= OPP_CONFIG_CLK;
}
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 94d0101c254c..ed1906bbe8bb 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -186,9 +186,6 @@ void dev_pm_opp_clear_config(int token);
struct opp_table *dev_pm_opp_set_prop_name(struct device *dev, const char *name);
void dev_pm_opp_put_prop_name(struct opp_table *opp_table);
-struct opp_table *dev_pm_opp_set_clkname(struct device *dev, const char *name);
-void dev_pm_opp_put_clkname(struct opp_table *opp_table);
-int devm_pm_opp_set_clkname(struct device *dev, const char *name);
struct opp_table *dev_pm_opp_register_set_opp_helper(struct device *dev, int (*set_opp)(struct dev_pm_set_opp_data *data));
void dev_pm_opp_unregister_set_opp_helper(struct opp_table *opp_table);
int devm_pm_opp_register_set_opp_helper(struct device *dev, int (*set_opp)(struct dev_pm_set_opp_data *data));
@@ -387,18 +384,6 @@ static inline struct opp_table *dev_pm_opp_set_prop_name(struct device *dev, con
static inline void dev_pm_opp_put_prop_name(struct opp_table *opp_table) {}
-static inline struct opp_table *dev_pm_opp_set_clkname(struct device *dev, const char *name)
-{
- return ERR_PTR(-EOPNOTSUPP);
-}
-
-static inline void dev_pm_opp_put_clkname(struct opp_table *opp_table) {}
-
-static inline int devm_pm_opp_set_clkname(struct device *dev, const char *name)
-{
- return -EOPNOTSUPP;
-}
-
static inline struct opp_table *dev_pm_opp_attach_genpd(struct device *dev, const char * const *names, struct device ***virt_devs)
{
return ERR_PTR(-EOPNOTSUPP);
@@ -629,4 +614,30 @@ static inline int devm_pm_opp_set_supported_hw(struct device *dev,
return devm_pm_opp_set_config(dev, &config);
}
+/* clkname helpers */
+static inline int dev_pm_opp_set_clkname(struct device *dev, const char *name)
+{
+ const char *names[] = { name, NULL };
+ struct dev_pm_opp_config config = {
+ .clk_names = names,
+ };
+
+ return dev_pm_opp_set_config(dev, &config);
+}
+
+static inline void dev_pm_opp_put_clkname(int token)
+{
+ dev_pm_opp_clear_config(token);
+}
+
+static inline int devm_pm_opp_set_clkname(struct device *dev, const char *name)
+{
+ const char *names[] = { name, NULL };
+ struct dev_pm_opp_config config = {
+ .clk_names = names,
+ };
+
+ return devm_pm_opp_set_config(dev, &config);
+}
+
#endif /* __LINUX_OPP_H__ */
--
2.31.1.272.g89b43f80a514
The OPP core now provides dev_pm_opp_set_config_regulators() interface,
which needs the platforms to just set the OPP voltages instead of both
clk and voltage. The clock is set by the OPP core instead and hence
reduces code redundancy.
Migrate the only user of the custom set_opp() to
dev_pm_opp_set_config_regulators().
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/opp/ti-opp-supply.c | 75 +++++++++++++++++--------------------
1 file changed, 34 insertions(+), 41 deletions(-)
diff --git a/drivers/opp/ti-opp-supply.c b/drivers/opp/ti-opp-supply.c
index 40ebc9ac82dd..8f3f13fbbb25 100644
--- a/drivers/opp/ti-opp-supply.c
+++ b/drivers/opp/ti-opp-supply.c
@@ -36,11 +36,15 @@ struct ti_opp_supply_optimum_voltage_table {
* @vdd_table: Optimized voltage mapping table
* @num_vdd_table: number of entries in vdd_table
* @vdd_absolute_max_voltage_uv: absolute maximum voltage in UV for the supply
+ * @old_supplies: Placeholder for supplies information for old OPP.
+ * @new_supplies: Placeholder for supplies information for new OPP.
*/
struct ti_opp_supply_data {
struct ti_opp_supply_optimum_voltage_table *vdd_table;
u32 num_vdd_table;
u32 vdd_absolute_max_voltage_uv;
+ struct dev_pm_opp_supply old_supplies[2];
+ struct dev_pm_opp_supply new_supplies[2];
};
static struct ti_opp_supply_data opp_data;
@@ -266,27 +270,32 @@ static int _opp_set_voltage(struct device *dev,
return 0;
}
-/**
- * ti_opp_supply_set_opp() - do the opp supply transition
- * @data: information on regulators and new and old opps provided by
- * opp core to use in transition
- *
- * Return: If successful, 0, else appropriate error value.
- */
-static int ti_opp_supply_set_opp(struct dev_pm_set_opp_data *data)
+/* Do the opp supply transition */
+static int ti_opp_config_regulators(struct device *dev,
+ struct dev_pm_opp *old_opp, struct dev_pm_opp *new_opp,
+ struct regulator **regulators, unsigned int count)
{
- struct dev_pm_opp_supply *old_supply_vdd = &data->old_opp.supplies[0];
- struct dev_pm_opp_supply *old_supply_vbb = &data->old_opp.supplies[1];
- struct dev_pm_opp_supply *new_supply_vdd = &data->new_opp.supplies[0];
- struct dev_pm_opp_supply *new_supply_vbb = &data->new_opp.supplies[1];
- struct device *dev = data->dev;
- unsigned long old_freq = data->old_opp.rate, freq = data->new_opp.rate;
- struct clk *clk = data->clk;
- struct regulator *vdd_reg = data->regulators[0];
- struct regulator *vbb_reg = data->regulators[1];
+ struct dev_pm_opp_supply *old_supply_vdd = &opp_data.old_supplies[0];
+ struct dev_pm_opp_supply *old_supply_vbb = &opp_data.old_supplies[1];
+ struct dev_pm_opp_supply *new_supply_vdd = &opp_data.new_supplies[0];
+ struct dev_pm_opp_supply *new_supply_vbb = &opp_data.new_supplies[1];
+ struct regulator *vdd_reg = regulators[0];
+ struct regulator *vbb_reg = regulators[1];
+ unsigned long old_freq, freq;
int vdd_uv;
int ret;
+ /* We must have two regulators here */
+ WARN_ON(count != 2);
+
+ /* Fetch supplies and freq information from OPP core */
+ ret = dev_pm_opp_get_supplies(new_opp, opp_data.new_supplies);
+ WARN_ON(ret);
+
+ old_freq = dev_pm_opp_get_freq(old_opp);
+ freq = dev_pm_opp_get_freq(new_opp);
+ WARN_ON(!old_freq || !freq);
+
vdd_uv = _get_optimal_vdd_voltage(dev, &opp_data,
new_supply_vdd->u_volt);
@@ -303,39 +312,24 @@ static int ti_opp_supply_set_opp(struct dev_pm_set_opp_data *data)
ret = _opp_set_voltage(dev, new_supply_vbb, 0, vbb_reg, "vbb");
if (ret)
goto restore_voltage;
- }
-
- /* Change frequency */
- dev_dbg(dev, "%s: switching OPP: %lu Hz --> %lu Hz\n",
- __func__, old_freq, freq);
-
- ret = clk_set_rate(clk, freq);
- if (ret) {
- dev_err(dev, "%s: failed to set clock rate: %d\n", __func__,
- ret);
- goto restore_voltage;
- }
-
- /* Scaling down? Scale voltage after frequency */
- if (freq < old_freq) {
+ } else {
ret = _opp_set_voltage(dev, new_supply_vbb, 0, vbb_reg, "vbb");
if (ret)
- goto restore_freq;
+ goto restore_voltage;
ret = _opp_set_voltage(dev, new_supply_vdd, vdd_uv, vdd_reg,
"vdd");
if (ret)
- goto restore_freq;
+ goto restore_voltage;
}
return 0;
-restore_freq:
- ret = clk_set_rate(clk, old_freq);
- if (ret)
- dev_err(dev, "%s: failed to restore old-freq (%lu Hz)\n",
- __func__, old_freq);
restore_voltage:
+ /* Fetch old supplies information only if required */
+ ret = dev_pm_opp_get_supplies(old_opp, opp_data.old_supplies);
+ WARN_ON(ret);
+
/* This shouldn't harm even if the voltages weren't updated earlier */
if (old_supply_vdd->u_volt) {
ret = _opp_set_voltage(dev, old_supply_vbb, 0, vbb_reg, "vbb");
@@ -405,8 +399,7 @@ static int ti_opp_supply_probe(struct platform_device *pdev)
return ret;
}
- ret = dev_pm_opp_register_set_opp_helper(cpu_dev,
- ti_opp_supply_set_opp);
+ ret = dev_pm_opp_set_config_regulators(cpu_dev, ti_opp_config_regulators);
if (ret < 0)
_free_optimized_voltages(dev, &opp_data);
--
2.31.1.272.g89b43f80a514
Now that we have a central API to handle all OPP table configurations,
migrate the attach-genpd family of helpers to use the new
infrastructure.
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/opp/core.c | 85 +++++++++---------------------------------
include/linux/pm_opp.h | 48 +++++++++++++++---------
2 files changed, 49 insertions(+), 84 deletions(-)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 9da7dcf62cab..458584994c2b 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -2285,7 +2285,7 @@ static void _opp_unregister_set_opp_helper(struct opp_table *opp_table)
}
}
-static void _opp_detach_genpd(struct opp_table *opp_table)
+static void _detach_genpd(struct opp_table *opp_table)
{
int index;
@@ -2305,7 +2305,7 @@ static void _opp_detach_genpd(struct opp_table *opp_table)
}
/**
- * dev_pm_opp_attach_genpd - Attach genpd(s) for the device and save virtual device pointer
+ * _opp_attach_genpd - Attach genpd(s) for the device and save virtual device pointer
* @dev: Consumer device for which the genpd is getting attached.
* @names: Null terminated array of pointers containing names of genpd to attach.
* @virt_devs: Pointer to return the array of virtual devices.
@@ -2326,30 +2326,23 @@ static void _opp_detach_genpd(struct opp_table *opp_table)
* The order of entries in the names array must match the order in which
* "required-opps" are added in DT.
*/
-struct opp_table *dev_pm_opp_attach_genpd(struct device *dev,
- const char * const *names, struct device ***virt_devs)
+static int _opp_attach_genpd(struct opp_table *opp_table, struct device *dev,
+ const char * const *names, struct device ***virt_devs)
{
- struct opp_table *opp_table;
struct device *virt_dev;
int index = 0, ret = -EINVAL;
const char * const *name = names;
- opp_table = _add_opp_table(dev, false);
- if (IS_ERR(opp_table))
- return opp_table;
-
if (opp_table->genpd_virt_devs)
- return opp_table;
+ return 0;
/*
* If the genpd's OPP table isn't already initialized, parsing of the
* required-opps fail for dev. We should retry this after genpd's OPP
* table is added.
*/
- if (!opp_table->required_opp_count) {
- ret = -EPROBE_DEFER;
- goto put_table;
- }
+ if (!opp_table->required_opp_count)
+ return -EPROBE_DEFER;
mutex_lock(&opp_table->genpd_virt_dev_lock);
@@ -2382,78 +2375,38 @@ struct opp_table *dev_pm_opp_attach_genpd(struct device *dev,
*virt_devs = opp_table->genpd_virt_devs;
mutex_unlock(&opp_table->genpd_virt_dev_lock);
- return opp_table;
+ return 0;
err:
- _opp_detach_genpd(opp_table);
+ _detach_genpd(opp_table);
unlock:
mutex_unlock(&opp_table->genpd_virt_dev_lock);
+ return ret;
-put_table:
- dev_pm_opp_put_opp_table(opp_table);
-
- return ERR_PTR(ret);
}
-EXPORT_SYMBOL_GPL(dev_pm_opp_attach_genpd);
/**
- * dev_pm_opp_detach_genpd() - Detach genpd(s) from the device.
- * @opp_table: OPP table returned by dev_pm_opp_attach_genpd().
+ * _opp_detach_genpd() - Detach genpd(s) from the device.
+ * @opp_table: OPP table returned by _opp_attach_genpd().
*
* This detaches the genpd(s), resets the virtual device pointers, and puts the
* OPP table.
*/
-void dev_pm_opp_detach_genpd(struct opp_table *opp_table)
+static void _opp_detach_genpd(struct opp_table *opp_table)
{
- if (unlikely(!opp_table))
- return;
-
/*
* Acquire genpd_virt_dev_lock to make sure virt_dev isn't getting
* used in parallel.
*/
mutex_lock(&opp_table->genpd_virt_dev_lock);
- _opp_detach_genpd(opp_table);
+ _detach_genpd(opp_table);
mutex_unlock(&opp_table->genpd_virt_dev_lock);
-
- dev_pm_opp_put_opp_table(opp_table);
}
-EXPORT_SYMBOL_GPL(dev_pm_opp_detach_genpd);
-
-static void devm_pm_opp_detach_genpd(void *data)
-{
- dev_pm_opp_detach_genpd(data);
-}
-
-/**
- * devm_pm_opp_attach_genpd - Attach genpd(s) for the device and save virtual
- * device pointer
- * @dev: Consumer device for which the genpd is getting attached.
- * @names: Null terminated array of pointers containing names of genpd to attach.
- * @virt_devs: Pointer to return the array of virtual devices.
- *
- * This is a resource-managed version of dev_pm_opp_attach_genpd().
- *
- * Return: 0 on success and errorno otherwise.
- */
-int devm_pm_opp_attach_genpd(struct device *dev, const char * const *names,
- struct device ***virt_devs)
-{
- struct opp_table *opp_table;
-
- opp_table = dev_pm_opp_attach_genpd(dev, names, virt_devs);
- if (IS_ERR(opp_table))
- return PTR_ERR(opp_table);
-
- return devm_add_action_or_reset(dev, devm_pm_opp_detach_genpd,
- opp_table);
-}
-EXPORT_SYMBOL_GPL(devm_pm_opp_attach_genpd);
static void _opp_clear_config(struct opp_config_data *data)
{
if (data->flags & OPP_CONFIG_GENPD)
- dev_pm_opp_detach_genpd(data->opp_table);
+ _opp_detach_genpd(data->opp_table);
if (data->flags & OPP_CONFIG_REGULATOR)
_opp_put_regulators(data->opp_table);
if (data->flags & OPP_CONFIG_SUPPORTED_HW)
@@ -2564,12 +2517,10 @@ int dev_pm_opp_set_config(struct device *dev, struct dev_pm_opp_config *config)
/* Attach genpds */
if (config->genpd_names) {
- err = dev_pm_opp_attach_genpd(dev, config->genpd_names,
- config->virt_devs);
- if (IS_ERR(err)) {
- ret = PTR_ERR(err);
+ ret = _opp_attach_genpd(opp_table, dev, config->genpd_names,
+ config->virt_devs);
+ if (ret)
goto err;
- }
data->flags |= OPP_CONFIG_GENPD;
}
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 85a4b7353979..20e1e5060a8a 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -186,9 +186,6 @@ void dev_pm_opp_clear_config(int token);
struct opp_table *dev_pm_opp_set_prop_name(struct device *dev, const char *name);
void dev_pm_opp_put_prop_name(struct opp_table *opp_table);
-struct opp_table *dev_pm_opp_attach_genpd(struct device *dev, const char * const *names, struct device ***virt_devs);
-void dev_pm_opp_detach_genpd(struct opp_table *opp_table);
-int devm_pm_opp_attach_genpd(struct device *dev, const char * const *names, struct device ***virt_devs);
struct dev_pm_opp *dev_pm_opp_xlate_required_opp(struct opp_table *src_table, struct opp_table *dst_table, struct dev_pm_opp *src_opp);
int dev_pm_opp_xlate_performance_state(struct opp_table *src_table, struct opp_table *dst_table, unsigned int pstate);
int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
@@ -367,20 +364,6 @@ static inline struct opp_table *dev_pm_opp_set_prop_name(struct device *dev, con
static inline void dev_pm_opp_put_prop_name(struct opp_table *opp_table) {}
-static inline struct opp_table *dev_pm_opp_attach_genpd(struct device *dev, const char * const *names, struct device ***virt_devs)
-{
- return ERR_PTR(-EOPNOTSUPP);
-}
-
-static inline void dev_pm_opp_detach_genpd(struct opp_table *opp_table) {}
-
-static inline int devm_pm_opp_attach_genpd(struct device *dev,
- const char * const *names,
- struct device ***virt_devs)
-{
- return -EOPNOTSUPP;
-}
-
static inline int dev_pm_opp_set_config(struct device *dev, struct dev_pm_opp_config *config)
{
return -EOPNOTSUPP;
@@ -639,4 +622,35 @@ static inline void dev_pm_opp_unregister_set_opp_helper(int token)
dev_pm_opp_clear_config(token);
}
+/* genpd helpers */
+static inline int dev_pm_opp_attach_genpd(struct device *dev,
+ const char * const *names,
+ struct device ***virt_devs)
+{
+ struct dev_pm_opp_config config = {
+ .genpd_names = names,
+ .virt_devs = virt_devs,
+ };
+
+ return dev_pm_opp_set_config(dev, &config);
+}
+
+static inline void dev_pm_opp_detach_genpd(int token)
+{
+ dev_pm_opp_clear_config(token);
+}
+
+static inline int devm_pm_opp_attach_genpd(struct device *dev,
+ const char * const *names,
+ struct device ***virt_devs)
+{
+ struct dev_pm_opp_config config = {
+ .genpd_names = names,
+ .virt_devs = virt_devs,
+ };
+
+ return devm_pm_opp_set_config(dev, &config);
+}
+
+
#endif /* __LINUX_OPP_H__ */
--
2.31.1.272.g89b43f80a514
Now that we have a central API to handle all OPP table configurations,
migrate the set-regulators family of helpers to use the new
infrastructure.
The return type and parameter to the APIs change a bit due to this,
update the current users as well in the same commit in order to avoid
breaking builds.
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/cpufreq/cpufreq-dt.c | 12 ++---
drivers/devfreq/exynos-bus.c | 19 +++-----
drivers/opp/core.c | 91 ++++++++----------------------------
include/linux/pm_opp.h | 44 ++++++++++-------
4 files changed, 60 insertions(+), 106 deletions(-)
diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index be0c19b3ffa5..d69d13a26414 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -29,9 +29,9 @@ struct private_data {
cpumask_var_t cpus;
struct device *cpu_dev;
- struct opp_table *opp_table;
struct cpufreq_frequency_table *freq_table;
bool have_static_opps;
+ int opp_token;
};
static LIST_HEAD(priv_list);
@@ -220,9 +220,9 @@ static int dt_cpufreq_early_init(struct device *dev, int cpu)
*/
reg_name[0] = find_supply_name(cpu_dev);
if (reg_name[0]) {
- priv->opp_table = dev_pm_opp_set_regulators(cpu_dev, reg_name);
- if (IS_ERR(priv->opp_table)) {
- ret = PTR_ERR(priv->opp_table);
+ priv->opp_token = dev_pm_opp_set_regulators(cpu_dev, reg_name);
+ if (priv->opp_token < 0) {
+ ret = priv->opp_token;
if (ret != -EPROBE_DEFER)
dev_err(cpu_dev, "failed to set regulators: %d\n",
ret);
@@ -294,7 +294,7 @@ static int dt_cpufreq_early_init(struct device *dev, int cpu)
out:
if (priv->have_static_opps)
dev_pm_opp_of_cpumask_remove_table(priv->cpus);
- dev_pm_opp_put_regulators(priv->opp_table);
+ dev_pm_opp_put_regulators(priv->opp_token);
free_cpumask:
free_cpumask_var(priv->cpus);
return ret;
@@ -308,7 +308,7 @@ static void dt_cpufreq_release(void)
dev_pm_opp_free_cpufreq_table(priv->cpu_dev, &priv->freq_table);
if (priv->have_static_opps)
dev_pm_opp_of_cpumask_remove_table(priv->cpus);
- dev_pm_opp_put_regulators(priv->opp_table);
+ dev_pm_opp_put_regulators(priv->opp_token);
free_cpumask_var(priv->cpus);
list_del(&priv->node);
}
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 541baff93ee8..d1235242367f 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -33,7 +33,7 @@ struct exynos_bus {
unsigned long curr_freq;
- struct opp_table *opp_table;
+ int opp_token;
struct clk *clk;
unsigned int ratio;
};
@@ -161,8 +161,7 @@ static void exynos_bus_exit(struct device *dev)
dev_pm_opp_of_remove_table(dev);
clk_disable_unprepare(bus->clk);
- dev_pm_opp_put_regulators(bus->opp_table);
- bus->opp_table = NULL;
+ dev_pm_opp_put_regulators(bus->opp_token);
}
static void exynos_bus_passive_exit(struct device *dev)
@@ -179,18 +178,16 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
struct exynos_bus *bus)
{
struct device *dev = bus->dev;
- struct opp_table *opp_table;
const char *supplies[] = { "vdd", NULL };
int i, ret, count, size;
- opp_table = dev_pm_opp_set_regulators(dev, supplies);
- if (IS_ERR(opp_table)) {
- ret = PTR_ERR(opp_table);
+ ret = dev_pm_opp_set_regulators(dev, supplies);
+ if (ret < 0) {
dev_err(dev, "failed to set regulators %d\n", ret);
return ret;
}
- bus->opp_table = opp_table;
+ bus->opp_token = ret;
/*
* Get the devfreq-event devices to get the current utilization of
@@ -236,8 +233,7 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
return 0;
err_regulator:
- dev_pm_opp_put_regulators(bus->opp_table);
- bus->opp_table = NULL;
+ dev_pm_opp_put_regulators(bus->opp_token);
return ret;
}
@@ -459,8 +455,7 @@ static int exynos_bus_probe(struct platform_device *pdev)
dev_pm_opp_of_remove_table(dev);
clk_disable_unprepare(bus->clk);
err_reg:
- dev_pm_opp_put_regulators(bus->opp_table);
- bus->opp_table = NULL;
+ dev_pm_opp_put_regulators(bus->opp_token);
return ret;
}
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 7ab20c3b91ed..6ff9b5b69d07 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -991,8 +991,8 @@ static int _set_opp_custom(const struct opp_table *opp_table,
int size;
/*
- * We support this only if dev_pm_opp_set_regulators() was called
- * earlier.
+ * We support this only if dev_pm_opp_set_config() was called
+ * earlier to set regulators.
*/
if (opp_table->sod_supplies) {
size = sizeof(*old_opp->supplies) * opp_table->regulator_count;
@@ -2097,7 +2097,7 @@ void dev_pm_opp_put_prop_name(struct opp_table *opp_table)
EXPORT_SYMBOL_GPL(dev_pm_opp_put_prop_name);
/**
- * dev_pm_opp_set_regulators() - Set regulator names for the device
+ * _opp_set_regulators() - Set regulator names for the device
* @dev: Device for which regulator name is being set.
* @names: Array of pointers to the names of the regulator.
* @count: Number of regulators.
@@ -2108,12 +2108,11 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_put_prop_name);
*
* This must be called before any OPPs are initialized for the device.
*/
-struct opp_table *dev_pm_opp_set_regulators(struct device *dev,
- const char * const names[])
+static int _opp_set_regulators(struct opp_table *opp_table, struct device *dev,
+ const char * const names[])
{
struct dev_pm_opp_supply *supplies;
const char * const *temp = names;
- struct opp_table *opp_table;
struct regulator *reg;
int count = 0, ret, i;
@@ -2122,29 +2121,17 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev,
count++;
if (!count)
- return ERR_PTR(-EINVAL);
-
- opp_table = _add_opp_table(dev, false);
- if (IS_ERR(opp_table))
- return opp_table;
-
- /* This should be called before OPPs are initialized */
- if (WARN_ON(!list_empty(&opp_table->opp_list))) {
- ret = -EBUSY;
- goto err;
- }
+ return -EINVAL;
/* Another CPU that shares the OPP table has set the regulators ? */
if (opp_table->regulators)
- return opp_table;
+ return 0;
opp_table->regulators = kmalloc_array(count,
sizeof(*opp_table->regulators),
GFP_KERNEL);
- if (!opp_table->regulators) {
- ret = -ENOMEM;
- goto err;
- }
+ if (!opp_table->regulators)
+ return -ENOMEM;
for (i = 0; i < count; i++) {
reg = regulator_get_optional(dev, names[i]);
@@ -2174,7 +2161,7 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev,
}
mutex_unlock(&opp_table->lock);
- return opp_table;
+ return 0;
free_regulators:
while (i != 0)
@@ -2183,26 +2170,20 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev,
kfree(opp_table->regulators);
opp_table->regulators = NULL;
opp_table->regulator_count = -1;
-err:
- dev_pm_opp_put_opp_table(opp_table);
- return ERR_PTR(ret);
+ return ret;
}
-EXPORT_SYMBOL_GPL(dev_pm_opp_set_regulators);
/**
- * dev_pm_opp_put_regulators() - Releases resources blocked for regulator
- * @opp_table: OPP table returned from dev_pm_opp_set_regulators().
+ * _opp_put_regulators() - Releases resources blocked for regulator
+ * @opp_table: OPP table returned from _opp_set_regulators().
*/
-void dev_pm_opp_put_regulators(struct opp_table *opp_table)
+static void _opp_put_regulators(struct opp_table *opp_table)
{
int i;
- if (unlikely(!opp_table))
- return;
-
if (!opp_table->regulators)
- goto put_opp_table;
+ return;
if (opp_table->enabled) {
for (i = opp_table->regulator_count - 1; i >= 0; i--)
@@ -2225,40 +2206,7 @@ void dev_pm_opp_put_regulators(struct opp_table *opp_table)
kfree(opp_table->regulators);
opp_table->regulators = NULL;
opp_table->regulator_count = -1;
-
-put_opp_table:
- dev_pm_opp_put_opp_table(opp_table);
-}
-EXPORT_SYMBOL_GPL(dev_pm_opp_put_regulators);
-
-static void devm_pm_opp_regulators_release(void *data)
-{
- dev_pm_opp_put_regulators(data);
-}
-
-/**
- * devm_pm_opp_set_regulators() - Set regulator names for the device
- * @dev: Device for which regulator name is being set.
- * @names: Array of pointers to the names of the regulator.
- * @count: Number of regulators.
- *
- * This is a resource-managed variant of dev_pm_opp_set_regulators().
- *
- * Return: 0 on success and errorno otherwise.
- */
-int devm_pm_opp_set_regulators(struct device *dev,
- const char * const names[])
-{
- struct opp_table *opp_table;
-
- opp_table = dev_pm_opp_set_regulators(dev, names);
- if (IS_ERR(opp_table))
- return PTR_ERR(opp_table);
-
- return devm_add_action_or_reset(dev, devm_pm_opp_regulators_release,
- opp_table);
}
-EXPORT_SYMBOL_GPL(devm_pm_opp_set_regulators);
/**
* dev_pm_opp_set_clkname() - Set clk name for the device
@@ -2633,7 +2581,7 @@ static void _opp_clear_config(struct opp_config_data *data)
if (data->flags & OPP_CONFIG_GENPD)
dev_pm_opp_detach_genpd(data->opp_table);
if (data->flags & OPP_CONFIG_REGULATOR)
- dev_pm_opp_put_regulators(data->opp_table);
+ _opp_put_regulators(data->opp_table);
if (data->flags & OPP_CONFIG_SUPPORTED_HW)
dev_pm_opp_put_supported_hw(data->opp_table);
if (data->flags & OPP_CONFIG_REGULATOR_HELPER)
@@ -2758,11 +2706,10 @@ int dev_pm_opp_set_config(struct device *dev, struct dev_pm_opp_config *config)
/* Configure supplies */
if (config->regulator_names) {
- err = dev_pm_opp_set_regulators(dev, config->regulator_names);
- if (IS_ERR(err)) {
- ret = PTR_ERR(err);
+ ret = _opp_set_regulators(opp_table, dev,
+ config->regulator_names);
+ if (ret)
goto err;
- }
data->flags |= OPP_CONFIG_REGULATOR;
}
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index a08f9481efb3..f014bd172c99 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -189,9 +189,6 @@ void dev_pm_opp_put_supported_hw(struct opp_table *opp_table);
int devm_pm_opp_set_supported_hw(struct device *dev, const u32 *versions, unsigned int count);
struct opp_table *dev_pm_opp_set_prop_name(struct device *dev, const char *name);
void dev_pm_opp_put_prop_name(struct opp_table *opp_table);
-struct opp_table *dev_pm_opp_set_regulators(struct device *dev, const char * const names[]);
-void dev_pm_opp_put_regulators(struct opp_table *opp_table);
-int devm_pm_opp_set_regulators(struct device *dev, const char * const names[]);
struct opp_table *dev_pm_opp_set_clkname(struct device *dev, const char *name);
void dev_pm_opp_put_clkname(struct opp_table *opp_table);
int devm_pm_opp_set_clkname(struct device *dev, const char *name);
@@ -409,19 +406,6 @@ static inline struct opp_table *dev_pm_opp_set_prop_name(struct device *dev, con
static inline void dev_pm_opp_put_prop_name(struct opp_table *opp_table) {}
-static inline struct opp_table *dev_pm_opp_set_regulators(struct device *dev, const char * const names[])
-{
- return ERR_PTR(-EOPNOTSUPP);
-}
-
-static inline void dev_pm_opp_put_regulators(struct opp_table *opp_table) {}
-
-static inline int devm_pm_opp_set_regulators(struct device *dev,
- const char * const names[])
-{
- return -EOPNOTSUPP;
-}
-
static inline struct opp_table *dev_pm_opp_set_clkname(struct device *dev, const char *name)
{
return ERR_PTR(-EOPNOTSUPP);
@@ -606,4 +590,32 @@ static inline int dev_pm_opp_of_find_icc_paths(struct device *dev, struct opp_ta
}
#endif
+/* OPP Configuration helpers */
+
+/* Regulators helpers */
+static inline int dev_pm_opp_set_regulators(struct device *dev,
+ const char * const names[])
+{
+ struct dev_pm_opp_config config = {
+ .regulator_names = names,
+ };
+
+ return dev_pm_opp_set_config(dev, &config);
+}
+
+static inline void dev_pm_opp_put_regulators(int token)
+{
+ dev_pm_opp_clear_config(token);
+}
+
+static inline int devm_pm_opp_set_regulators(struct device *dev,
+ const char * const names[])
+{
+ struct dev_pm_opp_config config = {
+ .regulator_names = names,
+ };
+
+ return devm_pm_opp_set_config(dev, &config);
+}
+
#endif /* __LINUX_OPP_H__ */
--
2.31.1.272.g89b43f80a514
On 22. 7. 4. 21:07, Viresh Kumar wrote:
> Now that we have a central API to handle all OPP table configurations,
> migrate the set-regulators family of helpers to use the new
> infrastructure.
>
> The return type and parameter to the APIs change a bit due to this,
> update the current users as well in the same commit in order to avoid
> breaking builds.
>
> Signed-off-by: Viresh Kumar <[email protected]>
> ---
> drivers/cpufreq/cpufreq-dt.c | 12 ++---
> drivers/devfreq/exynos-bus.c | 19 +++-----
> drivers/opp/core.c | 91 ++++++++----------------------------
> include/linux/pm_opp.h | 44 ++++++++++-------
> 4 files changed, 60 insertions(+), 106 deletions(-)
>
(snip)
> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> index 541baff93ee8..d1235242367f 100644
> --- a/drivers/devfreq/exynos-bus.c
> +++ b/drivers/devfreq/exynos-bus.c
> @@ -33,7 +33,7 @@ struct exynos_bus {
>
> unsigned long curr_freq;
>
> - struct opp_table *opp_table;
> + int opp_token;
> struct clk *clk;
> unsigned int ratio;
> };
> @@ -161,8 +161,7 @@ static void exynos_bus_exit(struct device *dev)
>
> dev_pm_opp_of_remove_table(dev);
> clk_disable_unprepare(bus->clk);
> - dev_pm_opp_put_regulators(bus->opp_table);
> - bus->opp_table = NULL;
> + dev_pm_opp_put_regulators(bus->opp_token);
> }
>
> static void exynos_bus_passive_exit(struct device *dev)
> @@ -179,18 +178,16 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
> struct exynos_bus *bus)
> {
> struct device *dev = bus->dev;
> - struct opp_table *opp_table;
> const char *supplies[] = { "vdd", NULL };
> int i, ret, count, size;
>
> - opp_table = dev_pm_opp_set_regulators(dev, supplies);
> - if (IS_ERR(opp_table)) {
> - ret = PTR_ERR(opp_table);
> + ret = dev_pm_opp_set_regulators(dev, supplies);
> + if (ret < 0) {
> dev_err(dev, "failed to set regulators %d\n", ret);
> return ret;
> }
>
> - bus->opp_table = opp_table;
> + bus->opp_token = ret;
>
> /*
> * Get the devfreq-event devices to get the current utilization of
> @@ -236,8 +233,7 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
> return 0;
>
> err_regulator:
> - dev_pm_opp_put_regulators(bus->opp_table);
> - bus->opp_table = NULL;
> + dev_pm_opp_put_regulators(bus->opp_token);
>
> return ret;
> }
> @@ -459,8 +455,7 @@ static int exynos_bus_probe(struct platform_device *pdev)
> dev_pm_opp_of_remove_table(dev);
> clk_disable_unprepare(bus->clk);
> err_reg:
> - dev_pm_opp_put_regulators(bus->opp_table);
> - bus->opp_table = NULL;
> + dev_pm_opp_put_regulators(bus->opp_token);
>
> return ret;
> }
Reviewed-by: Chanwoo Choi <[email protected]>
Thanks.
(snip)
--
Best Regards,
Samsung Electronics
Chanwoo Choi