[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <60732c52-7774-48c6-8ec8-44ae82d86267@kernel.org>
Date: Tue, 25 Mar 2025 09:07:33 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: André Draszik <andre.draszik@...aro.org>,
Tudor Ambarus <tudor.ambarus@...aro.org>,
Alim Akhtar <alim.akhtar@...sung.com>
Cc: Peter Griffin <peter.griffin@...aro.org>,
Will McVicker <willmcvicker@...gle.com>, kernel-team@...roid.com,
linux-kernel@...r.kernel.org, linux-samsung-soc@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 2/2] firmware: exynos-acpm: allow use during system
shutdown
On 25/03/2025 09:01, André Draszik wrote:
> 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.
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.
Best regards,
Krzysztof
Powered by blists - more mailing lists