linux-kernel - [PATCH] regulator: bd718x7: Clarify comment by moving it

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <a90cb77e66a253f4055bbb99672dc81c7457de66.1749533040.git.mazziesaccount@gmail.com>
Date: Tue, 10 Jun 2025 08:32:06 +0300
From: Matti Vaittinen <mazziesaccount@...il.com>
To: Matti Vaittinen <mazziesaccount@...il.com>,
	Matti Vaittinen <matti.vaittinen@...rohmeurope.com>
Cc: Matti Vaittinen <mazziesaccount@...il.com>,
	Liam Girdwood <lgirdwood@...il.com>,
	Mark Brown <broonie@...nel.org>, linux-kernel@...r.kernel.org
Subject: [PATCH] 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@...il.com>
---
 drivers/regulator/bd718x7-regulator.c | 27 ++++++++++++++-------------
 1 file 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;
-- 
2.49.0


Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)