On Fri, 2024-09-13 at 10:41 -0700, Bart Van Assche wrote:
External email : Please do not click links or open attachments until you have verified the sender or the content. On 9/13/24 12:10 AM, Peter Wang (王信友) wrote:
Because the MediaTek UFS controller uses UTRLCLR to clear commands and fills OCS with ABORTED.
Regarding the specification of UTRCS: This bit is set to '1' by the host controller upon one of the following: Overall command Status (OCS) of the completed command is not equal to 'SUCCESS' even if its UTRD Interrupt bit set to '0'
So, MediaTek host controller will send interrupt in this case.
Hi Peter,
Thank you for having shared this information. Please consider introducing a quirk for ignoring completions triggered by clearing a command, e.g. as follows (there may be better approaches):
- In ufshcd_clear_cmd(), before a command is cleared, initialize the completion that will be used for waiting for the completion interrupt. After a command has been cleared, call wait_for_completion_timeout().
- In ufshcd_compl_one_cqe(), check whether the completion is the result of a command being cleared. If so, call complete() instead of executing the regular completion code.
Thanks,
Bart.
Hi Bart,
Sorry for the lack of response over the past few days due to the Mid-Autumn Festival in Taiwan. I think this would make the abort flow more complicated and difficult to understand.
Basically, this patch currently only needs to handle requeueing for the error handler abort. The approach for DBR mode and MCQ mode should be consistent. If receive an interrupt response (OCS:ABORTED or INVALID_OCS_VALUE), then set DID_REQUEUE. If there is no interrupt, it will also set SCSI DID_REQUEUE in ufshcd_err_handler through ufshcd_complete_requests with force_compl = true.
As for the SCSI abort (30s) situation, in DBR mode, if there is an interrupt with OCS:ABORTED or INVALID_OCS_VALUE, then set DID_REQUEUE; if not, ufshcd_abort will also return SUCCESS, allowing SCSI to decide whether to retry the command. This part is less problematic, and we can keep the original approach.
The more problematic part is with MCQ mode. To imitate the DBR approach, we just need to set DID_REQUEUE upon receiving an interrupt. Everything else remains the same. This would make things simpler.
Moving forward, if we want to simplify things and we have also taken stock of the two or three scenarios where OCS: ABORTED occurs, do we even need a flag? Couldn't we just set DID_REQUEUE directly for OCS: ABORTED? What do you think?
Thanks. Peter