]> www.infradead.org Git - users/jedix/linux-maple.git/commitdiff
regulator: bd718x7: Clarify comment by moving it
authorMatti Vaittinen <mazziesaccount@gmail.com>
Tue, 10 Jun 2025 05:32:06 +0000 (08:32 +0300)
committerMark Brown <broonie@kernel.org>
Wed, 11 Jun 2025 11:46:05 +0000 (12:46 +0100)
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>
Link: https://patch.msgid.link/a90cb77e66a253f4055bbb99672dc81c7457de66.1749533040.git.mazziesaccount@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
drivers/regulator/bd718x7-regulator.c

index 1bb048de3ecd5a8df1087a48afc728a64623a024..e803cc59d68a5c790c4278f68f5ba7d62e75773a 100644 (file)
@@ -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;