ufs: core: Remove unnecessary block I/O quiesce for clock scaling

From: Peter Wang <[email protected]>

According to the MIPI UniPro Specification v2.0:

5.3.2.3 PA_DL_PAUSE.ind
This primitive informs the PA Service User, the DL Layer in this case, that the PA Layer was requested to execute a operation that requires the usage of the Link, e.g. Power Mode change or PACP frame transmission.

5.3.2.4 PA_DL_PAUSE.rsp_L
This primitive informs the Service Provider that the PA Service User, the DL Layer in this case, has reached a state where the Link may be used by the PA Layer.

5.3.2.5 PA_DL_RESUME.ind
This primitive informs the PA Service User, the DL Layer in this case, that the PA Layer has completed its operation and the DL Layer may continue to use the Link.

The detailed flow can be found in Figure 52:
Power Mode Change Using PACP_PWR_req and PACP_PWR_cnf.

In short, when the PA layer do power mode change:
1. The DL layer receives PA_DL_PAUSE.ind.
2. The DL layer stops and responds to the PA layer with PA_DL_PAUSE.rsp_L.
3. Waits until the PA layer has completed its work.
4. The PA layer then informs the DL layer with PA_DL_RESUME.ind.

Hence, it is not necessary to stop I/O during a power mode change, and this step can be removed.

---
 drivers/ufs/core/ufshcd.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

index c3f08957d179..b979f5105eb5 100644
@@ -1468,7 +1468,7 @@ static int ufshcd_scale_gear(struct ufs_hba *hba, u32 target_gear, bool scale_up
  *
  * Return: 0 upon success; -EBUSY upon timeout.
  */
-static int ufshcd_clock_scaling_prepare(struct ufs_hba *hba, u64 timeout_us)
+static int ufshcd_clock_scaling_prepare(struct ufs_hba *hba)
 {
 	int ret = 0;
 	/*

@@ -1476,16 +1476,13 @@ static int ufshcd_clock_scaling_prepare(struct ufs_hba *hba, u64 timeout_us)
 	 * clock scaling is in progress
 	 */
 	mutex_lock(&hba->host->scan_mutex);
-	blk_mq_quiesce_tagset(&hba->host->tag_set);
 	mutex_lock(&hba->wb_mutex);
 	down_write(&hba->clk_scaling_lock);
 
-	if (!hba->clk_scaling.is_allowed ||
-	    ufshcd_wait_for_pending_cmds(hba, timeout_us)) {
+	if (!hba->clk_scaling.is_allowed) {
 		ret = -EBUSY;
 		up_write(&hba->clk_scaling_lock);
 		mutex_unlock(&hba->wb_mutex);
-		blk_mq_unquiesce_tagset(&hba->host->tag_set);
 		mutex_unlock(&hba->host->scan_mutex);
 		goto out;
 	}

@@ -1501,7 +1498,6 @@ static void ufshcd_clock_scaling_unprepare(struct ufs_hba *hba, int err)
 {
 	up_write(&hba->clk_scaling_lock);
 	mutex_unlock(&hba->wb_mutex);
-	blk_mq_unquiesce_tagset(&hba->host->tag_set);
 	mutex_unlock(&hba->host->scan_mutex);
 
 	/* Enable Write Booster if current gear requires it else disable it */

@@ -1529,7 +1525,7 @@ static int ufshcd_devfreq_scale(struct ufs_hba *hba, unsigned long freq,
 
 	new_gear = ufshcd_vops_freq_to_gear_speed(hba, freq);
 
-	ret = ufshcd_clock_scaling_prepare(hba, 1 * USEC_PER_SEC);
+	ret = ufshcd_clock_scaling_prepare(hba);
 	if (ret)
 		return ret;
 

2.45.2

Peter,

Applied to 7.3/scsi-staging, thanks!

Martin K. Petersen

I'm testing linux-next on a Qualcomm platform. UFS worked fine on next-20260713 and got a regression on next-20260720 like:

[   36.243001] ufshcd-qcom 1d44000.ufshc: ufshcd_abort: Device abort task at tag 48
[   36.250616] sd 0:0:0:5: [sdf] tag#48 CDB: opcode=0x28 28 00 00 02 5f 32 00 00 10 00
[   36.258521] host_regs: 00000000: d5c7033f 20e0071f 00000400 00000000
[   36.265065] host_regs: 00000010: 01000000 00010217 00000c96 00000000
[   36.271607] host_regs: 00000020: 00000000 00371af5 00000000 00000000
[   36.278146] host_regs: 00000030: 0000010f 00000003 00000000 00000000
[   36.284688] host_regs: 00000040: 00000000 00000000 00000000 00000000
[   36.291229] host_regs: 00000050: fffdf000 0000000f 00000000 00000000
[   36.297775] host_regs: 00000060: 00000001 00000000 00000000 00000000
[   36.304317] host_regs: 00000070: fffde000 0000000f 00000000 00000000
[   36.310860] host_regs: 00000080: 00000001 00000000 00000000 00000000
[   36.317403] host_regs: 00000090: 00000002 15710000 00000000 00000004
[   36.323939] ufshcd-qcom 1d44000.ufshc: No record of pa_err
[   36.329595] ufshcd-qcom 1d44000.ufshc: No record of dl_err
[   36.335237] ufshcd-qcom 1d44000.ufshc: No record of nl_err
[   36.340878] ufshcd-qcom 1d44000.ufshc: No record of tl_err
[   36.346523] ufshcd-qcom 1d44000.ufshc: No record of dme_err
[   36.352252] ufshcd-qcom 1d44000.ufshc: No record of auto_hibern8_err
[   36.358790] ufshcd-qcom 1d44000.ufshc: No record of fatal_err
[   36.364701] ufshcd-qcom 1d44000.ufshc: No record of link_startup_fail
[   36.371332] ufshcd-qcom 1d44000.ufshc: No record of resume_fail
[   36.377428] ufshcd-qcom 1d44000.ufshc: No record of suspend_fail
[   36.383603] ufshcd-qcom 1d44000.ufshc: No record of wlun resume_fail
[   36.390142] ufshcd-qcom 1d44000.ufshc: No record of wlun suspend_fail
[   36.396769] ufshcd-qcom 1d44000.ufshc: dev_reset[0] = 0x0 at 3853838 us
[   36.403575] ufshcd-qcom 1d44000.ufshc: dev_reset: total cnt=1
[   36.409482] ufshcd-qcom 1d44000.ufshc: No record of host_reset
[   36.415478] ufshcd-qcom 1d44000.ufshc: task_abort[0] = 0x30 at 36258488 us
[   36.422546] ufshcd-qcom 1d44000.ufshc: task_abort: total cnt=1
[   36.428550] ufshcd-qcom 1d44000.ufshc: HW_H8_ENTER_CNT=2
[   36.434015] ufshcd-qcom 1d44000.ufshc: HW_H8_EXIT_CNT=1
[   36.439393] ufshcd-qcom 1d44000.ufshc: SW_H8_ENTER_CNT=3
[   36.444861] ufshcd-qcom 1d44000.ufshc: SW_H8_EXIT_CNT=4
[   36.450239] ufshcd-qcom 1d44000.ufshc: SW_AFTER_HW_H8_ENTER_CNT=1
[   36.456526] HCI Vendor Specific Registers 00000000: 00000193 00000000 00000000 00000000
[   36.464758] HCI Vendor Specific Registers 00000010: 002aaa52 00000000 00000001 1c00052c
[   36.472987] HCI Vendor Specific Registers 00000020: 1b0113ff 60020000 00000000 00000000
[   36.481216] HCI Vendor Specific Registers 00000030: 00000000 00000000 00000000 0000183c
[   36.489481] UFS_UFS_DBG_RD_REG_OCSC 00000000: 00000000 00000000 00000000 00000000
[   36.497175] UFS_UFS_DBG_RD_REG_OCSC 00000010: 00000000 00000000 00000000 00000000
[   36.504867] UFS_UFS_DBG_RD_REG_OCSC 00000020: 00000000 00000000 00000000 00000000
[   36.512558] UFS_UFS_DBG_RD_REG_OCSC 00000030: 00000000 00000013 00000000 00000000

...

A revert of commit 85b2b9662854 ("scsi: ufs: core: Remove unnecessary block I/O quiesce for clock scaling") fixes the regression for me.

Shawn

Hi Shawn,

What is the probability of this issue occurring? Is this a design limitation of the Qualcomm platform or?

Thanks
Peter

Hi Peter,

It seems 100% per my testing.

I cannot tell. I copied Manivannan who is our UFS expert, for possible input.

Shawn

Whenever clk scaling gets triggered. So this affects all Qcom UFS platforms.

Qcom Host Controllers doesn't support scaling clocks when the traffic is ongoing. So this patch certainly cannot work on our platforms.

If you want to pursue, you should limit this change to MediaTek platforms. But nevertheless, this patch should be reverted as it breaks UFS on all Qcom platforms.

- Mani

மணிவண்ணன் சதாசிவம்

Hi Mani,

This patch is based on the JEDEC specification. As all SoCs should follow this spec, I have not applied it solely to MediaTek's platform. However, I think reverting it is okay. I will consider how to revise it later.

Thanks
Peter

What you quoted in the commit message is true for power mode change (gear, lane etc...), but clock scaling involves scaling up/down the HCI clock inputs as well, which is not covered by the JEDEC spec and is vendor specific. And this is where the discrepancy lives.

Qcom UFS HCI also allows power mode change with in-flight traffic, but scaling the clocks is not supported. I'm not sure about other vendors here.

- Mani

மணிவண்ணன் சதாசிவம்

JEDEC spec mandates that the power mode changes (gear, lane, etc...) can happen while the traffic is on-going. But clock scaling done by the ufshcd driver involves not just power mode change, but also scaling the vendor HCI clocks as well. And that is what causing issues on Qcom platforms, as our UFS controller doesn't support scaling HCI clocks during live traffic. I'm not sure about other vendors here.

- Mani

மணிவண்ணன் சதாசிவம்