Re: [PATCH] scsi: ufs: qcom: move ufs_qcom_host_reset() to ufs_qcom_device_reset()
From: Ziqi Chen
Date: Wed Nov 29 2023 - 07:11:23 EST
On 11/28/2023 7:27 PM, Manivannan Sadhasivam wrote:
On Tue, Nov 28, 2023 at 03:40:57AM +0800, Ziqi Chen wrote:
On 11/22/2023 2:14 PM, Can Guo wrote:
On 10/25/2023 3:41 PM, Manivannan Sadhasivam wrote:
On Tue, Oct 24, 2023 at 07:10:15PM +0800, Ziqi Chen wrote:
During PISI test, we found the issue that host Tx still bursting after
What is PISI test?
SI measurement.
Please expand it in the patch description.
Sure, I will update in next patch version.
H/W reset. Move ufs_qcom_host_reset() to ufs_qcom_device_reset() and
reset host before device reset to stop tx burst.
device_reset() callback is supposed to reset only the device and not
the host.
So NACK for this patch.
Agree, the change should come in a more reasonable way.
Actually, similar code is already there in ufs_mtk_device_reset() in
ufs-mediatek.c, I guess here is trying to mimic that fashion.
This change, from its functionality point of view, we do need it,
because I occasionally (2 out of 10) hit PHY error on lane 0 during
reboot test (in my case, I tried SM8350, SM8450 and SM8550， all same).
[ 1.911188] [DEBUG]ufshcd_update_uic_error: UECPA:0x80000002
[ 1.922843] [DEBUG]ufshcd_update_uic_error: UECDL:0x80004000
[ 1.934473] [DEBUG]ufshcd_update_uic_error: UECN:0x0
[ 1.944688] [DEBUG]ufshcd_update_uic_error: UECT:0x0
[ 1.954901] [DEBUG]ufshcd_update_uic_error: UECDME:0x0
I found out that the PHY error pops out right after UFS device gets
reset in the 2nd init. After having this change in place, the PA/DL
errors are gone.
Hi Mani,
There is another way that adding a new vops that call XXX_host_reset() from
soc vendor driver. in this way, we can call this vops in core layer without
the dependency of device reset.
due to we already observed such error and received many same reports from
different OEMs, we need to fix it in some way.
if you think above way is available, I will update new patch in soon. Or
could you give us other suggestion?
First, please describe the issue in detail. How the issue is getting triggered
and then justify your change. I do not have access to the bug reports that you
received.
From the waveform measured by Samsung , we can see at the end of 2nd
Link Startup, host still keep bursting after H/W reset. This abnormal
timing would cause the PA/DL error mentioned by Can.
On the other hand, at the end of 1st Link start up, Host ends bursting
at first and then sends H/W reset to device. So Samsung suggested to do
host reset before every time device reset to fix this issue. That's what
you saw in this patch. This patch has been verified by OEMs.
So do you think if we can keep this change with details update in commit
message. or need to do other improvement?
-Ziqi
- Mani
-Ziqi
Thanks,
Can Guo.
- Mani
Signed-off-by: Ziqi Chen <quic_ziqichen@xxxxxxxxxxx>
---
drivers/ufs/host/ufs-qcom.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 96cb8b5..43163d3 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -445,12 +445,6 @@ static int
ufs_qcom_power_up_sequence(struct ufs_hba *hba)
struct phy *phy = host->generic_phy;
int ret;
- /* Reset UFS Host Controller and PHY */
- ret = ufs_qcom_host_reset(hba);
- if (ret)
- dev_warn(hba->dev, "%s: host reset returned %d\n",
- __func__, ret);
-
/* phy initialization - calibrate the phy */
ret = phy_init(phy);
if (ret) {
@@ -1709,6 +1703,13 @@ static void ufs_qcom_dump_dbg_regs(struct
ufs_hba *hba)
static int ufs_qcom_device_reset(struct ufs_hba *hba)
{
struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+ int ret = 0;
+
+ /* Reset UFS Host Controller and PHY */
+ ret = ufs_qcom_host_reset(hba);
+ if (ret)
+ dev_warn(hba->dev, "%s: host reset returned %d\n",
+ __func__, ret);
/* reset gpio is optional */
if (!host->device_reset)
--
2.7.4