| Message ID | 20230609030357.64360-1-luhongfei@vivo.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] scsi: ufs: wb: Add explicit flush_threshold sysfs attribute | expand |
> > There are three flags that control Write Booster Feature: > > 1. WB ON/OFF > 2. WB Hibern Flush ON/OFF (implicitly) > 3. WB Flush ON/OFF (explicit) > > In the case of "Hibern Flush", one of the conditions for flush WB buffer is that > avail_wb_buff < wb_flush_threshold. > > As we know, different users have different requirements for power > consumption and performance. Therefore, we need the ability to manually > set wb_flush_threshold, so that users can easily and flexibly adjust the > wb_flush_threshold value, thereby achieving a balance between power > consumption and performance. > > So the sysfs attribute that controls this is necessary. > > The meaning of wb_flush_threshold is the percentage of WB's total size, such > as 1 representing 10%, 2 representing 20%, and so on. > > Signed-off-by: Lu Hongfei <luhongfei@vivo.com> > --- > The modifications made compared to the previous version are as follows: > 1. Using 'wb_flush_threshold == 0' insted of 'wb_flush_threshold <= 0'. > > drivers/ufs/core/ufs-sysfs.c | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) You are missing sysfs documentation. > > diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c index > cdf3d5f2b77b..347207f4e8ee > --- a/drivers/ufs/core/ufs-sysfs.c > +++ b/drivers/ufs/core/ufs-sysfs.c > @@ -298,6 +298,37 @@ static ssize_t enable_wb_buf_flush_store(struct > device *dev, > return res < 0 ? res : count; > } > > +static ssize_t wb_flush_threshold_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) { > + struct ufs_hba *hba = dev_get_drvdata(dev); > + > + return sysfs_emit(buf, "%u\n", hba->vps->wb_flush_threshold); } > + > +static ssize_t wb_flush_threshold_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct ufs_hba *hba = dev_get_drvdata(dev); > + unsigned int wb_flush_threshold; > + > + if (kstrtouint(buf, 0, &wb_flush_threshold)) > + return -EINVAL; Maybe also make note if wb not supported: if (!ufshcd_is_wb_allowed(hba) { dev_info(dev, "It is not allowed to configure WB buf flushing!\n"); return -EOPNOTSUPP; } Thanks, Avri > + > + /* The range of values for wb_flush_threshold is (0,10] */ > + if (wb_flush_threshold > UFS_WB_BUF_REMAIN_PERCENT(100) || > + wb_flush_threshold == 0) { > + dev_err(dev, "The value of wb_flush_threshold is invalid!\n"); > + return -EINVAL; > + } > + > + hba->vps->wb_flush_threshold = wb_flush_threshold; > + > + return count; > +} > + > static DEVICE_ATTR_RW(rpm_lvl); > static DEVICE_ATTR_RO(rpm_target_dev_state); > static DEVICE_ATTR_RO(rpm_target_link_state); > @@ -307,6 +338,7 @@ static DEVICE_ATTR_RO(spm_target_link_state); > static DEVICE_ATTR_RW(auto_hibern8); > static DEVICE_ATTR_RW(wb_on); > static DEVICE_ATTR_RW(enable_wb_buf_flush); > +static DEVICE_ATTR_RW(wb_flush_threshold); > > static struct attribute *ufs_sysfs_ufshcd_attrs[] = { > &dev_attr_rpm_lvl.attr, > @@ -318,6 +350,7 @@ static struct attribute *ufs_sysfs_ufshcd_attrs[] = { > &dev_attr_auto_hibern8.attr, > &dev_attr_wb_on.attr, > &dev_attr_enable_wb_buf_flush.attr, > + &dev_attr_wb_flush_threshold.attr, > NULL > }; > > -- > 2.39.0
> > > > There are three flags that control Write Booster Feature: > > > > 1. WB ON/OFF > > 2. WB Hibern Flush ON/OFF (implicitly) > > 3. WB Flush ON/OFF (explicit) > > > > In the case of "Hibern Flush", one of the conditions for flush WB > > buffer is that avail_wb_buff < wb_flush_threshold. > > > > As we know, different users have different requirements for power > > consumption and performance. Therefore, we need the ability to > > manually set wb_flush_threshold, so that users can easily and flexibly > > adjust the wb_flush_threshold value, thereby achieving a balance > > between power consumption and performance. > > > > So the sysfs attribute that controls this is necessary. > > > > The meaning of wb_flush_threshold is the percentage of WB's total > > size, such as 1 representing 10%, 2 representing 20%, and so on. > > > > Signed-off-by: Lu Hongfei <luhongfei@vivo.com> > > --- > > The modifications made compared to the previous version are as follows: > > 1. Using 'wb_flush_threshold == 0' insted of 'wb_flush_threshold <= 0'. > > > > drivers/ufs/core/ufs-sysfs.c | 33 +++++++++++++++++++++++++++++++++ > > 1 file changed, 33 insertions(+) > > You are missing sysfs documentation. > > > > diff --git a/drivers/ufs/core/ufs-sysfs.c > > b/drivers/ufs/core/ufs-sysfs.c index cdf3d5f2b77b..347207f4e8ee > > --- a/drivers/ufs/core/ufs-sysfs.c > > +++ b/drivers/ufs/core/ufs-sysfs.c > > @@ -298,6 +298,37 @@ static ssize_t enable_wb_buf_flush_store(struct > > device *dev, > > return res < 0 ? res : count; > > } > > > > +static ssize_t wb_flush_threshold_show(struct device *dev, > > + struct device_attribute *attr, > > + char *buf) { > > + struct ufs_hba *hba = dev_get_drvdata(dev); > > + > > + return sysfs_emit(buf, "%u\n", hba->vps->wb_flush_threshold); > > + } > > + > > +static ssize_t wb_flush_threshold_store(struct device *dev, > > + struct device_attribute *attr, > > + const char *buf, size_t > > +count) { > > + struct ufs_hba *hba = dev_get_drvdata(dev); > > + unsigned int wb_flush_threshold; > > + > > + if (kstrtouint(buf, 0, &wb_flush_threshold)) > > + return -EINVAL; > Maybe also make note if wb not supported: > if (!ufshcd_is_wb_allowed(hba) { > dev_info(dev, "It is not allowed to configure WB buf flushing!\n"); > return -EOPNOTSUPP; > } I just noticed that Bart asked you to leave it out - sorry. Please ignore. Thanks, Avri
diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c index cdf3d5f2b77b..347207f4e8ee --- a/drivers/ufs/core/ufs-sysfs.c +++ b/drivers/ufs/core/ufs-sysfs.c @@ -298,6 +298,37 @@ static ssize_t enable_wb_buf_flush_store(struct device *dev, return res < 0 ? res : count; } +static ssize_t wb_flush_threshold_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct ufs_hba *hba = dev_get_drvdata(dev); + + return sysfs_emit(buf, "%u\n", hba->vps->wb_flush_threshold); +} + +static ssize_t wb_flush_threshold_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct ufs_hba *hba = dev_get_drvdata(dev); + unsigned int wb_flush_threshold; + + if (kstrtouint(buf, 0, &wb_flush_threshold)) + return -EINVAL; + + /* The range of values for wb_flush_threshold is (0,10] */ + if (wb_flush_threshold > UFS_WB_BUF_REMAIN_PERCENT(100) || + wb_flush_threshold == 0) { + dev_err(dev, "The value of wb_flush_threshold is invalid!\n"); + return -EINVAL; + } + + hba->vps->wb_flush_threshold = wb_flush_threshold; + + return count; +} + static DEVICE_ATTR_RW(rpm_lvl); static DEVICE_ATTR_RO(rpm_target_dev_state); static DEVICE_ATTR_RO(rpm_target_link_state); @@ -307,6 +338,7 @@ static DEVICE_ATTR_RO(spm_target_link_state); static DEVICE_ATTR_RW(auto_hibern8); static DEVICE_ATTR_RW(wb_on); static DEVICE_ATTR_RW(enable_wb_buf_flush); +static DEVICE_ATTR_RW(wb_flush_threshold); static struct attribute *ufs_sysfs_ufshcd_attrs[] = { &dev_attr_rpm_lvl.attr, @@ -318,6 +350,7 @@ static struct attribute *ufs_sysfs_ufshcd_attrs[] = { &dev_attr_auto_hibern8.attr, &dev_attr_wb_on.attr, &dev_attr_enable_wb_buf_flush.attr, + &dev_attr_wb_flush_threshold.attr, NULL };
There are three flags that control Write Booster Feature: 1. WB ON/OFF 2. WB Hibern Flush ON/OFF (implicitly) 3. WB Flush ON/OFF (explicit) In the case of "Hibern Flush", one of the conditions for flush WB buffer is that avail_wb_buff < wb_flush_threshold. As we know, different users have different requirements for power consumption and performance. Therefore, we need the ability to manually set wb_flush_threshold, so that users can easily and flexibly adjust the wb_flush_threshold value, thereby achieving a balance between power consumption and performance. So the sysfs attribute that controls this is necessary. The meaning of wb_flush_threshold is the percentage of WB's total size, such as 1 representing 10%, 2 representing 20%, and so on. Signed-off-by: Lu Hongfei <luhongfei@vivo.com> --- The modifications made compared to the previous version are as follows: 1. Using 'wb_flush_threshold == 0' insted of 'wb_flush_threshold <= 0'. drivers/ufs/core/ufs-sysfs.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)