Re: [PATCH v4] ufs: core: fix lockdep warning of clk_scaling_lock

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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






[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux