| Message ID | 20190702043643.1746-2-rnayak@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | viresh kumar |
| Headers | show |
| Series | [1/2] opp: Export dev_pm_opp_set_genpd_virt_dev() | expand |
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
[].. >> > > 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?
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 :)
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.
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)) {
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(+)