All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Halaney <ahalaney@redhat.com>
To: lgirdwood@gmail.com, broonie@kernel.org
Cc: linux-kernel@vger.kernel.org, bmasney@redhat.com,
	dianders@chromium.org, Andrew Halaney <ahalaney@redhat.com>
Subject: [PATCH] regulator: core: Clean up on enable failure
Date: Fri, 19 Aug 2022 14:43:36 -0500	[thread overview]
Message-ID: <20220819194336.382740-1-ahalaney@redhat.com> (raw)

If regulator_enable() fails, enable_count is incremented still.
A consumer, assuming no matching regulator_disable() is necessary on
failure, will then get this error message upon regulator_put()
since enable_count is non-zero:

    [    1.277418] WARNING: CPU: 3 PID: 1 at drivers/regulator/core.c:2304 _regulator_put.part.0+0x168/0x170

The consumer could try to fix this in their driver by cleaning up on
error from regulator_enable() (i.e. call regulator_disable()), but that
results in the following since regulator_enable() failed and didn't
increment user_count:

    [    1.258112] unbalanced disables for vreg_l17c
    [    1.262606] WARNING: CPU: 4 PID: 1 at drivers/regulator/core.c:2899 _regulator_disable+0xd4/0x190

Fix this by decrementing enable_count upon failure to enable.

With this in place, just the reason for failure to enable is printed
as expected and developers can focus on the root cause of their issue
instead of thinking their usage of the regulator consumer api is
incorrect. For example, in my case:

    [    1.240426] vreg_l17c: invalid input voltage found

Fixes: 5451781dadf8 ("regulator: core: Only count load for enabled consumers")
Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
---

I'm new to using the regulator framework, but I _believe_ this is a
cosmetic bug that's fixed by this patch. I went down a bit of a rabbit
hole because of the original WARN() message, so I'm trying to prevent
others from doing the same :)

Please let me know what you think, I tested on the misconfigured system
and on a working system and things seemed to work as expected.

Thanks,
Andrew

 drivers/regulator/core.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index d8373cb04f90..d3e8dc32832d 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2733,13 +2733,18 @@ static int _regulator_do_enable(struct regulator_dev *rdev)
  */
 static int _regulator_handle_consumer_enable(struct regulator *regulator)
 {
+	int ret;
 	struct regulator_dev *rdev = regulator->rdev;
 
 	lockdep_assert_held_once(&rdev->mutex.base);
 
 	regulator->enable_count++;
-	if (regulator->uA_load && regulator->enable_count == 1)
-		return drms_uA_update(rdev);
+	if (regulator->uA_load && regulator->enable_count == 1) {
+		ret = drms_uA_update(rdev);
+		if (ret)
+			regulator->enable_count--;
+		return ret;
+	}
 
 	return 0;
 }
-- 
2.37.1


             reply	other threads:[~2022-08-19 19:44 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-19 19:43 Andrew Halaney [this message]
2022-08-19 20:03 ` [PATCH] regulator: core: Clean up on enable failure Doug Anderson
2022-08-19 20:46 ` Brian Masney
2022-08-19 22:48 ` Mark Brown
2022-08-19 23:03   ` Doug Anderson
2022-08-22 19:42     ` Andrew Halaney
2022-08-22 16:16 ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220819194336.382740-1-ahalaney@redhat.com \
    --to=ahalaney@redhat.com \
    --cc=bmasney@redhat.com \
    --cc=broonie@kernel.org \
    --cc=dianders@chromium.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.