On 9/10/24 12:30 AM, peter.wang@mediatek.com wrote:
ufshcd_abort_all froce abort all on-going command and the host
^^^^^ ^^^^^ ^^^^^^^ ^^^^ forcibly? aborts? commands? host controller?
will automatically fill in the OCS field of the corresponding response with OCS_ABORTED based on different working modes.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ The host controller only sets the OCS field to OCS_ABORTED in MCQ mode if the host controller successfully aborted the command. If the abort TMF is submitted to the UFS device, the OCS field won't be changed into OCS_ABORTED. In SDB mode, the host controller does not modify the OCS field either.
SDB mode: aborts a command using UTRLCLR. Task Management response which means a Transfer Request was aborted.
Hmm ... my understanding is that clearing a bit from UTRLCLR is only allowed *after* a command has been aborted and also that clearing a bit from this register does not abort a command but only frees the resources in the host controller associated with the command.
For these two cases, set a flag to notify SCSI to requeue the command after receiving response with OCS_ABORTED.
I think there is only one case when the SCSI core needs to be requested to requeue a command, namely when the UFS driver decided to initiate the abort (ufshcd_abort_all()).
@@ -7561,6 +7551,20 @@ int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag) goto out; }
- /*
* When the host software receives a "FUNCTION COMPLETE", set flag* to requeue command after receive response with OCS_ABORTED* SDB mode: UTRLCLR Task Management response which means a Transfer* Request was aborted.* MCQ mode: Host will post to CQ with OCS_ABORTED after SQ cleanup* This flag is set because ufshcd_abort_all forcibly aborts all* commands, and the host will automatically fill in the OCS field* of the corresponding response with OCS_ABORTED.* Therefore, upon receiving this response, it needs to be requeued.*/- if (!err)
lrbp->abort_initiated_by_err = true;- err = ufshcd_clear_cmd(hba, tag); if (err) dev_err(hba->dev, "%s: Failed clearing cmd at tag %d, err %d\n",
The above change is misplaced. ufshcd_try_to_abort_task() can be called when the SCSI core decides to abort a command while abort_initiated_by_err must not be set in that case. Please move the above code block into ufshcd_abort_one().
Regarding the word "host" in the above comment block: the host is the Android device. I think that in the above comment "host" should be changed into "host controller".
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h index 0fd2aebac728..15b357672ca5 100644 --- a/include/ufs/ufshcd.h +++ b/include/ufs/ufshcd.h @@ -173,6 +173,8 @@ struct ufs_pm_lvl_states {
- @crypto_key_slot: the key slot to use for inline crypto (-1 if none)
- @data_unit_num: the data unit number for the first block for inline crypto
- @req_abort_skip: skip request abort task flag
- @abort_initiated_by_err: The flag is specifically used to handle aborts
caused by errors due to host/device communication
The "abort_initiated_by_err" name still seems confusing to me. Please make it more clear that this flag is only set if the UFS error handler decides to abort a command. How about "abort_initiated_by_eh"?
Please also make the description of this member variable more clear.
Thanks,
Bart.