[PATCH v1 0/2] Restore voltages before rebooting of NVIDIA Tegra SoCs

[PATCH v1 0/2] Restore voltages before rebooting of NVIDIA Tegra SoCs

[Dmitry Osipenko]

Hi,

Nikola Milosavljević reported that rebooting wasn't working properly on
Asus Transformer TF101, which is Tegra20-based tablet device.  We found
that TF101 and some other devices have bootloader which doesn't re-initialize
voltages properly on a reboot.  The problem is resolved by ensuring that
SoC voltages are at a levels that are suitable for the rebooting of the
SoC before reboot happens. This series adds reboot handler to the Tegra
regulator couplers, it bumps voltages on the reboot event.

Dmitry Osipenko (2):
  regulator: core: Add regulator_sync_voltage_rdev()
  soc/tegra: regulators: Bump voltages on system reboot

 drivers/regulator/core.c               | 23 ++++++++
 drivers/soc/tegra/regulators-tegra20.c | 77 ++++++++++++++++++++++++-
 drivers/soc/tegra/regulators-tegra30.c | 80 +++++++++++++++++++++++++-
 include/linux/regulator/driver.h       |  1 +
 4 files changed, 177 insertions(+), 4 deletions(-)

-- 
2.30.2

[PATCH v1 1/2] regulator: core: Add regulator_sync_voltage_rdev()

[Dmitry Osipenko]

Some NVIDIA Tegra devices use a CPU soft-reset method for the reboot and
in this case we need to restore the coupled voltages to the state that is
suitable for hardware during boot. Add new regulator_sync_voltage_rdev()
helper which is needed by regulator drivers in order to sync voltage of
a coupled regulators.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/regulator/core.c         | 23 +++++++++++++++++++++++
 include/linux/regulator/driver.h |  1 +
 2 files changed, 24 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index f192bf19492e..ead0b6d2af45 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -4105,6 +4105,29 @@ int regulator_set_voltage_time_sel(struct regulator_dev *rdev,
 }
 EXPORT_SYMBOL_GPL(regulator_set_voltage_time_sel);
 
+int regulator_sync_voltage_rdev(struct regulator_dev *rdev)
+{
+	int ret;
+
+	regulator_lock(rdev);
+
+	if (!rdev->desc->ops->set_voltage &&
+	    !rdev->desc->ops->set_voltage_sel) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* balance only, if regulator is coupled */
+	if (rdev->coupling_desc.n_coupled > 1)
+		ret = regulator_balance_voltage(rdev, PM_SUSPEND_ON);
+	else
+		ret = -EOPNOTSUPP;
+
+out:
+	regulator_unlock(rdev);
+	return ret;
+}
+
 /**
  * regulator_sync_voltage - re-apply last regulator output voltage
  * @regulator: regulator source
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 4ea520c248e9..35e5a611db81 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -540,6 +540,7 @@ int regulator_set_current_limit_regmap(struct regulator_dev *rdev,
 int regulator_get_current_limit_regmap(struct regulator_dev *rdev);
 void *regulator_get_init_drvdata(struct regulator_init_data *reg_init_data);
 int regulator_set_ramp_delay_regmap(struct regulator_dev *rdev, int ramp_delay);
+int regulator_sync_voltage_rdev(struct regulator_dev *rdev);
 
 /*
  * Helper functions intended to be used by regulator drivers prior registering
-- 
2.30.2

[PATCH v1 2/2] soc/tegra: regulators: Bump voltages on system reboot

[Dmitry Osipenko]

Ensure that SoC voltages are at a level suitable for a system reboot.
This is important for some devices that use CPU reset method for the
rebooting. SoC CPU and core voltages now are be restored to a level
that is suitable for rebooting. This patch fixes hang on reboot on
Asus Transformer TF101, it was also reported as fixing some of reboot
issues on Toshiba AC100.

Reported-by: Nikola Milosavljević <mnidza@outlook.com>
Tested-by: Nikola Milosavljević <mnidza@outlook.com> # TF101
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/soc/tegra/regulators-tegra20.c | 77 ++++++++++++++++++++++++-
 drivers/soc/tegra/regulators-tegra30.c | 80 +++++++++++++++++++++++++-
 2 files changed, 153 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/tegra/regulators-tegra20.c b/drivers/soc/tegra/regulators-tegra20.c
index e2c11d442591..81787ae3d03e 100644
--- a/drivers/soc/tegra/regulators-tegra20.c
+++ b/drivers/soc/tegra/regulators-tegra20.c
@@ -12,6 +12,7 @@
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/of.h>
+#include <linux/reboot.h>
 #include <linux/regulator/coupler.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
@@ -23,7 +24,10 @@ struct tegra_regulator_coupler {
 	struct regulator_dev *core_rdev;
 	struct regulator_dev *cpu_rdev;
 	struct regulator_dev *rtc_rdev;
-	int core_min_uV;
+	struct notifier_block reboot_notifier;
+	int core_min_uV, cpu_min_uV;
+	bool sys_reboot_mode_req;
+	bool sys_reboot_mode;
 };
 
 static inline struct tegra_regulator_coupler *
@@ -50,7 +54,7 @@ static int tegra20_core_limit(struct tegra_regulator_coupler *tegra,
 	 * This means that we can't fully allow CORE voltage scaling until
 	 * the state of all DVFS-critical CORE devices is synced.
 	 */
-	if (tegra_soc_core_domain_state_synced()) {
+	if (tegra_soc_core_domain_state_synced() && !tegra->sys_reboot_mode) {
 		pr_info_once("voltage state synced\n");
 		return 0;
 	}
@@ -259,6 +263,10 @@ static int tegra20_cpu_voltage_update(struct tegra_regulator_coupler *tegra,
 	if (cpu_uV < 0)
 		return cpu_uV;
 
+	/* store boot voltage level */
+	if (!tegra->cpu_min_uV)
+		tegra->cpu_min_uV = cpu_uV;
+
 	/*
 	 * CPU's regulator may not have any consumers, hence the voltage
 	 * must not be changed in that case because CPU simply won't
@@ -267,6 +275,10 @@ static int tegra20_cpu_voltage_update(struct tegra_regulator_coupler *tegra,
 	if (!cpu_min_uV_consumers)
 		cpu_min_uV = cpu_uV;
 
+	/* restore boot voltage level */
+	if (tegra->sys_reboot_mode)
+		cpu_min_uV = max(cpu_min_uV, tegra->cpu_min_uV);
+
 	if (cpu_min_uV > cpu_uV) {
 		err = tegra20_core_rtc_update(tegra, core_rdev, rtc_rdev,
 					      cpu_uV, cpu_min_uV);
@@ -307,6 +319,8 @@ static int tegra20_regulator_balance_voltage(struct regulator_coupler *coupler,
 		return -EINVAL;
 	}
 
+	tegra->sys_reboot_mode = READ_ONCE(tegra->sys_reboot_mode_req);
+
 	if (rdev == cpu_rdev)
 		return tegra20_cpu_voltage_update(tegra, cpu_rdev,
 						  core_rdev, rtc_rdev);
@@ -320,6 +334,51 @@ static int tegra20_regulator_balance_voltage(struct regulator_coupler *coupler,
 	return -EPERM;
 }
 
+static int tegra20_regulator_prepare_reboot(struct tegra_regulator_coupler *tegra,
+					    bool sys_reboot_mode)
+{
+	int err;
+
+	if (!tegra->core_rdev || !tegra->rtc_rdev || !tegra->cpu_rdev)
+		return 0;
+
+	WRITE_ONCE(tegra->sys_reboot_mode_req, true);
+
+	/*
+	 * Some devices use CPU soft-reboot method and in this case we
+	 * should ensure that voltages are sane for the reboot by restoring
+	 * the minimum boot levels.
+	 */
+	err = regulator_sync_voltage_rdev(tegra->cpu_rdev);
+	if (err)
+		return err;
+
+	err = regulator_sync_voltage_rdev(tegra->core_rdev);
+	if (err)
+		return err;
+
+	WRITE_ONCE(tegra->sys_reboot_mode_req, sys_reboot_mode);
+
+	return 0;
+}
+
+static int tegra20_regulator_reboot(struct notifier_block *notifier,
+				    unsigned long event, void *cmd)
+{
+	struct tegra_regulator_coupler *tegra;
+	int ret;
+
+	if (event != SYS_RESTART)
+		return NOTIFY_DONE;
+
+	tegra = container_of(notifier, struct tegra_regulator_coupler,
+			     reboot_notifier);
+
+	ret = tegra20_regulator_prepare_reboot(tegra, true);
+
+	return notifier_from_errno(ret);
+}
+
 static int tegra20_regulator_attach(struct regulator_coupler *coupler,
 				    struct regulator_dev *rdev)
 {
@@ -352,6 +411,14 @@ static int tegra20_regulator_detach(struct regulator_coupler *coupler,
 {
 	struct tegra_regulator_coupler *tegra = to_tegra_coupler(coupler);
 
+	/*
+	 * We don't expect regulators to be decoupled during reboot,
+	 * this may race with the reboot handler and shouldn't ever
+	 * happen in practice.
+	 */
+	if (WARN_ON_ONCE(system_state > SYSTEM_RUNNING))
+		return -EPERM;
+
 	if (tegra->core_rdev == rdev) {
 		tegra->core_rdev = NULL;
 		return 0;
@@ -376,13 +443,19 @@ static struct tegra_regulator_coupler tegra20_coupler = {
 		.detach_regulator = tegra20_regulator_detach,
 		.balance_voltage = tegra20_regulator_balance_voltage,
 	},
+	.reboot_notifier.notifier_call = tegra20_regulator_reboot,
 };
 
 static int __init tegra_regulator_coupler_init(void)
 {
+	int err;
+
 	if (!of_machine_is_compatible("nvidia,tegra20"))
 		return 0;
 
+	err = register_reboot_notifier(&tegra20_coupler.reboot_notifier);
+	WARN_ON(err);
+
 	return regulator_coupler_register(&tegra20_coupler.coupler);
 }
 arch_initcall(tegra_regulator_coupler_init);
diff --git a/drivers/soc/tegra/regulators-tegra30.c b/drivers/soc/tegra/regulators-tegra30.c
index 42d675b79fa3..e0203f78b396 100644
--- a/drivers/soc/tegra/regulators-tegra30.c
+++ b/drivers/soc/tegra/regulators-tegra30.c
@@ -12,6 +12,7 @@
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/of.h>
+#include <linux/reboot.h>
 #include <linux/regulator/coupler.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
@@ -23,7 +24,10 @@ struct tegra_regulator_coupler {
 	struct regulator_coupler coupler;
 	struct regulator_dev *core_rdev;
 	struct regulator_dev *cpu_rdev;
-	int core_min_uV;
+	struct notifier_block reboot_notifier;
+	int core_min_uV, cpu_min_uV;
+	bool sys_reboot_mode_req;
+	bool sys_reboot_mode;
 };
 
 static inline struct tegra_regulator_coupler *
@@ -50,7 +54,7 @@ static int tegra30_core_limit(struct tegra_regulator_coupler *tegra,
 	 * This means that we can't fully allow CORE voltage scaling until
 	 * the state of all DVFS-critical CORE devices is synced.
 	 */
-	if (tegra_soc_core_domain_state_synced()) {
+	if (tegra_soc_core_domain_state_synced() && !tegra->sys_reboot_mode) {
 		pr_info_once("voltage state synced\n");
 		return 0;
 	}
@@ -188,6 +192,10 @@ static int tegra30_voltage_update(struct tegra_regulator_coupler *tegra,
 	if (cpu_uV < 0)
 		return cpu_uV;
 
+	/* store boot voltage level */
+	if (!tegra->cpu_min_uV)
+		tegra->cpu_min_uV = cpu_uV;
+
 	/*
 	 * CPU's regulator may not have any consumers, hence the voltage
 	 * must not be changed in that case because CPU simply won't
@@ -211,6 +219,10 @@ static int tegra30_voltage_update(struct tegra_regulator_coupler *tegra,
 	if (err)
 		return err;
 
+	/* restore boot voltage level */
+	if (tegra->sys_reboot_mode)
+		cpu_min_uV = max(cpu_min_uV, tegra->cpu_min_uV);
+
 	if (core_min_limited_uV > core_uV) {
 		pr_err("core voltage constraint violated: %d %d %d\n",
 		       core_uV, core_min_limited_uV, cpu_uV);
@@ -279,9 +291,56 @@ static int tegra30_regulator_balance_voltage(struct regulator_coupler *coupler,
 		return -EINVAL;
 	}
 
+	tegra->sys_reboot_mode = READ_ONCE(tegra->sys_reboot_mode_req);
+
 	return tegra30_voltage_update(tegra, cpu_rdev, core_rdev);
 }
 
+static int tegra30_regulator_prepare_reboot(struct tegra_regulator_coupler *tegra,
+					    bool sys_reboot_mode)
+{
+	int err;
+
+	if (!tegra->core_rdev || !tegra->cpu_rdev)
+		return 0;
+
+	WRITE_ONCE(tegra->sys_reboot_mode_req, true);
+
+	/*
+	 * Some devices use CPU soft-reboot method and in this case we
+	 * should ensure that voltages are sane for the reboot by restoring
+	 * the minimum boot levels.
+	 */
+	err = regulator_sync_voltage_rdev(tegra->cpu_rdev);
+	if (err)
+		return err;
+
+	err = regulator_sync_voltage_rdev(tegra->core_rdev);
+	if (err)
+		return err;
+
+	WRITE_ONCE(tegra->sys_reboot_mode_req, sys_reboot_mode);
+
+	return 0;
+}
+
+static int tegra30_regulator_reboot(struct notifier_block *notifier,
+				    unsigned long event, void *cmd)
+{
+	struct tegra_regulator_coupler *tegra;
+	int ret;
+
+	if (event != SYS_RESTART)
+		return NOTIFY_DONE;
+
+	tegra = container_of(notifier, struct tegra_regulator_coupler,
+			     reboot_notifier);
+
+	ret = tegra30_regulator_prepare_reboot(tegra, true);
+
+	return notifier_from_errno(ret);
+}
+
 static int tegra30_regulator_attach(struct regulator_coupler *coupler,
 				    struct regulator_dev *rdev)
 {
@@ -308,6 +367,17 @@ static int tegra30_regulator_detach(struct regulator_coupler *coupler,
 {
 	struct tegra_regulator_coupler *tegra = to_tegra_coupler(coupler);
 
+	/*
+	 * We don't expect regulators to be decoupled during reboot,
+	 * this may race with the reboot handler and shouldn't ever
+	 * happen in practice.
+	 */
+	if (WARN_ON_ONCE(system_state > SYSTEM_RUNNING))
+		return -EPERM;
+
+	/* bring regulators to the state that is safe for reboot */
+	tegra30_regulator_prepare_reboot(tegra, false);
+
 	if (tegra->core_rdev == rdev) {
 		tegra->core_rdev = NULL;
 		return 0;
@@ -327,13 +397,19 @@ static struct tegra_regulator_coupler tegra30_coupler = {
 		.detach_regulator = tegra30_regulator_detach,
 		.balance_voltage = tegra30_regulator_balance_voltage,
 	},
+	.reboot_notifier.notifier_call = tegra30_regulator_reboot,
 };
 
 static int __init tegra_regulator_coupler_init(void)
 {
+	int err;
+
 	if (!of_machine_is_compatible("nvidia,tegra30"))
 		return 0;
 
+	err = register_reboot_notifier(&tegra30_coupler.reboot_notifier);
+	WARN_ON(err);
+
 	return regulator_coupler_register(&tegra30_coupler.coupler);
 }
 arch_initcall(tegra_regulator_coupler_init);
-- 
2.30.2

Re: [PATCH v1 1/2] regulator: core: Add regulator_sync_voltage_rdev()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 432 bytes --]

On Tue, May 11, 2021 at 01:05:25AM +0300, Dmitry Osipenko wrote:
> Some NVIDIA Tegra devices use a CPU soft-reset method for the reboot and
> in this case we need to restore the coupled voltages to the state that is
> suitable for hardware during boot. Add new regulator_sync_voltage_rdev()
> helper which is needed by regulator drivers in order to sync voltage of
> a coupled regulators.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 0/2] Restore voltages before rebooting of NVIDIA Tegra SoCs

[Michał Mirosław]

On Tue, May 11, 2021 at 01:05:24AM +0300, Dmitry Osipenko wrote:
> Hi,
> 
> Nikola Milosavljević reported that rebooting wasn't working properly on
> Asus Transformer TF101, which is Tegra20-based tablet device.  We found
> that TF101 and some other devices have bootloader which doesn't re-initialize
> voltages properly on a reboot.  The problem is resolved by ensuring that
> SoC voltages are at a levels that are suitable for the rebooting of the
> SoC before reboot happens. This series adds reboot handler to the Tegra
> regulator couplers, it bumps voltages on the reboot event.

Which tree does this series apply to?

Best Regards
Michał Mirosław

Re: [PATCH v1 0/2] Restore voltages before rebooting of NVIDIA Tegra SoCs

[Dmitry Osipenko]

15.05.2021 00:32, Michał Mirosław пишет:
> On Tue, May 11, 2021 at 01:05:24AM +0300, Dmitry Osipenko wrote:
>> Hi,
>>
>> Nikola Milosavljević reported that rebooting wasn't working properly on
>> Asus Transformer TF101, which is Tegra20-based tablet device.  We found
>> that TF101 and some other devices have bootloader which doesn't re-initialize
>> voltages properly on a reboot.  The problem is resolved by ensuring that
>> SoC voltages are at a levels that are suitable for the rebooting of the
>> SoC before reboot happens. This series adds reboot handler to the Tegra
>> regulator couplers, it bumps voltages on the reboot event.
> 
> Which tree does this series apply to?

It was made on top of linux-next, but it should apply to 5.13 as well.
What conflict do you get?

Re: [PATCH v1 0/2] Restore voltages before rebooting of NVIDIA Tegra SoCs

[Dmitry Osipenko]

15.05.2021 01:18, Dmitry Osipenko пишет:
> 15.05.2021 00:32, Michał Mirosław пишет:
>> On Tue, May 11, 2021 at 01:05:24AM +0300, Dmitry Osipenko wrote:
>>> Hi,
>>>
>>> Nikola Milosavljević reported that rebooting wasn't working properly on
>>> Asus Transformer TF101, which is Tegra20-based tablet device.  We found
>>> that TF101 and some other devices have bootloader which doesn't re-initialize
>>> voltages properly on a reboot.  The problem is resolved by ensuring that
>>> SoC voltages are at a levels that are suitable for the rebooting of the
>>> SoC before reboot happens. This series adds reboot handler to the Tegra
>>> regulator couplers, it bumps voltages on the reboot event.
>>
>> Which tree does this series apply to?
> 
> It was made on top of linux-next, but it should apply to 5.13 as well.
> What conflict do you get?
> 

I see where the problem is, there is no
tegra_soc_core_domain_state_synced() in upstream yet. I haven't rebased
patches properly, will be fixed in v2.