* [RFC PATCH v3 0/3] Support vendor specific operations for WB [not found] <CGME20200721095644epcas2p4aa2e620257686679e6c9e58ccabeac34@epcas2p4.samsung.com> @ 2020-07-21 9:57 ` SEO HOYOUNG [not found] ` <CGME20200721095648epcas2p18c3d496076ecd1537e47081c19dbb97e@epcas2p1.samsung.com> ` (2 more replies) 0 siblings, 3 replies; 15+ messages in thread From: SEO HOYOUNG @ 2020-07-21 9:57 UTC (permalink / raw) To: linux-scsi, alim.akhtar, avri.altman, jejb, martin.petersen, beanhuo, asutoshd, cang, bvanassche, grant.jung Cc: SEO HOYOUNG Hi all, Here is v3 of the patchset. This patchs for supporting WB of vendor specific. - when UFS reset and restore, need to cleare WB buffer. - need to check WB buffer and flush operate before enter suspend - do not enable WB with UFS probe. [v1 -> v2] - The ufshcd_reset_vendor() fuction for WB reset. So I modified function name. - uploade vendor wb code. [v2 -> v3] modified minor details about blank, typing error etc.. SEO HOYOUNG (3): scsi: ufs: modify write booster scsi: ufs: modify function call name When ufs reset and restore, need to disable write booster scsi: ufs: add vendor specific write booster drivers/scsi/ufs/Makefile | 1 + drivers/scsi/ufs/ufs-exynos.c | 6 + drivers/scsi/ufs/ufs_ctmwb.c | 270 ++++++++++++++++++++++++++++++++++ drivers/scsi/ufs/ufs_ctmwb.h | 26 ++++ drivers/scsi/ufs/ufshcd.c | 22 ++- drivers/scsi/ufs/ufshcd.h | 43 ++++++ 6 files changed, 363 insertions(+), 5 deletions(-) create mode 100644 drivers/scsi/ufs/ufs_ctmwb.c create mode 100644 drivers/scsi/ufs/ufs_ctmwb.h -- 2.26.0 ^ permalink raw reply [flat|nested] 15+ messages in thread
[parent not found: <CGME20200721095648epcas2p18c3d496076ecd1537e47081c19dbb97e@epcas2p1.samsung.com>]
* [RFC PATCH v3 1/3] scsi: ufs: modify write booster [not found] ` <CGME20200721095648epcas2p18c3d496076ecd1537e47081c19dbb97e@epcas2p1.samsung.com> @ 2020-07-21 9:57 ` SEO HOYOUNG 2020-07-26 6:30 ` Avri Altman 0 siblings, 1 reply; 15+ messages in thread From: SEO HOYOUNG @ 2020-07-21 9:57 UTC (permalink / raw) To: linux-scsi, alim.akhtar, avri.altman, jejb, martin.petersen, beanhuo, asutoshd, cang, bvanassche, grant.jung Cc: SEO HOYOUNG Add vendor specific functions for WB Use callback additional setting when use write booster. Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com> --- drivers/scsi/ufs/ufshcd.c | 22 +++++++++++++++----- drivers/scsi/ufs/ufshcd.h | 43 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index efc0a6cbfe22..9261519e7e9a 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -3306,11 +3306,11 @@ int ufshcd_read_string_desc(struct ufs_hba *hba, u8 desc_index, * * Return 0 in case of success, non-zero otherwise */ -static inline int ufshcd_read_unit_desc_param(struct ufs_hba *hba, - int lun, - enum unit_desc_param param_offset, - u8 *param_read_buf, - u32 param_size) +int ufshcd_read_unit_desc_param(struct ufs_hba *hba, + int lun, + enum unit_desc_param param_offset, + u8 *param_read_buf, + u32 param_size) { /* * Unit descriptors are only available for general purpose LUs (LUN id @@ -3322,6 +3322,7 @@ static inline int ufshcd_read_unit_desc_param(struct ufs_hba *hba, return ufshcd_read_desc_param(hba, QUERY_DESC_IDN_UNIT, lun, param_offset, param_read_buf, param_size); } +EXPORT_SYMBOL_GPL(ufshcd_read_unit_desc_param); static int ufshcd_get_ref_clk_gating_wait(struct ufs_hba *hba) { @@ -5257,6 +5258,10 @@ static int ufshcd_wb_ctrl(struct ufs_hba *hba, bool enable) if (!(enable ^ hba->wb_enabled)) return 0; + + if (!ufshcd_wb_ctrl_vendor(hba, enable)) + return 0; + if (enable) opcode = UPIU_QUERY_OPCODE_SET_FLAG; else @@ -6610,6 +6615,8 @@ static int ufshcd_reset_and_restore(struct ufs_hba *hba) int err = 0; int retries = MAX_HOST_RESET_RETRIES; + ufshcd_reset_vendor(hba); + do { /* Reset the attached device */ ufshcd_vops_device_reset(hba); @@ -6903,6 +6910,9 @@ static void ufshcd_wb_probe(struct ufs_hba *hba, u8 *desc_buf) if (!(dev_info->d_ext_ufs_feature_sup & UFS_DEV_WRITE_BOOSTER_SUP)) goto wb_disabled; + if (!ufshcd_wb_alloc_units_vendor(hba)) + return; + /* * WB may be supported but not configured while provisioning. * The spec says, in dedicated wb buffer mode, @@ -8273,6 +8283,8 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) ufshcd_wb_need_flush(hba)); } + ufshcd_wb_toggle_flush_vendor(hba, pm_op); + if (req_dev_pwr_mode != hba->curr_dev_pwr_mode) { if ((ufshcd_is_runtime_pm(pm_op) && !hba->auto_bkops_enabled) || !ufshcd_is_runtime_pm(pm_op)) { diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index 656c0691c858..deb9577e0eaa 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -254,6 +254,13 @@ struct ufs_pwr_mode_info { struct ufs_pa_layer_attr info; }; +struct ufs_wb_ops { + int (*wb_toggle_flush_vendor)(struct ufs_hba *hba, enum ufs_pm_op pm_op); + int (*wb_alloc_units_vendor)(struct ufs_hba *hba); + int (*wb_ctrl_vendor)(struct ufs_hba *hba, bool enable); + int (*wb_reset_vendor)(struct ufs_hba *hba, bool force); +}; + /** * struct ufs_hba_variant_ops - variant specific callbacks * @name: variant name @@ -752,6 +759,7 @@ struct ufs_hba { struct request_queue *bsg_queue; bool wb_buf_flush_enabled; bool wb_enabled; + struct ufs_wb_ops *wb_ops; struct delayed_work rpm_dev_flush_recheck_work; #ifdef CONFIG_SCSI_UFS_CRYPTO @@ -1004,6 +1012,10 @@ int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba, u8 *desc_buff, int *buff_len, enum query_opcode desc_op); +int ufshcd_read_unit_desc_param(struct ufs_hba *hba, + int lun, enum unit_desc_param param_offset, + u8 *param_read_buf, u32 param_size); + /* Wrapper functions for safely calling variant operations */ static inline const char *ufshcd_get_var_name(struct ufs_hba *hba) { @@ -1181,4 +1193,35 @@ static inline u8 ufshcd_scsi_to_upiu_lun(unsigned int scsi_lun) int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len, const char *prefix); +static inline int ufshcd_wb_toggle_flush_vendor(struct ufs_hba *hba, enum ufs_pm_op pm_op) +{ + if (!hba->wb_ops || !hba->wb_ops->wb_toggle_flush_vendor) + return -1; + + return hba->wb_ops->wb_toggle_flush_vendor(hba, pm_op); +} + +static int ufshcd_wb_alloc_units_vendor(struct ufs_hba *hba) +{ + if (!hba->wb_ops || !hba->wb_ops->wb_alloc_units_vendor) + return -1; + + return hba->wb_ops->wb_alloc_units_vendor(hba); +} + +static int ufshcd_wb_ctrl_vendor(struct ufs_hba *hba, bool enable) +{ + if (!hba->wb_ops || !hba->wb_ops->wb_ctrl_vendor) + return -1; + + return hba->wb_ops->wb_ctrl_vendor(hba, enable); +} + +static int ufshcd_reset_vendor(struct ufs_hba *hba) +{ + if (!hba->wb_ops || !hba->wb_ops->wb_reset_vendor) + return -1; + + return hba->wb_ops->wb_reset_vendor(hba, false); +} #endif /* End of Header */ -- 2.26.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* RE: [RFC PATCH v3 1/3] scsi: ufs: modify write booster 2020-07-21 9:57 ` [RFC PATCH v3 1/3] scsi: ufs: modify write booster SEO HOYOUNG @ 2020-07-26 6:30 ` Avri Altman 2020-07-27 9:57 ` 서호영 0 siblings, 1 reply; 15+ messages in thread From: Avri Altman @ 2020-07-26 6:30 UTC (permalink / raw) To: SEO HOYOUNG, linux-scsi, alim.akhtar, jejb, martin.petersen, beanhuo, asutoshd, cang, bvanassche, grant.jung > Add vendor specific functions for WB > Use callback additional setting when use write booster. > > Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com> > --- > drivers/scsi/ufs/ufshcd.c | 22 +++++++++++++++----- > drivers/scsi/ufs/ufshcd.h | 43 +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 60 insertions(+), 5 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index efc0a6cbfe22..9261519e7e9a 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -3306,11 +3306,11 @@ int ufshcd_read_string_desc(struct ufs_hba *hba, > u8 desc_index, > * > * Return 0 in case of success, non-zero otherwise > */ > -static inline int ufshcd_read_unit_desc_param(struct ufs_hba *hba, > - int lun, > - enum unit_desc_param param_offset, > - u8 *param_read_buf, > - u32 param_size) > +int ufshcd_read_unit_desc_param(struct ufs_hba *hba, > + int lun, > + enum unit_desc_param param_offset, > + u8 *param_read_buf, > + u32 param_size) > { > /* > * Unit descriptors are only available for general purpose LUs (LUN id > @@ -3322,6 +3322,7 @@ static inline int > ufshcd_read_unit_desc_param(struct ufs_hba *hba, > return ufshcd_read_desc_param(hba, QUERY_DESC_IDN_UNIT, lun, > param_offset, param_read_buf, param_size); > } > +EXPORT_SYMBOL_GPL(ufshcd_read_unit_desc_param); Like I already told you in your v1: If you are relaying on ufsfeatures - you need to wait for it to be merged. Meanwhile, it got nacked (nack^2 actually), so you need to take this into account. Thanks, Avri ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [RFC PATCH v3 1/3] scsi: ufs: modify write booster 2020-07-26 6:30 ` Avri Altman @ 2020-07-27 9:57 ` 서호영 2020-07-27 10:10 ` Avri Altman 0 siblings, 1 reply; 15+ messages in thread From: 서호영 @ 2020-07-27 9:57 UTC (permalink / raw) To: 'Avri Altman', linux-scsi, alim.akhtar, jejb, martin.petersen, beanhuo, asutoshd, cang, bvanassche, grant.jung > > Add vendor specific functions for WB > > Use callback additional setting when use write booster. > > > > Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com> > > --- > > drivers/scsi/ufs/ufshcd.c | 22 +++++++++++++++----- > > drivers/scsi/ufs/ufshcd.h | 43 +++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 60 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > > index efc0a6cbfe22..9261519e7e9a 100644 > > --- a/drivers/scsi/ufs/ufshcd.c > > +++ b/drivers/scsi/ufs/ufshcd.c > > @@ -3306,11 +3306,11 @@ int ufshcd_read_string_desc(struct ufs_hba > > *hba, > > u8 desc_index, > > * > > * Return 0 in case of success, non-zero otherwise > > */ > > -static inline int ufshcd_read_unit_desc_param(struct ufs_hba *hba, > > - int lun, > > - enum unit_desc_param param_offset, > > - u8 *param_read_buf, > > - u32 param_size) > > +int ufshcd_read_unit_desc_param(struct ufs_hba *hba, > > + int lun, > > + enum unit_desc_param param_offset, > > + u8 *param_read_buf, > > + u32 param_size) > > { > > /* > > * Unit descriptors are only available for general purpose LUs > > (LUN id @@ -3322,6 +3322,7 @@ static inline int > > ufshcd_read_unit_desc_param(struct ufs_hba *hba, > > return ufshcd_read_desc_param(hba, QUERY_DESC_IDN_UNIT, lun, > > param_offset, param_read_buf, > > param_size); } > > +EXPORT_SYMBOL_GPL(ufshcd_read_unit_desc_param); > Like I already told you in your v1: > If you are relaying on ufsfeatures - you need to wait for it to be merged. > Meanwhile, it got nacked (nack^2 actually), so you need to take this into > account. Sorry, for not catching this. Then can I know when the code was merged? I will remove this function. > > Thanks, > Avri ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [RFC PATCH v3 1/3] scsi: ufs: modify write booster 2020-07-27 9:57 ` 서호영 @ 2020-07-27 10:10 ` Avri Altman 2020-07-27 11:28 ` 서호영 0 siblings, 1 reply; 15+ messages in thread From: Avri Altman @ 2020-07-27 10:10 UTC (permalink / raw) To: 서호영, linux-scsi, alim.akhtar, jejb, martin.petersen, beanhuo, asutoshd, cang, bvanassche, grant.jung > > Like I already told you in your v1: > > If you are relaying on ufsfeatures - you need to wait for it to be merged. > > Meanwhile, it got nacked (nack^2 actually), so you need to take this into > > account. > > Sorry, for not catching this. > Then can I know when the code was merged? > I will remove this function. You can follow "scsi: ufs: Add Host Performance Booster Support" - Only if you are not removing the dependency in ufsfeatures. ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [RFC PATCH v3 1/3] scsi: ufs: modify write booster 2020-07-27 10:10 ` Avri Altman @ 2020-07-27 11:28 ` 서호영 0 siblings, 0 replies; 15+ messages in thread From: 서호영 @ 2020-07-27 11:28 UTC (permalink / raw) To: 'Avri Altman', linux-scsi, alim.akhtar, jejb, martin.petersen, beanhuo, asutoshd, cang, bvanassche, grant.jung > > > > Like I already told you in your v1: > > > If you are relaying on ufsfeatures - you need to wait for it to be > merged. > > > Meanwhile, it got nacked (nack^2 actually), so you need to take this > > > into account. > > > > Sorry, for not catching this. > > Then can I know when the code was merged? > > I will remove this function. > You can follow "scsi: ufs: Add Host Performance Booster Support" - Only if > you are not removing the dependency in ufsfeatures. OK, I will remove this ^ permalink raw reply [flat|nested] 15+ messages in thread
[parent not found: <CGME20200721095653epcas2p4575db5cbcd8897662ad19465339128b2@epcas2p4.samsung.com>]
* [RFC PATCH v3 2/3] scsi: ufs: modify function call name When ufs reset and restore, need to disable write booster [not found] ` <CGME20200721095653epcas2p4575db5cbcd8897662ad19465339128b2@epcas2p4.samsung.com> @ 2020-07-21 9:57 ` SEO HOYOUNG 2020-07-26 6:31 ` Avri Altman 0 siblings, 1 reply; 15+ messages in thread From: SEO HOYOUNG @ 2020-07-21 9:57 UTC (permalink / raw) To: linux-scsi, alim.akhtar, avri.altman, jejb, martin.petersen, beanhuo, asutoshd, cang, bvanassche, grant.jung Cc: SEO HOYOUNG Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com> --- drivers/scsi/ufs/ufshcd.c | 2 +- drivers/scsi/ufs/ufshcd.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 9261519e7e9a..3eb139406a7c 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -6615,7 +6615,7 @@ static int ufshcd_reset_and_restore(struct ufs_hba *hba) int err = 0; int retries = MAX_HOST_RESET_RETRIES; - ufshcd_reset_vendor(hba); + ufshcd_wb_reset_vendor(hba); do { /* Reset the attached device */ diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index deb9577e0eaa..61ae5259c62a 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -1217,7 +1217,7 @@ static int ufshcd_wb_ctrl_vendor(struct ufs_hba *hba, bool enable) return hba->wb_ops->wb_ctrl_vendor(hba, enable); } -static int ufshcd_reset_vendor(struct ufs_hba *hba) +static int ufshcd_wb_reset_vendor(struct ufs_hba *hba) { if (!hba->wb_ops || !hba->wb_ops->wb_reset_vendor) return -1; -- 2.26.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* RE: [RFC PATCH v3 2/3] scsi: ufs: modify function call name When ufs reset and restore, need to disable write booster 2020-07-21 9:57 ` [RFC PATCH v3 2/3] scsi: ufs: modify function call name When ufs reset and restore, need to disable " SEO HOYOUNG @ 2020-07-26 6:31 ` Avri Altman 2020-07-27 10:01 ` 서호영 0 siblings, 1 reply; 15+ messages in thread From: Avri Altman @ 2020-07-26 6:31 UTC (permalink / raw) To: SEO HOYOUNG, linux-scsi, alim.akhtar, jejb, martin.petersen, beanhuo, asutoshd, cang, bvanassche, grant.jung This patch is not really needed - just squash it to the previous one. > > > Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com> > --- > drivers/scsi/ufs/ufshcd.c | 2 +- > drivers/scsi/ufs/ufshcd.h | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index 9261519e7e9a..3eb139406a7c 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -6615,7 +6615,7 @@ static int ufshcd_reset_and_restore(struct ufs_hba > *hba) > int err = 0; > int retries = MAX_HOST_RESET_RETRIES; > > - ufshcd_reset_vendor(hba); > + ufshcd_wb_reset_vendor(hba); > > do { > /* Reset the attached device */ > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h > index deb9577e0eaa..61ae5259c62a 100644 > --- a/drivers/scsi/ufs/ufshcd.h > +++ b/drivers/scsi/ufs/ufshcd.h > @@ -1217,7 +1217,7 @@ static int ufshcd_wb_ctrl_vendor(struct ufs_hba > *hba, bool enable) > return hba->wb_ops->wb_ctrl_vendor(hba, enable); > } > > -static int ufshcd_reset_vendor(struct ufs_hba *hba) > +static int ufshcd_wb_reset_vendor(struct ufs_hba *hba) > { > if (!hba->wb_ops || !hba->wb_ops->wb_reset_vendor) > return -1; > -- > 2.26.0 ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [RFC PATCH v3 2/3] scsi: ufs: modify function call name When ufs reset and restore, need to disable write booster 2020-07-26 6:31 ` Avri Altman @ 2020-07-27 10:01 ` 서호영 2020-07-27 10:12 ` Avri Altman 0 siblings, 1 reply; 15+ messages in thread From: 서호영 @ 2020-07-27 10:01 UTC (permalink / raw) To: 'Avri Altman', linux-scsi, alim.akhtar, jejb, martin.petersen, beanhuo, asutoshd, cang, bvanassche, grant.jung > This patch is not really needed - just squash it to the previous one. Why you said this patch is not really needed? I don't understand Our WB device need to disable WB when called ufshcd_reset_and_restore() func. Please explain reason. > > > > > > > Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com> > > --- > > drivers/scsi/ufs/ufshcd.c | 2 +- > > drivers/scsi/ufs/ufshcd.h | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > > index 9261519e7e9a..3eb139406a7c 100644 > > --- a/drivers/scsi/ufs/ufshcd.c > > +++ b/drivers/scsi/ufs/ufshcd.c > > @@ -6615,7 +6615,7 @@ static int ufshcd_reset_and_restore(struct > > ufs_hba > > *hba) > > int err = 0; > > int retries = MAX_HOST_RESET_RETRIES; > > > > - ufshcd_reset_vendor(hba); > > + ufshcd_wb_reset_vendor(hba); > > > > do { > > /* Reset the attached device */ diff --git > > a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index > > deb9577e0eaa..61ae5259c62a 100644 > > --- a/drivers/scsi/ufs/ufshcd.h > > +++ b/drivers/scsi/ufs/ufshcd.h > > @@ -1217,7 +1217,7 @@ static int ufshcd_wb_ctrl_vendor(struct ufs_hba > > *hba, bool enable) > > return hba->wb_ops->wb_ctrl_vendor(hba, enable); } > > > > -static int ufshcd_reset_vendor(struct ufs_hba *hba) > > +static int ufshcd_wb_reset_vendor(struct ufs_hba *hba) > > { > > if (!hba->wb_ops || !hba->wb_ops->wb_reset_vendor) > > return -1; > > -- > > 2.26.0 ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [RFC PATCH v3 2/3] scsi: ufs: modify function call name When ufs reset and restore, need to disable write booster 2020-07-27 10:01 ` 서호영 @ 2020-07-27 10:12 ` Avri Altman 2020-07-27 11:27 ` 서호영 0 siblings, 1 reply; 15+ messages in thread From: Avri Altman @ 2020-07-27 10:12 UTC (permalink / raw) To: 서호영, linux-scsi, alim.akhtar, jejb, martin.petersen, beanhuo, asutoshd, cang, bvanassche, grant.jung > > > This patch is not really needed - just squash it to the previous one. > Why you said this patch is not really needed? > I don't understand > Our WB device need to disable WB when called ufshcd_reset_and_restore() > func. > Please explain reason. This patch only change the names of some functions defined in the first patch. Squash it to the first one. ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [RFC PATCH v3 2/3] scsi: ufs: modify function call name When ufs reset and restore, need to disable write booster 2020-07-27 10:12 ` Avri Altman @ 2020-07-27 11:27 ` 서호영 2020-07-27 11:29 ` Avri Altman 0 siblings, 1 reply; 15+ messages in thread From: 서호영 @ 2020-07-27 11:27 UTC (permalink / raw) To: 'Avri Altman', linux-scsi, alim.akhtar, jejb, martin.petersen, beanhuo, asutoshd, cang, bvanassche, grant.jung > > > > > > This patch is not really needed - just squash it to the previous one. > > Why you said this patch is not really needed? > > I don't understand > > Our WB device need to disable WB when called > > ufshcd_reset_and_restore() func. > > Please explain reason. > This patch only change the names of some functions defined in the first > patch. > Squash it to the first one. "Asutosh Das (asd) <asutoshd@codeaurora.org>" said this function not clearly. So I modify this function name. I think this name is clear than the previous name. ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [RFC PATCH v3 2/3] scsi: ufs: modify function call name When ufs reset and restore, need to disable write booster 2020-07-27 11:27 ` 서호영 @ 2020-07-27 11:29 ` Avri Altman 2020-07-27 11:33 ` 서호영 0 siblings, 1 reply; 15+ messages in thread From: Avri Altman @ 2020-07-27 11:29 UTC (permalink / raw) To: 서호영, linux-scsi, alim.akhtar, jejb, martin.petersen, beanhuo, asutoshd, cang, bvanassche, grant.jung > > > > This patch is not really needed - just squash it to the previous one. > > > Why you said this patch is not really needed? > > > I don't understand > > > Our WB device need to disable WB when called > > > ufshcd_reset_and_restore() func. > > > Please explain reason. > > This patch only change the names of some functions defined in the first > > patch. > > Squash it to the first one. > > "Asutosh Das (asd) <asutoshd@codeaurora.org>" said this function not > clearly. > So I modify this function name. > I think this name is clear than the previous name. I am not arguing about that. Are you familiar with the concept of squash? ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [RFC PATCH v3 2/3] scsi: ufs: modify function call name When ufs reset and restore, need to disable write booster 2020-07-27 11:29 ` Avri Altman @ 2020-07-27 11:33 ` 서호영 0 siblings, 0 replies; 15+ messages in thread From: 서호영 @ 2020-07-27 11:33 UTC (permalink / raw) To: 'Avri Altman', linux-scsi, alim.akhtar, jejb, martin.petersen, beanhuo, asutoshd, cang, bvanassche, grant.jung > > > > > This patch is not really needed - just squash it to the previous > one. > > > > Why you said this patch is not really needed? > > > > I don't understand > > > > Our WB device need to disable WB when called > > > > ufshcd_reset_and_restore() func. > > > > Please explain reason. > > > This patch only change the names of some functions defined in the > > > first patch. > > > Squash it to the first one. > > > > "Asutosh Das (asd) <asutoshd@codeaurora.org>" said this function not > > clearly. > > So I modify this function name. > > I think this name is clear than the previous name. > I am not arguing about that. > Are you familiar with the concept of squash? Sorry, I misunderstand. I do not know very well. Please can you explain concept of squash to me? ^ permalink raw reply [flat|nested] 15+ messages in thread
[parent not found: <CGME20200721095655epcas2p13c10101ef53de47384762754271c1ca5@epcas2p1.samsung.com>]
* [RFC PATCH v3 3/3] scsi: ufs: add vendor specific write booster [not found] ` <CGME20200721095655epcas2p13c10101ef53de47384762754271c1ca5@epcas2p1.samsung.com> @ 2020-07-21 9:57 ` SEO HOYOUNG 2020-07-26 6:39 ` Avri Altman 0 siblings, 1 reply; 15+ messages in thread From: SEO HOYOUNG @ 2020-07-21 9:57 UTC (permalink / raw) To: linux-scsi, alim.akhtar, avri.altman, jejb, martin.petersen, beanhuo, asutoshd, cang, bvanassche, grant.jung Cc: SEO HOYOUNG To support the fuction of writebooster by vendor. The WB behavior that the vendor wants is slightly different. But we have to support it Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com> --- drivers/scsi/ufs/Makefile | 1 + drivers/scsi/ufs/ufs-exynos.c | 6 + drivers/scsi/ufs/ufs_ctmwb.c | 270 ++++++++++++++++++++++++++++++++++ drivers/scsi/ufs/ufs_ctmwb.h | 26 ++++ 4 files changed, 303 insertions(+) create mode 100644 drivers/scsi/ufs/ufs_ctmwb.c create mode 100644 drivers/scsi/ufs/ufs_ctmwb.h diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile index 9810963bc049..b1ba36c7d66f 100644 --- a/drivers/scsi/ufs/Makefile +++ b/drivers/scsi/ufs/Makefile @@ -5,6 +5,7 @@ obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-g210-pltfrm.o ufshcd-dwc.o tc-d obj-$(CONFIG_SCSI_UFS_CDNS_PLATFORM) += cdns-pltfrm.o obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o obj-$(CONFIG_SCSI_UFS_EXYNOS) += ufs-exynos.o +obj-$(CONFIG_SCSI_UFS_VENDOR_WB) += ufs_ctmwb.o obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o ufshcd-core-y += ufshcd.o ufs-sysfs.o ufshcd-core-$(CONFIG_SCSI_UFS_BSG) += ufs_bsg.o diff --git a/drivers/scsi/ufs/ufs-exynos.c b/drivers/scsi/ufs/ufs-exynos.c index 32b61ba77241..f127f5f2bf36 100644 --- a/drivers/scsi/ufs/ufs-exynos.c +++ b/drivers/scsi/ufs/ufs-exynos.c @@ -22,6 +22,9 @@ #include "ufs-exynos.h" +#ifdef CONFIG_SCSI_UFS_VENDOR_WB +#include "ufs_ctmwb.h" +#endif /* * Exynos's Vendor specific registers for UFSHCI */ @@ -989,6 +992,9 @@ static int exynos_ufs_init(struct ufs_hba *hba) goto phy_off; ufs->hba = hba; +#ifdef CONFIG_SCSI_UFS_VENDOR_WB + ufs->hba->wb_ops = ufshcd_ctmwb_init(); +#endif ufs->opts = ufs->drv_data->opts; ufs->rx_sel_idx = PA_MAXDATALANES; if (ufs->opts & EXYNOS_UFS_OPT_BROKEN_RX_SEL_IDX) diff --git a/drivers/scsi/ufs/ufs_ctmwb.c b/drivers/scsi/ufs/ufs_ctmwb.c new file mode 100644 index 000000000000..e1cad936ebb3 --- /dev/null +++ b/drivers/scsi/ufs/ufs_ctmwb.c @@ -0,0 +1,270 @@ +#include "ufshcd.h" +#include "ufshci.h" +#include "ufs_ctmwb.h" + +static struct ufshba_ctmwb hba_ctmwb; + +/* Query request retries */ +#define QUERY_REQ_RETRIES 3 + +static int ufshcd_query_attr_retry(struct ufs_hba *hba, + enum query_opcode opcode, enum attr_idn idn, u8 index, u8 selector, + u32 *attr_val) +{ + int ret = 0; + u32 retries; + + for (retries = QUERY_REQ_RETRIES; retries > 0; retries--) { + ret = ufshcd_query_attr(hba, opcode, idn, index, + selector, attr_val); + if (ret) + dev_dbg(hba->dev, "%s: failed with error %d, retries %d\n", + __func__, ret, retries); + else + break; + } + + if (ret) + dev_err(hba->dev, + "%s: query attribute, idn %d, failed with error %d after %d retires\n", + __func__, idn, ret, QUERY_REQ_RETRIES); + return ret; +} + +static int ufshcd_query_flag_retry(struct ufs_hba *hba, + enum query_opcode opcode, enum flag_idn idn, bool *flag_res) +{ + int ret; + int retries; + + for (retries = 0; retries < QUERY_REQ_RETRIES; retries++) { + ret = ufshcd_query_flag(hba, opcode, idn, flag_res); + if (ret) + dev_dbg(hba->dev, + "%s: failed with error %d, retries %d\n", + __func__, ret, retries); + else + break; + } + + if (ret) + dev_err(hba->dev, + "%s: query attribute, opcode %d, idn %d, failed with error %d after %d retries\n", + __func__, (int)opcode, (int)idn, ret, retries); + return ret; +} + +static int ufshcd_reset_ctmwb(struct ufs_hba *hba, bool force) +{ + int err = 0; + + if (!ufshcd_is_wb_allowed(hba)) + return 0; + + if (ufshcd_is_ctmwb_off(hba_ctmwb)) { + dev_info(hba->dev, "%s: write booster already disabled. ctmwb_state = %d\n", + __func__, hba_ctmwb.ufs_ctmwb_state); + return 0; + } + + if (ufshcd_is_ctmwb_err(hba_ctmwb)) + dev_err(hba->dev, "%s: previous write booster control was failed.\n", + __func__); + + if (force) + err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_CLEAR_FLAG, + QUERY_FLAG_IDN_WB_EN, NULL); + + if (err) { + ufshcd_set_ctmwb_err(hba_ctmwb); + dev_err(hba->dev, "%s: disable write booster failed. err = %d\n", + __func__, err); + } else { + ufshcd_set_ctmwb_off(hba_ctmwb); + dev_info(hba->dev, "%s: ufs write booster disabled \n", __func__); + } + + return 0; +} + +static int ufshcd_get_ctmwb_buf_status(struct ufs_hba *hba, u32 *status) +{ + return ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR, + QUERY_ATTR_IDN_AVAIL_WB_BUFF_SIZE, 0, 0, status); +} + +static int ufshcd_ctmwb_manual_flush_ctrl(struct ufs_hba *hba, int en) +{ + int err = 0; + + dev_info(hba->dev, "%s: %sable write booster manual flush\n", + __func__, en ? "en" : "dis"); + if (en) { + err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_SET_FLAG, + QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, NULL); + if (err) + dev_err(hba->dev, "%s: enable write booster failed. err = %d\n", + __func__, err); + } else { + err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_CLEAR_FLAG, + QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, NULL); + if (err) + dev_err(hba->dev, "%s: disable write booster failed. err = %d\n", + __func__, err); + } + + return err; +} + +static int ufshcd_ctmwb_flush_ctrl(struct ufs_hba *hba) +{ + int err = 0; + u32 curr_status = 0; + + err = ufshcd_get_ctmwb_buf_status(hba, &curr_status); + + if (!err && (curr_status <= UFS_WB_MANUAL_FLUSH_THRESHOLD)) { + dev_info(hba->dev, "%s: enable ctmwb manual flush, buf status : %d\n", + __func__, curr_status); + scsi_block_requests(hba->host); + err = ufshcd_ctmwb_manual_flush_ctrl(hba, 1); + if (!err) { + mdelay(100); + err = ufshcd_ctmwb_manual_flush_ctrl(hba, 0); + if (err) + dev_err(hba->dev, "%s: disable ctmwb manual flush failed. err = %d\n", + __func__, err); + } else + dev_err(hba->dev, "%s: enable ctmwb manual flush failed. err = %d\n", + __func__, err); + scsi_unblock_requests(hba->host); + } + return err; +} + +static int ufshcd_ctmwb_ctrl(struct ufs_hba *hba, bool enable) +{ + int err; + + if (!ufshcd_is_wb_allowed(hba)) + return 0; + + if (hba->pm_op_in_progress) { + dev_err(hba->dev, "%s: ctmwb ctrl during pm operation is not allowed.\n", + __func__); + return 0; + } + + if (ufshcd_is_ctmwb_err(hba_ctmwb)) + dev_err(hba->dev, "%s: previous write booster control was failed.\n", + __func__); + if (enable) { + if (ufshcd_is_ctmwb_on(hba_ctmwb)) { + dev_err(hba->dev, "%s: write booster already enabled. ctmwb_state = %d\n", + __func__, hba_ctmwb.ufs_ctmwb_state); + return 0; + } + pm_runtime_get_sync(hba->dev); + err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_SET_FLAG, + QUERY_FLAG_IDN_WB_EN, NULL); + if (err) { + ufshcd_set_ctmwb_err(hba_ctmwb); + dev_err(hba->dev, "%s: enable write booster failed. err = %d\n", + __func__, err); + } else { + ufshcd_set_ctmwb_on(hba_ctmwb); + dev_info(hba->dev, "%s: ufs write booster enabled \n", __func__); + } + } else { + if (ufshcd_is_ctmwb_off(hba_ctmwb)) { + dev_err(hba->dev, "%s: write booster already disabled. ctmwb_state = %d\n", + __func__, hba_ctmwb.ufs_ctmwb_state); + return 0; + } + pm_runtime_get_sync(hba->dev); + err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_CLEAR_FLAG, + QUERY_FLAG_IDN_WB_EN, NULL); + if (err) { + ufshcd_set_ctmwb_err(hba_ctmwb); + dev_err(hba->dev, "%s: disable write booster failed. err = %d\n", + __func__, err); + } else { + ufshcd_set_ctmwb_off(hba_ctmwb); + dev_info(hba->dev, "%s: ufs write booster disabled \n", __func__); + } + } + + pm_runtime_put_sync(hba->dev); + + return 0; +} + +/** + * ufshcd_get_ctmwbbuf_unit - get ctmwb buffer alloc units + * @sdev: pointer to SCSI device + * + * Read dLUNumTurboWriteBufferAllocUnits in UNIT Descriptor + * to check if LU supports write booster feature + */ +static int ufshcd_get_ctmwbbuf_unit(struct ufs_hba *hba) +{ + struct scsi_device *sdev = hba->sdev_ufs_device; + struct ufshba_ctmwb *hba_ctmwb = (struct ufshba_ctmwb *)hba->wb_ops; + int ret = 0; + + u32 dLUNumTurboWriteBufferAllocUnits = 0; + u8 desc_buf[4]; + + if (!ufshcd_is_wb_allowed(hba)) + return 0; + + ret = ufshcd_read_unit_desc_param(hba, + ufshcd_scsi_to_upiu_lun(sdev->lun), + UNIT_DESC_PARAM_WB_BUF_ALLOC_UNITS, + desc_buf, + sizeof(dLUNumTurboWriteBufferAllocUnits)); + + /* Some WLUN doesn't support unit descriptor */ + if ((ret == -EOPNOTSUPP) || scsi_is_wlun(sdev->lun)){ + hba_ctmwb->support_ctmwb_lu = false; + dev_info(hba->dev,"%s: do not support WB\n", __func__); + return 0; + } + + dLUNumTurboWriteBufferAllocUnits = ((desc_buf[0] << 24)| + (desc_buf[1] << 16) | + (desc_buf[2] << 8) | + desc_buf[3]); + + if (dLUNumTurboWriteBufferAllocUnits) { + hba_ctmwb->support_ctmwb_lu = true; + dev_info(hba->dev, "%s: LU %d supports ctmwb, ctmwbbuf unit : 0x%x\n", + __func__, (int)sdev->lun, dLUNumTurboWriteBufferAllocUnits); + } else + hba_ctmwb->support_ctmwb_lu = false; + + return 0; +} + +static inline int ufshcd_ctmwb_toggle_flush(struct ufs_hba *hba, enum ufs_pm_op pm_op) +{ + ufshcd_ctmwb_flush_ctrl(hba); + + if (ufshcd_is_system_pm(pm_op)) + ufshcd_reset_ctmwb(hba, true); + + return 0; +} + +static struct ufs_wb_ops exynos_ctmwb_ops = { + .wb_toggle_flush_vendor = ufshcd_ctmwb_toggle_flush, + .wb_alloc_units_vendor = ufshcd_get_ctmwbbuf_unit, + .wb_ctrl_vendor = ufshcd_ctmwb_ctrl, + .wb_reset_vendor = ufshcd_reset_ctmwb, +}; + +struct ufs_wb_ops *ufshcd_ctmwb_init(void) +{ + return &exynos_ctmwb_ops; +} +EXPORT_SYMBOL_GPL(ufshcd_ctmwb_init); diff --git a/drivers/scsi/ufs/ufs_ctmwb.h b/drivers/scsi/ufs/ufs_ctmwb.h new file mode 100644 index 000000000000..e88b71824a2f --- /dev/null +++ b/drivers/scsi/ufs/ufs_ctmwb.h @@ -0,0 +1,26 @@ +#ifndef _UFS_CTMWB_H_ +#define _UFS_CTMWB_H_ + +enum ufs_ctmwb_state { + UFS_WB_OFF_STATE = 0, /* turbo write disabled state */ + UFS_WB_ON_STATE = 1, /* turbo write enabled state */ + UFS_WB_ERR_STATE = 2, /* turbo write error state */ +}; + +#define ufshcd_is_ctmwb_off(hba) ((hba).ufs_ctmwb_state == UFS_WB_OFF_STATE) +#define ufshcd_is_ctmwb_on(hba) ((hba).ufs_ctmwb_state == UFS_WB_ON_STATE) +#define ufshcd_is_ctmwb_err(hba) ((hba).ufs_ctmwb_state == UFS_WB_ERR_STATE) +#define ufshcd_set_ctmwb_off(hba) ((hba).ufs_ctmwb_state = UFS_WB_OFF_STATE) +#define ufshcd_set_ctmwb_on(hba) ((hba).ufs_ctmwb_state = UFS_WB_ON_STATE) +#define ufshcd_set_ctmwb_err(hba) ((hba).ufs_ctmwb_state = UFS_WB_ERR_STATE) + +#define UFS_WB_MANUAL_FLUSH_THRESHOLD 5 + +struct ufshba_ctmwb { + enum ufs_ctmwb_state ufs_ctmwb_state; + + bool support_ctmwb_lu; +}; + +struct ufs_wb_ops *ufshcd_ctmwb_init(void); +#endif -- 2.26.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* RE: [RFC PATCH v3 3/3] scsi: ufs: add vendor specific write booster 2020-07-21 9:57 ` [RFC PATCH v3 3/3] scsi: ufs: add vendor specific " SEO HOYOUNG @ 2020-07-26 6:39 ` Avri Altman 0 siblings, 0 replies; 15+ messages in thread From: Avri Altman @ 2020-07-26 6:39 UTC (permalink / raw) To: SEO HOYOUNG, linux-scsi, alim.akhtar, jejb, martin.petersen, beanhuo, asutoshd, cang, bvanassche, grant.jung > > > To support the fuction of writebooster by vendor. > The WB behavior that the vendor wants is slightly different. > But we have to support it I guess that mainly you are looking for other options than devfreq. Please add few lines how are you going to replace it. > > Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com> > --- > drivers/scsi/ufs/Makefile | 1 + > drivers/scsi/ufs/ufs-exynos.c | 6 + > drivers/scsi/ufs/ufs_ctmwb.c | 270 ++++++++++++++++++++++++++++++++++ > drivers/scsi/ufs/ufs_ctmwb.h | 26 ++++ > 4 files changed, 303 insertions(+) > create mode 100644 drivers/scsi/ufs/ufs_ctmwb.c > create mode 100644 drivers/scsi/ufs/ufs_ctmwb.h > > diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile > index 9810963bc049..b1ba36c7d66f 100644 > --- a/drivers/scsi/ufs/Makefile > +++ b/drivers/scsi/ufs/Makefile > @@ -5,6 +5,7 @@ obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc- > g210-pltfrm.o ufshcd-dwc.o tc-d > obj-$(CONFIG_SCSI_UFS_CDNS_PLATFORM) += cdns-pltfrm.o > obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o > obj-$(CONFIG_SCSI_UFS_EXYNOS) += ufs-exynos.o > +obj-$(CONFIG_SCSI_UFS_VENDOR_WB) += ufs_ctmwb.o You need to define it first in Kconfig ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2020-07-27 11:33 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20200721095644epcas2p4aa2e620257686679e6c9e58ccabeac34@epcas2p4.samsung.com> 2020-07-21 9:57 ` [RFC PATCH v3 0/3] Support vendor specific operations for WB SEO HOYOUNG [not found] ` <CGME20200721095648epcas2p18c3d496076ecd1537e47081c19dbb97e@epcas2p1.samsung.com> 2020-07-21 9:57 ` [RFC PATCH v3 1/3] scsi: ufs: modify write booster SEO HOYOUNG 2020-07-26 6:30 ` Avri Altman 2020-07-27 9:57 ` 서호영 2020-07-27 10:10 ` Avri Altman 2020-07-27 11:28 ` 서호영 [not found] ` <CGME20200721095653epcas2p4575db5cbcd8897662ad19465339128b2@epcas2p4.samsung.com> 2020-07-21 9:57 ` [RFC PATCH v3 2/3] scsi: ufs: modify function call name When ufs reset and restore, need to disable " SEO HOYOUNG 2020-07-26 6:31 ` Avri Altman 2020-07-27 10:01 ` 서호영 2020-07-27 10:12 ` Avri Altman 2020-07-27 11:27 ` 서호영 2020-07-27 11:29 ` Avri Altman 2020-07-27 11:33 ` 서호영 [not found] ` <CGME20200721095655epcas2p13c10101ef53de47384762754271c1ca5@epcas2p1.samsung.com> 2020-07-21 9:57 ` [RFC PATCH v3 3/3] scsi: ufs: add vendor specific " SEO HOYOUNG 2020-07-26 6:39 ` Avri Altman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).