Re: [v8,3/7] PCI: mediatek-gen3: Add MediaTek Gen3 driver for MT8192

[Jianjun Wang <jianjun.wang@mediatek.com>]

On Thu, 2021-03-11 at 13:38 +0100, Pali Rohár wrote:
> On Wednesday 24 February 2021 14:11:28 Jianjun Wang wrote:
> > +static int mtk_pcie_startup_port(struct mtk_pcie_port *port)
> > +{
> ...
> > +
> > +	/* Delay 100ms to wait the reference clocks become stable */
> > +	msleep(100);
> > +
> > +	/* De-assert PERST# signal */
> > +	val &= ~PCIE_PE_RSTB;
> > +	writel_relaxed(val, port->base + PCIE_RST_CTRL_REG);
> 
> Hello! This is a new driver which introduce yet another custom timeout
> prior PERST# signal for PCIe card is de-asserted. Timeouts for other
> drivers I collected in older email [2].
> 
> Please look at my email [1] about PCIe Warm Reset if you have any clue
> about it. Lorenzo and Rob already expressed that this timeout should not
> be driver specific. But nobody was able to "decode" and "understand"
> PCIe spec yet about these timeouts.

Hi Pali,

I think this is more like a platform specific timeout, which is used to
wait for the reference clocks to become stable and finish the reset flow
of HW blocks.

Here is the steps to start a link training in this HW:

1. Assert all reset signals which including the transaction layer, PIPE
interface and internal bus interface;

2. De-assert reset signals except the PERST#, this will make the
physical layer active and start to output the reference clock, but the
EP device remains in the reset state.
   Before releasing the PERST# signal, the HW blocks needs at least 10ms
to finish the reset flow, and ref-clk needs about 30us to become stable.

3. De-assert PERST# signal, wait LTSSM enter L0 state.

This 100ms timeout is reference to TPVPERL in the PCIe CEM spec. Since
we are in the kernel stage, the power supply has already stabled, this
timeout may not take that long.

> > +
> > +	/* Check if the link is up or not */
> > +	err = readl_poll_timeout(port->base + PCIE_LINK_STATUS_REG, val,
> > +				 !!(val & PCIE_PORT_LINKUP), 20,
> > +				 50 * USEC_PER_MSEC);
> 
> IIRC, you need to wait at least 100ms after de-asserting PERST# signal
> as it is required by PCIe specs and also because experiments proved that
> some Compex wifi cards (e.g. WLE900VX) are not detected if you do not
> wait this minimal time.

Yes, this should be 100ms, I will fix it at next version, thanks for
your review.

Thanks.
> 
> > +	if (err) {
> > +		val = readl_relaxed(port->base + PCIE_LTSSM_STATUS_REG);
> > +		dev_err(port->dev, "PCIe link down, ltssm reg val: %#x\n", val);
> > +		return err;
> > +	}
> 
> [1] - https://lore.kernel.org/linux-pci/20210310110535.zh4pnn4vpmvzwl5q@pali/
> [2] - https://lore.kernel.org/linux-pci/20200424092546.25p3hdtkehohe3xw@pali/