| Message ID | 20220912140000.95483-1-hy50.seo@samsung.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1] scsi: ufs: add a variant operation in struct ufs_hba_variant_ops | expand |
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/202209131145.uQlsf9Uv-lkp@intel.com/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 <lkp@intel.com>
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
>-----Original Message----- >From: SEO HOYOUNG [mailto:hy50.seo@samsung.com] >Sent: Monday, September 12, 2022 7:30 PM >To: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org; >alim.akhtar@samsung.com; avri.altman@wdc.com; jejb@linux.ibm.com; >martin.petersen@oracle.com; beanhuo@micron.com; >asutoshd@codeaurora.org; cang@codeaurora.org; bvanassche@acm.org; >bhoon95.kim@samsung.com; kwmad.kim@samsung.com >Cc: SEO HOYOUNG <hy50.seo@samsung.com> >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 <hy50.seo@samsung.com> >--- > 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:alim.akhtar@samsung.com] > Sent: Tuesday, September 13, 2022 1:19 PM > To: 'SEO HOYOUNG'; linux-scsi@vger.kernel.org; linux- > kernel@vger.kernel.org; avri.altman@wdc.com; jejb@linux.ibm.com; > martin.petersen@oracle.com; beanhuo@micron.com; asutoshd@codeaurora.org; > cang@codeaurora.org; bvanassche@acm.org; bhoon95.kim@samsung.com; > kwmad.kim@samsung.com > Subject: RE: [PATCH v1] scsi: ufs: add a variant operation in struct > ufs_hba_variant_ops > > > > >-----Original Message----- > >From: SEO HOYOUNG [mailto:hy50.seo@samsung.com] > >Sent: Monday, September 12, 2022 7:30 PM > >To: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org; > >alim.akhtar@samsung.com; avri.altman@wdc.com; jejb@linux.ibm.com; > >martin.petersen@oracle.com; beanhuo@micron.com; > >asutoshd@codeaurora.org; cang@codeaurora.org; bvanassche@acm.org; > >bhoon95.kim@samsung.com; kwmad.kim@samsung.com > >Cc: SEO HOYOUNG <hy50.seo@samsung.com> > >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@samsung.com> > >--- > > 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 >
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 */
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 <hy50.seo@samsung.com> --- 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(+)