On 12/9/24 16:39, Richard Zhu wrote:
> The dw_pcie_suspend_noirq() function currently returns success directly
> if no endpoint (EP) device is connected. However, on some platforms,
> power loss occurs during suspend, causing dw_resume() to do nothing in
> this case. This results in a system halt because the DWC controller is
> not initialized after power-on during resume.
>
> Call deinit() in suspend and init() at resume regardless of whether
> there are EP device connections or not. It is not harmful to perform
> deinit() and init() again for the no power-off case, and it keeps the
> code simple and consistent in logic.
>
> Fixes: 4774faf854f5 ("PCI: dwc: Implement generic suspend/resume functionality")
> Signed-off-by: Richard Zhu <hongxing.zhu@xxxxxxx>
> Reviewed-by: Frank Li <Frank.Li@xxxxxxx>
> ---
> .../pci/controller/dwc/pcie-designware-host.c | 30 +++++++++----------
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index f882b11fd7b94..11563402c571b 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -982,23 +982,23 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
> if (dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKCTL) & PCI_EXP_LNKCTL_ASPM_L1)
> return 0;
>
> - if (dw_pcie_get_ltssm(pci) <= DW_PCIE_LTSSM_DETECT_ACT)
> - return 0;
> -
> - if (pci->pp.ops->pme_turn_off)
> - pci->pp.ops->pme_turn_off(&pci->pp);
> - else
> - ret = dw_pcie_pme_turn_off(pci);
> + /* Only send out PME_TURN_OFF when PCIE link is up */
> + if (dw_pcie_get_ltssm(pci) > DW_PCIE_LTSSM_DETECT_ACT) {
> + if (pci->pp.ops->pme_turn_off)
> + pci->pp.ops->pme_turn_off(&pci->pp);
> + else
> + ret = dw_pcie_pme_turn_off(pci);
>
> - if (ret)
> - return ret;
> + if (ret)
> + return ret;
Same comment as for patch 3. This "if (ret) return ret;" can go inside the else.
It is harmless, but there is also no point in having it here.
>
> - ret = read_poll_timeout(dw_pcie_get_ltssm, val, val == DW_PCIE_LTSSM_L2_IDLE,
> - PCIE_PME_TO_L2_TIMEOUT_US/10,
> - PCIE_PME_TO_L2_TIMEOUT_US, false, pci);
> - if (ret) {
> - dev_err(pci->dev, "Timeout waiting for L2 entry! LTSSM: 0x%x\n", val);
> - return ret;
> + ret = read_poll_timeout(dw_pcie_get_ltssm, val, val == DW_PCIE_LTSSM_L2_IDLE,
> + PCIE_PME_TO_L2_TIMEOUT_US/10,
> + PCIE_PME_TO_L2_TIMEOUT_US, false, pci);
> + if (ret) {
> + dev_err(pci->dev, "Timeout waiting for L2 entry! LTSSM: 0x%x\n", val);
> + return ret;
> + }
> }
>
> if (pci->pp.ops->deinit)
--
Damien Le Moal
Western Digital Research
