From: Matti Vaittinen Date: Tue, 10 Jun 2025 05:32:06 +0000 (+0300) Subject: regulator: bd718x7: Clarify comment by moving it X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=55d9fd9819de09e70401b3b5262ff46d5de951b7;p=users%2Fjedix%2Flinux-maple.git 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 Link: https://patch.msgid.link/a90cb77e66a253f4055bbb99672dc81c7457de66.1749533040.git.mazziesaccount@gmail.com Signed-off-by: Mark Brown --- diff --git a/drivers/regulator/bd718x7-regulator.c b/drivers/regulator/bd718x7-regulator.c index 1bb048de3ecd5..e803cc59d68a5 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;