[PATCH] regulator: bd718x7: Clarify comment by moving it

[PATCH] regulator: bd718x7: Clarify comment by moving it

[Matti Vaittinen]

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

The BD718x7 needs to disable voltage monitoring for a duration of
certain voltage changes.

The comment explaining use of msleep(1) instead of a more accurate
delay(), was placed to a function which disabled the protection. The
actual sleeping is done in a different place of the code, after the
voltage has been changed.

Browsing through the comment and code after the years made me to scratch
my head for a second. I may have figured why me and so many fellow
developers are slowly getting bald.

Clarify things a bit and move the comment about required delay directly
above the sleep. Leave only a small comment explaining why the protection
is disabled to the spot where the logic for disabling is.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
 drivers/regulator/bd718x7-regulator.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/regulator/bd718x7-regulator.c b/drivers/regulator/bd718x7-regulator.c
index 1bb048de3ecd..e803cc59d68a 100644
--- a/drivers/regulator/bd718x7-regulator.c
+++ b/drivers/regulator/bd718x7-regulator.c
@@ -134,9 +134,19 @@ static void voltage_change_done(struct regulator_dev *rdev, unsigned int sel,
 
 	if (*mask) {
 		/*
-		 * Let's allow scheduling as we use I2C anyways. We just need to
-		 * guarantee minimum of 1ms sleep - it shouldn't matter if we
-		 * exceed it due to the scheduling.
+		 * We had fault detection disabled for the duration of the
+		 * voltage change.
+		 *
+		 * According to HW colleagues the maximum time it takes is
+		 * 1000us. I assume that on systems with light load this
+		 * might be less - and we could probably use DT to give
+		 * system specific delay value if performance matters.
+		 *
+		 * Well, knowing we use I2C here and can add scheduling delays
+		 * I don't think it is worth the hassle and I just add fixed
+		 * 1ms sleep here (and allow scheduling). If this turns out to
+		 * be a problem we can change it to delay and make the delay
+		 * time configurable.
 		 */
 		msleep(1);
 
@@ -173,16 +183,7 @@ static int voltage_change_prepare(struct regulator_dev *rdev, unsigned int sel,
 		/*
 		 * If we increase LDO voltage when LDO is enabled we need to
 		 * disable the power-good detection until voltage has reached
-		 * the new level. According to HW colleagues the maximum time
-		 * it takes is 1000us. I assume that on systems with light load
-		 * this might be less - and we could probably use DT to give
-		 * system specific delay value if performance matters.
-		 *
-		 * Well, knowing we use I2C here and can add scheduling delays
-		 * I don't think it is worth the hassle and I just add fixed
-		 * 1ms sleep here (and allow scheduling). If this turns out to
-		 * be a problem we can change it to delay and make the delay
-		 * time configurable.
+		 * the new level.
 		 */
 		if (new > now) {
 			int tmp;
-- 
2.49.0


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

Re: [PATCH] regulator: bd718x7: Clarify comment by moving it

[Mark Brown]

On Tue, 10 Jun 2025 08:32:06 +0300, Matti Vaittinen wrote:
> The BD718x7 needs to disable voltage monitoring for a duration of
> certain voltage changes.
> 
> The comment explaining use of msleep(1) instead of a more accurate
> delay(), was placed to a function which disabled the protection. The
> actual sleeping is done in a different place of the code, after the
> voltage has been changed.
> 
> [...]

Applied to

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

Thanks!

[1/1] regulator: bd718x7: Clarify comment by moving it
      commit: 55d9fd9819de09e70401b3b5262ff46d5de951b7

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