Re: [PATCH] net: Use of_reserved_mem_region_to_resource{_byname}() for "memory-region"

[Simon Horman <horms@kernel.org>]

On Thu, Jul 03, 2025 at 01:34:57PM -0500, Rob Herring (Arm) wrote:
> Use the newly added of_reserved_mem_region_to_resource{_byname}()
> functions to handle "memory-region" properties.
> 
> The error handling is a bit different for mtk_wed_mcu_load_firmware().
> A failed match of the "memory-region-names" would skip the entry, but
> then other errors in the lookup and retrieval of the address would not
> skip the entry. However, that distinction is not really important.
> Either the region is available and usable or it is not. So now, errors
> from of_reserved_mem_region_to_resource() are ignored so the region is
> simply skipped.

Thanks for explaining this, it's much appreciated.

> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---
>  drivers/net/ethernet/airoha/airoha_npu.c    | 25 ++++++----------
>  drivers/net/ethernet/mediatek/mtk_wed.c     | 24 ++++------------
>  drivers/net/ethernet/mediatek/mtk_wed_mcu.c | 32 +++++++--------------
>  drivers/net/ipa/ipa_main.c                  | 12 ++------

FWIIW, I would slightly prefer one patch per driver.

...

> diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
> index 351dd152f4f3..73c26fcfd85e 100644
> --- a/drivers/net/ethernet/mediatek/mtk_wed.c
> +++ b/drivers/net/ethernet/mediatek/mtk_wed.c
> @@ -1318,26 +1318,14 @@ mtk_wed_rro_ring_alloc(struct mtk_wed_device *dev, struct mtk_wed_ring *ring,
>  static int
>  mtk_wed_rro_alloc(struct mtk_wed_device *dev)
>  {
> -	struct reserved_mem *rmem;
> -	struct device_node *np;
> -	int index;
> +	struct resource res;
> +	int ret;
>  
> -	index = of_property_match_string(dev->hw->node, "memory-region-names",
> -					 "wo-dlm");
> -	if (index < 0)
> -		return index;
> -
> -	np = of_parse_phandle(dev->hw->node, "memory-region", index);
> -	if (!np)
> -		return -ENODEV;
> -
> -	rmem = of_reserved_mem_lookup(np);
> -	of_node_put(np);
> -
> -	if (!rmem)
> -		return -ENODEV;
> +	ret = of_reserved_mem_region_to_resource_byname(dev->hw->node, "wo-dlm", &res);

Please consider line-wrapping the line above so it is 80 columns wide or
less, as is still preferred for Networking code.

> +	if (ret)
> +		return ret;
>  
> -	dev->rro.miod_phys = rmem->base;
> +	dev->rro.miod_phys = res.start;
>  	dev->rro.fdbk_phys = MTK_WED_MIOD_COUNT + dev->rro.miod_phys;
>  
>  	return mtk_wed_rro_ring_alloc(dev, &dev->rro.ring,

> diff --git a/drivers/net/ethernet/mediatek/mtk_wed_mcu.c b/drivers/net/ethernet/mediatek/mtk_wed_mcu.c

...

> @@ -319,13 +313,7 @@ mtk_wed_mcu_load_firmware(struct mtk_wed_wo *wo)
>  
>  	/* load firmware region metadata */
>  	for (i = 0; i < ARRAY_SIZE(mem_region); i++) {
> -		int index = of_property_match_string(wo->hw->node,
> -						     "memory-region-names",
> -						     mem_region[i].name);
> -		if (index < 0)
> -			continue;
> -
> -		ret = mtk_wed_get_memory_region(wo->hw, index, &mem_region[i]);
> +		ret = mtk_wed_get_memory_region(wo->hw, mem_region[i].name, &mem_region[i]);

Ditto.

>  		if (ret)
>  			return ret;
>  	}

...