| 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: <0442f060-1e8b-469a-a700-c96d18d5b737@linux.intel.com>
Date: Sun, 15 Jun 2025 00:25:38 -0700
From: Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@...ux.intel.com>
To: Manivannan Sadhasivam <mani@...nel.org>, bhelgaas@...gle.com,
brgl@...ev.pl
Cc: linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org, lukas@...ner.de,
Jim Quinlan <james.quinlan@...adcom.com>, Bjorn Helgaas <helgaas@...nel.org>
Subject: Re: [PATCH v2] PCI/pwrctrl: Move pci_pwrctrl_create_device()
definition to drivers/pci/pwrctrl/
On 6/14/25 4:20 AM, Manivannan Sadhasivam wrote:
> pci_pwrctrl_create_device() is a PWRCTRL framework API. So it should be
> built only when CONFIG_PWRCTRL is enabled. Currently, it is built
> independently of CONFIG_PWRCTRL. This creates enumeration failure on
> platforms like brcmstb using out-of-tree devicetree that describes the
> power supplies for endpoints in the PCIe child node, but doesn't use
> PWRCTRL framework to manage the supplies. The controller driver itself
> manages the supplies.
>
> But in any case, the API should be built only when CONFIG_PWRCTRL is
> enabled. So move its definition to drivers/pci/pwrctrl/core.c and provide
> a stub in drivers/pci/pci.h when CONFIG_PWRCTRL is not enabled. This also
> fixes the enumeration issues on the affected platforms.
>
> Fixes: 957f40d039a9 ("PCI/pwrctrl: Move creation of pwrctrl devices to pci_scan_device()")
> Reported-by: Jim Quinlan <james.quinlan@...adcom.com>
> Closes: https://lore.kernel.org/r/CA+-6iNwgaByXEYD3j=-+H_PKAxXRU78svPMRHDKKci8AGXAUPg@mail.gmail.com
> Suggested-by: Bjorn Helgaas <helgaas@...nel.org>
> Signed-off-by: Manivannan Sadhasivam <mani@...nel.org>
> ---
>
> Changes in v2:
>
> * Dropped the unused headers from probe.c (Lukas)
>
> drivers/pci/pci.h | 8 ++++++++
> drivers/pci/probe.c | 32 --------------------------------
> drivers/pci/pwrctrl/core.c | 36 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 44 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 12215ee72afb..c5efd8b9c96a 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -1159,4 +1159,12 @@ static inline int pci_msix_write_tph_tag(struct pci_dev *pdev, unsigned int inde
> (PCI_CONF1_ADDRESS(bus, dev, func, reg) | \
> PCI_CONF1_EXT_REG(reg))
>
> +#ifdef CONFIG_PCI_PWRCTRL
Use IS_ENABLED?
> +struct platform_device *pci_pwrctrl_create_device(struct pci_bus *bus,
> + int devfn);
> +#else
> +static inline struct platform_device *
> +pci_pwrctrl_create_device(struct pci_bus *bus, int devfn) { return NULL; }
> +#endif
> +
> #endif /* DRIVERS_PCI_H */
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 4b8693ec9e4c..478e217928a6 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -9,8 +9,6 @@
> #include <linux/pci.h>
> #include <linux/msi.h>
> #include <linux/of_pci.h>
> -#include <linux/of_platform.h>
> -#include <linux/platform_device.h>
> #include <linux/pci_hotplug.h>
> #include <linux/slab.h>
> #include <linux/module.h>
> @@ -2508,36 +2506,6 @@ bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l,
> }
> EXPORT_SYMBOL(pci_bus_read_dev_vendor_id);
>
> -static struct platform_device *pci_pwrctrl_create_device(struct pci_bus *bus, int devfn)
> -{
> - struct pci_host_bridge *host = pci_find_host_bridge(bus);
> - struct platform_device *pdev;
> - struct device_node *np;
> -
> - np = of_pci_find_child_device(dev_of_node(&bus->dev), devfn);
> - if (!np || of_find_device_by_node(np))
> - return NULL;
> -
> - /*
> - * First check whether the pwrctrl device really needs to be created or
> - * not. This is decided based on at least one of the power supplies
> - * being defined in the devicetree node of the device.
> - */
> - if (!of_pci_supply_present(np)) {
> - pr_debug("PCI/pwrctrl: Skipping OF node: %s\n", np->name);
> - return NULL;
> - }
> -
> - /* Now create the pwrctrl device */
> - pdev = of_platform_device_create(np, NULL, &host->dev);
> - if (!pdev) {
> - pr_err("PCI/pwrctrl: Failed to create pwrctrl device for node: %s\n", np->name);
> - return NULL;
> - }
> -
> - return pdev;
> -}
> -
> /*
> * Read the config data for a PCI device, sanity-check it,
> * and fill in the dev structure.
> diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
> index 6bdbfed584d6..20585b2c3681 100644
> --- a/drivers/pci/pwrctrl/core.c
> +++ b/drivers/pci/pwrctrl/core.c
> @@ -6,11 +6,47 @@
> #include <linux/device.h>
> #include <linux/export.h>
> #include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/of_pci.h>
> +#include <linux/of_platform.h>
> #include <linux/pci.h>
> #include <linux/pci-pwrctrl.h>
> +#include <linux/platform_device.h>
> #include <linux/property.h>
> #include <linux/slab.h>
>
> +#include "../pci.h"
> +
> +struct platform_device *pci_pwrctrl_create_device(struct pci_bus *bus, int devfn)
> +{
> + struct pci_host_bridge *host = pci_find_host_bridge(bus);
> + struct platform_device *pdev;
> + struct device_node *np;
> +
> + np = of_pci_find_child_device(dev_of_node(&bus->dev), devfn);
> + if (!np || of_find_device_by_node(np))
> + return NULL;
> +
> + /*
> + * First check whether the pwrctrl device really needs to be created or
> + * not. This is decided based on at least one of the power supplies
> + * being defined in the devicetree node of the device.
> + */
> + if (!of_pci_supply_present(np)) {
> + pr_debug("PCI/pwrctrl: Skipping OF node: %s\n", np->name);
> + return NULL;
> + }
> +
> + /* Now create the pwrctrl device */
> + pdev = of_platform_device_create(np, NULL, &host->dev);
> + if (!pdev) {
> + pr_err("PCI/pwrctrl: Failed to create pwrctrl device for node: %s\n", np->name);
> + return NULL;
> + }
> +
> + return pdev;
> +}
> +
> static int pci_pwrctrl_notify(struct notifier_block *nb, unsigned long action,
> void *data)
> {
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
Powered by blists - more mailing lists