| Message ID | 9ff613809e88496b5802a2d45984d2a8dddf92dd.1743057420.git.quic_nguyenb@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/1] scsi: ufs: core: Removed ufshcd_wb_presrv_usrspc_keep_vcc_on() | expand |
> Merge the ufshcd_wb_presrv_usrspc_keep_vcc_on() function into > ufshcd_wb_need_flush(). The "_keep_vcc_on" part of the function name is > misleading. The function definition may be deviated from its original intention. > This is a small function only invoked by the ufshcd_wb_need_flush(). To > improve the readability, remove this function and merge its content into its > caller. There is no change to the functionality. > > Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com> > --- > drivers/ufs/core/ufshcd.c | 53 +++++++++++++++++++-------------------------- > -- > 1 file changed, 21 insertions(+), 32 deletions(-) > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index > 4e1e214..b9272b1 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -6083,32 +6083,6 @@ int ufshcd_wb_toggle_buf_flush(struct ufs_hba > *hba, bool enable) > return ret; > } > > -static bool ufshcd_wb_presrv_usrspc_keep_vcc_on(struct ufs_hba *hba, > - u32 avail_buf) Maybe just change the function name to better describe what it does, e.g. ufshcd_wb_exceed_threshold ? Thanks, Avri > -{ > - u32 cur_buf; > - int ret; > - u8 index; > - > - index = ufshcd_wb_get_query_index(hba); > - ret = ufshcd_query_attr_retry(hba, > UPIU_QUERY_OPCODE_READ_ATTR, > - > QUERY_ATTR_IDN_CURR_WB_BUFF_SIZE, > - index, 0, &cur_buf); > - if (ret) { > - dev_err(hba->dev, "%s: dCurWriteBoosterBufferSize read > failed %d\n", > - __func__, ret); > - return false; > - } > - > - if (!cur_buf) { > - dev_info(hba->dev, "dCurWBBuf: %d WB disabled until free- > space is available\n", > - cur_buf); > - return false; > - } > - /* Let it continue to flush when available buffer exceeds threshold */ > - return avail_buf < hba->vps->wb_flush_threshold; > -} > - > static void ufshcd_wb_force_disable(struct ufs_hba *hba) { > if (ufshcd_is_wb_buf_flush_allowed(hba)) > @@ -6152,9 +6126,9 @@ static bool > ufshcd_is_wb_buf_lifetime_available(struct ufs_hba *hba) > > static bool ufshcd_wb_need_flush(struct ufs_hba *hba) { > - int ret; > - u32 avail_buf; > + u32 avail_buf, cur_buf; > u8 index; > + int ret; > > if (!ufshcd_is_wb_allowed(hba)) > return false; > @@ -6165,15 +6139,13 @@ static bool ufshcd_wb_need_flush(struct > ufs_hba *hba) > } > > /* > - * The ufs device needs the vcc to be ON to flush. > * With user-space reduction enabled, it's enough to enable flush > * by checking only the available buffer. The threshold > * defined here is > 90% full. > * With user-space preserved enabled, the current-buffer > * should be checked too because the wb buffer size can reduce > * when disk tends to be full. This info is provided by current > - * buffer (dCurrentWriteBoosterBufferSize). There's no point in > - * keeping vcc on when current buffer is empty. > + * buffer (dCurrentWriteBoosterBufferSize). > */ > index = ufshcd_wb_get_query_index(hba); > ret = ufshcd_query_attr_retry(hba, > UPIU_QUERY_OPCODE_READ_ATTR, @@ -6188,7 +6160,24 @@ static bool > ufshcd_wb_need_flush(struct ufs_hba *hba) > if (!hba->dev_info.b_presrv_uspc_en) > return avail_buf <= UFS_WB_BUF_REMAIN_PERCENT(10); > > - return ufshcd_wb_presrv_usrspc_keep_vcc_on(hba, avail_buf); > + index = ufshcd_wb_get_query_index(hba); > + ret = ufshcd_query_attr_retry(hba, > UPIU_QUERY_OPCODE_READ_ATTR, > + QUERY_ATTR_IDN_CURR_WB_BUFF_SIZE, > + index, 0, &cur_buf); > + if (ret) { > + dev_err(hba->dev, "%s: dCurWriteBoosterBufferSize read > failed %d\n", > + __func__, ret); > + return false; > + } > + > + if (!cur_buf) { > + dev_info(hba->dev, "dCurWBBuf: %d WB disabled until free- > space is available\n", > + cur_buf); > + return false; > + } > + > + /* Let it continue to flush when available buffer exceeds threshold */ > + return avail_buf < hba->vps->wb_flush_threshold; > } > > static void ufshcd_rpm_dev_flush_recheck_work(struct work_struct *work) > -- > 2.7.4
On 3/28/25 1:29 AM, Avri Altman wrote: > Maybe just change the function name to better describe what it does, > e.g. ufshcd_wb_exceed_threshold ? I'm also in favor of renaming ufshcd_wb_presrv_usrspc_keep_vcc_on() instead of inlining it. Thanks, Bart.
On 3/28/2025 12:33 PM, Bart Van Assche wrote: > On 3/28/25 1:29 AM, Avri Altman wrote: >> Maybe just change the function name to better describe what it does, >> e.g. ufshcd_wb_exceed_threshold ? > > I'm also in favor of renaming ufshcd_wb_presrv_usrspc_keep_vcc_on() > instead of inlining it. > Thank you both. I will update. Thanks, Bao
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 4e1e214..b9272b1 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -6083,32 +6083,6 @@ int ufshcd_wb_toggle_buf_flush(struct ufs_hba *hba, bool enable) return ret; } -static bool ufshcd_wb_presrv_usrspc_keep_vcc_on(struct ufs_hba *hba, - u32 avail_buf) -{ - u32 cur_buf; - int ret; - u8 index; - - index = ufshcd_wb_get_query_index(hba); - ret = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR, - QUERY_ATTR_IDN_CURR_WB_BUFF_SIZE, - index, 0, &cur_buf); - if (ret) { - dev_err(hba->dev, "%s: dCurWriteBoosterBufferSize read failed %d\n", - __func__, ret); - return false; - } - - if (!cur_buf) { - dev_info(hba->dev, "dCurWBBuf: %d WB disabled until free-space is available\n", - cur_buf); - return false; - } - /* Let it continue to flush when available buffer exceeds threshold */ - return avail_buf < hba->vps->wb_flush_threshold; -} - static void ufshcd_wb_force_disable(struct ufs_hba *hba) { if (ufshcd_is_wb_buf_flush_allowed(hba)) @@ -6152,9 +6126,9 @@ static bool ufshcd_is_wb_buf_lifetime_available(struct ufs_hba *hba) static bool ufshcd_wb_need_flush(struct ufs_hba *hba) { - int ret; - u32 avail_buf; + u32 avail_buf, cur_buf; u8 index; + int ret; if (!ufshcd_is_wb_allowed(hba)) return false; @@ -6165,15 +6139,13 @@ static bool ufshcd_wb_need_flush(struct ufs_hba *hba) } /* - * The ufs device needs the vcc to be ON to flush. * With user-space reduction enabled, it's enough to enable flush * by checking only the available buffer. The threshold * defined here is > 90% full. * With user-space preserved enabled, the current-buffer * should be checked too because the wb buffer size can reduce * when disk tends to be full. This info is provided by current - * buffer (dCurrentWriteBoosterBufferSize). There's no point in - * keeping vcc on when current buffer is empty. + * buffer (dCurrentWriteBoosterBufferSize). */ index = ufshcd_wb_get_query_index(hba); ret = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR, @@ -6188,7 +6160,24 @@ static bool ufshcd_wb_need_flush(struct ufs_hba *hba) if (!hba->dev_info.b_presrv_uspc_en) return avail_buf <= UFS_WB_BUF_REMAIN_PERCENT(10); - return ufshcd_wb_presrv_usrspc_keep_vcc_on(hba, avail_buf); + index = ufshcd_wb_get_query_index(hba); + ret = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR, + QUERY_ATTR_IDN_CURR_WB_BUFF_SIZE, + index, 0, &cur_buf); + if (ret) { + dev_err(hba->dev, "%s: dCurWriteBoosterBufferSize read failed %d\n", + __func__, ret); + return false; + } + + if (!cur_buf) { + dev_info(hba->dev, "dCurWBBuf: %d WB disabled until free-space is available\n", + cur_buf); + return false; + } + + /* Let it continue to flush when available buffer exceeds threshold */ + return avail_buf < hba->vps->wb_flush_threshold; } static void ufshcd_rpm_dev_flush_recheck_work(struct work_struct *work)
Merge the ufshcd_wb_presrv_usrspc_keep_vcc_on() function into ufshcd_wb_need_flush(). The "_keep_vcc_on" part of the function name is misleading. The function definition may be deviated from its original intention. This is a small function only invoked by the ufshcd_wb_need_flush(). To improve the readability, remove this function and merge its content into its caller. There is no change to the functionality. Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com> --- drivers/ufs/core/ufshcd.c | 53 +++++++++++++++++++---------------------------- 1 file changed, 21 insertions(+), 32 deletions(-)