| Message ID | 20240925095546.19492-4-peter.wang@mediatek.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fix abort defect | expand |
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.
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 --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 {