Re: [PATCH v2] clocksource/drivers/integrator-ap: add missing of_node_put()

[Daniel Lezcano]

On 25/11/2018 06:00, 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.
> integrator_ap_timer_init_of() doesn't do that.The pri_node and the
> sec_node are used as an identifier to compare against the current node,
> we can directly drop the refcount after getting the node from path as
> it is not used aspointer. In addition, a single variable is needed,
> so fix it.

I can apply the patch after adding a comment in the code below or
alternatively you can resend a V3 after adding a comment yourself.

What is your preference?

> Signed-off-by: Yangtao Li <tiny.windzz@xxxxxxxxx>
> ---
> Changes in v2:
> -update changeelog
> -simplify fix
> -change two variable to one
> ---
>  drivers/clocksource/timer-integrator-ap.c | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/clocksource/timer-integrator-ap.c b/drivers/clocksource/timer-integrator-ap.c
> index 76e526f58620..bbd0977d579a 100644
> --- a/drivers/clocksource/timer-integrator-ap.c
> +++ b/drivers/clocksource/timer-integrator-ap.c
> @@ -181,8 +181,7 @@ static int __init integrator_ap_timer_init_of(struct device_node *node)
>  	int irq;
>  	struct clk *clk;
>  	unsigned long rate;
> -	struct device_node *pri_node;
> -	struct device_node *sec_node;
> +	struct device_node *alias_node;
>  
>  	base = of_io_request_and_map(node, 0, "integrator-timer");
>  	if (IS_ERR(base))
> @@ -204,7 +203,14 @@ static int __init integrator_ap_timer_init_of(struct device_node *node)
>  		return err;
>  	}
>  
> -	pri_node = of_find_node_by_path(path);
> +	alias_node = of_find_node_by_path(path);
> +
> +	/* Drop the refcount of node */
> +	of_node_put(alias_node);
> +
> +	if (node == alias_node)
> +		/* The primary timer lacks IRQ, use as clocksource */
> +		return integrator_clocksource_init(rate, base);
>  
>  	err = of_property_read_string(of_aliases,
>  				"arm,timer-secondary", &path);
> @@ -213,14 +219,12 @@ static int __init integrator_ap_timer_init_of(struct device_node *node)
>  		return err;
>  	}
>  
> +	alias_node = of_find_node_by_path(path);
>  
> -	sec_node = of_find_node_by_path(path);
> -
> -	if (node == pri_node)
> -		/* The primary timer lacks IRQ, use as clocksource */
> -		return integrator_clocksource_init(rate, base);
> +	/* Drop the refcount of node */
> +	of_node_put(alias_node);
>  
> -	if (node == sec_node) {
> +	if (node == alias_node) {
>  		/* The secondary timer will drive the clock event */
>  		irq = irq_of_parse_and_map(node, 0);
>  		return integrator_clockevent_init(rate, base, irq);
> 


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