[PATCH] net: stmmac: Use interrupt mode INTM=1 for per channel irq

[PATCH] net: stmmac: Use interrupt mode INTM=1 for per channel irq

[muhammad.nazim.amirul.nazle.asmade]

From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>

commit 6ccf12ae111e ("net: stmmac: use interrupt mode INTM=1 for
multi-MSI") introduced INTM=1 interrupt mode for platforms using MSI.

Apply a similar approach to enable per-channel interrupts using shared
peripheral interrupt (SPI), so that only per-channel TX and RX
interrupts (TI/RI) are handled by the TX/RX ISR without invoking the
common interrupt ISR.

The TX/RX NORMAL interrupt check is decoupled since the NIS bit is not
asserted for TI/RI events when INTM=1.

Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h     | 3 +++
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 7 +++++++
 include/linux/stmmac.h                             | 6 ++++++
 3 files changed, 16 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
index 51943705a2b0..94cbf24b3118 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
@@ -320,6 +320,9 @@
 /* DMA Registers */
 #define XGMAC_DMA_MODE			0x00003000
 #define XGMAC_SWR			BIT(0)
+#define DMA_MODE_INTM_MASK		GENMASK(13, 12)
+#define DMA_MODE_INTM_SHIFT		12
+#define DMA_MODE_INTM_MODE1		0x1
 #define XGMAC_DMA_SYSBUS_MODE		0x00003004
 #define XGMAC_WR_OSR_LMT		GENMASK(29, 24)
 #define XGMAC_RD_OSR_LMT		GENMASK(21, 16)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index 03437f1cf3df..3789b62cd2e3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -31,6 +31,13 @@ static void dwxgmac2_dma_init(void __iomem *ioaddr,
 		value |= XGMAC_EAME;
 
 	writel(value, ioaddr + XGMAC_DMA_SYSBUS_MODE);
+
+	if (dma_cfg->multi_irq_en) {
+		value = readl(ioaddr + XGMAC_DMA_MODE);
+		value &= ~DMA_MODE_INTM_MASK;
+		value |= (DMA_MODE_INTM_MODE1 << DMA_MODE_INTM_SHIFT);
+		writel(value, ioaddr + XGMAC_DMA_MODE);
+	}
 }
 
 static void dwxgmac2_dma_init_chan(struct stmmac_priv *priv,
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 4430b967abde..5deaa1a797a5 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -122,6 +122,7 @@ struct stmmac_dma_cfg {
 	bool eame;
 	/* multi_msi_en: stmmac core internal */
 	bool multi_msi_en;
+	bool multi_irq_en;
 	/* atds: stmmac core internal */
 	bool atds;
 };
@@ -349,6 +350,11 @@ struct plat_stmmacenet_data {
 	u8 vlan_fail_q;
 	bool provide_bus_info;
 	int int_snapshot_num;
+	int ext_snapshot_num;
+	bool int_snapshot_en;
+	bool ext_snapshot_en;
+	bool multi_msi_en;
+	bool multi_irq_en;
 	int msi_mac_vec;
 	int msi_wol_vec;
 	int msi_sfty_ce_vec;
-- 
2.43.7

Re: [PATCH] net: stmmac: Use interrupt mode INTM=1 for per channel irq

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: stmmac: Use interrupt mode INTM=1 for per channel irq

This commit adds programming of the XGMAC DMA_MODE.INTM=1 bit on xgmac2
to deliver per-channel TX/RX interrupts via shared peripheral interrupts
(SPI), mirroring the approach of the earlier MSI-oriented commit
6ccf12ae111e. New fields are added to stmmac_dma_cfg and
plat_stmmacenet_data to drive the new path.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> index 51943705a2b0..94cbf24b3118 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> @@ -320,6 +320,9 @@
>  /* DMA Registers */
>  #define XGMAC_DMA_MODE			0x00003000
>  #define XGMAC_SWR			BIT(0)
> +#define DMA_MODE_INTM_MASK		GENMASK(13, 12)
> +#define DMA_MODE_INTM_SHIFT		12
> +#define DMA_MODE_INTM_MODE1		0x1

Should these three macros carry the XGMAC_ prefix used by every other
symbol in dwxgmac2.h (XGMAC_DMA_MODE, XGMAC_SWR, XGMAC_DMA_SYSBUS_MODE,
and so on)?  The dwmac4 sibling header defines analogous symbols as
DMA_BUS_MODE_INTM_MASK / DMA_BUS_MODE_INTM_MODE1, but within dwxgmac2.h
the established convention appears to be XGMAC_*.

The referenced precedent commit 6ccf12ae111e uses FIELD_PREP() /
u32_replace_bits() rather than a hand-rolled SHIFT constant.  Would it
be cleaner to drop DMA_MODE_INTM_SHIFT and use FIELD_PREP() here too?

>  #define XGMAC_DMA_SYSBUS_MODE		0x00003004
>  #define XGMAC_WR_OSR_LMT		GENMASK(29, 24)
>  #define XGMAC_RD_OSR_LMT		GENMASK(21, 16)
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
> index 03437f1cf3df..3789b62cd2e3 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
> @@ -31,6 +31,13 @@ static void dwxgmac2_dma_init(void __iomem *ioaddr,
>  		value |= XGMAC_EAME;
>  
>  	writel(value, ioaddr + XGMAC_DMA_SYSBUS_MODE);
> +
> +	if (dma_cfg->multi_irq_en) {
> +		value = readl(ioaddr + XGMAC_DMA_MODE);
> +		value &= ~DMA_MODE_INTM_MASK;
> +		value |= (DMA_MODE_INTM_MODE1 << DMA_MODE_INTM_SHIFT);
> +		writel(value, ioaddr + XGMAC_DMA_MODE);
> +	}
>  }

Is this new block reachable on any platform?

A tree-wide search for multi_irq_en returns only three references: this
read site and the two struct declarations added below.  Nothing assigns
dma_cfg->multi_irq_en anywhere, so the condition appears to always be
false and INTM=1 would never be programmed.

For comparison, the analogous multi_msi_en is set up in stmmac_main.c:

    priv->plat->dma_cfg->multi_msi_en =
            (priv->plat->flags & STMMAC_FLAG_MULTI_MSI_EN);

and is consumed from glue drivers such as dwmac-intel.c, dwmac-loongson.c,
and dwmac-s32.c.  Should an equivalent assignment and a corresponding
plat flag (or the new plat->multi_irq_en field) be wired up in this
patch so the feature actually takes effect?

The commit message also states:

    The TX/RX NORMAL interrupt check is decoupled since the NIS bit is
    not asserted for TI/RI events when INTM=1.

but the diff touches only dwxgmac2_dma_init() and does not modify
dwxgmac2_dma_interrupt() or any other ISR/interrupt-status path.  The
existing dwxgmac2_dma_interrupt() already tests XGMAC_RI and
XGMAC_TI | XGMAC_TBU independently of XGMAC_NIS:

    if (likely(intr_status & XGMAC_RI)) {
            ...
    }
    if (likely(intr_status & (XGMAC_TI | XGMAC_TBU))) {
            ...
    }

Is this paragraph carried over from the earlier dwmac4 commit
6ccf12ae111e, or is a companion hunk missing from this patch?

>  static void dwxgmac2_dma_init_chan(struct stmmac_priv *priv,
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index 4430b967abde..5deaa1a797a5 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -122,6 +122,7 @@ struct stmmac_dma_cfg {
>  	bool eame;
>  	/* multi_msi_en: stmmac core internal */
>  	bool multi_msi_en;
> +	bool multi_irq_en;
>  	/* atds: stmmac core internal */
>  	bool atds;
>  };
> @@ -349,6 +350,11 @@ struct plat_stmmacenet_data {
>  	u8 vlan_fail_q;
>  	bool provide_bus_info;
>  	int int_snapshot_num;
> +	int ext_snapshot_num;
> +	bool int_snapshot_en;
> +	bool ext_snapshot_en;
> +	bool multi_msi_en;
> +	bool multi_irq_en;
>  	int msi_mac_vec;
>  	int msi_wol_vec;
>  	int msi_sfty_ce_vec;

Are these five new plat_stmmacenet_data fields used by anything?

A tree-wide search finds no reader or writer for ext_snapshot_num,
int_snapshot_en, ext_snapshot_en, plat->multi_msi_en, or
plat->multi_irq_en, and they are not mentioned in the commit message.
The commit message scope is limited to enabling INTM=1 per-channel SPI
IRQs, which would only require stmmac_dma_cfg::multi_irq_en.

The same concepts already appear to be represented via plat->flags bits
(STMMAC_FLAG_MULTI_MSI_EN, STMMAC_FLAG_INT_SNAPSHOT_EN,
STMMAC_FLAG_EXT_SNAPSHOT_EN) and the existing int_snapshot_num member.
Could the four unrelated fields be dropped from this patch and, if
still needed, be introduced in a separate series that actually wires
them up?  Having both a flags bit and a bool for the same concept
risks future drift about which representation is authoritative.
-- 
pw-bot: cr

Re: [PATCH] net: stmmac: Use interrupt mode INTM=1 for per channel irq

[Nazle Asmade, Muhammad Nazim Amirul]

On 1/5/2026 8:37 am, Jakub Kicinski 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.
> ---
> net: stmmac: Use interrupt mode INTM=1 for per channel irq
> 
> This commit adds programming of the XGMAC DMA_MODE.INTM=1 bit on xgmac2
> to deliver per-channel TX/RX interrupts via shared peripheral interrupts
> (SPI), mirroring the approach of the earlier MSI-oriented commit
> 6ccf12ae111e. New fields are added to stmmac_dma_cfg and
> plat_stmmacenet_data to drive the new path.
> 
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
>> index 51943705a2b0..94cbf24b3118 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
>> @@ -320,6 +320,9 @@
>>   /* DMA Registers */
>>   #define XGMAC_DMA_MODE			0x00003000
>>   #define XGMAC_SWR			BIT(0)
>> +#define DMA_MODE_INTM_MASK		GENMASK(13, 12)
>> +#define DMA_MODE_INTM_SHIFT		12
>> +#define DMA_MODE_INTM_MODE1		0x1
> 
> Should these three macros carry the XGMAC_ prefix used by every other
> symbol in dwxgmac2.h (XGMAC_DMA_MODE, XGMAC_SWR, XGMAC_DMA_SYSBUS_MODE,
> and so on)?  The dwmac4 sibling header defines analogous symbols as
> DMA_BUS_MODE_INTM_MASK / DMA_BUS_MODE_INTM_MODE1, but within dwxgmac2.h
> the established convention appears to be XGMAC_*.
> 
> The referenced precedent commit 6ccf12ae111e uses FIELD_PREP() /
> u32_replace_bits() rather than a hand-rolled SHIFT constant.  Would it
> be cleaner to drop DMA_MODE_INTM_SHIFT and use FIELD_PREP() here too?
> 
>>   #define XGMAC_DMA_SYSBUS_MODE		0x00003004
>>   #define XGMAC_WR_OSR_LMT		GENMASK(29, 24)
>>   #define XGMAC_RD_OSR_LMT		GENMASK(21, 16)
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
>> index 03437f1cf3df..3789b62cd2e3 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
>> @@ -31,6 +31,13 @@ static void dwxgmac2_dma_init(void __iomem *ioaddr,
>>   		value |= XGMAC_EAME;
>>   
>>   	writel(value, ioaddr + XGMAC_DMA_SYSBUS_MODE);
>> +
>> +	if (dma_cfg->multi_irq_en) {
>> +		value = readl(ioaddr + XGMAC_DMA_MODE);
>> +		value &= ~DMA_MODE_INTM_MASK;
>> +		value |= (DMA_MODE_INTM_MODE1 << DMA_MODE_INTM_SHIFT);
>> +		writel(value, ioaddr + XGMAC_DMA_MODE);
>> +	}
>>   }
> 
> Is this new block reachable on any platform?
> 
> A tree-wide search for multi_irq_en returns only three references: this
> read site and the two struct declarations added below.  Nothing assigns
> dma_cfg->multi_irq_en anywhere, so the condition appears to always be
> false and INTM=1 would never be programmed.
> 
> For comparison, the analogous multi_msi_en is set up in stmmac_main.c:
> 
>      priv->plat->dma_cfg->multi_msi_en =
>              (priv->plat->flags & STMMAC_FLAG_MULTI_MSI_EN);
> 
> and is consumed from glue drivers such as dwmac-intel.c, dwmac-loongson.c,
> and dwmac-s32.c.  Should an equivalent assignment and a corresponding
> plat flag (or the new plat->multi_irq_en field) be wired up in this
> patch so the feature actually takes effect?
> 
> The commit message also states:
> 
>      The TX/RX NORMAL interrupt check is decoupled since the NIS bit is
>      not asserted for TI/RI events when INTM=1.
> 
> but the diff touches only dwxgmac2_dma_init() and does not modify
> dwxgmac2_dma_interrupt() or any other ISR/interrupt-status path.  The
> existing dwxgmac2_dma_interrupt() already tests XGMAC_RI and
> XGMAC_TI | XGMAC_TBU independently of XGMAC_NIS:
> 
>      if (likely(intr_status & XGMAC_RI)) {
>              ...
>      }
>      if (likely(intr_status & (XGMAC_TI | XGMAC_TBU))) {
>              ...
>      }
> 
> Is this paragraph carried over from the earlier dwmac4 commit
> 6ccf12ae111e, or is a companion hunk missing from this patch?
> 
>>   static void dwxgmac2_dma_init_chan(struct stmmac_priv *priv,
>> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
>> index 4430b967abde..5deaa1a797a5 100644
>> --- a/include/linux/stmmac.h
>> +++ b/include/linux/stmmac.h
>> @@ -122,6 +122,7 @@ struct stmmac_dma_cfg {
>>   	bool eame;
>>   	/* multi_msi_en: stmmac core internal */
>>   	bool multi_msi_en;
>> +	bool multi_irq_en;
>>   	/* atds: stmmac core internal */
>>   	bool atds;
>>   };
>> @@ -349,6 +350,11 @@ struct plat_stmmacenet_data {
>>   	u8 vlan_fail_q;
>>   	bool provide_bus_info;
>>   	int int_snapshot_num;
>> +	int ext_snapshot_num;
>> +	bool int_snapshot_en;
>> +	bool ext_snapshot_en;
>> +	bool multi_msi_en;
>> +	bool multi_irq_en;
>>   	int msi_mac_vec;
>>   	int msi_wol_vec;
>>   	int msi_sfty_ce_vec;
> 
> Are these five new plat_stmmacenet_data fields used by anything?
> 
> A tree-wide search finds no reader or writer for ext_snapshot_num,
> int_snapshot_en, ext_snapshot_en, plat->multi_msi_en, or
> plat->multi_irq_en, and they are not mentioned in the commit message.
> The commit message scope is limited to enabling INTM=1 per-channel SPI
> IRQs, which would only require stmmac_dma_cfg::multi_irq_en.
> 
> The same concepts already appear to be represented via plat->flags bits
> (STMMAC_FLAG_MULTI_MSI_EN, STMMAC_FLAG_INT_SNAPSHOT_EN,
> STMMAC_FLAG_EXT_SNAPSHOT_EN) and the existing int_snapshot_num member.
> Could the four unrelated fields be dropped from this patch and, if
> still needed, be introduced in a separate series that actually wires
> them up?  Having both a flags bit and a bool for the same concept
> risks future drift about which representation is authoritative.
All comments have been addressed and updated in v2

https://lore.kernel.org/all/20260505080311.17405-1-muhammad.nazim.amirul.nazle.asmade@altera.com/

Nazim