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

[Jan Petrous]

On Wed, Feb 25, 2026 at 09:24:56AM +0000, Simon Horman wrote:
> 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.
> 

Hi Simon,
I already addressed this in v6, I sent today.

/Jan