Hi, This series is the proper version for toggling PERST# from the pwrctrl framework after the initial RFC posted earlier [1]. Problem statement ================= Pwrctrl framework is intented to control the supplies to the components on the PCI bus. However, if the platform supports the PERST# signal, it should be toggled as per the requirements in the electromechanical specifications like PCIe CEM, Mini, and M.2. Since the pwrctrl framework is controlling the power supplies, it should also toggle PERST# as per the requirements in the above mentioned specifications. Right now, it is just controlling the power to the components and rely on controller drivers to toggle PERST#, which goes against the specs. For instance, controller drivers will deassert PERST# even before the pwrctrl driver enables the supplies. This causes the device to see PERST# deassert immediately after power on, thereby violating the delay requirements in the PCI Electromechanical specs. Proposal ======== To fix this issue, the pwrctrl framework has to control the PERST# signal. But unfortunately, it is not straightforward. This is mostly due to controller drivers still need to assert PERST# as a part of their own initialization sequence. The controller drivers will parse PERST# from the devicetree nodes even before the pwrctrl drivers get probed. So the PERST# control needs to be shared between both drivers in a logical manner. This is achieved by adding a new callback, 'pci_host_bridge::perst_assert'. This callback if available, will be called by the pwrctrl framework during the power on and power off scenarios. The callback implementation in the controller driver has to take care of asserting/deasserting PERST# in an implementation specific way i.e., if the PERST# signal is a GPIO, then it should be toggled using gpiod APIs, or if the signal is implemented as a CSR, then the relevant registers should be written. Ideally, the PERST# delay requirements should be implemented in the pwrctrl framework (before/after calling the callback), but some controller drivers perform some post-link_up operations requiring them to control the delay within the driver. Those drivers may use this callback to assert/deassert PERST# and perform post-link_up operations. For reference, I've implemented the callback in the Qcom RC driver where it just asserts/deasserts PERST# and handles delay. Since the Qcom driver supports both legacy DT binding (all Root Port properties in host bridge node) and new binding (Root Port properies in Root Port node), I've moved the PERST# handling to pwrctrl driver only if the newly introduced 'qcom_pcie::legacy_binding' flag is not set. This flag if set, implies that the platform is using the legacy DT binding, so the controller driver has to control PERST#. DT binding requirement ====================== This series has some assumptions on the DT binding. But some of them are not enforced right now: 1. Pwrctrl driver requires the PCIe device node to have atleast one -supply property. Those supplies are already documented in the dtschema [2], but they are optional. So if those supplies are not present, pwrctrl driver will not get probed. For platforms having a fixed power supply to the components, they should describe those fixed supplies in DT. Otherwise, they cannot use pwrctrl drivers. (NOT ENFROCED) 2. Optional PERST# GPIO (reset-gpios property) is only allowed in the bridge node in the DT binding [3]. So for looking up the PERST# for an endpoint node, the controller driver has to look up the parent node where PERST# would be available. (ENFORCED) 3. If shared PERST# is implemented, all the bridge nodes (Root port and switch downstream) should have the same 'reset-gpios' property. This way, the controller drivers parsing PERST# could know if it is shared and invoke relevant gpiod APIs/flags. (NOT ENFORCED) I don't know how we can make sure DT binding enforces option 1 and 3. Testing ======= This series is tested on Qcom X1E based T14s laptop, SM8250 based RB5 board, and QCS6490 based RB3Gen2 board with DTS specifying Root Port properties in host bridge node and in Root Port node. [1] https://lore.kernel.org/linux-pci/20250707-pci-pwrctrl-perst-v1-0-c3c7e513e312@xxxxxxxxxx/ [2] https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/pci/pci-bus-common.yaml#L173 [3] https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/pci/pci-bus-common.yaml#L141 Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx> --- Changes in v3: - Dropped the pci_pwrctrl_init() move patch as it is no longer required - Added a patch to cleanup the usage of 'phy' and 'reset' pointers - Renamed the callback from 'toggle_perst' to 'perst_assert' - Reworded the patch descriptions - Link to v2: https://lore.kernel.org/r/20250903-pci-pwrctrl-perst-v2-0-2d461ed0e061@xxxxxxxxxxxxxxxx Changes in v2: - Reworked the PERST# lookup logic to use the node pointer instead of BDF - Added PWRCTRL guard to the toggle_perst callback - Link to v1: https://lore.kernel.org/r/20250819-pci-pwrctrl-perst-v1-0-4b74978d2007@xxxxxxxxxxxxxxxx Changes since RFC: * Implemented PERST# toggling using a callback since GPIO based PERST# is not available on all platforms. This also moves all PERST# handling to the controller drivers allowing them to add any additional post-link_up logic. --- Manivannan Sadhasivam (4): PCI/pwrctrl: Add support for asserting/deasserting PERST# PCI: qcom: Move host bridge 'phy' and 'reset' pointers to struct qcom_pcie_port PCI: qcom: Parse PERST# from all PCIe bridge nodes PCI: qcom: Allow pwrctrl core to control PERST# if 'reset-gpios' property is available drivers/pci/controller/dwc/pcie-qcom.c | 269 ++++++++++++++++++++++++--------- drivers/pci/pwrctrl/core.c | 27 ++++ include/linux/pci.h | 3 + 3 files changed, 227 insertions(+), 72 deletions(-) --- base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585 change-id: 20250818-pci-pwrctrl-perst-0bb7dd62b542 Best regards, -- Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
