Messages in this thread | | | Subject | Re: [PATCH 1/3] scsi: ufs: Add "wb_on" sysfs node to control WB on/off | From | "Asutosh Das (asd)" <> | Date | Mon, 30 Nov 2020 15:19:05 -0800 |
| |
On 11/30/2020 10:11 AM, Bean Huo wrote: > From: Bean Huo <beanhuo@micron.com> > > Currently we let UFS WriteBooster driver use 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. > > Signed-off-by: Bean Huo <beanhuo@micron.com> > --- > drivers/scsi/ufs/ufs-sysfs.c | 33 +++++++++++++++++++++++++++++++++ > drivers/scsi/ufs/ufshcd.c | 3 +-- > drivers/scsi/ufs/ufshcd.h | 2 ++ > 3 files changed, 36 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c > index 08e72b7eef6a..e41d8eb779ec 100644 > --- a/drivers/scsi/ufs/ufs-sysfs.c > +++ b/drivers/scsi/ufs/ufs-sysfs.c > @@ -189,6 +189,37 @@ 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 scnprintf(buf, PAGE_SIZE, "%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)) > + 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);
Say, a platform supports clock-scaling and this bit is toggled. The control goes into ufshcd_wb_ctrl for both this sysfs and clock-scaling contexts. The clock-scaling context passes all checks and blocks on waiting for this wb control to be disabled and then tries to enable wb when it's already disabled. Perhaps that's a race there?
> + 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 +227,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 +237,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 d169db41ee16..639ba9d1ccbb 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -247,7 +247,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); > @@ -5299,7 +5298,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 d0b68df07eef..c7bb61a4e484 100644 > --- a/drivers/scsi/ufs/ufshcd.h > +++ b/drivers/scsi/ufs/ufshcd.h > @@ -1067,6 +1067,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) > { >
-- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, Linux Foundation Collaborative Project
| |