Re: [PATCH 1/3] of: device: Do some clean up with use of __free()

[Rob Herring <robh@kernel.org>]

On Fri, Aug 30, 2024 at 10:06:24AM +0800, Zhang Zekun wrote:
> __free() provides a scoped of_node_put() functionality to put the
> device_node automatically, and we don't need to call of_node_put()
> directly. Let's simplify the code a bit with the use of __free().
> 
> Signed-off-by: Zhang Zekun <zhangzekun11@huawei.com>
> ---
>  drivers/of/device.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index edf3be197265..7a71ef2aa16e 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -35,7 +35,7 @@ EXPORT_SYMBOL(of_match_device);
>  static void
>  of_dma_set_restricted_buffer(struct device *dev, struct device_node *np)
>  {
> -	struct device_node *node, *of_node = dev->of_node;
> +	struct device_node *of_node = dev->of_node;
>  	int count, i;
>  
>  	if (!IS_ENABLED(CONFIG_DMA_RESTRICTED_POOL))
> @@ -54,17 +54,16 @@ of_dma_set_restricted_buffer(struct device *dev, struct device_node *np)
>  	}
>  
>  	for (i = 0; i < count; i++) {
> -		node = of_parse_phandle(of_node, "memory-region", i);
> +		struct device_node *node __free(device_node) =
> +			of_parse_phandle(of_node, "memory-region", i);
>  		/*
>  		 * There might be multiple memory regions, but only one
>  		 * restricted-dma-pool region is allowed.
>  		 */
>  		if (of_device_is_compatible(node, "restricted-dma-pool") &&
>  		    of_device_is_available(node)) {
> -			of_node_put(node);
>  			break;
>  		}
> -		of_node_put(node);
>  	}

Actually, I'd re-write this function like this (untested):

static void
of_dma_set_restricted_buffer(struct device *dev, struct device_node *np)
{
	struct device_node *of_node = dev->of_node;
	struct of_phandle_iterator it;
	int rc, match = -1, i = 0;

	if (!IS_ENABLED(CONFIG_DMA_RESTRICTED_POOL))
		return;

	/*
	 * If dev->of_node doesn't exist or doesn't contain memory-region, try
	 * the OF node having DMA configuration.
	 */
	if (!of_property_present(of_node, "memory-region"))
		of_node = np;

	of_for_each_phandle(&it, of_node, rc, "memory-region", NULL, 0) {
		/*
		 * There might be multiple memory regions, but only one
		 * restricted-dma-pool region is allowed.
		 */
		if ((match < 0) && of_device_is_compatible(it.node, "restricted-dma-pool") &&
		    of_device_is_available(it.node)) {
			match = i;
			if (of_reserved_mem_device_init_by_idx(dev, of_node, i))
				dev_warn(dev, "failed to initialise \"restricted-dma-pool\" memory node\n");
		}
		i++;
	}
}


of_parse_phandle() is implemented using of_for_each_phandle(), so every 
call to of_parse_phandle() is iterating i times.

Rob