Re: [PATCH v2 2/2] firmware: exynos-acpm: allow use during system shutdown

["André Draszik" <andre.draszik@linaro.org>]

On Wed, 2025-03-26 at 08:36 +0100, Krzysztof Kozlowski wrote:
> On 26/03/2025 08:24, Tudor Ambarus wrote:
> > > > > 
> > > > > What happens if system_state is changed to shutdown in this particular
> > > > > moment? How did you prevent this from happening?
> > > > 
> > > > Yes, and that's also what the I2C subsystem is doing, AFAICS, see
> > > > i2c_in_atomic_xfer_mode() and its use. This is to make a very
> > > > specific corner case work, similar to I2C which has to deal with
> > > > the same issue during shutdown.
> > > 
> > > But they don't have a choice so they try to do the best to avoid
> > > sleeping. And it is a subsystem, not a driver, which means their
> > > patterns are sometimes special. Drivers should not replicate subsystem
> > > workarounds.
> > > 
> > > > 
> > > > Would you have a better suggestion?
> > > 
> > > Yes, you have a choice, you can always use udelay. Driver code is
> > > supposed to be always correct.
> > 
> > Using udelay() is good enough for now. I see that downstream uses a
> > usleep_range(50, 100) and I'm concerned that we're going to waste lots
> > of cpu cyles once more and more clients get added.
> 
> 
> If this is going to be the case, then we can revisit it with some
> numbers. Especially if this ACPM turns out to be a bus driver.
> 
> > 
> > If there's no concurrency on the ACPM queue mutexes at late system
> > shutdown, would it work to pass the don't sleep requirement from the
> > client to ACPM and use udelay only then?
> 
> 
> You mean the client will choose what sort of delay it expects (sleeping
> or not)? That would work, but can you actually control it from the
> client side?

I can know this requirement from the RTC driver indeed
https://lore.kernel.org/all/20250323-s2mpg10-v1-29-d08943702707@linaro.org/
but a) I'm not sure how to do that with regmap, I'll have to have a look and
b) if the concern is that system_state is changed to shutdown while one
of the (other) drivers is using the ACPM (as per your earlier email), then
having a different API wouldn't help I believe.

Cheers,
Andre'