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

Manivannan Sadhasivam via B4 Relay posted 5 patches 1 month ago
There is a newer version of this series
[PATCH 1/5] PCI/pwrctl: Use of_platform_device_create() to create pwrctl devices
Posted by Manivannan Sadhasivam via B4 Relay 1 month ago
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
Re: [PATCH 1/5] PCI/pwrctl: Use of_platform_device_create() to create pwrctl devices
Posted by Bartosz Golaszewski 1 month ago
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 1/5] PCI/pwrctl: Use of_platform_device_create() to create pwrctl devices
Posted by Manivannan Sadhasivam 1 month ago
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

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