[RESEND PATCH v2 0/2] scsi: ufs: introduce a callback to override OCS value

[RESEND PATCH v2 0/2] scsi: ufs: introduce a callback to override OCS value

[Kiwoong Kim]

I send this again adding more reviewers.

UFSHCI defines OCS values but doesn't specify what exact
conditions raise them. So I think it needs another callback
to replace the original OCS value with the value that works
the way you want.

v1 -> v2: fix build error for arguments

Kiwoong Kim (2):
  scsi: ufs: core: introduce override_cqe_ocs
  scsi: ufs: ufs-exynos: implement override_cqe_ocs

 drivers/ufs/core/ufshcd-priv.h |  9 +++++++++
 drivers/ufs/core/ufshcd.c      | 11 +++++++----
 drivers/ufs/host/ufs-exynos.c  |  8 ++++++++
 include/ufs/ufshcd.h           |  1 +
 4 files changed, 25 insertions(+), 4 deletions(-)


base-commit: 3ba963597d19d88eb06b50af8e8757abbdc9035b
-- 
2.26.0

[RESEND PATCH v2 1/2] scsi: ufs: core: introduce override_cqe_ocs

[Kiwoong Kim]

This patch is to declare override_cqe_ocs callback to
override OCS value.

Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
---
 drivers/ufs/core/ufshcd-priv.h |  9 +++++++++
 drivers/ufs/core/ufshcd.c      | 11 +++++++----
 include/ufs/ufshcd.h           |  1 +
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index ce36154ce963..6ebc83029e09 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -275,6 +275,15 @@ static inline int ufshcd_mcq_vops_config_esi(struct ufs_hba *hba)
 	return -EOPNOTSUPP;
 }
 
+static inline enum utp_ocs ufshcd_vops_override_cqe_ocs(struct ufs_hba *hba,
+							enum utp_ocs ocs)
+{
+	if (hba->vops && hba->vops->override_cqe_ocs)
+		return hba->vops->override_cqe_ocs(ocs);
+
+	return ocs;
+}
+
 extern const struct ufs_pm_lvl_states ufs_pm_lvl_states[];
 
 /**
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 0dd26059f5d7..0615e372fe44 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -821,11 +821,14 @@ static inline bool ufshcd_is_device_present(struct ufs_hba *hba)
  *
  * Return: the OCS field in the UTRD.
  */
-static enum utp_ocs ufshcd_get_tr_ocs(struct ufshcd_lrb *lrbp,
+static enum utp_ocs ufshcd_get_tr_ocs(struct ufs_hba *hba,
+				      struct ufshcd_lrb *lrbp,
 				      struct cq_entry *cqe)
 {
 	if (cqe)
-		return le32_to_cpu(cqe->status) & MASK_OCS;
+		return ufshcd_vops_override_cqe_ocs(hba,
+						    le32_to_cpu(cqe->status) &
+						    MASK_OCS);
 
 	return lrbp->utr_descriptor_ptr->header.ocs & MASK_OCS;
 }
@@ -3180,7 +3183,7 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba,
 		 * not trigger any race conditions.
 		 */
 		hba->dev_cmd.complete = NULL;
-		err = ufshcd_get_tr_ocs(lrbp, NULL);
+		err = ufshcd_get_tr_ocs(hba, lrbp, NULL);
 		if (!err)
 			err = ufshcd_dev_cmd_completion(hba, lrbp);
 	} else {
@@ -5351,7 +5354,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
 		scsi_set_resid(lrbp->cmd, resid);
 
 	/* overall command status of utrd */
-	ocs = ufshcd_get_tr_ocs(lrbp, cqe);
+	ocs = ufshcd_get_tr_ocs(hba, lrbp, cqe);
 
 	if (hba->quirks & UFSHCD_QUIRK_BROKEN_OCS_FATAL_ERROR) {
 		if (lrbp->ucd_rsp_ptr->header.response ||
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index a43b14276bc3..3dbd3e41b022 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -382,6 +382,7 @@ struct ufs_hba_variant_ops {
 	int	(*get_outstanding_cqs)(struct ufs_hba *hba,
 				       unsigned long *ocqs);
 	int	(*config_esi)(struct ufs_hba *hba);
+	enum utp_ocs	(*override_cqe_ocs)(enum utp_ocs);
 };
 
 /* clock gating state  */
-- 
2.26.0

[RESEND PATCH v2 2/2] scsi: ufs: ufs-exynos: implement override_cqe_ocs

[Kiwoong Kim]

Exynos host reports OCS_ABORT when a command is nullifed
or cleaned up with MCQ enabled. I think the command in those
situations should be issued again, rather than fail, because
when some conditions that caused the nullification or cleaning up
disppears after recovery, the command could be processed.

Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
---
 drivers/ufs/host/ufs-exynos.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/ufs/host/ufs-exynos.c b/drivers/ufs/host/ufs-exynos.c
index 16ad3528d80b..7ff0e84adaab 100644
--- a/drivers/ufs/host/ufs-exynos.c
+++ b/drivers/ufs/host/ufs-exynos.c
@@ -1376,6 +1376,13 @@ static void exynos_ufs_fmp_resume(struct ufs_hba *hba)
 
 #endif /* !CONFIG_SCSI_UFS_CRYPTO */
 
+static enum utp_ocs exynos_ufs_override_cqe_ocs(enum utp_ocs ocs)
+{
+	if (ocs == OCS_ABORTED)
+		ocs = OCS_INVALID_COMMAND_STATUS;
+	return ocs;
+}
+
 static int exynos_ufs_init(struct ufs_hba *hba)
 {
 	struct device *dev = hba->dev;
@@ -1926,6 +1933,7 @@ static const struct ufs_hba_variant_ops ufs_hba_exynos_ops = {
 	.suspend			= exynos_ufs_suspend,
 	.resume				= exynos_ufs_resume,
 	.fill_crypto_prdt		= exynos_ufs_fmp_fill_prdt,
+	.override_cqe_ocs		= exynos_ufs_override_cqe_ocs,
 };
 
 static struct ufs_hba_variant_ops ufs_hba_exynosauto_vh_ops = {
-- 
2.26.0

Re: [RESEND PATCH v2 2/2] scsi: ufs: ufs-exynos: implement override_cqe_ocs

[Bart Van Assche]

On 9/1/24 9:26 PM, Kiwoong Kim wrote:
> Exynos host reports OCS_ABORT when a command is nullifed
> or cleaned up with MCQ enabled.

That is the behavior that is required by the UFSHCI 4.0 standard. Hence,
handling the OCS_ABORTED response should be the same for all host
controllers and no new callback function should be introduced to handle
nullified commands.

> I think the command in those
> situations should be issued again, rather than fail, because
> when some conditions that caused the nullification or cleaning up
> disppears after recovery, the command could be processed.

ufshcd_mcq_nullify_sqe() is called by ufshcd_mcq_sqe_search() and the
latter function is called by ufshcd_mcq_abort(). It is up to the SCSI
core to decide whether or not commands aborted by ufshcd_mcq_abort()
should be resubmitted. This is not something the host driver should
decide about.

Thanks,

Bart.