Hi Can, On Mon, 2020-02-17 at 21:22 +0800, Can Guo wrote: > On 2020-02-17 21:12, Stanley Chu wrote: > > Hi Can, > > > > > >> > } else if (!on && clki->enabled) { > >> > clk_disable_unprepare(clki->clk); > >> > + wait_us = hba->dev_info.clk_gating_wait_us; > >> > + if (ref_clk && wait_us) > >> > + usleep_range(wait_us, wait_us + 10); > >> > >> Hi St,anley, > >> > >> If wait_us is 1us, it would be inappropriate to use usleep_range() > >> here. > >> You have checks of the delay in patch #2, but why it is not needed > >> here? > >> > >> Thanks, > >> Can Guo. > > > > You are right. I could make that delay checking as common function so > > it > > can be used here as well to cover all possible values. > > > > Thanks for suggestion. > > Stanley > > Hi Stanley, > > One more thing, as in patch #2, you have already added delays in your > ufshcd_vops_setup_clocks(OFF, PRE_CHANGE) path, plus this delay here, > don't you delay for 2*bRefClkGatingWaitTime in ufshcd_setup_clocks()? > As the delay added in your vops also delays the actions of turning > off all the other clocks in ufshcd_setup_clocks(), you don't need the > delay here again, do you agree? MediaTek driver is not using reference clocks named as "ref_clk" defined in device tree, thus the delay specific for "ref_clk" in ufshcd_setup_clocks() will not be applied in MediaTek platform. This patch is aimed to add delay for this kind of "ref_clk" used by any future vendors. Anyway thanks for the reminding : ) > > Thanks, > Can Guo. Thanks, Stanley