[PATCH] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators

[PATCH] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators

[Christian Kohlschuetter]

Regulators marked with "regulator-always-on" or "regulator-boot-on"
as well as an "off-on-delay-us", may run into cycling issues that are
hard to detect.

This is caused by the "last_off" state not being initialized in this
case.

Fix the "last_off" initialization by setting it to the current kernel
time upon initialization, regardless of always_on/boot_on state.

Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>
---
 drivers/regulator/core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index c4d844ffad7a..48ed33ad48c8 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1522,6 +1522,9 @@ static int set_machine_constraints(struct regulator_dev *rdev)
 		}
 	}
 
+	if (rdev->desc->off_on_delay)
+		rdev->last_off = ktime_get();
+
 	/* If the constraints say the regulator should be on at this point
 	 * and we have control then make sure it is enabled.
 	 */
@@ -1549,8 +1552,6 @@ static int set_machine_constraints(struct regulator_dev *rdev)
 
 		if (rdev->constraints->always_on)
 			rdev->use_count++;
-	} else if (rdev->desc->off_on_delay) {
-		rdev->last_off = ktime_get();
 	}
 
 	print_constraints(rdev);
-- 
2.36.1

Re: [PATCH] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators

[Christian Kohlschütter]

This is _somewhat_ related to "[PATCH] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators"
That other patch is the right solution for my specific problem, and no further fixes are necessary.

This change fixes an attempted "bandage" solution (adding an "off-on-delay-us") that we had initially tried to no avail.
While the cleanup not only reduces complexity, it may also prevent future code changes reintroducing the non-delayed cycling after registration.

from arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi:
vcc3v0_sd: vcc3v0-sd {
        compatible = "regulator-fixed";
        enable-active-high;
        gpio = <&gpio0 RK_PA1 GPIO_ACTIVE_HIGH>;
        pinctrl-names = "default";
        pinctrl-0 = <&sdmmc0_pwr_h>;
        regulator-always-on; // prevents setting last_off upon registration
        off-on-delay-us = <500000>; // would not be honored upon regulator_register
        regulator-min-microvolt = <3000000>;
        regulator-max-microvolt = <3000000>;
        regulator-name = "vcc3v0_sd";
        vin-supply = <&vcc3v3_sys>;
};

> Am 17.07.2022 um 13:30 schrieb Christian Kohlschuetter <christian@kohlschuetter.com>:
> 
> Regulators marked with "regulator-always-on" or "regulator-boot-on"
> as well as an "off-on-delay-us", may run into cycling issues that are
> hard to detect.
> 
> This is caused by the "last_off" state not being initialized in this
> case.
> 
> Fix the "last_off" initialization by setting it to the current kernel
> time upon initialization, regardless of always_on/boot_on state.
> 
> Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>
> ---
> drivers/regulator/core.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index c4d844ffad7a..48ed33ad48c8 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -1522,6 +1522,9 @@ static int set_machine_constraints(struct regulator_dev *rdev)
> 		}
> 	}
> 
> +	if (rdev->desc->off_on_delay)
> +		rdev->last_off = ktime_get();
> +
> 	/* If the constraints say the regulator should be on at this point
> 	 * and we have control then make sure it is enabled.
> 	 */
> @@ -1549,8 +1552,6 @@ static int set_machine_constraints(struct regulator_dev *rdev)
> 
> 		if (rdev->constraints->always_on)
> 			rdev->use_count++;
> -	} else if (rdev->desc->off_on_delay) {
> -		rdev->last_off = ktime_get();
> 	}
> 
> 	print_constraints(rdev);
> -- 
> 2.36.1
> 
> 



-- 
Dr. Christian Kohlschütter

Re: [PATCH] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators

[Christian Kohlschütter]

+CC mmc/rockchip folks

> Am 17.07.2022 um 13:39 schrieb Christian Kohlschütter <christian@kohlschutter.com>:
> 
> This is _somewhat_ related to "[PATCH] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators"
> That other patch is the right solution for my specific problem, and no further fixes are necessary.
> 
> This change fixes an attempted "bandage" solution (adding an "off-on-delay-us") that we had initially tried to no avail.
> While the cleanup not only reduces complexity, it may also prevent future code changes reintroducing the non-delayed cycling after registration.
> 
> from arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi:
> vcc3v0_sd: vcc3v0-sd {
>        compatible = "regulator-fixed";
>        enable-active-high;
>        gpio = <&gpio0 RK_PA1 GPIO_ACTIVE_HIGH>;
>        pinctrl-names = "default";
>        pinctrl-0 = <&sdmmc0_pwr_h>;
>        regulator-always-on; // prevents setting last_off upon registration
>        off-on-delay-us = <500000>; // would not be honored upon regulator_register
>        regulator-min-microvolt = <3000000>;
>        regulator-max-microvolt = <3000000>;
>        regulator-name = "vcc3v0_sd";
>        vin-supply = <&vcc3v3_sys>;
> };
> 
>> Am 17.07.2022 um 13:30 schrieb Christian Kohlschuetter <christian@kohlschuetter.com>:
>> 
>> Regulators marked with "regulator-always-on" or "regulator-boot-on"
>> as well as an "off-on-delay-us", may run into cycling issues that are
>> hard to detect.
>> 
>> This is caused by the "last_off" state not being initialized in this
>> case.
>> 
>> Fix the "last_off" initialization by setting it to the current kernel
>> time upon initialization, regardless of always_on/boot_on state.
>> 
>> Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>
>> ---
>> drivers/regulator/core.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
>> index c4d844ffad7a..48ed33ad48c8 100644
>> --- a/drivers/regulator/core.c
>> +++ b/drivers/regulator/core.c
>> @@ -1522,6 +1522,9 @@ static int set_machine_constraints(struct regulator_dev *rdev)
>> 		}
>> 	}
>> 
>> +	if (rdev->desc->off_on_delay)
>> +		rdev->last_off = ktime_get();
>> +
>> 	/* If the constraints say the regulator should be on at this point
>> 	 * and we have control then make sure it is enabled.
>> 	 */
>> @@ -1549,8 +1552,6 @@ static int set_machine_constraints(struct regulator_dev *rdev)
>> 
>> 		if (rdev->constraints->always_on)
>> 			rdev->use_count++;
>> -	} else if (rdev->desc->off_on_delay) {
>> -		rdev->last_off = ktime_get();
>> 	}
>> 
>> 	print_constraints(rdev);
>> -- 
>> 2.36.1
>> 
>> 
> 
> 
> 
> -- 
> Dr. Christian Kohlschütter
> 
> 
> 

Re: [PATCH] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators

[Mark Brown]

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

On Sun, Jul 17, 2022 at 01:30:36PM +0200, Christian Kohlschuetter wrote:
> Regulators marked with "regulator-always-on" or "regulator-boot-on"
> as well as an "off-on-delay-us", may run into cycling issues that are
> hard to detect.

This doesn't apply against current code, please check and resend.

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

[PATCH REBASE] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators

[Christian Kohlschuetter]

Regulators marked with "regulator-always-on" or "regulator-boot-on"
as well as an "off-on-delay-us", may run into cycling issues that are
hard to detect.

This is caused by the "last_off" state not being initialized in this
case.

Fix the "last_off" initialization by setting it to the current kernel
time upon initialization, regardless of always_on/boot_on state.

Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>
---
 drivers/regulator/core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 1e54a833f..398c8d6af 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1565,6 +1565,9 @@ static int set_machine_constraints(struct regulator_dev *rdev)
 			rdev->constraints->always_on = true;
 	}
 
+	if (rdev->desc->off_on_delay)
+		rdev->last_off = ktime_get();
+
 	/* If the constraints say the regulator should be on at this point
 	 * and we have control then make sure it is enabled.
 	 */
@@ -1592,8 +1595,6 @@ static int set_machine_constraints(struct regulator_dev *rdev)
 
 		if (rdev->constraints->always_on)
 			rdev->use_count++;
-	} else if (rdev->desc->off_on_delay) {
-		rdev->last_off = ktime_get();
 	}
 
 	print_constraints(rdev);
-- 
2.36.1

Re: [PATCH] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators

[Mark Brown]

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

On Mon, Jul 18, 2022 at 07:24:37PM +0200, Christian Kohlschuetter wrote:

> Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>

You have an umlaut in your signoff here but not in your e-mail address
which causes tooling to complain that there's a missing signoff - you
might get some complaints about this.

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

[PATCH REBASE] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators

[Christian Kohlschütter]

Regulators marked with "regulator-always-on" or "regulator-boot-on"
as well as an "off-on-delay-us", may run into cycling issues that are
hard to detect.

This is caused by the "last_off" state not being initialized in this
case.

Fix the "last_off" initialization by setting it to the current kernel
time upon initialization, regardless of always_on/boot_on state.

Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>
---
drivers/regulator/core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 1e54a833f..398c8d6af 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1565,6 +1565,9 @@ static int set_machine_constraints(struct regulator_dev *rdev)
			rdev->constraints->always_on = true;
	}

+	if (rdev->desc->off_on_delay)
+		rdev->last_off = ktime_get();
+
	/* If the constraints say the regulator should be on at this point
	 * and we have control then make sure it is enabled.
	 */
@@ -1592,8 +1595,6 @@ static int set_machine_constraints(struct regulator_dev *rdev)

		if (rdev->constraints->always_on)
			rdev->use_count++;
-	} else if (rdev->desc->off_on_delay) {
-		rdev->last_off = ktime_get();
	}

	print_constraints(rdev);
-- 
2.36.1

Re: [PATCH] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators

[Christian Kohlschuetter]

> Am 19.07.2022 um 15:57 schrieb Mark Brown <broonie@kernel.org>:
> 
> On Mon, Jul 18, 2022 at 07:24:37PM +0200, Christian Kohlschuetter wrote:
> 
>> Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>
> 
> You have an umlaut in your signoff here but not in your e-mail address
> which causes tooling to complain that there's a missing signoff - you
> might get some complaints about this.

dang! Thanks for noticing. I've just sent a "rebased" patch.

Re: [PATCH REBASE] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators

[Mark Brown]

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

On Tue, Jul 19, 2022 at 04:02:00PM +0200, Christian Kohlschütter wrote:
> Regulators marked with "regulator-always-on" or "regulator-boot-on"
> as well as an "off-on-delay-us", may run into cycling issues that are
> hard to detect.

This doesn't apply against current code, please check and resend...

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

Re: [PATCH REBASE] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators

[Mark Brown]

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

On Tue, Jul 19, 2022 at 04:02:00PM +0200, Christian Kohlschütter wrote:
> Regulators marked with "regulator-always-on" or "regulator-boot-on"
> as well as an "off-on-delay-us", may run into cycling issues that are
> hard to detect.

Specifically the error I'm seeing is

Applying: regulator: core: Fix off-on-delay-us for always-on/boot-on regulators
error: corrupt patch at line 10
error: could not build fake ancestor
Patch failed at 0001 regulator: core: Fix off-on-delay-us for always-on/boot-on regulators

(after fetching with b4).

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

Re: [PATCH] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators

[Mark Brown]

On Sun, 17 Jul 2022 13:30:36 +0200, Christian Kohlschuetter wrote:
> Regulators marked with "regulator-always-on" or "regulator-boot-on"
> as well as an "off-on-delay-us", may run into cycling issues that are
> hard to detect.
> 
> This is caused by the "last_off" state not being initialized in this
> case.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/1] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators
      commit: 218320fec29430438016f88dd4fbebfa1b95ad8d

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH REBASE] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators

[Mark Brown]

On Mon, 18 Jul 2022 19:24:37 +0200, Christian Kohlschuetter wrote:
> Regulators marked with "regulator-always-on" or "regulator-boot-on"
> as well as an "off-on-delay-us", may run into cycling issues that are
> hard to detect.
> 
> This is caused by the "last_off" state not being initialized in this
> case.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/1] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators
      commit: 218320fec29430438016f88dd4fbebfa1b95ad8d

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH REBASE] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators

[Mark Brown]

On Tue, 19 Jul 2022 16:02:00 +0200, Christian Kohlschütter wrote:
> Regulators marked with "regulator-always-on" or "regulator-boot-on"
> as well as an "off-on-delay-us", may run into cycling issues that are
> hard to detect.
> 
> This is caused by the "last_off" state not being initialized in this
> case.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/1] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators
      commit: 218320fec29430438016f88dd4fbebfa1b95ad8d

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

[PATCH REBASE] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators

[Christian Kohlschütter]

Regulators marked with "regulator-always-on" or "regulator-boot-on"
as well as an "off-on-delay-us", may run into cycling issues that are
hard to detect.

This is caused by the "last_off" state not being initialized in this
case.

Fix the "last_off" initialization by setting it to the current kernel
time upon initialization, regardless of always_on/boot_on state.

Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>
---
 drivers/regulator/core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 1e54a833f2c..398c8d6afd4 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1565,6 +1565,9 @@ static int set_machine_constraints(struct regulator_dev *rdev)
 			rdev->constraints->always_on = true;
 	}
 
+	if (rdev->desc->off_on_delay)
+		rdev->last_off = ktime_get();
+
 	/* If the constraints say the regulator should be on at this point
 	 * and we have control then make sure it is enabled.
 	 */
@@ -1592,8 +1595,6 @@ static int set_machine_constraints(struct regulator_dev *rdev)
 
 		if (rdev->constraints->always_on)
 			rdev->use_count++;
-	} else if (rdev->desc->off_on_delay) {
-		rdev->last_off = ktime_get();
 	}
 
 	print_constraints(rdev);
-- 
2.36.2

Re: [PATCH REBASE] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators

[Christian Kohlschütter]

Woohoo, my first patch — thanks a lot, Mark!

> Am 19.07.2022 um 20:48 schrieb Mark Brown <broonie@kernel.org>:
> 
> On Tue, 19 Jul 2022 16:02:00 +0200, Christian Kohlschütter wrote:
>> Regulators marked with "regulator-always-on" or "regulator-boot-on"
>> as well as an "off-on-delay-us", may run into cycling issues that are
>> hard to detect.
>> 
>> This is caused by the "last_off" state not being initialized in this
>> case.
>> 
>> [...]
> 
> Applied to
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
> 
> Thanks!
> 
> [1/1] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators
>      commit: 218320fec29430438016f88dd4fbebfa1b95ad8d
> 
> All being well this means that it will be integrated into the linux-next
> tree (usually sometime in the next 24 hours) and sent to Linus during
> the next merge window (or sooner if it is a bug fix), however if
> problems are discovered then the patch may be dropped or reverted.
> 
> You may get further e-mails resulting from automated or manual testing
> and review of the tree, please engage with people reporting problems and
> send followup patches addressing any issues that are reported if needed.
> 
> If any updates are required or you are submitting further changes they
> should be sent as incremental updates against current git, existing
> patches will not be replaced.
> 
> Please add any relevant lists and maintainers to the CCs when replying
> to this mail.
> 
> Thanks,
> Mark

Re: [PATCH REBASE] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators

[Mark Brown]

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

On Tue, Jul 19, 2022 at 06:49:44PM +0000, Christian Kohlschütter wrote:
> Regulators marked with "regulator-always-on" or "regulator-boot-on"
> as well as an "off-on-delay-us", may run into cycling issues that are
> hard to detect.

I think this already got applied, I had another go at persuading things
to cope which seemed to work - not 100% sure what was going on, git
seemed less forgiving than raw patch here.  It didn't look like a
rebasing issue, it looked like the umlaut was upsetting git.  Should be
commit 218320fec29430438016f88dd4fbebfa1b95ad8d in my tree.

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

Re: [PATCH REBASE] regulator: core: Fix off-on-delay-us for always-on/boot-on regulators

[Christian Kohlschütter]

> Am 19.07.2022 um 21:38 schrieb Mark Brown <broonie@kernel.org>:
> 
> On Tue, Jul 19, 2022 at 06:49:44PM +0000, Christian Kohlschütter wrote:
>> Regulators marked with "regulator-always-on" or "regulator-boot-on"
>> as well as an "off-on-delay-us", may run into cycling issues that are
>> hard to detect.
> 
> I think this already got applied, I had another go at persuading things
> to cope which seemed to work - not 100% sure what was going on, git
> seemed less forgiving than raw patch here.  It didn't look like a
> rebasing issue, it looked like the umlaut was upsetting git.  Should be
> commit 218320fec29430438016f88dd4fbebfa1b95ad8d in my tree.

Yes, I just got your emails after I resubmitted. I had thought my email client had messed things up, and I had resent it with a proper git send-email setup.
Thanks again!