[PATCH v2 0/4] PCI: dwc: Link Up IRQ fixes

[Laszlo Fiat]

Hello,
-------- Original Message --------
On 16/05/2025 12:00, Niklas Cassel <cassel at kernel.org> wrote:

>  On Thu, May 15, 2025 at 05:33:41PM +0000, Laszlo Fiat wrote:
>  > I am the one experiencing the issue with my Orange PI 3B (RK3566, 8 GB RAM) and a PLEXTOR PX-256M8PeGN NVMe SSD.
>  >
>  > I first detected the problem while upgrading from 6.13.8 to 6.14.3, that my system cannot find the NVME SSD which contains the rootfs. After reverting the two patches:
>  >
>  > - ec9fd499b9c6 ("PCI: dw-rockchip: Don't wait for link since we can detect Link Up")
>  > - 0e0b45ab5d77 ("PCI: dw-rockchip: Enumerate endpoints based on dll_link_up IRQ")
>  >
>  > my system booted fine again.
>  > After that I tested the patches sent by Niklas in this thread, which fixed the issue, so I sent Tested-by.
>  >
>  > I did another test Today with 6.15.0-rc6, which in itself does not find my SSD. Niklas asked me to test with these
>  >
>  > - revert ec9fd499b9c6 ("PCI: dw-rockchip: Don't wait for link since we can detect Link Up")
>  > - revert 0e0b45ab5d77 ("PCI: dw-rockchip: Enumerate endpoints based on dll_link_up IRQ")
>  > - apply the following patch:
>  >
>  > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
>  > index b3615d125942..5dee689ecd95 100644
>  > --- a/drivers/pci/controller/dwc/pcie-designware.c
>  > +++ b/drivers/pci/controller/dwc/pcie-designware.c
>  > @@ -692,7 +692,7 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
>  >                 if (dw_pcie_link_up(pci))
>  >                         break;
>  >
>  > -               msleep(LINK_WAIT_SLEEP_MS);
>  > +               usleep_range(100, 200);
>  >         }
>  >
>  >         if (retries >= LINK_WAIT_MAX_RETRIES) {
>  >
>  >
>  > which restores the original behaviour to wait for link-up, then shorten the time. This resulted again a non booting system, this time with "Phy link never came up" error message.
>  
>  That message was unexpected.
>  
>  What I expected to happen was that the link would come up, but by reducing
>  delay between "link is up" and device is accessed (from 90 ms -> 100 us),
>  I was that you would see the same problem on "older" kernels as you do with
>  the "link up IRQ" patches (which originally had no delay, but this series
>  basically re-added the same delay (PCIE_T_RRS_READY_MS, 100 ms) as we had
>  before (LINK_WAIT_SLEEP_MS, 90 ms).
>  
>  But I see the problem with the test code that I asked you to test to verify
>  that this problem also existed before (if you had a shorter delay).
>  (By reducing the delay, the LINK_WAIT_MAX_RETRIES also need to be bumped.)
>  
>  Could you please test:
>  diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
>  index b3615d125942..5dee689ecd95 100644
>  --- a/drivers/pci/controller/dwc/pcie-designware.c
>  +++ b/drivers/pci/controller/dwc/pcie-designware.c
>  @@ -692,7 +692,7 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
>                  if (dw_pcie_link_up(pci))
>                          break;
>  
>  -               msleep(LINK_WAIT_SLEEP_MS);
>  +               usleep_range(100, 200);
>          }
>  
>          if (retries >= LINK_WAIT_MAX_RETRIES) {
>  diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
>  index 4dd16aa4b39e..8422661b79d5 100644
>  --- a/drivers/pci/controller/dwc/pcie-designware.h
>  +++ b/drivers/pci/controller/dwc/pcie-designware.h
>  @@ -61,7 +61,7 @@
>          set_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
>  
>   /* Parameters for the waiting for link up routine */
>  -#define LINK_WAIT_MAX_RETRIES          10
>  +#define LINK_WAIT_MAX_RETRIES          10000
>   #define LINK_WAIT_SLEEP_MS             90
>  
>   /* Parameters for the waiting for iATU enabled routine */
>  
>  
>  On top of an old kernel instead?
>

I have compiled a vanilla 6.12.28, that booted fine, as expeced. Then compiled a  version with the patch directly above.

>  We expect the link to come up, but that you will not be able to mount rootfs.
>  

That is exactly what happened. 

>  If that is the case, we are certain that the this patch series is 100% needed
>  for your device to have the same functional behavior as before.

That is the case.

Bye,

Laszlo Fiat 
>  
>  
>  Kind regards,
>  Niklas
>