On 28/10/2024 12:24, Daniel Lezcano wrote:

Hi Javier,

thanks for spotting the issue

On 13/10/2024 12:14, Javier Carrasco wrote:

...

of_find_compatible_node() increments the node's refcount, and it must be decremented again with a call to of_node_put() when the pointer is no longer required to avoid leaking memory.

Add the missing calls to of_node_put() in dmtimer_percpu_quirck_init() for the 'arm_timer' device node.

Cc: stable@vger.kernel.org Fixes: 25de4ce5ed02 ("clocksource/drivers/timer-ti-dm: Handle dra7 timer wrap errata i940") Signed-off-by: Javier Carrasco javier.carrasco.cruz@gmail.com

---

drivers/clocksource/timer-ti-dm-systimer.c | 2 ++   1 file changed, 2 insertions(+)

diff --git a/drivers/clocksource/timer-ti-dm-systimer.c b/drivers/ clocksource/timer-ti-dm-systimer.c index c2dcd8d68e45..23be1d21ce21 100644 --- a/drivers/clocksource/timer-ti-dm-systimer.c +++ b/drivers/clocksource/timer-ti-dm-systimer.c @@ -691,8 +691,10 @@ static int __init dmtimer_percpu_quirk_init(struct device_node *np, u32 pa)       arm_timer = of_find_compatible_node(NULL, NULL, "arm,armv7-timer");       if (of_device_is_available(arm_timer)) {           pr_warn_once("ARM architected timer wrap issue i940 detected\n"); +        of_node_put(arm_timer);           return 0;       } +    of_node_put(arm_timer);

Best practice would be to group of_node_put into a single place.

bool available;

[ ... ]

available = of_device_is_available(arm_timer);     of_node_put(arm_timer);

if (available) {         pr_warn_once("ARM architected timer wrap issue i940 detected\n");         return 0;     }

...

if (pa == 0x4882c000)           /* dra7 dmtimer15 */           return dmtimer_percpu_timer_init(np, 0);

---

base-commit: d61a00525464bfc5fe92c6ad713350988e492b88 change-id: 20241013-timer-ti-dm-systimer-of_node_put-d42735687698

Best regards,

Hi Daniel, thanks for your feedback.

Actually, if we are going to refactor the code, we would not need the extra variable or even the call to of_node_put(), since we could use the __free() macro. That would be a second patch after the fix, which could stay as it is without refactoring, because it is only to backport the missing calls to of_node_put().

I can send a v2 with the extra patch leaving this one as it is, or if really desired, with the available variable.

Best regards, Javier Carrasco