> > Regarding 5.7.12.11 in Unipro v1.8, PA rejects sebsequent requests
> > following the first request from upper layer or remote.
> > In this situation, PA responds w/ BUSY in cases when they come from
> > upper layer and does nothing for the requests. So HCI doesn't receive
> > ind, a.k.a. indicator for its requests and an interrupt, IS.UPMS isn't
> > generated.
> >
> > When LINERESET occurs, the error handler issues PMC which is
> > recognized as a request for PA. If a host's PA gets or raises
> > LINERESET, and a request for PMC, this could be a concurrent situation
> > mentioned above. And I found that situation w/ my environment.
> Can you please elaborate on how this concurrency can happen?
> My understanding is that both line reset indication and uic command are
> protected by host_lock?
Yes and one thing I have to correct on the clause: 5.7.12.11 -> 5.7.12.1.1
And I’m talking about the situation w/ some requests w/ PACP.
> >
> > [ 222.929539]I[0:DefaultDispatch:20410] exynos-ufs 13500000.ufs:
> > ufshcd_update_uic_error: uecdl : 0x80000002 [ 222.999009]I[0:
> > arch_disk_io_1:20413] exynos-ufs 13500000.ufs:
> > ufshcd_update_uic_error: uecpa : 0x80000010 [ 222.999200] [6:
> > kworker/u16:2: 132] exynos-ufs 13500000.ufs:
> > ufs_pwr_mode_restore_needed : mode = 0x15, pwr_rx = 1, pwr_tx = 1 [
> > 223.002876]I[0: arch_disk_io_3:20422] exynos-ufs 13500000.ufs:
> > ufshcd_update_uic_error: uecpa : 0x80000010 [ 223.501050] [4:
> > kworker/u16:2: 132] exynos-ufs 13500000.ufs: pwr ctrl cmd
> > 0x2 with mode 0x11 completion timeout
> > [ 223.502305] [4: kworker/u16:2: 132] exynos-ufs 13500000.ufs:
> > ufshcd_change_power_mode: power mode change failed -110 [ 223.502312]
> > [4: kworker/u16:2: 132] exynos-ufs 13500000.ufs:
> > ufshcd_err_handler: Failed to restore power mode, err = -110 [
> > 223.502392] [4: kworker/u16:2: 132] exynos-ufs 13500000.ufs:
> > ufshcd_is_pwr_mode_restore_needed : mode = 0x11, pwr_rx = 1, pwr_tx =
> > 1
> >
> > This patch is to poll PMC's result w/o checking its ind until the
> > result is not busy, i.e. 09h, to avoid the rejection.
> >
> > Signed-off-by: Kiwoong Kim <kwmad.kim@xxxxxxxxxxx>
> > ---
> > drivers/ufs/core/ufshcd.c | 92
> > ++++++++++++++++++++++++++++++++++---------
> > ----
> > 1 file changed, 67 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> > index 9434328..3fa58d9 100644
> > --- a/drivers/ufs/core/ufshcd.c
> > +++ b/drivers/ufs/core/ufshcd.c
> > @@ -98,6 +98,9 @@
> > /* Polling time to wait for fDeviceInit */ #define
> > FDEVICEINIT_COMPL_TIMEOUT 1500 /* millisecs */
> >
> > +/* Polling time to wait until PA is ready */
> > +#define UIC_PA_RDY_TIMEOUT 30 /* millisecs */
> Is this something that is common to all hosts?
The values is based on the description of T LINE-RESET in Table 11 in M-PHY v4.1
It says 20ms but it's just something like expectation.
So we can't know its biggest value in the real world through the spec.
So I talked to some experts on it and decided to add some margin, 10ms.
>
> > +
> > /* UFSHC 4.0 compliant HC support this mode, refer
> > param_set_mcq_mode() */ static bool use_mcq_mode = true;
> >
> > @@ -4138,6 +4141,64 @@ int ufshcd_dme_get_attr(struct ufs_hba *hba,
> > u32 attr_sel, } EXPORT_SYMBOL_GPL(ufshcd_dme_get_attr);
> >
> > +static int __ufshcd_poll_uic_pwr(struct ufs_hba *hba, struct
> > +uic_command
> > *cmd,
> > + struct completion *cnf) {
> > + unsigned long flags;
> > + int ret;
> > + ktime_t timeout;
> > + u32 mode = cmd->argument3;
> > +
> > + timeout = ktime_add_ms(ktime_get(), UIC_PA_RDY_TIMEOUT);
> > + do {
> > + spin_lock_irqsave(hba->host->host_lock, flags);
> > + hba->active_uic_cmd = NULL;
> > + if (ufshcd_is_link_broken(hba)) {
> > + spin_unlock_irqrestore(hba->host->host_lock, flags);
> > + ret = -ENOLINK;
> > + goto out;
> > + }
> > + hba->uic_async_done = cnf;
> > + cmd->argument2 = 0;
> > + cmd->argument3 = mode;
> > + ret = __ufshcd_send_uic_cmd(hba, cmd, true);
> > + spin_unlock_irqrestore(hba->host->host_lock, flags);
> > + if (ret) {
> > + dev_err(hba->dev,
> > + "pwr ctrl cmd 0x%x with mode 0x%x uic
> error %d\n",
> > + cmd->command, cmd->argument3, ret);
> > + goto out;
> > + }
> > +
> > + /* This value is heuristic */
> > + if (!wait_for_completion_timeout(&cmd->done,
> > + msecs_to_jiffies(5))) {
> > + ret = -ETIMEDOUT;
> > + dev_err(hba->dev,
> > + "pwr ctrl cmd 0x%x with mode 0x%x timeout\n",
> > + cmd->command, cmd->argument3);
> > + if (cmd->cmd_active)
> > + goto out;
> > +
> > + dev_info(hba->dev, "%s: pwr ctrl cmd has
> > + already been
> > completed\n", __func__);
> > + }
> > +
> > + /* retry for only busy cases */
> > + ret = cmd->argument2 & MASK_UIC_COMMAND_RESULT;
> > + if (ret != UIC_CMD_RESULT_BUSY)
> > + break;
> > +
> > + dev_info(hba->dev, "%s: PA is busy and can't handle a
> > + requeest\n",
> > __func__);
> > +
> > + } while (ktime_before(ktime_get(), timeout));
> > +out:
> > + spin_lock_irqsave(hba->host->host_lock, flags);
> > + hba->active_uic_cmd = NULL;
> > + spin_unlock_irqrestore(hba->host->host_lock, flags);
> > +
> > + return ret;
> > +}
> > +
> > /**
> > * ufshcd_uic_pwr_ctrl - executes UIC commands (which affects the
> > link power
> > * state) and waits for it to take effect.
> > @@ -4160,33 +4221,16 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba
> > *hba, struct uic_command *cmd)
> > unsigned long flags;
> > u8 status;
> > int ret;
> > - bool reenable_intr = false;
> >
> > mutex_lock(&hba->uic_cmd_mutex);
> > ufshcd_add_delay_before_dme_cmd(hba);
> >
> > - spin_lock_irqsave(hba->host->host_lock, flags);
> > - if (ufshcd_is_link_broken(hba)) {
> > - ret = -ENOLINK;
> > - goto out_unlock;
> > - }
> > - hba->uic_async_done = &uic_async_done;
> > - if (ufshcd_readl(hba, REG_INTERRUPT_ENABLE) &
> > UIC_COMMAND_COMPL) {
> > - ufshcd_disable_intr(hba, UIC_COMMAND_COMPL);
> > - /*
> > - * Make sure UIC command completion interrupt is disabled
> before
> > - * issuing UIC command.
> > - */
> > - wmb();
> > - reenable_intr = true;
> > - }
> > - ret = __ufshcd_send_uic_cmd(hba, cmd, false);
> > - spin_unlock_irqrestore(hba->host->host_lock, flags);
> > + ret = __ufshcd_poll_uic_pwr(hba, cmd, &uic_async_done);
> > if (ret) {
> > - dev_err(hba->dev,
> > - "pwr ctrl cmd 0x%x with mode 0x%x uic error %d\n",
> > - cmd->command, cmd->argument3, ret);
> > - goto out;
> > + if (ret == -ENOLINK)
> > + goto out_unlock;
> > + else
> > + goto out;
> > }
> >
> > if (!wait_for_completion_timeout(hba->uic_async_done,
> > @@ -4223,14 +4267,12 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba
> > *hba, struct uic_command *cmd)
> > spin_lock_irqsave(hba->host->host_lock, flags);
> > hba->active_uic_cmd = NULL;
> > hba->uic_async_done = NULL;
> > - if (reenable_intr)
> > - ufshcd_enable_intr(hba, UIC_COMMAND_COMPL);
> > if (ret) {
> > ufshcd_set_link_broken(hba);
> > ufshcd_schedule_eh_work(hba);
> > }
> > -out_unlock:
> > spin_unlock_irqrestore(hba->host->host_lock, flags);
> > +out_unlock:
> > mutex_unlock(&hba->uic_cmd_mutex);
> >
> > return ret;
> > --
> > 2.7.4
I found one thing to be fixed and so will update this patch.
