Date:   Tue, 14 Mar 2023 18:53:52 +0000
From:   Mark Brown <broonie@...nel.org>
To:     Doug Anderson <dianders@...omium.org>
Cc:     mka@...omium.org, christian@...lschutter.com,
        Liam Girdwood <lgirdwood@...il.com>,
        linux-kernel@...r.kernel.org,
        Brian Norris <briannorris@...omium.org>,
        Saravana Kannan <saravanak@...gle.com>
Subject: Re: [PATCH 2/2] regulator: fixed: Set PROBE_PREFER_ASYNCHRONOUS

On Tue, Mar 14, 2023 at 09:35:28AM -0700, Doug Anderson wrote:

> The "special" things are:

> * It's been confirmed that this one random driver is involved in
> slowing down boot a significant amount.

> * The fixed regulator driver is among the simplest of the regulators
> and doesn't have the complex interactions that are typically
> associated with async probe problems.

I would be surprised to see regulators being in that category.

> For reference, some of the problems I'm aware of that have been seen
> in the past when trying to enable async probe more broadly:

> a) Apparently, on ACPI child devices aren't guaranteed to be probed
> before parent devices once you turn on async probe. This is not
> typically the case with device-tree probed devices where children show
> up due to of_platform_populate() which is called by a parent device.
> This can be handled properly but often isn't.

That seems unlikely to be an issue here.

> b) Some drivers try to poke directly at other devices and can get
> confused if the other device is probing at the same time. One example
> was dual-MIPI on Rockchip [1]. Again, this can be handled properly but
> often isn't because nobody tested it.

Again, unlikely to be an issue for regulators - where multiple
regulators interact with each other that's already coordinated through
the core.

> c) Dynamically allocated ID numbers can change unpredictably from boot
> to boot with async probe. This showed up on MMC where eMMC and SD
> would change each boot between "mmcblk0" and "mmcblk1".

No numbers exposed to userspace here so that'll be fine.

> d) Async probe (obviously) changes timing and that can expose latent
> bugs. Almost always those bugs should have been fixed anyway.

> e) Async probe tends to stress out other driver's (consumers of the
> device that's now probing async) error handling (since they are more
> likely to see -EPROBE_DEFER) and can expose latent bugs there.

Right, not sure I'd worry about either of those though and given how
widley used fixed regualtors are I'd expect it to be one of the riskiest
here.

> ...but b) _could_ be a problem. Specifically, today I think that
> (unless some of its supplies aren't available at probe time) the PMIC
> driver will _always_ finish probing before the RTC/clock driver
> because of the order specified in the source code:

>   { .name = "max77686-pmic", },
>   { .name = "max77686-rtc", },
>   { .name = "max77686-clk", },

> One would need to do deeper code inspection (and, ideally, testing) to
> find out if indeed the RTC driver and clock driver will have problems
> if the regulator is not finished probing before their probes start.

Behind the scenes interactions would be quite unusual, the power demands
of the on chip devices are typically minimal so there's not much to do -
it's typically all always on functions.  Could happen of course but it'd
be surprising.

> I guess the last thing I'll say is that PROBE_PREFER_ASYNCHRONOUS was
> added specifically so that we could enable this on drivers that were
> found to be slow to boot and that were tested to work with async
> probe. Without being able to add PROBE_PREFER_ASYNCHRONOUS people were
> open-coding solutions per driver to speed probe.

Sure, but my point is that at least every regulator with a ramp time
(which will be most of them, even those that don't have one they tell
the kernel about one probably should) is going to have a similar issue
so could probably benefit from a similar solution.

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

Powered by blists - more mailing lists