[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <22fd0d8e-ef10-8ce5-09d1-ff136f594c8c@baylibre.com>
Date: Fri, 11 Aug 2023 13:09:17 +0200
From: Alexandre Mergnat <amergnat@...libre.com>
To: NĂcolas F. R. A. Prado
<nfraprado@...labora.com>, Stephen Boyd <sboyd@...nel.org>
Cc: Chen-Yu Tsai <wenst@...gle.com>, kernel@...labora.com,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Hsin-Hsiung Wang <hsin-hsiung.wang@...iatek.com>,
James Lo <james.lo@...iatek.com>,
Matthias Brugger <matthias.bgg@...il.com>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH v3] spmi: mtk-pmif: Serialize PMIF status check and
command submission
Reviewed-by: Alexandre Mergnat <amergnat@...libre.com>
On 24/07/2023 17:47, NĂcolas F. R. A. Prado wrote:
> Before writing the read or write command to the SPMI arbiter through the
> PMIF interface, the current status of the channel is checked to ensure
> it is idle. However, since the status only changes from idle when the
> command is written, it is possible for two concurrent calls to determine
> that the channel is idle and simultaneously send their commands. At this
> point the PMIF interface hangs, with the status register no longer being
> updated, and thus causing all subsequent operations to time out.
>
> This was observed on the mt8195-cherry-tomato-r2 machine, particularly
> after commit 46600ab142f8 ("regulator: Set PROBE_PREFER_ASYNCHRONOUS for
> drivers between 5.10 and 5.15") was applied, since then the two MT6315
> devices present on the SPMI bus would probe assynchronously and
> sometimes (during probe or at a later point) read the bus
> simultaneously, breaking the PMIF interface and consequently slowing
> down the whole system.
>
> To fix the issue at its root cause, introduce locking around the channel
> status check and the command write, so that both become an atomic
> operation, preventing race conditions between two (or more) SPMI bus
> read/write operations. A spinlock is used since this is a fast bus, as
> indicated by the usage of the atomic variant of readl_poll, and
> '.fast_io = true' being used in the mt6315 driver, so spinlocks are
> already used for the regmap access.
--
Regards,
Alexandre
Powered by blists - more mailing lists