Re: [PATCH v1] PCI: rcar: Release temporary device tree node reference

[Marek Vasut]

On 8/2/26 1:38 AM, Yuho Choi wrote:
> of_find_matching_node() returns a referenced node even when it is used
> only as a boolean test. Keep the result in a scoped cleanup variable
> so the reference is released after the match check.
>
> Fixes: a115b1bd3af0 ("PCI: rcar: Add L1 link state fix into data abort hook")
> Signed-off-by: Yuho Choi <dbgh9129@xxxxxxxxx>
> ---
>   drivers/pci/controller/pcie-rcar-host.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/pcie-rcar-host.c b/drivers/pci/controller/pcie-rcar-host.c
> index cd9171eebc28..7ed1bc0732ce 100644
> --- a/drivers/pci/controller/pcie-rcar-host.c
> +++ b/drivers/pci/controller/pcie-rcar-host.c
> @@ -1122,7 +1122,10 @@ static const struct of_device_id rcar_pcie_abort_handler_of_match[] __initconst
>   
>   static int __init rcar_pcie_init(void)
>   {
> -	if (of_find_matching_node(NULL, rcar_pcie_abort_handler_of_match)) {
> +	struct device_node *np __free(device_node) =
> +		of_find_matching_node(NULL, rcar_pcie_abort_handler_of_match);
> +
> +	if (np) {
>   #ifdef CONFIG_ARM_LPAE
>   		hook_fault_code(17, rcar_pcie_aarch32_abort_handler, SIGBUS, 0,
>   				"asynchronous external abort");

How about this simpler version:

"
diff --git a/drivers/pci/controller/pcie-rcar-host.c 
b/drivers/pci/controller/pcie-rcar-host.c
index 213028052aa58..a4a42e435947f 100644
--- a/drivers/pci/controller/pcie-rcar-host.c
+++ b/drivers/pci/controller/pcie-rcar-host.c
@@ -1123,7 +1123,9 @@ static const struct of_device_id 
rcar_pcie_abort_handler_of_match[] __initconst

 static int __init rcar_pcie_init(void)
 {
-       if (of_find_matching_node(NULL, rcar_pcie_abort_handler_of_match)) {
+       struct device_node *np = of_find_matching_node(NULL, 
rcar_pcie_abort_handler_of_match);
+       if (np) {
+               of_node_put(np);
 #ifdef CONFIG_ARM_LPAE
                hook_fault_code(17, rcar_pcie_aarch32_abort_handler, 
SIGBUS, 0,
                                "asynchronous external abort");
"

Also, and this is more of a question for DT maintainers, would it make 
sense to introduce of_find_matching_node_and_match() variant called e.g. 
of_test_matching_node_exists() which would return boolean and would NOT 
call of_node_get() on the matched node in the first place, and then use 
it here ?