diff mbox series

[v6,1/6] scsi: ufs: Add "wb_on" sysfs node to control WB on/off

Message ID 20210118201039.2398-2-huobean@gmail.com (mailing list archive)
State Superseded
Headers show
Series Several changes for UFS WriteBooster | expand

Commit Message

Bean Huo Jan. 18, 2021, 8:10 p.m. UTC 
From: Bean Huo <beanhuo@micron.com>

Currently UFS WriteBooster driver uses clock scaling up/down to set
WB on/off, for the platform which doesn't support UFSHCD_CAP_CLK_SCALING,
WB will be always on. Provide a sysfs attribute to enable/disable WB
during runtime. Write 1/0 to "wb_on" sysfs node to enable/disable UFS WB.

Reviewed-by: Avri Altman <avri.altman@wdc.com>
Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 drivers/scsi/ufs/ufs-sysfs.c | 46 ++++++++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd.c    |  3 +--
 drivers/scsi/ufs/ufshcd.h    |  2 ++
 3 files changed, 49 insertions(+), 2 deletions(-)

Comments

Adrian Hunter Jan. 19, 2021, 7:01 a.m. UTC | #1 
On 18/01/21 10:10 pm, Bean Huo wrote:
> From: Bean Huo <beanhuo@micron.com>
> 
> Currently UFS WriteBooster driver uses clock scaling up/down to set
> WB on/off, for the platform which doesn't support UFSHCD_CAP_CLK_SCALING,
> WB will be always on. Provide a sysfs attribute to enable/disable WB
> during runtime. Write 1/0 to "wb_on" sysfs node to enable/disable UFS WB.

Is it so, that after a full reset, WB is always enabled again?  Is that
intended?
Bean Huo Jan. 19, 2021, 9:33 a.m. UTC | #2 
On Tue, 2021-01-19 at 09:01 +0200, Adrian Hunter wrote:
> On 18/01/21 10:10 pm, Bean Huo wrote:
> > From: Bean Huo <beanhuo@micron.com>
> > 
> > Currently UFS WriteBooster driver uses clock scaling up/down to set
> > WB on/off, for the platform which doesn't support
> > UFSHCD_CAP_CLK_SCALING,
> > WB will be always on. Provide a sysfs attribute to enable/disable
> > WB
> > during runtime. Write 1/0 to "wb_on" sysfs node to enable/disable
> > UFS WB.
> 
> Is it so, that after a full reset, WB is always enabled again?  Is
> that
> intended?

Hello Adrian
Good questions. yes, after a full reset, the UFS device side by default
is wb disabled,  then WB will be always enabled agaion in
ufshcd_wb_config(hba). but, for the platform which
supports UFSHCD_CAP_CLK_SCALING, wb will be disabled again while clk
scaling down and enabled while clk scaling up.

Regarding the last question, I think OEM wants to do that. maybe they
suppose there will be a lot of writing after reset?? From the UFS
device's point of view, the control of WB is up to the user.

Thanks,
Bean
Adrian Hunter Jan. 19, 2021, 10 a.m. UTC | #3 
On 19/01/21 11:33 am, Bean Huo wrote:
> On Tue, 2021-01-19 at 09:01 +0200, Adrian Hunter wrote:
>> On 18/01/21 10:10 pm, Bean Huo wrote:
>>> From: Bean Huo <beanhuo@micron.com>
>>>
>>> Currently UFS WriteBooster driver uses clock scaling up/down to set
>>> WB on/off, for the platform which doesn't support
>>> UFSHCD_CAP_CLK_SCALING,
>>> WB will be always on. Provide a sysfs attribute to enable/disable
>>> WB
>>> during runtime. Write 1/0 to "wb_on" sysfs node to enable/disable
>>> UFS WB.
>>
>> Is it so, that after a full reset, WB is always enabled again?  Is
>> that
>> intended?
> 
> Hello Adrian
> Good questions. yes, after a full reset, the UFS device side by default
> is wb disabled,  then WB will be always enabled agaion in
> ufshcd_wb_config(hba). but, for the platform which
> supports UFSHCD_CAP_CLK_SCALING, wb will be disabled again while clk
> scaling down and enabled while clk scaling up.
> 
> Regarding the last question, I think OEM wants to do that. maybe they
> suppose there will be a lot of writing after reset?? From the UFS
> device's point of view, the control of WB is up to the user.

If it is by design enabled after reset, then perhaps it should be mentioned
in the sysfs documentation.
Bean Huo Jan. 19, 2021, 4:31 p.m. UTC | #4 
On Tue, 2021-01-19 at 12:00 +0200, Adrian Hunter wrote:
> > > Is it so, that after a full reset, WB is always enabled again? 
> > > Is
> > > that
> > > intended?
> > 
> > Hello Adrian
> > Good questions. yes, after a full reset, the UFS device side by
> > default
> > is wb disabled,  then WB will be always enabled agaion in
> > ufshcd_wb_config(hba). but, for the platform which
> > supports UFSHCD_CAP_CLK_SCALING, wb will be disabled again while
> > clk
> > scaling down and enabled while clk scaling up.
> > 
> > Regarding the last question, I think OEM wants to do that. maybe
> > they
> > suppose there will be a lot of writing after reset?? From the UFS
> > device's point of view, the control of WB is up to the user.
> 
> If it is by design enabled after reset, then perhaps it should be
> mentioned
> in the sysfs documentation.

ok, will add it in the next version.

thanks,
Bean
diff mbox series

Patch

diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
index c092f249d6f9..76db8593ca09 100644
--- a/drivers/scsi/ufs/ufs-sysfs.c
+++ b/drivers/scsi/ufs/ufs-sysfs.c
@@ -209,6 +209,50 @@  static ssize_t auto_hibern8_store(struct device *dev,
 	return ret ? ret : count;
 }
 
+static ssize_t wb_on_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->wb_enabled);
+}
+
+static ssize_t wb_on_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_enable;
+	ssize_t res;
+
+	if (!ufshcd_is_wb_allowed(hba) || ufshcd_is_clkscaling_supported(hba)) {
+		/*
+		 * If the platform supports UFSHCD_CAP_CLK_SCALING, turn WB
+		 * on/off will be done while clock scaling up/down.
+		 */
+		dev_warn(dev, "To control WB through wb_on is not allowed!\n");
+		return -EOPNOTSUPP;
+	}
+
+	if (kstrtouint(buf, 0, &wb_enable))
+		return -EINVAL;
+
+	if (wb_enable != 0 && wb_enable != 1)
+		return -EINVAL;
+
+	down(&hba->host_sem);
+	if (!ufshcd_is_user_access_allowed(hba)) {
+		res = -EBUSY;
+		goto out;
+	}
+
+	pm_runtime_get_sync(hba->dev);
+	res = ufshcd_wb_ctrl(hba, wb_enable);
+	pm_runtime_put_sync(hba->dev);
+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);
@@ -216,6 +260,7 @@  static DEVICE_ATTR_RW(spm_lvl);
 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 struct attribute *ufs_sysfs_ufshcd_attrs[] = {
 	&dev_attr_rpm_lvl.attr,
@@ -225,6 +270,7 @@  static struct attribute *ufs_sysfs_ufshcd_attrs[] = {
 	&dev_attr_spm_target_dev_state.attr,
 	&dev_attr_spm_target_link_state.attr,
 	&dev_attr_auto_hibern8.attr,
+	&dev_attr_wb_on.attr,
 	NULL
 };
 
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 5c6ee9394af3..3f2b495b36ee 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -249,7 +249,6 @@  static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
 static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag);
 static int ufshcd_wb_buf_flush_enable(struct ufs_hba *hba);
 static int ufshcd_wb_buf_flush_disable(struct ufs_hba *hba);
-static int ufshcd_wb_ctrl(struct ufs_hba *hba, bool enable);
 static int ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set);
 static inline void ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable);
 static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba);
@@ -5413,7 +5412,7 @@  static void ufshcd_bkops_exception_event_handler(struct ufs_hba *hba)
 				__func__, err);
 }
 
-static int ufshcd_wb_ctrl(struct ufs_hba *hba, bool enable)
+int ufshcd_wb_ctrl(struct ufs_hba *hba, bool enable)
 {
 	int ret;
 	u8 index;
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 5bbe4715d4e9..ac0f03f69e42 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -1089,6 +1089,8 @@  int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba,
 			     u8 *desc_buff, int *buff_len,
 			     enum query_opcode desc_op);
 
+int ufshcd_wb_ctrl(struct ufs_hba *hba, bool enable);
+
 /* Wrapper functions for safely calling variant operations */
 static inline const char *ufshcd_get_var_name(struct ufs_hba *hba)
 {