[PATCH 0/5] PCI/pwrctl: Ensure that the pwrctl drivers are probed before PCI client drivers

[PATCH 0/5] PCI/pwrctl: Ensure that the pwrctl drivers are probed before PCI client drivers

[Manivannan Sadhasivam via B4 Relay]

Hi,

This series reworks the PCI/pwrctl integration to ensure that the pwrctl drivers
are always probed before the PCI client drivers. This series addresses a race
condition when both pwrctl and pwrctl/pwrseq drivers probe parallely (even when
the later one probes last). One such issue was reported for the Qcom X13s
platform with WLAN module and fixed with 'commit a9aaf1ff88a8 ("power:
sequencing: request the WLAN enable GPIO as-is")'.

Though the issue was fixed with a hack in the pwrseq driver, it was clear that
the issue is applicable to all pwrctl drivers. Hence, this series tries to
address the issue in the PCI/pwrctl integration.

- Mani

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
Manivannan Sadhasivam (5):
      PCI/pwrctl: Use of_platform_device_create() to create pwrctl devices
      PCI/pwrctl: Create pwrctl devices only if at least one power supply is present
      PCI/pwrctl: Ensure that the pwrctl drivers are probed before the PCI client drivers
      PCI/pwrctl: Move pwrctl device creation to its own helper function
      PCI/pwrctl: Remove pwrctl device without iterating over all children of pwrctl parent

 drivers/pci/bus.c         | 64 +++++++++++++++++++++++++++++++++++++++++------
 drivers/pci/of.c          | 27 ++++++++++++++++++++
 drivers/pci/pci.h         |  5 ++++
 drivers/pci/pwrctl/core.c | 10 --------
 drivers/pci/remove.c      | 17 ++++++-------
 5 files changed, 96 insertions(+), 27 deletions(-)
---
base-commit: 48dc7986beb60522eb217c0016f999cc7afaf0b7
change-id: 20241022-pci-pwrctl-rework-a1b024158555

Best regards,
-- 
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

[PATCH 1/5] PCI/pwrctl: Use of_platform_device_create() to create pwrctl devices

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

of_platform_populate() API creates platform devices by descending through
the children of the parent node. But it provides no control over the child
nodes, which makes it difficult to add checks for the child nodes in the
future. So use of_platform_device_create() API together with
for_each_child_of_node_scoped() so that it is possible to add checks for
each node before creating the platform device.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/pci/bus.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 55c853686051..959044b059b5 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -13,6 +13,7 @@
 #include <linux/ioport.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
+#include <linux/platform_device.h>
 #include <linux/proc_fs.h>
 #include <linux/slab.h>
 
@@ -329,6 +330,7 @@ void __weak pcibios_bus_add_device(struct pci_dev *pdev) { }
 void pci_bus_add_device(struct pci_dev *dev)
 {
 	struct device_node *dn = dev->dev.of_node;
+	struct platform_device *pdev;
 	int retval;
 
 	/*
@@ -351,11 +353,11 @@ void pci_bus_add_device(struct pci_dev *dev)
 	pci_dev_assign_added(dev, true);
 
 	if (dev_of_node(&dev->dev) && pci_is_bridge(dev)) {
-		retval = of_platform_populate(dev_of_node(&dev->dev), NULL, NULL,
-					      &dev->dev);
-		if (retval)
-			pci_err(dev, "failed to populate child OF nodes (%d)\n",
-				retval);
+		for_each_child_of_node_scoped(dn, child) {
+			pdev = of_platform_device_create(child, NULL, &dev->dev);
+			if (!pdev)
+				pci_err(dev, "failed to create OF node: %s\n", child->name);
+		}
 	}
 }
 EXPORT_SYMBOL_GPL(pci_bus_add_device);

-- 
2.25.1

[PATCH 2/5] PCI/pwrctl: Create pwrctl devices only if at least one power supply is present

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Currently, pwrctl devices are created if the corresponding PCI nodes are
defined in devicetree. But this is not correct, because not all PCI nodes
defined in devicetree require pwrctl support. Pwrctl comes into picture
only when the device requires kernel to manage its power state. This can
be determined using the power supply properties present in the devicetree
node of the device.

So add a new API, of_pci_is_supply_present() that checks the devicetree
node if at least one power supply property is present or not. If present,
then the pwrctl device will be created for that PCI node. Otherwise, it
will be skipped.

Cc: stable+noautosel@kernel.org # Depends on of_platform_device_create() rework
Fixes: 8fb18619d910 ("PCI/pwrctl: Create platform devices for child OF nodes of the port node")
Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/pci/bus.c | 11 +++++++++++
 drivers/pci/of.c  | 27 +++++++++++++++++++++++++++
 drivers/pci/pci.h |  5 +++++
 3 files changed, 43 insertions(+)

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 959044b059b5..698ec98b9388 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -354,6 +354,17 @@ void pci_bus_add_device(struct pci_dev *dev)
 
 	if (dev_of_node(&dev->dev) && pci_is_bridge(dev)) {
 		for_each_child_of_node_scoped(dn, child) {
+			/*
+			 * First check whether the pwrctl device 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_is_supply_present(child)) {
+				pci_dbg(dev, "skipping OF node: %s\n", child->name);
+				continue;
+			}
+
 			pdev = of_platform_device_create(child, NULL, &dev->dev);
 			if (!pdev)
 				pci_err(dev, "failed to create OF node: %s\n", child->name);
diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index dacea3fc5128..1f6a15a35a82 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -728,6 +728,33 @@ void of_pci_make_dev_node(struct pci_dev *pdev)
 }
 #endif
 
+/**
+ * of_pci_is_supply_present() - Check if the power supply is present for the PCI
+ *				device
+ * @np: Device tree node
+ *
+ * Check if the power supply for the PCI device is present in the device tree
+ * node or not.
+ *
+ * Return: true if at least one power supply exists; false otherwise.
+ */
+bool of_pci_is_supply_present(struct device_node *np)
+{
+	struct property *prop;
+	char *supply;
+
+	if (!np)
+		return false;
+
+	for_each_property_of_node(np, prop) {
+		supply = strrchr(prop->name, '-');
+		if (supply && !strcmp(supply, "-supply"))
+			return true;
+	}
+
+	return false;
+}
+
 #endif /* CONFIG_PCI */
 
 /**
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 14d00ce45bfa..c8081b427267 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -746,6 +746,7 @@ void pci_set_bus_of_node(struct pci_bus *bus);
 void pci_release_bus_of_node(struct pci_bus *bus);
 
 int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge);
+bool of_pci_is_supply_present(struct device_node *np);
 
 #else
 static inline int
@@ -793,6 +794,10 @@ static inline int devm_of_pci_bridge_init(struct device *dev, struct pci_host_br
 	return 0;
 }
 
+static inline bool of_pci_is_supply_present(struct device_node *np);
+{
+	return false;
+}
 #endif /* CONFIG_OF */
 
 struct of_changeset;

-- 
2.25.1

[PATCH 3/5] PCI/pwrctl: Ensure that the pwrctl drivers are probed before the PCI client drivers

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

As per the kernel device driver model, pwrctl device is the supplier for
the PCI device. But the device link that enforces the supplier-consumer
relationship is created inside the pwrctl driver currently. Due to this,
the driver model doesn't prevent probing of the PCI client drivers before
probing the corresponding pwrctl drivers. This may lead to a race condition
if the PCI device was already powered on by the bootloader (before the
pwrctl driver).

If the bootloader did not power on the PCI device, this wouldn't create any
problem as the pwrctl driver will be the one powering on the device, so the
PCI client driver always gets probed afterward. But if the device was
already powered on, then the device will be seen by the PCI core and the
PCI client driver may get probed before its pwrctl driver. This creates a
race condition as the pwrctl driver may change the device power state while
the device is being accessed by the client driver.

One such issue was already reported on the Qcom X13s platform with the
WLAN device and fixed with a hack in the WCN pwrseq driver by the 'commit
a9aaf1ff88a8 ("power: sequencing: request the WLAN enable GPIO as-is")'.

But a cleaner way to fix the above mentioned race condition would be to
ensure that the pwrctl drivers are always probed before the client drivers.
This is achieved by creating the device link between pwrctl device and PCI
device before device_attach() in pci_bus_add_device().

Note that there is no need to explicitly remove the device link as that
will be taken care by the driver core when the PCI device gets removed.

Cc: stable+noautosel@kernel.org # Depends on power supply check
Fixes: 4565d2652a37 ("PCI/pwrctl: Add PCI power control core code")
Fixes: 8fb18619d910 ("PCI/pwrctl: Create platform devices for child OF nodes of the port node")
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/pci/bus.c         | 26 +++++++++++++++++++-------
 drivers/pci/pwrctl/core.c | 10 ----------
 2 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 698ec98b9388..351af581904f 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -345,13 +345,6 @@ void pci_bus_add_device(struct pci_dev *dev)
 	pci_proc_attach_device(dev);
 	pci_bridge_d3_update(dev);
 
-	dev->match_driver = !dn || of_device_is_available(dn);
-	retval = device_attach(&dev->dev);
-	if (retval < 0 && retval != -EPROBE_DEFER)
-		pci_warn(dev, "device attach failed (%d)\n", retval);
-
-	pci_dev_assign_added(dev, true);
-
 	if (dev_of_node(&dev->dev) && pci_is_bridge(dev)) {
 		for_each_child_of_node_scoped(dn, child) {
 			/*
@@ -370,6 +363,25 @@ void pci_bus_add_device(struct pci_dev *dev)
 				pci_err(dev, "failed to create OF node: %s\n", child->name);
 		}
 	}
+
+	/*
+	 * Create a device link between the PCI device and pwrctl device (if
+	 * exists). This ensures that the pwrctl drivers are probed before the
+	 * PCI client drivers.
+	 */
+	pdev = of_find_device_by_node(dn);
+	if (pdev) {
+		if (!device_link_add(&dev->dev, &pdev->dev, DL_FLAG_AUTOREMOVE_CONSUMER))
+			pci_err(dev, "failed to add device link between %s and %s\n",
+				dev_name(&dev->dev), pdev->name);
+	}
+
+	dev->match_driver = !dn || of_device_is_available(dn);
+	retval = device_attach(&dev->dev);
+	if (retval < 0 && retval != -EPROBE_DEFER)
+		pci_warn(dev, "device attach failed (%d)\n", retval);
+
+	pci_dev_assign_added(dev, true);
 }
 EXPORT_SYMBOL_GPL(pci_bus_add_device);
 
diff --git a/drivers/pci/pwrctl/core.c b/drivers/pci/pwrctl/core.c
index 01d913b60316..bb5a23712434 100644
--- a/drivers/pci/pwrctl/core.c
+++ b/drivers/pci/pwrctl/core.c
@@ -33,16 +33,6 @@ static int pci_pwrctl_notify(struct notifier_block *nb, unsigned long action,
 		 */
 		dev->of_node_reused = true;
 		break;
-	case BUS_NOTIFY_BOUND_DRIVER:
-		pwrctl->link = device_link_add(dev, pwrctl->dev,
-					       DL_FLAG_AUTOREMOVE_CONSUMER);
-		if (!pwrctl->link)
-			dev_err(pwrctl->dev, "Failed to add device link\n");
-		break;
-	case BUS_NOTIFY_UNBOUND_DRIVER:
-		if (pwrctl->link)
-			device_link_remove(dev, pwrctl->dev);
-		break;
 	}
 
 	return NOTIFY_DONE;

-- 
2.25.1

[PATCH 4/5] PCI/pwrctl: Move pwrctl device creation to its own helper function

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

This makes the pci_bus_add_device() API easier to maintain. Also add more
comments to the helper to describe how the devices are created.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/pci/bus.c | 59 ++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 41 insertions(+), 18 deletions(-)

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 351af581904f..c4cae1775c9e 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -321,6 +321,46 @@ void __weak pcibios_resource_survey_bus(struct pci_bus *bus) { }
 
 void __weak pcibios_bus_add_device(struct pci_dev *pdev) { }
 
+/*
+ * Create pwrctl devices (if required) for the PCI devices to handle the power
+ * state.
+ */
+static void pci_pwrctl_create_devices(struct pci_dev *dev)
+{
+	struct device_node *np = dev_of_node(&dev->dev);
+	struct device *parent = &dev->dev;
+	struct platform_device *pdev;
+
+	/*
+	 * First ensure that we are starting from a PCI bridge and it has a
+	 * corresponding devicetree node.
+	 */
+	if (np && pci_is_bridge(dev)) {
+		/*
+		 * Now look for the child PCI device nodes and create pwrctl
+		 * devices for them. The pwrctl device drivers will manage the
+		 * power state of the devices.
+		 */
+		for_each_child_of_node_scoped(np, child) {
+			/*
+			 * First check whether the pwrctl 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_is_supply_present(child)) {
+				pci_dbg(dev, "skipping OF node: %s\n", child->name);
+				return;
+			}
+
+			/* Now create the pwrctl device */
+			pdev = of_platform_device_create(child, NULL, parent);
+			if (!pdev)
+				pci_err(dev, "failed to create OF node: %s\n", child->name);
+		}
+	}
+}
+
 /**
  * pci_bus_add_device - start driver for a single device
  * @dev: device to add
@@ -345,24 +385,7 @@ void pci_bus_add_device(struct pci_dev *dev)
 	pci_proc_attach_device(dev);
 	pci_bridge_d3_update(dev);
 
-	if (dev_of_node(&dev->dev) && pci_is_bridge(dev)) {
-		for_each_child_of_node_scoped(dn, child) {
-			/*
-			 * First check whether the pwrctl device 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_is_supply_present(child)) {
-				pci_dbg(dev, "skipping OF node: %s\n", child->name);
-				continue;
-			}
-
-			pdev = of_platform_device_create(child, NULL, &dev->dev);
-			if (!pdev)
-				pci_err(dev, "failed to create OF node: %s\n", child->name);
-		}
-	}
+	pci_pwrctl_create_devices(dev);
 
 	/*
 	 * Create a device link between the PCI device and pwrctl device (if

-- 
2.25.1

[PATCH 5/5] PCI/pwrctl: Remove pwrctl device without iterating over all children of pwrctl parent

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

There is no need to iterate over all children of the pwrctl device parent
to remove the pwrctl device. Since the pwrctl device associated with the
PCI device can be found using of_find_device_by_node() API, use it directly
instead.

If any pwrctl devices lying around without getting associated with the PCI
devices, then those will be removed once their parent device gets removed.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/pci/remove.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index e4ce1145aa3e..3dd9b3024956 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -17,16 +17,16 @@ static void pci_free_resources(struct pci_dev *dev)
 	}
 }
 
-static int pci_pwrctl_unregister(struct device *dev, void *data)
+static void pci_pwrctl_unregister(struct device *dev)
 {
-	struct device_node *pci_node = data, *plat_node = dev_of_node(dev);
+	struct platform_device *pdev;
 
-	if (dev_is_platform(dev) && plat_node && plat_node == pci_node) {
-		of_device_unregister(to_platform_device(dev));
-		of_node_clear_flag(plat_node, OF_POPULATED);
-	}
+	pdev = of_find_device_by_node(dev_of_node(dev));
+	if (!pdev)
+		return;
 
-	return 0;
+	of_device_unregister(pdev);
+	of_node_clear_flag(dev_of_node(dev), OF_POPULATED);
 }
 
 static void pci_stop_dev(struct pci_dev *dev)
@@ -34,8 +34,7 @@ static void pci_stop_dev(struct pci_dev *dev)
 	pci_pme_active(dev, false);
 
 	if (pci_dev_is_added(dev)) {
-		device_for_each_child(dev->dev.parent, dev_of_node(&dev->dev),
-				      pci_pwrctl_unregister);
+		pci_pwrctl_unregister(&dev->dev);
 		device_release_driver(&dev->dev);
 		pci_proc_detach_device(dev);
 		pci_remove_sysfs_dev_files(dev);

-- 
2.25.1

Re: [PATCH 1/5] PCI/pwrctl: Use of_platform_device_create() to create pwrctl devices

[Bartosz Golaszewski]

On Tue, 22 Oct 2024 at 12:28, Manivannan Sadhasivam via B4 Relay
<devnull+manivannan.sadhasivam.linaro.org@kernel.org> wrote:
>
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>
> of_platform_populate() API creates platform devices by descending through
> the children of the parent node. But it provides no control over the child
> nodes, which makes it difficult to add checks for the child nodes in the
> future. So use of_platform_device_create() API together with
> for_each_child_of_node_scoped() so that it is possible to add checks for
> each node before creating the platform device.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/pci/bus.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index 55c853686051..959044b059b5 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -13,6 +13,7 @@
>  #include <linux/ioport.h>
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
> +#include <linux/platform_device.h>
>  #include <linux/proc_fs.h>
>  #include <linux/slab.h>
>
> @@ -329,6 +330,7 @@ void __weak pcibios_bus_add_device(struct pci_dev *pdev) { }
>  void pci_bus_add_device(struct pci_dev *dev)
>  {
>         struct device_node *dn = dev->dev.of_node;
> +       struct platform_device *pdev;
>         int retval;
>
>         /*
> @@ -351,11 +353,11 @@ void pci_bus_add_device(struct pci_dev *dev)
>         pci_dev_assign_added(dev, true);
>
>         if (dev_of_node(&dev->dev) && pci_is_bridge(dev)) {
> -               retval = of_platform_populate(dev_of_node(&dev->dev), NULL, NULL,
> -                                             &dev->dev);
> -               if (retval)
> -                       pci_err(dev, "failed to populate child OF nodes (%d)\n",
> -                               retval);
> +               for_each_child_of_node_scoped(dn, child) {

Since we won't be populating any disabled nodes, I'd use
for_each_available_child_of_node_scoped() here.

Bart

> +                       pdev = of_platform_device_create(child, NULL, &dev->dev);
> +                       if (!pdev)
> +                               pci_err(dev, "failed to create OF node: %s\n", child->name);
> +               }
>         }
>  }
>  EXPORT_SYMBOL_GPL(pci_bus_add_device);
>
> --
> 2.25.1
>
>

Re: [PATCH 2/5] PCI/pwrctl: Create pwrctl devices only if at least one power supply is present

[Bartosz Golaszewski]

On Tue, 22 Oct 2024 at 12:28, Manivannan Sadhasivam via B4 Relay
<devnull+manivannan.sadhasivam.linaro.org@kernel.org> wrote:
>
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>
> Currently, pwrctl devices are created if the corresponding PCI nodes are
> defined in devicetree. But this is not correct, because not all PCI nodes
> defined in devicetree require pwrctl support. Pwrctl comes into picture
> only when the device requires kernel to manage its power state. This can
> be determined using the power supply properties present in the devicetree
> node of the device.
>
> So add a new API, of_pci_is_supply_present() that checks the devicetree
> node if at least one power supply property is present or not. If present,
> then the pwrctl device will be created for that PCI node. Otherwise, it
> will be skipped.
>
> Cc: stable+noautosel@kernel.org # Depends on of_platform_device_create() rework
> Fixes: 8fb18619d910 ("PCI/pwrctl: Create platform devices for child OF nodes of the port node")
> Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Re: [PATCH 3/5] PCI/pwrctl: Ensure that the pwrctl drivers are probed before the PCI client drivers

[Bartosz Golaszewski]

On Tue, 22 Oct 2024 at 12:28, Manivannan Sadhasivam via B4 Relay
<devnull+manivannan.sadhasivam.linaro.org@kernel.org> wrote:
>
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>
> As per the kernel device driver model, pwrctl device is the supplier for
> the PCI device. But the device link that enforces the supplier-consumer
> relationship is created inside the pwrctl driver currently. Due to this,
> the driver model doesn't prevent probing of the PCI client drivers before
> probing the corresponding pwrctl drivers. This may lead to a race condition
> if the PCI device was already powered on by the bootloader (before the
> pwrctl driver).
>
> If the bootloader did not power on the PCI device, this wouldn't create any
> problem as the pwrctl driver will be the one powering on the device, so the
> PCI client driver always gets probed afterward. But if the device was
> already powered on, then the device will be seen by the PCI core and the
> PCI client driver may get probed before its pwrctl driver. This creates a
> race condition as the pwrctl driver may change the device power state while
> the device is being accessed by the client driver.
>
> One such issue was already reported on the Qcom X13s platform with the
> WLAN device and fixed with a hack in the WCN pwrseq driver by the 'commit
> a9aaf1ff88a8 ("power: sequencing: request the WLAN enable GPIO as-is")'.
>
> But a cleaner way to fix the above mentioned race condition would be to
> ensure that the pwrctl drivers are always probed before the client drivers.
> This is achieved by creating the device link between pwrctl device and PCI
> device before device_attach() in pci_bus_add_device().
>
> Note that there is no need to explicitly remove the device link as that
> will be taken care by the driver core when the PCI device gets removed.
>
> Cc: stable+noautosel@kernel.org # Depends on power supply check
> Fixes: 4565d2652a37 ("PCI/pwrctl: Add PCI power control core code")
> Fixes: 8fb18619d910 ("PCI/pwrctl: Create platform devices for child OF nodes of the port node")
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/pci/bus.c         | 26 +++++++++++++++++++-------
>  drivers/pci/pwrctl/core.c | 10 ----------
>  2 files changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index 698ec98b9388..351af581904f 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -345,13 +345,6 @@ void pci_bus_add_device(struct pci_dev *dev)
>         pci_proc_attach_device(dev);
>         pci_bridge_d3_update(dev);
>
> -       dev->match_driver = !dn || of_device_is_available(dn);
> -       retval = device_attach(&dev->dev);
> -       if (retval < 0 && retval != -EPROBE_DEFER)
> -               pci_warn(dev, "device attach failed (%d)\n", retval);
> -
> -       pci_dev_assign_added(dev, true);
> -
>         if (dev_of_node(&dev->dev) && pci_is_bridge(dev)) {
>                 for_each_child_of_node_scoped(dn, child) {
>                         /*
> @@ -370,6 +363,25 @@ void pci_bus_add_device(struct pci_dev *dev)
>                                 pci_err(dev, "failed to create OF node: %s\n", child->name);
>                 }
>         }
> +
> +       /*
> +        * Create a device link between the PCI device and pwrctl device (if
> +        * exists). This ensures that the pwrctl drivers are probed before the
> +        * PCI client drivers.
> +        */
> +       pdev = of_find_device_by_node(dn);
> +       if (pdev) {
> +               if (!device_link_add(&dev->dev, &pdev->dev, DL_FLAG_AUTOREMOVE_CONSUMER))
> +                       pci_err(dev, "failed to add device link between %s and %s\n",
> +                               dev_name(&dev->dev), pdev->name);
> +       }
> +
> +       dev->match_driver = !dn || of_device_is_available(dn);
> +       retval = device_attach(&dev->dev);
> +       if (retval < 0 && retval != -EPROBE_DEFER)
> +               pci_warn(dev, "device attach failed (%d)\n", retval);
> +
> +       pci_dev_assign_added(dev, true);
>  }
>  EXPORT_SYMBOL_GPL(pci_bus_add_device);
>
> diff --git a/drivers/pci/pwrctl/core.c b/drivers/pci/pwrctl/core.c
> index 01d913b60316..bb5a23712434 100644
> --- a/drivers/pci/pwrctl/core.c
> +++ b/drivers/pci/pwrctl/core.c
> @@ -33,16 +33,6 @@ static int pci_pwrctl_notify(struct notifier_block *nb, unsigned long action,
>                  */
>                 dev->of_node_reused = true;
>                 break;
> -       case BUS_NOTIFY_BOUND_DRIVER:
> -               pwrctl->link = device_link_add(dev, pwrctl->dev,
> -                                              DL_FLAG_AUTOREMOVE_CONSUMER);
> -               if (!pwrctl->link)
> -                       dev_err(pwrctl->dev, "Failed to add device link\n");
> -               break;
> -       case BUS_NOTIFY_UNBOUND_DRIVER:
> -               if (pwrctl->link)
> -                       device_link_remove(dev, pwrctl->dev);
> -               break;
>         }
>
>         return NOTIFY_DONE;
>
> --
> 2.25.1
>
>

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Re: [PATCH 4/5] PCI/pwrctl: Move pwrctl device creation to its own helper function

[Bartosz Golaszewski]

On Tue, 22 Oct 2024 at 12:28, Manivannan Sadhasivam via B4 Relay
<devnull+manivannan.sadhasivam.linaro.org@kernel.org> wrote:
>
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>
> This makes the pci_bus_add_device() API easier to maintain. Also add more
> comments to the helper to describe how the devices are created.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/pci/bus.c | 59 ++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 41 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index 351af581904f..c4cae1775c9e 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -321,6 +321,46 @@ void __weak pcibios_resource_survey_bus(struct pci_bus *bus) { }
>
>  void __weak pcibios_bus_add_device(struct pci_dev *pdev) { }
>
> +/*
> + * Create pwrctl devices (if required) for the PCI devices to handle the power
> + * state.
> + */
> +static void pci_pwrctl_create_devices(struct pci_dev *dev)
> +{
> +       struct device_node *np = dev_of_node(&dev->dev);
> +       struct device *parent = &dev->dev;
> +       struct platform_device *pdev;
> +
> +       /*
> +        * First ensure that we are starting from a PCI bridge and it has a
> +        * corresponding devicetree node.
> +        */
> +       if (np && pci_is_bridge(dev)) {
> +               /*
> +                * Now look for the child PCI device nodes and create pwrctl
> +                * devices for them. The pwrctl device drivers will manage the
> +                * power state of the devices.
> +                */
> +               for_each_child_of_node_scoped(np, child) {
> +                       /*
> +                        * First check whether the pwrctl 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_is_supply_present(child)) {
> +                               pci_dbg(dev, "skipping OF node: %s\n", child->name);
> +                               return;
> +                       }
> +
> +                       /* Now create the pwrctl device */
> +                       pdev = of_platform_device_create(child, NULL, parent);
> +                       if (!pdev)
> +                               pci_err(dev, "failed to create OF node: %s\n", child->name);
> +               }
> +       }
> +}
> +
>  /**
>   * pci_bus_add_device - start driver for a single device
>   * @dev: device to add
> @@ -345,24 +385,7 @@ void pci_bus_add_device(struct pci_dev *dev)
>         pci_proc_attach_device(dev);
>         pci_bridge_d3_update(dev);
>
> -       if (dev_of_node(&dev->dev) && pci_is_bridge(dev)) {
> -               for_each_child_of_node_scoped(dn, child) {
> -                       /*
> -                        * First check whether the pwrctl device 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_is_supply_present(child)) {
> -                               pci_dbg(dev, "skipping OF node: %s\n", child->name);
> -                               continue;
> -                       }
> -
> -                       pdev = of_platform_device_create(child, NULL, &dev->dev);
> -                       if (!pdev)
> -                               pci_err(dev, "failed to create OF node: %s\n", child->name);
> -               }
> -       }
> +       pci_pwrctl_create_devices(dev);
>
>         /*
>          * Create a device link between the PCI device and pwrctl device (if
>
> --
> 2.25.1
>
>

Would it be possible to move this into drivers/pwrctl/ and provide a
header stub for when PCI_PWRCTL is disabled in Kconfig?

Bart

Re: [PATCH 5/5] PCI/pwrctl: Remove pwrctl device without iterating over all children of pwrctl parent

[Bartosz Golaszewski]

On Tue, 22 Oct 2024 at 12:28, Manivannan Sadhasivam via B4 Relay
<devnull+manivannan.sadhasivam.linaro.org@kernel.org> wrote:
>
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>
> There is no need to iterate over all children of the pwrctl device parent
> to remove the pwrctl device. Since the pwrctl device associated with the
> PCI device can be found using of_find_device_by_node() API, use it directly
> instead.
>
> If any pwrctl devices lying around without getting associated with the PCI
> devices, then those will be removed once their parent device gets removed.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/pci/remove.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
> index e4ce1145aa3e..3dd9b3024956 100644
> --- a/drivers/pci/remove.c
> +++ b/drivers/pci/remove.c
> @@ -17,16 +17,16 @@ static void pci_free_resources(struct pci_dev *dev)
>         }
>  }
>
> -static int pci_pwrctl_unregister(struct device *dev, void *data)
> +static void pci_pwrctl_unregister(struct device *dev)
>  {
> -       struct device_node *pci_node = data, *plat_node = dev_of_node(dev);
> +       struct platform_device *pdev;
>
> -       if (dev_is_platform(dev) && plat_node && plat_node == pci_node) {
> -               of_device_unregister(to_platform_device(dev));
> -               of_node_clear_flag(plat_node, OF_POPULATED);
> -       }
> +       pdev = of_find_device_by_node(dev_of_node(dev));
> +       if (!pdev)
> +               return;
>
> -       return 0;
> +       of_device_unregister(pdev);
> +       of_node_clear_flag(dev_of_node(dev), OF_POPULATED);
>  }
>
>  static void pci_stop_dev(struct pci_dev *dev)
> @@ -34,8 +34,7 @@ static void pci_stop_dev(struct pci_dev *dev)
>         pci_pme_active(dev, false);
>
>         if (pci_dev_is_added(dev)) {
> -               device_for_each_child(dev->dev.parent, dev_of_node(&dev->dev),
> -                                     pci_pwrctl_unregister);
> +               pci_pwrctl_unregister(&dev->dev);
>                 device_release_driver(&dev->dev);
>                 pci_proc_detach_device(dev);
>                 pci_remove_sysfs_dev_files(dev);
>
> --
> 2.25.1
>
>

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Re: [PATCH 0/5] PCI/pwrctl: Ensure that the pwrctl drivers are probed before PCI client drivers

[Bartosz Golaszewski]

On Tue, 22 Oct 2024 at 12:28, Manivannan Sadhasivam via B4 Relay
<devnull+manivannan.sadhasivam.linaro.org@kernel.org> wrote:
>
> Hi,
>
> This series reworks the PCI/pwrctl integration to ensure that the pwrctl drivers
> are always probed before the PCI client drivers. This series addresses a race
> condition when both pwrctl and pwrctl/pwrseq drivers probe parallely (even when
> the later one probes last). One such issue was reported for the Qcom X13s
> platform with WLAN module and fixed with 'commit a9aaf1ff88a8 ("power:
> sequencing: request the WLAN enable GPIO as-is")'.
>
> Though the issue was fixed with a hack in the pwrseq driver, it was clear that
> the issue is applicable to all pwrctl drivers. Hence, this series tries to
> address the issue in the PCI/pwrctl integration.
>
> - Mani
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
> Manivannan Sadhasivam (5):
>       PCI/pwrctl: Use of_platform_device_create() to create pwrctl devices
>       PCI/pwrctl: Create pwrctl devices only if at least one power supply is present
>       PCI/pwrctl: Ensure that the pwrctl drivers are probed before the PCI client drivers
>       PCI/pwrctl: Move pwrctl device creation to its own helper function
>       PCI/pwrctl: Remove pwrctl device without iterating over all children of pwrctl parent
>
>  drivers/pci/bus.c         | 64 +++++++++++++++++++++++++++++++++++++++++------
>  drivers/pci/of.c          | 27 ++++++++++++++++++++
>  drivers/pci/pci.h         |  5 ++++
>  drivers/pci/pwrctl/core.c | 10 --------
>  drivers/pci/remove.c      | 17 ++++++-------
>  5 files changed, 96 insertions(+), 27 deletions(-)
> ---
> base-commit: 48dc7986beb60522eb217c0016f999cc7afaf0b7
> change-id: 20241022-pci-pwrctl-rework-a1b024158555
>
> Best regards,
> --
> Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>
>

Excellent work, thanks for doing this.

Tested on: sc8280xp-crd, RB5 and sm8450-hdk.

Tested-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Just a couple nits from my side under respective patches.

Bart

Re: [PATCH 0/5] PCI/pwrctl: Ensure that the pwrctl drivers are probed before PCI client drivers

[Krishna Chaitanya Chundru]

On 10/23/2024 4:00 PM, Bartosz Golaszewski wrote:
> On Tue, 22 Oct 2024 at 12:28, Manivannan Sadhasivam via B4 Relay
> <devnull+manivannan.sadhasivam.linaro.org@kernel.org> wrote:
>>
>> Hi,
>>
>> This series reworks the PCI/pwrctl integration to ensure that the pwrctl drivers
>> are always probed before the PCI client drivers. This series addresses a race
>> condition when both pwrctl and pwrctl/pwrseq drivers probe parallely (even when
>> the later one probes last). One such issue was reported for the Qcom X13s
>> platform with WLAN module and fixed with 'commit a9aaf1ff88a8 ("power:
>> sequencing: request the WLAN enable GPIO as-is")'.
>>
>> Though the issue was fixed with a hack in the pwrseq driver, it was clear that
>> the issue is applicable to all pwrctl drivers. Hence, this series tries to
>> address the issue in the PCI/pwrctl integration.
>>
>> - Mani
>>
>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>> ---
>> Manivannan Sadhasivam (5):
>>        PCI/pwrctl: Use of_platform_device_create() to create pwrctl devices
>>        PCI/pwrctl: Create pwrctl devices only if at least one power supply is present
>>        PCI/pwrctl: Ensure that the pwrctl drivers are probed before the PCI client drivers
>>        PCI/pwrctl: Move pwrctl device creation to its own helper function
>>        PCI/pwrctl: Remove pwrctl device without iterating over all children of pwrctl parent
>>
>>   drivers/pci/bus.c         | 64 +++++++++++++++++++++++++++++++++++++++++------
>>   drivers/pci/of.c          | 27 ++++++++++++++++++++
>>   drivers/pci/pci.h         |  5 ++++
>>   drivers/pci/pwrctl/core.c | 10 --------
>>   drivers/pci/remove.c      | 17 ++++++-------
>>   5 files changed, 96 insertions(+), 27 deletions(-)
>> ---
>> base-commit: 48dc7986beb60522eb217c0016f999cc7afaf0b7
>> change-id: 20241022-pci-pwrctl-rework-a1b024158555
>>
>> Best regards,
>> --
>> Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>>
>>
> 
> Excellent work, thanks for doing this.
> 
> Tested on: sc8280xp-crd, RB5 and sm8450-hdk.
> 
> Tested-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Just a couple nits from my side under respective patches.
> 
> Bart
Tested on: qcs6490-rb3gen board with work in progress qps615 pcie switch

Tested-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>

- Krishna Chaitanya.
> 

Re: [PATCH 1/5] PCI/pwrctl: Use of_platform_device_create() to create pwrctl devices

[Manivannan Sadhasivam]

On Wed, Oct 23, 2024 at 11:18:51AM +0200, Bartosz Golaszewski wrote:
> On Tue, 22 Oct 2024 at 12:28, Manivannan Sadhasivam via B4 Relay
> <devnull+manivannan.sadhasivam.linaro.org@kernel.org> wrote:
> >
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> >
> > of_platform_populate() API creates platform devices by descending through
> > the children of the parent node. But it provides no control over the child
> > nodes, which makes it difficult to add checks for the child nodes in the
> > future. So use of_platform_device_create() API together with
> > for_each_child_of_node_scoped() so that it is possible to add checks for
> > each node before creating the platform device.
> >
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >  drivers/pci/bus.c | 12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> > index 55c853686051..959044b059b5 100644
> > --- a/drivers/pci/bus.c
> > +++ b/drivers/pci/bus.c
> > @@ -13,6 +13,7 @@
> >  #include <linux/ioport.h>
> >  #include <linux/of.h>
> >  #include <linux/of_platform.h>
> > +#include <linux/platform_device.h>
> >  #include <linux/proc_fs.h>
> >  #include <linux/slab.h>
> >
> > @@ -329,6 +330,7 @@ void __weak pcibios_bus_add_device(struct pci_dev *pdev) { }
> >  void pci_bus_add_device(struct pci_dev *dev)
> >  {
> >         struct device_node *dn = dev->dev.of_node;
> > +       struct platform_device *pdev;
> >         int retval;
> >
> >         /*
> > @@ -351,11 +353,11 @@ void pci_bus_add_device(struct pci_dev *dev)
> >         pci_dev_assign_added(dev, true);
> >
> >         if (dev_of_node(&dev->dev) && pci_is_bridge(dev)) {
> > -               retval = of_platform_populate(dev_of_node(&dev->dev), NULL, NULL,
> > -                                             &dev->dev);
> > -               if (retval)
> > -                       pci_err(dev, "failed to populate child OF nodes (%d)\n",
> > -                               retval);
> > +               for_each_child_of_node_scoped(dn, child) {
> 
> Since we won't be populating any disabled nodes, I'd use
> for_each_available_child_of_node_scoped() here.
> 

Ack.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 4/5] PCI/pwrctl: Move pwrctl device creation to its own helper function

[Manivannan Sadhasivam]

On Wed, Oct 23, 2024 at 12:23:57PM +0200, Bartosz Golaszewski wrote:
> On Tue, 22 Oct 2024 at 12:28, Manivannan Sadhasivam via B4 Relay
> <devnull+manivannan.sadhasivam.linaro.org@kernel.org> wrote:
> >
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> >
> > This makes the pci_bus_add_device() API easier to maintain. Also add more
> > comments to the helper to describe how the devices are created.
> >
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >  drivers/pci/bus.c | 59 ++++++++++++++++++++++++++++++++++++++-----------------
> >  1 file changed, 41 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> > index 351af581904f..c4cae1775c9e 100644
> > --- a/drivers/pci/bus.c
> > +++ b/drivers/pci/bus.c
> > @@ -321,6 +321,46 @@ void __weak pcibios_resource_survey_bus(struct pci_bus *bus) { }
> >
> >  void __weak pcibios_bus_add_device(struct pci_dev *pdev) { }
> >
> > +/*
> > + * Create pwrctl devices (if required) for the PCI devices to handle the power
> > + * state.
> > + */
> > +static void pci_pwrctl_create_devices(struct pci_dev *dev)
> > +{
> > +       struct device_node *np = dev_of_node(&dev->dev);
> > +       struct device *parent = &dev->dev;
> > +       struct platform_device *pdev;
> > +
> > +       /*
> > +        * First ensure that we are starting from a PCI bridge and it has a
> > +        * corresponding devicetree node.
> > +        */
> > +       if (np && pci_is_bridge(dev)) {
> > +               /*
> > +                * Now look for the child PCI device nodes and create pwrctl
> > +                * devices for them. The pwrctl device drivers will manage the
> > +                * power state of the devices.
> > +                */
> > +               for_each_child_of_node_scoped(np, child) {
> > +                       /*
> > +                        * First check whether the pwrctl 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_is_supply_present(child)) {
> > +                               pci_dbg(dev, "skipping OF node: %s\n", child->name);
> > +                               return;
> > +                       }
> > +
> > +                       /* Now create the pwrctl device */
> > +                       pdev = of_platform_device_create(child, NULL, parent);
> > +                       if (!pdev)
> > +                               pci_err(dev, "failed to create OF node: %s\n", child->name);
> > +               }
> > +       }
> > +}
> > +
> >  /**
> >   * pci_bus_add_device - start driver for a single device
> >   * @dev: device to add
> > @@ -345,24 +385,7 @@ void pci_bus_add_device(struct pci_dev *dev)
> >         pci_proc_attach_device(dev);
> >         pci_bridge_d3_update(dev);
> >
> > -       if (dev_of_node(&dev->dev) && pci_is_bridge(dev)) {
> > -               for_each_child_of_node_scoped(dn, child) {
> > -                       /*
> > -                        * First check whether the pwrctl device 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_is_supply_present(child)) {
> > -                               pci_dbg(dev, "skipping OF node: %s\n", child->name);
> > -                               continue;
> > -                       }
> > -
> > -                       pdev = of_platform_device_create(child, NULL, &dev->dev);
> > -                       if (!pdev)
> > -                               pci_err(dev, "failed to create OF node: %s\n", child->name);
> > -               }
> > -       }
> > +       pci_pwrctl_create_devices(dev);
> >
> >         /*
> >          * Create a device link between the PCI device and pwrctl device (if
> >
> > --
> > 2.25.1
> >
> >
> 
> Would it be possible to move this into drivers/pwrctl/ and provide a
> header stub for when PCI_PWRCTL is disabled in Kconfig?
> 

Unfortunately, no. Because the pwrctl drivers are modular and PCI core is
built-in. So if we use the pwrctl APIs in PCI core, then it will require pwrctl
to always be built-in, which we do not want.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 4/5] PCI/pwrctl: Move pwrctl device creation to its own helper function

[Bartosz Golaszewski]

On Fri, Oct 25, 2024 at 10:05 AM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> >
> > Would it be possible to move this into drivers/pwrctl/ and provide a
> > header stub for when PCI_PWRCTL is disabled in Kconfig?
> >
>
> Unfortunately, no. Because the pwrctl drivers are modular and PCI core is
> built-in. So if we use the pwrctl APIs in PCI core, then it will require pwrctl
> to always be built-in, which we do not want.
>

Got it.

Bartosz

Re: [PATCH 0/5] PCI/pwrctl: Ensure that the pwrctl drivers are probed before PCI client drivers

[Krzysztof Wilczyński]

Hello,

> This series reworks the PCI/pwrctl integration to ensure that the pwrctl drivers
> are always probed before the PCI client drivers. This series addresses a race
> condition when both pwrctl and pwrctl/pwrseq drivers probe parallely (even when
> the later one probes last). One such issue was reported for the Qcom X13s
> platform with WLAN module and fixed with 'commit a9aaf1ff88a8 ("power:
> sequencing: request the WLAN enable GPIO as-is")'.
> 
> Though the issue was fixed with a hack in the pwrseq driver, it was clear that
> the issue is applicable to all pwrctl drivers. Hence, this series tries to
> address the issue in the PCI/pwrctl integration.

Applied to bwctrl, thank you!

[01/05] PCI/pwrctl: Use of_platform_device_create() to create pwrctl devices
        https://git.kernel.org/pci/pci/c/d2b6619e7419

[02/05] PCI/pwrctl: Create pwrctl devices only if at least one power supply is present
        https://git.kernel.org/pci/pci/c/5f2710a4c275

[03/05] PCI/pwrctl: Ensure that the pwrctl drivers are probed before the PCI client drivers
        https://git.kernel.org/pci/pci/c/4c963d4c13b9

[04/05] PCI/pwrctl: Move pwrctl device creation to its own helper function
        https://git.kernel.org/pci/pci/c/73ae23a6af78

[05/05] PCI/pwrctl: Remove pwrctl device without iterating over all children of pwrctl parent
        https://git.kernel.org/pci/pci/c/5ccc52fd1e5a

	Krzysztof

Re: [PATCH 0/5] PCI/pwrctl: Ensure that the pwrctl drivers are probed before PCI client drivers

[Bjorn Helgaas]

On Mon, Nov 04, 2024 at 05:31:07AM +0900, Krzysztof Wilczyński wrote:
> Hello,
> 
> > This series reworks the PCI/pwrctl integration to ensure that the pwrctl drivers
> > are always probed before the PCI client drivers. This series addresses a race
> > condition when both pwrctl and pwrctl/pwrseq drivers probe parallely (even when
> > the later one probes last). One such issue was reported for the Qcom X13s
> > platform with WLAN module and fixed with 'commit a9aaf1ff88a8 ("power:
> > sequencing: request the WLAN enable GPIO as-is")'.
> > 
> > Though the issue was fixed with a hack in the pwrseq driver, it was clear that
> > the issue is applicable to all pwrctl drivers. Hence, this series tries to
> > address the issue in the PCI/pwrctl integration.
> 
> Applied to bwctrl, thank you!

Should be pci/pwrctl.  bwctrl (bandwidth control) and pwrctl (power
control) are quite different despite the confusingly similar names.

> [01/05] PCI/pwrctl: Use of_platform_device_create() to create pwrctl devices
>         https://git.kernel.org/pci/pci/c/d2b6619e7419
> 
> [02/05] PCI/pwrctl: Create pwrctl devices only if at least one power supply is present
>         https://git.kernel.org/pci/pci/c/5f2710a4c275
> 
> [03/05] PCI/pwrctl: Ensure that the pwrctl drivers are probed before the PCI client drivers
>         https://git.kernel.org/pci/pci/c/4c963d4c13b9
> 
> [04/05] PCI/pwrctl: Move pwrctl device creation to its own helper function
>         https://git.kernel.org/pci/pci/c/73ae23a6af78
> 
> [05/05] PCI/pwrctl: Remove pwrctl device without iterating over all children of pwrctl parent
>         https://git.kernel.org/pci/pci/c/5ccc52fd1e5a
> 
> 	Krzysztof

Re: [PATCH 0/5] PCI/pwrctl: Ensure that the pwrctl drivers are probed before PCI client drivers

[Krzysztof Wilczyński]

Hello,

> > > This series reworks the PCI/pwrctl integration to ensure that the pwrctl drivers
> > > are always probed before the PCI client drivers. This series addresses a race
> > > condition when both pwrctl and pwrctl/pwrseq drivers probe parallely (even when
> > > the later one probes last). One such issue was reported for the Qcom X13s
> > > platform with WLAN module and fixed with 'commit a9aaf1ff88a8 ("power:
> > > sequencing: request the WLAN enable GPIO as-is")'.
> > > 
> > > Though the issue was fixed with a hack in the pwrseq driver, it was clear that
> > > the issue is applicable to all pwrctl drivers. Hence, this series tries to
> > > address the issue in the PCI/pwrctl integration.
> > 
> > Applied to bwctrl, thank you!
> 
> Should be pci/pwrctl.  bwctrl (bandwidth control) and pwrctl (power
> control) are quite different despite the confusingly similar names.

Correct.  I moved patches to the correct branch.  Sorry about that!

	Krzysztof