Re: [v5,1/4] net: stmmac: platform: read channels irq

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: stmmac: platform: read channels irq

This commit adds support for reading IRQ resources for all rx/tx channels
in the stmmac platform driver to enable Multi-IRQ mode for platform glue
drivers.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 5c9fd91a1db9..6166a9b0c0bb 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -700,6 +700,9 @@ EXPORT_SYMBOL_GPL(stmmac_pltfr_find_clk);
>  int stmmac_get_platform_resources(struct platform_device *pdev,
>  				  struct stmmac_resources *stmmac_res)
>  {
> +	char name[16];
> +	int i;
> +
>  	memset(stmmac_res, 0, sizeof(*stmmac_res));
>
>  	/* Get IRQ information early to have an ability to ask for deferred
> @@ -735,7 +738,42 @@ int stmmac_get_platform_resources(struct platform_device *pdev,
>
>  	stmmac_res->addr = devm_platform_ioremap_resource(pdev, 0);
>
> -	return PTR_ERR_OR_ZERO(stmmac_res->addr);
> +	if (IS_ERR(stmmac_res->addr))
> +		return PTR_ERR(stmmac_res->addr);
> +
> +	/* RX channels irq */
> +	for (i = 0; i < MTL_MAX_RX_QUEUES; i++) {
> +		scnprintf(name, sizeof(name), "rx-queue-%d", i);
> +		stmmac_res->rx_irq[i] = platform_get_irq_byname_optional(pdev,
> +									 name);
> +		if (stmmac_res->rx_irq[i] <= 0) {
> +			if (stmmac_res->rx_irq[i] == -EPROBE_DEFER)
> +				return -EPROBE_DEFER;
> +			dev_dbg(&pdev->dev, "IRQ rx-queue-%d not found\n", i);
> +
> +			/* Stop on first unset rx-queue-%i property member */
> +			stmmac_res->rx_irq[i] = 0;
> +			break;
> +		}
> +	}
> +
> +	/* TX channels irq */
> +	for (i = 0; i < MTL_MAX_TX_QUEUES; i++) {
> +		scnprintf(name, sizeof(name), "tx-queue-%d", i);
> +		stmmac_res->tx_irq[i] = platform_get_irq_byname_optional(pdev,
> +									 name);
> +		if (stmmac_res->tx_irq[i] <= 0) {
> +			if (stmmac_res->tx_irq[i] == -EPROBE_DEFER)
> +				return -EPROBE_DEFER;
> +			dev_dbg(&pdev->dev, "IRQ tx-queue-%d not found\n", i);
> +
> +			/* Stop on first unset tx-queue-%i property member */
> +			stmmac_res->tx_irq[i] = 0;
> +			break;
> +		}
> +	}

This isn't a bug, but the RX and TX channel IRQ handling blocks follow an
identical 14-line pattern that differs only in the queue name format string
and target array. Both blocks format the IRQ name with scnprintf(), call
platform_get_irq_byname_optional(), check for -EPROBE_DEFER, log a debug
message on error, and break on the first missing IRQ.

Could this duplication be reduced with a helper function? Note that the
existing code in this function already contains similar non-factored
patterns for wol_irq and sfty_irq handling, so this level of duplication
may be acceptable in this file's style.

> +
> +	return 0;
>  }
>  EXPORT_SYMBOL_GPL(stmmac_get_platform_resources);