Messages in this thread | | | Date | Tue, 25 Jan 2022 11:18:09 +0200 | From | Heikki Krogerus <> | Subject | Re: [PATCH v2 2/2] usb: ulpi: Call of_node_put correctly |
| |
On Mon, Jan 24, 2022 at 12:33:44PM -0500, Sean Anderson wrote: > of_node_put should always be called on device nodes gotten from > of_get_*. Additionally, it should only be called after there are no > remaining users. To address the first issue, call of_node_put if later > steps in ulpi_register fail. To address the latter, call of_node_put > only after calling device_unregister.
This looks like a fix, but you don't have the fix tag.
> Signed-off-by: Sean Anderson <sean.anderson@seco.com> > --- > > Changes in v2: > - New > > drivers/usb/common/ulpi.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c > index 87deb514eb78..c6ba72544f2b 100644 > --- a/drivers/usb/common/ulpi.c > +++ b/drivers/usb/common/ulpi.c > @@ -301,11 +301,11 @@ static int ulpi_register(struct device *dev, struct ulpi *ulpi) > > ret = ulpi_read_id(ulpi); > if (ret) > - return ret; > + goto err; > > ret = device_register(&ulpi->dev); > if (ret) > - return ret; > + goto err;
I think there is another bug in the code here. Missing put_device().
If you first fix that, you should then be able to call fwnode_handle_put() (instead of of_node_put()) from ulpi_dev_release(), and that should cover all cases.
> root = debugfs_create_dir(dev_name(dev), ULPI_ROOT); > debugfs_create_file("regs", 0444, root, ulpi, &ulpi_regs_ops); > @@ -314,6 +314,10 @@ static int ulpi_register(struct device *dev, struct ulpi *ulpi) > ulpi->id.vendor, ulpi->id.product); > > return 0; > + > +err: > + of_node_put(ulpi->dev.of_node); > + return ret;
So no need for that.
> } > > /** > @@ -357,8 +361,8 @@ void ulpi_unregister_interface(struct ulpi *ulpi) > { > debugfs_remove_recursive(debugfs_lookup(dev_name(&ulpi->dev), > ULPI_ROOT)); > - of_node_put(ulpi->dev.of_node); > device_unregister(&ulpi->dev); > + of_node_put(ulpi->dev.of_node); > }
And here you can just remove that of_node_put() call.
thanks,
-- heikki
| |