| Message ID | 1681888769-36587-1-git-send-email-kwmad.kim@samsung.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC,v1] ufs: poll pmc until another pa request is completed | expand |
> 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? > > [ 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@samsung.com> > --- > 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? Thanks, Avri > + > /* 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
> > 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@samsung.com> > > --- > > 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.
> > > 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. OK. Thanks. However, Please note that Clause 5.7.12.1.1 "Concurrency Resolution" of the unipro spec, is dealing with local-peer concurrency. Not 2 concurrent local requests. As such, I think you need to explain that this is not a host issue. Thanks, Avri
> > > > 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. > OK. Thanks. > > However, Please note that Clause 5.7.12.1.1 "Concurrency Resolution" of > the unipro spec, is dealing with local-peer concurrency. Not 2 concurrent > local requests. > As such, I think you need to explain that this is not a host issue. > > Thanks, > Avri I talked with the experts on this again. They said the situation also includes 'two local requests case' because they see 'a local request' mentioned in the spec could be also PA_INIT.req form the local's upper layer. -- "A local request shall be rejected when the PA Layer is processing a local request or a peer request 2679 from the peer Device." Could you explain more why you don't think it's the case? Thanks. Kiwoong Kim
> > > > > 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. > > OK. Thanks. > > > > However, Please note that Clause 5.7.12.1.1 "Concurrency Resolution" of > > the unipro spec, is dealing with local-peer concurrency. Not 2 concurrent > > local requests. > > As such, I think you need to explain that this is not a host issue. > > > > Thanks, > > Avri > > I talked with the experts on this again. They said the situation also includes > 'two local requests case' > because they see 'a local request' mentioned in the spec could be also > PA_INIT.req form the local's upper layer. > -- > "A local request shall be rejected when the PA Layer is processing a local > request or a peer request 2679 from the peer Device." Yes. You are correct. Thanks, Avri > > Could you explain more why you don't think it's the case? > > Thanks. > Kiwoong Kim >
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 */ + /* 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;
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. [ 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@samsung.com> --- drivers/ufs/core/ufshcd.c | 92 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 67 insertions(+), 25 deletions(-)