mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Wadim Mueller <wafgo01@gmail.com>
Cc: "Maxwell Doose" <maxwell@maxwelld.cc>,
	"David Lechner" <dlechner@baylibre.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	"Jonathan Cameron" <jonathan.cameron@oss.qualcomm.com>,
	linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Li Youhong" <dayou5941@163.com>,
	"Li Youhong" <liyouhong@kylinos.cn>,
	"Brendan Higgins" <brendan.higgins@linux.dev>,
	"David Gow" <david@davidgow.net>,
	"Rae Moar" <raemoar63@gmail.com>,
	linux-kselftest@vger.kernel.org, kunit-dev@googlegroups.com
Subject: Re: [PATCH 0/2] iio: flow: slf3s: fix and test the vdd supply balance
Date: Mon, 21 Sep 2026 04:19:57 +0100	[thread overview]
Message-ID: <20260921041957.25b97f64@jic23-hlaptop> (raw)
In-Reply-To: <20260904-iio-slf3s-pm-regulator-kunit-v1-0-5ea61e6d6db2@gmail.com>

On Fri, 04 Sep 2026 21:24:24 +0200
Wadim Mueller <wafgo01@gmail.com> wrote:

> Li Youhong reported that slf3s_resume() leaves vdd enabled when the
> measurement restart fails [1].  The report is right, but the patch and
> my own Reviewed-by on it were both too narrow.
> 
> Andy asked whether the regulator refcount can go sideways here.  It
> cannot.  device_suspend() sets power.is_suspended only when the callback
> returned 0, and device_resume() returns before the callback when the
> flag is clear.  So the two alternate and the count stays in {0, 1}.
> Please drop my Reviewed-by, the reasoning in it is wrong.
> 
> Three things do go wrong though:
> 
>  - resume enables vdd, start_meas() fails, the supply stays on.
> 
>  - the part is then powered but idle, so it NACKs the stop command.
>    suspend() returns early on that error and never disables vdd, so the
>    system cannot suspend any more until rebind.  This one hurts most.
> 
>  - if regulator_enable() fails in resume, the next suspend disables a
>    supply that was never enabled: "unbalanced disables" in the regulator
>    core, and -EIO.  Unbind hits the same case.
> 
> The original patch fixes the first one only.
> 
> Patch 1 tracks the supply state and drives enable/disable from it.
> Patch 2 adds KUnit tests with a fake i2c adapter and a counting
> regulator, so no hardware is needed:
> 
>   ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/iio/flow
> 
> Four of the eight tests fail without patch 1.
> 
> [1] https://lore.kernel.org/linux-iio/20260901025010.356735-1-dayou5941@163.com/
> 
> Signed-off-by: Wadim Mueller <wafgo01@gmail.com>
Given a few outstanding bits on patch 1 and lack of discussion of patch 2
(which doesn't need to go in as quickly) I am marking this in patchwork
as changes requested.

Thanks

Jonathan

> ---
> Wadim Mueller (2):
>       iio: flow: slf3s: keep the vdd supply balanced across suspend/resume
>       iio: flow: slf3s: add KUnit tests for the PM regulator balance
> 
>  MAINTAINERS                    |   2 +
>  drivers/iio/flow/.kunitconfig  |   6 +
>  drivers/iio/flow/Kconfig       |  15 +
>  drivers/iio/flow/Makefile      |   1 +
>  drivers/iio/flow/slf3s-kunit.c | 734 +++++++++++++++++++++++++++++++++++++++++
>  drivers/iio/flow/slf3s.c       |  35 +-
>  6 files changed, 787 insertions(+), 6 deletions(-)
> ---
> base-commit: bc35965f6940a9bf834d54187b6088b8eb09206d
> change-id: 20260904-iio-slf3s-pm-regulator-kunit-43ee079b36f0
> 
> Best regards,


      parent reply	other threads:[~2026-09-21  3:20 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-04 19:24 Wadim Mueller
2026-09-04 19:24 ` [PATCH 1/2] iio: flow: slf3s: keep the vdd supply balanced across suspend/resume Wadim Mueller
2026-09-05  1:13   ` Jonathan Cameron
2026-09-04 19:24 ` [PATCH 2/2] iio: flow: slf3s: add KUnit tests for the PM regulator balance Wadim Mueller
2026-09-05  1:31   ` Jonathan Cameron
2026-09-21  3:19 ` Jonathan Cameron [this message]

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=20260921041957.25b97f64@jic23-hlaptop \
    --to=jic23@kernel.org \
    --cc=andy@kernel.org \
    --cc=brendan.higgins@linux.dev \
    --cc=david@davidgow.net \
    --cc=dayou5941@163.com \
    --cc=dlechner@baylibre.com \
    --cc=jonathan.cameron@oss.qualcomm.com \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=liyouhong@kylinos.cn \
    --cc=maxwell@maxwelld.cc \
    --cc=nuno.sa@analog.com \
    --cc=raemoar63@gmail.com \
    --cc=wafgo01@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®