| Message ID | 20201215230519.15158-2-huobean@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Several changes for UFS WriteBooster | expand |
Hi Bean, On Wed, 2020-12-16 at 00:05 +0100, 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. > > 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 | 41 ++++++++++++++++++++++++++++++++++++ > drivers/scsi/ufs/ufshcd.c | 3 +-- > drivers/scsi/ufs/ufshcd.h | 2 ++ > 3 files changed, 44 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c > index 08e72b7eef6a..f3ca3d6b82c4 100644 > --- a/drivers/scsi/ufs/ufs-sysfs.c > +++ b/drivers/scsi/ufs/ufs-sysfs.c > @@ -189,6 +189,45 @@ static ssize_t auto_hibern8_store(struct device *dev, > return 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_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 (!ufshcd_is_wb_allowed(hba)) > + return -EOPNOTSUPP; > + > + if (kstrtouint(buf, 0, &wb_enable)) > + return -EINVAL; > + > + if (wb_enable != 0 && wb_enable != 1) > + return -EINVAL; > + > + pm_runtime_get_sync(hba->dev); > + res = ufshcd_wb_ctrl(hba, wb_enable); May this operation race with UFS shutdown flow? To be more clear, ufshcd_wb_ctrl() here may be executed after host clock is disabled by shutdown flow? If yes, we need to avoid it. Thanks, Stanley Chu > + pm_runtime_put_sync(hba->dev); > + > + 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); > @@ -196,6 +235,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, > @@ -205,6 +245,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 e221add25a7e..5e1dcf4de67e 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -246,7 +246,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); > @@ -5351,7 +5350,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 9bb5f0ed4124..2a97006a2c93 100644 > --- a/drivers/scsi/ufs/ufshcd.h > +++ b/drivers/scsi/ufs/ufshcd.h > @@ -1068,6 +1068,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) > {
On 2020-12-22 14:08, Stanley Chu wrote: > Hi Bean, > > On Wed, 2020-12-16 at 00:05 +0100, 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. >> >> 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 | 41 >> ++++++++++++++++++++++++++++++++++++ >> drivers/scsi/ufs/ufshcd.c | 3 +-- >> drivers/scsi/ufs/ufshcd.h | 2 ++ >> 3 files changed, 44 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/scsi/ufs/ufs-sysfs.c >> b/drivers/scsi/ufs/ufs-sysfs.c >> index 08e72b7eef6a..f3ca3d6b82c4 100644 >> --- a/drivers/scsi/ufs/ufs-sysfs.c >> +++ b/drivers/scsi/ufs/ufs-sysfs.c >> @@ -189,6 +189,45 @@ static ssize_t auto_hibern8_store(struct device >> *dev, >> return 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_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 (!ufshcd_is_wb_allowed(hba)) >> + return -EOPNOTSUPP; >> + >> + if (kstrtouint(buf, 0, &wb_enable)) >> + return -EINVAL; >> + >> + if (wb_enable != 0 && wb_enable != 1) >> + return -EINVAL; >> + >> + pm_runtime_get_sync(hba->dev); >> + res = ufshcd_wb_ctrl(hba, wb_enable); > > May this operation race with UFS shutdown flow? > > To be more clear, ufshcd_wb_ctrl() here may be executed after host > clock > is disabled by shutdown flow? > > If yes, we need to avoid it. I have the same doubt - can user still access sysfs nodes after system starts to run shutdown routines? If yes, then we need to remove all UFS sysfs nodes in ufshcd_shutdown(). Thanks, Can Guo. > > Thanks, > Stanley Chu > >> + pm_runtime_put_sync(hba->dev); >> + >> + 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); >> @@ -196,6 +235,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, >> @@ -205,6 +245,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 e221add25a7e..5e1dcf4de67e 100644 >> --- a/drivers/scsi/ufs/ufshcd.c >> +++ b/drivers/scsi/ufs/ufshcd.c >> @@ -246,7 +246,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); >> @@ -5351,7 +5350,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 9bb5f0ed4124..2a97006a2c93 100644 >> --- a/drivers/scsi/ufs/ufshcd.h >> +++ b/drivers/scsi/ufs/ufshcd.h >> @@ -1068,6 +1068,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) >> {
On Tue, 2020-12-22 at 14:08 +0800, Stanley Chu wrote: > > + if (kstrtouint(buf, 0, &wb_enable)) > > + return -EINVAL; > > + > > + if (wb_enable != 0 && wb_enable != 1) > > + return -EINVAL; > > + > > + pm_runtime_get_sync(hba->dev); > > + res = ufshcd_wb_ctrl(hba, wb_enable); > > May this operation race with UFS shutdown flow? > > To be more clear, ufshcd_wb_ctrl() here may be executed after host > clock > is disabled by shutdown flow? > > If yes, we need to avoid it. > > Thanks, > Stanley Chu Yes, it is quite possible, but who will mess up this on purpose? ok, to counterbalance our concerns, I can add checkup: /* UFS device & link must be active before we change WB status */ if (!ufshcd_is_ufs_dev_active(hba) || !ufshcd_is_link_active(hba)) return -EINVAL; how do you think about? Bean
On Tue, 2020-12-22 at 14:12 +0800, Can Guo wrote: > > > + return -EOPNOTSUPP; > > > + > > > + if (kstrtouint(buf, 0, &wb_enable)) > > > + return -EINVAL; > > > + > > > + if (wb_enable != 0 && wb_enable != 1) > > > + return -EINVAL; > > > + > > > + pm_runtime_get_sync(hba->dev); > > > + res = ufshcd_wb_ctrl(hba, wb_enable); > > > > May this operation race with UFS shutdown flow? > > > > To be more clear, ufshcd_wb_ctrl() here may be executed after host > > clock > > is disabled by shutdown flow? > > > > If yes, we need to avoid it. > > I have the same doubt - can user still access sysfs nodes after > system > starts to run shutdown routines? If yes, then we need to remove all > UFS > sysfs nodes in ufshcd_shutdown(). > No, we shouldn't do in this way, user space complains this. I think the nodes in the sysfs can be shileded write, but the nodes shouldn't be flash of its presence frequently. Thanks, Bean > Thanks, > > Can Guo.
On Tue, 2020-12-22 at 21:50 +0100, Bean Huo wrote: > > > > May this operation race with UFS shutdown flow? > > > > To be more clear, ufshcd_wb_ctrl() here may be executed after host > > clock > > is disabled by shutdown flow? > > > > If yes, we need to avoid it. > > > > Thanks, > > Stanley Chu > > Yes, it is quite possible, but who will mess up this on purpose? > > ok, to counterbalance our concerns, I can add checkup: > > > /* UFS device & link must be active before we change WB status */ > if (!ufshcd_is_ufs_dev_active(hba) || !ufshcd_is_link_active(hba)) > return -EINVAL; > > > how do you think about? > > Bean seems there are only auto_hibern8 and wb will issue command to UFS device. if you think auto_hibern8 also needs this protection. I will add in the next version patch, otherwise, just add this checkup in the WB. Look forward to your feedback! Thanks, Bean
On Tue, 2020-12-22 at 21:57 +0100, Bean Huo wrote: > > > May this operation race with UFS shutdown flow? > > > > > > To be more clear, ufshcd_wb_ctrl() here may be executed after > > > host > > > clock > > > is disabled by shutdown flow? > > > > > > If yes, we need to avoid it. > > > > I have the same doubt - can user still access sysfs nodes after > > system > > starts to run shutdown routines? If yes, then we need to remove all > > UFS > > sysfs nodes in ufshcd_shutdown(). > > > > No, we shouldn't do in this way, user space complains this. I think > the nodes in the sysfs can be shileded write, but the nodes shouldn't > be flash of its presence frequently. > > Thanks, > Bean > > > > Thanks, > > > > Can Guo. Hi Can Got your point, you don't want user space to interrupt UFS by sysyfs if UFS is in power down mode. if this is true, insteading of removing all sysfs node in ufshcd_shutdown, maybe just add this checkup before accessing UFS device descriptors/flag/attributes/LU: for example, for the device descriptor: diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs- sysfs.c index b3bf7fca00e5..881fe1c24a9f 100644 --- a/drivers/scsi/ufs/ufs-sysfs.c +++ b/drivers/scsi/ufs/ufs-sysfs.c @@ -262,6 +262,9 @@ static ssize_t ufs_sysfs_read_desc_param(struct ufs_hba *hba, u8 desc_buf[8] = {0}; int ret; + if (!ufshcd_is_ufs_dev_active(hba) || !ufshcd_is_link_active(hba)) + return -EACCES; + Bean
On 2020-12-23 06:11, Bean Huo wrote: > On Tue, 2020-12-22 at 21:57 +0100, Bean Huo wrote: >> > > May this operation race with UFS shutdown flow? >> > > >> > > To be more clear, ufshcd_wb_ctrl() here may be executed after >> > > host >> > > clock >> > > is disabled by shutdown flow? >> > > >> > > If yes, we need to avoid it. >> > >> > I have the same doubt - can user still access sysfs nodes after >> > system >> > starts to run shutdown routines? If yes, then we need to remove all >> > UFS >> > sysfs nodes in ufshcd_shutdown(). >> > >> >> No, we shouldn't do in this way, user space complains this. I think >> the nodes in the sysfs can be shileded write, but the nodes shouldn't >> be flash of its presence frequently. >> >> Thanks, >> Bean >> >> >> > Thanks, >> > >> > Can Guo. > > > Hi Can > Got your point, you don't want user space to interrupt UFS by sysyfs if > UFS is in power down mode. if this is true, insteading of removing all > sysfs node in ufshcd_shutdown, maybe just add this checkup before > accessing UFS device descriptors/flag/attributes/LU: > > for example, for the device descriptor: > > > diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs- > sysfs.c > index b3bf7fca00e5..881fe1c24a9f 100644 > --- a/drivers/scsi/ufs/ufs-sysfs.c > +++ b/drivers/scsi/ufs/ufs-sysfs.c > @@ -262,6 +262,9 @@ static ssize_t ufs_sysfs_read_desc_param(struct > ufs_hba *hba, > u8 desc_buf[8] = {0}; > int ret; > > + if (!ufshcd_is_ufs_dev_active(hba) || > !ufshcd_is_link_active(hba)) > + return -EACCES; > + > > Bean First of all, this check is not helping at all, during ufshcd_shutdown(), both the link and dev can be active for a moment, so this check is not helping the race condition. And assume you come up with a better check, you want to add the check everywhere? You must have noticed the fix to the func ufshcd_clk_gate_enable_store() from Jaegeuk Kim. So, don't expect any luck from user space, so long the sysfs nodes are available, users can grasp them and even stress them just for testing purposes. I don't see why removing the sysfs nodes during ufshcd_shutdown() is a concern to customer - we should do whatever is needed to protect LLD contexts from user space intervene. What do you think? Can Guo. Thanks, Can Guo.
On Wed, 2020-12-23 at 09:31 +0800, Can Guo wrote: > I don't see why removing the sysfs nodes during ufshcd_shutdown() is > a > concern to customer - we should do whatever is needed to protect LLD > contexts from user space intervene. What do you think? The sysfs nodes can be removed only when the device is remvoed. Bean
On Wed, 2020-12-23 at 09:31 +0800, Can Guo wrote: > First of all, this check is not helping at all, during > ufshcd_shutdown(), > both the link and dev can be active for a moment, so this check is > not > helping the race condition. yes, This checkup doesn't fix race, it is to address your remove of sysfs nodes in the ufshcd_shutdown(). Bean
On Wed, 2020-12-23 at 09:31 +0800, Can Guo wrote: > And assume you come up with a better check, > you want to add the check everywhere? You must have noticed the fix > to > the func ufshcd_clk_gate_enable_store() from Jaegeuk Kim. Do you mean lock spin_lock_irqsave(hba->host->host_lock, flags)? It can completely fix race issue, but it is different with here. ufshcd_clkgate_enable_store() doesn't call ufshcd_hold(). If you want using this lock, we should change ufshcd_hold() and ufshcd_query_*(). Bean
On Tue, 2020-12-22 at 14:08 +0800, Stanley Chu wrote: > Hi Bean, > > On Wed, 2020-12-16 at 00:05 +0100, 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. > > > > 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 | 41 > > ++++++++++++++++++++++++++++++++++++ > > drivers/scsi/ufs/ufshcd.c | 3 +-- > > drivers/scsi/ufs/ufshcd.h | 2 ++ > > 3 files changed, 44 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs- > > sysfs.c > > index 08e72b7eef6a..f3ca3d6b82c4 100644 > > --- a/drivers/scsi/ufs/ufs-sysfs.c > > +++ b/drivers/scsi/ufs/ufs-sysfs.c > > @@ -189,6 +189,45 @@ static ssize_t auto_hibern8_store(struct > > device *dev, > > return 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_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 (!ufshcd_is_wb_allowed(hba)) > > + return -EOPNOTSUPP; > > + > > + if (kstrtouint(buf, 0, &wb_enable)) > > + return -EINVAL; > > + > > + if (wb_enable != 0 && wb_enable != 1) > > + return -EINVAL; > > + > > + pm_runtime_get_sync(hba->dev); > > + res = ufshcd_wb_ctrl(hba, wb_enable); > > May this operation race with UFS shutdown flow? > > To be more clear, ufshcd_wb_ctrl() here may be executed after host > clock > is disabled by shutdown flow? > > If yes, we need to avoid it. > > Thanks, > Stanley Chu > Hi Stanley and Can I just sent a new patch to address this issue, let's discusss in that patch set, I added PM maintainer in the mail as well to help us address concern about pm_runtime_get_sync and shutdown flow. If that way can fix this issue, then I will update this patch again. one note: I didn't add "if (!ufshcd_is_ufs_dev_active(hba) || !ufshcd_is_link_active(hba))" checkup, since sysfs node access is a normal request as well, UFS driver should give correct response as possbile as it can, even if device/link is lower power mode. Thanks, Bean
diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c index 08e72b7eef6a..f3ca3d6b82c4 100644 --- a/drivers/scsi/ufs/ufs-sysfs.c +++ b/drivers/scsi/ufs/ufs-sysfs.c @@ -189,6 +189,45 @@ static ssize_t auto_hibern8_store(struct device *dev, return 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_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 (!ufshcd_is_wb_allowed(hba)) + return -EOPNOTSUPP; + + if (kstrtouint(buf, 0, &wb_enable)) + return -EINVAL; + + if (wb_enable != 0 && wb_enable != 1) + return -EINVAL; + + pm_runtime_get_sync(hba->dev); + res = ufshcd_wb_ctrl(hba, wb_enable); + pm_runtime_put_sync(hba->dev); + + 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); @@ -196,6 +235,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, @@ -205,6 +245,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 e221add25a7e..5e1dcf4de67e 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -246,7 +246,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); @@ -5351,7 +5350,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 9bb5f0ed4124..2a97006a2c93 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -1068,6 +1068,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) {