[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6eafbd2f-e2cf-4d4b-9432-026eb9c7a01a@quicinc.com>
Date: Thu, 22 May 2025 03:17:33 +0530
From: Nitin Rawat <quic_nitirawa@...cinc.com>
To: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
CC: <vkoul@...nel.org>, <kishon@...nel.org>,
<James.Bottomley@...senpartnership.com>, <martin.petersen@...cle.com>,
<bvanassche@....org>, <andersson@...nel.org>,
<neil.armstrong@...aro.org>, <dmitry.baryshkov@....qualcomm.com>,
<konrad.dybcio@....qualcomm.com>, <quic_rdwivedi@...cinc.com>,
<quic_cang@...cinc.com>, <linux-arm-msm@...r.kernel.org>,
<linux-phy@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
<linux-scsi@...r.kernel.org>
Subject: Re: [PATCH V5 04/11] phy: qcom-qmp-ufs: Refactor UFS PHY reset
On 5/21/2025 6:56 PM, Manivannan Sadhasivam wrote:
> On Thu, May 15, 2025 at 09:57:15PM +0530, Nitin Rawat wrote:
>> Refactor the UFS PHY reset handling to parse the reset logic only once
>> during initialization, instead of every resume.
>>
>> As part of this change, move the UFS PHY reset parsing logic from
>> qmp_phy_power_on to the new qmp_ufs_phy_init function.
>>
Sure. I'll update the commit text in next patchset.
>
> More importantly, you are introducing the phy_ops::init callback, which
> should've been mentioned.
>
>> Co-developed-by: Ram Kumar Dwivedi <quic_rdwivedi@...cinc.com>
>> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@...cinc.com>
>> Signed-off-by: Nitin Rawat <quic_nitirawa@...cinc.com>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
>> ---
>> drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 59 +++++++++++++------------
>> 1 file changed, 31 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
>> index ade8e9c4b9ae..33d238cf49aa 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
>> @@ -1800,38 +1800,11 @@ static int qmp_ufs_com_exit(struct qmp_ufs *qmp)
>> static int qmp_ufs_power_on(struct phy *phy)
>> {
>> struct qmp_ufs *qmp = phy_get_drvdata(phy);
>> - const struct qmp_phy_cfg *cfg = qmp->cfg;
>> int ret;
>> dev_vdbg(qmp->dev, "Initializing QMP phy\n");
>>
>> - if (cfg->no_pcs_sw_reset) {
>> - /*
>> - * Get UFS reset, which is delayed until now to avoid a
>> - * circular dependency where UFS needs its PHY, but the PHY
>> - * needs this UFS reset.
>> - */
>> - if (!qmp->ufs_reset) {
>> - qmp->ufs_reset =
>> - devm_reset_control_get_exclusive(qmp->dev,
>> - "ufsphy");
>> -
>> - if (IS_ERR(qmp->ufs_reset)) {
>> - ret = PTR_ERR(qmp->ufs_reset);
>> - dev_err(qmp->dev,
>> - "failed to get UFS reset: %d\n",
>> - ret);
>> -
>> - qmp->ufs_reset = NULL;
>> - return ret;
>> - }
>> - }
>> - }
>> -
>> ret = qmp_ufs_com_init(qmp);
>> - if (ret)
>> - return ret;
>> -
>> - return 0;
>> + return ret;
>
> This can't be:
> return qmp_ufs_com_init; ?
This is already taken care in next patch (#6) of this series.
Thanks,
Nitin
>
> - Mani
>
Powered by blists - more mailing lists