linux-kernel - Re: [PATCH V3 4/9] phy: qcom-qmp-ufs: Refactor UFS PHY reset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1a623099-40bb-4884-8d93-132138a4150b@quicinc.com>
Date: Wed, 16 Apr 2025 17:56:44 +0530
From: Nitin Rawat <quic_nitirawa@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
CC: <vkoul@...nel.org>, <kishon@...nel.org>,
        <manivannan.sadhasivam@...aro.org>,
        <James.Bottomley@...senpartnership.com>, <martin.petersen@...cle.com>,
        <bvanassche@....org>, <bjorande@...cinc.com>,
        <neil.armstrong@...aro.org>, <konrad.dybcio@....qualcomm.com>,
        <quic_rdwivedi@...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 V3 4/9] phy: qcom-qmp-ufs: Refactor UFS PHY reset



On 4/16/2025 5:43 PM, Dmitry Baryshkov wrote:
> On Wed, 16 Apr 2025 at 12:08, Nitin Rawat <quic_nitirawa@...cinc.com> wrote:
>>
>>
>>
>> On 4/15/2025 2:59 PM, Dmitry Baryshkov wrote:
>>> On 14/04/2025 23:34, Nitin Rawat wrote:
>>>>
>>>>
>>>> On 4/11/2025 4:38 PM, Dmitry Baryshkov wrote:
>>>>> On Fri, 11 Apr 2025 at 13:50, Nitin Rawat <quic_nitirawa@...cinc.com>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 4/11/2025 1:38 AM, Dmitry Baryshkov wrote:
>>>>>>> On Thu, Apr 10, 2025 at 02:30:57PM +0530, Nitin Rawat wrote:
>>>>>>>> Refactor the UFS PHY reset handling to parse the reset logic only
>>>>>>>> once
>>>>>>>> during probe, instead of every resume.
>>>>>>>>
>>>>>>>> Move the UFS PHY reset parsing logic from qmp_phy_power_on to
>>>>>>>> qmp_ufs_probe to avoid unnecessary parsing during resume.
>>>>>>>
>>>>>>> How did you solve the circular dependency issue being noted below?
>>>>>>
>>>>>> Hi Dmitry,
>>>>>> As part of my patch, I moved the parsing logic from qmp_phy_power_on to
>>>>>> qmp_ufs_probe to avoid unnecessary parsing during resume. I'm uncertain
>>>>>> about the circular dependency issue and whether if it still exists.
>>>>>
>>>>> It surely does. The reset controller is registered in the beginning of
>>>>> ufs_qcom_init() and the PHY is acquired only a few lines below. It
>>>>> creates a very small window for PHY driver to probe.
>>>>> Which means, NAK, this patch doesn't look acceptable.
>>>>
>>>> Hi Dmitry,
>>>>
>>>> Thanks for pointing this out. I agree that it leaves very little time
>>>> for the PHY to probe, which may cause issues with targets where
>>>> no_pcs_sw_reset is set to true.
>>>>
>>>> As an experiment, I kept no_pcs_sw_reset set to true for the SM8750,
>>>> and it caused bootup probe issues in some of the iterations I ran.
>>>>
>>>> To address this, I propose updating the patch to move the
>>>> qmp_ufs_get_phy_reset call to phy_calibrate, just before the
>>>> reset_control_assert call.
>>>
>>> Will it cause an issue if we move it to phy_init() instead of
>>> phy_calibrate()?
>>
>> Hi Dmitry,
>>
>> Thanks for suggestion.
>> Phy_init is invoked before phy_set_mode_ext and ufs_qcom_phy_power_on,
>> whereas calibrate is called after ufs_qcom_phy_power_on. Keeping the UFS
>> PHY reset in phy_calibrate introduces relatively more delay, providing
>> more buffer time for the PHY driver probe, ensuring the UFS PHY reset is
>> handled correctly the first time.
> 
> We are requesting the PHY anyway, so the PHY driver should have probed
> well before phy_init() call. I don't get this comment.
> 
>>
>> Moving the calibration to phy_init shouldn't cause any issues. However,
>> since we currently don't have an initialization operations registered
>> for init, we would need to add a new PHY initialization ops if we decide
>> to move it to phy_init.
> 
> Yes. I don't see it as a problem. Is there any kind of an issue there?

No issues. In my next patchset, I would add a new init ops registered 
for qcom phy and move get ufs phy reset handler to it.

Regards,
Nitin

> 
>>
>> Please let me know if this looks fine to you, or if you have any
>> suggestions. I am open to your suggestions.
> 
> phy_init() callback
>