| Message ID | 6306e18beab9deff6ee6b32f489390908495fe14.1570703431.git.viresh.kumar@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | viresh kumar |
| Headers | show |
| Series | opp: of: drop incorrect lockdep_assert_held() | expand |
On 10-10-19, 16:00, Viresh Kumar wrote: > _find_opp_of_np() doesn't traverse the list of OPP tables but instead > just the entries within an OPP table and so only requires to lock the > OPP table itself. > > The lockdep_assert_held() was added there by mistake and isn't really > required. > > Fixes: 5d6d106fa455 ("OPP: Populate required opp tables from "required-opps" property") > Cc: v5.0+ <stable@vger.kernel.org> # v5.0+ > Reported-by: Niklas Cassel <niklas.cassel@linaro.org> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > drivers/opp/of.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/opp/of.c b/drivers/opp/of.c > index 1813f5ad5fa2..6dc41faf74b5 100644 > --- a/drivers/opp/of.c > +++ b/drivers/opp/of.c > @@ -77,8 +77,6 @@ static struct dev_pm_opp *_find_opp_of_np(struct opp_table *opp_table, > { > struct dev_pm_opp *opp; > > - lockdep_assert_held(&opp_table_lock); > - > mutex_lock(&opp_table->lock); > > list_for_each_entry(opp, &opp_table->opp_list, node) { @Niklas, any inputs from your side here would be appreciated :)
On Wed, Oct 23, 2019 at 11:00:05AM +0530, Viresh Kumar wrote: > On 10-10-19, 16:00, Viresh Kumar wrote: > > _find_opp_of_np() doesn't traverse the list of OPP tables but instead > > just the entries within an OPP table and so only requires to lock the > > OPP table itself. > > > > The lockdep_assert_held() was added there by mistake and isn't really > > required. > > > > Fixes: 5d6d106fa455 ("OPP: Populate required opp tables from "required-opps" property") > > Cc: v5.0+ <stable@vger.kernel.org> # v5.0+ > > Reported-by: Niklas Cassel <niklas.cassel@linaro.org> > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > --- > > drivers/opp/of.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/opp/of.c b/drivers/opp/of.c > > index 1813f5ad5fa2..6dc41faf74b5 100644 > > --- a/drivers/opp/of.c > > +++ b/drivers/opp/of.c > > @@ -77,8 +77,6 @@ static struct dev_pm_opp *_find_opp_of_np(struct opp_table *opp_table, > > { > > struct dev_pm_opp *opp; > > > > - lockdep_assert_held(&opp_table_lock); > > - > > mutex_lock(&opp_table->lock); > > > > list_for_each_entry(opp, &opp_table->opp_list, node) { > > @Niklas, any inputs from your side here would be appreciated :) Tested-by: Niklas Cassel <niklas.cassel@linaro.org> After this patch, there is still a single lockdep_assert_held() left, inside _find_table_of_opp_np(), since you kept this, I assume that that one is still needed? Kind regards, Niklas
On 23-10-19, 14:01, Niklas Cassel wrote: > On Wed, Oct 23, 2019 at 11:00:05AM +0530, Viresh Kumar wrote: > > On 10-10-19, 16:00, Viresh Kumar wrote: > > > _find_opp_of_np() doesn't traverse the list of OPP tables but instead > > > just the entries within an OPP table and so only requires to lock the > > > OPP table itself. > > > > > > The lockdep_assert_held() was added there by mistake and isn't really > > > required. > > > > > > Fixes: 5d6d106fa455 ("OPP: Populate required opp tables from "required-opps" property") > > > Cc: v5.0+ <stable@vger.kernel.org> # v5.0+ > > > Reported-by: Niklas Cassel <niklas.cassel@linaro.org> > > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > > --- > > > drivers/opp/of.c | 2 -- > > > 1 file changed, 2 deletions(-) > > > > > > diff --git a/drivers/opp/of.c b/drivers/opp/of.c > > > index 1813f5ad5fa2..6dc41faf74b5 100644 > > > --- a/drivers/opp/of.c > > > +++ b/drivers/opp/of.c > > > @@ -77,8 +77,6 @@ static struct dev_pm_opp *_find_opp_of_np(struct opp_table *opp_table, > > > { > > > struct dev_pm_opp *opp; > > > > > > - lockdep_assert_held(&opp_table_lock); > > > - > > > mutex_lock(&opp_table->lock); > > > > > > list_for_each_entry(opp, &opp_table->opp_list, node) { > > > > @Niklas, any inputs from your side here would be appreciated :) > > Tested-by: Niklas Cassel <niklas.cassel@linaro.org> > > After this patch, there is still a single lockdep_assert_held() > left, inside _find_table_of_opp_np(), since you kept this, > I assume that that one is still needed? Yeah, that one is required as we are traversing the list of OPP tables there.
diff --git a/drivers/opp/of.c b/drivers/opp/of.c index 1813f5ad5fa2..6dc41faf74b5 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -77,8 +77,6 @@ static struct dev_pm_opp *_find_opp_of_np(struct opp_table *opp_table, { struct dev_pm_opp *opp; - lockdep_assert_held(&opp_table_lock); - mutex_lock(&opp_table->lock); list_for_each_entry(opp, &opp_table->opp_list, node) {
_find_opp_of_np() doesn't traverse the list of OPP tables but instead just the entries within an OPP table and so only requires to lock the OPP table itself. The lockdep_assert_held() was added there by mistake and isn't really required. Fixes: 5d6d106fa455 ("OPP: Populate required opp tables from "required-opps" property") Cc: v5.0+ <stable@vger.kernel.org> # v5.0+ Reported-by: Niklas Cassel <niklas.cassel@linaro.org> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- drivers/opp/of.c | 2 -- 1 file changed, 2 deletions(-)