Re: [RESEND] Re: [PATCH] PCI: dw-rockchip: Skip waiting for link up

[FUKAUMI Naoki]

Hi Niklas,

On 11/11/25 04:59, Niklas Cassel wrote:
> On Mon, Nov 10, 2025 at 09:23:02PM +0530, Manivannan Sadhasivam wrote:
> > > Considering what Shawn says, that the switch gets enumerated properly
> > > if we simply add a msleep() in ->start_link(), which will be called
> > > by dw_pcie_host_init() before pci_host_probe() is called...
> >
> > Yes, that delay probably gives enough time for the link up IRQ to kick in before
> > the initial bus scan happens.
>
> I think that the problem is that even for platforms with link up IRQ,
> we will always do:
> 1) dw_pcie_start_link() (if (!dw_pcie_link_up()))
> 2) pci_host_probe() from dw_pcie_host_init(), this will enumerate the bus
> 3) pci_rescan_bus() from the Link Up IRQ handler
>
> Thus, in 2, when enumerating the bus, without performing any of the delays
> mandated by the PCIe spec, it still seems possible to detect a device (it
> must have been really quick to initialize), and to communicate with that
> device, however since we have not performed the delays mandated by the spec,
> it appears that the device might not yet behave properly.
>
> Hence my suggestion to never call pci_host_probe() in dw_pcie_host_init()
> for platforms with Link Up IRQ.
>
> At least for pcie-dw-rockchip.c, we only unmask the Link Up IRQ after
> dw_pcie_host_init() has returned, so I think that your theory: that Shawn's
> suggested delay causes the Link Up IRQ to kick in before the initial bus
> scan, is incorrect. (Since the IRQ should not be able to trigger until
> dw_pcie_host_init() has returned.)
>
>
> > > ...we already have a delay in the link up IRQ handler, before calling
> > > pci_rescan_bus().
> >
> > That delay won't help in this case.
>
> Sure, I was just saying that even though Shawn's patch made things work,
> it seems incorrect, as we do not want to add "the same delay" that we
> already have in the Link Up IRQ. (The delay in the Link Up IRQ should be
> the only one.)
>
>
> > > I think a better solution would be something like:
>
> (snip)
>
> > This solution will work as long as the PCIe device is powered ON before
> > start_link(). For CEM and M.2 Key M connectors, the host controller can power
> > manage the components. But for other specifications/keys requiring custom power
> > management, a separate driver would be needed.
> >
> > That's why I suggested using pwrctrl framework as it can satisfy both usecases.
> > However, as I said, it needs a bit of rework and I'm close to submitting it.
> >
> > But until that gets merged, either we need to revert your link up IRQ change or
> > have the below patch. IMO, the revert seems simple.
>
> Using pwrctrl framework once it can handle this use case sounds good to me.
>
> FUKAUMI, could you please send a revert of the two patches?

Well, I cannot write a detailed explanation...

Best regards,

--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.

> Kind regards,
> Niklas