When calling of_parse_phandle_with_args(), the caller is responsible
to call of_node_put() to release the reference of device node.
In scpi_dev_domain_id, it does not release the reference.

Fixes: 45ca7df7c345 ("firmware: arm_scpi: add support to populate OPPs and get transition latency")

Signed-off-by: Felix Gu <gu_0233@qq.com>
---
 drivers/firmware/arm_scpi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
index 87c323de17b9..b6233f81ef3e 100644
--- a/drivers/firmware/arm_scpi.c
+++ b/drivers/firmware/arm_scpi.c
@@ -665,6 +665,7 @@ static int scpi_dev_domain_id(struct device *dev)
 				       0, &clkspec))
 		return -EINVAL;
 
+	of_node_put(clkspec.np);
 	return clkspec.args[0];
 }
 

---
base-commit: 9b7977f9e39b7768c70c2aa497f04e7569fd3e00
change-id: 20260116-arm_scpi-0132a9eee0c2

Best regards,
-- 
Felix Gu <gu_0233@qq.com>

On Fri, Jan 16, 2026 at 08:54:39PM +0800, Felix Gu wrote:
> When calling of_parse_phandle_with_args(), the caller is responsible
> to call of_node_put() to release the reference of device node.
> In scpi_dev_domain_id, it does not release the reference.
> 
> Fixes: 45ca7df7c345 ("firmware: arm_scpi: add support to populate OPPs and get transition latency")
> 

nit: delete the blank line after the Fixes tag.

> Signed-off-by: Felix Gu <gu_0233@qq.com>

Did you find this issue just by reviewing the code?  I read the comments
and then looked at a sample of four other callers and none of them
called of_node_put().  So it's a weird thing.  If this is really the rule
then we should have a static checker heuristic to enforce it.

regards,
dan carpenter

Hi Dan,

Thanks for the review.

I found this issue through code review.

 From the comments and implementation, I believe this is a problem.

The caller often misses calling of_node_put(), and I have found a lot of

similar issues in the driver code.


Best regards,

Felix Gu

在 2026/1/19 3:15, Dan Carpenter 写道:
> On Fri, Jan 16, 2026 at 08:54:39PM +0800, Felix Gu wrote:
>> When calling of_parse_phandle_with_args(), the caller is responsible
>> to call of_node_put() to release the reference of device node.
>> In scpi_dev_domain_id, it does not release the reference.
>>
>> Fixes: 45ca7df7c345 ("firmware: arm_scpi: add support to populate OPPs and get transition latency")
>>
> nit: delete the blank line after the Fixes tag.
>
>> Signed-off-by: Felix Gu <gu_0233@qq.com>
> Did you find this issue just by reviewing the code?  I read the comments
> and then looked at a sample of four other callers and none of them
> called of_node_put().  So it's a weird thing.  If this is really the rule
> then we should have a static checker heuristic to enforce it.
>
> regards,
> dan carpenter

On Sun, Jan 18, 2026 at 10:15:37PM +0300, Dan Carpenter wrote:
> On Fri, Jan 16, 2026 at 08:54:39PM +0800, Felix Gu wrote:
> > When calling of_parse_phandle_with_args(), the caller is responsible
> > to call of_node_put() to release the reference of device node.
> > In scpi_dev_domain_id, it does not release the reference.
> > 
> > Fixes: 45ca7df7c345 ("firmware: arm_scpi: add support to populate OPPs and get transition latency")
> > 
> 
> nit: delete the blank line after the Fixes tag.
> 
> > Signed-off-by: Felix Gu <gu_0233@qq.com>
> 
> Did you find this issue just by reviewing the code?  I read the comments
> and then looked at a sample of four other callers and none of them
> called of_node_put().  So it's a weird thing.  If this is really the rule
> then we should have a static checker heuristic to enforce it.
> 

I agree. I see bit of inconsistency at call sites especially in SC{P,M}I
related modules I maintain. It seem to me based on the description at
of_parse_phandle_with_args() definition and various other call sites as
examples, we need to have of_node_put(). That said, I don't have any easier
way to validate these, so I am happy for any reviews and recommendations.

-- 
Regards,
Sudeep

…
> In scpi_dev_domain_id, it does not release the reference.

See also once more:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.19-rc5#n94

Regards,
Markus

On Fri, Jan 16, 2026 at 02:36:58PM +0100, Markus Elfring wrote:
> …
> > In scpi_dev_domain_id, it does not release the reference.
> 
> See also once more:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.19-rc5#n94
> 

This is useless and looks like coming from the bot. Note to the author:
ignore this and no need to act on this.

-- 
Regards,
Sudeep

>> …
>>> In scpi_dev_domain_id, it does not release the reference.
>>
>> See also once more:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.19-rc5#n94
>>
> 
> This is useless

Do you really disagree here to the usage of imperative mood in change descriptions?


> and looks like coming from the bot.

I hope that views can become more constructive again.


> Note to the author:
> ignore this and no need to act on this.

Will other contributors get more positive attention for recurring patch review concerns?

Regards,
Markus

Markus, there are real issues to discuss with this patch.  You're
derailing the email thread for no reason.

regards,
dan carpenter