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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aRJEDEEJr9Ic-RKN@fedora>
Date: Mon, 10 Nov 2025 20:59:08 +0100
From: Niklas Cassel <cassel@...nel.org>
To: Manivannan Sadhasivam <mani@...nel.org>
Cc: Shawn Lin <shawn.lin@...k-chips.com>, FUKAUMI Naoki <naoki@...xa.com>,
	Damien Le Moal <dlemoal@...nel.org>,
	Anand Moon <linux.amoon@...il.com>, linux-pci@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org,
	Dragan Simic <dsimic@...jaro.org>,
	Lorenzo Pieralisi <lpieralisi@...nel.org>,
	Krzysztof Wilczyński <kw@...ux.com>,
	Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
	Heiko Stuebner <heiko@...ech.de>
Subject: Re: [RESEND] Re: [PATCH] PCI: dw-rockchip: Skip waiting for link up

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?

Kind regards,
Niklas