diff mbox series

[v9,3/3] ufs: core: add a quirk for MediaTek SDB mode aborted

Message ID 20240925095546.19492-4-peter.wang@mediatek.com (mailing list archive)
State Superseded
Headers show
Series fix abort defect | expand

Commit Message

Peter Wang (王信友) Sept. 25, 2024, 9:55 a.m. UTC 
From: Peter Wang <peter.wang@mediatek.com>

Because the MediaTek UFS controller uses UTRLCLR to clear commands
and fills the OCS with ABORTED, this patch introduces a quirk to
treat ABORTED as INVALID_OCS_VALUE.

Signed-off-by: Peter Wang <peter.wang@mediatek.com>
---
 drivers/ufs/core/ufshcd.c       | 5 ++++-
 drivers/ufs/host/ufs-mediatek.c | 1 +
 include/ufs/ufshcd.h            | 6 ++++++
 3 files changed, 11 insertions(+), 1 deletion(-)

Comments

Bart Van Assche Sept. 25, 2024, 4:56 p.m. UTC | #1 
On 9/25/24 2:55 AM, peter.wang@mediatek.com wrote:
> From: Peter Wang <peter.wang@mediatek.com>
> 
> Because the MediaTek UFS controller uses UTRLCLR to clear commands
> and fills the OCS with ABORTED, this patch introduces a quirk to
> treat ABORTED as INVALID_OCS_VALUE.
> 
> Signed-off-by: Peter Wang <peter.wang@mediatek.com>
> ---
>   drivers/ufs/core/ufshcd.c       | 5 ++++-
>   drivers/ufs/host/ufs-mediatek.c | 1 +
>   include/ufs/ufshcd.h            | 6 ++++++
>   3 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 4fff929b70d6..d429817fca94 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -5404,7 +5404,10 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
>   		}
>   		break;
>   	case OCS_ABORTED:
> -		result |= DID_ABORT << 16;
> +		if (hba->quirks & UFSHCD_QUIRK_OCS_ABORTED)
> +			result |= DID_REQUEUE << 16;
> +		else
> +			result |= DID_ABORT << 16;
>   		dev_warn(hba->dev,
>   				"OCS aborted from controller for tag %d\n",
>   				lrbp->task_tag);
> diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
> index 02c9064284e1..8a4c1b8f5a26 100644
> --- a/drivers/ufs/host/ufs-mediatek.c
> +++ b/drivers/ufs/host/ufs-mediatek.c
> @@ -1021,6 +1021,7 @@ static int ufs_mtk_init(struct ufs_hba *hba)
>   	hba->quirks |= UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL;
>   	hba->quirks |= UFSHCD_QUIRK_MCQ_BROKEN_INTR;
>   	hba->quirks |= UFSHCD_QUIRK_MCQ_BROKEN_RTC;
> +	hba->quirks |= UFSHCD_QUIRK_OCS_ABORTED;
>   	hba->vps->wb_flush_threshold = UFS_WB_BUF_REMAIN_PERCENT(80);
>   
>   	if (host->caps & UFS_MTK_CAP_DISABLE_AH8)
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index 0fd2aebac728..8f156803d703 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -684,6 +684,12 @@ enum ufshcd_quirks {
>   	 * single doorbell mode.
>   	 */
>   	UFSHCD_QUIRK_BROKEN_LSDBS_CAP			= 1 << 25,
> +
> +	/*
> +	 * Some host controllers set OCS_ABORTED after UTRLCLR (SDB mode),
> +	 * this quirk is set to treat OCS: ABORTED as INVALID_OCS_VALUE
> +	 */
> +	UFSHCD_QUIRK_OCS_ABORTED			= 1 << 26,
>   };
>   
>   enum ufshcd_caps {

ufshcd_transfer_rsp_status() only has one caller, namely 
ufshcd_compl_one_cqe(). The previous patch makes sure that that 
ufshcd_compl_one_cqe() is not called if a SCSI command is aborted. So
why does this patch modify how OCS_ABORTED is processed? Is this patch
necessary or can it perhaps be dropped?

Thanks,

Bart.
Peter Wang (王信友) Sept. 26, 2024, 3:42 a.m. UTC | #2 
On Wed, 2024-09-25 at 09:56 -0700, Bart Van Assche wrote:
> 
> 
> ufshcd_transfer_rsp_status() only has one caller, namely 
> ufshcd_compl_one_cqe(). The previous patch makes sure that that 
> ufshcd_compl_one_cqe() is not called if a SCSI command is aborted. So
> why does this patch modify how OCS_ABORTED is processed? Is this
> patch
> necessary or can it perhaps be dropped?
> 
> Thanks,
> 
> Bart.

Hi Bart,

Because in Legacy SDB mode, the error handler still has the 
chance to call ufshcd_compl_one_cqe(); 
The call flow is as follows:

ufshcd_err_handler()
  ufshcd_abort_all()
    ufshcd_abort_one()
      ufshcd_try_to_abort_task()	// mediatek controller fill
OCS: ABORTED
    ufshcd_complete_requests()
      ufshcd_transfer_req_compl()
        ufshcd_poll()
          get outstanding_lock
          clear outstanding_reqs tag
          release outstanding_lock	
          __ufshcd_transfer_req_compl()
            ufshcd_compl_one_cqe()
              cmd->result = DID_REQUEUE // mediatek need quirk change
DID_ABORT to DID_REQUEUE
              ufshcd_release_scsi_cmd()
              scsi_done()

Thanks.
Peter
diff mbox series

Patch

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 4fff929b70d6..d429817fca94 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -5404,7 +5404,10 @@  ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
 		}
 		break;
 	case OCS_ABORTED:
-		result |= DID_ABORT << 16;
+		if (hba->quirks & UFSHCD_QUIRK_OCS_ABORTED)
+			result |= DID_REQUEUE << 16;
+		else
+			result |= DID_ABORT << 16;
 		dev_warn(hba->dev,
 				"OCS aborted from controller for tag %d\n",
 				lrbp->task_tag);
diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
index 02c9064284e1..8a4c1b8f5a26 100644
--- a/drivers/ufs/host/ufs-mediatek.c
+++ b/drivers/ufs/host/ufs-mediatek.c
@@ -1021,6 +1021,7 @@  static int ufs_mtk_init(struct ufs_hba *hba)
 	hba->quirks |= UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL;
 	hba->quirks |= UFSHCD_QUIRK_MCQ_BROKEN_INTR;
 	hba->quirks |= UFSHCD_QUIRK_MCQ_BROKEN_RTC;
+	hba->quirks |= UFSHCD_QUIRK_OCS_ABORTED;
 	hba->vps->wb_flush_threshold = UFS_WB_BUF_REMAIN_PERCENT(80);
 
 	if (host->caps & UFS_MTK_CAP_DISABLE_AH8)
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 0fd2aebac728..8f156803d703 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -684,6 +684,12 @@  enum ufshcd_quirks {
 	 * single doorbell mode.
 	 */
 	UFSHCD_QUIRK_BROKEN_LSDBS_CAP			= 1 << 25,
+
+	/*
+	 * Some host controllers set OCS_ABORTED after UTRLCLR (SDB mode),
+	 * this quirk is set to treat OCS: ABORTED as INVALID_OCS_VALUE
+	 */
+	UFSHCD_QUIRK_OCS_ABORTED			= 1 << 26,
 };
 
 enum ufshcd_caps {