[v3] cpuidle: big.LITTLE: add of_node_put()
diff mbox series

Message ID 20181120161451.21149-1-tiny.windzz@gmail.com
State New
Headers show
Series
  • [v3] cpuidle: big.LITTLE: add of_node_put()
Related show

Commit Message

Frank Lee Nov. 20, 2018, 4:14 p.m. UTC
of_find_node_by_path() acquires a reference to the node
returned by it and that reference needs to be dropped by its caller.
bl_idle_init() doesn't do that, so fix it.

Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
Changes in v3:
-update changelog
---
 drivers/cpuidle/cpuidle-big_little.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Frank Lee Dec. 9, 2018, 6:20 a.m. UTC | #1
On Wed, Nov 21, 2018 at 12:14 AM Yangtao Li <tiny.windzz@gmail.com> wrote:
>
> of_find_node_by_path() acquires a reference to the node
> returned by it and that reference needs to be dropped by its caller.
> bl_idle_init() doesn't do that, so fix it.
>
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> ---
> Changes in v3:
> -update changelog
> ---
>  drivers/cpuidle/cpuidle-big_little.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cpuidle/cpuidle-big_little.c b/drivers/cpuidle/cpuidle-big_little.c
> index db2ede565f1a..650f063ef809 100644
> --- a/drivers/cpuidle/cpuidle-big_little.c
> +++ b/drivers/cpuidle/cpuidle-big_little.c
> @@ -174,8 +174,12 @@ static int __init bl_idle_init(void)
>         /*
>          * Initialize the driver just for a compliant set of machines
>          */
> -       if (!of_match_node(compatible_machine_match, root))
> +       if (!of_match_node(compatible_machine_match, root)){
> +               of_node_put(root);
>                 return -ENODEV;
> +       }
> +
> +       of_node_put(root);
>
>         if (!mcpm_is_available())
>                 return -EUNATCH;
> --
> 2.17.0
>
ping......
Rafael J. Wysocki Dec. 10, 2018, 11:31 a.m. UTC | #2
On Sunday, December 9, 2018 7:20:26 AM CET Frank Lee wrote:
> On Wed, Nov 21, 2018 at 12:14 AM Yangtao Li <tiny.windzz@gmail.com> wrote:
> >
> > of_find_node_by_path() acquires a reference to the node
> > returned by it and that reference needs to be dropped by its caller.
> > bl_idle_init() doesn't do that, so fix it.
> >
> > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> > ---
> > Changes in v3:
> > -update changelog
> > ---
> >  drivers/cpuidle/cpuidle-big_little.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/cpuidle/cpuidle-big_little.c b/drivers/cpuidle/cpuidle-big_little.c
> > index db2ede565f1a..650f063ef809 100644
> > --- a/drivers/cpuidle/cpuidle-big_little.c
> > +++ b/drivers/cpuidle/cpuidle-big_little.c
> > @@ -174,8 +174,12 @@ static int __init bl_idle_init(void)
> >         /*
> >          * Initialize the driver just for a compliant set of machines
> >          */
> > -       if (!of_match_node(compatible_machine_match, root))
> > +       if (!of_match_node(compatible_machine_match, root)){
> > +               of_node_put(root);
> >                 return -ENODEV;
> > +       }
> > +
> > +       of_node_put(root);
> >
> >         if (!mcpm_is_available())
> >                 return -EUNATCH;
> > --
> > 2.17.0
> >
> ping......

I've been waiting for the maintainers of this driver to speak up.

I'll pick up the patch later today or tomorrow unless there are objections.

Thanks,
Rafael
Daniel Lezcano Dec. 10, 2018, 11:59 a.m. UTC | #3
Hi Yangtao,


On 20/11/2018 17:14, Yangtao Li wrote:
> of_find_node_by_path() acquires a reference to the node
> returned by it and that reference needs to be dropped by its caller.
> bl_idle_init() doesn't do that, so fix it.
> 
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> ---
> Changes in v3:
> -update changelog
> ---
>  drivers/cpuidle/cpuidle-big_little.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpuidle/cpuidle-big_little.c b/drivers/cpuidle/cpuidle-big_little.c
> index db2ede565f1a..650f063ef809 100644
> --- a/drivers/cpuidle/cpuidle-big_little.c
> +++ b/drivers/cpuidle/cpuidle-big_little.c
> @@ -174,8 +174,12 @@ static int __init bl_idle_init(void)
>  	/*
>  	 * Initialize the driver just for a compliant set of machines
>  	 */
> -	if (!of_match_node(compatible_machine_match, root))
> +	if (!of_match_node(compatible_machine_match, root)){
> +		of_node_put(root);
>  		return -ENODEV;
> +	}
> +
> +	of_node_put(root);

Don't duplicate the of_node_put.

I suggest:

const struct of_device_id *match_id;

[ ... ]


match_id = of_match_node(compatible_machine_match, root);

of_node_put(root);

if (!match_id)
	return -ENODEV;


>  	if (!mcpm_is_available())
>  		return -EUNATCH;
>
Frank Lee Dec. 10, 2018, 4:27 p.m. UTC | #4
On Mon, Dec 10, 2018 at 7:59 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
>
> Hi Yangtao,
>
>
> On 20/11/2018 17:14, Yangtao Li wrote:
> > of_find_node_by_path() acquires a reference to the node
> > returned by it and that reference needs to be dropped by its caller.
> > bl_idle_init() doesn't do that, so fix it.
> >
> > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> > ---
> > Changes in v3:
> > -update changelog
> > ---
> >  drivers/cpuidle/cpuidle-big_little.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/cpuidle/cpuidle-big_little.c b/drivers/cpuidle/cpuidle-big_little.c
> > index db2ede565f1a..650f063ef809 100644
> > --- a/drivers/cpuidle/cpuidle-big_little.c
> > +++ b/drivers/cpuidle/cpuidle-big_little.c
> > @@ -174,8 +174,12 @@ static int __init bl_idle_init(void)
> >       /*
> >        * Initialize the driver just for a compliant set of machines
> >        */
> > -     if (!of_match_node(compatible_machine_match, root))
> > +     if (!of_match_node(compatible_machine_match, root)){
> > +             of_node_put(root);
> >               return -ENODEV;
> > +     }
> > +
> > +     of_node_put(root);
>
> Don't duplicate the of_node_put.
>
> I suggest:
>
> const struct of_device_id *match_id;
>
> [ ... ]
>
>
> match_id = of_match_node(compatible_machine_match, root);
>
> of_node_put(root);
>
> if (!match_id)
>         return -ENODEV;
>
>
Thank you for your suggestion, the new one has been sent.

Yours,
Yangtao
> >       if (!mcpm_is_available())
> >               return -EUNATCH;
> >
>
>
> --
>  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
>

Patch
diff mbox series

diff --git a/drivers/cpuidle/cpuidle-big_little.c b/drivers/cpuidle/cpuidle-big_little.c
index db2ede565f1a..650f063ef809 100644
--- a/drivers/cpuidle/cpuidle-big_little.c
+++ b/drivers/cpuidle/cpuidle-big_little.c
@@ -174,8 +174,12 @@  static int __init bl_idle_init(void)
 	/*
 	 * Initialize the driver just for a compliant set of machines
 	 */
-	if (!of_match_node(compatible_machine_match, root))
+	if (!of_match_node(compatible_machine_match, root)){
+		of_node_put(root);
 		return -ENODEV;
+	}
+
+	of_node_put(root);
 
 	if (!mcpm_is_available())
 		return -EUNATCH;