Re: [PATCH 08/13] phy: qcom-qmp-pcie: drop power-down delay config

From: Dmitry Baryshkov
Date: Tue Oct 11 2022 - 10:04:15 EST

On 11/10/2022 16:53, Johan Hovold wrote:
On Tue, Oct 11, 2022 at 04:46:53PM +0300, Dmitry Baryshkov wrote:
On 11/10/2022 16:14, Johan Hovold wrote:
The power-down delay was included in the first version of the QMP driver
as an optional delay after powering on the PHY (using
POWER_DOWN_CONTROL) and just before starting it. Later changes modified
this sequence by powering on before initialising the PHY, but the
optional delay stayed where it was (i.e. before starting the PHY).

The vendor driver does not use a delay before starting the PHY and this
is likely not needed on any platform unless there is a corresponding
delay in the vendor kernel init sequence tables (i.e. in devicetree).

Let's keep the delay for now, but drop the redundant delay period
configuration while increasing the unnecessarily low timer slack

Actually, the vendor driver does this 995..1005 sleep. But contrary to
our driver it does that after programming whole PHY init sequence, which
includes SW_RESET / START_CTL, but before programming the pipe clocks.

Right, it does it after starting the PHY which means that you don't have
to poll for as long for the PHY status.

It's a different delay entirely.

No-no-no. The 995-1005 delay was added guess for which SoC? For ipq8074, where the config tables contain the ugly CFG_L writes for SW_RESET / START_CTRL. So, it is the same delay, but added by somebody who didn't care enough. The original 10-11 delay is a completely different story, you are correct here.

Thus, I'd say, the PCIe delay should be moved after the registers programming.

I think we can either drop this delay completely, or move it before

It definitely shouldn't be used for any new platforms, but I opted for
the conservative route of keeping it in case some of the older platforms
actually do need it.

My bet is that this is all copy-paste cruft that could be removed, but
I'd rather do that as a separate follow-on change. Perhaps after testing
some more SoC after removing the delay.

SC8280XP certainly doesn't need it.

I think in our case this delay just falls into status polling. We'd probably need it, if we'd add the noretain handling.


With best wishes