MIME-Version: 1.0
References: <CGME20230218083300eucas1p28c7c584877b8914a3b88904690be82f6@eucas1p2.samsung.com>
 <20230218083252.2044423-1-saravanak@google.com> <e3814c81-c74d-7087-e87d-12dcb49e6444@samsung.com>
 <CAGETcx-UcVnDw-FJAPeA1mLpPno4OE3AAv4WsfP852zOdKqPCw@mail.gmail.com> <Y/VLREM7n+wstHbn@sirena.org.uk>
In-Reply-To: <Y/VLREM7n+wstHbn@sirena.org.uk>
From:   Saravana Kannan <saravanak@google.com>
Date:   Tue, 21 Feb 2023 19:13:39 -0800
Message-ID: <CAGETcx_hp_P09diJWgP7=4QiKJi91oVXie3iddpkTOdO4uuZjg@mail.gmail.com>
Subject: Re: [RFC v1 0/4] Simplify regulator supply resolution code by
 offloading to driver core
To:     Mark Brown <broonie@kernel.org>
Cc:     Marek Szyprowski <m.szyprowski@samsung.com>,
        Liam Girdwood <lgirdwood@gmail.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Geert Uytterhoeven <geert@linux-m68k.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Sudeep Holla <sudeep.holla@arm.com>,
        Tony Lindgren <tony@atomide.com>,
        Doug Anderson <dianders@chromium.org>,
        Guenter Roeck <linux@roeck-us.net>,
        Luca Weiss <luca.weiss@fairphone.com>, kernel-team@android.com,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Feb 21, 2023 at 2:52 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Tue, Feb 21, 2023 at 02:36:52PM -0800, Saravana Kannan wrote:
>
> > Thanks for testing it Marek! I don't want people to spend more time
> > testing this before I hear Mark/Liam's thoughts. So, let's hold off
> > for now.
>
> My main thought right now is that I'm not going to think about it
> too hard if it doesn't work correctly...

:( I'm not asking for a thorough code review. Just if you are okay
with the idea/approach of pushing the ordering logic to driver core to
avoid reimplementing what's already available and avoiding some races
in the regulator code (stuff like, checking if some other thread
resolved a supply while you were working on it). The patch at least
works on my device and works for most regulators in Marek's devices.
So, it's not a complete broken mess :)

On a separate note, I have some questions about setting machine
constraints during regulator_register(). Why do we even try to set
machine constraints before a regulator's supply is resolved? None of
the consumers can make any requests anyway. So what else is going to
need those constraints? Wouldn't the regulator just be in whatever
state the bootloader left it at?

The current logic is something like:

1. Try to resolve supply if it's always on or a boot on regulator.
2. Set machine constraints -- this might fail for multiple reasons.
One of them being unresolved supply.
3. If it failed due to unresolved supply, but it wasn't resolved in step 1.
3. a. Try to resolve supply,
3. b. If 3.a. didn't fail, try to set machine constraints.
3. c. If 3.b failed, fail registration.

Why isn't this just:
1. Try to resolve supply (for all regulators).
2. If we are able to resolve supply set machine constraints.
3. If we weren't able to resolve supply, set machine constraints when
we resolve supply in the future?

Or if you need to set machine constraints without waiting for supply,
then why not at least:

1. Try to resolve supply (for all regulators).
2. Set machine constraints.
3. When we resolve supply in the future, do whatever remaining bits
that you need to do.

Thanks,
Saravana