From 87686cc845c3be7dea777f1dbf2de0767007cda8 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Mon, 4 Jul 2022 16:10:39 +0530 Subject: OPP: Make dev_pm_opp_set_regulators() accept NULL terminated list Make dev_pm_opp_set_regulators() accept a NULL terminated list of names instead of making the callers keep the two parameters in sync, which creates an opportunity for bugs to get in. Suggested-by: Greg Kroah-Hartman Reviewed-by: Steven Price # panfrost Reviewed-by: Chanwoo Choi Signed-off-by: Viresh Kumar --- drivers/devfreq/exynos-bus.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/devfreq') diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c index e689101abc93..541baff93ee8 100644 --- a/drivers/devfreq/exynos-bus.c +++ b/drivers/devfreq/exynos-bus.c @@ -180,10 +180,10 @@ static int exynos_bus_parent_parse_of(struct device_node *np, { struct device *dev = bus->dev; struct opp_table *opp_table; - const char *vdd = "vdd"; + const char *supplies[] = { "vdd", NULL }; int i, ret, count, size; - opp_table = dev_pm_opp_set_regulators(dev, &vdd, 1); + opp_table = dev_pm_opp_set_regulators(dev, supplies); if (IS_ERR(opp_table)) { ret = PTR_ERR(opp_table); dev_err(dev, "failed to set regulators %d\n", ret); -- cgit v1.2.3 From b0ec09428621daee5101130c307634a390b0213b Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Mon, 4 Jul 2022 16:36:26 +0530 Subject: OPP: Migrate set-regulators API to use set-config helpers 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. Reviewed-by: Chanwoo Choi Signed-off-by: Viresh Kumar --- 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(-) (limited to 'drivers/devfreq') 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 @@ err: 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 @@ free_regulators: 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__ */ -- cgit v1.2.3 From 1b195626850d90663d17299bd378db30f23307e4 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Mon, 4 Jul 2022 21:10:08 +0530 Subject: PM / devfreq: tegra30: Register config_clks helper There is a corner case with Tegra30, where we want to skip clk configuration that happens via dev_pm_opp_set_opp(), but still want the OPP core to read the "opp-hz" property so we can find the right OPP via freq finding helpers. The OPP core provides support for the platforms to provide config_clks helpers now, lets use that instead of devm_pm_opp_of_add_table_noclk() to achieve the same result, as the OPP core won't parse the DT's "opp-hz" property anymore if the clock isn't provided. Tested-by: Dmitry Osipenko Signed-off-by: Viresh Kumar --- drivers/devfreq/tegra30-devfreq.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) (limited to 'drivers/devfreq') diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c index 65ecf17a36f4..0e0a4058f45c 100644 --- a/drivers/devfreq/tegra30-devfreq.c +++ b/drivers/devfreq/tegra30-devfreq.c @@ -821,6 +821,15 @@ static int devm_tegra_devfreq_init_hw(struct device *dev, return err; } +static int tegra_devfreq_config_clks_nop(struct device *dev, + struct opp_table *opp_table, + struct dev_pm_opp *opp, void *data, + bool scaling_down) +{ + /* We want to skip clk configuration via dev_pm_opp_set_opp() */ + return 0; +} + static int tegra_devfreq_probe(struct platform_device *pdev) { u32 hw_version = BIT(tegra_sku_info.soc_speedo_id); @@ -830,6 +839,13 @@ static int tegra_devfreq_probe(struct platform_device *pdev) unsigned int i; long rate; int err; + const char *clk_names[] = { "actmon", NULL }; + struct dev_pm_opp_config config = { + .supported_hw = &hw_version, + .supported_hw_count = 1, + .clk_names = clk_names, + .config_clks = tegra_devfreq_config_clks_nop, + }; tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL); if (!tegra) @@ -874,13 +890,13 @@ static int tegra_devfreq_probe(struct platform_device *pdev) return err; } - err = devm_pm_opp_set_supported_hw(&pdev->dev, &hw_version, 1); + err = devm_pm_opp_set_config(&pdev->dev, &config); if (err) { - dev_err(&pdev->dev, "Failed to set supported HW: %d\n", err); + dev_err(&pdev->dev, "Failed to set OPP config: %d\n", err); return err; } - err = devm_pm_opp_of_add_table_noclk(&pdev->dev, 0); + err = devm_pm_opp_of_add_table_indexed(&pdev->dev, 0); if (err) { dev_err(&pdev->dev, "Failed to add OPP table: %d\n", err); return err; -- cgit v1.2.3