diff mbox series

ARM: ux500: add missing of_node_put()

Message ID 1555139666-948-10-git-send-email-hofrat@osadl.org (mailing list archive)
State Mainlined
Commit dbc3c6295195267ea7bc48d46030c7b244f8b11e
Headers show
Series ARM: ux500: add missing of_node_put() | expand

Commit Message

Nicholas Mc Guire April 13, 2019, 7:14 a.m. UTC
of_find_compatible_node() returns a pointer with refcount incremented
so both in the error path as well as after usage in soc_info_populate()
respectively actually b8500_read_soc_id() an explicit of_node_put is
needed to release backupram.

Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
Fixes: commit 18a992787896 ("ARM: ux500: move soc_id driver to drivers/soc")
---

Problem located with experimental cocinelle script

get_maintainer.pl only returns linux-kernel@vger.kernel.org for
this file ? Is MAINTAINERS entry missing ?

Not really sure about the referenced fixes commit 18a992787896
("ARM: ux500: move soc_id driver to drivers/soc") the commit log notes
only that the driver is being moved and not expected to change (v4.8)
but looking at the previous version in v4.7 it does seem that while
moving the driver there was also a relevant change to the driver code
including the switch to using of_find_compatible_node().

Patch was compiletested with: u8500_defconfig (implies
ONFIG_UX500_SOC_ID=y)

Patch is against 4.18-rc3 (localversion-next is next-20180706)

 drivers/soc/ux500/ux500-soc-id.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Ulf Hansson April 15, 2019, 9:52 a.m. UTC | #1
On Sat, 13 Apr 2019 at 09:20, Nicholas Mc Guire <hofrat@osadl.org> wrote:
>
>  of_find_compatible_node() returns a pointer with refcount incremented
> so both in the error path as well as after usage in soc_info_populate()
> respectively actually b8500_read_soc_id() an explicit of_node_put is
> needed to release backupram.
>
> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> Fixes: commit 18a992787896 ("ARM: ux500: move soc_id driver to drivers/soc")

Acked-by: Ulf Hansson <ulf.hansson@linaro.org>

> ---
>
> Problem located with experimental cocinelle script
>
> get_maintainer.pl only returns linux-kernel@vger.kernel.org for
> this file ? Is MAINTAINERS entry missing ?

drivers/soc/ux500 should be added to the ARM/NOMADIK/U300/Ux500
ARCHITECTURES section, which is maintained by Linus Walleij.

If you send a patch, I am sure Linus will ack it.

>
> Not really sure about the referenced fixes commit 18a992787896
> ("ARM: ux500: move soc_id driver to drivers/soc") the commit log notes
> only that the driver is being moved and not expected to change (v4.8)
> but looking at the previous version in v4.7 it does seem that while
> moving the driver there was also a relevant change to the driver code
> including the switch to using of_find_compatible_node().
>
> Patch was compiletested with: u8500_defconfig (implies
> ONFIG_UX500_SOC_ID=y)
>
> Patch is against 4.18-rc3 (localversion-next is next-20180706)
>
>  drivers/soc/ux500/ux500-soc-id.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/ux500/ux500-soc-id.c b/drivers/soc/ux500/ux500-soc-id.c
> index 6c1be74..e22597d 100644
> --- a/drivers/soc/ux500/ux500-soc-id.c
> +++ b/drivers/soc/ux500/ux500-soc-id.c
> @@ -203,10 +203,13 @@ static int __init ux500_soc_device_init(void)
>         ux500_setup_id();
>
>         soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> -       if (!soc_dev_attr)
> +       if (!soc_dev_attr) {
> +               of_node_put(backupram);
>                 return -ENOMEM;
> +       }
>
>         soc_info_populate(soc_dev_attr, backupram);
> +       of_node_put(backupram);
>
>         soc_dev = soc_device_register(soc_dev_attr);
>         if (IS_ERR(soc_dev)) {
> --
> 2.1.4
>
Linus Walleij April 16, 2019, 11:49 a.m. UTC | #2
On Sat, Apr 13, 2019 at 9:20 AM Nicholas Mc Guire <hofrat@osadl.org> wrote:

>  of_find_compatible_node() returns a pointer with refcount incremented
> so both in the error path as well as after usage in soc_info_populate()
> respectively actually b8500_read_soc_id() an explicit of_node_put is
> needed to release backupram.
>
> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> Fixes: commit 18a992787896 ("ARM: ux500: move soc_id driver to drivers/soc")

Patch applied to my Ux500 tree with Ulf's ACK.

Yours,
Linus Walleij
diff mbox series

Patch

diff --git a/drivers/soc/ux500/ux500-soc-id.c b/drivers/soc/ux500/ux500-soc-id.c
index 6c1be74..e22597d 100644
--- a/drivers/soc/ux500/ux500-soc-id.c
+++ b/drivers/soc/ux500/ux500-soc-id.c
@@ -203,10 +203,13 @@  static int __init ux500_soc_device_init(void)
 	ux500_setup_id();
 
 	soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
-	if (!soc_dev_attr)
+	if (!soc_dev_attr) {
+		of_node_put(backupram);
 		return -ENOMEM;
+	}
 
 	soc_info_populate(soc_dev_attr, backupram);
+	of_node_put(backupram);
 
 	soc_dev = soc_device_register(soc_dev_attr);
 	if (IS_ERR(soc_dev)) {