| Message ID | 20220802143223.1276-2-peter.wang@mediatek.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ufs: allow host driver disable wb toggle druing clock scaling | expand |
Hi Bart, On 8/3/22 2:44 AM, Bart Van Assche wrote: > On 8/2/22 07:32, peter.wang@mediatek.com wrote: >> + /* Allow WB with clk scaling */ > > This comment is misleading. Consider changing this comment into > something like "Enable WB when scaling up the clock and disable WB > when scaling the clock down". Will change comment next version, thanks. > >> + UFSHCD_CAP_WB_WITH_CLK_SCALING = 1 << 12, > > Whether or not the WriteBooster is toggled when the clock is scaled is > not a host controller capability. It is a policy that is tied by this > patch to the host controller. So I think that using the "UFSHCD_CAP_" > prefix is misleading. Consider using e.g. the prefix UFSHCD_POLICY_. > The prefix UFSHCD_CAP_ is used in ufshcd_caps and not all of them is host capability. Ex. UFSHCD_CAP_HIBERN8_WITH_CLK_GATING is a policy host send hiberb8 with clk gating or not. Ex. UFSHCD_CAP_WB_EN is a host policy to turn-on WriteBooster or not. So, I think it is not suitable to break the naming rule in ufshcd_caps now. > > +static inline bool ufshcd_can_wb_during_scaling(struct ufs_hba *hba) > > +{ > > + return hba->caps & UFSHCD_CAP_WB_WITH_CLK_SCALING; > > +} > > The name of this function is misleading. Consider renaming this > function into e.g. ufshcd_enable_wb_if_scaling_up(). > > Thanks, > > Bart. Will change function name next version. Thanks. Peter
diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c index 0a088b47d557..8fa1679ffd1a 100644 --- a/drivers/ufs/core/ufs-sysfs.c +++ b/drivers/ufs/core/ufs-sysfs.c @@ -225,7 +225,8 @@ static ssize_t wb_on_store(struct device *dev, struct device_attribute *attr, unsigned int wb_enable; ssize_t res; - if (!ufshcd_is_wb_allowed(hba) || ufshcd_is_clkscaling_supported(hba)) { + if (!ufshcd_is_wb_allowed(hba) || (ufshcd_is_clkscaling_supported(hba) + && ufshcd_can_wb_during_scaling(hba))) { /* * If the platform supports UFSHCD_CAP_CLK_SCALING, turn WB * on/off will be done while clock scaling up/down. diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 3d367be71728..d86826e958e6 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -1301,9 +1301,11 @@ static int ufshcd_devfreq_scale(struct ufs_hba *hba, bool scale_up) } /* Enable Write Booster if we have scaled up else disable it */ - downgrade_write(&hba->clk_scaling_lock); - is_writelock = false; - ufshcd_wb_toggle(hba, scale_up); + if (ufshcd_can_wb_during_scaling(hba)) { + downgrade_write(&hba->clk_scaling_lock); + is_writelock = false; + ufshcd_wb_toggle(hba, scale_up); + } out_unprepare: ufshcd_clock_scaling_unprepare(hba, is_writelock); diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h index a92271421718..4bd35d68acde 100644 --- a/include/ufs/ufshcd.h +++ b/include/ufs/ufshcd.h @@ -648,6 +648,9 @@ enum ufshcd_caps { * notification if it is supported by the UFS device. */ UFSHCD_CAP_TEMP_NOTIF = 1 << 11, + + /* Allow WB with clk scaling */ + UFSHCD_CAP_WB_WITH_CLK_SCALING = 1 << 12, }; struct ufs_hba_variant_params { @@ -1004,6 +1007,10 @@ static inline bool ufshcd_is_wb_allowed(struct ufs_hba *hba) { return hba->caps & UFSHCD_CAP_WB_EN; } +static inline bool ufshcd_can_wb_during_scaling(struct ufs_hba *hba) +{ + return hba->caps & UFSHCD_CAP_WB_WITH_CLK_SCALING; +} #define ufshcd_writel(hba, val, reg) \ writel((val), (hba)->mmio_base + (reg))