00:56 <pgwipeout[m]> Someone suspected that the grf clock wasn't being enabled, as such the grf wasn't being updated with the delay values.

00:59 <pgwipeout[m]> Ohhhhhh

00:59 <pgwipeout[m]> They are different drivers.

01:02 <pgwipeout[m]> We ran into this issue when porting, the rk356x is gmac4 core, where the rk3399 and previous used the older core.

01:11 <pgwipeout[m]> Okay I know what's wrong

01:12 <pgwipeout[m]> The rgmii delays are set on calling runtime powerup.

01:12 <pgwipeout[m]> They should be set during initial probe.

01:13 <pgwipeout[m]> They aren't seeing the issue because we have been flirting with rgmii-id mode on rk356x.

01:15 <pgwipeout[m]> I'll move the delay set into a separate function that can be called during probe then again during runtime power enable (to cover resuming from sleep when grf may have been lost).

01:19 <pgwipeout[m]> Never mind, powerup is already called during probe, continuing.

01:22 <pgwipeout[m]> Oh, wow.

01:28 <pgwipeout[m]> So during probe we called rk_gmac_powerup, which set the ids. During rk_gmac_powerup runtime_pm was enabled then synced, which called rk_gmac_resume. It would check if suspended was set, which it isn't, then calls stmmac_resume, which sets up everything, before we call stmmac_dvr_probe to actually set everything up.

01:31 <pgwipeout[m]> So uh, if this broke rk3399 by taking this away, there's something much deeper that's broken here.

01:36 <pgwipeout[m]> Why are the usbdrd devices the only devices that take a reference to the grf clock?

02:10 <punit> I too suspected that rk_gmac_resume() was reached indirectly when pm_runtime_get_sync() is called but some tracing suggests that's not the case.

02:21 <pgwipeout[m]> I'll have to poke a few places in the code tomorrow. I still suspect the delays are the issue here, but how or why isn't obvious to my exhausted brain.

02:25 <punit> Ack - I'll pick up where I left off later in the evening

14:00 <CounterPillow> I recommend rewriting anything that looks like it's abusing resume callbacks for initial enablement, the I2S/TDM driver did this and I just rewrote all that code instead of trying to understand whether it was technically correct.

14:27 <punit> In this case it looks like the resume callbacks lead to enabling the associated power domain - which now occur later(?) and cause problems.

14:33 <punit> Hopefully somebody looking at this can pick up from there...

17:05 <pgwipeout[m]> I'm building the current kernel for experimentation now.