2022-07-01 08:31:57

by Jinyoung Choi

[permalink] [raw]
Subject: [PATCH v3 0/2] scsi: ufs: wb: Add sysfs and cleanup

This patch series is to clean up UFS's Write Booster code and adds sysfs
which can control the specific feature of it.

V2:
- modify commit message
- move & modify err messages
- remove unnesscessary debug messages
V3:
- split patch (functional, non-functional)

Jinyoung Choi (2):
scsi: ufs: wb: renaming & cleanups functions
scsi: ufs: wb: Add Manual Flush sysfs

drivers/ufs/core/ufs-sysfs.c | 46 +++++++++++++++++++++++-
drivers/ufs/core/ufshcd.c | 69 +++++++++++++++++-------------------
include/ufs/ufshcd.h | 7 ++++
3 files changed, 84 insertions(+), 38 deletions(-)

--
2.25.1


2022-07-01 08:32:19

by Jinyoung Choi

[permalink] [raw]
Subject: [PATCH v3 2/2] scsi: ufs: wb: Add Manual Flush sysfs

There is the following quirk to bypass "WB Manual Flush" in Write Booster.

- UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL

If this quirk is not set, there is no knob that can control "WB Manual Flush".

There are three flags that control Write Booster Feature.
1. WB ON/OFF
2. WB Hibern Flush ON/OFF
3. WB Flush ON/OFF

The sysfs that controls the WB was implemented. (1)

In the case of "Hibern Flush", it is always good to turn on.
Control may not be required. (2)

Finally, "Manual flush" may be necessary because the Auto-Hibern8 is not
supported in a specific environment.
So the sysfs that controls this is necessary. (3)

Signed-off-by: Jinyoung Choi <[email protected]>
---
drivers/ufs/core/ufs-sysfs.c | 44 ++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)

diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
index 6253606b93b4..b1c51d8df9f4 100644
--- a/drivers/ufs/core/ufs-sysfs.c
+++ b/drivers/ufs/core/ufs-sysfs.c
@@ -254,6 +254,48 @@ static ssize_t wb_on_store(struct device *dev, struct device_attribute *attr,
return res < 0 ? res : count;
}

+static ssize_t wb_buf_flush_en_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct ufs_hba *hba = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%d\n", hba->dev_info.wb_buf_flush_enabled);
+}
+
+static ssize_t wb_buf_flush_en_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_buf_flush_en;
+ ssize_t res;
+
+ if (!ufshcd_is_wb_buf_flush_allowed(hba)) {
+ dev_warn(dev, "It is not allowed to control WB buf flush!\n");
+ return -EOPNOTSUPP;
+ }
+
+ if (kstrtouint(buf, 0, &wb_buf_flush_en))
+ return -EINVAL;
+
+ if (wb_buf_flush_en != 0 && wb_buf_flush_en != 1)
+ return -EINVAL;
+
+ down(&hba->host_sem);
+ if (!ufshcd_is_user_access_allowed(hba)) {
+ res = -EBUSY;
+ goto out;
+ }
+
+ ufshcd_rpm_get_sync(hba);
+ res = ufshcd_wb_toggle_buf_flush(hba, wb_buf_flush_en);
+ ufshcd_rpm_put_sync(hba);
+out:
+ up(&hba->host_sem);
+ return res < 0 ? res : count;
+}
+
static DEVICE_ATTR_RW(rpm_lvl);
static DEVICE_ATTR_RO(rpm_target_dev_state);
static DEVICE_ATTR_RO(rpm_target_link_state);
@@ -262,6 +304,7 @@ static DEVICE_ATTR_RO(spm_target_dev_state);
static DEVICE_ATTR_RO(spm_target_link_state);
static DEVICE_ATTR_RW(auto_hibern8);
static DEVICE_ATTR_RW(wb_on);
+static DEVICE_ATTR_RW(wb_buf_flush_en);

static struct attribute *ufs_sysfs_ufshcd_attrs[] = {
&dev_attr_rpm_lvl.attr,
@@ -272,6 +315,7 @@ static struct attribute *ufs_sysfs_ufshcd_attrs[] = {
&dev_attr_spm_target_link_state.attr,
&dev_attr_auto_hibern8.attr,
&dev_attr_wb_on.attr,
+ &dev_attr_wb_buf_flush_en.attr,
NULL
};

--
2.25.1

2022-07-03 13:58:58

by Avri Altman

[permalink] [raw]
Subject: RE: [PATCH v3 2/2] scsi: ufs: wb: Add Manual Flush sysfs

> There is the following quirk to bypass "WB Manual Flush" in Write Booster.
>
> - UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL
>
> If this quirk is not set, there is no knob that can control "WB Manual Flush".
>
> There are three flags that control Write Booster Feature.
> 1. WB ON/OFF
> 2. WB Hibern Flush ON/OFF
> 3. WB Flush ON/OFF
>
> The sysfs that controls the WB was implemented. (1)
>
> In the case of "Hibern Flush", it is always good to turn on.
> Control may not be required. (2)
>
> Finally, "Manual flush" may be necessary because the Auto-Hibern8 is not
> supported in a specific environment.
> So the sysfs that controls this is necessary. (3)
>
> Signed-off-by: Jinyoung Choi <[email protected]>
Reviewed-by: Avri Altman <[email protected]>

2022-07-07 22:53:23

by Bart Van Assche

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] scsi: ufs: wb: Add Manual Flush sysfs

On 7/1/22 00:48, Jinyoung CHOI wrote:
>

The subject of this email is incomplete. "sysfs" should be changed into
"sysfs attribute".

Additional, the use of the word "manual" in the subject seems weird to
me. I think the term "explicit" from the UFS specification describes the
purpose of the new sysfs attribute better.

Thanks,

Bart.