| Message ID | 20230823092948.22734-3-peter.wang@mediatek.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix abnormal clock scaling behaviors | expand |
On 8/23/23 02:29, peter.wang@mediatek.com wrote: > From: Peter Wang <peter.wang@mediatek.com> > > When ufshcd_clk_scaling_suspend_work(Thread A) running and new command > coming, ufshcd_clk_scaling_start_busy(Thread B) may get host_lock > after Thread A first time release host_lock. Then Thread A second time > get host_lock will set clk_scaling.window_start_t = 0 which scale up > clock abnormal next polling_ms time. > > Below is racing step: > 1 hba->clk_scaling.suspend_work (Thread A) > ufshcd_clk_scaling_suspend_work > 2 spin_lock_irqsave(hba->host->host_lock, irq_flags); > 3 hba->clk_scaling.is_suspended = true; > 4 spin_unlock_irqrestore(hba->host->host_lock, irq_flags); > __ufshcd_suspend_clkscaling > 7 spin_lock_irqsave(hba->host->host_lock, flags); > 8 hba->clk_scaling.window_start_t = 0; > 9 spin_unlock_irqrestore(hba->host->host_lock, flags); > > ufshcd_send_command (Thread B) > ufshcd_clk_scaling_start_busy > 5 spin_lock_irqsave(hba->host->host_lock, flags); > .... > 6 spin_unlock_irqrestore(hba->host->host_lock, flags); > > Signed-off-by: Peter Wang <peter.wang@mediatek.com> > --- > drivers/ufs/core/ufshcd.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > index e3672e55efae..017f32b3a789 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -1385,9 +1385,10 @@ static void ufshcd_clk_scaling_suspend_work(struct work_struct *work) > return; > } > hba->clk_scaling.is_suspended = true; > + hba->clk_scaling.window_start_t = 0; > spin_unlock_irqrestore(hba->host->host_lock, irq_flags); > > - __ufshcd_suspend_clkscaling(hba); > + devfreq_suspend_device(hba->devfreq); > } > > static void ufshcd_clk_scaling_resume_work(struct work_struct *work) There is a second __ufshcd_suspend_clkscaling() call in the same source file. Please fix that call too. Thanks, Bart.
On 8/29/23 20:00, Peter Wang (王信友) wrote: > Second __ufshcd_suspend_clkscaling is called from ufs wl suspend, which no on-going cmd. > > So, it should be not a problem. (cmd finish suspend clock scaling racing with new cmd start busy) This patch inlines one of the two __ufshcd_suspend_clkscaling() calls. I think it would be better if both calls are inlined. Thanks, Bart.
On Wed, 2023-08-30 at 12:43 -0700, Bart Van Assche wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > On 8/29/23 20:00, Peter Wang (王信友) wrote: > > Second __ufshcd_suspend_clkscaling is called from ufs wl suspend, > which no on-going cmd. > > > > So, it should be not a problem. (cmd finish suspend clock scaling > racing with new cmd start busy) > > This patch inlines one of the two __ufshcd_suspend_clkscaling() > calls. > I think it would be better if both calls are inlined. > > Thanks, > > Bart. Hi Bart, Could you help review the new patch. Thanks. Peter
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index e3672e55efae..017f32b3a789 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -1385,9 +1385,10 @@ static void ufshcd_clk_scaling_suspend_work(struct work_struct *work) return; } hba->clk_scaling.is_suspended = true; + hba->clk_scaling.window_start_t = 0; spin_unlock_irqrestore(hba->host->host_lock, irq_flags); - __ufshcd_suspend_clkscaling(hba); + devfreq_suspend_device(hba->devfreq); } static void ufshcd_clk_scaling_resume_work(struct work_struct *work)