Re: [net-next v5 3/3] net: ethernet: mtk_eth_soc: skip first IRQ if not used

[Daniel Golle <daniel@makrotopia.org>]

On Wed, Jun 18, 2025 at 03:07:14PM +0200, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> On SoCs without MTK_SHARED_INT capability (all except mt7621 and
> mt7628) platform_get_irq() is called for the first IRQ (eth->irq[0])
> but it is never used.

I know that technically MTK_SHARED_INT is a capability flag, but it's
rather a non-capability. Hardware having dedicated interrupts for RX and
TX is "more capable" than (older, legacy) hardware with just one shared
interrupt for both...

So maybe better:
"On SoCs with dedicated RX and TX interrupts (all except MT7621 and
MT7628) ..."

Reading the datasheet of some recent MediaTek SoC it is worth
noting that there are 4 interrupts assigned to the frame engine and
the FE_INT_GRP register can be used to assign functions to them.

So technically, calling them RX and TX in DT is wrong, becaues they
are fe_int0, fe_int1, fe_int2 and fe_int3, which are then assigned
one or more functions by the driver using that FE_INT_GRP register.

However, it's the driver then assigns QDMA TX to fe_int1 and RX to
fe_int2 while leaving fe_int0 and fe_int3 unsued.

That's what the magic value 0x21021000 which is written to FE_INT_GRP
register does.

On MT7988 and newer, in addition to those 4 frame engine interrupts
there are **another 4** interrupts for PDMA, typically used to
service 4 RX rings while one of the fe_int* is used to indicate
TX done.

> Skip the first IRQ and reduce the IRQ-count to 2.
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
> v5:
> - change commit title and description
> v4:
> - drop >2 condition as max is already 2 and drop the else continue
> - update comment to explain which IRQs are taken in legacy way
> ---
>  drivers/net/ethernet/mediatek/mtk_eth_soc.c | 12 ++++++++----
>  drivers/net/ethernet/mediatek/mtk_eth_soc.h |  4 ++--
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index 875e477a987b..7990c84b2b56 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -3354,10 +3354,14 @@ static int mtk_get_irqs(struct platform_device *pdev, struct mtk_eth *eth)
>  	 * the second is for TX, and the third is for RX.
>  	 */
>  	for (i = 0; i < MTK_FE_IRQ_NUM; i++) {
> -		if (MTK_HAS_CAPS(eth->soc->caps, MTK_SHARED_INT) && i > 0)
> -			eth->irq[i] = eth->irq[MTK_FE_IRQ_SHARED];
> -		else
> -			eth->irq[i] = platform_get_irq(pdev, i);
> +		if (MTK_HAS_CAPS(eth->soc->caps, MTK_SHARED_INT)) {
> +			if (i == 0)

This would make it even more readable:
			if (i == MTK_FE_IRQ_SHARED)


Other than that looks good to me:

Reviewed-by: Daniel Golle <daniel@makrotopia.org>

> +				eth->irq[MTK_FE_IRQ_SHARED] = platform_get_irq(pdev, i);
> +			else
> +				eth->irq[i] = eth->irq[MTK_FE_IRQ_SHARED];
> +		} else {
> +			eth->irq[i] = platform_get_irq(pdev, i + 1);
> +		}
>  
>  		if (eth->irq[i] < 0) {
>  			dev_err(&pdev->dev, "no IRQ%d resource found\n", i);
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> index 8cdf1317dff5..9261c0e13b59 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> @@ -643,8 +643,8 @@
>  #define MTK_MAC_FSM(x)		(0x1010C + ((x) * 0x100))
>  
>  #define MTK_FE_IRQ_SHARED	0
> -#define MTK_FE_IRQ_TX		1
> -#define MTK_FE_IRQ_RX		2
> +#define MTK_FE_IRQ_TX		0
> +#define MTK_FE_IRQ_RX		1
>  #define MTK_FE_IRQ_NUM		(MTK_FE_IRQ_RX + 1)
>  
>  struct mtk_rx_dma {
> -- 
> 2.43.0
> 
>