On 6/11/25 19:51, Niklas Cassel wrote:
> Commit ec9fd499b9c6 ("PCI: dw-rockchip: Don't wait for link since we can
> detect Link Up") changed so that we no longer call dw_pcie_wait_for_link(),
> and instead enumerate the bus directly after receiving the Link Up IRQ.
>
> This means that there is no longer any delay between link up and the bus
> getting enumerated.
>
> As per PCIe r6.0, sec 6.6.1, a Downstream Port that supports Link speeds
> greater than 5.0 GT/s, software must wait a minimum of 100 ms after Link
> training completes before sending a Configuration Request.
>
> Add this delay in the threaded link up IRQ handler in order to satisfy
> the requirements of the PCIe spec.
>
> Laszlo Fiat reported (off-list) that his PLEXTOR PX-256M8PeGN NVMe SSD is
> no longer functional, and simply reverting commit ec9fd499b9c6 ("PCI:
> dw-rockchip: Don't wait for link since we can detect Link Up") makes his
> SSD functional again. Adding the 100 ms delay as required by the spec also
> makes the SSD functional again.
>
> Cc: Laszlo Fiat <laszlo.fiat@xxxxxxxxx>
> Fixes: ec9fd499b9c6 ("PCI: dw-rockchip: Don't wait for link since we can detect Link Up")
> Signed-off-by: Niklas Cassel <cassel@xxxxxxxxxx>
> ---
> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 93171a392879..a941a239cbfc 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -459,6 +459,13 @@ static irqreturn_t rockchip_pcie_rc_sys_irq_thread(int irq, void *arg)
> if (reg & PCIE_RDLH_LINK_UP_CHGED) {
> if (rockchip_pcie_link_up(pci)) {
> dev_dbg(dev, "Received Link up event. Starting enumeration!\n");
Should maybe this message be moved after the sleep ?
> + /*
> + * As per PCIe r6.0, sec 6.6.1, a Downstream Port that
> + * supports Link speeds greater than 5.0 GT/s, software
> + * must wait a minimum of 100 ms after Link training
> + * completes before sending a Configuration Request.
> + */
> + msleep(PCIE_T_RRS_READY_MS);
> /* Rescan the bus to enumerate endpoint devices */
> pci_lock_rescan_remove();
> pci_rescan_bus(pp->bridge->bus);
Other than that, looks good to me.
Reviewed-by: Damien Le Moal <dlemoal@xxxxxxxxxx>
--
Damien Le Moal
Western Digital Research
