| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID:
<VI0PR04MB12114013DF00830F8C9B2D9B89262A@VI0PR04MB12114.eurprd04.prod.outlook.com>
Date: Tue, 10 Feb 2026 03:09:11 +0000
From: Sherry Sun <sherry.sun@....com>
To: Frank Li <frank.li@....com>
CC: Hongxing Zhu <hongxing.zhu@....com>, "l.stach@...gutronix.de"
<l.stach@...gutronix.de>, "bhelgaas@...gle.com" <bhelgaas@...gle.com>,
"lpieralisi@...nel.org" <lpieralisi@...nel.org>, "kwilczynski@...nel.org"
<kwilczynski@...nel.org>, "mani@...nel.org" <mani@...nel.org>,
"robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org"
<krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>, "festevam@...il.com"
<festevam@...il.com>, "will@...nel.org" <will@...nel.org>,
"imx@...ts.linux.dev" <imx@...ts.linux.dev>, "kernel@...gutronix.de"
<kernel@...gutronix.de>, "linux-pci@...r.kernel.org"
<linux-pci@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "devicetree@...r.kernel.org"
<devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH V4 02/11] PCI: host-generic: Add common helpers for
parsing Root Port properties
> Subject: Re: [PATCH V4 02/11] PCI: host-generic: Add common helpers for
> parsing Root Port properties
>
> On Mon, Feb 09, 2026 at 04:24:45PM +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 and
> > add
> > pci_host_common_parse_ports() to parse Root Port nodes from device
> > tree,
> > pci_host_common_delete_ports() to cleanup the port lists.
> >
> > Signed-off-by: Sherry Sun <sherry.sun@....com>
> > ---
> > drivers/pci/controller/pci-host-common.c | 75
> > ++++++++++++++++++++++++ drivers/pci/controller/pci-host-common.h |
> > 17 ++++++
> > 2 files changed, 92 insertions(+)
> >
> > diff --git a/drivers/pci/controller/pci-host-common.c
> > b/drivers/pci/controller/pci-host-common.c
> > index c473e7c03bac..287e92df0092 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,80 @@
> >
> > #include "pci-host-common.h"
> >
> > +/**
> > + * pci_host_common_delete_ports - Cleanup function for port list
> > + * @data: Pointer to the port list head */ void
> > +pci_host_common_delete_ports(void *data) {
> > + struct list_head *ports = data;
> > + struct pci_host_port *port, *tmp;
> > +
> > + list_for_each_entry_safe(port, tmp, ports, list)
> > + list_del(&port->list);
> > +}
> > +EXPORT_SYMBOL_GPL(pci_host_common_delete_ports);
> > +
> > +/**
> > + * pci_host_common_parse_port - Parse a single Root Port node
> > + * @dev: Device pointer
> > + * @node: Device tree node of the Root Port
> > + * @ports: List head to add the parsed port to
> > + *
> > + * Returns: 0 on success, negative error code on failure */ static
> > +int pci_host_common_parse_port(struct device *dev,
> > + struct device_node *node,
> > + struct list_head *ports)
> > +{
> > + struct pci_host_port *port;
> > + struct gpio_desc *reset;
> > +
> > + reset = devm_fwnode_gpiod_get(dev, of_fwnode_handle(node),
> > + "reset", GPIOD_OUT_HIGH, "PERST#");
> > + 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, ports);
>
> Suppose need spin_lock for link list.
Hi Frank,
I think a spinlock is not necessary in this case for the following reasons:
1. The ports list is private to each device instance, not a globally shared resource.
2. The list is built during the driver probe phase(pci_host_common_parse_ports),
which is serialized by the driver core. And the list cleanup
(pci_host_common_delete_ports) is registered as a devm action and only
called during device removal, which is also serialized by the driver core. There
should no concurrent access during list construction.
3. Once the ports list is populated during probe, it's not modified during runtime.
There are no code paths that add/remove ports after initialization completes.
4. There's no scenario where multiple threads would simultaneously access or
modify this list during normal operation.
So at least currently seems spinlock would not provide any actual protection benefit.
>
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * pci_host_common_parse_ports - Parse Root Port nodes from device
> > +tree
> > + * @dev: Device pointer
> > + * @ports: List head to store parsed ports
> > + *
> > + * 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 device *dev, struct list_head
> > +*ports) {
> > + 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(dev, of_port, ports);
> > + if (ret) {
> > + pci_host_common_delete_ports(ports);
> > + 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..2c8df230886f 100644
> > --- a/drivers/pci/controller/pci-host-common.h
> > +++ b/drivers/pci/controller/pci-host-common.h
> > @@ -12,6 +12,23 @@
> >
> > 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;
> > +};
>
> I think it should be include/linux/pci.h
>
> struct pci_host_bridge {
> ...
> struct list_head ports_header;
> }
>
> So all API should pass down struct pci_host_bridge *.
Ok, this sounds reasonable, will try this in V5.
Best Regards
Sherry
>
> Frank
> > +
> > +void pci_host_common_delete_ports(void *data); int
> > +pci_host_common_parse_ports(struct device *dev,
> > + struct list_head *ports);
> > +
> > int pci_host_common_probe(struct platform_device *pdev); int
> > pci_host_common_init(struct platform_device *pdev,
> > struct pci_host_bridge *bridge,
> > --
> > 2.37.1
> >
Powered by blists - more mailing lists