opp: core: Revert "add regulators enable and disable"
diff mbox series

Message ID 20191017102758.8104-1-m.szyprowski@samsung.com
State Not Applicable
Headers show
Series
  • opp: core: Revert "add regulators enable and disable"
Related show

Commit Message

Marek Szyprowski Oct. 17, 2019, 10:27 a.m. UTC
All the drivers, which use the OPP framework control regulators, which
are already enabled. Typically those regulators are also system critical,
due to providing power to CPU core or system buses. It turned out that
there are cases, where calling regulator_enable() on such boot-enabled
regulator has side-effects and might change its initial voltage due to
performing initial voltage balancing without all restrictions from the
consumers. Until this issue becomes finally solved in regulator core,
avoid calling regulator_enable()/disable() from the OPP framework.

This reverts commit 7f93ff73f7c8c8bfa6be33bcc16470b0b44682aa.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
This is a follow-up from the following discussion:
https://lkml.org/lkml/2019/10/9/541
---
 drivers/opp/core.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

Comments

Viresh Kumar Oct. 18, 2019, 4:30 a.m. UTC | #1
On 17-10-19, 12:27, Marek Szyprowski wrote:
> All the drivers, which use the OPP framework control regulators, which
> are already enabled. Typically those regulators are also system critical,
> due to providing power to CPU core or system buses. It turned out that
> there are cases, where calling regulator_enable() on such boot-enabled
> regulator has side-effects and might change its initial voltage due to
> performing initial voltage balancing without all restrictions from the
> consumers. Until this issue becomes finally solved in regulator core,
> avoid calling regulator_enable()/disable() from the OPP framework.
> 
> This reverts commit 7f93ff73f7c8c8bfa6be33bcc16470b0b44682aa.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> This is a follow-up from the following discussion:
> https://lkml.org/lkml/2019/10/9/541

I suppose this must go the v5.4-rcs, right ?
Marek Szyprowski Oct. 18, 2019, 7:11 a.m. UTC | #2
On 18.10.2019 06:30, Viresh Kumar wrote:
> On 17-10-19, 12:27, Marek Szyprowski wrote:
>> All the drivers, which use the OPP framework control regulators, which
>> are already enabled. Typically those regulators are also system critical,
>> due to providing power to CPU core or system buses. It turned out that
>> there are cases, where calling regulator_enable() on such boot-enabled
>> regulator has side-effects and might change its initial voltage due to
>> performing initial voltage balancing without all restrictions from the
>> consumers. Until this issue becomes finally solved in regulator core,
>> avoid calling regulator_enable()/disable() from the OPP framework.
>>
>> This reverts commit 7f93ff73f7c8c8bfa6be33bcc16470b0b44682aa.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>> This is a follow-up from the following discussion:
>> https://lkml.org/lkml/2019/10/9/541
> I suppose this must go the v5.4-rcs, right ?

Yes, please.

Best regards
Mark Brown Oct. 18, 2019, 11:42 a.m. UTC | #3
On Thu, Oct 17, 2019 at 12:27:58PM +0200, Marek Szyprowski wrote:
> All the drivers, which use the OPP framework control regulators, which
> are already enabled. Typically those regulators are also system critical,
> due to providing power to CPU core or system buses. It turned out that
> there are cases, where calling regulator_enable() on such boot-enabled
> regulator has side-effects and might change its initial voltage due to
> performing initial voltage balancing without all restrictions from the
> consumers. Until this issue becomes finally solved in regulator core,
> avoid calling regulator_enable()/disable() from the OPP framework.

Reviewed-by: Mark Brown <broonie@kernel.org>

Patch
diff mbox series

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 3b7ffd0234e9..9ff0538ee83a 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1626,12 +1626,6 @@  struct opp_table *dev_pm_opp_set_regulators(struct device *dev,
 			goto free_regulators;
 		}
 
-		ret = regulator_enable(reg);
-		if (ret < 0) {
-			regulator_put(reg);
-			goto free_regulators;
-		}
-
 		opp_table->regulators[i] = reg;
 	}
 
@@ -1645,10 +1639,8 @@  struct opp_table *dev_pm_opp_set_regulators(struct device *dev,
 	return opp_table;
 
 free_regulators:
-	while (i--) {
-		regulator_disable(opp_table->regulators[i]);
-		regulator_put(opp_table->regulators[i]);
-	}
+	while (i != 0)
+		regulator_put(opp_table->regulators[--i]);
 
 	kfree(opp_table->regulators);
 	opp_table->regulators = NULL;
@@ -1674,10 +1666,8 @@  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--) {
-		regulator_disable(opp_table->regulators[i]);
+	for (i = opp_table->regulator_count - 1; i >= 0; i--)
 		regulator_put(opp_table->regulators[i]);
-	}
 
 	_free_set_opp_data(opp_table);