2020-11-03 06:28:43

by Can Guo

[permalink] [raw]
Subject: [PATCH] scsi: ufs: Try to save power mode change and UIC cmd completion timeout

Use the uic_cmd->cmd_active as a flag to track the lifecycle of an UIC cmd.
The flag is set before send the UIC cmd and cleared after the completion is
raised in IRQ handler. For a power mode change operation, including hibern8
enter/exit, the flag is cleared only after hba->uic_async_done completion
is raised. When completion timeout happens, if the flag is cleared, instead
of returning timeout error, simply ignore it.

Change-Id: Ie3cd6ae6221a44619925fb2cf78136a5617fdd5d
Signed-off-by: Can Guo <[email protected]>
---
drivers/scsi/ufs/ufshcd.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 252e022..8b291c3 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2131,10 +2131,20 @@ ufshcd_wait_for_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd)
unsigned long flags;

if (wait_for_completion_timeout(&uic_cmd->done,
- msecs_to_jiffies(UIC_CMD_TIMEOUT)))
+ msecs_to_jiffies(UIC_CMD_TIMEOUT))) {
ret = uic_cmd->argument2 & MASK_UIC_COMMAND_RESULT;
- else
+ } else {
ret = -ETIMEDOUT;
+ dev_err(hba->dev,
+ "uic cmd 0x%x with arg3 0x%x completion timeout\n",
+ uic_cmd->command, uic_cmd->argument3);
+
+ if (!uic_cmd->cmd_active) {
+ dev_err(hba->dev, "%s: UIC cmd has been completed, return the result\n",
+ __func__);
+ ret = uic_cmd->argument2 & MASK_UIC_COMMAND_RESULT;
+ }
+ }

spin_lock_irqsave(hba->host->host_lock, flags);
hba->active_uic_cmd = NULL;
@@ -2166,6 +2176,7 @@ __ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd,
if (completion)
init_completion(&uic_cmd->done);

+ uic_cmd->cmd_active = 1;
ufshcd_dispatch_uic_cmd(hba, uic_cmd);

return 0;
@@ -3944,10 +3955,18 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba, struct uic_command *cmd)
dev_err(hba->dev,
"pwr ctrl cmd 0x%x with mode 0x%x completion timeout\n",
cmd->command, cmd->argument3);
+
+ if (!cmd->cmd_active) {
+ dev_err(hba->dev, "%s: Power Mode Change operation has been completed, go check UPMCRS\n",
+ __func__);
+ goto check_upmcrs;
+ }
+
ret = -ETIMEDOUT;
goto out;
}

+check_upmcrs:
status = ufshcd_get_upmcrs(hba);
if (status != PWR_LOCAL) {
dev_err(hba->dev,
@@ -5060,11 +5079,14 @@ static irqreturn_t ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
hba->active_uic_cmd->argument3 =
ufshcd_get_dme_attr_val(hba);
complete(&hba->active_uic_cmd->done);
+ if (!hba->uic_async_done)
+ hba->active_uic_cmd->cmd_active = 0;
retval = IRQ_HANDLED;
}

if ((intr_status & UFSHCD_UIC_PWR_MASK) && hba->uic_async_done) {
complete(hba->uic_async_done);
+ hba->active_uic_cmd->cmd_active = 0;
retval = IRQ_HANDLED;
}
return retval;
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


2020-11-03 13:31:38

by Can Guo

[permalink] [raw]
Subject: Re: [PATCH] scsi: ufs: Try to save power mode change and UIC cmd completion timeout

Sorry, please ignore this specific change, which is wrongly sent out.

On 2020-11-03 14:24, Can Guo wrote:
> Use the uic_cmd->cmd_active as a flag to track the lifecycle of an UIC
> cmd.
> The flag is set before send the UIC cmd and cleared after the
> completion is
> raised in IRQ handler. For a power mode change operation, including
> hibern8
> enter/exit, the flag is cleared only after hba->uic_async_done
> completion
> is raised. When completion timeout happens, if the flag is cleared,
> instead
> of returning timeout error, simply ignore it.
>
> Change-Id: Ie3cd6ae6221a44619925fb2cf78136a5617fdd5d
> Signed-off-by: Can Guo <[email protected]>
> ---
> drivers/scsi/ufs/ufshcd.c | 26 ++++++++++++++++++++++++--
> 1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 252e022..8b291c3 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -2131,10 +2131,20 @@ ufshcd_wait_for_uic_cmd(struct ufs_hba *hba,
> struct uic_command *uic_cmd)
> unsigned long flags;
>
> if (wait_for_completion_timeout(&uic_cmd->done,
> - msecs_to_jiffies(UIC_CMD_TIMEOUT)))
> + msecs_to_jiffies(UIC_CMD_TIMEOUT))) {
> ret = uic_cmd->argument2 & MASK_UIC_COMMAND_RESULT;
> - else
> + } else {
> ret = -ETIMEDOUT;
> + dev_err(hba->dev,
> + "uic cmd 0x%x with arg3 0x%x completion timeout\n",
> + uic_cmd->command, uic_cmd->argument3);
> +
> + if (!uic_cmd->cmd_active) {
> + dev_err(hba->dev, "%s: UIC cmd has been completed, return the
> result\n",
> + __func__);
> + ret = uic_cmd->argument2 & MASK_UIC_COMMAND_RESULT;
> + }
> + }
>
> spin_lock_irqsave(hba->host->host_lock, flags);
> hba->active_uic_cmd = NULL;
> @@ -2166,6 +2176,7 @@ __ufshcd_send_uic_cmd(struct ufs_hba *hba,
> struct uic_command *uic_cmd,
> if (completion)
> init_completion(&uic_cmd->done);
>
> + uic_cmd->cmd_active = 1;
> ufshcd_dispatch_uic_cmd(hba, uic_cmd);
>
> return 0;
> @@ -3944,10 +3955,18 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba
> *hba, struct uic_command *cmd)
> dev_err(hba->dev,
> "pwr ctrl cmd 0x%x with mode 0x%x completion timeout\n",
> cmd->command, cmd->argument3);
> +
> + if (!cmd->cmd_active) {
> + dev_err(hba->dev, "%s: Power Mode Change operation has been
> completed, go check UPMCRS\n",
> + __func__);
> + goto check_upmcrs;
> + }
> +
> ret = -ETIMEDOUT;
> goto out;
> }
>
> +check_upmcrs:
> status = ufshcd_get_upmcrs(hba);
> if (status != PWR_LOCAL) {
> dev_err(hba->dev,
> @@ -5060,11 +5079,14 @@ static irqreturn_t ufshcd_uic_cmd_compl(struct
> ufs_hba *hba, u32 intr_status)
> hba->active_uic_cmd->argument3 =
> ufshcd_get_dme_attr_val(hba);
> complete(&hba->active_uic_cmd->done);
> + if (!hba->uic_async_done)
> + hba->active_uic_cmd->cmd_active = 0;
> retval = IRQ_HANDLED;
> }
>
> if ((intr_status & UFSHCD_UIC_PWR_MASK) && hba->uic_async_done) {
> complete(hba->uic_async_done);
> + hba->active_uic_cmd->cmd_active = 0;
> retval = IRQ_HANDLED;
> }
> return retval;