From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
To: Aleksander Morgado <aleksander@aleksander.es>
Cc: mhi@lists.linux.dev, Loic Poulain <loic.poulain@linaro.org>,
Thomas Perrot <thomas.perrot@bootlin.com>,
Hemant Kumar <hemantk@codeaurora.org>,
Bhaumik Bhatt <bbhatt@codeaurora.org>,
quic_jhugo@quicinc.com,
linux-arm-msm <linux-arm-msm@vger.kernel.org>,
stable <stable@vger.kernel.org>
Subject: Re: [PATCH v3] bus: mhi: Fix race while handling SYS_ERR at power up
Date: Sat, 20 Nov 2021 17:06:26 +0530 [thread overview]
Message-ID: <20211120113626.GB100286@thinkpad> (raw)
In-Reply-To: <20211120112508.GA100286@thinkpad>
On Sat, Nov 20, 2021 at 04:55:15PM +0530, Manivannan Sadhasivam wrote:
> Hey,
>
> On Thu, Nov 18, 2021 at 10:55:51AM +0100, Aleksander Morgado wrote:
> > Hey Mani,
> >
> > On Thu, Nov 18, 2021 at 6:57 AM Manivannan Sadhasivam
> > <manivannan.sadhasivam@linaro.org> wrote:
> > >
> > > Some devices tend to trigger SYS_ERR interrupt while the host handling
> > > SYS_ERR state of the device during power up. This creates a race
> > > condition and causes a failure in booting up the device.
> > >
> > > The issue is seen on the Sierra Wireless EM9191 modem during SYS_ERR
> > > handling in mhi_async_power_up(). Once the host detects that the device
> > > is in SYS_ERR state, it issues MHI_RESET and waits for the device to
> > > process the reset request. During this time, the device triggers SYS_ERR
> > > interrupt to the host and host starts handling SYS_ERR execution.
> > >
> > > So by the time the device has completed reset, host starts SYS_ERR
> > > handling. This causes the race condition and the modem fails to boot.
> > >
> > > Hence, register the IRQ handler only after handling the SYS_ERR check
> > > to avoid getting spurious IRQs from the device.
> > >
> > > Cc: stable@vger.kernel.org
> > > Fixes: e18d4e9fa79b ("bus: mhi: core: Handle syserr during power_up")
> > > Reported-by: Aleksander Morgado <aleksander@aleksander.es>
> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > ---
> > >
> > > Changes in v3:
> > >
> > > * Moved BHI_INTVEC setup after irq setup
> > > * Used interval_us as the delay for the polling API
> > >
> > > Changes in v2:
> > >
> > > * Switched to "mhi_poll_reg_field" for detecting MHI reset in device.
> > >
> >
> > I tried this v3 patch and I'm not sure if it's working properly in my
> > setup; not all boots are successfully bringing the modem up.
> >
>
> Ouch!
>
> > Once I installed it, I kept having this kind of logs on every boot:
> > [ 7.030407] mhi-pci-generic 0000:01:00.0: BAR 0: assigned [mem
> > 0x600000000-0x600000fff 64bit]
> > [ 7.038984] mhi-pci-generic 0000:01:00.0: enabling device (0000 -> 0002)
> > [ 7.045814] mhi-pci-generic 0000:01:00.0: using shared MSI
> > [ 7.052191] mhi mhi0: Requested to power ON
> > [ 7.168042] mhi mhi0: Power on setup success
> > [ 7.168141] mhi mhi0: Wait for device to enter SBL or Mission mode
> > [ 15.687938] mhi-pci-generic 0000:01:00.0: failed to suspend device: -16
>
> [...]
>
> > I didn't try the v1 or v2 patches (sorry!), so not sure if the issues
> > come in this last iteration or in an earlier one. Do you want me to
> > try with v1 and v2 as well?
> >
>
> Yes, please. Nothing changed other than moving the BHI_INTVEC programming.
>
Or if you want to do it quickly, please test the diff below:
diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
index ee0515a25e46..21484a61bbed 100644
--- a/drivers/bus/mhi/core/pm.c
+++ b/drivers/bus/mhi/core/pm.c
@@ -1055,7 +1055,9 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl)
mutex_lock(&mhi_cntrl->pm_mutex);
mhi_cntrl->pm_state = MHI_PM_DISABLE;
+ /* Setup BHI INTVEC */
write_lock_irq(&mhi_cntrl->pm_lock);
+ mhi_write_reg(mhi_cntrl, mhi_cntrl->bhi, BHI_INTVEC, 0);
mhi_cntrl->pm_state = MHI_PM_POR;
mhi_cntrl->ee = MHI_EE_MAX;
current_ee = mhi_get_exec_env(mhi_cntrl);
@@ -1094,9 +1096,6 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl)
if (ret)
goto error_setup_irq;
- /* Setup BHI INTVEC */
- mhi_write_reg(mhi_cntrl, mhi_cntrl->bhi, BHI_INTVEC, 0);
-
/* Transition to next state */
next_state = MHI_IN_PBL(current_ee) ?
DEV_ST_TRANSITION_PBL : DEV_ST_TRANSITION_READY;
Thanks,
Mani
> Thanks,
> Mani
>
> > The patch that was working very reliably (100%) for me was the "bus:
> > mhi: Register IRQ handler after SYS_ERR check during power up" one,
> > which you attached here:
> > https://www.spinics.net/lists/linux-arm-msm/msg97646.html
> >
> > --
> > Aleksander
> > https://aleksander.es
next prev parent reply other threads:[~2021-11-20 11:36 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-18 5:57 [PATCH v3] bus: mhi: Fix race while handling SYS_ERR at power up Manivannan Sadhasivam
2021-11-18 9:55 ` Aleksander Morgado
2021-11-20 11:25 ` Manivannan Sadhasivam
2021-11-20 11:36 ` Manivannan Sadhasivam [this message]
2021-11-22 13:13 ` Aleksander Morgado
2021-11-22 15:27 ` Manivannan Sadhasivam
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211120113626.GB100286@thinkpad \
--to=manivannan.sadhasivam@linaro.org \
--cc=aleksander@aleksander.es \
--cc=bbhatt@codeaurora.org \
--cc=hemantk@codeaurora.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=loic.poulain@linaro.org \
--cc=mhi@lists.linux.dev \
--cc=quic_jhugo@quicinc.com \
--cc=stable@vger.kernel.org \
--cc=thomas.perrot@bootlin.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).