lkml.org 
[lkml]   [2020]   [Mar]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [<RFC PATCH v1> 1/2] scsi: ufs: add write booster feature support
From
Date
On 2/25/2020 4:50 AM, Avri Altman wrote:
>> +/*
>> + * ufshcd_get_wb_alloc_units - returns
>> "dLUNumWriteBoosterBufferAllocUnits"
>> + * @hba: per-adapter instance
>> + * @lun: UFS device lun id
>> + * @d_lun_wbb_au: pointer to buffer to hold the LU's alloc units info
>> + *
>> + * Returns 0 in case of success and d_lun_wbb_au would be returned
>> + * Returns -ENOTSUPP if reading d_lun_wbb_au is not supported.
>> + * Returns -EINVAL in case of invalid parameters passed to this function.
>> + */
>> +static int ufshcd_get_wb_alloc_units(struct ufs_hba *hba,
>> + u8 lun,
>> + u8 *d_lun_wbb_au)
>> +{
>> + int ret;
>> +
>> + if (!d_lun_wbb_au)
>> + ret = -EINVAL;
>> +
>> + /* WB can be supported only from LU0..LU7 */
>> + else if (lun >= UFS_UPIU_MAX_GENERAL_LUN)
>> + ret = -ENOTSUPP;
>> + else
>> + ret = ufshcd_read_unit_desc_param(hba,
>> + lun,
>> + UNIT_DESC_PARAM_WB_BUF_ALLOC_UNITS,
>> + d_lun_wbb_au,
>> + sizeof(*d_lun_wbb_au));
> You are reading here a single byte, instead of 4
>
>> + return ret;
>> +}
>> +
>> /**
>> * ufshcd_get_lu_power_on_wp_status - get LU's power on write protect
>> * status
>> @@ -5194,6 +5267,165 @@ static void
>> ufshcd_bkops_exception_event_handler(struct ufs_hba *hba)
>> __func__, err);
>> }
>>
>> +static bool ufshcd_wb_sup(struct ufs_hba *hba)
>> +{
>> + return ((hba->dev_info.d_ext_ufs_feature_sup &
>> + UFS_DEV_WRITE_BOOSTER_SUP) &&
> Don't you want to have a vendor cap as well,
> to allow the platform vendor to control this feature?
I presume each platform vendor would provide a provisioning
script/method which would configure the WB properties.
It can be controlled through that.
>
>> + (hba->dev_info.b_wb_buffer_type
>> + || hba->dev_info.wb_config_lun));
>> +}
>> +
>> +
>
>
>
>> +static bool ufshcd_wb_is_buf_flush_needed(struct ufs_hba *hba)
>> +{
>> + int ret;
>> + u32 cur_buf, status, avail_buf;
>> +
>> + if (!ufshcd_wb_sup(hba))
>> + return false;
>> +
>> + ret = ufshcd_query_attr_retry(hba,
>> UPIU_QUERY_OPCODE_READ_ATTR,
>> + QUERY_ATTR_IDN_AVAIL_WB_BUFF_SIZE,
>> + 0, 0, &avail_buf);
>> + if (ret) {
>> + dev_warn(hba->dev, "%s dAvailableWriteBoosterBufferSize read
>> failed %d\n",
>> + __func__, ret);
>> + return false;
>> + }
>> +
>> + ret = ufshcd_vops_get_user_cap_mode(hba);
>> + if (ret <= 0) {
>> + dev_dbg(hba->dev, "Get user-cap reduction mode: failed: %d\n",
>> + ret);
>> + /* Most commonly used */
>> + ret = UFS_WB_BUFF_PRESERVE_USER_SPACE;
>> + }
>> +
>> + hba->dev_info.keep_vcc_on = false;
>> + if (ret == UFS_WB_BUFF_USER_SPACE_RED_EN) {
>> + if (avail_buf <= UFS_WB_10_PERCENT_BUF_REMAIN) {
>> + hba->dev_info.keep_vcc_on = true;
>> + return true;
>> + }
>> + return false;
>> + } else if (ret == UFS_WB_BUFF_PRESERVE_USER_SPACE) {
>> + ret = ufshcd_query_attr_retry(hba,
>> UPIU_QUERY_OPCODE_READ_ATTR,
>> + QUERY_ATTR_IDN_CURR_WB_BUFF_SIZE,
>> + 0, 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;
>> + }
>> +
>> + ret = ufshcd_get_ee_status(hba, &status);
>> + if (ret) {
>> + dev_err(hba->dev, "%s: failed to get exception status %d\n",
>> + __func__, ret);
>> + if (avail_buf < UFS_WB_40_PERCENT_BUF_REMAIN) {
>> + hba->dev_info.keep_vcc_on = true;
>> + return true;
>> + }
>> + return false;
>> + }
>> +
>> + status &= hba->ee_ctrl_mask;
>> +
>> + if ((status & MASK_EE_URGENT_BKOPS) ||
> So you are getting the status, but not the bkops level.
> And what about WRITEBOOSTER_EVENT_EN? After all it was invented specifically for WB...
Yeah - WB exception event is not supported in this series. I will push
another change to support that.
With WB enabled, any BKOPS exception level warrants the vcc to be on.
This is to minimize performance impact. Please correct this
understanding from device perspective.

>
>> + (avail_buf < UFS_WB_40_PERCENT_BUF_REMAIN)) {
>> + hba->dev_info.keep_vcc_on = true;
>> + return true;
>> + }
>> + }
>> + return false;
>> +}
>
> Thanks,
> Avri
>


--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
Linux Foundation Collaborative Project

\
 
 \ /
  Last update: 2020-03-17 00:53    [W:0.160 / U:2.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site