mbox series

[v7,0/4] fix abort defect

Message ID 20240920090643.3566-1-peter.wang@mediatek.com
Headers show
Series fix abort defect | expand

Message

Peter Wang (王信友) Sept. 20, 2024, 9:06 a.m. UTC
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(-)

Comments

Bart Van Assche Sept. 20, 2024, 7:36 p.m. UTC | #1
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.
Peter Wang (王信友) Sept. 23, 2024, 7:07 a.m. UTC | #2
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