Re: [PATCH v3 1/2] phy: cadence: cdns-dphy: Fix PLL lock and O_CMN_READY polling

[Devarsh Thakkar]

Hello,

On 20/06/25 20:05, Devarsh Thakkar wrote:
> Hi Tomi,
>
> Thanks for the review.
>
> On 26/05/25 16:29, Tomi Valkeinen wrote:
> > Hi,
> >
> > On 02/05/2025 06:34, Devarsh Thakkar wrote:
> > > PLL lockup and O_CMN_READY assertion can only happen after common state
> > > machine gets enabled (by programming DPHY_CMN_SSM register), but 
> > > driver was
> > > polling them before the common state machine was enabled. To fix this :
> > >
> > > - Add new function callbacks for polling on PLL lock and O_CMN_READY
> > >    assertion.
> > > - As state machine and clocks get enabled in power_on callback only, 
> > > move
> > >    the clock related programming part from configure callback to 
> > > power_on
> > >    callback and poll for the PLL lockup and O_CMN_READY assertion after
> > >    state machine gets enabled.
> > > - The configure callback only saves the PLL configuration received 
> > > from the
> > >    client driver which will be applied later on in power_on callback.
> > > - Add checks to ensure configure is called before power_on and state
> > >    machine is in disabled state before power_on callback is called.
> > > - Disable state machine in power_off so that client driver can
> > >    re-configure the PLL by following up a power_off, configure, power_on
> > >    sequence.
> >
> > Is the DPHY & PLL documented in the TRM somewhere?
>
> I had got this information from cadence support. But I think it is also 
> documented in J721E TRM [1]. DPHY Tx startup sequence is same as DPHY Tx 
> and there is a initialization diagram for the same in J721E TRM [1] 
> referenced at 12.7.2.4.1.2.1 Start-up Sequence Timing Diagram. It shows 
> O_CMN_READY polling at the end after common configuration pin setup and 
> Table 12-1533. Common Configuration-Related Setup mentions state machine 
> enable part under the common configuration setup which happens before 
> the polling. And the observations with this patch do sync with 
> understanding as we see PLL locking up faster without any timeout which 
> was the case before this patch.
>
> > I just find the sequence a bit odd. For example, you wait for the PLL to
> > lock, and after that, you enable the PLL ref clock. Maybe I'm missing
> > something here, but... that should not work.
>
> I think it's my bad, but it works somehow. But it makes sense to enable 
> the clock before we start polling for PLL lock, so will update it in 
> next revision.

Sorry, had missed to add the reference :

[1]: https://www.ti.com/lit/zip/spruil1

Regards
Devarsh

> >   Tomi
> Regards
> Devarsh