* [PATCH 1/2] opp: Export dev_pm_opp_set_genpd_virt_dev() @ 2019-07-02 4:36 Rajendra Nayak 2019-07-02 4:36 ` [PATCH 2/2] opp: Manage empty OPP tables with clk handle Rajendra Nayak 0 siblings, 1 reply; 6+ messages in thread From: Rajendra Nayak @ 2019-07-02 4:36 UTC (permalink / raw) To: vireshk, sboyd; +Cc: linux-pm, linux-kernel, linux-arm-msm, Rajendra Nayak Export dev_pm_opp_set_genpd_virt_dev() so loadable modules can use it. Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> --- drivers/opp/core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 8fbdbedc009c..ae033bb1e5b7 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -1797,6 +1797,7 @@ struct opp_table *dev_pm_opp_set_genpd_virt_dev(struct device *dev, return opp_table; } +EXPORT_SYMBOL_GPL(dev_pm_opp_set_genpd_virt_dev); /** * dev_pm_opp_put_genpd_virt_dev() - Releases resources blocked for genpd device. -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] opp: Manage empty OPP tables with clk handle 2019-07-02 4:36 [PATCH 1/2] opp: Export dev_pm_opp_set_genpd_virt_dev() Rajendra Nayak @ 2019-07-02 4:36 ` Rajendra Nayak 2019-07-03 8:50 ` Viresh Kumar 0 siblings, 1 reply; 6+ messages in thread From: Rajendra Nayak @ 2019-07-02 4:36 UTC (permalink / raw) To: vireshk, sboyd; +Cc: linux-pm, linux-kernel, linux-arm-msm, Rajendra Nayak With OPP core now supporting DVFS for IO devices, we have instances of IO devices (same IP block) with require an OPP on some platforms/SoCs while just needing to scale the clock on some others. In order to avoid conditional code in every driver, (to check for availability of OPPs and then deciding to do either dev_pm_opp_set_rate() or clk_set_rate()) add support to manage empty OPP tables with a clk handle. This makes dev_pm_opp_set_rate() equivalent of a clk_set_rate() for devices with just a clk and no OPPs specified. Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> --- drivers/opp/core.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index ae033bb1e5b7..fa7d4d6d37b3 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -801,6 +801,11 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) goto put_opp_table; } + if (!_get_opp_count(opp_table)) { + ret = _generic_set_opp_clk_only(dev, clk, freq); + goto put_opp_table; + } + temp_freq = old_freq; old_opp = _find_freq_ceil(opp_table, &temp_freq); if (IS_ERR(old_opp)) { -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] opp: Manage empty OPP tables with clk handle 2019-07-02 4:36 ` [PATCH 2/2] opp: Manage empty OPP tables with clk handle Rajendra Nayak @ 2019-07-03 8:50 ` Viresh Kumar 2019-07-03 9:11 ` Rajendra Nayak 0 siblings, 1 reply; 6+ messages in thread From: Viresh Kumar @ 2019-07-03 8:50 UTC (permalink / raw) To: Rajendra Nayak; +Cc: vireshk, sboyd, linux-pm, linux-kernel, linux-arm-msm On 02-07-19, 10:06, Rajendra Nayak wrote: > With OPP core now supporting DVFS for IO devices, we have instances of > IO devices (same IP block) with require an OPP on some platforms/SoCs which > while just needing to scale the clock on some others. Blank line here. > In order to avoid conditional code in every driver, (to check for remove , > availability of OPPs and then deciding to do either dev_pm_opp_set_rate() > or clk_set_rate()) add support to manage empty OPP tables with a clk handle. Blank line here. > This makes dev_pm_opp_set_rate() equivalent of a clk_set_rate() for devices > with just a clk and no OPPs specified. > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > --- > drivers/opp/core.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > index ae033bb1e5b7..fa7d4d6d37b3 100644 > --- a/drivers/opp/core.c > +++ b/drivers/opp/core.c > @@ -801,6 +801,11 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) > goto put_opp_table; > } > Explain the rationale behind this code here in a comment. > + if (!_get_opp_count(opp_table)) { > + ret = _generic_set_opp_clk_only(dev, clk, freq); > + goto put_opp_table; > + } > + > temp_freq = old_freq; > old_opp = _find_freq_ceil(opp_table, &temp_freq); > if (IS_ERR(old_opp)) { Also, rebase over the OPP branch please: git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git opp/linux-next or pm/linux-next -- viresh ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] opp: Manage empty OPP tables with clk handle 2019-07-03 8:50 ` Viresh Kumar @ 2019-07-03 9:11 ` Rajendra Nayak 2019-07-03 9:47 ` Viresh Kumar 0 siblings, 1 reply; 6+ messages in thread From: Rajendra Nayak @ 2019-07-03 9:11 UTC (permalink / raw) To: Viresh Kumar; +Cc: vireshk, sboyd, linux-pm, linux-kernel, linux-arm-msm [].. >> > > Explain the rationale behind this code here in a comment. > >> + if (!_get_opp_count(opp_table)) { >> + ret = _generic_set_opp_clk_only(dev, clk, freq); >> + goto put_opp_table; >> + } >> + >> temp_freq = old_freq; >> old_opp = _find_freq_ceil(opp_table, &temp_freq); >> if (IS_ERR(old_opp)) { > > Also, rebase over the OPP branch please: thanks, I will fix/rebase and repost, in the meantime while I was testing this a little more I realized I also need something like the change below to avoid a refcount mismatch WARN when empty OPP table is removed using dev_pm_opp_of_remove_table() diff --git a/drivers/opp/core.c b/drivers/opp/core.c index fa7d4d6d37b3..20128a88baf2 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -2118,7 +2118,8 @@ void _dev_pm_opp_find_and_remove_table(struct device *dev) return; } - _put_opp_list_kref(opp_table); + if (_get_opp_count(opp_table)) + _put_opp_list_kref(opp_table); /* Drop reference taken by _find_opp_table() */ dev_pm_opp_put_opp_table(opp_table); Does this look like a good way to fix it? -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] opp: Manage empty OPP tables with clk handle 2019-07-03 9:11 ` Rajendra Nayak @ 2019-07-03 9:47 ` Viresh Kumar 2019-07-03 10:47 ` Rajendra Nayak 0 siblings, 1 reply; 6+ messages in thread From: Viresh Kumar @ 2019-07-03 9:47 UTC (permalink / raw) To: Rajendra Nayak; +Cc: vireshk, sboyd, linux-pm, linux-kernel, linux-arm-msm On 03-07-19, 14:41, Rajendra Nayak wrote: > [].. > > > > Explain the rationale behind this code here in a comment. > > > > > + if (!_get_opp_count(opp_table)) { > > > + ret = _generic_set_opp_clk_only(dev, clk, freq); > > > + goto put_opp_table; > > > + } > > > + > > > temp_freq = old_freq; > > > old_opp = _find_freq_ceil(opp_table, &temp_freq); > > > if (IS_ERR(old_opp)) { > > > > Also, rebase over the OPP branch please: > > thanks, I will fix/rebase and repost, > in the meantime while I was testing this a little more I realized I also need > something like the change below to avoid a refcount mismatch WARN when empty OPP > table is removed using dev_pm_opp_of_remove_table() > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > index fa7d4d6d37b3..20128a88baf2 100644 > --- a/drivers/opp/core.c > +++ b/drivers/opp/core.c > @@ -2118,7 +2118,8 @@ void _dev_pm_opp_find_and_remove_table(struct device *dev) > return; > } > - _put_opp_list_kref(opp_table); > + if (_get_opp_count(opp_table)) > + _put_opp_list_kref(opp_table); > /* Drop reference taken by _find_opp_table() */ > dev_pm_opp_put_opp_table(opp_table); > > Does this look like a good way to fix it? No. If an OPP table only has dynamic OPPs, this will still generate warning. Below is the fix I would suggest. Please test it, I haven't tested it at all :) -- viresh -------------------------8<------------------------- From: Viresh Kumar <viresh.kumar@linaro.org> Date: Wed, 3 Jul 2019 15:03:14 +0530 Subject: [PATCH] opp: Don't decrement uninitialized list_kref The list_kref was added for static OPPs and to track their users. The kref is initialized while the static OPPs are added, but removed unconditionally even if the static OPPs were never added. This causes refcount mismatch warnings currently. Fix that by always initializing the kref when the OPP table is first initialized. The refcount is later incremented only for the second user onwards. Fixes: d0e8ae6c26da ("OPP: Create separate kref for static OPPs list") Reported-by: Rajendra Nayak <rnayak@codeaurora.org> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- drivers/opp/core.c | 1 + drivers/opp/of.c | 21 ++++----------------- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 89ec6aa220cf..2958cc7bbb58 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -943,6 +943,7 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index) BLOCKING_INIT_NOTIFIER_HEAD(&opp_table->head); INIT_LIST_HEAD(&opp_table->opp_list); kref_init(&opp_table->kref); + kref_init(&opp_table->list_kref); /* Secure the device table modification */ list_add(&opp_table->node, &opp_tables); diff --git a/drivers/opp/of.c b/drivers/opp/of.c index a637f30552a3..bf62b357437c 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -665,8 +665,6 @@ static int _of_add_opp_table_v2(struct device *dev, struct opp_table *opp_table) return 0; } - kref_init(&opp_table->list_kref); - /* We have opp-table node now, iterate over it and add OPPs */ for_each_available_child_of_node(opp_table->np, np) { opp = _opp_add_static_v2(opp_table, dev, np); @@ -675,17 +673,15 @@ static int _of_add_opp_table_v2(struct device *dev, struct opp_table *opp_table) dev_err(dev, "%s: Failed to add OPP, %d\n", __func__, ret); of_node_put(np); - goto put_list_kref; + return ret; } else if (opp) { count++; } } /* There should be one of more OPP defined */ - if (WARN_ON(!count)) { - ret = -ENOENT; - goto put_list_kref; - } + if (WARN_ON(!count)) + return -ENOENT; list_for_each_entry(opp, &opp_table->opp_list, node) pstate_count += !!opp->pstate; @@ -694,8 +690,7 @@ static int _of_add_opp_table_v2(struct device *dev, struct opp_table *opp_table) if (pstate_count && pstate_count != count) { dev_err(dev, "Not all nodes have performance state set (%d: %d)\n", count, pstate_count); - ret = -ENOENT; - goto put_list_kref; + return -ENOENT; } if (pstate_count) @@ -704,11 +699,6 @@ static int _of_add_opp_table_v2(struct device *dev, struct opp_table *opp_table) opp_table->parsed_static_opps = true; return 0; - -put_list_kref: - _put_opp_list_kref(opp_table); - - return ret; } /* Initializes OPP tables based on old-deprecated bindings */ @@ -734,8 +724,6 @@ static int _of_add_opp_table_v1(struct device *dev, struct opp_table *opp_table) return -EINVAL; } - kref_init(&opp_table->list_kref); - val = prop->value; while (nr) { unsigned long freq = be32_to_cpup(val++) * 1000; @@ -745,7 +733,6 @@ static int _of_add_opp_table_v1(struct device *dev, struct opp_table *opp_table) if (ret) { dev_err(dev, "%s: Failed to add OPP %ld (%d)\n", __func__, freq, ret); - _put_opp_list_kref(opp_table); return ret; } nr -= 2; ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] opp: Manage empty OPP tables with clk handle 2019-07-03 9:47 ` Viresh Kumar @ 2019-07-03 10:47 ` Rajendra Nayak 0 siblings, 0 replies; 6+ messages in thread From: Rajendra Nayak @ 2019-07-03 10:47 UTC (permalink / raw) To: Viresh Kumar; +Cc: vireshk, sboyd, linux-pm, linux-kernel, linux-arm-msm On 7/3/2019 3:17 PM, Viresh Kumar wrote: > On 03-07-19, 14:41, Rajendra Nayak wrote: >> [].. >>> >>> Explain the rationale behind this code here in a comment. >>> >>>> + if (!_get_opp_count(opp_table)) { >>>> + ret = _generic_set_opp_clk_only(dev, clk, freq); >>>> + goto put_opp_table; >>>> + } >>>> + >>>> temp_freq = old_freq; >>>> old_opp = _find_freq_ceil(opp_table, &temp_freq); >>>> if (IS_ERR(old_opp)) { >>> >>> Also, rebase over the OPP branch please: >> >> thanks, I will fix/rebase and repost, >> in the meantime while I was testing this a little more I realized I also need >> something like the change below to avoid a refcount mismatch WARN when empty OPP >> table is removed using dev_pm_opp_of_remove_table() >> >> diff --git a/drivers/opp/core.c b/drivers/opp/core.c >> index fa7d4d6d37b3..20128a88baf2 100644 >> --- a/drivers/opp/core.c >> +++ b/drivers/opp/core.c >> @@ -2118,7 +2118,8 @@ void _dev_pm_opp_find_and_remove_table(struct device *dev) >> return; >> } >> - _put_opp_list_kref(opp_table); >> + if (_get_opp_count(opp_table)) >> + _put_opp_list_kref(opp_table); >> /* Drop reference taken by _find_opp_table() */ >> dev_pm_opp_put_opp_table(opp_table); >> >> Does this look like a good way to fix it? > > No. If an OPP table only has dynamic OPPs, this will still generate > warning. Below is the fix I would suggest. Please test it, I haven't > tested it at all :) thanks, yes, this seems to work. -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-07-03 10:47 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-02 4:36 [PATCH 1/2] opp: Export dev_pm_opp_set_genpd_virt_dev() Rajendra Nayak 2019-07-02 4:36 ` [PATCH 2/2] opp: Manage empty OPP tables with clk handle Rajendra Nayak 2019-07-03 8:50 ` Viresh Kumar 2019-07-03 9:11 ` Rajendra Nayak 2019-07-03 9:47 ` Viresh Kumar 2019-07-03 10:47 ` Rajendra Nayak
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).