Commit dd461cd9 authored by Stephan Gerhold's avatar Stephan Gerhold Committed by Viresh Kumar

opp: Allow dev_pm_opp_get_opp_table() to return -EPROBE_DEFER

The OPP core manages various resources, e.g. clocks or interconnect paths.
These resources are looked up when the OPP table is allocated once
dev_pm_opp_get_opp_table() is called the first time (either directly
or indirectly through one of the many helper functions).

At this point, the resources may not be available yet, i.e. looking them
up will result in -EPROBE_DEFER. Unfortunately, dev_pm_opp_get_opp_table()
is currently unable to propagate this error code since it only returns
the allocated OPP table or NULL.

This means that all consumers of the OPP core are required to make sure
that all necessary resources are available. Usually this happens by
requesting them, checking the result and releasing them immediately after.

For example, we have added "dev_pm_opp_of_find_icc_paths(dev, NULL)" to
several drivers now just to make sure the interconnect providers are
ready before the OPP table is allocated. If this call is missing,
the OPP core will only warn about this and then attempt to continue
without interconnect. This will eventually fail horribly, e.g.:

    cpu cpu0: _allocate_opp_table: Error finding interconnect paths: -517
    ... later ...
    of: _read_bw: Mismatch between opp-peak-kBps and paths (1 0)
    cpu cpu0: _opp_add_static_v2: opp key field not found
    cpu cpu0: _of_add_opp_table_v2: Failed to add OPP, -22

This example happens when trying to use interconnects for a CPU OPP
table together with qcom-cpufreq-nvmem.c. qcom-cpufreq-nvmem calls
dev_pm_opp_set_supported_hw(), which ends up allocating the OPP table
early. To fix the problem with the current approach we would need to add
yet another call to dev_pm_opp_of_find_icc_paths(dev, NULL).
But actually qcom-cpufreq-nvmem.c has nothing to do with interconnects...

This commit attempts to make this more robust by allowing
dev_pm_opp_get_opp_table() to return an error pointer. Fixing all
the usages is trivial because the function is usually used indirectly
through another helper (e.g. dev_pm_opp_set_supported_hw() above).
These other helpers already return an error pointer.

The example above then works correctly because set_supported_hw() will
return -EPROBE_DEFER, and qcom-cpufreq-nvmem.c already propagates that
error. It should also be possible to remove the remaining usages of
"dev_pm_opp_of_find_icc_paths(dev, NULL)" from other drivers as well.

Note that this commit currently only handles -EPROBE_DEFER for the
clock/interconnects within _allocate_opp_table(). Other errors are just
ignored as before. Eventually those should be propagated as well.
Signed-off-by: default avatarStephan Gerhold <stephan@gerhold.net>
Acked-by: default avatarKrzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: default avatarUlf Hansson <ulf.hansson@linaro.org>
[ Viresh: skip checking return value of dev_pm_opp_get_opp_table() for
	  EPROBE_DEFER in domain.c, fix NULL return value and reorder
	  code a bit in core.c, and update exynos-asv.c ]
Signed-off-by: default avatarViresh Kumar <viresh.kumar@linaro.org>
parent 9123e3a7
...@@ -2044,6 +2044,7 @@ int of_genpd_add_provider_simple(struct device_node *np, ...@@ -2044,6 +2044,7 @@ int of_genpd_add_provider_simple(struct device_node *np,
if (genpd->set_performance_state) { if (genpd->set_performance_state) {
ret = dev_pm_opp_of_add_table(&genpd->dev); ret = dev_pm_opp_of_add_table(&genpd->dev);
if (ret) { if (ret) {
if (ret != -EPROBE_DEFER)
dev_err(&genpd->dev, "Failed to add OPP table: %d\n", dev_err(&genpd->dev, "Failed to add OPP table: %d\n",
ret); ret);
goto unlock; goto unlock;
...@@ -2054,7 +2055,7 @@ int of_genpd_add_provider_simple(struct device_node *np, ...@@ -2054,7 +2055,7 @@ int of_genpd_add_provider_simple(struct device_node *np,
* state. * state.
*/ */
genpd->opp_table = dev_pm_opp_get_opp_table(&genpd->dev); genpd->opp_table = dev_pm_opp_get_opp_table(&genpd->dev);
WARN_ON(!genpd->opp_table); WARN_ON(IS_ERR(genpd->opp_table));
} }
ret = genpd_add_provider(np, genpd_xlate_simple, genpd); ret = genpd_add_provider(np, genpd_xlate_simple, genpd);
...@@ -2111,6 +2112,7 @@ int of_genpd_add_provider_onecell(struct device_node *np, ...@@ -2111,6 +2112,7 @@ int of_genpd_add_provider_onecell(struct device_node *np,
if (genpd->set_performance_state) { if (genpd->set_performance_state) {
ret = dev_pm_opp_of_add_table_indexed(&genpd->dev, i); ret = dev_pm_opp_of_add_table_indexed(&genpd->dev, i);
if (ret) { if (ret) {
if (ret != -EPROBE_DEFER)
dev_err(&genpd->dev, "Failed to add OPP table for index %d: %d\n", dev_err(&genpd->dev, "Failed to add OPP table for index %d: %d\n",
i, ret); i, ret);
goto error; goto error;
...@@ -2121,7 +2123,7 @@ int of_genpd_add_provider_onecell(struct device_node *np, ...@@ -2121,7 +2123,7 @@ int of_genpd_add_provider_onecell(struct device_node *np,
* performance state. * performance state.
*/ */
genpd->opp_table = dev_pm_opp_get_opp_table_indexed(&genpd->dev, i); genpd->opp_table = dev_pm_opp_get_opp_table_indexed(&genpd->dev, i);
WARN_ON(!genpd->opp_table); WARN_ON(IS_ERR(genpd->opp_table));
} }
genpd->provider = &np->fwnode; genpd->provider = &np->fwnode;
......
...@@ -1063,7 +1063,7 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index) ...@@ -1063,7 +1063,7 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index)
*/ */
opp_table = kzalloc(sizeof(*opp_table), GFP_KERNEL); opp_table = kzalloc(sizeof(*opp_table), GFP_KERNEL);
if (!opp_table) if (!opp_table)
return NULL; return ERR_PTR(-ENOMEM);
mutex_init(&opp_table->lock); mutex_init(&opp_table->lock);
mutex_init(&opp_table->genpd_virt_dev_lock); mutex_init(&opp_table->genpd_virt_dev_lock);
...@@ -1074,8 +1074,8 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index) ...@@ -1074,8 +1074,8 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index)
opp_dev = _add_opp_dev(dev, opp_table); opp_dev = _add_opp_dev(dev, opp_table);
if (!opp_dev) { if (!opp_dev) {
kfree(opp_table); ret = -ENOMEM;
return NULL; goto err;
} }
_of_init_opp_table(opp_table, dev, index); _of_init_opp_table(opp_table, dev, index);
...@@ -1084,16 +1084,21 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index) ...@@ -1084,16 +1084,21 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index)
opp_table->clk = clk_get(dev, NULL); opp_table->clk = clk_get(dev, NULL);
if (IS_ERR(opp_table->clk)) { if (IS_ERR(opp_table->clk)) {
ret = PTR_ERR(opp_table->clk); ret = PTR_ERR(opp_table->clk);
if (ret != -EPROBE_DEFER) if (ret == -EPROBE_DEFER)
dev_dbg(dev, "%s: Couldn't find clock: %d\n", __func__, goto err;
ret);
dev_dbg(dev, "%s: Couldn't find clock: %d\n", __func__, ret);
} }
/* Find interconnect path(s) for the device */ /* Find interconnect path(s) for the device */
ret = dev_pm_opp_of_find_icc_paths(dev, opp_table); ret = dev_pm_opp_of_find_icc_paths(dev, opp_table);
if (ret) if (ret) {
if (ret == -EPROBE_DEFER)
goto err;
dev_warn(dev, "%s: Error finding interconnect paths: %d\n", dev_warn(dev, "%s: Error finding interconnect paths: %d\n",
__func__, ret); __func__, ret);
}
BLOCKING_INIT_NOTIFIER_HEAD(&opp_table->head); BLOCKING_INIT_NOTIFIER_HEAD(&opp_table->head);
INIT_LIST_HEAD(&opp_table->opp_list); INIT_LIST_HEAD(&opp_table->opp_list);
...@@ -1102,6 +1107,10 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index) ...@@ -1102,6 +1107,10 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index)
/* Secure the device table modification */ /* Secure the device table modification */
list_add(&opp_table->node, &opp_tables); list_add(&opp_table->node, &opp_tables);
return opp_table; return opp_table;
err:
kfree(opp_table);
return ERR_PTR(ret);
} }
void _get_opp_table_kref(struct opp_table *opp_table) void _get_opp_table_kref(struct opp_table *opp_table)
...@@ -1124,7 +1133,7 @@ static struct opp_table *_opp_get_opp_table(struct device *dev, int index) ...@@ -1124,7 +1133,7 @@ static struct opp_table *_opp_get_opp_table(struct device *dev, int index)
if (opp_table) { if (opp_table) {
if (!_add_opp_dev_unlocked(dev, opp_table)) { if (!_add_opp_dev_unlocked(dev, opp_table)) {
dev_pm_opp_put_opp_table(opp_table); dev_pm_opp_put_opp_table(opp_table);
opp_table = NULL; opp_table = ERR_PTR(-ENOMEM);
} }
goto unlock; goto unlock;
} }
...@@ -1568,8 +1577,8 @@ struct opp_table *dev_pm_opp_set_supported_hw(struct device *dev, ...@@ -1568,8 +1577,8 @@ struct opp_table *dev_pm_opp_set_supported_hw(struct device *dev,
struct opp_table *opp_table; struct opp_table *opp_table;
opp_table = dev_pm_opp_get_opp_table(dev); opp_table = dev_pm_opp_get_opp_table(dev);
if (!opp_table) if (IS_ERR(opp_table))
return ERR_PTR(-ENOMEM); return opp_table;
/* Make sure there are no concurrent readers while updating opp_table */ /* Make sure there are no concurrent readers while updating opp_table */
WARN_ON(!list_empty(&opp_table->opp_list)); WARN_ON(!list_empty(&opp_table->opp_list));
...@@ -1627,8 +1636,8 @@ struct opp_table *dev_pm_opp_set_prop_name(struct device *dev, const char *name) ...@@ -1627,8 +1636,8 @@ struct opp_table *dev_pm_opp_set_prop_name(struct device *dev, const char *name)
struct opp_table *opp_table; struct opp_table *opp_table;
opp_table = dev_pm_opp_get_opp_table(dev); opp_table = dev_pm_opp_get_opp_table(dev);
if (!opp_table) if (IS_ERR(opp_table))
return ERR_PTR(-ENOMEM); return opp_table;
/* Make sure there are no concurrent readers while updating opp_table */ /* Make sure there are no concurrent readers while updating opp_table */
WARN_ON(!list_empty(&opp_table->opp_list)); WARN_ON(!list_empty(&opp_table->opp_list));
...@@ -1720,8 +1729,8 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev, ...@@ -1720,8 +1729,8 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev,
int ret, i; int ret, i;
opp_table = dev_pm_opp_get_opp_table(dev); opp_table = dev_pm_opp_get_opp_table(dev);
if (!opp_table) if (IS_ERR(opp_table))
return ERR_PTR(-ENOMEM); return opp_table;
/* This should be called before OPPs are initialized */ /* This should be called before OPPs are initialized */
if (WARN_ON(!list_empty(&opp_table->opp_list))) { if (WARN_ON(!list_empty(&opp_table->opp_list))) {
...@@ -1830,8 +1839,8 @@ struct opp_table *dev_pm_opp_set_clkname(struct device *dev, const char *name) ...@@ -1830,8 +1839,8 @@ struct opp_table *dev_pm_opp_set_clkname(struct device *dev, const char *name)
int ret; int ret;
opp_table = dev_pm_opp_get_opp_table(dev); opp_table = dev_pm_opp_get_opp_table(dev);
if (!opp_table) if (IS_ERR(opp_table))
return ERR_PTR(-ENOMEM); return opp_table;
/* This should be called before OPPs are initialized */ /* This should be called before OPPs are initialized */
if (WARN_ON(!list_empty(&opp_table->opp_list))) { if (WARN_ON(!list_empty(&opp_table->opp_list))) {
...@@ -1898,8 +1907,8 @@ struct opp_table *dev_pm_opp_register_set_opp_helper(struct device *dev, ...@@ -1898,8 +1907,8 @@ struct opp_table *dev_pm_opp_register_set_opp_helper(struct device *dev,
return ERR_PTR(-EINVAL); return ERR_PTR(-EINVAL);
opp_table = dev_pm_opp_get_opp_table(dev); opp_table = dev_pm_opp_get_opp_table(dev);
if (!opp_table) if (!IS_ERR(opp_table))
return ERR_PTR(-ENOMEM); return opp_table;
/* This should be called before OPPs are initialized */ /* This should be called before OPPs are initialized */
if (WARN_ON(!list_empty(&opp_table->opp_list))) { if (WARN_ON(!list_empty(&opp_table->opp_list))) {
...@@ -1979,8 +1988,8 @@ struct opp_table *dev_pm_opp_attach_genpd(struct device *dev, ...@@ -1979,8 +1988,8 @@ struct opp_table *dev_pm_opp_attach_genpd(struct device *dev,
const char **name = names; const char **name = names;
opp_table = dev_pm_opp_get_opp_table(dev); opp_table = dev_pm_opp_get_opp_table(dev);
if (!opp_table) if (IS_ERR(opp_table))
return ERR_PTR(-ENOMEM); return opp_table;
/* /*
* If the genpd's OPP table isn't already initialized, parsing of the * If the genpd's OPP table isn't already initialized, parsing of the
...@@ -2150,8 +2159,8 @@ int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt) ...@@ -2150,8 +2159,8 @@ int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
int ret; int ret;
opp_table = dev_pm_opp_get_opp_table(dev); opp_table = dev_pm_opp_get_opp_table(dev);
if (!opp_table) if (IS_ERR(opp_table))
return -ENOMEM; return PTR_ERR(opp_table);
/* Fix regulator count for dynamic OPPs */ /* Fix regulator count for dynamic OPPs */
opp_table->regulator_count = 1; opp_table->regulator_count = 1;
......
...@@ -947,8 +947,8 @@ int dev_pm_opp_of_add_table(struct device *dev) ...@@ -947,8 +947,8 @@ int dev_pm_opp_of_add_table(struct device *dev)
int ret; int ret;
opp_table = dev_pm_opp_get_opp_table_indexed(dev, 0); opp_table = dev_pm_opp_get_opp_table_indexed(dev, 0);
if (!opp_table) if (IS_ERR(opp_table))
return -ENOMEM; return PTR_ERR(opp_table);
/* /*
* OPPs have two version of bindings now. Also try the old (v1) * OPPs have two version of bindings now. Also try the old (v1)
...@@ -1002,8 +1002,8 @@ int dev_pm_opp_of_add_table_indexed(struct device *dev, int index) ...@@ -1002,8 +1002,8 @@ int dev_pm_opp_of_add_table_indexed(struct device *dev, int index)
} }
opp_table = dev_pm_opp_get_opp_table_indexed(dev, index); opp_table = dev_pm_opp_get_opp_table_indexed(dev, index);
if (!opp_table) if (IS_ERR(opp_table))
return -ENOMEM; return PTR_ERR(opp_table);
ret = _of_add_opp_table_v2(dev, opp_table); ret = _of_add_opp_table_v2(dev, opp_table);
if (ret) if (ret)
......
...@@ -93,7 +93,7 @@ static int exynos_asv_update_opps(struct exynos_asv *asv) ...@@ -93,7 +93,7 @@ static int exynos_asv_update_opps(struct exynos_asv *asv)
continue; continue;
opp_table = dev_pm_opp_get_opp_table(cpu); opp_table = dev_pm_opp_get_opp_table(cpu);
if (IS_ERR_OR_NULL(opp_table)) if (IS_ERR(opp_table))
continue; continue;
if (!last_opp_table || opp_table != last_opp_table) { if (!last_opp_table || opp_table != last_opp_table) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment