Skip to content

Commit 55d9fd9

Browse files
M-Vaittinenbroonie
authored andcommitted
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 <[email protected]> Link: https://patch.msgid.link/a90cb77e66a253f4055bbb99672dc81c7457de66.1749533040.git.mazziesaccount@gmail.com Signed-off-by: Mark Brown <[email protected]>
1 parent d6fa0ca commit 55d9fd9

File tree

1 file changed

+14
-13
lines changed

1 file changed

+14
-13
lines changed

drivers/regulator/bd718x7-regulator.c

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,19 @@ static void voltage_change_done(struct regulator_dev *rdev, unsigned int sel,
134134

135135
if (*mask) {
136136
/*
137-
* Let's allow scheduling as we use I2C anyways. We just need to
138-
* guarantee minimum of 1ms sleep - it shouldn't matter if we
139-
* exceed it due to the scheduling.
137+
* We had fault detection disabled for the duration of the
138+
* voltage change.
139+
*
140+
* According to HW colleagues the maximum time it takes is
141+
* 1000us. I assume that on systems with light load this
142+
* might be less - and we could probably use DT to give
143+
* system specific delay value if performance matters.
144+
*
145+
* Well, knowing we use I2C here and can add scheduling delays
146+
* I don't think it is worth the hassle and I just add fixed
147+
* 1ms sleep here (and allow scheduling). If this turns out to
148+
* be a problem we can change it to delay and make the delay
149+
* time configurable.
140150
*/
141151
msleep(1);
142152

@@ -173,16 +183,7 @@ static int voltage_change_prepare(struct regulator_dev *rdev, unsigned int sel,
173183
/*
174184
* If we increase LDO voltage when LDO is enabled we need to
175185
* disable the power-good detection until voltage has reached
176-
* the new level. According to HW colleagues the maximum time
177-
* it takes is 1000us. I assume that on systems with light load
178-
* this might be less - and we could probably use DT to give
179-
* system specific delay value if performance matters.
180-
*
181-
* Well, knowing we use I2C here and can add scheduling delays
182-
* I don't think it is worth the hassle and I just add fixed
183-
* 1ms sleep here (and allow scheduling). If this turns out to
184-
* be a problem we can change it to delay and make the delay
185-
* time configurable.
186+
* the new level.
186187
*/
187188
if (new > now) {
188189
int tmp;

0 commit comments

Comments
 (0)