Linux-Samsung-soc Archive on lore.kernel.org
 help / color / Atom feed
WARNING: multiple messages have this Message-ID
From: Marek Szyprowski <m.szyprowski@samsung.com>
To: linux-kernel@vger.kernel.org, Mark Brown <broonie@kernel.org>,
	Dmitry Osipenko <digetx@gmail.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Lucas Stach <l.stach@pengutronix.de>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	linux-samsung-soc@vger.kernel.org
Subject: [PATCH] regulator: core: Skip balancing of the enabled regulators in regulator_enable()
Date: Tue,  8 Oct 2019 12:17:09 +0200
Message-ID: <20191008101709.13827-1-m.szyprowski@samsung.com> (raw)
In-Reply-To: <CGME20191008101720eucas1p2e0d1bca6e696848bf689067e05620679@eucas1p2.samsung.com>

Commit f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators
locking"), regardless of the subject, added additional call to
regulator_balance_voltage() during regulator_enable(). This is basically
a good idea, however it causes some issue for the regulators which are
already enabled at boot and are critical for system operation (for example
provides supply to the CPU).

CPUfreq or other drivers typically call regulator_enable() on such
regulators during their probe, although the regulators are already enabled
by bootloader. The mentioned patch however added a call to
regulator_balance_voltage(), what in case of system boot, where no
additional requirements are set yet, typically causes to limit the voltage
to the minimal value defined at regulator constraints. This causes a crash
of the system when voltage on the CPU regulator is set to the lowest
possible value without adjusting the operation frequency. Fix this by
adding a check if regulator is already enabled - if so, then skip the
balancing procedure. The voltage will be balanced later anyway once the
required voltage value is requested.

Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking")
Reported-by: "kernelci.org bot" <bot@kernelci.org>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
This patch fixes enabling coupled requlators on Exynos5800-based Peach-Pi
board done by the following patch:
https://patchwork.kernel.org/patch/11172427/
Once it has been applied, the following issue has been reported:
https://patchwork.kernel.org/patch/11176661/
---
 drivers/regulator/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index afe94470b67f..aca74b83f3bc 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2481,7 +2481,8 @@ static int _regulator_enable(struct regulator *regulator)
 	}
 
 	/* balance only if there are regulators coupled */
-	if (rdev->coupling_desc.n_coupled > 1) {
+	if (rdev->coupling_desc.n_coupled > 1 &&
+	    !_regulator_is_enabled(rdev)) {
 		ret = regulator_balance_voltage(rdev, PM_SUSPEND_ON);
 		if (ret < 0)
 			goto err_disable_supply;
-- 
2.17.1

From: Marek Szyprowski <m.szyprowski@samsung.com>
To: linux-kernel@vger.kernel.org, Mark Brown <broonie@kernel.org>,
	Dmitry Osipenko <digetx@gmail.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Lucas Stach <l.stach@pengutronix.de>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	linux-samsung-soc@vger.kernel.org
Subject: [PATCH] regulator: core: Skip balancing of the enabled regulators in regulator_enable()
Date: Tue,  8 Oct 2019 12:17:09 +0200
Message-ID: <20191008101709.13827-1-m.szyprowski@samsung.com> (raw)
Message-ID: <20191008101709.qVNy8eijBi0LynOteWFMnTg4GUwKG599n6OyYoX1Abs@z> (raw)
In-Reply-To: <CGME20191008101720eucas1p2e0d1bca6e696848bf689067e05620679@eucas1p2.samsung.com>

Commit f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators
locking"), regardless of the subject, added additional call to
regulator_balance_voltage() during regulator_enable(). This is basically
a good idea, however it causes some issue for the regulators which are
already enabled at boot and are critical for system operation (for example
provides supply to the CPU).

CPUfreq or other drivers typically call regulator_enable() on such
regulators during their probe, although the regulators are already enabled
by bootloader. The mentioned patch however added a call to
regulator_balance_voltage(), what in case of system boot, where no
additional requirements are set yet, typically causes to limit the voltage
to the minimal value defined at regulator constraints. This causes a crash
of the system when voltage on the CPU regulator is set to the lowest
possible value without adjusting the operation frequency. Fix this by
adding a check if regulator is already enabled - if so, then skip the
balancing procedure. The voltage will be balanced later anyway once the
required voltage value is requested.

Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking")
Reported-by: "kernelci.org bot" <bot@kernelci.org>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
This patch fixes enabling coupled requlators on Exynos5800-based Peach-Pi
board done by the following patch:
https://patchwork.kernel.org/patch/11172427/
Once it has been applied, the following issue has been reported:
https://patchwork.kernel.org/patch/11176661/
---
 drivers/regulator/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index afe94470b67f..aca74b83f3bc 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2481,7 +2481,8 @@ static int _regulator_enable(struct regulator *regulator)
 	}
 
 	/* balance only if there are regulators coupled */
-	if (rdev->coupling_desc.n_coupled > 1) {
+	if (rdev->coupling_desc.n_coupled > 1 &&
+	    !_regulator_is_enabled(rdev)) {
 		ret = regulator_balance_voltage(rdev, PM_SUSPEND_ON);
 		if (ret < 0)
 			goto err_disable_supply;
-- 
2.17.1


       reply index

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20191008101720eucas1p2e0d1bca6e696848bf689067e05620679@eucas1p2.samsung.com>
2019-10-08 10:17 ` Marek Szyprowski [this message]
2019-10-08 10:17   ` Marek Szyprowski
2019-10-08 11:50   ` Mark Brown
2019-10-08 11:50     ` Mark Brown
2019-10-08 12:01     ` Marek Szyprowski
2019-10-08 12:01       ` Marek Szyprowski
2019-10-08 12:06       ` Mark Brown
2019-10-08 12:06         ` Mark Brown
2019-10-08 12:38         ` Marek Szyprowski
2019-10-08 12:38           ` Marek Szyprowski
2019-10-08 12:47           ` Mark Brown
2019-10-08 12:47             ` Mark Brown
2019-10-08 13:24             ` Bartlomiej Zolnierkiewicz
2019-10-08 13:24               ` Bartlomiej Zolnierkiewicz
2019-10-08 15:02               ` Dmitry Osipenko
2019-10-08 15:02                 ` Dmitry Osipenko
2019-10-08 16:15                 ` Mark Brown
2019-10-08 16:15                   ` Mark Brown
2019-10-08 17:05                   ` Dmitry Osipenko
2019-10-08 17:05                     ` Dmitry Osipenko
2019-10-08 17:17                     ` Mark Brown
2019-10-08 17:17                       ` Mark Brown
2019-10-08 18:00                       ` Dmitry Osipenko
2019-10-08 18:00                         ` Dmitry Osipenko
2019-10-08 18:07                         ` Mark Brown
2019-10-08 18:07                           ` Mark Brown
2019-10-09 10:29                           ` Marek Szyprowski
2019-10-09 10:29                             ` Marek Szyprowski
2019-10-09 14:13                             ` Mark Brown
2019-10-09 14:13                               ` Mark Brown
2019-10-10  7:29                               ` Viresh Kumar
2019-10-10  7:29                                 ` Viresh Kumar
2019-10-10 10:19                               ` Marek Szyprowski
2019-10-10 10:19                                 ` Marek Szyprowski
2019-10-10 13:55                                 ` Mark Brown
2019-10-10 13:55                                   ` Mark Brown
2019-10-17 10:29                                   ` Marek Szyprowski
2019-10-08 15:48               ` Mark Brown
2019-10-08 15:48                 ` Mark Brown
2019-10-08 16:02                 ` Bartlomiej Zolnierkiewicz
2019-10-08 16:02                   ` Bartlomiej Zolnierkiewicz
2019-10-08 16:21                   ` Mark Brown
2019-10-08 16:21                     ` Mark Brown

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=20191008101709.13827-1-m.szyprowski@samsung.com \
    --to=m.szyprowski@samsung.com \
    --cc=b.zolnierkie@samsung.com \
    --cc=broonie@kernel.org \
    --cc=digetx@gmail.com \
    --cc=krzk@kernel.org \
    --cc=l.stach@pengutronix.de \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-samsung-soc@vger.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: link

Linux-Samsung-soc Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-samsung-soc/0 linux-samsung-soc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-samsung-soc linux-samsung-soc/ https://lore.kernel.org/linux-samsung-soc \
		linux-samsung-soc@vger.kernel.org
	public-inbox-index linux-samsung-soc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-samsung-soc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git