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 <[email protected]>
---
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
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 <[email protected]>:
>
> 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 <[email protected]>
> ---
> 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
+CC mmc/rockchip folks
> Am 17.07.2022 um 13:39 schrieb Christian Kohlschütter <[email protected]>:
>
> 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 <[email protected]>:
>>
>> 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 <[email protected]>
>> ---
>> 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
>
>
>
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.
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 <[email protected]>
---
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
> Am 19.07.2022 um 15:57 schrieb Mark Brown <[email protected]>:
>
> On Mon, Jul 18, 2022 at 07:24:37PM +0200, Christian Kohlschuetter wrote:
>
>> Signed-off-by: Christian Kohlschütter <[email protected]>
>
> 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.
On Mon, Jul 18, 2022 at 07:24:37PM +0200, Christian Kohlschuetter wrote:
> Signed-off-by: Christian Kohlsch?tter <[email protected]>
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.
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 <[email protected]>
---
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
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...
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).
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
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
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
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 <[email protected]>
---
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
Woohoo, my first patch — thanks a lot, Mark!
> Am 19.07.2022 um 20:48 schrieb Mark Brown <[email protected]>:
>
> 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
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.
> Am 19.07.2022 um 21:38 schrieb Mark Brown <[email protected]>:
>
> 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!