[PATCH V5 02/12] PCI: host-generic: Add common helpers for parsing Root Port properties
Sherry Sun
sherry.sun at nxp.com
Tue Feb 24 02:24:41 PST 2026
> Subject: Re: [PATCH V5 02/12] PCI: host-generic: Add common helpers for
> parsing Root Port properties
>
> On Fri, Feb 13, 2026 at 12:08:42PM +0800, Sherry Sun wrote:
> > Introduce generic helper functions to parse Root Port device tree
> > nodes and extract common properties like reset GPIOs. This allows
> > multiple PCI host controller drivers to share the same parsing logic.
> >
> > Define struct pci_host_port to hold common Root Port properties
> > (currently only reset GPIO descriptor) and add
> > pci_host_common_parse_ports() to parse Root Port nodes from device
> tree.
> >
> > Also add the 'ports' list to struct pci_host_bridge for better
> > maintain parsed Root Port information.
> >
> > Signed-off-by: Sherry Sun <sherry.sun at nxp.com>
> > ---
> > drivers/pci/controller/pci-host-common.c | 58
> > ++++++++++++++++++++++++ drivers/pci/controller/pci-host-common.h |
> 15 ++++++
> > drivers/pci/probe.c | 2 +
> > include/linux/pci.h | 1 +
> > 4 files changed, 76 insertions(+)
> >
> > diff --git a/drivers/pci/controller/pci-host-common.c
> > b/drivers/pci/controller/pci-host-common.c
> > index d6258c1cffe5..0c35907a5076 100644
> > --- a/drivers/pci/controller/pci-host-common.c
> > +++ b/drivers/pci/controller/pci-host-common.c
> > @@ -9,6 +9,7 @@
> >
> > #include <linux/kernel.h>
> > #include <linux/module.h>
> > +#include <linux/gpio/consumer.h>
> > #include <linux/of.h>
> > #include <linux/of_address.h>
> > #include <linux/of_pci.h>
> > @@ -17,6 +18,63 @@
> >
> > #include "pci-host-common.h"
> >
> > +/**
> > + * pci_host_common_parse_port - Parse a single Root Port node
> > + * @bridge: PCI host bridge
> > + * @node: Device tree node of the Root Port
> > + *
> > + * Returns: 0 on success, negative error code on failure */ static
> > +int pci_host_common_parse_port(struct pci_host_bridge *bridge,
> > + struct device_node *node)
> > +{
> > + struct device *dev = &bridge->dev;
> > + struct pci_host_port *port;
> > + struct gpio_desc *reset;
> > +
> > + reset = devm_fwnode_gpiod_get(dev, of_fwnode_handle(node),
> > + "reset", GPIOD_OUT_HIGH, "PERST#");
>
> For usecases like link retention from bootloader to kernel, this could be
> requested as GPIOD_ASIS:
> https://lore.ke/
> rnel.org%2Flinux-pci%2F20260109-link_retain-v1-3-
> 7e6782230f4b%40oss.qualcomm.com%2F&data=05%7C02%7Csherry.sun%40
> nxp.com%7C55c78c3dde694150dd1408de6d778ccd%7C686ea1d3bc2b4c6fa9
> 2cd99c5c301635%7C0%7C0%7C639068557422583280%7CUnknown%7CTWFp
> bGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4z
> MiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=zZAzwcH
> U2y8kH4YP0OoTVN66tUlCEq6m2aAKkWCFeTM%3D&reserved=0
>
Hi Manivannan,
I understand the concern about supporting use‑cases where the PCIe link is
intentionally retained from bootloader to kernel. However, relying on GPIOD_ASIS
may introduces a practical problem: it removes any guarantee about the PERST#
level during the early power‑on window.
According to the PCIe initialization requirements, PERST# must remain asserted
until power rails and REFCLK are valid. If we request the GPIO as GPIOD_ASIS, the
kernel no longer controls or even knows the actual state of PERST# at probe time,
which means the device may observe a deassert reset before power/clock stable,
it is risky even xx_pcie_host_init() asserts/deasserts PERST# again after enable
power rails hoping to reset the device cleanly. Once PERST# is released before
power or clock rails are fully valid, the device may already have entered undefined
or partially‑initialized states. Even if the driver asserts PERST# later, this does not
guarantee that all internal domains return to a well‑defined reset state. Some
implementations do not route PERST# to all functional blocks, or early deassert
during unstable power/clock conditions can leave the PCIe controller or endpoint
PHY/LTSSM in inconsistent conditions. Consequently, such a sequence can still lead
to undefined device state, failed link training, or inconsistent enumeration behavior.
In contrast, explicitly requesting the GPIO as GPIOD_OUT_HIGH ensures that PERST#
remains asserted until the driver is ready to perform the proper bring‑up sequence,
keeping the behavior deterministic and compliant with the PCIe reset‑ordering
expectations for this hardware.
If link retention from bootloader is a desired scenario for some systems, maybe we
can consider adding a DT property or quirk. But making GPIOD_ASIS the default
would compromise correct power‑on reset handling on platforms that require
PERST# to be actively driven.
Best Regards
Sherry
>
> > + if (IS_ERR(reset))
> > + return PTR_ERR(reset);
> > +
> > + port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
> > + if (!port)
> > + return -ENOMEM;
> > +
> > + port->reset = reset;
> > + INIT_LIST_HEAD(&port->list);
> > + list_add_tail(&port->list, &bridge->ports);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * pci_host_common_parse_ports - Parse Root Port nodes from device
> > +tree
> > + * @bridge: PCI host bridge
> > + *
> > + * This function iterates through child nodes of the host bridge and
> > +parses
> > + * Root Port properties (currently only reset GPIO).
> > + *
> > + * Returns: 0 on success, -ENOENT if no ports found, other negative
> > +error codes
> > + * on failure
> > + */
> > +int pci_host_common_parse_ports(struct pci_host_bridge *bridge) {
> > + struct device *dev = &bridge->dev;
> > + int ret = -ENOENT;
> > +
> > + for_each_available_child_of_node_scoped(dev->of_node, of_port) {
> > + if (!of_node_is_type(of_port, "pci"))
> > + continue;
> > + ret = pci_host_common_parse_port(bridge, of_port);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(pci_host_common_parse_ports);
> > +
> > static void gen_pci_unmap_cfg(void *ptr) {
> > pci_ecam_free((struct pci_config_window *)ptr); diff --git
> > a/drivers/pci/controller/pci-host-common.h
> > b/drivers/pci/controller/pci-host-common.h
> > index b5075d4bd7eb..25d808319836 100644
> > --- a/drivers/pci/controller/pci-host-common.h
> > +++ b/drivers/pci/controller/pci-host-common.h
> > @@ -12,6 +12,21 @@
> >
> > struct pci_ecam_ops;
> >
> > +/**
> > + * struct pci_host_port - Generic Root Port properties
> > + * @list: List node for linking multiple ports
> > + * @reset: GPIO descriptor for PERST# signal
> > + *
> > + * This structure contains common properties that can be parsed from
> > + * Root Port device tree nodes.
> > + */
> > +struct pci_host_port {
> > + struct list_head list;
> > + struct gpio_desc *reset;
> > +};
> > +
> > +int pci_host_common_parse_ports(struct pci_host_bridge *bridge);
> > +
> > int pci_host_common_probe(struct platform_device *pdev); int
> > pci_host_common_init(struct platform_device *pdev,
> > struct pci_host_bridge *bridge,
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index
> > 2975974f35e8..007a3fb8da86 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -647,6 +647,7 @@ static void pci_release_host_bridge_dev(struct
> > device *dev)
> >
> > pci_free_resource_list(&bridge->windows);
> > pci_free_resource_list(&bridge->dma_ranges);
> > + pci_free_resource_list(&bridge->ports);
> >
> > /* Host bridges only have domain_nr set in the emulation case */
> > if (bridge->domain_nr != PCI_DOMAIN_NR_NOT_SET) @@ -671,6
> +672,7 @@
> > static void pci_init_host_bridge(struct pci_host_bridge *bridge) {
> > INIT_LIST_HEAD(&bridge->windows);
> > INIT_LIST_HEAD(&bridge->dma_ranges);
> > + INIT_LIST_HEAD(&bridge->ports);
> >
> > /*
> > * We assume we can manage these PCIe features. Some systems
> may
> > diff --git a/include/linux/pci.h b/include/linux/pci.h index
> > 1c270f1d5123..b05482355abc 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -634,6 +634,7 @@ struct pci_host_bridge {
> > int domain_nr;
> > struct list_head windows; /* resource_entry */
> > struct list_head dma_ranges; /* dma ranges resource list */
> > + struct list_head ports; /* Root Port list (pci_host_port) */
> > #ifdef CONFIG_PCI_IDE
> > u16 nr_ide_streams; /* Max streams possibly active in
> @ide_stream_ida */
> > struct ida ide_stream_ida;
> > --
> > 2.37.1
> >
>
> --
> மணிவண்ணன் சதாசிவம்
More information about the linux-arm-kernel
mailing list