Re: [PATCH v2 2/3] ufs: core: fix abnormal scale up after last cmd finish

["Peter Wang (王信友)" <peter.wang@mediatek.com>]

Hi Bart,

This patch still need your review.

Thanks.
Peter


On Thu, 2023-08-31 at 21:08 +0800, 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.
> 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@mediatek.com>
> ---
>  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)