lkml.org 
[lkml]   [2022]   [Jan]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2022-01-25 10:35    [W:0.096 / U:0.100 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site