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

Dmitry Baryshkov
2025-04-16T12:14:05Z

On Wed, 16 Apr 2025 at 12:08, Nitin Rawat <quic_nitirawa@quicinc.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@quicinc.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?

>
> 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

--
With best wishes
Dmitry