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

Sherry Sun
2026-04-20T08:25:02Z

> On Fri, Apr 17, 2026 at 02:55:33PM -0500, Bjorn Helgaas wrote:
> > 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%2Fgi
> > > thub.com%2Fdevicetree-org%2Fdt-
> schema%2Fissues%2F168&data=05%7C02%7C
> > >
> sherry.sun%40nxp.com%7C232644f8bbe64279f77908de9ea20b09%7C686ea1
> d3bc
> > >
> 2b4c6fa92cd99c5c301635%7C0%7C0%7C639122615977862858%7CUnknown
> %7CTWFp
> > >
> bGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4z
> MiIs
> > >
> IkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=r7szCLCsGFN2
> 1ULZ
> > > ibH7Ga%2FH0e6VyIdqznKCJ6yIGM4%3D&reserved=0
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
> > > thub.com%2Fdevicetree-org%2Fdt-
> schema%2Fpull%2F126&data=05%7C02%7Csh
> > >
> erry.sun%40nxp.com%7C232644f8bbe64279f77908de9ea20b09%7C686ea1d
> 3bc2b
> > >
> 4c6fa92cd99c5c301635%7C0%7C0%7C639122615977892044%7CUnknown%7
> CTWFpbG
> > >
> Zsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiI
> sIk
> > >
> FOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=o3RIy1SfvTGfkX
> 9rm8
> > > dNH2or5SZ7v5bYF%2Fl1XGaf8aA%3D&reserved=0
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
> > > thub.com%2Fdevicetree-org%2Fdt-
> schema%2Fpull%2F170&data=05%7C02%7Csh
> > >
> erry.sun%40nxp.com%7C232644f8bbe64279f77908de9ea20b09%7C686ea1d
> 3bc2b
> > >
> 4c6fa92cd99c5c301635%7C0%7C0%7C639122615977910169%7CUnknown%7
> CTWFpbG
> > >
> Zsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiI
> sIk
> > >
> FOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=d8SBGcqKcjYe1i
> iqs9
> > > %2F%2Bg1o%2FbECHYtnEULg7hTXyKmY%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.
> >
>
> Even if there are PERST# GPIOs from the host, connected to downstream
> ports of a PCIe switch, they could be stored in the Root Port's (pci_host_port)
> struct as a list of PERST#. This is what pcie-qcom driver does.
>
> It is too clumsy to handle PERST# individually for each device. We tried it
> before with pwrctrl, but it always ended up biting us on who gets to control
> the PERST#. We can't let pwrctrl handle PERST# for a switch port and host
> controller driver handle it for RP. And we cannot let pwrctrl handle PERST# for
> all ports, because, host controller drivers also need to control them for RC
> initialization.
>
> That's why it was decided to handle PERST# for all ports in the host controller
> drivers. So following that pattern, this helper could also be extended to parse
> the PERST# from all ports defined in DT and store them in the same Root Port
> struct.
>
> It should be trivial to implement this logic in the current helper. @Sherry:
> Could you please implement this logic?

Hi Mani, do you mean the similar logic in this patch?
https://lore.kernel.org/all/20251216-pci-pwrctrl-rework-v2-1-745a563b9be6@oss.qualcomm.com/
If yes, of cause I can do this for current helper functions in pci-host-common.c.

Best Regards
Sherry