From: Saravana Kannan <saravanak@google.com>
To: Liam Girdwood <lgirdwood@gmail.com>,
Mark Brown <broonie@kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Saravana Kannan <saravanak@google.com>,
Marek Szyprowski <m.szyprowski@samsung.com>,
John Stultz <john.stultz@linaro.org>,
linux-kernel@vger.kernel.org, kernel-team@android.com
Subject: [PATCH v3 4/4] regulator: core: Add voltage support for sync_state() callbacks
Date: Wed, 15 Jul 2020 21:20:53 -0700 [thread overview]
Message-ID: <20200716042053.1927676-5-saravanak@google.com> (raw)
In-Reply-To: <20200716042053.1927676-1-saravanak@google.com>
Consider the following example:
- regulator-X is provided by device-X.
- regulator-X is a supplier to device-A, device-B and device-C.
- device-A is off/inactive from boot.
- device-B and device-C are left on/active by the bootloader
- regulator-X is left on boot by the bootloader at 2000 mV to supply
device-B and device-C.
Example boot sequence:
1. device-X is probed successfully.
2. device-B is probed by driver-B
a. driver-B gets regulator-X
b. driver-B votes on regulator-X
c. driver-B lowers device-B performance point.
d. driver-B lowers voltage vote to 1000 mV.
e. regulator-X voltage is lowered to 1000 mV.
3. System crashes or device-C becomes unreliable because regulator-X
voltage was lowered to 1000 mV when device-C still needed it at 2000 mV
The issue reported by Marek Szyprowski [1] between vdd_int and vdd_arm
is very similar to example 2, except driver-B lowers the voltage of
device-C due to a regulator coupling instead of a direct request from
driver-B.
This patch addresses the problem in the example by:
1. When a boot-on regulator is registered, a minimum voltage limit that
matches the boot time voltage is placed on the regulator.
2. When the regulator_sync_state() callback comes, the minimum voltage
limit is removed along with the rest of the boot limits.
So with this patch and regulator_cleanup_timeout=0, the example will
work as follows:
1. device-X is probed successfully.
a. regulator-X is registered.
b. Since regulator-X is on, a minimum voltage of 2000 mV is made by
the BOOT-LIMITS consumer.
c. Since regulator-X is on, an enable vote is made by the BOOT-LIMITS
consumer.
2. device-B is probed by driver-B
a. driver-B gets regulator-X
b. driver-B votes on regulator-X
c. driver-B lowers device-B performance point.
d. driver-B lowers voltage vote to 1000 mV.
e. regulator-X voltage is NOT lowered to 1000 mV.
3. device-C is probed by driver-C
a. Does stuff similar to device-B.
4. Since all consumers of device-X have probed, device-X gets
sync_state() callback which is a call to regulator_sync_state():
a. BOOT-LIMITS removes enable vote for regulator-X
b. regulator-X remains on becaue device-B and device-C have their
enable votes in.
c. BOOT-LIMITS consumer is freed.
d. regulator-X voltage drops is lowered to 1000 mV.
5. The system is stable because regulator-X voltage is NOT lowered to
1000 mV when device-C still needed it at 2000 mV.
[1] - https://lore.kernel.org/lkml/20200605063724.9030-1-m.szyprowski@samsung.com/#t
Signed-off-by: Saravana Kannan <saravanak@google.com>
---
drivers/regulator/core.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index f10ef6ec1af1..9b70295820f3 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1783,6 +1783,8 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev,
*/
static void regulator_add_boot_limits(struct regulator_dev *rdev)
{
+ int boot_uV;
+
/* We already set up boot limits. */
if (rdev->boot_limits)
return;
@@ -1815,6 +1817,13 @@ static void regulator_add_boot_limits(struct regulator_dev *rdev)
}
rdev->open_count++;
+ if (regulator_ops_is_valid(rdev, REGULATOR_CHANGE_VOLTAGE)) {
+ boot_uV = regulator_get_voltage_rdev(rdev);
+ if (boot_uV > 0)
+ regulator_set_voltage(rdev->boot_limits, boot_uV,
+ INT_MAX);
+ }
+
if (regulator_enable(rdev->boot_limits)) {
dev_err(&rdev->dev, "Unable to set boot limits\n");
destroy_regulator(rdev->boot_limits);
@@ -1847,10 +1856,12 @@ static int regulator_del_boot_limits(struct regulator_dev *rdev, bool handoff)
return 0;
rdev_info(rdev, "removing boot limits\n");
- if (!handoff)
+ if (!handoff) {
regulator_disable(rdev->boot_limits);
- else
+ regulator_set_voltage(rdev->boot_limits, 0, INT_MAX);
+ } else {
rdev->use_count--;
+ }
destroy_regulator(rdev->boot_limits);
/*
* Set it to an error value so that boot limits can't be set again once
--
2.28.0.rc0.105.gf9edc3c819-goog
next prev parent reply other threads:[~2020-07-16 4:21 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-16 4:20 [PATCH v3 0/4] regulator_sync_state() support Saravana Kannan
2020-07-16 4:20 ` [PATCH v3 1/4] driver core: Add dev_set_drv_sync_state() Saravana Kannan
2020-07-16 4:20 ` [PATCH v3 2/4] regulator: core: Add destroy_regulator() Saravana Kannan
2020-07-16 4:20 ` [PATCH v3 3/4] regulator: core: Add basic enable/disable support for sync_state() callbacks Saravana Kannan
2020-07-20 14:27 ` Mark Brown
2020-07-21 3:22 ` Saravana Kannan
2020-07-21 20:18 ` Mark Brown
2020-07-28 21:14 ` Saravana Kannan
2020-08-04 21:10 ` Mark Brown
2020-08-25 19:58 ` Mark Brown
2020-07-16 4:20 ` Saravana Kannan [this message]
2020-07-20 14:35 ` [PATCH v3 4/4] regulator: core: Add voltage " Mark Brown
2020-07-21 3:24 ` Saravana Kannan
2020-07-21 12:19 ` Mark Brown
2020-07-21 3:29 ` [PATCH v3 0/4] regulator_sync_state() support Saravana Kannan
2020-07-22 0:57 ` 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=20200716042053.1927676-5-saravanak@google.com \
--to=saravanak@google.com \
--cc=broonie@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=john.stultz@linaro.org \
--cc=kernel-team@android.com \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).