RE: [PATCH v1] scsi: ufs: add a variant operation in struct ufs_hba_variant_ops

[Hoyoung SEO]

> -----Original Message-----
> From: Alim Akhtar [mailto:alim.akhtar@xxxxxxxxxxx]
> Sent: Tuesday, September 13, 2022 1:19 PM
> To: 'SEO HOYOUNG'; linux-scsi@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; avri.altman@xxxxxxx; jejb@xxxxxxxxxxxxx;
> martin.petersen@xxxxxxxxxx; beanhuo@xxxxxxxxxx; asutoshd@xxxxxxxxxxxxxx;
> cang@xxxxxxxxxxxxxx; bvanassche@xxxxxxx; bhoon95.kim@xxxxxxxxxxx;
> kwmad.kim@xxxxxxxxxxx
> Subject: RE: [PATCH v1] scsi: ufs: add a variant operation in struct
> ufs_hba_variant_ops
> 
> 
> 
> >-----Original Message-----
> >From: SEO HOYOUNG [mailto:hy50.seo@xxxxxxxxxxx]
> >Sent: Monday, September 12, 2022 7:30 PM
> >To: linux-scsi@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> >alim.akhtar@xxxxxxxxxxx; avri.altman@xxxxxxx; jejb@xxxxxxxxxxxxx;
> >martin.petersen@xxxxxxxxxx; beanhuo@xxxxxxxxxx;
> >asutoshd@xxxxxxxxxxxxxx; cang@xxxxxxxxxxxxxx; bvanassche@xxxxxxx;
> >bhoon95.kim@xxxxxxxxxxx; kwmad.kim@xxxxxxxxxxx
> >Cc: SEO HOYOUNG <hy50.seo@xxxxxxxxxxx>
> >Subject: [PATCH v1] scsi: ufs: add a variant operation in struct
> >ufs_hba_variant_ops
> >
> >Add ufs_hba_variant_ops about vendor error in check_error.
> >It need to reset when occurred ah8 related error.
> >At that time could not recovery with pwr cmd.
> >So add vendor error check function at ufs_hba_variant_ops.
> >
> I didn't understand why you need this, looks like you are using AH8.
> So IS (interrupt status) bit[5], bit[6] , HIBERNATE_EXIT and
> HIBERNATE_ENTRY should still give you the error status?

The AH8 FSM machine check power_local result.
If the expected value of power_local is different 
during hibern8 Enter/Exit, then will occur UHES/UHXS.
Other than that, errors could not occurred IS(interrupt status)
bit[5], bit[6].
So we need to check 'vendor IS' for checking error of AH8 FSM.

> 
> 
> >Change-Id: I24c76a372931e702b357c86a5dc36e93ce4b5fda
> This is something unwanted
> 
> >Signed-off-by: SEO HOYOUNG <hy50.seo@xxxxxxxxxxx>
> >---
> > drivers/ufs/core/ufshcd-priv.h |  7 +++++++
> > drivers/ufs/core/ufshcd.c      |  2 ++
> > drivers/ufs/host/ufs-exynos.c  | 19 +++++++++++++++++++
> > include/ufs/ufshcd.h           |  1 +
> > 4 files changed, 29 insertions(+)
> >
> >diff --git a/drivers/ufs/core/ufshcd-priv.h
> >b/drivers/ufs/core/ufshcd-priv.h index d00dba17297d..6172da4d3484
> >100644
> >--- a/drivers/ufs/core/ufshcd-priv.h
> >+++ b/drivers/ufs/core/ufshcd-priv.h
> >@@ -221,6 +221,13 @@ static inline void
> >ufshcd_vops_config_scaling_param(struct ufs_hba *hba,
> > 		hba->vops->config_scaling_param(hba, p, data);  }
> >
> >+static inline void ufshcd_vops_check_int_error(struct ufs_hba *hba,
> >+					       bool *queue_eh_work)
> >+{
> >+	if (hba->vops & hba->vops->check_int_error)
> >+		hba->vops->check_int_error(hba, queue_eh_work); }
> >+
> > 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 7c15cbc737b4..39ee5192f26a 100644
> >--- a/drivers/ufs/core/ufshcd.c
> >+++ b/drivers/ufs/core/ufshcd.c
> >@@ -6542,6 +6542,8 @@ static irqreturn_t ufshcd_check_errors(struct
> >ufs_hba *hba, u32 intr_status)
> > 		queue_eh_work = true;
> > 	}
> >
> >+	ufshcd_vops_check_int_error(hba, &queue_eh_work);
> >+
> > 	if (queue_eh_work) {
> > 		/*
> > 		 * update the transfer error masks to sticky bits, let's do
> this
> >diff --git a/drivers/ufs/host/ufs-exynos.c
> >b/drivers/ufs/host/ufs-exynos.c index eced97538082..b351b8e48b08 100644
> >--- a/drivers/ufs/host/ufs-exynos.c
> >+++ b/drivers/ufs/host/ufs-exynos.c
> >@@ -67,6 +67,10 @@
> > #define CLK_CTRL_EN_MASK	(REFCLK_CTRL_EN |\
> > 				 UNIPRO_PCLK_CTRL_EN |\
> > 				 UNIPRO_MCLK_CTRL_EN)
> >+
> >+#define HCI_AH8_STATE		0x50C
> >+#define HCI_AH8_STATE_ERROR	BIT(16)
> >+
> > /* Device fatal error */
> > #define DFES_ERR_EN		BIT(31)
> > #define DFES_DEF_L2_ERRS	(UIC_DATA_LINK_LAYER_ERROR_RX_BUF_OF
> >|\
> >@@ -1376,6 +1380,20 @@ static void exynos_ufs_hibern8_notify(struct
> >ufs_hba *hba,
> > 	}
> > }
> >
> >+static void exynos_ufs_check_int_error(struct ufs_hba *hba, bool
> >+*queue_eh_work) {
> >+	struct exynos_ufs *ufs = ufshcd_get_variant(hba);
> >+
> >+	if (ufshcd_is_auto_hibern8_supported(hba)) {
> >+		val = hci_readl(ufs, HCI_AH8_STATE);
> >+
> >+		if (val & HCI_AH8_STATE_ERROR) {
> >+			 ufshcd_set_link_broken(hba);
> >+			 queue_eh_work = true;
> >+		}
> >+	}
> >+}
> >+
> > static int exynos_ufs_suspend(struct ufs_hba *hba, enum ufs_pm_op
> >pm_op,
> > 	enum ufs_notify_change_status status)  { @@ -1569,6 +1587,7 @@
> static
> >const struct ufs_hba_variant_ops ufs_hba_exynos_ops = {
> > 	.setup_xfer_req			=
> >exynos_ufs_specify_nexus_t_xfer_req,
> > 	.setup_task_mgmt		=
> >exynos_ufs_specify_nexus_t_tm_req,
> > 	.hibern8_notify			= exynos_ufs_hibern8_notify,
> >+	.check_int_error		= exynos_ufs_check_int_error,
> > 	.suspend			= exynos_ufs_suspend,
> > 	.resume				= exynos_ufs_resume,
> > };
> >diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h index
> >24c97e0772bb..40078c4b9f55 100644
> >--- a/include/ufs/ufshcd.h
> >+++ b/include/ufs/ufshcd.h
> >@@ -335,6 +335,7 @@ struct ufs_hba_variant_ops {
> > 			       const union ufs_crypto_cfg_entry *cfg, int slot);
> > 	void	(*event_notify)(struct ufs_hba *hba,
> > 				enum ufs_event_type evt, void *data);
> >+	void	(*check_int_error)(struct ufs_hba *hba, bool
> >*queue_eh_work);
> > };
> >
> > /* clock gating state  */
> >--
> >2.26.0
>