Message ID | 1700729190-17268-7-git-send-email-quic_cang@quicinc.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:ce62:0:b0:403:3b70:6f57 with SMTP id o2csp304116vqx; Thu, 23 Nov 2023 00:47:43 -0800 (PST) X-Google-Smtp-Source: AGHT+IHeI/auZus5INO8rISZszgMdGKy5FvIXHVzWGP4IXGBHMDIO8Bh5ASyOb012cVig7T/1omC X-Received: by 2002:a05:6a20:7f95:b0:187:440b:6e40 with SMTP id d21-20020a056a207f9500b00187440b6e40mr5590386pzj.17.1700729263141; Thu, 23 Nov 2023 00:47:43 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1700729263; cv=none; d=google.com; s=arc-20160816; b=LZ5g6hKlPo+VXJl8xu8a0b1yEdf3TIeU5uL+CXSx7Da1AXkV0u8uKIisDBX7eYE5KQ QnYH1qG6MHO5I5FxGCJpYwQ6xle1XKxj2//897A7vjIB+cYAhGy98ESbDMLdDv0ln9UQ Vto2bHv/YoKobIYPBjGOddRdbcGcPemcjNdXxOMiJjysmMGkFiJTx9G3+IYqrYSQwTti pAQA2Iqs27NgwuYURa49GbBzLLUKBuwcfNeA7GnVEaThXJ3iJJctcWeGyMnnble6CdJH SwfVnJHrxqXouvYgALiBb4mid5cH+hEiji8jbmsxpORXPXjxYz0P7ntazZYW7d3Q/7pg vVaA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:references:in-reply-to:message-id:date:subject :cc:to:from:dkim-signature; bh=bOrJSSNFsLGbiSSB8YF589JZQLsAbNb5oTKc3fog6kw=; fh=Mo6TxgGzGLrlS+5dVlNWdqd+fZYn09cU+8+JAb6EqZk=; b=mPNZQ5bocsTfMKNdaqVMt2pl1oD6jgG4+8ttvL9FnKM63dG0VADk1IPOnkuGQUkdfy fF/5OWUUikLd0lvt601w1Xb75kfICo24hxVvZ/PAaKpvP4JR/TgOjPyXKqrAUAAjjCbN 2FTU+QfM98vxzo8bnLlhVeuPjMZQjZ1sJMhhG75PHKBK0Tf56/d5+9invNtayaWZf1zP q/sWz9DSEXD8oeNsnW/oSeu5Pkzob3xJahou04MTifJkINPZfLj9/I4BCwkiisRnnyU5 Jj4huhPMDXnGBT8wJfxNUn3mCeecCrRLu5Dd8M6NqG9x4DL9QZYfU4eT+cDGUhdvwZyH dz/Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=m8pDXV8O; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: from agentk.vger.email (agentk.vger.email. [23.128.96.32]) by mx.google.com with ESMTPS id g2-20020a17090adb0200b0027d7eebd4c0si896175pjv.109.2023.11.23.00.47.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Nov 2023 00:47:43 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32; Authentication-Results: mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=m8pDXV8O; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by agentk.vger.email (Postfix) with ESMTP id 14D96823C122; Thu, 23 Nov 2023 00:47:40 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at agentk.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1345003AbjKWIra (ORCPT <rfc822;ouuuleilei@gmail.com> + 99 others); Thu, 23 Nov 2023 03:47:30 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39362 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1344994AbjKWIrQ (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 23 Nov 2023 03:47:16 -0500 Received: from mx0a-0031df01.pphosted.com (mx0a-0031df01.pphosted.com [205.220.168.131]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 30C9DD65; Thu, 23 Nov 2023 00:47:21 -0800 (PST) Received: from pps.filterd (m0279863.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3AN8Bxjc027512; Thu, 23 Nov 2023 08:47:09 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h=from : to : cc : subject : date : message-id : in-reply-to : references; s=qcppdkim1; bh=bOrJSSNFsLGbiSSB8YF589JZQLsAbNb5oTKc3fog6kw=; b=m8pDXV8OUsStX/rCd5C/9uuvPYiMtEutycu2Bj0255C03BdwAs3yoBQacQTsPUEi1dwR ROY+HFE5ZmAiN9R7CMuMAEUawfFyp6w/NHa8Ze1A4zfE+/GfKAvvMJyzMtFhGpfBorwW d7zBXtbXAihY5/agX/7hhNultLI6dzyQrYoGsn0CcSmDtx1jzPXVvKxITIWgzxac5OSo sGuFAqps7HhY/gx2Dj8PikgkEDaiHMjNFUIwJ+mpmPXLlt0GhYJWgInn90y2DUzzXp/g 6KLrQHNRWo+YTYlzjK4hmbe7Dqnh6chSFhXPI2uvNj0iSRAzxJN3ZyRx6BcmZeiLczGR rA== Received: from nasanppmta04.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3uhgajtnen-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 23 Nov 2023 08:47:09 +0000 Received: from pps.filterd (NASANPPMTA04.qualcomm.com [127.0.0.1]) by NASANPPMTA04.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTP id 3AN8hn0o004542; Thu, 23 Nov 2023 08:47:08 GMT Received: from pps.reinject (localhost [127.0.0.1]) by NASANPPMTA04.qualcomm.com (PPS) with ESMTP id 3uhpmqpm71-1; Thu, 23 Nov 2023 08:47:08 +0000 Received: from NASANPPMTA04.qualcomm.com (NASANPPMTA04.qualcomm.com [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 3AN8l0qn008257; Thu, 23 Nov 2023 08:47:08 GMT Received: from stor-dylan.qualcomm.com (stor-dylan.qualcomm.com [192.168.140.207]) by NASANPPMTA04.qualcomm.com (PPS) with ESMTP id 3AN8l8UA008457; Thu, 23 Nov 2023 08:47:08 +0000 Received: by stor-dylan.qualcomm.com (Postfix, from userid 359480) id 403C420A68; Thu, 23 Nov 2023 00:47:08 -0800 (PST) From: Can Guo <quic_cang@quicinc.com> To: quic_cang@quicinc.com, bvanassche@acm.org, mani@kernel.org, adrian.hunter@intel.com, beanhuo@micron.com, avri.altman@wdc.com, junwoo80.lee@samsung.com, martin.petersen@oracle.com Cc: linux-scsi@vger.kernel.org, linux-arm-msm@vger.kernel.org, Andy Gross <agross@kernel.org>, Bjorn Andersson <andersson@kernel.org>, Konrad Dybcio <konrad.dybcio@linaro.org>, "James E.J. Bottomley" <jejb@linux.ibm.com>, linux-kernel@vger.kernel.org (open list) Subject: [PATCH v5 06/10] scsi: ufs: ufs-qcom: Limit HS-G5 Rate-A to hosts with HW version 5 Date: Thu, 23 Nov 2023 00:46:26 -0800 Message-Id: <1700729190-17268-7-git-send-email-quic_cang@quicinc.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1700729190-17268-1-git-send-email-quic_cang@quicinc.com> References: <1700729190-17268-1-git-send-email-quic_cang@quicinc.com> X-QCInternal: smtphost X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-GUID: -3QLVZLed4klSknEXm2IZwPsNexNWSvE X-Proofpoint-ORIG-GUID: -3QLVZLed4klSknEXm2IZwPsNexNWSvE X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.987,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-11-23_06,2023-11-22_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 phishscore=0 mlxlogscore=724 priorityscore=1501 lowpriorityscore=0 spamscore=0 bulkscore=0 clxscore=1015 impostorscore=0 malwarescore=0 mlxscore=0 adultscore=0 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311060000 definitions=main-2311230062 X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Thu, 23 Nov 2023 00:47:40 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1783343888169828614 X-GMAIL-MSGID: 1783343888169828614 |
Series |
Enable HS-G5 support on SM8550
|
|
Commit Message
Can Guo
Nov. 23, 2023, 8:46 a.m. UTC
Qcom UFS hosts, with HW ver 5, can only support up to HS-G5 Rate-A due to
HW limitations. If the HS-G5 PHY gear is used, update host_params->hs_rate
to Rate-A, so that the subsequent power mode changes shall stick to Rate-A.
Signed-off-by: Can Guo <quic_cang@quicinc.com>
---
drivers/ufs/host/ufs-qcom.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
Comments
On 11/23/2023 2:16 PM, Can Guo wrote: > Qcom UFS hosts, with HW ver 5, can only support up to HS-G5 Rate-A due to > HW limitations. If the HS-G5 PHY gear is used, update host_params->hs_rate > to Rate-A, so that the subsequent power mode changes shall stick to Rate-A. > > Signed-off-by: Can Guo <quic_cang@quicinc.com> > --- > drivers/ufs/host/ufs-qcom.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c > index 9613ad9..6756f8d 100644 > --- a/drivers/ufs/host/ufs-qcom.c > +++ b/drivers/ufs/host/ufs-qcom.c > @@ -442,9 +442,25 @@ static u32 ufs_qcom_get_hs_gear(struct ufs_hba *hba) > static int ufs_qcom_power_up_sequence(struct ufs_hba *hba) > { > struct ufs_qcom_host *host = ufshcd_get_variant(hba); > + struct ufs_host_params *host_params = &host->host_params; > struct phy *phy = host->generic_phy; > + enum phy_mode mode; > int ret; > > + /* > + * HW ver 5 can only support up to HS-G5 Rate-A due to HW limitations. > + * If the HS-G5 PHY gear is used, update host_params->hs_rate to Rate-A, > + * so that the subsequent power mode change shall stick to Rate-A. > + */ > + if (host->hw_ver.major == 0x5) { > + if (host->phy_gear == UFS_HS_G5) > + host_params->hs_rate = PA_HS_MODE_A; > + else > + host_params->hs_rate = PA_HS_MODE_B; > + } > + > + mode = host_params->hs_rate == PA_HS_MODE_B ? PHY_MODE_UFS_HS_B : PHY_MODE_UFS_HS_A; > + > /* Reset UFS Host Controller and PHY */ > ret = ufs_qcom_host_reset(hba); > if (ret) > @@ -459,7 +475,7 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba) > return ret; > } > > - phy_set_mode_ext(phy, PHY_MODE_UFS_HS_B, host->phy_gear); > + phy_set_mode_ext(phy, mode, host->phy_gear); > > /* power on phy - start serdes and phy's power and clocks */ > ret = phy_power_on(phy); Reviewed-by: Nitin Rawat <quic_nitirawa@quicinc.com>
On Thu, Nov 23, 2023 at 12:46:26AM -0800, Can Guo wrote: > Qcom UFS hosts, with HW ver 5, can only support up to HS-G5 Rate-A due to > HW limitations. If the HS-G5 PHY gear is used, update host_params->hs_rate > to Rate-A, so that the subsequent power mode changes shall stick to Rate-A. > > Signed-off-by: Can Guo <quic_cang@quicinc.com> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> One question below... > --- > drivers/ufs/host/ufs-qcom.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c > index 9613ad9..6756f8d 100644 > --- a/drivers/ufs/host/ufs-qcom.c > +++ b/drivers/ufs/host/ufs-qcom.c > @@ -442,9 +442,25 @@ static u32 ufs_qcom_get_hs_gear(struct ufs_hba *hba) > static int ufs_qcom_power_up_sequence(struct ufs_hba *hba) > { > struct ufs_qcom_host *host = ufshcd_get_variant(hba); > + struct ufs_host_params *host_params = &host->host_params; > struct phy *phy = host->generic_phy; > + enum phy_mode mode; > int ret; > > + /* > + * HW ver 5 can only support up to HS-G5 Rate-A due to HW limitations. > + * If the HS-G5 PHY gear is used, update host_params->hs_rate to Rate-A, > + * so that the subsequent power mode change shall stick to Rate-A. > + */ > + if (host->hw_ver.major == 0x5) { > + if (host->phy_gear == UFS_HS_G5) > + host_params->hs_rate = PA_HS_MODE_A; > + else > + host_params->hs_rate = PA_HS_MODE_B; Is this 'else' part really needed? Since there wouldn't be any 2nd init, I think we can skip that. - Mani > + } > + > + mode = host_params->hs_rate == PA_HS_MODE_B ? PHY_MODE_UFS_HS_B : PHY_MODE_UFS_HS_A; > + > /* Reset UFS Host Controller and PHY */ > ret = ufs_qcom_host_reset(hba); > if (ret) > @@ -459,7 +475,7 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba) > return ret; > } > > - phy_set_mode_ext(phy, PHY_MODE_UFS_HS_B, host->phy_gear); > + phy_set_mode_ext(phy, mode, host->phy_gear); > > /* power on phy - start serdes and phy's power and clocks */ > ret = phy_power_on(phy); > -- > 2.7.4 >
Hi Mani, On 11/28/2023 1:55 PM, Manivannan Sadhasivam wrote: > On Thu, Nov 23, 2023 at 12:46:26AM -0800, Can Guo wrote: >> Qcom UFS hosts, with HW ver 5, can only support up to HS-G5 Rate-A due to >> HW limitations. If the HS-G5 PHY gear is used, update host_params->hs_rate >> to Rate-A, so that the subsequent power mode changes shall stick to Rate-A. >> >> Signed-off-by: Can Guo <quic_cang@quicinc.com> > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > One question below... > >> --- >> drivers/ufs/host/ufs-qcom.c | 18 +++++++++++++++++- >> 1 file changed, 17 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c >> index 9613ad9..6756f8d 100644 >> --- a/drivers/ufs/host/ufs-qcom.c >> +++ b/drivers/ufs/host/ufs-qcom.c >> @@ -442,9 +442,25 @@ static u32 ufs_qcom_get_hs_gear(struct ufs_hba *hba) >> static int ufs_qcom_power_up_sequence(struct ufs_hba *hba) >> { >> struct ufs_qcom_host *host = ufshcd_get_variant(hba); >> + struct ufs_host_params *host_params = &host->host_params; >> struct phy *phy = host->generic_phy; >> + enum phy_mode mode; >> int ret; >> >> + /* >> + * HW ver 5 can only support up to HS-G5 Rate-A due to HW limitations. >> + * If the HS-G5 PHY gear is used, update host_params->hs_rate to Rate-A, >> + * so that the subsequent power mode change shall stick to Rate-A. >> + */ >> + if (host->hw_ver.major == 0x5) { >> + if (host->phy_gear == UFS_HS_G5) >> + host_params->hs_rate = PA_HS_MODE_A; >> + else >> + host_params->hs_rate = PA_HS_MODE_B; > > Is this 'else' part really needed? Since there wouldn't be any 2nd init, I think > we can skip that. We need it because, even there is only one init, if a UFS3.1 device is attached, phy_gear is given as UFS_HS_G4 in ufs_qcom_set_phy_gear(), hence we need to put the UFS at HS-G4 Rate B, not Rate A. Thanks, Can Guo. > > - Mani > >> + } >> + >> + mode = host_params->hs_rate == PA_HS_MODE_B ? PHY_MODE_UFS_HS_B : PHY_MODE_UFS_HS_A; >> + >> /* Reset UFS Host Controller and PHY */ >> ret = ufs_qcom_host_reset(hba); >> if (ret) >> @@ -459,7 +475,7 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba) >> return ret; >> } >> >> - phy_set_mode_ext(phy, PHY_MODE_UFS_HS_B, host->phy_gear); >> + phy_set_mode_ext(phy, mode, host->phy_gear); >> >> /* power on phy - start serdes and phy's power and clocks */ >> ret = phy_power_on(phy); >> -- >> 2.7.4 >> >
On Tue, Nov 28, 2023 at 03:48:02PM +0800, Can Guo wrote: > Hi Mani, > > On 11/28/2023 1:55 PM, Manivannan Sadhasivam wrote: > > On Thu, Nov 23, 2023 at 12:46:26AM -0800, Can Guo wrote: > > > Qcom UFS hosts, with HW ver 5, can only support up to HS-G5 Rate-A due to > > > HW limitations. If the HS-G5 PHY gear is used, update host_params->hs_rate > > > to Rate-A, so that the subsequent power mode changes shall stick to Rate-A. > > > > > > Signed-off-by: Can Guo <quic_cang@quicinc.com> > > > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > > > One question below... > > > > > --- > > > drivers/ufs/host/ufs-qcom.c | 18 +++++++++++++++++- > > > 1 file changed, 17 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c > > > index 9613ad9..6756f8d 100644 > > > --- a/drivers/ufs/host/ufs-qcom.c > > > +++ b/drivers/ufs/host/ufs-qcom.c > > > @@ -442,9 +442,25 @@ static u32 ufs_qcom_get_hs_gear(struct ufs_hba *hba) > > > static int ufs_qcom_power_up_sequence(struct ufs_hba *hba) > > > { > > > struct ufs_qcom_host *host = ufshcd_get_variant(hba); > > > + struct ufs_host_params *host_params = &host->host_params; > > > struct phy *phy = host->generic_phy; > > > + enum phy_mode mode; > > > int ret; > > > + /* > > > + * HW ver 5 can only support up to HS-G5 Rate-A due to HW limitations. > > > + * If the HS-G5 PHY gear is used, update host_params->hs_rate to Rate-A, > > > + * so that the subsequent power mode change shall stick to Rate-A. > > > + */ > > > + if (host->hw_ver.major == 0x5) { > > > + if (host->phy_gear == UFS_HS_G5) > > > + host_params->hs_rate = PA_HS_MODE_A; > > > + else > > > + host_params->hs_rate = PA_HS_MODE_B; > > > > Is this 'else' part really needed? Since there wouldn't be any 2nd init, I think > > we can skip that. > > We need it because, even there is only one init, if a UFS3.1 device is > attached, phy_gear is given as UFS_HS_G4 in ufs_qcom_set_phy_gear(), hence > we need to put the UFS at HS-G4 Rate B, not Rate A. > But the default hs_rate is PA_HS_MODE_B only and the else condition would be not needed for the 1st init. - Mani > Thanks, > Can Guo. > > > > > - Mani > > > > > + } > > > + > > > + mode = host_params->hs_rate == PA_HS_MODE_B ? PHY_MODE_UFS_HS_B : PHY_MODE_UFS_HS_A; > > > + > > > /* Reset UFS Host Controller and PHY */ > > > ret = ufs_qcom_host_reset(hba); > > > if (ret) > > > @@ -459,7 +475,7 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba) > > > return ret; > > > } > > > - phy_set_mode_ext(phy, PHY_MODE_UFS_HS_B, host->phy_gear); > > > + phy_set_mode_ext(phy, mode, host->phy_gear); > > > /* power on phy - start serdes and phy's power and clocks */ > > > ret = phy_power_on(phy); > > > -- > > > 2.7.4 > > > > >
On 11/28/2023 6:55 PM, Manivannan Sadhasivam wrote: > On Tue, Nov 28, 2023 at 03:48:02PM +0800, Can Guo wrote: >> Hi Mani, >> >> On 11/28/2023 1:55 PM, Manivannan Sadhasivam wrote: >>> On Thu, Nov 23, 2023 at 12:46:26AM -0800, Can Guo wrote: >>>> Qcom UFS hosts, with HW ver 5, can only support up to HS-G5 Rate-A due to >>>> HW limitations. If the HS-G5 PHY gear is used, update host_params->hs_rate >>>> to Rate-A, so that the subsequent power mode changes shall stick to Rate-A. >>>> >>>> Signed-off-by: Can Guo <quic_cang@quicinc.com> >>> >>> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> >>> >>> One question below... >>> >>>> --- >>>> drivers/ufs/host/ufs-qcom.c | 18 +++++++++++++++++- >>>> 1 file changed, 17 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c >>>> index 9613ad9..6756f8d 100644 >>>> --- a/drivers/ufs/host/ufs-qcom.c >>>> +++ b/drivers/ufs/host/ufs-qcom.c >>>> @@ -442,9 +442,25 @@ static u32 ufs_qcom_get_hs_gear(struct ufs_hba *hba) >>>> static int ufs_qcom_power_up_sequence(struct ufs_hba *hba) >>>> { >>>> struct ufs_qcom_host *host = ufshcd_get_variant(hba); >>>> + struct ufs_host_params *host_params = &host->host_params; >>>> struct phy *phy = host->generic_phy; >>>> + enum phy_mode mode; >>>> int ret; >>>> + /* >>>> + * HW ver 5 can only support up to HS-G5 Rate-A due to HW limitations. >>>> + * If the HS-G5 PHY gear is used, update host_params->hs_rate to Rate-A, >>>> + * so that the subsequent power mode change shall stick to Rate-A. >>>> + */ >>>> + if (host->hw_ver.major == 0x5) { >>>> + if (host->phy_gear == UFS_HS_G5) >>>> + host_params->hs_rate = PA_HS_MODE_A; >>>> + else >>>> + host_params->hs_rate = PA_HS_MODE_B; >>> >>> Is this 'else' part really needed? Since there wouldn't be any 2nd init, I think >>> we can skip that. >> >> We need it because, even there is only one init, if a UFS3.1 device is >> attached, phy_gear is given as UFS_HS_G4 in ufs_qcom_set_phy_gear(), hence >> we need to put the UFS at HS-G4 Rate B, not Rate A. >> > > But the default hs_rate is PA_HS_MODE_B only and the else condition would be not > needed for the 1st init. You are right, but still we need this in case the UFS device version is not populated, meaning dual init can also happen to SM8550. We need to apply the right hs_rate in case the 2nd init asks for HS_G4. Thanks, Can Guo. > > - Mani > >> Thanks, >> Can Guo. >> >>> >>> - Mani >>> >>>> + } >>>> + >>>> + mode = host_params->hs_rate == PA_HS_MODE_B ? PHY_MODE_UFS_HS_B : PHY_MODE_UFS_HS_A; >>>> + >>>> /* Reset UFS Host Controller and PHY */ >>>> ret = ufs_qcom_host_reset(hba); >>>> if (ret) >>>> @@ -459,7 +475,7 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba) >>>> return ret; >>>> } >>>> - phy_set_mode_ext(phy, PHY_MODE_UFS_HS_B, host->phy_gear); >>>> + phy_set_mode_ext(phy, mode, host->phy_gear); >>>> /* power on phy - start serdes and phy's power and clocks */ >>>> ret = phy_power_on(phy); >>>> -- >>>> 2.7.4 >>>> >>> >
On Tue, Nov 28, 2023 at 06:59:39PM +0800, Can Guo wrote: > > > On 11/28/2023 6:55 PM, Manivannan Sadhasivam wrote: > > On Tue, Nov 28, 2023 at 03:48:02PM +0800, Can Guo wrote: > > > Hi Mani, > > > > > > On 11/28/2023 1:55 PM, Manivannan Sadhasivam wrote: > > > > On Thu, Nov 23, 2023 at 12:46:26AM -0800, Can Guo wrote: > > > > > Qcom UFS hosts, with HW ver 5, can only support up to HS-G5 Rate-A due to > > > > > HW limitations. If the HS-G5 PHY gear is used, update host_params->hs_rate > > > > > to Rate-A, so that the subsequent power mode changes shall stick to Rate-A. > > > > > > > > > > Signed-off-by: Can Guo <quic_cang@quicinc.com> > > > > > > > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > > > > > > > One question below... > > > > > > > > > --- > > > > > drivers/ufs/host/ufs-qcom.c | 18 +++++++++++++++++- > > > > > 1 file changed, 17 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c > > > > > index 9613ad9..6756f8d 100644 > > > > > --- a/drivers/ufs/host/ufs-qcom.c > > > > > +++ b/drivers/ufs/host/ufs-qcom.c > > > > > @@ -442,9 +442,25 @@ static u32 ufs_qcom_get_hs_gear(struct ufs_hba *hba) > > > > > static int ufs_qcom_power_up_sequence(struct ufs_hba *hba) > > > > > { > > > > > struct ufs_qcom_host *host = ufshcd_get_variant(hba); > > > > > + struct ufs_host_params *host_params = &host->host_params; > > > > > struct phy *phy = host->generic_phy; > > > > > + enum phy_mode mode; > > > > > int ret; > > > > > + /* > > > > > + * HW ver 5 can only support up to HS-G5 Rate-A due to HW limitations. > > > > > + * If the HS-G5 PHY gear is used, update host_params->hs_rate to Rate-A, > > > > > + * so that the subsequent power mode change shall stick to Rate-A. > > > > > + */ > > > > > + if (host->hw_ver.major == 0x5) { > > > > > + if (host->phy_gear == UFS_HS_G5) > > > > > + host_params->hs_rate = PA_HS_MODE_A; > > > > > + else > > > > > + host_params->hs_rate = PA_HS_MODE_B; > > > > > > > > Is this 'else' part really needed? Since there wouldn't be any 2nd init, I think > > > > we can skip that. > > > > > > We need it because, even there is only one init, if a UFS3.1 device is > > > attached, phy_gear is given as UFS_HS_G4 in ufs_qcom_set_phy_gear(), hence > > > we need to put the UFS at HS-G4 Rate B, not Rate A. > > > > > > > But the default hs_rate is PA_HS_MODE_B only and the else condition would be not > > needed for the 1st init. > > You are right, but still we need this in case the UFS device version is not > populated, meaning dual init can also happen to SM8550. We need to apply the > right hs_rate in case the 2nd init asks for HS_G4. > Hmm, yeah I missed that corner case. This is fine. - Mani > Thanks, > Can Guo. > > > > > - Mani > > > > > Thanks, > > > Can Guo. > > > > > > > > > > > - Mani > > > > > > > > > + } > > > > > + > > > > > + mode = host_params->hs_rate == PA_HS_MODE_B ? PHY_MODE_UFS_HS_B : PHY_MODE_UFS_HS_A; > > > > > + > > > > > /* Reset UFS Host Controller and PHY */ > > > > > ret = ufs_qcom_host_reset(hba); > > > > > if (ret) > > > > > @@ -459,7 +475,7 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba) > > > > > return ret; > > > > > } > > > > > - phy_set_mode_ext(phy, PHY_MODE_UFS_HS_B, host->phy_gear); > > > > > + phy_set_mode_ext(phy, mode, host->phy_gear); > > > > > /* power on phy - start serdes and phy's power and clocks */ > > > > > ret = phy_power_on(phy); > > > > > -- > > > > > 2.7.4 > > > > > > > > > > >
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c index 9613ad9..6756f8d 100644 --- a/drivers/ufs/host/ufs-qcom.c +++ b/drivers/ufs/host/ufs-qcom.c @@ -442,9 +442,25 @@ static u32 ufs_qcom_get_hs_gear(struct ufs_hba *hba) static int ufs_qcom_power_up_sequence(struct ufs_hba *hba) { struct ufs_qcom_host *host = ufshcd_get_variant(hba); + struct ufs_host_params *host_params = &host->host_params; struct phy *phy = host->generic_phy; + enum phy_mode mode; int ret; + /* + * HW ver 5 can only support up to HS-G5 Rate-A due to HW limitations. + * If the HS-G5 PHY gear is used, update host_params->hs_rate to Rate-A, + * so that the subsequent power mode change shall stick to Rate-A. + */ + if (host->hw_ver.major == 0x5) { + if (host->phy_gear == UFS_HS_G5) + host_params->hs_rate = PA_HS_MODE_A; + else + host_params->hs_rate = PA_HS_MODE_B; + } + + mode = host_params->hs_rate == PA_HS_MODE_B ? PHY_MODE_UFS_HS_B : PHY_MODE_UFS_HS_A; + /* Reset UFS Host Controller and PHY */ ret = ufs_qcom_host_reset(hba); if (ret) @@ -459,7 +475,7 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba) return ret; } - phy_set_mode_ext(phy, PHY_MODE_UFS_HS_B, host->phy_gear); + phy_set_mode_ext(phy, mode, host->phy_gear); /* power on phy - start serdes and phy's power and clocks */ ret = phy_power_on(phy);