Re: [PATCH v3 0/4] PCI: dwc: Rework the error handling of dw_pcie_wait_for_link() API

[Manivannan Sadhasivam]

On Mon, Jan 05, 2026 at 11:04:21AM +0100, Vincent Guittot wrote:
> On Tue, 30 Dec 2025 at 16:07, Manivannan Sadhasivam via B4 Relay
> <devnull+manivannan.sadhasivam.oss.qualcomm.com@xxxxxxxxxx> wrote:
> >
> > Hi,
> >
> > This series reworks the dw_pcie_wait_for_link() API to allow the callers to
> > detect the absence of the device on the bus and skip the failure.
> >
> > Compared to v2, I've reworked the patch 2 to improve the API further and
> > dropped the patch 1 that got applied (hence changed the subject). I've also
> > modified the error code based on the feedback in v2 to return -ENODEV if device
> > is not detected on the bus and -ETIMEDOUT otherwise. This allows the callers to
> > skip the failure if device is not detected and handle error for other failure.
> >
> > Testing
> > =======
> >
> > Tested this series on Rb3Gen2 board without powering on the PCIe switch. Now the
> > dw_pcie_wait_for_link() API prints:
> >
> >         qcom-pcie 1c08000.pcie: Device not found
> >
> > Instead of the previous log:
> >
> >         qcom-pcie 1c08000.pcie: Phy link never came up
> 
> I tested the patchset with s32g399a-rdb3 and during the resume, I have:
> 
> [  460.255927] s32g-pcie 44100000.pcie: Device not found
> [  460.256021] s32g-pcie 44100000.pcie: PM: dpm_run_callback():
> s32g_pcie_resume_noirq returns -19
> [  460.256278] s32g-pcie 44100000.pcie: PM: failed to resume noirq: error -19
> 
> I was not expecting more lines than the 1st line: Device not found,
> like the init
> 
> [    2.668921] s32g-pcie 44100000.pcie: Device not found
> [    2.675342] s32g-pcie 44100000.pcie: PCI host bridge to bus 0001:00
> 
> where with skip the -ENODEV case if dw_pcie_wait_for_link() fails
> 
> Should we skip the -ENODEV case in dw_pcie_resume_noirq() too ?
> 

I proposed it initially, but then there were concerns raised that there is a
possibility that the device could be removed during suspend and we would fail to
detect it.

But I think that could be handled by checking for 'pci_bus::devices' list. If
this list is empty, then we for sure know that there was no device connected to
the bus before suspend. So if dw_pcie_wait_for_link() returns -ENODEV, and then
this list is also empty, we can safely ignore the failure.

I'll do it in v4.

- Mani

> >
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
> > ---
> > Changes in v3:
> > - Dropped patch 1 that got appplied
> > - Reworked the error handling of dw_pcie_wait_for_link() API further
> > - Link to v2: https://lore.kernel.org/r/20251218-pci-dwc-suspend-rework-v2-0-5a7778c6094a@xxxxxxxxxxxxxxxx
> >
> > Changes in v2:
> > - Changed the logic to check for Detect.Quiet/Active states
> > - Collected tags and rebased on top of v6.19-rc1
> > - Link to v1: https://lore.kernel.org/r/20251119-pci-dwc-suspend-rework-v1-0-aad104828562@xxxxxxxxxxxxxxxx
> >
> > ---
> > Manivannan Sadhasivam (4):
> >       PCI: dwc: Return -ENODEV from dw_pcie_wait_for_link() if device is not found
> >       PCI: dwc: Rename and move ltssm_status_string() to pcie-designware.c
> >       PCI: dwc: Rework the error print of dw_pcie_wait_for_link()
> >       PCI: dwc: Only skip the dw_pcie_wait_for_link() failure if it returns -ENODEV
> >
> >  .../pci/controller/dwc/pcie-designware-debugfs.c   | 54 +---------------
> >  drivers/pci/controller/dwc/pcie-designware-host.c  |  6 +-
> >  drivers/pci/controller/dwc/pcie-designware.c       | 75 +++++++++++++++++++++-
> >  drivers/pci/controller/dwc/pcie-designware.h       |  2 +
> >  4 files changed, 80 insertions(+), 57 deletions(-)
> > ---
> > base-commit: 68ac85fb42cfeb081cf029acdd8aace55ed375a2
> > change-id: 20251119-pci-dwc-suspend-rework-8b0515a38679
> >
> > Best regards,
> > --
> > Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
> >
> >

-- 
மணிவண்ணன் சதாசிவம்