[net-next v4 0/3] rework IRQ handling in mtk_eth_soc

[net-next v4 0/3] rework IRQ handling in mtk_eth_soc

[Frank Wunderlich]

From: Frank Wunderlich <frank-w@public-files.de>

This series introduces named IRQs while keeping the index based way
for older dts.
Further it makes some cleanup like adding consts for index access and
avoids loading first IRQ which was not used on non SHARED_INT SoCs.

changes:

v4:
- calculate max from last (rx) irq index and use it for array size too
- drop >2 condition as max is already 2 and drop the else continue
- update comment to explain which IRQs are taken in legacy way

v3:
added patches
- #2 (add constants for irq index)
- #3 (skip first IRQ on ! MTK_SHARED_INT)
to the v2 non-series patch
https://patchwork.kernel.org/project/netdevbpf/patch/20250615084521.32329-1-linux@fw-web.de/

Tested on BPI-R4/mt7988 with IRQ names and BPI-R2/mt7623 and BPI-R3/mt7986 with upstreamed
dts via index-mode.
I do not have any MTK_SHARED_INT (mt7621/mt7628) boards to testing.

Frank Wunderlich (3):
  net: ethernet: mtk_eth_soc: support named IRQs
  net: ethernet: mtk_eth_soc: add consts for irq index
  net: ethernet: mtk_eth_soc: change code to skip first IRQ completely

 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 63 +++++++++++++++------
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |  7 ++-
 2 files changed, 52 insertions(+), 18 deletions(-)

-- 
2.43.0

[net-next v4 1/3] net: ethernet: mtk_eth_soc: support named IRQs

[Frank Wunderlich]

From: Frank Wunderlich <frank-w@public-files.de>

Add named interrupts and keep index based fallback for exiting devicetrees.

Currently only rx and tx IRQs are defined to be used with mt7988, but
later extended with RSS/LRO support.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
Reviewed-by: Simon Horman <horms@kernel.org>
---
v2:
- move irqs loading part into own helper function
- reduce indentation
- place mtk_get_irqs helper before the irq_handler (note for simon)
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 39 +++++++++++++++------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index b76d35069887..81ae8a6fe838 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -3337,6 +3337,30 @@ static void mtk_tx_timeout(struct net_device *dev, unsigned int txqueue)
 	schedule_work(&eth->pending_work);
 }
 
+static int mtk_get_irqs(struct platform_device *pdev, struct mtk_eth *eth)
+{
+	int i;
+
+	eth->irq[1] = platform_get_irq_byname(pdev, "tx");
+	eth->irq[2] = platform_get_irq_byname(pdev, "rx");
+	if (eth->irq[1] >= 0 && eth->irq[2] >= 0)
+		return 0;
+
+	for (i = 0; i < 3; i++) {
+		if (MTK_HAS_CAPS(eth->soc->caps, MTK_SHARED_INT) && i > 0)
+			eth->irq[i] = eth->irq[0];
+		else
+			eth->irq[i] = platform_get_irq(pdev, i);
+
+		if (eth->irq[i] < 0) {
+			dev_err(&pdev->dev, "no IRQ%d resource found\n", i);
+			return -ENXIO;
+		}
+	}
+
+	return 0;
+}
+
 static irqreturn_t mtk_handle_irq_rx(int irq, void *_eth)
 {
 	struct mtk_eth *eth = _eth;
@@ -5106,17 +5130,10 @@ static int mtk_probe(struct platform_device *pdev)
 		}
 	}
 
-	for (i = 0; i < 3; i++) {
-		if (MTK_HAS_CAPS(eth->soc->caps, MTK_SHARED_INT) && i > 0)
-			eth->irq[i] = eth->irq[0];
-		else
-			eth->irq[i] = platform_get_irq(pdev, i);
-		if (eth->irq[i] < 0) {
-			dev_err(&pdev->dev, "no IRQ%d resource found\n", i);
-			err = -ENXIO;
-			goto err_wed_exit;
-		}
-	}
+	err = mtk_get_irqs(pdev, eth);
+	if (err)
+		goto err_wed_exit;
+
 	for (i = 0; i < ARRAY_SIZE(eth->clks); i++) {
 		eth->clks[i] = devm_clk_get(eth->dev,
 					    mtk_clks_source_name[i]);
-- 
2.43.0

[net-next v4 2/3] net: ethernet: mtk_eth_soc: add consts for irq index

[Frank Wunderlich]

From: Frank Wunderlich <frank-w@public-files.de>

Use consts instead of fixed integers for accessing IRQ array.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
v4:
- calculate max from last (rx) irq index and use it for array size too
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 22 ++++++++++-----------
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |  7 ++++++-
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 81ae8a6fe838..3ecb399dcf81 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -3341,14 +3341,14 @@ static int mtk_get_irqs(struct platform_device *pdev, struct mtk_eth *eth)
 {
 	int i;
 
-	eth->irq[1] = platform_get_irq_byname(pdev, "tx");
-	eth->irq[2] = platform_get_irq_byname(pdev, "rx");
-	if (eth->irq[1] >= 0 && eth->irq[2] >= 0)
+	eth->irq[MTK_ETH_IRQ_TX] = platform_get_irq_byname(pdev, "tx");
+	eth->irq[MTK_ETH_IRQ_RX] = platform_get_irq_byname(pdev, "rx");
+	if (eth->irq[MTK_ETH_IRQ_TX] >= 0 && eth->irq[MTK_ETH_IRQ_RX] >= 0)
 		return 0;
 
-	for (i = 0; i < 3; i++) {
+	for (i = 0; i < MTK_ETH_IRQ_MAX; i++) {
 		if (MTK_HAS_CAPS(eth->soc->caps, MTK_SHARED_INT) && i > 0)
-			eth->irq[i] = eth->irq[0];
+			eth->irq[i] = eth->irq[MTK_ETH_IRQ_SHARED];
 		else
 			eth->irq[i] = platform_get_irq(pdev, i);
 
@@ -3414,7 +3414,7 @@ static void mtk_poll_controller(struct net_device *dev)
 
 	mtk_tx_irq_disable(eth, MTK_TX_DONE_INT);
 	mtk_rx_irq_disable(eth, eth->soc->rx.irq_done_mask);
-	mtk_handle_irq_rx(eth->irq[2], dev);
+	mtk_handle_irq_rx(eth->irq[MTK_ETH_IRQ_RX], dev);
 	mtk_tx_irq_enable(eth, MTK_TX_DONE_INT);
 	mtk_rx_irq_enable(eth, eth->soc->rx.irq_done_mask);
 }
@@ -4900,7 +4900,7 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
 	eth->netdev[id]->features |= eth->soc->hw_features;
 	eth->netdev[id]->ethtool_ops = &mtk_ethtool_ops;
 
-	eth->netdev[id]->irq = eth->irq[0];
+	eth->netdev[id]->irq = eth->irq[MTK_ETH_IRQ_SHARED];
 	eth->netdev[id]->dev.of_node = np;
 
 	if (MTK_HAS_CAPS(eth->soc->caps, MTK_SOC_MT7628))
@@ -5177,17 +5177,17 @@ static int mtk_probe(struct platform_device *pdev)
 	}
 
 	if (MTK_HAS_CAPS(eth->soc->caps, MTK_SHARED_INT)) {
-		err = devm_request_irq(eth->dev, eth->irq[0],
+		err = devm_request_irq(eth->dev, eth->irq[MTK_ETH_IRQ_SHARED],
 				       mtk_handle_irq, 0,
 				       dev_name(eth->dev), eth);
 	} else {
-		err = devm_request_irq(eth->dev, eth->irq[1],
+		err = devm_request_irq(eth->dev, eth->irq[MTK_ETH_IRQ_TX],
 				       mtk_handle_irq_tx, 0,
 				       dev_name(eth->dev), eth);
 		if (err)
 			goto err_free_dev;
 
-		err = devm_request_irq(eth->dev, eth->irq[2],
+		err = devm_request_irq(eth->dev, eth->irq[MTK_ETH_IRQ_RX],
 				       mtk_handle_irq_rx, 0,
 				       dev_name(eth->dev), eth);
 	}
@@ -5233,7 +5233,7 @@ static int mtk_probe(struct platform_device *pdev)
 		} else
 			netif_info(eth, probe, eth->netdev[i],
 				   "mediatek frame engine at 0x%08lx, irq %d\n",
-				   eth->netdev[i]->base_addr, eth->irq[0]);
+				   eth->netdev[i]->base_addr, eth->irq[MTK_ETH_IRQ_SHARED]);
 	}
 
 	/* we run 2 devices on the same DMA ring so we need a dummy device
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 6f72a8c8ae1e..96b724dca0e2 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -642,6 +642,11 @@
 
 #define MTK_MAC_FSM(x)		(0x1010C + ((x) * 0x100))
 
+#define MTK_ETH_IRQ_SHARED	0
+#define MTK_ETH_IRQ_TX		1
+#define MTK_ETH_IRQ_RX		2
+#define MTK_ETH_IRQ_MAX		(MTK_ETH_IRQ_RX + 1)
+
 struct mtk_rx_dma {
 	unsigned int rxd1;
 	unsigned int rxd2;
@@ -1292,7 +1297,7 @@ struct mtk_eth {
 	struct net_device		*dummy_dev;
 	struct net_device		*netdev[MTK_MAX_DEVS];
 	struct mtk_mac			*mac[MTK_MAX_DEVS];
-	int				irq[3];
+	int				irq[MTK_ETH_IRQ_MAX];
 	u32				msg_enable;
 	unsigned long			sysclk;
 	struct regmap			*ethsys;
-- 
2.43.0

[net-next v4 3/3] net: ethernet: mtk_eth_soc: change code to skip first IRQ completely

[Frank Wunderlich]

From: Frank Wunderlich <frank-w@public-files.de>

On SoCs without MTK_SHARED_INT capability (mt7621 + mt7628) the first
IRQ (eth->irq[0]) was read but never used. Do not read it and reduce
the IRQ-count to 2 because of skipped index 0.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
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 | 20 ++++++++++++++++----
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |  4 ++--
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 3ecb399dcf81..f3fcbb00822c 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -3341,16 +3341,28 @@ static int mtk_get_irqs(struct platform_device *pdev, struct mtk_eth *eth)
 {
 	int i;
 
+	/* future SoCs beginning with MT7988 should use named IRQs in dts */
 	eth->irq[MTK_ETH_IRQ_TX] = platform_get_irq_byname(pdev, "tx");
 	eth->irq[MTK_ETH_IRQ_RX] = platform_get_irq_byname(pdev, "rx");
 	if (eth->irq[MTK_ETH_IRQ_TX] >= 0 && eth->irq[MTK_ETH_IRQ_RX] >= 0)
 		return 0;
 
+	/* legacy way:
+	 * On MTK_SHARED_INT SoCs (MT7621 + MT7628) the first IRQ is taken from
+	 * devicetree and used for rx+tx.
+	 * On SoCs with non-shared IRQ the first was not used, second entry is
+	 * TX and third is RX.
+	 */
+
 	for (i = 0; i < MTK_ETH_IRQ_MAX; i++) {
-		if (MTK_HAS_CAPS(eth->soc->caps, MTK_SHARED_INT) && i > 0)
-			eth->irq[i] = eth->irq[MTK_ETH_IRQ_SHARED];
-		else
-			eth->irq[i] = platform_get_irq(pdev, i);
+		if (MTK_HAS_CAPS(eth->soc->caps, MTK_SHARED_INT)) {
+			if (i == 0)
+				eth->irq[MTK_ETH_IRQ_SHARED] = platform_get_irq(pdev, i);
+			else
+				eth->irq[i] = eth->irq[MTK_ETH_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 96b724dca0e2..9d91fe721ad0 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_ETH_IRQ_SHARED	0
-#define MTK_ETH_IRQ_TX		1
-#define MTK_ETH_IRQ_RX		2
+#define MTK_ETH_IRQ_TX		0
+#define MTK_ETH_IRQ_RX		1
 #define MTK_ETH_IRQ_MAX		(MTK_ETH_IRQ_RX + 1)
 
 struct mtk_rx_dma {
-- 
2.43.0

Re: [net-next v4 3/3] net: ethernet: mtk_eth_soc: change code to skip first IRQ completely

[Simon Horman]

On Mon, Jun 16, 2025 at 10:07:36AM +0200, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> On SoCs without MTK_SHARED_INT capability (mt7621 + mt7628) the first
> IRQ (eth->irq[0]) was read but never used. Do not read it and reduce
> the IRQ-count to 2 because of skipped index 0.

Describing the first IRQ as read seems a bit confusing to me - do we read
it? And saying get or got seems hard to parse. So perhaps something like
this would be clearer?

... platform_get_irq() is called for the first IRQ (eth->irq[0]) but
it is never used.

> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
> 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 | 20 ++++++++++++++++----
>  drivers/net/ethernet/mediatek/mtk_eth_soc.h |  4 ++--
>  2 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index 3ecb399dcf81..f3fcbb00822c 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -3341,16 +3341,28 @@ static int mtk_get_irqs(struct platform_device *pdev, struct mtk_eth *eth)
>  {
>  	int i;
>  
> +	/* future SoCs beginning with MT7988 should use named IRQs in dts */

Perhaps this comment belongs in the patch that adds support for named IRQs.

>  	eth->irq[MTK_ETH_IRQ_TX] = platform_get_irq_byname(pdev, "tx");
>  	eth->irq[MTK_ETH_IRQ_RX] = platform_get_irq_byname(pdev, "rx");
>  	if (eth->irq[MTK_ETH_IRQ_TX] >= 0 && eth->irq[MTK_ETH_IRQ_RX] >= 0)
>  		return 0;
>  
> +	/* legacy way:
> +	 * On MTK_SHARED_INT SoCs (MT7621 + MT7628) the first IRQ is taken from
> +	 * devicetree and used for rx+tx.
> +	 * On SoCs with non-shared IRQ the first was not used, second entry is
> +	 * TX and third is RX.

Maybe I am slow. But I had a bit of trouble parsing this.
Perhaps this is clearer?

        * devicetree and used for both RX and TX - it is shared.
	* On SoCs with non-shared IRQs the first entry is not used,
        * the second is for TX, and the third is for RX.

> +	 */
> +
>  	for (i = 0; i < MTK_ETH_IRQ_MAX; i++) {
> -		if (MTK_HAS_CAPS(eth->soc->caps, MTK_SHARED_INT) && i > 0)
> -			eth->irq[i] = eth->irq[MTK_ETH_IRQ_SHARED];
> -		else
> -			eth->irq[i] = platform_get_irq(pdev, i);
> +		if (MTK_HAS_CAPS(eth->soc->caps, MTK_SHARED_INT)) {
> +			if (i == 0)
> +				eth->irq[MTK_ETH_IRQ_SHARED] = platform_get_irq(pdev, i);
> +			else
> +				eth->irq[i] = eth->irq[MTK_ETH_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);

...

Re: [net-next v4 2/3] net: ethernet: mtk_eth_soc: add consts for irq index

[Simon Horman]

On Mon, Jun 16, 2025 at 10:07:35AM +0200, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Use consts instead of fixed integers for accessing IRQ array.
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
> v4:
> - calculate max from last (rx) irq index and use it for array size too

Reviewed-by: Simon Horman <horms@kernel.org>

Aw: Re: [net-next v4 3/3] net: ethernet: mtk_eth_soc: change code to skip first IRQ completely

[Frank Wunderlich]

Hi Simon,

thanks for your review

> Gesendet: Mittwoch, 18. Juni 2025 um 10:35
> Von: "Simon Horman" <horms@kernel.org>
> Betreff: Re: [net-next v4 3/3] net: ethernet: mtk_eth_soc: change code to skip first IRQ completely
>
> On Mon, Jun 16, 2025 at 10:07:36AM +0200, Frank Wunderlich wrote:
> > From: Frank Wunderlich <frank-w@public-files.de>
> > 
> > On SoCs without MTK_SHARED_INT capability (mt7621 + mt7628) the first
> > IRQ (eth->irq[0]) was read but never used. Do not read it and reduce
> > the IRQ-count to 2 because of skipped index 0.
> 
> Describing the first IRQ as read seems a bit confusing to me - do we read
> it? And saying get or got seems hard to parse. So perhaps something like
> this would be clearer?
> 
> ... platform_get_irq() is called for the first IRQ (eth->irq[0]) but
> it is never used.

ok, i change it in next version

> > 
> > Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> > ---
> > 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 | 20 ++++++++++++++++----
> >  drivers/net/ethernet/mediatek/mtk_eth_soc.h |  4 ++--
> >  2 files changed, 18 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > index 3ecb399dcf81..f3fcbb00822c 100644
> > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > @@ -3341,16 +3341,28 @@ static int mtk_get_irqs(struct platform_device *pdev, struct mtk_eth *eth)
> >  {
> >  	int i;
> >  
> > +	/* future SoCs beginning with MT7988 should use named IRQs in dts */
> 
> Perhaps this comment belongs in the patch that adds support for named IRQs.

also thought that after sending it :)

> >  	eth->irq[MTK_ETH_IRQ_TX] = platform_get_irq_byname(pdev, "tx");
> >  	eth->irq[MTK_ETH_IRQ_RX] = platform_get_irq_byname(pdev, "rx");
> >  	if (eth->irq[MTK_ETH_IRQ_TX] >= 0 && eth->irq[MTK_ETH_IRQ_RX] >= 0)
> >  		return 0;
> >  
> > +	/* legacy way:
> > +	 * On MTK_SHARED_INT SoCs (MT7621 + MT7628) the first IRQ is taken from
> > +	 * devicetree and used for rx+tx.
> > +	 * On SoCs with non-shared IRQ the first was not used, second entry is
> > +	 * TX and third is RX.
> 
> Maybe I am slow. But I had a bit of trouble parsing this.
> Perhaps this is clearer?
> 
>         * devicetree and used for both RX and TX - it is shared.
> 	* On SoCs with non-shared IRQs the first entry is not used,
>         * the second is for TX, and the third is for RX.

I would also move this comment in first patch with your changes requested.

/* legacy way:
 * On MTK_SHARED_INT SoCs (MT7621 + MT7628) the first IRQ is taken from
 * devicetree and used for both RX and TX - it is shared.
 * On SoCs with non-shared IRQs the first entry is not used,
 * the second is for TX, and the third is for RX.
 */

i can keep your RB there?

> > +	 */
> > +
> >  	for (i = 0; i < MTK_ETH_IRQ_MAX; i++) {
> > -		if (MTK_HAS_CAPS(eth->soc->caps, MTK_SHARED_INT) && i > 0)
> > -			eth->irq[i] = eth->irq[MTK_ETH_IRQ_SHARED];
> > -		else
> > -			eth->irq[i] = platform_get_irq(pdev, i);
> > +		if (MTK_HAS_CAPS(eth->soc->caps, MTK_SHARED_INT)) {
> > +			if (i == 0)
> > +				eth->irq[MTK_ETH_IRQ_SHARED] = platform_get_irq(pdev, i);
> > +			else
> > +				eth->irq[i] = eth->irq[MTK_ETH_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);

code changes are OK?

regards Frank

Aw: Re: [net-next v4 2/3] net: ethernet: mtk_eth_soc: add consts for irq index

[Frank Wunderlich]

Hi,

> Gesendet: Mittwoch, 18. Juni 2025 um 10:36
> Von: "Simon Horman" <horms@kernel.org>
> Betreff: Re: [net-next v4 2/3] net: ethernet: mtk_eth_soc: add consts for irq index
>
> On Mon, Jun 16, 2025 at 10:07:35AM +0200, Frank Wunderlich wrote:
> > From: Frank Wunderlich <frank-w@public-files.de>
> > 
> > Use consts instead of fixed integers for accessing IRQ array.
> > 
> > Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> > ---
> > v4:
> > - calculate max from last (rx) irq index and use it for array size too
> 
> Reviewed-by: Simon Horman <horms@kernel.org>

thanks for review and the RB

i thinking about changing the const names to this:

MTK_ETH_IRQ_SHARED	=> MTK_FE_IRQ_SHARED
MTK_ETH_IRQ_TX		=> MTK_FE_IRQ_TX
MTK_ETH_IRQ_RX		=> MTK_FE_IRQ_RX
MTK_ETH_IRQ_MAX		=> MTK_FE_IRQ_NUM

because of i currently working on RSS/LRO patches and here MTK_FE_IRQ_NUM is used as name 
with same meaning like my current MTK_ETH_IRQ_MAX (where max should be same as RX and NUM
is one more because it is a count).
Current IRQs also target the MTK frame-engine which is different to the PDMA RX engine
used for RSS/LRO later, so MTK_ETH_IRQ_RX and MTK_ETH_IRQ_MAX are maybe misleading when RSS/LRO 
support will come in.

can i change the consts like this and keep your RB?

regards Frank

Re: Re: [net-next v4 3/3] net: ethernet: mtk_eth_soc: change code to skip first IRQ completely

[Simon Horman]

On Wed, Jun 18, 2025 at 09:14:47AM +0000, Frank Wunderlich wrote:
> Hi Simon,
> 
> thanks for your review
> 
> > Gesendet: Mittwoch, 18. Juni 2025 um 10:35
> > Von: "Simon Horman" <horms@kernel.org>
> > Betreff: Re: [net-next v4 3/3] net: ethernet: mtk_eth_soc: change code to skip first IRQ completely
> >
> > On Mon, Jun 16, 2025 at 10:07:36AM +0200, Frank Wunderlich wrote:
> > > From: Frank Wunderlich <frank-w@public-files.de>
> > > 
> > > On SoCs without MTK_SHARED_INT capability (mt7621 + mt7628) the first
> > > IRQ (eth->irq[0]) was read but never used. Do not read it and reduce
> > > the IRQ-count to 2 because of skipped index 0.
> > 
> > Describing the first IRQ as read seems a bit confusing to me - do we read
> > it? And saying get or got seems hard to parse. So perhaps something like
> > this would be clearer?
> > 
> > ... platform_get_irq() is called for the first IRQ (eth->irq[0]) but
> > it is never used.
> 
> ok, i change it in next version
> 
> > > 
> > > Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> > > ---
> > > 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 | 20 ++++++++++++++++----
> > >  drivers/net/ethernet/mediatek/mtk_eth_soc.h |  4 ++--
> > >  2 files changed, 18 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > index 3ecb399dcf81..f3fcbb00822c 100644
> > > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > @@ -3341,16 +3341,28 @@ static int mtk_get_irqs(struct platform_device *pdev, struct mtk_eth *eth)
> > >  {
> > >  	int i;
> > >  
> > > +	/* future SoCs beginning with MT7988 should use named IRQs in dts */
> > 
> > Perhaps this comment belongs in the patch that adds support for named IRQs.
> 
> also thought that after sending it :)
> 
> > >  	eth->irq[MTK_ETH_IRQ_TX] = platform_get_irq_byname(pdev, "tx");
> > >  	eth->irq[MTK_ETH_IRQ_RX] = platform_get_irq_byname(pdev, "rx");
> > >  	if (eth->irq[MTK_ETH_IRQ_TX] >= 0 && eth->irq[MTK_ETH_IRQ_RX] >= 0)
> > >  		return 0;
> > >  
> > > +	/* legacy way:
> > > +	 * On MTK_SHARED_INT SoCs (MT7621 + MT7628) the first IRQ is taken from
> > > +	 * devicetree and used for rx+tx.
> > > +	 * On SoCs with non-shared IRQ the first was not used, second entry is
> > > +	 * TX and third is RX.
> > 
> > Maybe I am slow. But I had a bit of trouble parsing this.
> > Perhaps this is clearer?
> > 
> >         * devicetree and used for both RX and TX - it is shared.
> > 	* On SoCs with non-shared IRQs the first entry is not used,
> >         * the second is for TX, and the third is for RX.
> 
> I would also move this comment in first patch with your changes requested.

Yes, good point.

> 
> /* legacy way:
>  * On MTK_SHARED_INT SoCs (MT7621 + MT7628) the first IRQ is taken from
>  * devicetree and used for both RX and TX - it is shared.
>  * On SoCs with non-shared IRQs the first entry is not used,
>  * the second is for TX, and the third is for RX.
>  */
> 
> i can keep your RB there?

Yes, thanks for asking.

> 
> > > +	 */
> > > +
> > >  	for (i = 0; i < MTK_ETH_IRQ_MAX; i++) {
> > > -		if (MTK_HAS_CAPS(eth->soc->caps, MTK_SHARED_INT) && i > 0)
> > > -			eth->irq[i] = eth->irq[MTK_ETH_IRQ_SHARED];
> > > -		else
> > > -			eth->irq[i] = platform_get_irq(pdev, i);
> > > +		if (MTK_HAS_CAPS(eth->soc->caps, MTK_SHARED_INT)) {
> > > +			if (i == 0)
> > > +				eth->irq[MTK_ETH_IRQ_SHARED] = platform_get_irq(pdev, i);
> > > +			else
> > > +				eth->irq[i] = eth->irq[MTK_ETH_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);
> 
> code changes are OK?

Yes. I did try to think of a more succinct way to express the logic.
But I think what you have is good.

In any case, let me review the next revision of this patch.
That will be easier for me than imagining what it looks like
with the non-code changes discussed above in place.

Re: Re: [net-next v4 2/3] net: ethernet: mtk_eth_soc: add consts for irq index

[Simon Horman]

On Wed, Jun 18, 2025 at 09:24:57AM +0000, Frank Wunderlich wrote:
> Hi,
> 
> > Gesendet: Mittwoch, 18. Juni 2025 um 10:36
> > Von: "Simon Horman" <horms@kernel.org>
> > Betreff: Re: [net-next v4 2/3] net: ethernet: mtk_eth_soc: add consts for irq index
> >
> > On Mon, Jun 16, 2025 at 10:07:35AM +0200, Frank Wunderlich wrote:
> > > From: Frank Wunderlich <frank-w@public-files.de>
> > > 
> > > Use consts instead of fixed integers for accessing IRQ array.
> > > 
> > > Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> > > ---
> > > v4:
> > > - calculate max from last (rx) irq index and use it for array size too
> > 
> > Reviewed-by: Simon Horman <horms@kernel.org>
> 
> thanks for review and the RB
> 
> i thinking about changing the const names to this:
> 
> MTK_ETH_IRQ_SHARED	=> MTK_FE_IRQ_SHARED
> MTK_ETH_IRQ_TX		=> MTK_FE_IRQ_TX
> MTK_ETH_IRQ_RX		=> MTK_FE_IRQ_RX
> MTK_ETH_IRQ_MAX		=> MTK_FE_IRQ_NUM
> 
> because of i currently working on RSS/LRO patches and here MTK_FE_IRQ_NUM is used as name 
> with same meaning like my current MTK_ETH_IRQ_MAX (where max should be same as RX and NUM
> is one more because it is a count).
> Current IRQs also target the MTK frame-engine which is different to the PDMA RX engine
> used for RSS/LRO later, so MTK_ETH_IRQ_RX and MTK_ETH_IRQ_MAX are maybe misleading when RSS/LRO 
> support will come in.
> 
> can i change the consts like this and keep your RB?

Yes, no objections from my side.

Re: [net-next v4 1/3] net: ethernet: mtk_eth_soc: support named IRQs

[Frank Wunderlich]

Am 16. Juni 2025 10:07:34 MESZ schrieb Frank Wunderlich <linux@fw-web.de>:
>From: Frank Wunderlich <frank-w@public-files.de>
>
>Add named interrupts and keep index based fallback for exiting devicetrees.
>
>Currently only rx and tx IRQs are defined to be used with mt7988, but
>later extended with RSS/LRO support.
>
>Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>Reviewed-by: Simon Horman <horms@kernel.org>
>---
>v2:
>- move irqs loading part into own helper function
>- reduce indentation
>- place mtk_get_irqs helper before the irq_handler (note for simon)
>---
> drivers/net/ethernet/mediatek/mtk_eth_soc.c | 39 +++++++++++++++------
> 1 file changed, 28 insertions(+), 11 deletions(-)
>
>diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>index b76d35069887..81ae8a6fe838 100644
>--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>@@ -3337,6 +3337,30 @@ static void mtk_tx_timeout(struct net_device *dev, unsigned int txqueue)
> 	schedule_work(&eth->pending_work);
> }
> 
>+static int mtk_get_irqs(struct platform_device *pdev, struct mtk_eth *eth)
>+{
>+	int i;
>+
>+	eth->irq[1] = platform_get_irq_byname(pdev, "tx");
>+	eth->irq[2] = platform_get_irq_byname(pdev, "rx");

Hi Simon,

I got information that reserved frame-engine 
 irqs are not unusable and have no fixed
 meaning. So i would add fe0..fe3 in
 dts+binding and change these names from
tx/rx to fe1 and fe2.

Can i keep your RB here?


regards Frank

Re: [net-next v4 1/3] net: ethernet: mtk_eth_soc: support named IRQs

[Simon Horman]

On Thu, Jun 19, 2025 at 09:44:34AM +0200, Frank Wunderlich wrote:
> Am 16. Juni 2025 10:07:34 MESZ schrieb Frank Wunderlich <linux@fw-web.de>:
> >From: Frank Wunderlich <frank-w@public-files.de>
> >
> >Add named interrupts and keep index based fallback for exiting devicetrees.
> >
> >Currently only rx and tx IRQs are defined to be used with mt7988, but
> >later extended with RSS/LRO support.
> >
> >Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> >Reviewed-by: Simon Horman <horms@kernel.org>
> >---
> >v2:
> >- move irqs loading part into own helper function
> >- reduce indentation
> >- place mtk_get_irqs helper before the irq_handler (note for simon)
> >---
> > drivers/net/ethernet/mediatek/mtk_eth_soc.c | 39 +++++++++++++++------
> > 1 file changed, 28 insertions(+), 11 deletions(-)
> >
> >diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> >index b76d35069887..81ae8a6fe838 100644
> >--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> >+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> >@@ -3337,6 +3337,30 @@ static void mtk_tx_timeout(struct net_device *dev, unsigned int txqueue)
> > 	schedule_work(&eth->pending_work);
> > }
> > 
> >+static int mtk_get_irqs(struct platform_device *pdev, struct mtk_eth *eth)
> >+{
> >+	int i;
> >+
> >+	eth->irq[1] = platform_get_irq_byname(pdev, "tx");
> >+	eth->irq[2] = platform_get_irq_byname(pdev, "rx");
> 
> Hi Simon,
> 
> I got information that reserved frame-engine 
>  irqs are not unusable and have no fixed
>  meaning. So i would add fe0..fe3 in
>  dts+binding and change these names from
> tx/rx to fe1 and fe2.
> 
> Can i keep your RB here?

Since the meaning is changing somewhat maybe best to drop the RB.
I'll look out for the new version to review.