Hi Bart,
On 8/3/22 2:44 AM, Bart Van Assche wrote:
On 8/2/22 07:32, peter.wang@xxxxxxxxxxxx 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
