Hi Lizhi,

On 2024/07/26 11:45 AM, Lizhi Hou wrote:

On 7/26/24 10:52, Rob Herring wrote:

quoted

On Thu, Jul 25, 2024 at 6:06 PM Lizhi Hou [off-list ref] wrote:

quoted

Hi Amit,


I try to follow the option which add a OF flag. If Rob is ok with this,
I would suggest to use it instead of V1 patch

diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index dda6092e6d3a..a401ed0463d9 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c

@@ -382,6 +382,11 @@ void of_node_release(struct kobject *kobj)
                                 __func__, node);
          }

+       if (of_node_check_flag(node, OF_CREATED_WITH_CSET)) {
+               of_changeset_revert(node->data);
+               of_changeset_destroy(node->data);
+       }

What happens if multiple nodes are created in the changeset?

Ok. multiple nodes will not work.

quoted

quoted

+
          if (node->child)
                  pr_err("ERROR: %s() unexpected children for %pOF/%s\n",
                          __func__, node->parent, node->full_name);

@@ -507,6 +512,7 @@ struct device_node *of_changeset_create_node(struct

of_changeset *ocs,
          np = __of_node_dup(NULL, full_name);
          if (!np)
                  return NULL;
+       of_node_set_flag(np, OF_CREATED_WITH_CSET);

This should be set where the data ptr is set.

Ok. It sounds the fix could be simplified to 3 lines change.

Thanks for the patch. The hot-plug and hot-unplug of PCI device seem to work
fine as expected. I see this patch would attempt to remove only the nodes which
were created in `of_pci_make_dev_node()` with the help of the newly introduced
flag, which looks good to me.

Also, since a call to `of_pci_make_dev_node()` from `pci_bus_add_device()`, that
creates devices nodes only for bridge devices, is conditional on
`pci_is_bridge()`, it only makes sense to retain the logical symmetry and call
`of_pci_remove_node()` conditionally on `pci_is_bridge()` as well in
`pci_stop_dev()`. Hence, I would like to propose the below change along with the
above patch:

diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 910387e5bdbf..c6394bf562cd 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c

@@ -23,7 +23,8 @@ static void pci_stop_dev(struct pci_dev *dev)
                device_release_driver(&dev->dev);
                pci_proc_detach_device(dev);
                pci_remove_sysfs_dev_files(dev);
-               of_pci_remove_node(dev);
+               if (pci_is_bridge(dev))
+                       of_pci_remove_node(dev);
 
                pci_dev_assign_added(dev, false);
        }

Please let me know of your thoughts on this and based on that I can spin the v3
of this patch.

In addition to this, can this patch be taken as part of 6.11 as a bug fix?

Thanks,
Amit