RE: [PATCH v1 1/1] scsi: ufs: core: Removed ufshcd_wb_presrv_usrspc_keep_vcc_on()
From: Avri Altman
Date: Fri Mar 28 2025 - 04:29:54 EST
> 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@xxxxxxxxxxx>
> ---
> 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