From: Viresh Kumar <viresh.kumar@linaro.org> To: Kamil Konieczny <k.konieczny@partner.samsung.com> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>, Marek Szyprowski <m.szyprowski@samsung.com>, Chanwoo Choi <cw00.choi@samsung.com>, Krzysztof Kozlowski <krzk@kernel.org>, Kukjin Kim <kgene@kernel.org>, Kyungmin Park <kyungmin.park@samsung.com>, Mark Rutland <mark.rutland@arm.com>, MyungJoo Ham <myungjoo.ham@samsung.com>, Nishanth Menon <nm@ti.com>, Rob Herring <robh+dt@kernel.org>, Stephen Boyd <sboyd@kernel.org>, Viresh Kumar <vireshk@kernel.org>, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, linux-samsung-soc@vger.kernel.org Subject: Re: [PATCH v2 1/4] opp: core: add regulators enable and disable Date: Tue, 16 Jul 2019 15:35:39 +0530 [thread overview] Message-ID: <20190716100539.4uqelbxqz7bmtmea@vireshk-i7> (raw) In-Reply-To: <20190715120416.3561-2-k.konieczny@partner.samsung.com> On 15-07-19, 14:04, Kamil Konieczny wrote: > Add enable regulators to dev_pm_opp_set_regulators() and disable > regulators to dev_pm_opp_put_regulators(). This prepares for > converting exynos-bus devfreq driver to use dev_pm_opp_set_rate(). > > Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com> > -- > Changes in v2: > > - move regulator enable and disable into loop > > --- > drivers/opp/core.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > index 0e7703fe733f..069c5cf8827e 100644 > --- a/drivers/opp/core.c > +++ b/drivers/opp/core.c > @@ -1570,6 +1570,10 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev, > goto free_regulators; > } > > + ret = regulator_enable(reg); > + if (ret < 0) > + goto disable; The name of this label is logically incorrect because we won't disable the regulator from there but put it. Over that, I would rather prefer to remove the label and add regulator_put() here itself. > + > opp_table->regulators[i] = reg; > } > > @@ -1582,9 +1586,15 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev, > > return opp_table; > > +disable: > + regulator_put(reg); > + --i; > + > free_regulators: > - while (i != 0) > - regulator_put(opp_table->regulators[--i]); > + for (; i >= 0; --i) { > + regulator_disable(opp_table->regulators[i]); > + regulator_put(opp_table->regulators[i]); This is incorrect as this will now try to put/disable the regulator which we failed to acquire. As --i happens only after the loop has run once. You can rather do: while (i--) { regulator_disable(opp_table->regulators[i]); regulator_put(opp_table->regulators[i]); } > + } > > kfree(opp_table->regulators); > opp_table->regulators = NULL; > @@ -1610,8 +1620,10 @@ void dev_pm_opp_put_regulators(struct opp_table *opp_table) > /* Make sure there are no concurrent readers while updating opp_table */ > WARN_ON(!list_empty(&opp_table->opp_list)); > > - for (i = opp_table->regulator_count - 1; i >= 0; i--) > + for (i = opp_table->regulator_count - 1; i >= 0; i--) { > + regulator_disable(opp_table->regulators[i]); > regulator_put(opp_table->regulators[i]); > + } > > _free_set_opp_data(opp_table); > > -- > 2.22.0 -- viresh
WARNING: multiple messages have this Message-ID (diff)
From: Viresh Kumar <viresh.kumar@linaro.org> To: Kamil Konieczny <k.konieczny@partner.samsung.com> Cc: Mark Rutland <mark.rutland@arm.com>, Nishanth Menon <nm@ti.com>, linux-samsung-soc@vger.kernel.org, Rob Herring <robh+dt@kernel.org>, linux-arm-kernel@lists.infradead.org, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>, Stephen Boyd <sboyd@kernel.org>, Viresh Kumar <vireshk@kernel.org>, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, Krzysztof Kozlowski <krzk@kernel.org>, Chanwoo Choi <cw00.choi@samsung.com>, Kyungmin Park <kyungmin.park@samsung.com>, Kukjin Kim <kgene@kernel.org>, MyungJoo Ham <myungjoo.ham@samsung.com>, devicetree@vger.kernel.org, Marek Szyprowski <m.szyprowski@samsung.com> Subject: Re: [PATCH v2 1/4] opp: core: add regulators enable and disable Date: Tue, 16 Jul 2019 15:35:39 +0530 [thread overview] Message-ID: <20190716100539.4uqelbxqz7bmtmea@vireshk-i7> (raw) In-Reply-To: <20190715120416.3561-2-k.konieczny@partner.samsung.com> On 15-07-19, 14:04, Kamil Konieczny wrote: > Add enable regulators to dev_pm_opp_set_regulators() and disable > regulators to dev_pm_opp_put_regulators(). This prepares for > converting exynos-bus devfreq driver to use dev_pm_opp_set_rate(). > > Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com> > -- > Changes in v2: > > - move regulator enable and disable into loop > > --- > drivers/opp/core.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > index 0e7703fe733f..069c5cf8827e 100644 > --- a/drivers/opp/core.c > +++ b/drivers/opp/core.c > @@ -1570,6 +1570,10 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev, > goto free_regulators; > } > > + ret = regulator_enable(reg); > + if (ret < 0) > + goto disable; The name of this label is logically incorrect because we won't disable the regulator from there but put it. Over that, I would rather prefer to remove the label and add regulator_put() here itself. > + > opp_table->regulators[i] = reg; > } > > @@ -1582,9 +1586,15 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev, > > return opp_table; > > +disable: > + regulator_put(reg); > + --i; > + > free_regulators: > - while (i != 0) > - regulator_put(opp_table->regulators[--i]); > + for (; i >= 0; --i) { > + regulator_disable(opp_table->regulators[i]); > + regulator_put(opp_table->regulators[i]); This is incorrect as this will now try to put/disable the regulator which we failed to acquire. As --i happens only after the loop has run once. You can rather do: while (i--) { regulator_disable(opp_table->regulators[i]); regulator_put(opp_table->regulators[i]); } > + } > > kfree(opp_table->regulators); > opp_table->regulators = NULL; > @@ -1610,8 +1620,10 @@ void dev_pm_opp_put_regulators(struct opp_table *opp_table) > /* Make sure there are no concurrent readers while updating opp_table */ > WARN_ON(!list_empty(&opp_table->opp_list)); > > - for (i = opp_table->regulator_count - 1; i >= 0; i--) > + for (i = opp_table->regulator_count - 1; i >= 0; i--) { > + regulator_disable(opp_table->regulators[i]); > regulator_put(opp_table->regulators[i]); > + } > > _free_set_opp_data(opp_table); > > -- > 2.22.0 -- viresh _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-07-16 10:05 UTC|newest] Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <CGME20190715120430eucas1p1dd216e552403899e614845295373e467@eucas1p1.samsung.com> 2019-07-15 12:04 ` [PATCH v2 0/4] add coupled regulators for Exynos5422/5800 Kamil Konieczny 2019-07-15 12:04 ` Kamil Konieczny 2019-07-15 12:04 ` Kamil Konieczny [not found] ` <CGME20190715120430eucas1p19dddcc93756e6a110d3476229f9428b3@eucas1p1.samsung.com> 2019-07-15 12:04 ` [PATCH v2 1/4] opp: core: add regulators enable and disable Kamil Konieczny 2019-07-15 12:04 ` Kamil Konieczny 2019-07-16 4:03 ` Chanwoo Choi 2019-07-16 4:03 ` Chanwoo Choi 2019-07-17 14:12 ` Kamil Konieczny 2019-07-17 14:12 ` Kamil Konieczny 2019-07-16 10:05 ` Viresh Kumar [this message] 2019-07-16 10:05 ` Viresh Kumar 2019-07-17 14:14 ` Kamil Konieczny 2019-07-17 14:14 ` Kamil Konieczny [not found] ` <CGME20190715120431eucas1p215eae81d0ca772d7e2a22a803669068a@eucas1p2.samsung.com> 2019-07-15 12:04 ` [PATCH v2 2/4] devfreq: exynos-bus: convert to use dev_pm_opp_set_rate() Kamil Konieczny 2019-07-15 12:04 ` Kamil Konieczny 2019-07-16 3:56 ` Chanwoo Choi 2019-07-16 3:56 ` Chanwoo Choi 2019-07-16 10:13 ` Bartlomiej Zolnierkiewicz 2019-07-16 10:13 ` Bartlomiej Zolnierkiewicz 2019-07-16 10:33 ` Chanwoo Choi 2019-07-16 10:33 ` Chanwoo Choi 2019-07-16 10:59 ` Bartlomiej Zolnierkiewicz 2019-07-16 10:59 ` Bartlomiej Zolnierkiewicz 2019-07-16 11:26 ` Chanwoo Choi 2019-07-16 11:26 ` Chanwoo Choi 2019-07-16 11:39 ` Bartlomiej Zolnierkiewicz 2019-07-16 11:39 ` Bartlomiej Zolnierkiewicz 2019-07-16 11:56 ` Bartlomiej Zolnierkiewicz 2019-07-16 11:56 ` Bartlomiej Zolnierkiewicz 2019-07-16 11:56 ` Bartlomiej Zolnierkiewicz [not found] ` <CGME20190715120432eucas1p1b32d72d239420b861bf8596d4e8a053d@eucas1p1.samsung.com> 2019-07-15 12:04 ` [PATCH v2 3/4] ARM: dts: exynos: add initial data for coupled regulators for Exynos5422/5800 Kamil Konieczny 2019-07-15 12:04 ` Kamil Konieczny 2019-07-16 9:00 ` Chanwoo Choi 2019-07-16 9:00 ` Chanwoo Choi 2019-07-16 9:22 ` Krzysztof Kozlowski 2019-07-16 9:22 ` Krzysztof Kozlowski 2019-07-16 9:22 ` Krzysztof Kozlowski 2019-07-16 10:30 ` Bartlomiej Zolnierkiewicz 2019-07-16 10:30 ` Bartlomiej Zolnierkiewicz 2019-07-16 10:30 ` Bartlomiej Zolnierkiewicz [not found] ` <CGME20190715120433eucas1p26681c5c2d87423253b651d88446c538c@eucas1p2.samsung.com> 2019-07-15 12:04 ` [PATCH v2 4/4] dt-bindings: devfreq: exynos-bus: remove unused property Kamil Konieczny 2019-07-15 12:04 ` Kamil Konieczny 2019-07-16 8:54 ` Chanwoo Choi 2019-07-16 8:54 ` Chanwoo Choi
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190716100539.4uqelbxqz7bmtmea@vireshk-i7 \ --to=viresh.kumar@linaro.org \ --cc=b.zolnierkie@samsung.com \ --cc=cw00.choi@samsung.com \ --cc=devicetree@vger.kernel.org \ --cc=k.konieczny@partner.samsung.com \ --cc=kgene@kernel.org \ --cc=krzk@kernel.org \ --cc=kyungmin.park@samsung.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=linux-samsung-soc@vger.kernel.org \ --cc=m.szyprowski@samsung.com \ --cc=mark.rutland@arm.com \ --cc=myungjoo.ham@samsung.com \ --cc=nm@ti.com \ --cc=robh+dt@kernel.org \ --cc=sboyd@kernel.org \ --cc=vireshk@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.