Re: [PATCH v2 2/2] firmware: exynos-acpm: allow use during system shutdown
From: André Draszik
Date: Tue Mar 25 2025 - 04:02:20 EST
Hi Krzysztof,
On Tue, 2025-03-25 at 08:57 +0100, Krzysztof Kozlowski wrote:
> On 24/03/2025 16:34, André Draszik wrote:
> > +static bool acpm_may_sleep(void)
> > +{
> > + return system_state <= SYSTEM_RUNNING ||
> > + (IS_ENABLED(CONFIG_PREEMPT_COUNT) ? preemptible() : !irqs_disabled());
> > +}
> > +
> > /**
> > * acpm_dequeue_by_polling() - RX dequeue by polling.
> > * @achan: ACPM channel info.
> > @@ -300,7 +314,10 @@ static int acpm_dequeue_by_polling(struct acpm_chan *achan,
> > return 0;
> >
> > /* Determined experimentally. */
> > - usleep_range(20, 30);
> > + if (!acpm_may_sleep())
> > + udelay(10);
> > + else
>
> ... and what do you do if IRQs get disabled exactly in this moment? This
> is just racy. You cannot check for a condition and assume it will be
> valid for whatever time you want it to be valid.
>
> 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.
Would you have a better suggestion?
Cheers,
Andre'