mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-08-31 08:15:07 -04:00
opp: Use clk_get_optional() to avoid leaving opp_table->clk as an error pointer
_update_opp_table_clk() uses clk_get(dev, NULL) to acquire the
device's clock. On platforms where the perf domain device has no
Linux clock and is instead managed entirely by firmware via
devm_pm_opp_of_add_table() (through
of_genpd_add_provider_simple()/onecell()), clk_get() returns
-ENOENT. That case is treated as valid (the OPP table can still
have entries sourced from firmware), but opp_table->clk is left
holding ERR_PTR(-ENOENT) rather than being reset to NULL:
opp_table->clk = clk_get(dev, NULL);
ret = PTR_ERR_OR_ZERO(opp_table->clk);
...
if (ret == -ENOENT) {
opp_table->clk_count = 1;
return opp_table; /* opp_table->clk is still ERR_PTR(-ENOENT) */
}
Consumers that only check IS_ERR(opp_table->clk) treat this as a
valid clk and pass it straight into the clk consumer API. In
particular, dev_pm_opp_set_rate() calls
clk_round_rate(opp_table->clk, target_freq), and clk_round_rate()
only guards against a NULL clk, so it dereferences the error pointer
to read clk->exclusive_count and crashes:
Unable to handle kernel NULL pointer dereference at virtual
address 000000000000002e
...
pc : clk_round_rate+0x3c/0x188
...
Call trace:
clk_round_rate+0x3c/0x188 (P)
dev_pm_opp_set_rate+0x114/0x33c
Rather than teaching every clk consumer API to special-case
ERR_PTR(-ENOENT), fix it at the source: use clk_get_optional()
instead of clk_get() in _update_opp_table_clk(), which already
translates -ENOENT into a NULL clk. This documents that the clock is
genuinely optional for such devices, and keeps opp_table->clk holding
either a valid clk or NULL, never a lingering -ENOENT error pointer.
_opp_config_clk_single() is only wired up via opp_table->config_clks
when a clk was actually found, and every other opp_table->clk
consumer already tolerates NULL through the standard clk API (which
treats a NULL clk as a no-op), so no other call site needs to change.
Suggested-by: Sebastian Reichel <sre@kernel.org>
Reviewed-by: Sebastian Reichel <sre@kernel.org>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
This commit is contained in:
committed by
Viresh Kumar
parent
ce46fede77
commit
84f05af097
@@ -1581,8 +1581,6 @@ static struct opp_table *_update_opp_table_clk(struct device *dev,
|
||||
struct opp_table *opp_table,
|
||||
bool getclk)
|
||||
{
|
||||
int ret;
|
||||
|
||||
/*
|
||||
* Return early if we don't need to get clk or we have already done it
|
||||
* earlier.
|
||||
@@ -1591,39 +1589,35 @@ static struct opp_table *_update_opp_table_clk(struct device *dev,
|
||||
opp_table->clks)
|
||||
return opp_table;
|
||||
|
||||
/* Find clk for the device */
|
||||
opp_table->clk = clk_get(dev, NULL);
|
||||
/*
|
||||
* There are few platforms which don't want the OPP core to manage
|
||||
* device's clock settings. In such cases neither the platform
|
||||
* provides the clks explicitly to us, nor the DT contains a valid
|
||||
* clk entry. The OPP nodes in DT may still contain "opp-hz" property
|
||||
* though, which we need to parse and allow the platform to find an
|
||||
* OPP based on freq later on.
|
||||
*
|
||||
* This is a simple solution to take care of such corner cases, i.e.
|
||||
* make the clk_count 1, which lets us allocate space for frequency
|
||||
* in opp->rates and also parse the entries in DT. Use
|
||||
* clk_get_optional() instead of clk_get() so opp_table->clk stays
|
||||
* NULL for such devices, instead of holding an ERR_PTR(-ENOENT) that
|
||||
* consumers must remember to special-case.
|
||||
*/
|
||||
opp_table->clk = clk_get_optional(dev, NULL);
|
||||
|
||||
ret = PTR_ERR_OR_ZERO(opp_table->clk);
|
||||
if (!ret) {
|
||||
if (IS_ERR(opp_table->clk)) {
|
||||
dev_pm_opp_put_opp_table(opp_table);
|
||||
dev_err_probe(dev, PTR_ERR(opp_table->clk), "Couldn't find clock\n");
|
||||
return ERR_CAST(opp_table->clk);
|
||||
}
|
||||
|
||||
if (opp_table->clk)
|
||||
opp_table->config_clks = _opp_config_clk_single;
|
||||
opp_table->clk_count = 1;
|
||||
return opp_table;
|
||||
}
|
||||
|
||||
if (ret == -ENOENT) {
|
||||
/*
|
||||
* There are few platforms which don't want the OPP core to
|
||||
* manage device's clock settings. In such cases neither the
|
||||
* platform provides the clks explicitly to us, nor the DT
|
||||
* contains a valid clk entry. The OPP nodes in DT may still
|
||||
* contain "opp-hz" property though, which we need to parse and
|
||||
* allow the platform to find an OPP based on freq later on.
|
||||
*
|
||||
* This is a simple solution to take care of such corner cases,
|
||||
* i.e. make the clk_count 1, which lets us allocate space for
|
||||
* frequency in opp->rates and also parse the entries in DT.
|
||||
*/
|
||||
opp_table->clk_count = 1;
|
||||
opp_table->clk_count = 1;
|
||||
|
||||
dev_dbg(dev, "%s: Couldn't find clock: %d\n", __func__, ret);
|
||||
return opp_table;
|
||||
}
|
||||
|
||||
dev_pm_opp_put_opp_table(opp_table);
|
||||
dev_err_probe(dev, ret, "Couldn't find clock\n");
|
||||
|
||||
return ERR_PTR(ret);
|
||||
return opp_table;
|
||||
}
|
||||
|
||||
/*
|
||||
|
||||
Reference in New Issue
Block a user