RE: [PATCH V13 02/12] PCI: host-generic: Add common helpers for parsing Root Port properties

Sherry Sun
2026-04-20T02:59:08Z

> On Fri, Apr 17, 2026 at 03:17:16AM +0000, Sherry Sun wrote:
> > > On Thu, Apr 16, 2026 at 07:14:12PM +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.
> > >
> > > Are the Root Port and the RC the only possible places for 'reset'
> > > GPIO descriptions in DT? I think PERST# routing is outside the PCIe
> > > spec, so it seems like a system could provide a PERST# GPIO routed
> > > to any Switch Upstream Port or Endpoint (I assume a PERST# connected
> > > to a switch would apply to both the upstream port and the downstream
> > > ports).
> >
> > Thanks for the feedback. You're right that PERST# routing could
> > theoretically be connected to any device in the hierarchy. However,
> > for this patch series, I've focused on the most common use case in
> > practice: use Root Port level PERST# instead of the legacy Root
> > Complex level PERST#.
> >
> > Root Port level PERST# - This is the primary target, where each Root
> > Port has individual control over devices connected to it. RC level
> > PERST# - Legacy binding support, where a single GPIO controls all
> > ports.
> >
> > We can extend this framework later if real hardware emerges that needs
> > Switch or EP-level PERST# control. I can add a comment documenting
> > this limitation if needed.
> >
> > BTW, Mani and Rob had some great discussions in dt-schema about PERST#
> > and WAKE# sideband signals settings.
>
> > You can check here:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> > ub.com%2Fdevicetree-org%2Fdt-
> schema%2Fissues%2F168&data=05%7C02%7Csher
> >
> ry.sun%40nxp.com%7Cd68515fdc0f842ac82d708de9cbb4b2e%7C686ea1d3bc
> 2b4c6f
> >
> a92cd99c5c301635%7C0%7C0%7C639120525411363026%7CUnknown%7CTW
> FpbGZsb3d8
> >
> eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIj
> oiTW
> >
> FpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=ISzWCMSwqYBdw5w%2
> BDB5ERK51Dr
> > Tf2jzGtGh3wKNCMZ8%3D&reserved=0
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> > ub.com%2Fdevicetree-org%2Fdt-
> schema%2Fpull%2F126&data=05%7C02%7Csherry
> > .sun%40nxp.com%7Cd68515fdc0f842ac82d708de9cbb4b2e%7C686ea1d3bc
> 2b4c6fa9
> >
> 2cd99c5c301635%7C0%7C0%7C639120525411380537%7CUnknown%7CTWFp
> bGZsb3d8ey
> >
> JFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiT
> WFp
> >
> bCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=5cohkQIgl0CDlrOmD0dDIbj
> Q3%2BVg
> > VOhiOMDRQD1iwLM%3D&reserved=0
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> > ub.com%2Fdevicetree-org%2Fdt-
> schema%2Fpull%2F170&data=05%7C02%7Csherry
> > .sun%40nxp.com%7Cd68515fdc0f842ac82d708de9cbb4b2e%7C686ea1d3bc
> 2b4c6fa9
> >
> 2cd99c5c301635%7C0%7C0%7C639120525411391138%7CUnknown%7CTWFp
> bGZsb3d8ey
> >
> JFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiT
> WFp
> >
> bCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=WtjyGdvt4s84HynGc3%2F0
> K3UfkZii
> > naNAW1ypEd%2B11RY%3D&reserved=0
>
> The upshot of all those conversations is that WAKE# and PERST# can be
> routed to arbitrary devices independent of the PCI topology.
>
> I think extending host-generic to look for 'reset' in Root Port nodes is the right
> thing. My concern is more about where we store it. This patch saves it in a
> new "pci_host_port" struct, but someday we'll want a place to save the
> PERST# GPIOs for several slots behind a switch.
> Then we'll have two different ways to save the same information.
>
> WAKE# signals might be more pertinent -- we definitely need to support
> multiple WAKE# signals below a single Root Port, and it seems like PERST#
> and WAKE# GPIOs should be saved the same place.
>
> I'm wondering if both should go in the pci_dev itself. I guess the implication
> is that a pci_dev->reset GPIO would describe a PERST# connected to the
> device *below* the pci_dev, at least for Downstream Ports.

Hi Bjorn,
Ok, understand your concern, currently I've defined the struct pci_host_port to
store the common Root Port properties and added the list of 'ports' to struct
pci_host_bridge.
I will let @mani to comment and see if this is the appropriate place to store
these info, or if we should store them in struct pci_dev. Mani, can you please help?

But for now, the motivation for this patch is that many PCI host controller drivers
currently share the same requirement: extracting common Root Port properties like
reset GPIOs (at least currently in many drivers, perst is placed in the root port). We
introduce generic helper functions to parse the Root Port t device tree nodes and extract
these properties so that multiple PCI drivers can share the same parsing logic. I'm not
sure if we should take a step forward here anyway?

Best Regards
Sherry
>
> I don't know about WAKE# signals. When it's in a connector, there's probably
> only a single possible WAKE# per Downstream Port. But is it possible have
> multiple WAKE# signals from a multi-function device that's on the
> motherboard? Saving the WAKE# GPIO in the Downstream Port wouldn't
> accommodate that case.