Skip to content

Commit 6d366d0

Browse files
committed
OPP: Use _set_opp_level() for single genpd case
There are two genpd (as required-opp) cases that we need to handle, devices with a single genpd and ones with multiple genpds. The multiple genpds case is clear, where the OPP core calls dev_pm_domain_attach_by_name() for them and uses the virtual devices returned by this helper to call dev_pm_domain_set_performance_state() later to change the performance state. The single genpd case however requires special handling as we need to use the same `dev` structure (instead of a virtual one provided by genpd core) for setting the performance state via dev_pm_domain_set_performance_state(). As we move towards more generic code to take care of the required OPPs, where we will recursively call dev_pm_opp_set_opp() for all the required OPPs, the above special case becomes a problem. It doesn't make sense for a device's DT entry to have both "opp-level" and single "required-opps" entry pointing to a genpd's OPP, as that would make the OPP core call dev_pm_domain_set_performance_state() for two different values for the same device structure. And so we can reuse the 'opp->level" field in such a case and call _set_opp_level() for the device. Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Tested-by: Stephan Gerhold <stephan.gerhold@kernkonzept.com> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
1 parent 073d3d2 commit 6d366d0

File tree

2 files changed

+32
-5
lines changed

2 files changed

+32
-5
lines changed

drivers/opp/core.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,10 +1088,12 @@ static int _opp_set_required_opps_generic(struct device *dev,
10881088
static int _opp_set_required_opps_genpd(struct device *dev,
10891089
struct opp_table *opp_table, struct dev_pm_opp *opp, bool scaling_down)
10901090
{
1091-
struct device **genpd_virt_devs =
1092-
opp_table->genpd_virt_devs ? opp_table->genpd_virt_devs : &dev;
1091+
struct device **genpd_virt_devs = opp_table->genpd_virt_devs;
10931092
int index, target, delta, ret;
10941093

1094+
if (!genpd_virt_devs)
1095+
return 0;
1096+
10951097
/* Scaling up? Set required OPPs in normal order, else reverse */
10961098
if (!scaling_down) {
10971099
index = 0;

drivers/opp/of.c

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ void _of_clear_opp(struct opp_table *opp_table, struct dev_pm_opp *opp)
296296
of_node_put(opp->np);
297297
}
298298

299-
static int _link_required_opps(struct dev_pm_opp *opp,
299+
static int _link_required_opps(struct dev_pm_opp *opp, struct opp_table *opp_table,
300300
struct opp_table *required_table, int index)
301301
{
302302
struct device_node *np;
@@ -314,6 +314,31 @@ static int _link_required_opps(struct dev_pm_opp *opp,
314314
return -ENODEV;
315315
}
316316

317+
/*
318+
* There are two genpd (as required-opp) cases that we need to handle,
319+
* devices with a single genpd and ones with multiple genpds.
320+
*
321+
* The single genpd case requires special handling as we need to use the
322+
* same `dev` structure (instead of a virtual one provided by genpd
323+
* core) for setting the performance state.
324+
*
325+
* It doesn't make sense for a device's DT entry to have both
326+
* "opp-level" and single "required-opps" entry pointing to a genpd's
327+
* OPP, as that would make the OPP core call
328+
* dev_pm_domain_set_performance_state() for two different values for
329+
* the same device structure. Lets treat single genpd configuration as a
330+
* case where the OPP's level is directly available without required-opp
331+
* link in the DT.
332+
*
333+
* Just update the `level` with the right value, which
334+
* dev_pm_opp_set_opp() will take care of in the normal path itself.
335+
*/
336+
if (required_table->is_genpd && opp_table->required_opp_count == 1 &&
337+
!opp_table->genpd_virt_devs) {
338+
if (!WARN_ON(opp->level != OPP_LEVEL_UNSET))
339+
opp->level = opp->required_opps[0]->level;
340+
}
341+
317342
return 0;
318343
}
319344

@@ -338,7 +363,7 @@ static int _of_opp_alloc_required_opps(struct opp_table *opp_table,
338363
if (IS_ERR_OR_NULL(required_table))
339364
continue;
340365

341-
ret = _link_required_opps(opp, required_table, i);
366+
ret = _link_required_opps(opp, opp_table, required_table, i);
342367
if (ret)
343368
goto free_required_opps;
344369
}
@@ -359,7 +384,7 @@ static int lazy_link_required_opps(struct opp_table *opp_table,
359384
int ret;
360385

361386
list_for_each_entry(opp, &opp_table->opp_list, node) {
362-
ret = _link_required_opps(opp, new_table, index);
387+
ret = _link_required_opps(opp, opp_table, new_table, index);
363388
if (ret)
364389
return ret;
365390
}

0 commit comments

Comments
 (0)