| Message ID | 20240920090643.3566-1-peter.wang@mediatek.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | fix abort defect | expand |
On 9/20/24 2:06 AM, peter.wang@mediatek.com wrote:
> This series fixes MCQ and SDB abort defect.
Hi Peter,
Patches 2, 3 and 4 in this series introduce a significant amount of
complexity. Additionally, code paths are introduced that can only be
triggered by UFS controllers that (incorrectly) generate a completion
interrupt for aborted commands. I'm concerned that these patches will
make the UFS host controller driver harder to maintain than necessary.
Please take another look at the approach I proposed, namely making
ufshcd_compl_one_cqe() ignore commands with completion status
OCS_ABORTED. I think this approach will result in a simpler
implementation, does not require a new quirk and minimizes the code
paths that are only triggered by UFS host controllers that trigger a
completion interrupt for aborted commands.
Thanks,
Bart.
On Fri, 2024-09-20 at 12:36 -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/20/24 2:06 AM, peter.wang@mediatek.com wrote: > > This series fixes MCQ and SDB abort defect. > > Hi Peter, > > Patches 2, 3 and 4 in this series introduce a significant amount of > complexity. Additionally, code paths are introduced that can only be > triggered by UFS controllers that (incorrectly) generate a completion > interrupt for aborted commands. I'm concerned that these patches will > make the UFS host controller driver harder to maintain than > necessary. > Please take another look at the approach I proposed, namely making > ufshcd_compl_one_cqe() ignore commands with completion status > OCS_ABORTED. I think this approach will result in a simpler > implementation, does not require a new quirk and minimizes the code > paths that are only triggered by UFS host controllers that trigger a > completion interrupt for aborted commands. > > Thanks, > > Bart. Hi Bart, Because I feel it's a bit weird to intentionally ignore a CQ slot in MCQ mode and directly requeue, but I will try to make changes according to your idea. Thanks. Peter
From: Peter Wang <peter.wang@mediatek.com> This series fixes MCQ and SDB abort defect. V7: - Use a variable instead of a flag. - Add a check for MCQ mode when setting this variable to UFS_ERR_HANDLER. - Print OCS information for OCS_ABORTED and OCS_INVALID_COMMAND_STATUS. - Add a MediaTek quirk for handling OCS_ABORTED in SDB mode. - Skip notifying SCSI from ISR during SCSI abort (ufshcd_abort()). V6: - Add err handler check before set flag true. V5: - Change flag name. - Amend comment and patch description. V4: - Remove nullify SQ entry abort requeue. - Add more comment for flag usage and set description. - Fix build warning. V3: - Change comment and use variable(rtc) for error print - Change flag name and move flag set before ufshcd_clear_cmd - Add SDB mode clear UTRLCLR tag receive OCS_ABORTED requeue V2: - Fix mcq_enabled build error. Peter Wang (4): ufs: core: fix the issue of ICU failure ufs: core: requeue aborted request ufs: core: add a quirk for MediaTek SDB mode aborted ufs: core: skip ISR notifying scsi when ufshcd_abort drivers/ufs/core/ufs-mcq.c | 21 ++++++---- drivers/ufs/core/ufshcd.c | 68 +++++++++++++++++++++++++-------- drivers/ufs/host/ufs-mediatek.c | 1 + include/ufs/ufshcd.h | 15 ++++++++ 4 files changed, 83 insertions(+), 22 deletions(-)