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.
Change-Id: I24c76a372931e702b357c86a5dc36e93ce4b5fda
Signed-off-by: SEO HOYOUNG <[email protected]>
---
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
Hi SEO,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on jejb-scsi/for-next]
[also build test WARNING on mkp-scsi/for-next krzk/for-next linus/master v6.0-rc5 next-20220912]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/SEO-HOYOUNG/scsi-ufs-add-a-variant-operation-in-struct-ufs_hba_variant_ops/20220913-101855
base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: loongarch-allyesconfig (https://download.01.org/0day-ci/archive/20220913/[email protected]/config)
compiler: loongarch64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/60f9cc2a287e2bfe58c8355519797a9071b00afa
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review SEO-HOYOUNG/scsi-ufs-add-a-variant-operation-in-struct-ufs_hba_variant_ops/20220913-101855
git checkout 60f9cc2a287e2bfe58c8355519797a9071b00afa
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch SHELL=/bin/bash drivers/ufs/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
drivers/ufs/host/ufs-exynos.c: In function 'exynos_ufs_check_int_error':
drivers/ufs/host/ufs-exynos.c:1388:17: error: 'val' undeclared (first use in this function)
1388 | val = hci_readl(ufs, HCI_AH8_STATE);
| ^~~
drivers/ufs/host/ufs-exynos.c:1388:17: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/ufs/host/ufs-exynos.c:1392:40: warning: assignment to 'bool *' {aka '_Bool *'} from 'int' makes pointer from integer without a cast [-Wint-conversion]
1392 | queue_eh_work = true;
| ^
>> drivers/ufs/host/ufs-exynos.c:1383:67: warning: parameter 'queue_eh_work' set but not used [-Wunused-but-set-parameter]
1383 | static void exynos_ufs_check_int_error(struct ufs_hba *hba, bool *queue_eh_work)
| ~~~~~~^~~~~~~~~~~~~
vim +1392 drivers/ufs/host/ufs-exynos.c
1382
> 1383 static void exynos_ufs_check_int_error(struct ufs_hba *hba, bool *queue_eh_work)
1384 {
1385 struct exynos_ufs *ufs = ufshcd_get_variant(hba);
1386
1387 if (ufshcd_is_auto_hibern8_supported(hba)) {
1388 val = hci_readl(ufs, HCI_AH8_STATE);
1389
1390 if (val & HCI_AH8_STATE_ERROR) {
1391 ufshcd_set_link_broken(hba);
> 1392 queue_eh_work = true;
1393 }
1394 }
1395 }
1396
--
0-DAY CI Kernel Test Service
https://01.org/lkp
>-----Original Message-----
>From: SEO HOYOUNG [mailto:[email protected]]
>Sent: Monday, September 12, 2022 7:30 PM
>To: [email protected]; [email protected];
>[email protected]; [email protected]; [email protected];
>[email protected]; [email protected];
>[email protected]; [email protected]; [email protected];
>[email protected]; [email protected]
>Cc: SEO HOYOUNG <[email protected]>
>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?
>Change-Id: I24c76a372931e702b357c86a5dc36e93ce4b5fda
This is something unwanted
>Signed-off-by: SEO HOYOUNG <[email protected]>
>---
> 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
> -----Original Message-----
> From: Alim Akhtar [mailto:[email protected]]
> Sent: Tuesday, September 13, 2022 1:19 PM
> To: 'SEO HOYOUNG'; [email protected]; linux-
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: RE: [PATCH v1] scsi: ufs: add a variant operation in struct
> ufs_hba_variant_ops
>
>
>
> >-----Original Message-----
> >From: SEO HOYOUNG [mailto:[email protected]]
> >Sent: Monday, September 12, 2022 7:30 PM
> >To: [email protected]; [email protected];
> >[email protected]; [email protected]; [email protected];
> >[email protected]; [email protected];
> >[email protected]; [email protected]; [email protected];
> >[email protected]; [email protected]
> >Cc: SEO HOYOUNG <[email protected]>
> >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 <[email protected]>
> >---
> > 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
>