On 7/28/22 2:12 AM, Bart Van Assche wrote:
On 7/26/22 20:21, peter.wang@xxxxxxxxxxxx wrote:
- /* 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);
+ /* Disable clk_scaling until ufshcd_wb_toggle finish */
+ hba->clk_scaling.is_allowed = false;
+ wb_toggle = true;
out_unprepare:
- ufshcd_clock_scaling_unprepare(hba, is_writelock);
+ ufshcd_clock_scaling_unprepare(hba);
+
+ /* Enable Write Booster if we have scaled up else disable it */
+ if (wb_toggle) {
+ ufshcd_wb_toggle(hba, scale_up);
+ ufshcd_clk_scaling_allow(hba, true);
+ }
I'm concerned that briefly disabling clock scaling may cause the clock
to remain at a high frequency even if it shouldn't. Has the following
approach been considered? Instead of moving the
ufshcd_clk_scaling_allow() call, convert dev_cmd.lock into a
semaphore, lock it near the start of ufshcd_devfreq_scale() and unlock
it near the end of the same function.
Thanks,
Bart.
Hi Bart,
Clock scaling up/down have a polling_ms, so it shouldn't have this
condition that scale up block scale down.
Convert dev_cmd.lock into a semaphore is more risky, and dev_cmd.lock
should hold when send dev command only.
I think it is not suitable to hold this dev_cmd.lock in
ufshcd_devfreq_scale.
Maybe we can have another choice, let vendor decide ufshcd_wb_toggle
with clock scaling or not?
Thanks.
Peter
