diff mbox series

[2/2] scsi: ufs: core: Introduce a new clock_scaling lock

Message ID 20241027082519.576869-3-avri.altman@wdc.com (mailing list archive)
State Superseded
Headers show
Series Untie the host lock entanglement - part 2 | expand

Commit Message

Avri Altman Oct. 27, 2024, 8:25 a.m. UTC 
Introduce a new clock gating lock to seriliaze access to the clock
scaling members instead of the host_lock.

Signed-off-by: Avri Altman <avri.altman@wdc.com>
---
 drivers/ufs/core/ufshcd.c | 48 ++++++++++++++++++++-------------------
 include/ufs/ufshcd.h      |  2 ++
 2 files changed, 27 insertions(+), 23 deletions(-)

Comments

Bart Van Assche Oct. 28, 2024, 8:06 p.m. UTC | #1 
On 10/27/24 1:25 AM, Avri Altman wrote:
> Introduce a new clock gating lock to seriliaze access to the clock
> scaling members instead of the host_lock.

Same feedback as for the previous patch: please fix the spelling of 
"seriliaze", please use guard() and scoped_guard() where appropriate and
please reorder the members of struct ufs_clk_scaling.

Thanks,

Bart.
Avri Altman Oct. 29, 2024, 5:40 a.m. UTC | #2 
> On 10/27/24 1:25 AM, Avri Altman wrote:
> > Introduce a new clock gating lock to seriliaze access to the clock
> > scaling members instead of the host_lock.
> 
> Same feedback as for the previous patch: please fix the spelling of "seriliaze",
> please use guard() and scoped_guard() where appropriate and please
> reorder the members of struct ufs_clk_scaling.
Done.

Thanks,
Avri

> 
> Thanks,
> 
> Bart.
diff mbox series

Patch

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index b7c7a7dd327f..fbaee68064c9 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -1449,14 +1449,14 @@  static void ufshcd_clk_scaling_suspend_work(struct work_struct *work)
 					   clk_scaling.suspend_work);
 	unsigned long irq_flags;
 
-	spin_lock_irqsave(hba->host->host_lock, irq_flags);
+	spin_lock_irqsave(&hba->clk_scaling.lock, irq_flags);
 	if (hba->clk_scaling.active_reqs || hba->clk_scaling.is_suspended) {
-		spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
+		spin_unlock_irqrestore(&hba->clk_scaling.lock, irq_flags);
 		return;
 	}
 	hba->clk_scaling.is_suspended = true;
 	hba->clk_scaling.window_start_t = 0;
-	spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
+	spin_unlock_irqrestore(&hba->clk_scaling.lock, irq_flags);
 
 	devfreq_suspend_device(hba->devfreq);
 }
@@ -1467,13 +1467,13 @@  static void ufshcd_clk_scaling_resume_work(struct work_struct *work)
 					   clk_scaling.resume_work);
 	unsigned long irq_flags;
 
-	spin_lock_irqsave(hba->host->host_lock, irq_flags);
+	spin_lock_irqsave(&hba->clk_scaling.lock, irq_flags);
 	if (!hba->clk_scaling.is_suspended) {
-		spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
+		spin_unlock_irqrestore(&hba->clk_scaling.lock, irq_flags);
 		return;
 	}
 	hba->clk_scaling.is_suspended = false;
-	spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
+	spin_unlock_irqrestore(&hba->clk_scaling.lock, irq_flags);
 
 	devfreq_resume_device(hba->devfreq);
 }
@@ -1508,15 +1508,15 @@  static int ufshcd_devfreq_target(struct device *dev,
 		*freq =	(unsigned long) clk_round_rate(clki->clk, *freq);
 	}
 
-	spin_lock_irqsave(hba->host->host_lock, irq_flags);
+	spin_lock_irqsave(&hba->clk_scaling.lock, irq_flags);
 	if (ufshcd_eh_in_progress(hba)) {
-		spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
+		spin_unlock_irqrestore(&hba->clk_scaling.lock, irq_flags);
 		return 0;
 	}
 
 	/* Skip scaling clock when clock scaling is suspended */
 	if (hba->clk_scaling.is_suspended) {
-		spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
+		spin_unlock_irqrestore(&hba->clk_scaling.lock, irq_flags);
 		dev_warn(hba->dev, "clock scaling is suspended, skip");
 		return 0;
 	}
@@ -1525,7 +1525,7 @@  static int ufshcd_devfreq_target(struct device *dev,
 		sched_clk_scaling_suspend_work = true;
 
 	if (list_empty(clk_list)) {
-		spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
+		spin_unlock_irqrestore(&hba->clk_scaling.lock, irq_flags);
 		goto out;
 	}
 
@@ -1540,11 +1540,11 @@  static int ufshcd_devfreq_target(struct device *dev,
 
 	/* Update the frequency */
 	if (!ufshcd_is_devfreq_scaling_required(hba, *freq, scale_up)) {
-		spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
+		spin_unlock_irqrestore(&hba->clk_scaling.lock, irq_flags);
 		ret = 0;
 		goto out; /* no state change required */
 	}
-	spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
+	spin_unlock_irqrestore(&hba->clk_scaling.lock, irq_flags);
 
 	start = ktime_get();
 	ret = ufshcd_devfreq_scale(hba, *freq, scale_up);
@@ -1577,7 +1577,7 @@  static int ufshcd_devfreq_get_dev_status(struct device *dev,
 
 	memset(stat, 0, sizeof(*stat));
 
-	spin_lock_irqsave(hba->host->host_lock, flags);
+	spin_lock_irqsave(&scaling->lock, flags);
 	curr_t = ktime_get();
 	if (!scaling->window_start_t)
 		goto start_window;
@@ -1613,7 +1613,7 @@  static int ufshcd_devfreq_get_dev_status(struct device *dev,
 		scaling->busy_start_t = 0;
 		scaling->is_busy_started = false;
 	}
-	spin_unlock_irqrestore(hba->host->host_lock, flags);
+	spin_unlock_irqrestore(&scaling->lock, flags);
 	return 0;
 }
 
@@ -1683,13 +1683,13 @@  static void ufshcd_suspend_clkscaling(struct ufs_hba *hba)
 	cancel_work_sync(&hba->clk_scaling.suspend_work);
 	cancel_work_sync(&hba->clk_scaling.resume_work);
 
-	spin_lock_irqsave(hba->host->host_lock, flags);
+	spin_lock_irqsave(&hba->clk_scaling.lock, flags);
 	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);
+	spin_unlock_irqrestore(&hba->clk_scaling.lock, flags);
 
 	if (suspend)
 		devfreq_suspend_device(hba->devfreq);
@@ -1700,12 +1700,12 @@  static void ufshcd_resume_clkscaling(struct ufs_hba *hba)
 	unsigned long flags;
 	bool resume = false;
 
-	spin_lock_irqsave(hba->host->host_lock, flags);
+	spin_lock_irqsave(&hba->clk_scaling.lock, flags);
 	if (hba->clk_scaling.is_suspended) {
 		resume = true;
 		hba->clk_scaling.is_suspended = false;
 	}
-	spin_unlock_irqrestore(hba->host->host_lock, flags);
+	spin_unlock_irqrestore(&hba->clk_scaling.lock, flags);
 
 	if (resume)
 		devfreq_resume_device(hba->devfreq);
@@ -1791,6 +1791,8 @@  static void ufshcd_init_clk_scaling(struct ufs_hba *hba)
 	INIT_WORK(&hba->clk_scaling.resume_work,
 		  ufshcd_clk_scaling_resume_work);
 
+	spin_lock_init(&hba->clk_scaling.lock);
+
 	hba->clk_scaling.workq = alloc_ordered_workqueue(
 		"ufs_clkscaling_%d", WQ_MEM_RECLAIM, hba->host->host_no);
 
@@ -2161,12 +2163,12 @@  static void ufshcd_clk_scaling_start_busy(struct ufs_hba *hba)
 	if (!ufshcd_is_clkscaling_supported(hba))
 		return;
 
-	spin_lock_irqsave(hba->host->host_lock, flags);
+	spin_lock_irqsave(&hba->clk_scaling.lock, flags);
 	if (!hba->clk_scaling.active_reqs++)
 		queue_resume_work = true;
 
 	if (!hba->clk_scaling.is_enabled || hba->pm_op_in_progress) {
-		spin_unlock_irqrestore(hba->host->host_lock, flags);
+		spin_unlock_irqrestore(&hba->clk_scaling.lock, flags);
 		return;
 	}
 
@@ -2184,7 +2186,7 @@  static void ufshcd_clk_scaling_start_busy(struct ufs_hba *hba)
 		hba->clk_scaling.busy_start_t = curr_t;
 		hba->clk_scaling.is_busy_started = true;
 	}
-	spin_unlock_irqrestore(hba->host->host_lock, flags);
+	spin_unlock_irqrestore(&hba->clk_scaling.lock, flags);
 }
 
 static void ufshcd_clk_scaling_update_busy(struct ufs_hba *hba)
@@ -2195,7 +2197,7 @@  static void ufshcd_clk_scaling_update_busy(struct ufs_hba *hba)
 	if (!ufshcd_is_clkscaling_supported(hba))
 		return;
 
-	spin_lock_irqsave(hba->host->host_lock, flags);
+	spin_lock_irqsave(&hba->clk_scaling.lock, flags);
 	hba->clk_scaling.active_reqs--;
 	if (!scaling->active_reqs && scaling->is_busy_started) {
 		scaling->tot_busy_t += ktime_to_us(ktime_sub(ktime_get(),
@@ -2203,7 +2205,7 @@  static void ufshcd_clk_scaling_update_busy(struct ufs_hba *hba)
 		scaling->busy_start_t = 0;
 		scaling->is_busy_started = false;
 	}
-	spin_unlock_irqrestore(hba->host->host_lock, flags);
+	spin_unlock_irqrestore(&hba->clk_scaling.lock, flags);
 }
 
 static inline int ufshcd_monitor_opcode2dir(u8 opcode)
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 52c822fe2944..058d27bc1e86 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -453,6 +453,7 @@  struct ufs_clk_gating {
  * @is_initialized: Indicates whether clock scaling is initialized or not
  * @is_busy_started: tracks if busy period has started or not
  * @is_suspended: tracks if devfreq is suspended or not
+ * @lock: seriliaze access to the clock_scaling members
  */
 struct ufs_clk_scaling {
 	int active_reqs;
@@ -472,6 +473,7 @@  struct ufs_clk_scaling {
 	bool is_busy_started;
 	bool is_suspended;
 	bool suspend_on_no_request;
+	spinlock_t lock;
 };
 
 #define UFS_EVENT_HIST_LENGTH 8