Re: [PATCH v9 0/9] PCI: dw-rockchip: add system suspend support

[Shawn Lin <shawn.lin@rock-chips.com>]

Hi Sebasitain,

在 2026/11/05 星期四 21:33, Sebastian Reichel 写道:
> I've been working on fixing up at least basic system suspend support on
> the Rockchip RK3576 platform. This series is a follow-up for Shawn Lin's
> series with feedback from Niklas Cassel and Manivannan Sadhasivam being
> handled as well as some of my own changes fixing up things I noticed.
> 
> Tested boards:
>   * Sige5 (RK3576)
>   * Rock 4D (RK3576)
>   * RK3588 EVB1 (RK3588)
>   * ODROID-M1 (RK3568), Igor Velkov (previous version)
> 
> Somewhat related, RK3588 also needs [0] to fix PCIe IOMMU issues after
> resume. Additionally RK3588 runs into SError on resume due to a USB2 PHY
> problem, which is fixed via [1]. RK3576 does not use PCIe IOMMU and does
> not run into the USB2 PHY problem (at least Sige5 and Rock 4D), so this
> series can easily be tested standalone on that platform.
> 
> [0] https://lore.kernel.org/all/20260929034510.2023173-1-praan@google.com/
> [1] https://lore.kernel.org/linux-phy/20261105-phy-rockchip-inno-usb2-clock-fix-v3-0-b20608c08b94@collabora.com/
> 
> Changes in v9:
> - Link to v8: https://patch.msgid.link/20261105-rockchip-pcie-system-suspend-v8-0-0434b4f373bc@collabora.com
> - Collect Reviewed-by tag from Shawn Lin for pme_turn_off patch
> - Ignore suggestion from Shawn Lin to make pme_turn_off return error
>    codes as out-of-scope. I think it is sensible, but should be done
>    in a separate series as it will touch all glue drivers. (Shawn Lin)
> - Update rockchip_pcie_rc_reset_root_port() to also use the
>    rockchip_pcie_set_controller_mode() helper (Sashiko)
> - Update DLL IRQ unmask helper to also handle instances not involving
>    PCIE_RDLH_LINK_UP_CHGED (Sashiko)
> - Move INTx caching after dw_pcie_suspend_noirq() to avoid a race
>    condition (Sashiko)
> 

You seem to forget to take TB tag from Igor Velkov as he tested your v8.

> Changes in v8:
> - Link to v7: https://patch.msgid.link/20261104-rockchip-pcie-system-suspend-v7-0-6ae6875939a2@collabora.com
> - Drop the final debug patch; I took that over from Rockchip's vendor
>    tree as it seems useful for debug purposes. Using it involves reading
>    the register via /dev/mem. It's not directly tied to actual system PM
>    support itself, so it can be submitted separetely. It's not worth
>    fighting Sashiko for it.
> - In LTSSM control mode patch also replace one occurance that I missed
> - In system PM support patch add guard not suspending an already
>    suspended PCIe devices, which can happen when resume failed following
>    the example of UltraRISC (Sashiko)
> - In the same patch replace 0xffff mask with PCIE_INTR_MASK to avoid
>    overflow issue; the new mask is not functional equivalent as we just
>    work on the lower 8 bit now. The following 8 bit are reserved on
>    RK3588 and RK3576 anyways, though. Thus this effectivly better
>    documents the purpose of the cached value. (Sashiko)
> - Drop incorrect custom PERST# handling in the resume handler (Sashiko)
> - Modify interrupt masking to follow RC instead of EP logic (Sashiko)
> - I've not picked up the Tested-by tag from Igor Velkov on v7
>    as there are some changes (even though I'm fairly sure they
>    don't matter), which add RK3568 to the tested SoC range.
> 
> Changes in v7:
> * Link to v6: https://patch.msgid.link/20260908-rockchip-pcie-system-suspend-v6-0-fbabcca9921b@collabora.com
> * Rebased to v7.4-rc1
> * Collect Reviewed-by tags from Shawn Lin to (mostly) unchanged patches;
>    I skipped some due to changes in v7
> * Keep dev_err_probe() message for rockchip_pcie_phy_init in patch
>    restoring vpcie3v3 regulator handling
> * Avoid dynamic mask for extra flags in "enhanced LTSSM control mode helper"
>    by just adding specific support for the extra flag needed. Apparently
>    some compiler configurations don't like it.
> * Avoid duplicated register define in In pme_turn_off patch
> * Move bit defines to the correct register defines in pme_turn_off patch
> * In broken L1SS resume flag patch move the explanation comment from the
>    code to the commit message (FWIW this comment is not AI generated by
>    me, but was copied from the Qcom implementation).
> * In system PM support patch, set pp->suspend_poweroff to fix the issue
>    identified by Igor
> * In system PM support patch, just return 0 for DW_PCIE_EP_TYPE (Mani)
>    to keep the status-quo (I've not tested this specifically).
> * In system PM support patch, use the same init order as probe (Sashiko)
> * In clear debug buffer patch, drop dw_pcie_dbi_ro_wr_dis(), which
>    is already done by the core nowadays (Sashiko, Mani)
> * In clear debug buffer patch, fix actual register write (Sashiko)
> * In clear debug buffer patch, update patch description with
>    better explanation about the register (Mani)
> 
> Changes in v6:
>   * https://lore.kernel.org/r/20260316-rockchip-pcie-system-suspend-v5-0-5bb5ad37d643@collabora.com
>   * Rebase to pci/next
>   * Minor changes to make checkpatch happy
>   * Update patch description of the last patch and drop RFC tag
>     according to feedback from Shawn Lin
>   * Prepend new patch, which fixes Nicolas Frattaroli's conversion
>     to FIELD_PREP_WM16 accidently dropping a flag.
>   * Add another new patch, which sets .broken_l1ss_resume to tell the
>     PCI core that the controller is powered off during suspend following
>     the Qcom example
> 
> Changes in v5:
>   * https://lore.kernel.org/r/20251029-rockchip-pcie-system-suspend-v4-0-ce2e1b0692d2@collabora.com
>   * rebase to v7.0-rc1
>   * Add an RFC patch at the end with some bits I found in the Rockchip vendor kernel;
>   * Made some tests on Rock 5B+ (RK3588); with this patch series suspending works
>     when setting pm_test to devices. A full system suspend fails in a later step,
>     but that is independent of PCIe.
> 
> Changes in v4:
>   * https://lore.kernel.org/linux-pci/1744940759-23823-1-git-send-email-shawn.lin@rock-chips.com/
>   * rename rockchip_pcie_get_ltssm to rockchip_pcie_get_ltssm_status_reg
>     in a separate patch (Niklas Cassel)
>   * rename rockchip_pcie_get_pure_ltssm to rockchip_pcie_get_ltssm_state
>     in a separate patch (Niklas Cassel)
>   * Move devm_phy_get out of phy_init to probe in a separate patch
>     (Manivannan Sadhasivam)
>   * Add helper function for enhanced LTSSM control mode in a separate patch
>     (Niklas Cassel)
>   * Add helper function for controller mode in a separate patch
>     (Niklas Cassel)
>   * Add helper function for DDL indicator in a separate patch
>     (Niklas Cassel)
>   * Move rockchip_pcie_pme_turn_off implementation in a separate patch
>   * Rebase to v6.18-rc3 using new FIELD_PREP_WM16()
>   * Improve readability of PME_TURN_OFF/PME_TO_ACK defines (Manivannan Sadhasivam)
>   * Fix usage of reverse Xmas (Manivannan Sadhasivam)
>   * Assert PERST# before turning off other resources (Manivannan Sadhasivam)
>   * Improve some error messages (Manivannan Sadhasivam)
>   * Rename goto labels as per their purpose (Manivannan Sadhasivam)
>   * Add extra patch for dw_pcie_resume_noirq, since I've seen errors
>     during resume on boards not having anything plugged into their PCIe
>     port
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
> Sebastian Reichel (9):
>        PCI: dw-rockchip: Fix LTSSM set functions
>        PCI: dw-rockchip: Restore vpcie3v3 regulator handle
>        PCI: dw-rockchip: Move devm_phy_get out of phy_init
>        PCI: dw-rockchip: Add helper function for enhanced LTSSM control mode
>        PCI: dw-rockchip: Add helper function for controller mode
>        PCI: dw-rockchip: Add helper function for DLL interrupts
>        PCI: dw-rockchip: Add pme_turn_off support
>        PCI: dw-rockchip: Set broken L1SS resume flag
>        PCI: dw-rockchip: Add system PM support
> 
>   drivers/pci/controller/dwc/pcie-dw-rockchip.c | 283 +++++++++++++++++++++-----
>   1 file changed, 228 insertions(+), 55 deletions(-)
> ---
> base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
> change-id: 20251028-rockchip-pcie-system-suspend-86cf08a7b229
> 
> Best regards,
> --
> Sebastian Reichel <sebastian.reichel@collabora.com>
> 
>