| Message ID | 20201211140035.20016-6-huobean@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Several changes for UFS WriteBooster | expand |
Hi Bean, On Fri, 2020-12-11 at 15:00 +0100, Bean Huo wrote: > From: Bean Huo <beanhuo@micron.com> > > Delete ufshcd_wb_buf_flush_enable() and ufshcd_wb_buf_flush_disable(), > move the implementation into ufshcd_wb_toggle_flush(). > > Signed-off-by: Bean Huo <beanhuo@micron.com> > --- > drivers/scsi/ufs/ufshcd.c | 69 ++++++++++++++------------------------- > 1 file changed, 24 insertions(+), 45 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index 0998e6103cd7..fb3c98724005 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -244,10 +244,8 @@ static int ufshcd_setup_vreg(struct ufs_hba *hba, bool on); > static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba, > struct ufs_vreg *vreg); > static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag); > -static int ufshcd_wb_buf_flush_enable(struct ufs_hba *hba); > -static int ufshcd_wb_buf_flush_disable(struct ufs_hba *hba); > static int ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set); > -static inline void ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable); > +static inline int ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable); > static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba); > static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba); > > @@ -5398,60 +5396,41 @@ static int ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set) > index, NULL); > } > > -static inline void ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable) > -{ > - if (hba->quirks & UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL) > - return; > - > - if (enable) > - ufshcd_wb_buf_flush_enable(hba); > - else > - ufshcd_wb_buf_flush_disable(hba); > - > -} > - > -static int ufshcd_wb_buf_flush_enable(struct ufs_hba *hba) > +static inline int ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable) > { > int ret; > u8 index; > + enum query_opcode opcode; > > - if (!ufshcd_is_wb_allowed(hba) || hba->dev_info.wb_buf_flush_enabled) > + if (hba->quirks & UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL) > return 0; > > - index = ufshcd_wb_get_query_index(hba); > - ret = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_SET_FLAG, > - QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, > - index, NULL); > - if (ret) > - dev_err(hba->dev, "%s WB - buf flush enable failed %d\n", > - __func__, ret); > - else > - hba->dev_info.wb_buf_flush_enabled = true; > - > - dev_dbg(hba->dev, "WB - Flush enabled: %d\n", ret); > - return ret; > -} > - > -static int ufshcd_wb_buf_flush_disable(struct ufs_hba *hba) > -{ > - int ret; > - u8 index; > - > - if (!ufshcd_is_wb_allowed(hba) || !hba->dev_info.wb_buf_flush_enabled) > + if (!ufshcd_is_wb_allowed(hba) || > + hba->dev_info.wb_buf_flush_enabled == enable) > return 0; > > + if (enable) > + opcode = UPIU_QUERY_OPCODE_SET_FLAG; > + else > + opcode = UPIU_QUERY_OPCODE_CLEAR_FLAG; > + > index = ufshcd_wb_get_query_index(hba); > - ret = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_CLEAR_FLAG, > - QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, > - index, NULL); > + ret = ufshcd_query_flag_retry(hba, opcode, > + QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, index, > + NULL); > if (ret) { > - dev_warn(hba->dev, "%s: WB - buf flush disable failed %d\n", > - __func__, ret); > - } else { > - hba->dev_info.wb_buf_flush_enabled = false; > - dev_dbg(hba->dev, "WB - Flush disabled: %d\n", ret); > + dev_err(hba->dev, "%s WB-Buf Flush %s failed %d\n", __func__, > + enable ? "enable" : "disable", ret); > + goto out; > } > > + if (enable) > + hba->dev_info.wb_buf_flush_enabled = true; > + else > + hba->dev_info.wb_buf_flush_enabled = false; Perhaps this could be simpler as below? hba->dev_info.wb_buf_flush_enabled = enable; Thanks, Stanley Chu > + > + dev_dbg(hba->dev, "WB-Buf Flush %s\n", enable ? "enabled" : "disabled"); > +out: > return ret; > } >
On Tue, 2020-12-15 at 17:07 +0800, Stanley Chu wrote: > > if (ret) { > > - dev_warn(hba->dev, "%s: WB - buf flush disable failed > > %d\n", > > - __func__, ret); > > - } else { > > - hba->dev_info.wb_buf_flush_enabled = false; > > - dev_dbg(hba->dev, "WB - Flush disabled: %d\n", ret); > > + dev_err(hba->dev, "%s WB-Buf Flush %s failed %d\n", > > __func__, > > + enable ? "enable" : "disable", ret); > > + goto out; > > } > > > > + if (enable) > > + hba->dev_info.wb_buf_flush_enabled = true; > > + else > > + hba->dev_info.wb_buf_flush_enabled = false; > > Perhaps this could be simpler as below? > > hba->dev_info.wb_buf_flush_enabled = enable; Thanks, will be changed in next version. Bean
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 0998e6103cd7..fb3c98724005 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -244,10 +244,8 @@ static int ufshcd_setup_vreg(struct ufs_hba *hba, bool on); static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba, struct ufs_vreg *vreg); static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag); -static int ufshcd_wb_buf_flush_enable(struct ufs_hba *hba); -static int ufshcd_wb_buf_flush_disable(struct ufs_hba *hba); static int ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set); -static inline void ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable); +static inline int ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable); static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba); static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba); @@ -5398,60 +5396,41 @@ static int ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set) index, NULL); } -static inline void ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable) -{ - if (hba->quirks & UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL) - return; - - if (enable) - ufshcd_wb_buf_flush_enable(hba); - else - ufshcd_wb_buf_flush_disable(hba); - -} - -static int ufshcd_wb_buf_flush_enable(struct ufs_hba *hba) +static inline int ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable) { int ret; u8 index; + enum query_opcode opcode; - if (!ufshcd_is_wb_allowed(hba) || hba->dev_info.wb_buf_flush_enabled) + if (hba->quirks & UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL) return 0; - index = ufshcd_wb_get_query_index(hba); - ret = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_SET_FLAG, - QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, - index, NULL); - if (ret) - dev_err(hba->dev, "%s WB - buf flush enable failed %d\n", - __func__, ret); - else - hba->dev_info.wb_buf_flush_enabled = true; - - dev_dbg(hba->dev, "WB - Flush enabled: %d\n", ret); - return ret; -} - -static int ufshcd_wb_buf_flush_disable(struct ufs_hba *hba) -{ - int ret; - u8 index; - - if (!ufshcd_is_wb_allowed(hba) || !hba->dev_info.wb_buf_flush_enabled) + if (!ufshcd_is_wb_allowed(hba) || + hba->dev_info.wb_buf_flush_enabled == enable) return 0; + if (enable) + opcode = UPIU_QUERY_OPCODE_SET_FLAG; + else + opcode = UPIU_QUERY_OPCODE_CLEAR_FLAG; + index = ufshcd_wb_get_query_index(hba); - ret = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_CLEAR_FLAG, - QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, - index, NULL); + ret = ufshcd_query_flag_retry(hba, opcode, + QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, index, + NULL); if (ret) { - dev_warn(hba->dev, "%s: WB - buf flush disable failed %d\n", - __func__, ret); - } else { - hba->dev_info.wb_buf_flush_enabled = false; - dev_dbg(hba->dev, "WB - Flush disabled: %d\n", ret); + dev_err(hba->dev, "%s WB-Buf Flush %s failed %d\n", __func__, + enable ? "enable" : "disable", ret); + goto out; } + if (enable) + hba->dev_info.wb_buf_flush_enabled = true; + else + hba->dev_info.wb_buf_flush_enabled = false; + + dev_dbg(hba->dev, "WB-Buf Flush %s\n", enable ? "enabled" : "disabled"); +out: return ret; }