Re: [PATCH v3 1/2] ufs: core: introduce a choice of wb toggle during clock scaling

[Peter Wang <peter.wang@mediatek.com>]

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