August 25, 2021 9:55 PM, "Rob Herring" <robh@xxxxxxxxxx> wrote: > On Wed, Aug 25, 2021 at 3:34 AM Yajun Deng <yajun.deng@xxxxxxxxx> wrote: > >> device_del() should be called first and then called put_device() in >> unregister path, becase if that the final reference count, the device >> will be cleaned up via device_release() above. So use device_unregister() >> instead. >> >> Fixes: 9885440b16b8 (PCI: Fix pci_host_bridge struct device release/free handling) >> Signed-off-by: Yajun Deng <yajun.deng@xxxxxxxxx> >> --- >> drivers/pci/probe.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) > > NAK. > > The current code is correct. Go read the comments for device_add/device_del. But the device_unregister() is only contains device_del() and put_device(). It just put device_del() before put_device(). > >> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c >> index 0ec5c792c27d..abd481a15a17 100644 >> --- a/drivers/pci/probe.c >> +++ b/drivers/pci/probe.c >> @@ -994,9 +994,7 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge) >> return 0; >> >> unregister: > > We get here if device_register() failed. Calling device_unregister() > in that case is never right. > >> - put_device(&bridge->dev); > > This is for the get_device() we do above, not the get the driver core does. > >> - device_del(&bridge->dev); > > This undoes the device_add() we do following the comment: "NOTE: this > should be called manually _iff_ device_add() was also called > manually." > >> - >> + device_unregister(&bridge->dev); >> free: >> kfree(bus); >> return err; >> -- >> 2.32.0
