Hi Bart,
This patch still need your review.
Thanks.
Peter
On Thu, 2023-08-31 at 21:08 +0800, peter.wang@xxxxxxxxxxxx wrote:
> From: Peter Wang <peter.wang@xxxxxxxxxxxx>
>
> 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.
> Also inlines another __ufshcd_suspend_clkscaling calls.
>
> 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@xxxxxxxxxxxx>
> ---
> drivers/ufs/core/ufshcd.c | 17 ++++-------------
> 1 file changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index e3672e55efae..057549b0e586 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -275,7 +275,6 @@ static inline void
> ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba);
> static int ufshcd_host_reset_and_restore(struct ufs_hba *hba);
> static void ufshcd_resume_clkscaling(struct ufs_hba *hba);
> static void ufshcd_suspend_clkscaling(struct ufs_hba *hba);
> -static void __ufshcd_suspend_clkscaling(struct ufs_hba *hba);
> static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up);
> static irqreturn_t ufshcd_intr(int irq, void *__hba);
> static int ufshcd_change_power_mode(struct ufs_hba *hba,
> @@ -1385,9 +1384,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)
> @@ -1564,16 +1564,6 @@ static void ufshcd_devfreq_remove(struct
> ufs_hba *hba)
> dev_pm_opp_remove(hba->dev, clki->max_freq);
> }
>
> -static void __ufshcd_suspend_clkscaling(struct ufs_hba *hba)
> -{
> - unsigned long flags;
> -
> - devfreq_suspend_device(hba->devfreq);
> - spin_lock_irqsave(hba->host->host_lock, flags);
> - hba->clk_scaling.window_start_t = 0;
> - spin_unlock_irqrestore(hba->host->host_lock, flags);
> -}
> -
> static void ufshcd_suspend_clkscaling(struct ufs_hba *hba)
> {
> unsigned long flags;
> @@ -1586,11 +1576,12 @@ static void ufshcd_suspend_clkscaling(struct
> ufs_hba *hba)
> if (!hba->clk_scaling.is_suspended) {
> suspend = true;
> hba->clk_scaling.is_suspended = true;
> + hba->clk_scaling.window_start_t = 0;
> }
> spin_unlock_irqrestore(hba->host->host_lock, flags);
>
> if (suspend)
> - __ufshcd_suspend_clkscaling(hba);
> + devfreq_suspend_device(hba->devfreq);
> }
>
> static void ufshcd_resume_clkscaling(struct ufs_hba *hba)
