Two changes to fix ufs power down/on specs violation

PATCH Superseded Ziqi Chen 2 participants 6 messages Jan 8, 2021

Subsystems (2)

SCSI Arm Qualcomm mailing list

lore

Share & tools

lore

Ziqi Chen

author

2021-01-08 07:29 UTC #0 lore raw

v5 cover Two changes to fix ufs power down/on specs violation

This series is made based on 5.11-scsi-staging branch.

As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device, RST_N signal and REF_CLK signal should be between VSS(Ground) and VCCQ/VCCQ2. The sequence after fixing as below:

Power down:
1. Assert RST_N low
2. Turn-off REF_CLK
3. Turn-off VCC
4. Turn-off VCCQ/VCCQ2.
power on:
1. Turn-on VCC
2. Turn-on VCCQ/VCCQ2
3. Turn-On REF_CLK
4. Deassert RST_N high.

The 1st change let ref-clk to be disabled before VCC & VCCQ turning off while to be enabled after VCC & VCCQ turning on. The 2nd change is used to pull down RST_n during UFS power off.

Change since v4:
- Split the original change "scsi: ufs: Fix ufs power down/on specs violation" into two changes, one fixs clk specs violation and the other one fixs RST_n specs violation.
- The 2nd change is just only for QCOM platform.

Change since v3:
- Rename vops callback func toggle_device_reset(sturct ufs_hba *hba, bool down) to device_reset(sturct ufs_hba *hba, bool assert).
- Move the dealy after pulling donw RST_n back into vops.

Change since v2:
- Correct commit message.

Change since v1:
- Rename vops callback func device_reset(sturct ufs_hba *hba) to toggle_device_reset(sturct ufs_hba *hba, bool down) to fix this specs violation for all SoC vendors platform.

Ziqi Chen (2):
scsi: ufs: Fix ufs clk specs violation
scsi: ufs-qcom: Fix ufs RST_n specs violation

 drivers/scsi/ufs/ufs-qcom.c |  4 ++++
 drivers/scsi/ufs/ufshcd.c   | 20 ++++++++++----------
 2 files changed, 14 insertions(+), 10 deletions(-)

Signature · 2 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Ziqi Chen

author

2021-01-08 07:29 UTC #1 lore raw

PATCH v5 1/2 scsi: ufs: Fix ufs clk specs violation

As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device, REF_CLK signal should be between VSS(Ground) and VCCQ/VCCQ2.

Signed-off-by Ziqi Chen <[email protected]>

---
 drivers/scsi/ufs/ufshcd.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

drivers/scsi/ufs/ufshcd.c +10 ~~-10~~

index e221add..3f807f7 100644
@@ -8686,8 +8686,6 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 	if (ret)
 		goto set_dev_active;
 
-	ufshcd_vreg_set_lpm(hba);
-
 disable_clks:
 	/*
 	 * Call vendor specific suspend callback. As these callbacks may access

@@ -8711,6 +8709,8 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 					hba->clk_gating.state);
 	}
 
+	ufshcd_vreg_set_lpm(hba);
+
 	/* Put the host controller in low power mode if possible */
 	ufshcd_hba_vreg_set_lpm(hba);
 	goto out;

@@ -8778,18 +8778,18 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 	old_link_state = hba->uic_link_state;
 
 	ufshcd_hba_vreg_set_hpm(hba);
+	ret = ufshcd_vreg_set_hpm(hba);
+	if (ret)
+		goto out;
+
 	/* Make sure clocks are enabled before accessing controller */
 	ret = ufshcd_setup_clocks(hba, true);
 	if (ret)
-		goto out;
+		goto disable_vreg;
 
 	/* enable the host irq as host controller would be active soon */
 	ufshcd_enable_irq(hba);
 
-	ret = ufshcd_vreg_set_hpm(hba);
-	if (ret)
-		goto disable_irq_and_vops_clks;
-
 	/*
 	 * Call vendor specific resume callback. As these callbacks may access
 	 * vendor specific host controller register space call them when the

@@ -8797,7 +8797,7 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 	 */
 	ret = ufshcd_vops_resume(hba, pm_op);
 	if (ret)
-		goto disable_vreg;
+		goto disable_irq_and_vops_clks;
 
 	/* For DeepSleep, the only supported option is to have the link off */
 	WARN_ON(ufshcd_is_ufs_dev_deepsleep(hba) && !ufshcd_is_link_off(hba));

@@ -8864,8 +8864,6 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 	ufshcd_link_state_transition(hba, old_link_state, 0);
 vendor_suspend:
 	ufshcd_vops_suspend(hba, pm_op);
-disable_vreg:
-	ufshcd_vreg_set_lpm(hba);
 disable_irq_and_vops_clks:
 	ufshcd_disable_irq(hba);
 	if (hba->clk_scaling.is_allowed)

@@ -8876,6 +8874,8 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 		trace_ufshcd_clk_gating(dev_name(hba->dev),
 					hba->clk_gating.state);
 	}
+disable_vreg:
+	ufshcd_vreg_set_lpm(hba);
 out:
 	hba->pm_op_in_progress = 0;
 	if (ret)

Signature · 2 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Can Guo

Reviewed-by on v5 1/2

2021-01-08 08:24 UTC #2 lore raw

Re: [PATCH v5 1/2] scsi: ufs: Fix ufs clk specs violation

Quoted 5 lines · Ziqi Chen

As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device, REF_CLK signal should be between VSS(Ground) and VCCQ/VCCQ2.
Signed-off-by Ziqi Chen <[email protected]>

Reviewed-by Can Guo <[email protected]>

Quoted 87 lines

---
 drivers/scsi/ufs/ufshcd.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

drivers/scsi/ufs/ufshcd.c +0 -2

index e221add..3f807f7 100644
@@ -8686,8 +8686,6 @@ static int ufshcd_suspend(struct ufs_hba *hba,
 enum ufs_pm_op pm_op)
 	if (ret)
 		goto set_dev_active;
 
-	ufshcd_vreg_set_lpm(hba);
-
 disable_clks:
 	/*

* Call vendor specific suspend callback. As these callbacks may
access

drivers/scsi/ufs/ufshcd.c +2 -0

@@ -8711,6 +8709,8 @@ static int ufshcd_suspend(struct ufs_hba *hba,
 enum ufs_pm_op pm_op)
 					hba->clk_gating.state);
 	}
 
+	ufshcd_vreg_set_lpm(hba);
+
 	/* Put the host controller in low power mode if possible */
 	ufshcd_hba_vreg_set_lpm(hba);

drivers/scsi/ufs/ufshcd.c +5 -5

 	goto out;

@@ -8778,18 +8778,18 @@ static int ufshcd_resume(struct ufs_hba *hba,
 enum ufs_pm_op pm_op)
 	old_link_state = hba->uic_link_state;
 
 	ufshcd_hba_vreg_set_hpm(hba);
+	ret = ufshcd_vreg_set_hpm(hba);
+	if (ret)
+		goto out;
+
 	/* Make sure clocks are enabled before accessing controller */
 	ret = ufshcd_setup_clocks(hba, true);
 	if (ret)
-		goto out;
+		goto disable_vreg;
 
 	/* enable the host irq as host controller would be active soon */
 	ufshcd_enable_irq(hba);
 
-	ret = ufshcd_vreg_set_hpm(hba);
-	if (ret)
-		goto disable_irq_and_vops_clks;
-
 	/*
 	 * Call vendor specific resume callback. As these callbacks may

access

drivers/scsi/ufs/ufshcd.c +1 -1

 	 * vendor specific host controller register space call them when the

@@ -8797,7 +8797,7 @@ static int ufshcd_resume(struct ufs_hba *hba,
 enum ufs_pm_op pm_op)
 	 */
 	ret = ufshcd_vops_resume(hba, pm_op);
 	if (ret)
-		goto disable_vreg;
+		goto disable_irq_and_vops_clks;
 
 	/* For DeepSleep, the only supported option is to have the link off

*/
WARN_ON(ufshcd_is_ufs_dev_deepsleep(hba) &&
!ufshcd_is_link_off(hba));

drivers/scsi/ufs/ufshcd.c +0 -2

@@ -8864,8 +8864,6 @@ static int ufshcd_resume(struct ufs_hba *hba,
 enum ufs_pm_op pm_op)
 	ufshcd_link_state_transition(hba, old_link_state, 0);
 vendor_suspend:
 	ufshcd_vops_suspend(hba, pm_op);
-disable_vreg:
-	ufshcd_vreg_set_lpm(hba);
 disable_irq_and_vops_clks:
 	ufshcd_disable_irq(hba);

drivers/scsi/ufs/ufshcd.c +2 -0

 	if (hba->clk_scaling.is_allowed)

@@ -8876,6 +8874,8 @@ static int ufshcd_resume(struct ufs_hba *hba,
 enum ufs_pm_op pm_op)
 		trace_ufshcd_clk_gating(dev_name(hba->dev),
 					hba->clk_gating.state);
 	}
+disable_vreg:
+	ufshcd_vreg_set_lpm(hba);
 out:
 	hba->pm_op_in_progress = 0;

if (ret)

Ziqi Chen

author

2021-01-08 07:30 UTC #3 lore raw

PATCH v5 2/2 scsi: ufs-qcom: Fix ufs RST_n specs violation

In reply to Ziqi Chen

As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device, RST_n signal should be between VSS(Ground) and VCCQ/VCCQ2.

Signed-off-by Ziqi Chen <[email protected]>

---
 drivers/scsi/ufs/ufs-qcom.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/scsi/ufs/ufs-qcom.c +4 -0

index 2206b1e..d8b896c 100644
@@ -582,6 +582,10 @@ static int ufs_qcom_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 		ufs_qcom_disable_lane_clks(host);
 		phy_power_off(phy);
 
+		/* reset the connected UFS device during power down */
+		if (host->device_reset)
+			gpiod_set_value_cansleep(host->device_reset, 1);
+
 	} else if (!ufs_qcom_is_link_active(hba)) {
 		ufs_qcom_disable_lane_clks(host);
 	}

Signature · 2 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Can Guo

2021-01-08 08:07 UTC #4 lore raw

Re: [PATCH v5 2/2] scsi: ufs-qcom: Fix ufs RST_n specs violation

Quoted 14 lines · Ziqi Chen

As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device, RST_n signal should be between VSS(Ground) and VCCQ/VCCQ2.
Signed-off-by Ziqi Chen <[email protected]>
---
 drivers/scsi/ufs/ufs-qcom.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/scsi/ufs/ufs-qcom.c +0 -0
index 2206b1e..d8b896c 100644

drivers/scsi/ufs/ufs-qcom.c static int ufs_qcom_suspend(struct ufs_hba *hba, · line 589

Quoted 8 lines

drivers/scsi/ufs/ufs-qcom.c +4 -0

@@ -582,6 +582,10 @@ static int ufs_qcom_suspend(struct ufs_hba *hba,
 enum ufs_pm_op pm_op)
 		ufs_qcom_disable_lane_clks(host);
 		phy_power_off(phy);
 
+		/* reset the connected UFS device during power down */
+		if (host->device_reset)
+			gpiod_set_value_cansleep(host->device_reset, 1);
+

Instead of calling gpiod_set_value(1/0) directly,
can we have a wrapper func for it?

Thanks,
Can Guo.

Quoted 3 lines

drivers/scsi/ufs/ufs-qcom.c +0 -0

 	} else if (!ufs_qcom_is_link_active(hba)) {
 		ufs_qcom_disable_lane_clks(host);

}

[email protected]

author

2021-01-08 08:16 UTC #5 lore raw

Re: [PATCH v5 2/2] scsi: ufs-qcom: Fix ufs RST_n specs violation

Quoted 27 lines · Can Guo

Quoted 14 lines · Ziqi Chen
As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device, RST_n signal should be between VSS(Ground) and VCCQ/VCCQ2.
Signed-off-by Ziqi Chen <[email protected]>
---
 drivers/scsi/ufs/ufs-qcom.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/scsi/ufs/ufs-qcom.c +0 -0
index 2206b1e..d8b896c 100644
drivers/scsi/ufs/ufs-qcom.c static int ufs_qcom_suspend(struct ufs_hba *hba, · line 589
Quoted 8 lines
drivers/scsi/ufs/ufs-qcom.c +4 -0
@@ -582,6 +582,10 @@ static int ufs_qcom_suspend(struct ufs_hba *hba,
 enum ufs_pm_op pm_op)
 		ufs_qcom_disable_lane_clks(host);
 		phy_power_off(phy);
 
+		/* reset the connected UFS device during power down */
+		if (host->device_reset)
+			gpiod_set_value_cansleep(host->device_reset, 1);
+
Instead of calling gpiod_set_value(1/0) directly,
can we have a wrapper func for it?

Quoted 2 lines

Thanks,
Can Guo.

Sure, it'll be better that way.

Best Regards,
Ziqi

Quoted 4 lines

Quoted 4 lines
drivers/scsi/ufs/ufs-qcom.c +0 -0
 	} else if (!ufs_qcom_is_link_active(hba)) {
 		ufs_qcom_disable_lane_clks(host);
}