* [PATCH V3 02/20] OPP: Make dev_pm_opp_set_regulators() accept NULL terminated list
2022-07-04 12:07 [PATCH V3 00/20] OPP: Add new configuration interface: dev_pm_opp_set_config() Viresh Kumar
@ 2022-07-04 12:07 ` Viresh Kumar
2022-07-04 14:35 ` Steven Price
` (2 more replies)
2022-07-04 12:07 ` [PATCH V3 08/20] soc/tegra: Add comment over devm_pm_opp_set_clkname() Viresh Kumar
` (2 subsequent siblings)
3 siblings, 3 replies; 11+ messages in thread
From: Viresh Kumar @ 2022-07-04 12:07 UTC (permalink / raw)
To: Rafael J. Wysocki, Viresh Kumar, Chanwoo Choi, MyungJoo Ham,
Kyungmin Park, Krzysztof Kozlowski, Alim Akhtar, Qiang Yu,
Rob Herring, Tomeu Vizoso, Steven Price, Alyssa Rosenzweig,
Nishanth Menon, Stephen Boyd, Thierry Reding, Jonathan Hunter
Cc: linux-pm, Vincent Guittot, Greg Kroah-Hartman, linux-kernel,
linux-samsung-soc, linux-arm-kernel, dri-devel, lima, linux-tegra
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 <gregkh@linuxfoundation.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq-dt.c | 9 ++++-----
drivers/cpufreq/ti-cpufreq.c | 7 +++----
drivers/devfreq/exynos-bus.c | 4 ++--
drivers/gpu/drm/lima/lima_devfreq.c | 3 ++-
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 4 ++--
drivers/opp/core.c | 18 ++++++++++++------
drivers/soc/tegra/pmc.c | 4 ++--
include/linux/pm_opp.h | 9 ++++-----
8 files changed, 31 insertions(+), 27 deletions(-)
diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 8fcaba541539..be0c19b3ffa5 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -193,7 +193,7 @@ static int dt_cpufreq_early_init(struct device *dev, int cpu)
struct private_data *priv;
struct device *cpu_dev;
bool fallback = false;
- const char *reg_name;
+ const char *reg_name[] = { NULL, NULL };
int ret;
/* Check if this CPU is already covered by some other policy */
@@ -218,10 +218,9 @@ static int dt_cpufreq_early_init(struct device *dev, int cpu)
* OPP layer will be taking care of regulators now, but it needs to know
* the name of the regulator first.
*/
- reg_name = find_supply_name(cpu_dev);
- if (reg_name) {
- priv->opp_table = dev_pm_opp_set_regulators(cpu_dev, ®_name,
- 1);
+ 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);
if (ret != -EPROBE_DEFER)
diff --git a/drivers/cpufreq/ti-cpufreq.c b/drivers/cpufreq/ti-cpufreq.c
index 8f9fdd864391..560d67a6bef1 100644
--- a/drivers/cpufreq/ti-cpufreq.c
+++ b/drivers/cpufreq/ti-cpufreq.c
@@ -173,7 +173,7 @@ static struct ti_cpufreq_soc_data omap34xx_soc_data = {
* seems to always read as 0).
*/
-static const char * const omap3_reg_names[] = {"cpu0", "vbb"};
+static const char * const omap3_reg_names[] = {"cpu0", "vbb", NULL};
static struct ti_cpufreq_soc_data omap36xx_soc_data = {
.reg_names = omap3_reg_names,
@@ -326,7 +326,7 @@ static int ti_cpufreq_probe(struct platform_device *pdev)
const struct of_device_id *match;
struct opp_table *ti_opp_table;
struct ti_cpufreq_data *opp_data;
- const char * const default_reg_names[] = {"vdd", "vbb"};
+ const char * const default_reg_names[] = {"vdd", "vbb", NULL};
int ret;
match = dev_get_platdata(&pdev->dev);
@@ -387,8 +387,7 @@ static int ti_cpufreq_probe(struct platform_device *pdev)
if (opp_data->soc_data->reg_names)
reg_names = opp_data->soc_data->reg_names;
ti_opp_table = dev_pm_opp_set_regulators(opp_data->cpu_dev,
- reg_names,
- ARRAY_SIZE(default_reg_names));
+ reg_names);
if (IS_ERR(ti_opp_table)) {
dev_pm_opp_put_supported_hw(opp_data->opp_table);
ret = PTR_ERR(ti_opp_table);
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);
diff --git a/drivers/gpu/drm/lima/lima_devfreq.c b/drivers/gpu/drm/lima/lima_devfreq.c
index 8989e215dfc9..dc83c5421125 100644
--- a/drivers/gpu/drm/lima/lima_devfreq.c
+++ b/drivers/gpu/drm/lima/lima_devfreq.c
@@ -111,6 +111,7 @@ int lima_devfreq_init(struct lima_device *ldev)
struct dev_pm_opp *opp;
unsigned long cur_freq;
int ret;
+ const char *regulator_names[] = { "mali", NULL };
if (!device_property_present(dev, "operating-points-v2"))
/* Optional, continue without devfreq */
@@ -122,7 +123,7 @@ int lima_devfreq_init(struct lima_device *ldev)
if (ret)
return ret;
- ret = devm_pm_opp_set_regulators(dev, (const char *[]){ "mali" }, 1);
+ ret = devm_pm_opp_set_regulators(dev, regulator_names);
if (ret) {
/* Continue if the optional regulator is missing */
if (ret != -ENODEV)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index 194af7f607a6..12784f349550 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -91,6 +91,7 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
struct devfreq *devfreq;
struct thermal_cooling_device *cooling;
struct panfrost_devfreq *pfdevfreq = &pfdev->pfdevfreq;
+ const char *supplies[] = { pfdev->comp->supply_names[0], NULL };
if (pfdev->comp->num_supplies > 1) {
/*
@@ -101,8 +102,7 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
return 0;
}
- ret = devm_pm_opp_set_regulators(dev, pfdev->comp->supply_names,
- pfdev->comp->num_supplies);
+ ret = devm_pm_opp_set_regulators(dev, supplies);
if (ret) {
/* Continue if the optional regulator is missing */
if (ret != -ENODEV) {
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index e166bfe5fc90..4e4593957ec5 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -2105,13 +2105,20 @@ 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[],
- unsigned int count)
+ const char * const names[])
{
struct dev_pm_opp_supply *supplies;
+ const char * const *temp = names;
struct opp_table *opp_table;
struct regulator *reg;
- int ret, i;
+ int count = 0, ret, i;
+
+ /* Count number of regulators */
+ while (*temp++)
+ count++;
+
+ if (!count)
+ return ERR_PTR(-EINVAL);
opp_table = _add_opp_table(dev, false);
if (IS_ERR(opp_table))
@@ -2236,12 +2243,11 @@ static void devm_pm_opp_regulators_release(void *data)
* Return: 0 on success and errorno otherwise.
*/
int devm_pm_opp_set_regulators(struct device *dev,
- const char * const names[],
- unsigned int count)
+ const char * const names[])
{
struct opp_table *opp_table;
- opp_table = dev_pm_opp_set_regulators(dev, names, count);
+ opp_table = dev_pm_opp_set_regulators(dev, names);
if (IS_ERR(opp_table))
return PTR_ERR(opp_table);
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 5611d14d3ba2..6a4b8f7e7948 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -1384,7 +1384,7 @@ tegra_pmc_core_pd_opp_to_performance_state(struct generic_pm_domain *genpd,
static int tegra_pmc_core_pd_add(struct tegra_pmc *pmc, struct device_node *np)
{
struct generic_pm_domain *genpd;
- const char *rname = "core";
+ const char *rname[] = { "core", NULL};
int err;
genpd = devm_kzalloc(pmc->dev, sizeof(*genpd), GFP_KERNEL);
@@ -1395,7 +1395,7 @@ static int tegra_pmc_core_pd_add(struct tegra_pmc *pmc, struct device_node *np)
genpd->set_performance_state = tegra_pmc_core_pd_set_performance_state;
genpd->opp_to_performance_state = tegra_pmc_core_pd_opp_to_performance_state;
- err = devm_pm_opp_set_regulators(pmc->dev, &rname, 1);
+ err = devm_pm_opp_set_regulators(pmc->dev, rname);
if (err)
return dev_err_probe(pmc->dev, err,
"failed to set core OPP regulator\n");
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 6708b4ec244d..4c490865d574 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -159,9 +159,9 @@ 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[], unsigned int count);
+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[], unsigned int count);
+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);
@@ -379,7 +379,7 @@ 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[], unsigned int count)
+static inline struct opp_table *dev_pm_opp_set_regulators(struct device *dev, const char * const names[])
{
return ERR_PTR(-EOPNOTSUPP);
}
@@ -387,8 +387,7 @@ static inline struct opp_table *dev_pm_opp_set_regulators(struct device *dev, co
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[],
- unsigned int count)
+ const char * const names[])
{
return -EOPNOTSUPP;
}
--
2.31.1.272.g89b43f80a514
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH V3 02/20] OPP: Make dev_pm_opp_set_regulators() accept NULL terminated list
2022-07-04 12:07 ` [PATCH V3 02/20] OPP: Make dev_pm_opp_set_regulators() accept NULL terminated list Viresh Kumar
@ 2022-07-04 14:35 ` Steven Price
2022-07-05 4:34 ` Viresh Kumar
2022-07-06 8:18 ` [PATCH V3.1 " Viresh Kumar
2022-07-07 19:04 ` [PATCH V3 " Chanwoo Choi
2 siblings, 1 reply; 11+ messages in thread
From: Steven Price @ 2022-07-04 14:35 UTC (permalink / raw)
To: Viresh Kumar, Rafael J. Wysocki, Chanwoo Choi, MyungJoo Ham,
Kyungmin Park, Krzysztof Kozlowski, Alim Akhtar, Qiang Yu,
Rob Herring, Tomeu Vizoso, Alyssa Rosenzweig, Nishanth Menon,
Stephen Boyd, Thierry Reding, Jonathan Hunter
Cc: linux-pm, Vincent Guittot, Greg Kroah-Hartman, linux-kernel,
linux-samsung-soc, linux-arm-kernel, dri-devel, lima, linux-tegra
On 04/07/2022 13:07, Viresh Kumar wrote:
> 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 <gregkh@linuxfoundation.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/cpufreq/cpufreq-dt.c | 9 ++++-----
> drivers/cpufreq/ti-cpufreq.c | 7 +++----
> drivers/devfreq/exynos-bus.c | 4 ++--
> drivers/gpu/drm/lima/lima_devfreq.c | 3 ++-
> drivers/gpu/drm/panfrost/panfrost_devfreq.c | 4 ++--
> drivers/opp/core.c | 18 ++++++++++++------
> drivers/soc/tegra/pmc.c | 4 ++--
> include/linux/pm_opp.h | 9 ++++-----
> 8 files changed, 31 insertions(+), 27 deletions(-)
>
[...]
> diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> index 194af7f607a6..12784f349550 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> @@ -91,6 +91,7 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
> struct devfreq *devfreq;
> struct thermal_cooling_device *cooling;
> struct panfrost_devfreq *pfdevfreq = &pfdev->pfdevfreq;
> + const char *supplies[] = { pfdev->comp->supply_names[0], NULL };
>
> if (pfdev->comp->num_supplies > 1) {
> /*
> @@ -101,8 +102,7 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
> return 0;
> }
>
> - ret = devm_pm_opp_set_regulators(dev, pfdev->comp->supply_names,
> - pfdev->comp->num_supplies);
> + ret = devm_pm_opp_set_regulators(dev, supplies);
> if (ret) {
> /* Continue if the optional regulator is missing */
> if (ret != -ENODEV) {
I have to say the 'new improved' list ending with NULL approach doesn't
work out so well for Panfrost. We already have to have a separate
'num_supplies' variable for devm_regulator_bulk_get() /
regulator_bulk_{en,dis}able(), so the keeping everything in sync
argument is lost here.
I would suggest added the NULL on the end of the lists in panfrost_drv.c
but then it would break the use of ARRAY_SIZE() to automagically keep
the length correct...
For now the approach isn't too bad because Panfrost doesn't yet support
enabling devfreq with more than one supply. But that array isn't going
to work so nicely when that restriction is removed.
The only sane way I can see of handling this in Panfrost would be
replicating the loop to count the supplies in the Panfrost code which
would allow dropping num_supplies from struct panfrost_compatible and
then supply_names in the same struct could be NULL terminated ready for
devm_pm_opp_set_regulators().
Steve
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index e166bfe5fc90..4e4593957ec5 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -2105,13 +2105,20 @@ 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[],
> - unsigned int count)
> + const char * const names[])
> {
> struct dev_pm_opp_supply *supplies;
> + const char * const *temp = names;
> struct opp_table *opp_table;
> struct regulator *reg;
> - int ret, i;
> + int count = 0, ret, i;
> +
> + /* Count number of regulators */
> + while (*temp++)
> + count++;
> +
> + if (!count)
> + return ERR_PTR(-EINVAL);
>
> opp_table = _add_opp_table(dev, false);
> if (IS_ERR(opp_table))
> @@ -2236,12 +2243,11 @@ static void devm_pm_opp_regulators_release(void *data)
> * Return: 0 on success and errorno otherwise.
> */
> int devm_pm_opp_set_regulators(struct device *dev,
> - const char * const names[],
> - unsigned int count)
> + const char * const names[])
> {
> struct opp_table *opp_table;
>
> - opp_table = dev_pm_opp_set_regulators(dev, names, count);
> + opp_table = dev_pm_opp_set_regulators(dev, names);
> if (IS_ERR(opp_table))
> return PTR_ERR(opp_table);
>
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index 5611d14d3ba2..6a4b8f7e7948 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -1384,7 +1384,7 @@ tegra_pmc_core_pd_opp_to_performance_state(struct generic_pm_domain *genpd,
> static int tegra_pmc_core_pd_add(struct tegra_pmc *pmc, struct device_node *np)
> {
> struct generic_pm_domain *genpd;
> - const char *rname = "core";
> + const char *rname[] = { "core", NULL};
> int err;
>
> genpd = devm_kzalloc(pmc->dev, sizeof(*genpd), GFP_KERNEL);
> @@ -1395,7 +1395,7 @@ static int tegra_pmc_core_pd_add(struct tegra_pmc *pmc, struct device_node *np)
> genpd->set_performance_state = tegra_pmc_core_pd_set_performance_state;
> genpd->opp_to_performance_state = tegra_pmc_core_pd_opp_to_performance_state;
>
> - err = devm_pm_opp_set_regulators(pmc->dev, &rname, 1);
> + err = devm_pm_opp_set_regulators(pmc->dev, rname);
> if (err)
> return dev_err_probe(pmc->dev, err,
> "failed to set core OPP regulator\n");
> diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
> index 6708b4ec244d..4c490865d574 100644
> --- a/include/linux/pm_opp.h
> +++ b/include/linux/pm_opp.h
> @@ -159,9 +159,9 @@ 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[], unsigned int count);
> +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[], unsigned int count);
> +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);
> @@ -379,7 +379,7 @@ 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[], unsigned int count)
> +static inline struct opp_table *dev_pm_opp_set_regulators(struct device *dev, const char * const names[])
> {
> return ERR_PTR(-EOPNOTSUPP);
> }
> @@ -387,8 +387,7 @@ static inline struct opp_table *dev_pm_opp_set_regulators(struct device *dev, co
> 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[],
> - unsigned int count)
> + const char * const names[])
> {
> return -EOPNOTSUPP;
> }
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH V3 02/20] OPP: Make dev_pm_opp_set_regulators() accept NULL terminated list
2022-07-04 14:35 ` Steven Price
@ 2022-07-05 4:34 ` Viresh Kumar
2022-07-06 8:09 ` Steven Price
0 siblings, 1 reply; 11+ messages in thread
From: Viresh Kumar @ 2022-07-05 4:34 UTC (permalink / raw)
To: Steven Price
Cc: Rafael J. Wysocki, Chanwoo Choi, MyungJoo Ham, Kyungmin Park,
Krzysztof Kozlowski, Alim Akhtar, Qiang Yu, Rob Herring,
Tomeu Vizoso, Alyssa Rosenzweig, Nishanth Menon, Stephen Boyd,
Thierry Reding, Jonathan Hunter, linux-pm, Vincent Guittot,
Greg Kroah-Hartman, linux-kernel, linux-samsung-soc,
linux-arm-kernel, dri-devel, lima, linux-tegra
On 04-07-22, 15:35, Steven Price wrote:
> I have to say the 'new improved' list ending with NULL approach doesn't
> work out so well for Panfrost. We already have to have a separate
> 'num_supplies' variable for devm_regulator_bulk_get() /
> regulator_bulk_{en,dis}able(), so the keeping everything in sync
> argument is lost here.
>
> I would suggest added the NULL on the end of the lists in panfrost_drv.c
> but then it would break the use of ARRAY_SIZE() to automagically keep
> the length correct...
Actually we can still make it work.
> For now the approach isn't too bad because Panfrost doesn't yet support
> enabling devfreq with more than one supply. But that array isn't going
> to work so nicely when that restriction is removed.
>
> The only sane way I can see of handling this in Panfrost would be
> replicating the loop to count the supplies in the Panfrost code which
> would allow dropping num_supplies from struct panfrost_compatible and
> then supply_names in the same struct could be NULL terminated ready for
> devm_pm_opp_set_regulators().
Or doing this, which will simplify both the cases.
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 7fcbc2a5b6cd..b3b55565b8ef 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -625,24 +625,29 @@ static int panfrost_remove(struct platform_device *pdev)
return 0;
}
-static const char * const default_supplies[] = { "mali" };
+/*
+ * The OPP core wants the supply names to be NULL terminated, but we need the
+ * correct num_supplies value for regulator core. Hence, we NULL terminate here
+ * and then initialize num_supplies with ARRAY_SIZE - 1.
+ */
+static const char * const default_supplies[] = { "mali", NULL };
static const struct panfrost_compatible default_data = {
- .num_supplies = ARRAY_SIZE(default_supplies),
+ .num_supplies = ARRAY_SIZE(default_supplies) - 1,
.supply_names = default_supplies,
.num_pm_domains = 1, /* optional */
.pm_domain_names = NULL,
};
static const struct panfrost_compatible amlogic_data = {
- .num_supplies = ARRAY_SIZE(default_supplies),
+ .num_supplies = ARRAY_SIZE(default_supplies) - 1,
.supply_names = default_supplies,
.vendor_quirk = panfrost_gpu_amlogic_quirk,
};
-static const char * const mediatek_mt8183_supplies[] = { "mali", "sram" };
+static const char * const mediatek_mt8183_supplies[] = { "mali", "sram", NULL };
static const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2" };
static const struct panfrost_compatible mediatek_mt8183_data = {
- .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies),
+ .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies) - 1,
.supply_names = mediatek_mt8183_supplies,
.num_pm_domains = ARRAY_SIZE(mediatek_mt8183_pm_domains),
.pm_domain_names = mediatek_mt8183_pm_domains,
--
viresh
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH V3 02/20] OPP: Make dev_pm_opp_set_regulators() accept NULL terminated list
2022-07-05 4:34 ` Viresh Kumar
@ 2022-07-06 8:09 ` Steven Price
0 siblings, 0 replies; 11+ messages in thread
From: Steven Price @ 2022-07-06 8:09 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael J. Wysocki, Chanwoo Choi, MyungJoo Ham, Kyungmin Park,
Krzysztof Kozlowski, Alim Akhtar, Qiang Yu, Rob Herring,
Tomeu Vizoso, Alyssa Rosenzweig, Nishanth Menon, Stephen Boyd,
Thierry Reding, Jonathan Hunter, linux-pm, Vincent Guittot,
Greg Kroah-Hartman, linux-kernel, linux-samsung-soc,
linux-arm-kernel, dri-devel, lima, linux-tegra
On 05/07/2022 05:34, Viresh Kumar wrote:
> On 04-07-22, 15:35, Steven Price wrote:
>> I have to say the 'new improved' list ending with NULL approach doesn't
>> work out so well for Panfrost. We already have to have a separate
>> 'num_supplies' variable for devm_regulator_bulk_get() /
>> regulator_bulk_{en,dis}able(), so the keeping everything in sync
>> argument is lost here.
>>
>> I would suggest added the NULL on the end of the lists in panfrost_drv.c
>> but then it would break the use of ARRAY_SIZE() to automagically keep
>> the length correct...
>
> Actually we can still make it work.
>
>> For now the approach isn't too bad because Panfrost doesn't yet support
>> enabling devfreq with more than one supply. But that array isn't going
>> to work so nicely when that restriction is removed.
>>
>> The only sane way I can see of handling this in Panfrost would be
>> replicating the loop to count the supplies in the Panfrost code which
>> would allow dropping num_supplies from struct panfrost_compatible and
>> then supply_names in the same struct could be NULL terminated ready for
>> devm_pm_opp_set_regulators().
>
> Or doing this, which will simplify both the cases.
Yes the below works, it's just a bit ugly having the "- 1", and
potentially easy to forgot when adding another. However I don't suppose
it would get far in that case so I think it would be spotted quickly
when adding a new compatible.
It's probably the best solution at the moment.
Thanks,
Steve
>
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index 7fcbc2a5b6cd..b3b55565b8ef 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -625,24 +625,29 @@ static int panfrost_remove(struct platform_device *pdev)
> return 0;
> }
>
> -static const char * const default_supplies[] = { "mali" };
> +/*
> + * The OPP core wants the supply names to be NULL terminated, but we need the
> + * correct num_supplies value for regulator core. Hence, we NULL terminate here
> + * and then initialize num_supplies with ARRAY_SIZE - 1.
> + */
> +static const char * const default_supplies[] = { "mali", NULL };
> static const struct panfrost_compatible default_data = {
> - .num_supplies = ARRAY_SIZE(default_supplies),
> + .num_supplies = ARRAY_SIZE(default_supplies) - 1,
> .supply_names = default_supplies,
> .num_pm_domains = 1, /* optional */
> .pm_domain_names = NULL,
> };
>
> static const struct panfrost_compatible amlogic_data = {
> - .num_supplies = ARRAY_SIZE(default_supplies),
> + .num_supplies = ARRAY_SIZE(default_supplies) - 1,
> .supply_names = default_supplies,
> .vendor_quirk = panfrost_gpu_amlogic_quirk,
> };
>
> -static const char * const mediatek_mt8183_supplies[] = { "mali", "sram" };
> +static const char * const mediatek_mt8183_supplies[] = { "mali", "sram", NULL };
> static const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2" };
> static const struct panfrost_compatible mediatek_mt8183_data = {
> - .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies),
> + .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies) - 1,
> .supply_names = mediatek_mt8183_supplies,
> .num_pm_domains = ARRAY_SIZE(mediatek_mt8183_pm_domains),
> .pm_domain_names = mediatek_mt8183_pm_domains,
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH V3.1 02/20] OPP: Make dev_pm_opp_set_regulators() accept NULL terminated list
2022-07-04 12:07 ` [PATCH V3 02/20] OPP: Make dev_pm_opp_set_regulators() accept NULL terminated list Viresh Kumar
2022-07-04 14:35 ` Steven Price
@ 2022-07-06 8:18 ` Viresh Kumar
2022-07-06 9:50 ` Steven Price
2022-07-07 19:04 ` [PATCH V3 " Chanwoo Choi
2 siblings, 1 reply; 11+ messages in thread
From: Viresh Kumar @ 2022-07-06 8:18 UTC (permalink / raw)
To: Viresh Kumar, Rafael J. Wysocki, Chanwoo Choi, MyungJoo Ham,
Kyungmin Park, Krzysztof Kozlowski, Alim Akhtar, Qiang Yu,
Rob Herring, Tomeu Vizoso, Steven Price, Alyssa Rosenzweig,
Nishanth Menon, Stephen Boyd, Thierry Reding, Jonathan Hunter,
Matthias Brugger
Cc: linux-pm, Vincent Guittot, Greg Kroah-Hartman, linux-kernel,
linux-samsung-soc, linux-arm-kernel, dri-devel, lima, linux-tegra,
linux-mediatek
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 <gregkh@linuxfoundation.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
V3->V3.1:
- Update panfrost_drv.c to include the NULL element.
drivers/cpufreq/cpufreq-dt.c | 9 ++++-----
drivers/cpufreq/ti-cpufreq.c | 7 +++----
drivers/devfreq/exynos-bus.c | 4 ++--
drivers/gpu/drm/lima/lima_devfreq.c | 3 ++-
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 3 +--
drivers/gpu/drm/panfrost/panfrost_drv.c | 15 ++++++++++-----
drivers/opp/core.c | 18 ++++++++++++------
drivers/soc/tegra/pmc.c | 4 ++--
include/linux/pm_opp.h | 9 ++++-----
9 files changed, 40 insertions(+), 32 deletions(-)
diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 8fcaba541539..be0c19b3ffa5 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -193,7 +193,7 @@ static int dt_cpufreq_early_init(struct device *dev, int cpu)
struct private_data *priv;
struct device *cpu_dev;
bool fallback = false;
- const char *reg_name;
+ const char *reg_name[] = { NULL, NULL };
int ret;
/* Check if this CPU is already covered by some other policy */
@@ -218,10 +218,9 @@ static int dt_cpufreq_early_init(struct device *dev, int cpu)
* OPP layer will be taking care of regulators now, but it needs to know
* the name of the regulator first.
*/
- reg_name = find_supply_name(cpu_dev);
- if (reg_name) {
- priv->opp_table = dev_pm_opp_set_regulators(cpu_dev, ®_name,
- 1);
+ 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);
if (ret != -EPROBE_DEFER)
diff --git a/drivers/cpufreq/ti-cpufreq.c b/drivers/cpufreq/ti-cpufreq.c
index 8f9fdd864391..560d67a6bef1 100644
--- a/drivers/cpufreq/ti-cpufreq.c
+++ b/drivers/cpufreq/ti-cpufreq.c
@@ -173,7 +173,7 @@ static struct ti_cpufreq_soc_data omap34xx_soc_data = {
* seems to always read as 0).
*/
-static const char * const omap3_reg_names[] = {"cpu0", "vbb"};
+static const char * const omap3_reg_names[] = {"cpu0", "vbb", NULL};
static struct ti_cpufreq_soc_data omap36xx_soc_data = {
.reg_names = omap3_reg_names,
@@ -326,7 +326,7 @@ static int ti_cpufreq_probe(struct platform_device *pdev)
const struct of_device_id *match;
struct opp_table *ti_opp_table;
struct ti_cpufreq_data *opp_data;
- const char * const default_reg_names[] = {"vdd", "vbb"};
+ const char * const default_reg_names[] = {"vdd", "vbb", NULL};
int ret;
match = dev_get_platdata(&pdev->dev);
@@ -387,8 +387,7 @@ static int ti_cpufreq_probe(struct platform_device *pdev)
if (opp_data->soc_data->reg_names)
reg_names = opp_data->soc_data->reg_names;
ti_opp_table = dev_pm_opp_set_regulators(opp_data->cpu_dev,
- reg_names,
- ARRAY_SIZE(default_reg_names));
+ reg_names);
if (IS_ERR(ti_opp_table)) {
dev_pm_opp_put_supported_hw(opp_data->opp_table);
ret = PTR_ERR(ti_opp_table);
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);
diff --git a/drivers/gpu/drm/lima/lima_devfreq.c b/drivers/gpu/drm/lima/lima_devfreq.c
index 8989e215dfc9..dc83c5421125 100644
--- a/drivers/gpu/drm/lima/lima_devfreq.c
+++ b/drivers/gpu/drm/lima/lima_devfreq.c
@@ -111,6 +111,7 @@ int lima_devfreq_init(struct lima_device *ldev)
struct dev_pm_opp *opp;
unsigned long cur_freq;
int ret;
+ const char *regulator_names[] = { "mali", NULL };
if (!device_property_present(dev, "operating-points-v2"))
/* Optional, continue without devfreq */
@@ -122,7 +123,7 @@ int lima_devfreq_init(struct lima_device *ldev)
if (ret)
return ret;
- ret = devm_pm_opp_set_regulators(dev, (const char *[]){ "mali" }, 1);
+ ret = devm_pm_opp_set_regulators(dev, regulator_names);
if (ret) {
/* Continue if the optional regulator is missing */
if (ret != -ENODEV)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index 194af7f607a6..5110cd9b2425 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -101,8 +101,7 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
return 0;
}
- ret = devm_pm_opp_set_regulators(dev, pfdev->comp->supply_names,
- pfdev->comp->num_supplies);
+ ret = devm_pm_opp_set_regulators(dev, pfdev->comp->supply_names);
if (ret) {
/* Continue if the optional regulator is missing */
if (ret != -ENODEV) {
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 7fcbc2a5b6cd..8a4bef65d38c 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -625,24 +625,29 @@ static int panfrost_remove(struct platform_device *pdev)
return 0;
}
-static const char * const default_supplies[] = { "mali" };
+/*
+ * The OPP core wants the supply names to be NULL terminated, but we need the
+ * correct num_supplies value for regulator core. Hence, we NULL terminate here
+ * and then initialize num_supplies with ARRAY_SIZE - 1.
+ */
+static const char * const default_supplies[] = { "mali", NULL };
static const struct panfrost_compatible default_data = {
- .num_supplies = ARRAY_SIZE(default_supplies),
+ .num_supplies = ARRAY_SIZE(default_supplies) - 1,
.supply_names = default_supplies,
.num_pm_domains = 1, /* optional */
.pm_domain_names = NULL,
};
static const struct panfrost_compatible amlogic_data = {
- .num_supplies = ARRAY_SIZE(default_supplies),
+ .num_supplies = ARRAY_SIZE(default_supplies) - 1,
.supply_names = default_supplies,
.vendor_quirk = panfrost_gpu_amlogic_quirk,
};
-static const char * const mediatek_mt8183_supplies[] = { "mali", "sram" };
+static const char * const mediatek_mt8183_supplies[] = { "mali", "sram", NULL };
static const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2" };
static const struct panfrost_compatible mediatek_mt8183_data = {
- .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies),
+ .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies) - 1,
.supply_names = mediatek_mt8183_supplies,
.num_pm_domains = ARRAY_SIZE(mediatek_mt8183_pm_domains),
.pm_domain_names = mediatek_mt8183_pm_domains,
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index e166bfe5fc90..4e4593957ec5 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -2105,13 +2105,20 @@ 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[],
- unsigned int count)
+ const char * const names[])
{
struct dev_pm_opp_supply *supplies;
+ const char * const *temp = names;
struct opp_table *opp_table;
struct regulator *reg;
- int ret, i;
+ int count = 0, ret, i;
+
+ /* Count number of regulators */
+ while (*temp++)
+ count++;
+
+ if (!count)
+ return ERR_PTR(-EINVAL);
opp_table = _add_opp_table(dev, false);
if (IS_ERR(opp_table))
@@ -2236,12 +2243,11 @@ static void devm_pm_opp_regulators_release(void *data)
* Return: 0 on success and errorno otherwise.
*/
int devm_pm_opp_set_regulators(struct device *dev,
- const char * const names[],
- unsigned int count)
+ const char * const names[])
{
struct opp_table *opp_table;
- opp_table = dev_pm_opp_set_regulators(dev, names, count);
+ opp_table = dev_pm_opp_set_regulators(dev, names);
if (IS_ERR(opp_table))
return PTR_ERR(opp_table);
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 5611d14d3ba2..6a4b8f7e7948 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -1384,7 +1384,7 @@ tegra_pmc_core_pd_opp_to_performance_state(struct generic_pm_domain *genpd,
static int tegra_pmc_core_pd_add(struct tegra_pmc *pmc, struct device_node *np)
{
struct generic_pm_domain *genpd;
- const char *rname = "core";
+ const char *rname[] = { "core", NULL};
int err;
genpd = devm_kzalloc(pmc->dev, sizeof(*genpd), GFP_KERNEL);
@@ -1395,7 +1395,7 @@ static int tegra_pmc_core_pd_add(struct tegra_pmc *pmc, struct device_node *np)
genpd->set_performance_state = tegra_pmc_core_pd_set_performance_state;
genpd->opp_to_performance_state = tegra_pmc_core_pd_opp_to_performance_state;
- err = devm_pm_opp_set_regulators(pmc->dev, &rname, 1);
+ err = devm_pm_opp_set_regulators(pmc->dev, rname);
if (err)
return dev_err_probe(pmc->dev, err,
"failed to set core OPP regulator\n");
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 6708b4ec244d..4c490865d574 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -159,9 +159,9 @@ 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[], unsigned int count);
+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[], unsigned int count);
+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);
@@ -379,7 +379,7 @@ 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[], unsigned int count)
+static inline struct opp_table *dev_pm_opp_set_regulators(struct device *dev, const char * const names[])
{
return ERR_PTR(-EOPNOTSUPP);
}
@@ -387,8 +387,7 @@ static inline struct opp_table *dev_pm_opp_set_regulators(struct device *dev, co
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[],
- unsigned int count)
+ const char * const names[])
{
return -EOPNOTSUPP;
}
--
2.31.1.272.g89b43f80a514
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH V3.1 02/20] OPP: Make dev_pm_opp_set_regulators() accept NULL terminated list
2022-07-06 8:18 ` [PATCH V3.1 " Viresh Kumar
@ 2022-07-06 9:50 ` Steven Price
0 siblings, 0 replies; 11+ messages in thread
From: Steven Price @ 2022-07-06 9:50 UTC (permalink / raw)
To: Viresh Kumar, Rafael J. Wysocki, Chanwoo Choi, MyungJoo Ham,
Kyungmin Park, Krzysztof Kozlowski, Alim Akhtar, Qiang Yu,
Rob Herring, Tomeu Vizoso, Alyssa Rosenzweig, Nishanth Menon,
Stephen Boyd, Thierry Reding, Jonathan Hunter, Matthias Brugger
Cc: linux-pm, Vincent Guittot, Greg Kroah-Hartman, linux-kernel,
linux-samsung-soc, linux-arm-kernel, dri-devel, lima, linux-tegra,
linux-mediatek
On 06/07/2022 09:18, Viresh Kumar wrote:
> 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 <gregkh@linuxfoundation.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> V3->V3.1:
> - Update panfrost_drv.c to include the NULL element.
>
> drivers/cpufreq/cpufreq-dt.c | 9 ++++-----
> drivers/cpufreq/ti-cpufreq.c | 7 +++----
> drivers/devfreq/exynos-bus.c | 4 ++--
> drivers/gpu/drm/lima/lima_devfreq.c | 3 ++-
> drivers/gpu/drm/panfrost/panfrost_devfreq.c | 3 +--
> drivers/gpu/drm/panfrost/panfrost_drv.c | 15 ++++++++++-----
> drivers/opp/core.c | 18 ++++++++++++------
> drivers/soc/tegra/pmc.c | 4 ++--
> include/linux/pm_opp.h | 9 ++++-----
> 9 files changed, 40 insertions(+), 32 deletions(-)
>
[...]
> diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> index 194af7f607a6..5110cd9b2425 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> @@ -101,8 +101,7 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
> return 0;
> }
>
> - ret = devm_pm_opp_set_regulators(dev, pfdev->comp->supply_names,
> - pfdev->comp->num_supplies);
> + ret = devm_pm_opp_set_regulators(dev, pfdev->comp->supply_names);
> if (ret) {
> /* Continue if the optional regulator is missing */
> if (ret != -ENODEV) {
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index 7fcbc2a5b6cd..8a4bef65d38c 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -625,24 +625,29 @@ static int panfrost_remove(struct platform_device *pdev)
> return 0;
> }
>
> -static const char * const default_supplies[] = { "mali" };
> +/*
> + * The OPP core wants the supply names to be NULL terminated, but we need the
> + * correct num_supplies value for regulator core. Hence, we NULL terminate here
> + * and then initialize num_supplies with ARRAY_SIZE - 1.
> + */
> +static const char * const default_supplies[] = { "mali", NULL };
> static const struct panfrost_compatible default_data = {
> - .num_supplies = ARRAY_SIZE(default_supplies),
> + .num_supplies = ARRAY_SIZE(default_supplies) - 1,
> .supply_names = default_supplies,
> .num_pm_domains = 1, /* optional */
> .pm_domain_names = NULL,
> };
>
> static const struct panfrost_compatible amlogic_data = {
> - .num_supplies = ARRAY_SIZE(default_supplies),
> + .num_supplies = ARRAY_SIZE(default_supplies) - 1,
> .supply_names = default_supplies,
> .vendor_quirk = panfrost_gpu_amlogic_quirk,
> };
>
> -static const char * const mediatek_mt8183_supplies[] = { "mali", "sram" };
> +static const char * const mediatek_mt8183_supplies[] = { "mali", "sram", NULL };
> static const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2" };
> static const struct panfrost_compatible mediatek_mt8183_data = {
> - .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies),
> + .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies) - 1,
> .supply_names = mediatek_mt8183_supplies,
> .num_pm_domains = ARRAY_SIZE(mediatek_mt8183_pm_domains),
> .pm_domain_names = mediatek_mt8183_pm_domains,
Reviewed-by: Steven Price <steven.price@arm.com>
Thanks for the rework, much cleaner.
Steve
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V3 02/20] OPP: Make dev_pm_opp_set_regulators() accept NULL terminated list
2022-07-04 12:07 ` [PATCH V3 02/20] OPP: Make dev_pm_opp_set_regulators() accept NULL terminated list Viresh Kumar
2022-07-04 14:35 ` Steven Price
2022-07-06 8:18 ` [PATCH V3.1 " Viresh Kumar
@ 2022-07-07 19:04 ` Chanwoo Choi
2 siblings, 0 replies; 11+ messages in thread
From: Chanwoo Choi @ 2022-07-07 19:04 UTC (permalink / raw)
To: Viresh Kumar, Rafael J. Wysocki, Chanwoo Choi, MyungJoo Ham,
Kyungmin Park, Krzysztof Kozlowski, Alim Akhtar, Qiang Yu,
Rob Herring, Tomeu Vizoso, Steven Price, Alyssa Rosenzweig,
Nishanth Menon, Stephen Boyd, Thierry Reding, Jonathan Hunter
Cc: linux-pm, Vincent Guittot, Greg Kroah-Hartman, linux-kernel,
linux-samsung-soc, linux-arm-kernel, dri-devel, lima, linux-tegra
On 22. 7. 4. 21:07, Viresh Kumar wrote:
> 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 <gregkh@linuxfoundation.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/cpufreq/cpufreq-dt.c | 9 ++++-----
> drivers/cpufreq/ti-cpufreq.c | 7 +++----
> drivers/devfreq/exynos-bus.c | 4 ++--
> drivers/gpu/drm/lima/lima_devfreq.c | 3 ++-
> drivers/gpu/drm/panfrost/panfrost_devfreq.c | 4 ++--
> drivers/opp/core.c | 18 ++++++++++++------
> drivers/soc/tegra/pmc.c | 4 ++--
> include/linux/pm_opp.h | 9 ++++-----
> 8 files changed, 31 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
> index 8fcaba541539..be0c19b3ffa5 100644
> --- a/drivers/cpufreq/cpufreq-dt.c
> +++ b/drivers/cpufreq/cpufreq-dt.c
> @@ -193,7 +193,7 @@ static int dt_cpufreq_early_init(struct device *dev, int cpu)
> struct private_data *priv;
> struct device *cpu_dev;
> bool fallback = false;
> - const char *reg_name;
> + const char *reg_name[] = { NULL, NULL };
> int ret;
>
> /* Check if this CPU is already covered by some other policy */
> @@ -218,10 +218,9 @@ static int dt_cpufreq_early_init(struct device *dev, int cpu)
> * OPP layer will be taking care of regulators now, but it needs to know
> * the name of the regulator first.
> */
> - reg_name = find_supply_name(cpu_dev);
> - if (reg_name) {
> - priv->opp_table = dev_pm_opp_set_regulators(cpu_dev, ®_name,
> - 1);
> + 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);
> if (ret != -EPROBE_DEFER)
> diff --git a/drivers/cpufreq/ti-cpufreq.c b/drivers/cpufreq/ti-cpufreq.c
> index 8f9fdd864391..560d67a6bef1 100644
> --- a/drivers/cpufreq/ti-cpufreq.c
> +++ b/drivers/cpufreq/ti-cpufreq.c
> @@ -173,7 +173,7 @@ static struct ti_cpufreq_soc_data omap34xx_soc_data = {
> * seems to always read as 0).
> */
>
> -static const char * const omap3_reg_names[] = {"cpu0", "vbb"};
> +static const char * const omap3_reg_names[] = {"cpu0", "vbb", NULL};
>
> static struct ti_cpufreq_soc_data omap36xx_soc_data = {
> .reg_names = omap3_reg_names,
> @@ -326,7 +326,7 @@ static int ti_cpufreq_probe(struct platform_device *pdev)
> const struct of_device_id *match;
> struct opp_table *ti_opp_table;
> struct ti_cpufreq_data *opp_data;
> - const char * const default_reg_names[] = {"vdd", "vbb"};
> + const char * const default_reg_names[] = {"vdd", "vbb", NULL};
> int ret;
>
> match = dev_get_platdata(&pdev->dev);
> @@ -387,8 +387,7 @@ static int ti_cpufreq_probe(struct platform_device *pdev)
> if (opp_data->soc_data->reg_names)
> reg_names = opp_data->soc_data->reg_names;
> ti_opp_table = dev_pm_opp_set_regulators(opp_data->cpu_dev,
> - reg_names,
> - ARRAY_SIZE(default_reg_names));
> + reg_names);
> if (IS_ERR(ti_opp_table)) {
> dev_pm_opp_put_supported_hw(opp_data->opp_table);
> ret = PTR_ERR(ti_opp_table);
> 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);
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
(snip)
--
Best Regards,
Samsung Electronics
Chanwoo Choi
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH V3 08/20] soc/tegra: Add comment over devm_pm_opp_set_clkname()
2022-07-04 12:07 [PATCH V3 00/20] OPP: Add new configuration interface: dev_pm_opp_set_config() Viresh Kumar
2022-07-04 12:07 ` [PATCH V3 02/20] OPP: Make dev_pm_opp_set_regulators() accept NULL terminated list Viresh Kumar
@ 2022-07-04 12:07 ` Viresh Kumar
2022-07-04 12:07 ` [PATCH V3 09/20] soc/tegra: Migrate to dev_pm_opp_set_config() Viresh Kumar
2022-07-04 12:07 ` [PATCH V3 11/20] OPP: Migrate set-supported-hw API to use set-config helpers Viresh Kumar
3 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2022-07-04 12:07 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter
Cc: Viresh Kumar, linux-pm, Vincent Guittot, Rafael J. Wysocki,
Stephen Boyd, Nishanth Menon, Dmitry Osipenko, linux-tegra,
linux-kernel
Explain why special handling was required here, it isn't obvious at all.
Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/soc/tegra/common.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c
index 32c346b72635..9f3fdeb1a11c 100644
--- a/drivers/soc/tegra/common.c
+++ b/drivers/soc/tegra/common.c
@@ -108,6 +108,13 @@ int devm_tegra_core_dev_init_opp_table(struct device *dev,
u32 hw_version;
int err;
+ /*
+ * For some devices we don't have any OPP table in the DT, and in order
+ * to use the same code path for all the devices, we create a dummy OPP
+ * table for them via this call. The dummy OPP table is only capable of
+ * doing clk_set_rate() on invocation of dev_pm_opp_set_rate() and
+ * doesn't provide any other functionality.
+ */
err = devm_pm_opp_set_clkname(dev, NULL);
if (err) {
dev_err(dev, "failed to set OPP clk: %d\n", err);
--
2.31.1.272.g89b43f80a514
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH V3 09/20] soc/tegra: Migrate to dev_pm_opp_set_config()
2022-07-04 12:07 [PATCH V3 00/20] OPP: Add new configuration interface: dev_pm_opp_set_config() Viresh Kumar
2022-07-04 12:07 ` [PATCH V3 02/20] OPP: Make dev_pm_opp_set_regulators() accept NULL terminated list Viresh Kumar
2022-07-04 12:07 ` [PATCH V3 08/20] soc/tegra: Add comment over devm_pm_opp_set_clkname() Viresh Kumar
@ 2022-07-04 12:07 ` Viresh Kumar
2022-07-04 12:07 ` [PATCH V3 11/20] OPP: Migrate set-supported-hw API to use set-config helpers Viresh Kumar
3 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2022-07-04 12:07 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter
Cc: Viresh Kumar, linux-pm, Vincent Guittot, Rafael J. Wysocki,
Stephen Boyd, Nishanth Menon, Dmitry Osipenko, linux-tegra,
linux-kernel
The OPP core now provides a unified API for setting all configuration
types, i.e. dev_pm_opp_set_config().
Lets start using it.
Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/soc/tegra/common.c | 52 +++++++++++++++++++++++---------------
1 file changed, 31 insertions(+), 21 deletions(-)
diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c
index 9f3fdeb1a11c..dff6d5ef4e46 100644
--- a/drivers/soc/tegra/common.c
+++ b/drivers/soc/tegra/common.c
@@ -107,36 +107,46 @@ int devm_tegra_core_dev_init_opp_table(struct device *dev,
{
u32 hw_version;
int err;
-
/*
- * For some devices we don't have any OPP table in the DT, and in order
- * to use the same code path for all the devices, we create a dummy OPP
- * table for them via this call. The dummy OPP table is only capable of
- * doing clk_set_rate() on invocation of dev_pm_opp_set_rate() and
- * doesn't provide any other functionality.
+ * The clk's connection id to set is NULL and this is a NULL terminated
+ * array, hence two NULL entries.
*/
- err = devm_pm_opp_set_clkname(dev, NULL);
- if (err) {
- dev_err(dev, "failed to set OPP clk: %d\n", err);
- return err;
- }
-
- /* Tegra114+ doesn't support OPP yet */
- if (!of_machine_is_compatible("nvidia,tegra20") &&
- !of_machine_is_compatible("nvidia,tegra30"))
- return -ENODEV;
-
- if (of_machine_is_compatible("nvidia,tegra20"))
+ const char *clk_names[] = { NULL, NULL };
+ struct dev_pm_opp_config config = {
+ /*
+ * For some devices we don't have any OPP table in the DT, and
+ * in order to use the same code path for all the devices, we
+ * create a dummy OPP table for them via this. The dummy OPP
+ * table is only capable of doing clk_set_rate() on invocation
+ * of dev_pm_opp_set_rate() and doesn't provide any other
+ * functionality.
+ */
+ .clk_names = clk_names,
+ };
+
+ if (of_machine_is_compatible("nvidia,tegra20")) {
hw_version = BIT(tegra_sku_info.soc_process_id);
- else
+ config.supported_hw = &hw_version;
+ config.supported_hw_count = 1;
+ } else if (of_machine_is_compatible("nvidia,tegra30")) {
hw_version = BIT(tegra_sku_info.soc_speedo_id);
+ config.supported_hw = &hw_version;
+ config.supported_hw_count = 1;
+ }
- err = devm_pm_opp_set_supported_hw(dev, &hw_version, 1);
+ err = devm_pm_opp_set_config(dev, &config);
if (err) {
- dev_err(dev, "failed to set OPP supported HW: %d\n", err);
+ dev_err(dev, "failed to set OPP config: %d\n", err);
return err;
}
+ /*
+ * Tegra114+ doesn't support OPP yet, return early for non tegra20/30
+ * case.
+ */
+ if (!config.supported_hw)
+ return -ENODEV;
+
/*
* Older device-trees have an empty OPP table, we will get
* -ENODEV from devm_pm_opp_of_add_table() in this case.
--
2.31.1.272.g89b43f80a514
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH V3 11/20] OPP: Migrate set-supported-hw API to use set-config helpers
2022-07-04 12:07 [PATCH V3 00/20] OPP: Add new configuration interface: dev_pm_opp_set_config() Viresh Kumar
` (2 preceding siblings ...)
2022-07-04 12:07 ` [PATCH V3 09/20] soc/tegra: Migrate to dev_pm_opp_set_config() Viresh Kumar
@ 2022-07-04 12:07 ` Viresh Kumar
3 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2022-07-04 12:07 UTC (permalink / raw)
To: Rafael J. Wysocki, Viresh Kumar, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
Thierry Reding, Jonathan Hunter, Krzysztof Kozlowski,
Nishanth Menon, Stephen Boyd
Cc: linux-pm, Vincent Guittot, linux-arm-kernel, linux-kernel,
linux-tegra
Now that we have a central API to handle all OPP table configurations,
migrate the set-supported-hw 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 <viresh.kumar@linaro.org>
---
drivers/cpufreq/imx-cpufreq-dt.c | 12 ++--
drivers/cpufreq/tegra20-cpufreq.c | 12 ++--
drivers/memory/tegra/tegra124-emc.c | 11 ++--
drivers/opp/core.c | 87 +++++++----------------------
include/linux/pm_opp.h | 49 +++++++++-------
5 files changed, 66 insertions(+), 105 deletions(-)
diff --git a/drivers/cpufreq/imx-cpufreq-dt.c b/drivers/cpufreq/imx-cpufreq-dt.c
index 3fe9125156b4..76e553af2071 100644
--- a/drivers/cpufreq/imx-cpufreq-dt.c
+++ b/drivers/cpufreq/imx-cpufreq-dt.c
@@ -31,8 +31,8 @@
/* cpufreq-dt device registered by imx-cpufreq-dt */
static struct platform_device *cpufreq_dt_pdev;
-static struct opp_table *cpufreq_opp_table;
static struct device *cpu_dev;
+static int cpufreq_opp_token;
enum IMX7ULP_CPUFREQ_CLKS {
ARM,
@@ -153,9 +153,9 @@ static int imx_cpufreq_dt_probe(struct platform_device *pdev)
dev_info(&pdev->dev, "cpu speed grade %d mkt segment %d supported-hw %#x %#x\n",
speed_grade, mkt_segment, supported_hw[0], supported_hw[1]);
- cpufreq_opp_table = dev_pm_opp_set_supported_hw(cpu_dev, supported_hw, 2);
- if (IS_ERR(cpufreq_opp_table)) {
- ret = PTR_ERR(cpufreq_opp_table);
+ cpufreq_opp_token = dev_pm_opp_set_supported_hw(cpu_dev, supported_hw, 2);
+ if (cpufreq_opp_token < 0) {
+ ret = cpufreq_opp_token;
dev_err(&pdev->dev, "Failed to set supported opp: %d\n", ret);
return ret;
}
@@ -163,7 +163,7 @@ static int imx_cpufreq_dt_probe(struct platform_device *pdev)
cpufreq_dt_pdev = platform_device_register_data(
&pdev->dev, "cpufreq-dt", -1, NULL, 0);
if (IS_ERR(cpufreq_dt_pdev)) {
- dev_pm_opp_put_supported_hw(cpufreq_opp_table);
+ dev_pm_opp_put_supported_hw(cpufreq_opp_token);
ret = PTR_ERR(cpufreq_dt_pdev);
dev_err(&pdev->dev, "Failed to register cpufreq-dt: %d\n", ret);
return ret;
@@ -176,7 +176,7 @@ static int imx_cpufreq_dt_remove(struct platform_device *pdev)
{
platform_device_unregister(cpufreq_dt_pdev);
if (!of_machine_is_compatible("fsl,imx7ulp"))
- dev_pm_opp_put_supported_hw(cpufreq_opp_table);
+ dev_pm_opp_put_supported_hw(cpufreq_opp_token);
else
clk_bulk_put(ARRAY_SIZE(imx7ulp_clks), imx7ulp_clks);
diff --git a/drivers/cpufreq/tegra20-cpufreq.c b/drivers/cpufreq/tegra20-cpufreq.c
index e8db3d75be25..ab7ac7df9e62 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -32,9 +32,9 @@ static bool cpu0_node_has_opp_v2_prop(void)
return ret;
}
-static void tegra20_cpufreq_put_supported_hw(void *opp_table)
+static void tegra20_cpufreq_put_supported_hw(void *opp_token)
{
- dev_pm_opp_put_supported_hw(opp_table);
+ dev_pm_opp_put_supported_hw((unsigned long) opp_token);
}
static void tegra20_cpufreq_dt_unregister(void *cpufreq_dt)
@@ -45,7 +45,6 @@ static void tegra20_cpufreq_dt_unregister(void *cpufreq_dt)
static int tegra20_cpufreq_probe(struct platform_device *pdev)
{
struct platform_device *cpufreq_dt;
- struct opp_table *opp_table;
struct device *cpu_dev;
u32 versions[2];
int err;
@@ -71,16 +70,15 @@ static int tegra20_cpufreq_probe(struct platform_device *pdev)
if (WARN_ON(!cpu_dev))
return -ENODEV;
- opp_table = dev_pm_opp_set_supported_hw(cpu_dev, versions, 2);
- err = PTR_ERR_OR_ZERO(opp_table);
- if (err) {
+ err = dev_pm_opp_set_supported_hw(cpu_dev, versions, 2);
+ if (err < 0) {
dev_err(&pdev->dev, "failed to set supported hw: %d\n", err);
return err;
}
err = devm_add_action_or_reset(&pdev->dev,
tegra20_cpufreq_put_supported_hw,
- opp_table);
+ (void *)((unsigned long) err));
if (err)
return err;
diff --git a/drivers/memory/tegra/tegra124-emc.c b/drivers/memory/tegra/tegra124-emc.c
index 908f8d5392b2..85bc936c02f9 100644
--- a/drivers/memory/tegra/tegra124-emc.c
+++ b/drivers/memory/tegra/tegra124-emc.c
@@ -1395,15 +1395,14 @@ static int tegra_emc_interconnect_init(struct tegra_emc *emc)
static int tegra_emc_opp_table_init(struct tegra_emc *emc)
{
u32 hw_version = BIT(tegra_sku_info.soc_speedo_id);
- struct opp_table *hw_opp_table;
- int err;
+ int opp_token, err;
- hw_opp_table = dev_pm_opp_set_supported_hw(emc->dev, &hw_version, 1);
- err = PTR_ERR_OR_ZERO(hw_opp_table);
- if (err) {
+ err = dev_pm_opp_set_supported_hw(emc->dev, &hw_version, 1);
+ if (err < 0) {
dev_err(emc->dev, "failed to set OPP supported HW: %d\n", err);
return err;
}
+ opp_token = err;
err = dev_pm_opp_of_add_table(emc->dev);
if (err) {
@@ -1430,7 +1429,7 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc)
remove_table:
dev_pm_opp_of_remove_table(emc->dev);
put_hw_table:
- dev_pm_opp_put_supported_hw(hw_opp_table);
+ dev_pm_opp_put_supported_hw(opp_token);
return err;
}
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 6ff9b5b69d07..8dbdfff38973 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1952,7 +1952,7 @@ int _opp_add_v1(struct opp_table *opp_table, struct device *dev,
}
/**
- * dev_pm_opp_set_supported_hw() - Set supported platforms
+ * _opp_set_supported_hw() - Set supported platforms
* @dev: Device for which supported-hw has to be set.
* @versions: Array of hierarchy of versions to match.
* @count: Number of elements in the array.
@@ -1962,84 +1962,39 @@ int _opp_add_v1(struct opp_table *opp_table, struct device *dev,
* OPPs, which are available for those versions, based on its 'opp-supported-hw'
* property.
*/
-struct opp_table *dev_pm_opp_set_supported_hw(struct device *dev,
- const u32 *versions, unsigned int count)
+static int _opp_set_supported_hw(struct opp_table *opp_table,
+ const u32 *versions, unsigned int count)
{
- struct opp_table *opp_table;
-
- opp_table = _add_opp_table(dev, false);
- if (IS_ERR(opp_table))
- return opp_table;
-
- /* Make sure there are no concurrent readers while updating opp_table */
- WARN_ON(!list_empty(&opp_table->opp_list));
-
/* Another CPU that shares the OPP table has set the property ? */
if (opp_table->supported_hw)
- return opp_table;
+ return 0;
opp_table->supported_hw = kmemdup(versions, count * sizeof(*versions),
GFP_KERNEL);
- if (!opp_table->supported_hw) {
- dev_pm_opp_put_opp_table(opp_table);
- return ERR_PTR(-ENOMEM);
- }
+ if (!opp_table->supported_hw)
+ return -ENOMEM;
opp_table->supported_hw_count = count;
- return opp_table;
+ return 0;
}
-EXPORT_SYMBOL_GPL(dev_pm_opp_set_supported_hw);
/**
- * dev_pm_opp_put_supported_hw() - Releases resources blocked for supported hw
- * @opp_table: OPP table returned by dev_pm_opp_set_supported_hw().
+ * _opp_put_supported_hw() - Releases resources blocked for supported hw
+ * @opp_table: OPP table returned by _opp_set_supported_hw().
*
* This is required only for the V2 bindings, and is called for a matching
- * dev_pm_opp_set_supported_hw(). Until this is called, the opp_table structure
+ * _opp_set_supported_hw(). Until this is called, the opp_table structure
* will not be freed.
*/
-void dev_pm_opp_put_supported_hw(struct opp_table *opp_table)
+static void _opp_put_supported_hw(struct opp_table *opp_table)
{
- if (unlikely(!opp_table))
- return;
-
- kfree(opp_table->supported_hw);
- opp_table->supported_hw = NULL;
- opp_table->supported_hw_count = 0;
-
- dev_pm_opp_put_opp_table(opp_table);
-}
-EXPORT_SYMBOL_GPL(dev_pm_opp_put_supported_hw);
-
-static void devm_pm_opp_supported_hw_release(void *data)
-{
- dev_pm_opp_put_supported_hw(data);
-}
-
-/**
- * devm_pm_opp_set_supported_hw() - Set supported platforms
- * @dev: Device for which supported-hw has to be set.
- * @versions: Array of hierarchy of versions to match.
- * @count: Number of elements in the array.
- *
- * This is a resource-managed variant of dev_pm_opp_set_supported_hw().
- *
- * Return: 0 on success and errorno otherwise.
- */
-int devm_pm_opp_set_supported_hw(struct device *dev, const u32 *versions,
- unsigned int count)
-{
- struct opp_table *opp_table;
-
- opp_table = dev_pm_opp_set_supported_hw(dev, versions, count);
- if (IS_ERR(opp_table))
- return PTR_ERR(opp_table);
-
- return devm_add_action_or_reset(dev, devm_pm_opp_supported_hw_release,
- opp_table);
+ if (opp_table->supported_hw) {
+ kfree(opp_table->supported_hw);
+ opp_table->supported_hw = NULL;
+ opp_table->supported_hw_count = 0;
+ }
}
-EXPORT_SYMBOL_GPL(devm_pm_opp_set_supported_hw);
/**
* dev_pm_opp_set_prop_name() - Set prop-extn name
@@ -2583,7 +2538,7 @@ static void _opp_clear_config(struct opp_config_data *data)
if (data->flags & OPP_CONFIG_REGULATOR)
_opp_put_regulators(data->opp_table);
if (data->flags & OPP_CONFIG_SUPPORTED_HW)
- dev_pm_opp_put_supported_hw(data->opp_table);
+ _opp_put_supported_hw(data->opp_table);
if (data->flags & OPP_CONFIG_REGULATOR_HELPER)
dev_pm_opp_unregister_set_opp_helper(data->opp_table);
if (data->flags & OPP_CONFIG_PROP_NAME)
@@ -2694,12 +2649,10 @@ int dev_pm_opp_set_config(struct device *dev, struct dev_pm_opp_config *config)
/* Configure supported hardware */
if (config->supported_hw) {
- err = dev_pm_opp_set_supported_hw(dev, config->supported_hw,
- config->supported_hw_count);
- if (IS_ERR(err)) {
- ret = PTR_ERR(err);
+ ret = _opp_set_supported_hw(opp_table, config->supported_hw,
+ config->supported_hw_count);
+ if (ret)
goto err;
- }
data->flags |= OPP_CONFIG_SUPPORTED_HW;
}
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index f014bd172c99..94d0101c254c 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -184,9 +184,6 @@ int dev_pm_opp_set_config(struct device *dev, struct dev_pm_opp_config *config);
int devm_pm_opp_set_config(struct device *dev, struct dev_pm_opp_config *config);
void dev_pm_opp_clear_config(int token);
-struct opp_table *dev_pm_opp_set_supported_hw(struct device *dev, const u32 *versions, unsigned int count);
-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_clkname(struct device *dev, const char *name);
@@ -369,22 +366,6 @@ static inline int dev_pm_opp_unregister_notifier(struct device *dev, struct noti
return -EOPNOTSUPP;
}
-static inline struct opp_table *dev_pm_opp_set_supported_hw(struct device *dev,
- const u32 *versions,
- unsigned int count)
-{
- return ERR_PTR(-EOPNOTSUPP);
-}
-
-static inline void dev_pm_opp_put_supported_hw(struct opp_table *opp_table) {}
-
-static inline int devm_pm_opp_set_supported_hw(struct device *dev,
- const u32 *versions,
- unsigned int count)
-{
- return -EOPNOTSUPP;
-}
-
static inline struct opp_table *dev_pm_opp_register_set_opp_helper(struct device *dev,
int (*set_opp)(struct dev_pm_set_opp_data *data))
{
@@ -618,4 +599,34 @@ static inline int devm_pm_opp_set_regulators(struct device *dev,
return devm_pm_opp_set_config(dev, &config);
}
+/* Supported-hw helpers */
+static inline int dev_pm_opp_set_supported_hw(struct device *dev,
+ const u32 *versions,
+ unsigned int count)
+{
+ struct dev_pm_opp_config config = {
+ .supported_hw = versions,
+ .supported_hw_count = count,
+ };
+
+ return dev_pm_opp_set_config(dev, &config);
+}
+
+static inline void dev_pm_opp_put_supported_hw(int token)
+{
+ dev_pm_opp_clear_config(token);
+}
+
+static inline int devm_pm_opp_set_supported_hw(struct device *dev,
+ const u32 *versions,
+ unsigned int count)
+{
+ struct dev_pm_opp_config config = {
+ .supported_hw = versions,
+ .supported_hw_count = count,
+ };
+
+ return devm_pm_opp_set_config(dev, &config);
+}
+
#endif /* __LINUX_OPP_H__ */
--
2.31.1.272.g89b43f80a514
^ permalink raw reply related [flat|nested] 11+ messages in thread