2023-05-10 05:39:10

by Bao D. Nguyen

[permalink] [raw]
Subject: [PATCH v3 2/7] ufs: core: Update the ufshcd_clear_cmds() functionality

In the ufshcd_clear_cmds(), the 2nd pamameter would be the
bit mask of the command to be cleared in the transfer request
door bell register. This bit mask mechanism does not scale well in
mcq mode when the queue depth becomes much greater than 64. Change the
2nd parameter to the function to be the task_tag number of the
corresponding to the bit to be cleared in the door bell register.
By doing so, mcq mode with a large queue depth can reuse this function.

Signed-off-by: Bao D. Nguyen <[email protected]>
---
drivers/ufs/core/ufshcd.c | 37 +++++++++++++++++++++----------------
1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 5527d45..3a7e853 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -3006,13 +3006,15 @@ static int ufshcd_compose_dev_cmd(struct ufs_hba *hba,
}

/*
- * Clear all the requests from the controller for which a bit has been set in
- * @mask and wait until the controller confirms that these requests have been
- * cleared.
+ * Clear the pending command in the controller and wait until
+ * the controller confirms that the command has been cleared.
+ * @hba: per adapter instance
+ * @task_tag: The tag number of the command to be cleared.
*/
-static int ufshcd_clear_cmds(struct ufs_hba *hba, u32 mask)
+static int ufshcd_clear_cmds(struct ufs_hba *hba, u32 task_tag)
{
unsigned long flags;
+ u32 mask = 1U << task_tag;

/* clear outstanding transaction before retry */
spin_lock_irqsave(hba->host->host_lock, flags);
@@ -3113,7 +3115,7 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba,
err = -ETIMEDOUT;
dev_dbg(hba->dev, "%s: dev_cmd request timedout, tag %d\n",
__func__, lrbp->task_tag);
- if (ufshcd_clear_cmds(hba, 1U << lrbp->task_tag) == 0) {
+ if (ufshcd_clear_cmds(hba, lrbp->task_tag) == 0) {
/* successfully cleared the command, retry if needed */
err = -EAGAIN;
/*
@@ -7286,7 +7288,7 @@ static int ufshcd_eh_device_reset_handler(struct scsi_cmnd *cmd)
unsigned long flags, pending_reqs = 0, not_cleared = 0;
struct Scsi_Host *host;
struct ufs_hba *hba;
- u32 pos;
+ u32 pos, not_cleared_mask = 0;
int err;
u8 resp = 0xF, lun;

@@ -7309,17 +7311,20 @@ static int ufshcd_eh_device_reset_handler(struct scsi_cmnd *cmd)
hba->outstanding_reqs &= ~pending_reqs;
spin_unlock_irqrestore(&hba->outstanding_lock, flags);

- if (ufshcd_clear_cmds(hba, pending_reqs) < 0) {
- spin_lock_irqsave(&hba->outstanding_lock, flags);
- not_cleared = pending_reqs &
- ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
- hba->outstanding_reqs |= not_cleared;
- spin_unlock_irqrestore(&hba->outstanding_lock, flags);
+ for_each_set_bit(pos, &pending_reqs, hba->nutrs) {
+ if (ufshcd_clear_cmds(hba, pos) < 0) {
+ spin_lock_irqsave(&hba->outstanding_lock, flags);
+ not_cleared = 1U << pos &
+ ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
+ hba->outstanding_reqs |= not_cleared;
+ not_cleared_mask |= not_cleared;
+ spin_unlock_irqrestore(&hba->outstanding_lock, flags);

- dev_err(hba->dev, "%s: failed to clear requests %#lx\n",
- __func__, not_cleared);
+ dev_err(hba->dev, "%s: failed to clear requests %#lx\n",
+ __func__, not_cleared);
+ }
}
- __ufshcd_transfer_req_compl(hba, pending_reqs & ~not_cleared);
+ __ufshcd_transfer_req_compl(hba, pending_reqs & ~not_cleared_mask);

out:
hba->req_abort_count = 0;
@@ -7416,7 +7421,7 @@ static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
goto out;
}

- err = ufshcd_clear_cmds(hba, 1U << tag);
+ err = ufshcd_clear_cmds(hba, tag);
if (err)
dev_err(hba->dev, "%s: Failed clearing cmd at tag %d, err %d\n",
__func__, tag, err);
--
2.7.4


2023-05-10 21:32:18

by Bart Van Assche

[permalink] [raw]
Subject: Re: [PATCH v3 2/7] ufs: core: Update the ufshcd_clear_cmds() functionality

On 5/9/23 22:24, Bao D. Nguyen wrote:
> - dev_err(hba->dev, "%s: failed to clear requests %#lx\n",
> - __func__, not_cleared);
> + dev_err(hba->dev, "%s: failed to clear requests %#lx\n",
> + __func__, not_cleared);
> + }

Please change "requests" into "request" and "not_cleared" into "pos".
Otherwise this patch looks good to me. Hence:

Reviewed-by: Bart Van Assche <[email protected]>

2023-05-10 22:08:26

by Bart Van Assche

[permalink] [raw]
Subject: Re: [PATCH v3 2/7] ufs: core: Update the ufshcd_clear_cmds() functionality

On 5/9/23 22:24, Bao D. Nguyen wrote:
> -static int ufshcd_clear_cmds(struct ufs_hba *hba, u32 mask)
> +static int ufshcd_clear_cmds(struct ufs_hba *hba, u32 task_tag)

Since this patch changes the behavior of this function from aborting
multiple commands into aborting a single command, please rename
ufshcd_clear_cmds() into ufshcd_clear_cmd() in this patch.

Thanks,

Bart.