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@xxxxxxxx>
> ---
>
> 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
