On 4/25/2023 5:08 PM, Bart Van Assche wrote:
On 4/17/23 14:05, Bao D. Nguyen wrote:
@@ -3110,7 +3128,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, 1UL << lrbp->task_tag) == 0) {
/* successfully cleared the command, retry if needed */
err = -EAGAIN;
/*
Is this change necessary?
My intention was to support tag greater than 31 and less than 64.
The 1U << only works up to 32 bits.
@@ -7379,6 +7397,20 @@ static int ufshcd_try_to_abort_task(struct
ufs_hba *hba, int tag)
*/
dev_err(hba->dev, "%s: cmd at tag %d not pending in the
device.\n",
__func__, tag);
+ if (is_mcq_enabled(hba)) {
+ /* MCQ mode */
+ if (lrbp->cmd) {
+ /* sleep for max. 200us to stabilize */
What is being stabilized here? Please make this comment more clear.
This is to keep the same operation/timing as in SDB mode.
+ usleep_range(100, 200);
+ continue;
+ }
+ /* command completed already */
+ dev_err(hba->dev, "%s: cmd at tag=%d is cleared.\n",
+ __func__, tag);
+ goto out;
+ }
Please do not use lrbp->cmd to check whether or not a command has
completed. See also my patch "scsi: ufs: Fix handling of lrbp->cmd".
I have been thinking how to replace lrbp->cmd, but could not find a good
solution. Throughout this patch series, I am using lrbp->cmd as a way to
find the pending command that is being aborted and clean up the
resources associated with it. Any suggestions to achieve it? Thanks.
@@ -7415,7 +7447,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, 1UL << tag);
if (err)
dev_err(hba->dev, "%s: Failed clearing cmd at tag %d, err
%d\n",
__func__, tag, err);
Is this change necessary?
Same as above, I intended to support 64 > tag > 31
- if (!(test_bit(tag, &hba->outstanding_reqs))) {
+ if (!is_mcq_enabled(hba) && !(test_bit(tag,
&hba->outstanding_reqs))) {
Please leave out one pair of superfluous parentheses from the above
expression.
Yes, I will change.
Thanks,
Bart.
