diff options
| author | Matti Vaittinen <mazziesaccount@gmail.com> | 2025-06-10 08:32:06 +0300 |
|---|---|---|
| committer | Mark Brown <broonie@kernel.org> | 2025-06-11 12:46:05 +0100 |
| commit | 55d9fd9819de09e70401b3b5262ff46d5de951b7 (patch) | |
| tree | 50e95998d1c1f137826cc0edc5d2533dc635a579 | |
| parent | d6fa0ca959db8efd4462d7beef4bdc5568640fd0 (diff) | |
regulator: bd718x7: Clarify comment by moving it
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>
| -rw-r--r-- | drivers/regulator/bd718x7-regulator.c | 27 |
1 files 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; |