Unverified Commit 55d9fd98 authored by Matti Vaittinen's avatar Matti Vaittinen Committed by Mark Brown
Browse files

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: default avatarMatti Vaittinen <mazziesaccount@gmail.com>
Link: https://patch.msgid.link/a90cb77e66a253f4055bbb99672dc81c7457de66.1749533040.git.mazziesaccount@gmail.com


Signed-off-by: default avatarMark Brown <broonie@kernel.org>
parent d6fa0ca9

drivers/regulator/bd718x7-regulator.c

+14 −13
Original line number Diff line number Diff line
@@ -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;