2024-01-05 07:10:00

by Swee, Leong Ching

[permalink] [raw]
Subject: [PATCH net-next v2 0/4] net: stmmac: Enable Per DMA Channel interrupt

From: Swee Leong Ching <[email protected]>

Hi,
Add Per DMA Channel interrupt feature for DWXGMAC IP.

Patchset (link below) contains per DMA channel interrupt, But it was
achieved.
https://lore.kernel.org/lkml/20230821203328.GA2197059-
[email protected]/t/#m849b529a642e1bff89c05a07efc25d6a94c8bfb4

Some of the changes in this patchset are based on reviewer comment on
patchset mentioned beforehand.

changelog v2:
*extend irq_name array to 9
*add empty line after int i
*avoid polluting rx_irq/tx_irq array with temporary variable
*move tx/rx IRQ loop to bottom of stmmac_get_platform_resource()
*rename stmmac_xx_queue_interrupt() to stmmac_dma_xx_interrupt()
*fix error message in stmmac_request_irq_multi()
*move STMMAC_FLAG_MULTI_IRQ_EN handling to glue driver

Swee Leong Ching (4):
dt-bindings: net: snps,dwmac: per channel irq
net: stmmac: Make MSI interrupt routine generic
net: stmmac: Add support for TX/RX channel interrupt
net: stmmac: Use interrupt mode INTM=1 for per channel irq

.../devicetree/bindings/net/snps,dwmac.yaml | 24 ++++++++++----
.../net/ethernet/stmicro/stmmac/dwmac-intel.c | 4 +--
.../ethernet/stmicro/stmmac/dwmac-socfpga.c | 3 ++
.../net/ethernet/stmicro/stmmac/dwmac4_dma.c | 2 +-
.../net/ethernet/stmicro/stmmac/dwxgmac2.h | 3 ++
.../ethernet/stmicro/stmmac/dwxgmac2_dma.c | 32 +++++++++++--------
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 30 ++++++++---------
.../ethernet/stmicro/stmmac/stmmac_platform.c | 28 ++++++++++++++++
include/linux/stmmac.h | 4 +--
9 files changed, 90 insertions(+), 40 deletions(-)

--
2.34.1



2024-01-05 07:10:36

by Swee, Leong Ching

[permalink] [raw]
Subject: [PATCH net-next v2 1/4] dt-bindings: net: snps,dwmac: per channel irq

From: Swee Leong Ching <[email protected]>

Add dt-bindings for per channel irq.

Signed-off-by: Rohan G Thomas <[email protected]>
Signed-off-by: Swee Leong Ching <[email protected]>
---
.../devicetree/bindings/net/snps,dwmac.yaml | 24 +++++++++++++------
1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 5c2769dc689a..e72dded824f4 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -103,17 +103,27 @@ properties:

interrupts:
minItems: 1
- items:
- - description: Combined signal for various interrupt events
- - description: The interrupt to manage the remote wake-up packet detection
- - description: The interrupt that occurs when Rx exits the LPI state
+ maxItems: 19

interrupt-names:
minItems: 1
+ maxItems: 19
items:
- - const: macirq
- - enum: [eth_wake_irq, eth_lpi]
- - const: eth_lpi
+ oneOf:
+ - description: Combined signal for various interrupt events
+ const: macirq
+ - description: The interrupt to manage the remote wake-up packet detection
+ const: eth_wake_irq
+ - description: The interrupt that occurs when Rx exits the LPI state
+ const: eth_lpi
+ - description: DMA Tx per-channel interrupt
+ pattern: '^dma_tx[0-7]?$'
+ - description: DMA Rx per-channel interrupt
+ pattern: '^dma_rx[0-7]?$'
+
+ allOf:
+ - contains:
+ const: macirq

clocks:
minItems: 1
--
2.34.1


2024-01-05 07:11:02

by Swee, Leong Ching

[permalink] [raw]
Subject: [PATCH net-next v2 3/4] net: stmmac: Add support for TX/RX channel interrupt

From: Swee Leong Ching <[email protected]>

Enable TX/RX channel interrupt registration for MAC that interrupts CPU
through shared peripheral interrupt (SPI).

Per channel interrupts and interrupt-names are registered through,
Eg: 4 tx and 4 rx channels:
interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
<GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
<GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
<GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
<GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>;
interrupt-names = "dma_tx0",
"dma_tx1",
"dma_tx2",
"dma_tx3",
"dma_rx0",
"dma_rx1",
"dma_rx2",
"dma_rx3";

Signed-off-by: Teoh Ji Sheng <[email protected]>
Signed-off-by: Swee Leong Ching <[email protected]>
---
.../ethernet/stmicro/stmmac/stmmac_platform.c | 28 +++++++++++++++++++
1 file changed, 28 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 70eadc83ca68..ae6859153e98 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -710,6 +710,10 @@ EXPORT_SYMBOL_GPL(devm_stmmac_probe_config_dt);
int stmmac_get_platform_resources(struct platform_device *pdev,
struct stmmac_resources *stmmac_res)
{
+ char irq_name[9];
+ int i;
+ int irq;
+
memset(stmmac_res, 0, sizeof(*stmmac_res));

/* Get IRQ information early to have an ability to ask for deferred
@@ -743,6 +747,30 @@ int stmmac_get_platform_resources(struct platform_device *pdev,
dev_info(&pdev->dev, "IRQ eth_lpi not found\n");
}

+ /* For RX Channel */
+ for (i = 0; i < MTL_MAX_RX_QUEUES; i++) {
+ snprintf(irq_name, sizeof(irq_name), "dma_rx%i", i);
+ irq = platform_get_irq_byname_optional(pdev, irq_name);
+ if (irq == -EPROBE_DEFER)
+ return irq;
+ else if (irq < 0)
+ break;
+
+ stmmac_res->rx_irq[i] = irq;
+ }
+
+ /* For TX Channel */
+ for (i = 0; i < MTL_MAX_TX_QUEUES; i++) {
+ snprintf(irq_name, sizeof(irq_name), "dma_tx%i", i);
+ irq = platform_get_irq_byname_optional(pdev, irq_name);
+ if (irq == -EPROBE_DEFER)
+ return irq;
+ else if (irq < 0)
+ break;
+
+ stmmac_res->tx_irq[i] = irq;
+ }
+
stmmac_res->addr = devm_platform_ioremap_resource(pdev, 0);

return PTR_ERR_OR_ZERO(stmmac_res->addr);
--
2.34.1


2024-01-05 07:11:04

by Swee, Leong Ching

[permalink] [raw]
Subject: [PATCH net-next v2 2/4] net: stmmac: Make MSI interrupt routine generic

From: Swee Leong Ching <[email protected]>

There is no support for per DMA channel interrupt for non-MSI platform,
where the MAC's per channel interrupt hooks up to interrupt controller(GIC)
through shared peripheral interrupt(SPI) to handle interrupt from TX/RX
transmit channel.

This patch generalize the existing MSI ISR to also support non-MSI
platform.

Signed-off-by: Teoh Ji Sheng <[email protected]>
Signed-off-by: Swee Leong Ching <[email protected]>
---
.../net/ethernet/stmicro/stmmac/dwmac-intel.c | 4 +--
.../ethernet/stmicro/stmmac/dwmac-socfpga.c | 3 ++
.../net/ethernet/stmicro/stmmac/dwmac4_dma.c | 2 +-
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 30 +++++++++----------
include/linux/stmmac.h | 4 +--
5 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
index 60283543ffc8..f0ec69af96c9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
@@ -952,7 +952,7 @@ static int stmmac_config_single_msi(struct pci_dev *pdev,

res->irq = pci_irq_vector(pdev, 0);
res->wol_irq = res->irq;
- plat->flags &= ~STMMAC_FLAG_MULTI_MSI_EN;
+ plat->flags &= ~STMMAC_FLAG_MULTI_IRQ_EN;
dev_info(&pdev->dev, "%s: Single IRQ enablement successful\n",
__func__);

@@ -1004,7 +1004,7 @@ static int stmmac_config_multi_msi(struct pci_dev *pdev,
if (plat->msi_sfty_ue_vec < STMMAC_MSI_VEC_MAX)
res->sfty_ue_irq = pci_irq_vector(pdev, plat->msi_sfty_ue_vec);

- plat->flags |= STMMAC_FLAG_MULTI_MSI_EN;
+ plat->flags |= STMMAC_FLAG_MULTI_IRQ_EN;
dev_info(&pdev->dev, "%s: multi MSI enablement successful\n", __func__);

return 0;
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index ba2ce776bd4d..cf43fb3c6cc5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -427,6 +427,9 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
plat_dat->bsp_priv = dwmac;
plat_dat->fix_mac_speed = socfpga_dwmac_fix_mac_speed;

+ if (stmmac_res.rx_irq[0] > 0 && stmmac_res.tx_irq[0] > 0)
+ plat_dat->flags |= STMMAC_FLAG_MULTI_IRQ_EN;
+
ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
if (ret)
return ret;
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
index 84d3a8551b03..5f649106ffcd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
@@ -175,7 +175,7 @@ static void dwmac4_dma_init(void __iomem *ioaddr,

value = readl(ioaddr + DMA_BUS_MODE);

- if (dma_cfg->multi_msi_en) {
+ if (dma_cfg->multi_irq_en) {
value &= ~DMA_BUS_MODE_INTM_MASK;
value |= (DMA_BUS_MODE_INTM_MODE1 << DMA_BUS_MODE_INTM_SHIFT);
}
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 47de466e432c..57873b879b33 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -129,8 +129,8 @@ static irqreturn_t stmmac_interrupt(int irq, void *dev_id);
/* For MSI interrupts handling */
static irqreturn_t stmmac_mac_interrupt(int irq, void *dev_id);
static irqreturn_t stmmac_safety_interrupt(int irq, void *dev_id);
-static irqreturn_t stmmac_msi_intr_tx(int irq, void *data);
-static irqreturn_t stmmac_msi_intr_rx(int irq, void *data);
+static irqreturn_t stmmac_dma_tx_interrupt(int irq, void *data);
+static irqreturn_t stmmac_dma_rx_interrupt(int irq, void *data);
static void stmmac_reset_rx_queue(struct stmmac_priv *priv, u32 queue);
static void stmmac_reset_tx_queue(struct stmmac_priv *priv, u32 queue);
static void stmmac_reset_queues_param(struct stmmac_priv *priv);
@@ -3602,7 +3602,7 @@ static void stmmac_free_irq(struct net_device *dev,
}
}

-static int stmmac_request_irq_multi_msi(struct net_device *dev)
+static int stmmac_request_irq_multi(struct net_device *dev)
{
struct stmmac_priv *priv = netdev_priv(dev);
enum request_irq_err irq_err;
@@ -3697,7 +3697,7 @@ static int stmmac_request_irq_multi_msi(struct net_device *dev)
}
}

- /* Request Rx MSI irq */
+ /* Request Rx irq */
for (i = 0; i < priv->plat->rx_queues_to_use; i++) {
if (i >= MTL_MAX_RX_QUEUES)
break;
@@ -3707,11 +3707,11 @@ static int stmmac_request_irq_multi_msi(struct net_device *dev)
int_name = priv->int_name_rx_irq[i];
sprintf(int_name, "%s:%s-%d", dev->name, "rx", i);
ret = request_irq(priv->rx_irq[i],
- stmmac_msi_intr_rx,
+ stmmac_dma_rx_interrupt,
0, int_name, &priv->dma_conf.rx_queue[i]);
if (unlikely(ret < 0)) {
netdev_err(priv->dev,
- "%s: alloc rx-%d MSI %d (error: %d)\n",
+ "%s: alloc rx-%d dma rx_irq %d (error: %d)\n",
__func__, i, priv->rx_irq[i], ret);
irq_err = REQ_IRQ_ERR_RX;
irq_idx = i;
@@ -3722,7 +3722,7 @@ static int stmmac_request_irq_multi_msi(struct net_device *dev)
irq_set_affinity_hint(priv->rx_irq[i], &cpu_mask);
}

- /* Request Tx MSI irq */
+ /* Request Tx irq */
for (i = 0; i < priv->plat->tx_queues_to_use; i++) {
if (i >= MTL_MAX_TX_QUEUES)
break;
@@ -3732,11 +3732,11 @@ static int stmmac_request_irq_multi_msi(struct net_device *dev)
int_name = priv->int_name_tx_irq[i];
sprintf(int_name, "%s:%s-%d", dev->name, "tx", i);
ret = request_irq(priv->tx_irq[i],
- stmmac_msi_intr_tx,
+ stmmac_dma_tx_interrupt,
0, int_name, &priv->dma_conf.tx_queue[i]);
if (unlikely(ret < 0)) {
netdev_err(priv->dev,
- "%s: alloc tx-%d MSI %d (error: %d)\n",
+ "%s: alloc tx-%d dma tx_irq %d (error: %d)\n",
__func__, i, priv->tx_irq[i], ret);
irq_err = REQ_IRQ_ERR_TX;
irq_idx = i;
@@ -3811,8 +3811,8 @@ static int stmmac_request_irq(struct net_device *dev)
int ret;

/* Request the IRQ lines */
- if (priv->plat->flags & STMMAC_FLAG_MULTI_MSI_EN)
- ret = stmmac_request_irq_multi_msi(dev);
+ if (priv->plat->flags & STMMAC_FLAG_MULTI_IRQ_EN)
+ ret = stmmac_request_irq_multi(dev);
else
ret = stmmac_request_irq_single(dev);

@@ -6075,7 +6075,7 @@ static irqreturn_t stmmac_safety_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
}

-static irqreturn_t stmmac_msi_intr_tx(int irq, void *data)
+static irqreturn_t stmmac_dma_tx_interrupt(int irq, void *data)
{
struct stmmac_tx_queue *tx_q = (struct stmmac_tx_queue *)data;
struct stmmac_dma_conf *dma_conf;
@@ -6107,7 +6107,7 @@ static irqreturn_t stmmac_msi_intr_tx(int irq, void *data)
return IRQ_HANDLED;
}

-static irqreturn_t stmmac_msi_intr_rx(int irq, void *data)
+static irqreturn_t stmmac_dma_rx_interrupt(int irq, void *data)
{
struct stmmac_rx_queue *rx_q = (struct stmmac_rx_queue *)data;
struct stmmac_dma_conf *dma_conf;
@@ -7456,8 +7456,8 @@ int stmmac_dvr_probe(struct device *device,
priv->plat = plat_dat;
priv->ioaddr = res->addr;
priv->dev->base_addr = (unsigned long)res->addr;
- priv->plat->dma_cfg->multi_msi_en =
- (priv->plat->flags & STMMAC_FLAG_MULTI_MSI_EN);
+ priv->plat->dma_cfg->multi_irq_en =
+ (priv->plat->flags & STMMAC_FLAG_MULTI_IRQ_EN);

priv->dev->irq = res->irq;
priv->wol_irq = res->wol_irq;
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index dee5ad6e48c5..b950e6f9761d 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -98,7 +98,7 @@ struct stmmac_dma_cfg {
int mixed_burst;
bool aal;
bool eame;
- bool multi_msi_en;
+ bool multi_irq_en;
bool dche;
};

@@ -215,7 +215,7 @@ struct dwmac4_addrs {
#define STMMAC_FLAG_TSO_EN BIT(4)
#define STMMAC_FLAG_SERDES_UP_AFTER_PHY_LINKUP BIT(5)
#define STMMAC_FLAG_VLAN_FAIL_Q_EN BIT(6)
-#define STMMAC_FLAG_MULTI_MSI_EN BIT(7)
+#define STMMAC_FLAG_MULTI_IRQ_EN BIT(7)
#define STMMAC_FLAG_EXT_SNAPSHOT_EN BIT(8)
#define STMMAC_FLAG_INT_SNAPSHOT_EN BIT(9)
#define STMMAC_FLAG_RX_CLK_RUNS_IN_LPI BIT(10)
--
2.34.1


2024-01-05 07:11:22

by Swee, Leong Ching

[permalink] [raw]
Subject: [PATCH net-next v2 4/4] net: stmmac: Use interrupt mode INTM=1 for per channel irq

From: Swee Leong Ching <[email protected]>

Enable per DMA channel interrupt that uses shared peripheral
interrupt (SPI), so only per channel TX and RX intr (TI/RI)
are handled by TX/RX ISR without calling common interrupt ISR.

Signed-off-by: Teoh Ji Sheng <[email protected]>
Signed-off-by: Swee Leong Ching <[email protected]>
---
.../net/ethernet/stmicro/stmmac/dwxgmac2.h | 3 ++
.../ethernet/stmicro/stmmac/dwxgmac2_dma.c | 32 +++++++++++--------
2 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
index 207ff1799f2c..04bf731cb7ea 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
@@ -346,6 +346,9 @@
/* DMA Registers */
#define XGMAC_DMA_MODE 0x00003000
#define XGMAC_SWR BIT(0)
+#define XGMAC_DMA_MODE_INTM_MASK GENMASK(13, 12)
+#define XGMAC_DMA_MODE_INTM_SHIFT 12
+#define XGMAC_DMA_MODE_INTM_MODE1 0x1
#define XGMAC_DMA_SYSBUS_MODE 0x00003004
#define XGMAC_WR_OSR_LMT GENMASK(29, 24)
#define XGMAC_WR_OSR_LMT_SHIFT 24
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index 3cde695fec91..dcb9f094415d 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 &= ~XGMAC_DMA_MODE_INTM_MASK;
+ value |= (XGMAC_DMA_MODE_INTM_MODE1 << XGMAC_DMA_MODE_INTM_SHIFT);
+ writel(value, ioaddr + XGMAC_DMA_MODE);
+ }
}

static void dwxgmac2_dma_init_chan(struct stmmac_priv *priv,
@@ -365,19 +372,18 @@ static int dwxgmac2_dma_interrupt(struct stmmac_priv *priv,
}

/* TX/RX NORMAL interrupts */
- if (likely(intr_status & XGMAC_NIS)) {
- if (likely(intr_status & XGMAC_RI)) {
- u64_stats_update_begin(&rxq_stats->syncp);
- rxq_stats->rx_normal_irq_n++;
- u64_stats_update_end(&rxq_stats->syncp);
- ret |= handle_rx;
- }
- if (likely(intr_status & (XGMAC_TI | XGMAC_TBU))) {
- u64_stats_update_begin(&txq_stats->syncp);
- txq_stats->tx_normal_irq_n++;
- u64_stats_update_end(&txq_stats->syncp);
- ret |= handle_tx;
- }
+ if (likely(intr_status & XGMAC_RI)) {
+ u64_stats_update_begin(&rxq_stats->syncp);
+ rxq_stats->rx_normal_irq_n++;
+ u64_stats_update_end(&rxq_stats->syncp);
+ ret |= handle_rx;
+ }
+
+ if (likely(intr_status & (XGMAC_TI | XGMAC_TBU))) {
+ u64_stats_update_begin(&txq_stats->syncp);
+ txq_stats->tx_normal_irq_n++;
+ u64_stats_update_end(&txq_stats->syncp);
+ ret |= handle_tx;
}

/* Clear interrupts */
--
2.34.1


2024-01-07 16:40:36

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/4] net: stmmac: Enable Per DMA Channel interrupt

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <[email protected]>:

On Fri, 5 Jan 2024 15:09:21 +0800 you wrote:
> From: Swee Leong Ching <[email protected]>
>
> Hi,
> Add Per DMA Channel interrupt feature for DWXGMAC IP.
>
> Patchset (link below) contains per DMA channel interrupt, But it was
> achieved.
> https://lore.kernel.org/lkml/20230821203328.GA2197059-
> [email protected]/t/#m849b529a642e1bff89c05a07efc25d6a94c8bfb4
>
> [...]

Here is the summary with links:
- [net-next,v2,1/4] dt-bindings: net: snps,dwmac: per channel irq
https://git.kernel.org/netdev/net-next/c/67d47c8ada0f
- [net-next,v2,2/4] net: stmmac: Make MSI interrupt routine generic
https://git.kernel.org/netdev/net-next/c/477bd4beb93b
- [net-next,v2,3/4] net: stmmac: Add support for TX/RX channel interrupt
https://git.kernel.org/netdev/net-next/c/9072e03d3208
- [net-next,v2,4/4] net: stmmac: Use interrupt mode INTM=1 for per channel irq
https://git.kernel.org/netdev/net-next/c/36af9f25ddfd

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



2024-01-07 19:07:10

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/4] net: stmmac: Enable Per DMA Channel interrupt

On 07/01/2024 17:40, [email protected] wrote:
> Hello:
>
> This series was applied to netdev/net-next.git (main)
> by David S. Miller <[email protected]>:
>
> On Fri, 5 Jan 2024 15:09:21 +0800 you wrote:
>> From: Swee Leong Ching <[email protected]>
>>
>> Hi,
>> Add Per DMA Channel interrupt feature for DWXGMAC IP.
>>
>> Patchset (link below) contains per DMA channel interrupt, But it was
>> achieved.
>> https://lore.kernel.org/lkml/20230821203328.GA2197059-
>> [email protected]/t/#m849b529a642e1bff89c05a07efc25d6a94c8bfb4
>>
>> [...]
>
> Here is the summary with links:
> - [net-next,v2,1/4] dt-bindings: net: snps,dwmac: per channel irq
> https://git.kernel.org/netdev/net-next/c/67d47c8ada0f

Please wait for DT bindings review a bit more than one working day (I
don't count Saturday and Sunday, because we all have some life...).

Best regards,
Krzysztof


2024-01-07 20:10:53

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH net-next v2 1/4] dt-bindings: net: snps,dwmac: per channel irq

On Fri, Jan 05, 2024 at 03:09:22PM +0800, Leong Ching Swee wrote:
> From: Swee Leong Ching <[email protected]>
>
> Add dt-bindings for per channel irq.
>
> Signed-off-by: Rohan G Thomas <[email protected]>
> Signed-off-by: Swee Leong Ching <[email protected]>
> ---
> .../devicetree/bindings/net/snps,dwmac.yaml | 24 +++++++++++++------
> 1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index 5c2769dc689a..e72dded824f4 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -103,17 +103,27 @@ properties:
>
> interrupts:
> minItems: 1
> - items:
> - - description: Combined signal for various interrupt events
> - - description: The interrupt to manage the remote wake-up packet detection
> - - description: The interrupt that occurs when Rx exits the LPI state
> + maxItems: 19
>
> interrupt-names:
> minItems: 1
> + maxItems: 19
> items:
> - - const: macirq
> - - enum: [eth_wake_irq, eth_lpi]
> - - const: eth_lpi
> + oneOf:
> + - description: Combined signal for various interrupt events
> + const: macirq
> + - description: The interrupt to manage the remote wake-up packet detection
> + const: eth_wake_irq
> + - description: The interrupt that occurs when Rx exits the LPI state
> + const: eth_lpi
> + - description: DMA Tx per-channel interrupt
> + pattern: '^dma_tx[0-7]?$'
> + - description: DMA Rx per-channel interrupt
> + pattern: '^dma_rx[0-7]?$'
> +
> + allOf:
> + - contains:
> + const: macirq

In order to restore the v1 discussion around this change, here is my
comment copied from there:

> As Rob correctly noted it's also better to make sure that 'macirq' is placed first
> in the array. So instead of the constraint above I guess the next one would
> make sure both the array has 'macirq' name and it's the first item:
>
> allOf:
> - maxItems: 34
> items:
> - const: macirq

Leong said it didn't work:
https://lore.kernel.org/netdev/CH0PR11MB54904615B45E521DE6B1A7B3CF61A@CH0PR11MB5490.namprd11.prod.outlook.com/

Rob, Krzysztof, Conor could you please clarify whether this change is ok the
way it is or it would be better to preserve the stricter constraint
and fix the DT-schema validation tool somehow?

-Serge(y)

>
> clocks:
> minItems: 1
> --
> 2.34.1
>
>

2024-01-07 20:28:00

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH net-next v2 2/4] net: stmmac: Make MSI interrupt routine generic

On Fri, Jan 05, 2024 at 03:09:23PM +0800, Leong Ching Swee wrote:
> From: Swee Leong Ching <[email protected]>
>
> There is no support for per DMA channel interrupt for non-MSI platform,
> where the MAC's per channel interrupt hooks up to interrupt controller(GIC)
> through shared peripheral interrupt(SPI) to handle interrupt from TX/RX
> transmit channel.
>
> This patch generalize the existing MSI ISR to also support non-MSI
> platform.

Basically this patch just fixes the individual IRQ handling code
names.

>
> Signed-off-by: Teoh Ji Sheng <[email protected]>
> Signed-off-by: Swee Leong Ching <[email protected]>
> ---
> .../net/ethernet/stmicro/stmmac/dwmac-intel.c | 4 +--
> .../ethernet/stmicro/stmmac/dwmac-socfpga.c | 3 ++
> .../net/ethernet/stmicro/stmmac/dwmac4_dma.c | 2 +-
> .../net/ethernet/stmicro/stmmac/stmmac_main.c | 30 +++++++++----------
> include/linux/stmmac.h | 4 +--
> 5 files changed, 23 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> index 60283543ffc8..f0ec69af96c9 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> @@ -952,7 +952,7 @@ static int stmmac_config_single_msi(struct pci_dev *pdev,
>
> res->irq = pci_irq_vector(pdev, 0);
> res->wol_irq = res->irq;
> - plat->flags &= ~STMMAC_FLAG_MULTI_MSI_EN;
> + plat->flags &= ~STMMAC_FLAG_MULTI_IRQ_EN;
> dev_info(&pdev->dev, "%s: Single IRQ enablement successful\n",
> __func__);
>
> @@ -1004,7 +1004,7 @@ static int stmmac_config_multi_msi(struct pci_dev *pdev,
> if (plat->msi_sfty_ue_vec < STMMAC_MSI_VEC_MAX)
> res->sfty_ue_irq = pci_irq_vector(pdev, plat->msi_sfty_ue_vec);
>
> - plat->flags |= STMMAC_FLAG_MULTI_MSI_EN;
> + plat->flags |= STMMAC_FLAG_MULTI_IRQ_EN;
> dev_info(&pdev->dev, "%s: multi MSI enablement successful\n", __func__);
>
> return 0;
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> index ba2ce776bd4d..cf43fb3c6cc5 100644

> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> @@ -427,6 +427,9 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
> plat_dat->bsp_priv = dwmac;
> plat_dat->fix_mac_speed = socfpga_dwmac_fix_mac_speed;
>
> + if (stmmac_res.rx_irq[0] > 0 && stmmac_res.tx_irq[0] > 0)
> + plat_dat->flags |= STMMAC_FLAG_MULTI_IRQ_EN;
> +
> ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
> if (ret)
> return ret;

This is unrelated change. It adds the individual DMA IRQs support for
the SoC FPGA platform, which AFAICS doesn't have it supported at the
moment. Please move this into a separate patch with the commit log
describing the change.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> index 84d3a8551b03..5f649106ffcd 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> @@ -175,7 +175,7 @@ static void dwmac4_dma_init(void __iomem *ioaddr,
>
> value = readl(ioaddr + DMA_BUS_MODE);
>
> - if (dma_cfg->multi_msi_en) {
> + if (dma_cfg->multi_irq_en) {
> value &= ~DMA_BUS_MODE_INTM_MASK;
> value |= (DMA_BUS_MODE_INTM_MODE1 << DMA_BUS_MODE_INTM_SHIFT);
> }
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 47de466e432c..57873b879b33 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -129,8 +129,8 @@ static irqreturn_t stmmac_interrupt(int irq, void *dev_id);
> /* For MSI interrupts handling */
> static irqreturn_t stmmac_mac_interrupt(int irq, void *dev_id);
> static irqreturn_t stmmac_safety_interrupt(int irq, void *dev_id);
> -static irqreturn_t stmmac_msi_intr_tx(int irq, void *data);
> -static irqreturn_t stmmac_msi_intr_rx(int irq, void *data);
> +static irqreturn_t stmmac_dma_tx_interrupt(int irq, void *data);
> +static irqreturn_t stmmac_dma_rx_interrupt(int irq, void *data);
> static void stmmac_reset_rx_queue(struct stmmac_priv *priv, u32 queue);
> static void stmmac_reset_tx_queue(struct stmmac_priv *priv, u32 queue);
> static void stmmac_reset_queues_param(struct stmmac_priv *priv);
> @@ -3602,7 +3602,7 @@ static void stmmac_free_irq(struct net_device *dev,
> }
> }
>
> -static int stmmac_request_irq_multi_msi(struct net_device *dev)
> +static int stmmac_request_irq_multi(struct net_device *dev)
> {
> struct stmmac_priv *priv = netdev_priv(dev);
> enum request_irq_err irq_err;
> @@ -3697,7 +3697,7 @@ static int stmmac_request_irq_multi_msi(struct net_device *dev)
> }
> }
>
> - /* Request Rx MSI irq */

> + /* Request Rx irq */

s/irq/IRQ
(capitalize)

> for (i = 0; i < priv->plat->rx_queues_to_use; i++) {
> if (i >= MTL_MAX_RX_QUEUES)
> break;
> @@ -3707,11 +3707,11 @@ static int stmmac_request_irq_multi_msi(struct net_device *dev)
> int_name = priv->int_name_rx_irq[i];
> sprintf(int_name, "%s:%s-%d", dev->name, "rx", i);
> ret = request_irq(priv->rx_irq[i],
> - stmmac_msi_intr_rx,
> + stmmac_dma_rx_interrupt,
> 0, int_name, &priv->dma_conf.rx_queue[i]);
> if (unlikely(ret < 0)) {
> netdev_err(priv->dev,
> - "%s: alloc rx-%d MSI %d (error: %d)\n",

> + "%s: alloc rx-%d dma rx_irq %d (error: %d)\n",

s/ dma/DMA
(capitalize and drop extra space)

> __func__, i, priv->rx_irq[i], ret);
> irq_err = REQ_IRQ_ERR_RX;
> irq_idx = i;
> @@ -3722,7 +3722,7 @@ static int stmmac_request_irq_multi_msi(struct net_device *dev)
> irq_set_affinity_hint(priv->rx_irq[i], &cpu_mask);
> }
>
> - /* Request Tx MSI irq */

> + /* Request Tx irq */

s/irq/IRQ

> for (i = 0; i < priv->plat->tx_queues_to_use; i++) {
> if (i >= MTL_MAX_TX_QUEUES)
> break;
> @@ -3732,11 +3732,11 @@ static int stmmac_request_irq_multi_msi(struct net_device *dev)
> int_name = priv->int_name_tx_irq[i];
> sprintf(int_name, "%s:%s-%d", dev->name, "tx", i);
> ret = request_irq(priv->tx_irq[i],
> - stmmac_msi_intr_tx,
> + stmmac_dma_tx_interrupt,
> 0, int_name, &priv->dma_conf.tx_queue[i]);
> if (unlikely(ret < 0)) {
> netdev_err(priv->dev,
> - "%s: alloc tx-%d MSI %d (error: %d)\n",

> + "%s: alloc tx-%d dma tx_irq %d (error: %d)\n",

s/ dma/DMA

-Serge(y)

> __func__, i, priv->tx_irq[i], ret);
> irq_err = REQ_IRQ_ERR_TX;
> irq_idx = i;
> @@ -3811,8 +3811,8 @@ static int stmmac_request_irq(struct net_device *dev)
> int ret;
>
> /* Request the IRQ lines */
> - if (priv->plat->flags & STMMAC_FLAG_MULTI_MSI_EN)
> - ret = stmmac_request_irq_multi_msi(dev);
> + if (priv->plat->flags & STMMAC_FLAG_MULTI_IRQ_EN)
> + ret = stmmac_request_irq_multi(dev);
> else
> ret = stmmac_request_irq_single(dev);
>
> @@ -6075,7 +6075,7 @@ static irqreturn_t stmmac_safety_interrupt(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> -static irqreturn_t stmmac_msi_intr_tx(int irq, void *data)
> +static irqreturn_t stmmac_dma_tx_interrupt(int irq, void *data)
> {
> struct stmmac_tx_queue *tx_q = (struct stmmac_tx_queue *)data;
> struct stmmac_dma_conf *dma_conf;
> @@ -6107,7 +6107,7 @@ static irqreturn_t stmmac_msi_intr_tx(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> -static irqreturn_t stmmac_msi_intr_rx(int irq, void *data)
> +static irqreturn_t stmmac_dma_rx_interrupt(int irq, void *data)
> {
> struct stmmac_rx_queue *rx_q = (struct stmmac_rx_queue *)data;
> struct stmmac_dma_conf *dma_conf;
> @@ -7456,8 +7456,8 @@ int stmmac_dvr_probe(struct device *device,
> priv->plat = plat_dat;
> priv->ioaddr = res->addr;
> priv->dev->base_addr = (unsigned long)res->addr;
> - priv->plat->dma_cfg->multi_msi_en =
> - (priv->plat->flags & STMMAC_FLAG_MULTI_MSI_EN);
> + priv->plat->dma_cfg->multi_irq_en =
> + (priv->plat->flags & STMMAC_FLAG_MULTI_IRQ_EN);
>
> priv->dev->irq = res->irq;
> priv->wol_irq = res->wol_irq;
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index dee5ad6e48c5..b950e6f9761d 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -98,7 +98,7 @@ struct stmmac_dma_cfg {
> int mixed_burst;
> bool aal;
> bool eame;
> - bool multi_msi_en;
> + bool multi_irq_en;
> bool dche;
> };
>
> @@ -215,7 +215,7 @@ struct dwmac4_addrs {
> #define STMMAC_FLAG_TSO_EN BIT(4)
> #define STMMAC_FLAG_SERDES_UP_AFTER_PHY_LINKUP BIT(5)
> #define STMMAC_FLAG_VLAN_FAIL_Q_EN BIT(6)
> -#define STMMAC_FLAG_MULTI_MSI_EN BIT(7)
> +#define STMMAC_FLAG_MULTI_IRQ_EN BIT(7)
> #define STMMAC_FLAG_EXT_SNAPSHOT_EN BIT(8)
> #define STMMAC_FLAG_INT_SNAPSHOT_EN BIT(9)
> #define STMMAC_FLAG_RX_CLK_RUNS_IN_LPI BIT(10)
> --
> 2.34.1
>
>

2024-01-07 20:39:06

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH net-next v2 3/4] net: stmmac: Add support for TX/RX channel interrupt

On Fri, Jan 05, 2024 at 03:09:24PM +0800, Leong Ching Swee wrote:
> From: Swee Leong Ching <[email protected]>
>
> Enable TX/RX channel interrupt registration for MAC that interrupts CPU
> through shared peripheral interrupt (SPI).
>
> Per channel interrupts and interrupt-names are registered through,
> Eg: 4 tx and 4 rx channels:
> interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
> <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
> <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
> <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
> <GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>;
> interrupt-names = "dma_tx0",
> "dma_tx1",
> "dma_tx2",
> "dma_tx3",
> "dma_rx0",
> "dma_rx1",
> "dma_rx2",
> "dma_rx3";
>
> Signed-off-by: Teoh Ji Sheng <[email protected]>
> Signed-off-by: Swee Leong Ching <[email protected]>
> ---
> .../ethernet/stmicro/stmmac/stmmac_platform.c | 28 +++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 70eadc83ca68..ae6859153e98 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -710,6 +710,10 @@ EXPORT_SYMBOL_GPL(devm_stmmac_probe_config_dt);
> int stmmac_get_platform_resources(struct platform_device *pdev,
> struct stmmac_resources *stmmac_res)
> {

> + char irq_name[9];
> + int i;
> + int irq;
> +

Reverse xmas tree please. Also what the point in having "i" and "irq"
defined separately? Wouldn't it be better to merge them into a single
statement:
+ char irq_name[9];
+ int i, irq;

> memset(stmmac_res, 0, sizeof(*stmmac_res));
>
> /* Get IRQ information early to have an ability to ask for deferred
> @@ -743,6 +747,30 @@ int stmmac_get_platform_resources(struct platform_device *pdev,
> dev_info(&pdev->dev, "IRQ eth_lpi not found\n");
> }
>

> + /* For RX Channel */

Why haven't you added a more descriptive comment as I suggested on v1:

+ /* Get optional Tx/Rx DMA per-channel IRQs, which otherwise
+ * are supposed to be delivered via the common MAC IRQ line
+ */

?

> + for (i = 0; i < MTL_MAX_RX_QUEUES; i++) {
> + snprintf(irq_name, sizeof(irq_name), "dma_rx%i", i);
> + irq = platform_get_irq_byname_optional(pdev, irq_name);
> + if (irq == -EPROBE_DEFER)
> + return irq;
> + else if (irq < 0)
> + break;
> +
> + stmmac_res->rx_irq[i] = irq;
> + }
> +

> + /* For TX Channel */

* see the comment above

-Serge(y)

> + for (i = 0; i < MTL_MAX_TX_QUEUES; i++) {
> + snprintf(irq_name, sizeof(irq_name), "dma_tx%i", i);
> + irq = platform_get_irq_byname_optional(pdev, irq_name);
> + if (irq == -EPROBE_DEFER)
> + return irq;
> + else if (irq < 0)
> + break;
> +
> + stmmac_res->tx_irq[i] = irq;
> + }
> +
> stmmac_res->addr = devm_platform_ioremap_resource(pdev, 0);
>
> return PTR_ERR_OR_ZERO(stmmac_res->addr);
> --
> 2.34.1
>
>

2024-01-07 20:52:28

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH net-next v2 4/4] net: stmmac: Use interrupt mode INTM=1 for per channel irq

On Fri, Jan 05, 2024 at 03:09:25PM +0800, Leong Ching Swee wrote:
> From: Swee Leong Ching <[email protected]>
>
> Enable per DMA channel interrupt that uses shared peripheral
> interrupt (SPI), so only per channel TX and RX intr (TI/RI)
> are handled by TX/RX ISR without calling common interrupt ISR.
>
> Signed-off-by: Teoh Ji Sheng <[email protected]>
> Signed-off-by: Swee Leong Ching <[email protected]>
> ---
> .../net/ethernet/stmicro/stmmac/dwxgmac2.h | 3 ++
> .../ethernet/stmicro/stmmac/dwxgmac2_dma.c | 32 +++++++++++--------
> 2 files changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> index 207ff1799f2c..04bf731cb7ea 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> @@ -346,6 +346,9 @@
> /* DMA Registers */
> #define XGMAC_DMA_MODE 0x00003000
> #define XGMAC_SWR BIT(0)

> +#define XGMAC_DMA_MODE_INTM_MASK GENMASK(13, 12)
> +#define XGMAC_DMA_MODE_INTM_SHIFT 12
> +#define XGMAC_DMA_MODE_INTM_MODE1 0x1

AFAICS the DW XGMAC module doesn't maintain a convention of having the
CSR fields macro names prefixed with the CSR name. Let's drop the
DMA_MODE suffix from the macro name then:
+#define XGMAC_INTM_MASK GENMASK(13, 12)
+#define XGMAC_INTM_SHIFT 12
+#define XGMAC_INTM_MODE1 0x1
to have it unified with the rest of the macros in dwxgmac2.h.

Other than that the change looks good. Thanks.

-Serge(y)

> #define XGMAC_DMA_SYSBUS_MODE 0x00003004
> #define XGMAC_WR_OSR_LMT GENMASK(29, 24)
> #define XGMAC_WR_OSR_LMT_SHIFT 24
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
> index 3cde695fec91..dcb9f094415d 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 &= ~XGMAC_DMA_MODE_INTM_MASK;
> + value |= (XGMAC_DMA_MODE_INTM_MODE1 << XGMAC_DMA_MODE_INTM_SHIFT);
> + writel(value, ioaddr + XGMAC_DMA_MODE);
> + }
> }
>
> static void dwxgmac2_dma_init_chan(struct stmmac_priv *priv,
> @@ -365,19 +372,18 @@ static int dwxgmac2_dma_interrupt(struct stmmac_priv *priv,
> }
>
> /* TX/RX NORMAL interrupts */
> - if (likely(intr_status & XGMAC_NIS)) {
> - if (likely(intr_status & XGMAC_RI)) {
> - u64_stats_update_begin(&rxq_stats->syncp);
> - rxq_stats->rx_normal_irq_n++;
> - u64_stats_update_end(&rxq_stats->syncp);
> - ret |= handle_rx;
> - }
> - if (likely(intr_status & (XGMAC_TI | XGMAC_TBU))) {
> - u64_stats_update_begin(&txq_stats->syncp);
> - txq_stats->tx_normal_irq_n++;
> - u64_stats_update_end(&txq_stats->syncp);
> - ret |= handle_tx;
> - }
> + if (likely(intr_status & XGMAC_RI)) {
> + u64_stats_update_begin(&rxq_stats->syncp);
> + rxq_stats->rx_normal_irq_n++;
> + u64_stats_update_end(&rxq_stats->syncp);
> + ret |= handle_rx;
> + }
> +
> + if (likely(intr_status & (XGMAC_TI | XGMAC_TBU))) {
> + u64_stats_update_begin(&txq_stats->syncp);
> + txq_stats->tx_normal_irq_n++;
> + u64_stats_update_end(&txq_stats->syncp);
> + ret |= handle_tx;
> }
>
> /* Clear interrupts */
> --
> 2.34.1
>
>

2024-01-07 21:24:40

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/4] net: stmmac: Enable Per DMA Channel interrupt

On Sun, Jan 07, 2024 at 08:06:55PM +0100, Krzysztof Kozlowski wrote:
> On 07/01/2024 17:40, [email protected] wrote:
> > Hello:
> >
> > This series was applied to netdev/net-next.git (main)
> > by David S. Miller <[email protected]>:
> >
> > On Fri, 5 Jan 2024 15:09:21 +0800 you wrote:
> >> From: Swee Leong Ching <[email protected]>
> >>
> >> Hi,
> >> Add Per DMA Channel interrupt feature for DWXGMAC IP.
> >>
> >> Patchset (link below) contains per DMA channel interrupt, But it was
> >> achieved.
> >> https://lore.kernel.org/lkml/20230821203328.GA2197059-
> >> [email protected]/t/#m849b529a642e1bff89c05a07efc25d6a94c8bfb4
> >>
> >> [...]
> >
> > Here is the summary with links:
> > - [net-next,v2,1/4] dt-bindings: net: snps,dwmac: per channel irq
> > https://git.kernel.org/netdev/net-next/c/67d47c8ada0f
>
> Please wait for DT bindings review a bit more than one working day (I
> don't count Saturday and Sunday, because we all have some life...).

+1. Would be very nice to have some more time to review the rest of
the bits too. This would be specifically important for the STMMAC
driver which doesn't currently have active maintainer. What about 5-10
work days to make sure that no comment would be submitted? Besides I
thought that no features were supposed to be submitted during the
merge window. Are we over the merge window already? (I might have lost
track of time on the holidays.)

Leong, next time before re-submitting your patchsets please wait for
some more time than just two days. I waited for your response for
almost two weeks.

-Serge(y)

>
> Best regards,
> Krzysztof
>
>

2024-01-08 01:02:40

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/4] net: stmmac: Enable Per DMA Channel interrupt

On Mon, 8 Jan 2024 00:24:24 +0300 Serge Semin wrote:
> > Please wait for DT bindings review a bit more than one working day (I
> > don't count Saturday and Sunday, because we all have some life...).
>
> +1.

Sorry about that, reverting.

2024-01-09 08:06:18

by Swee, Leong Ching

[permalink] [raw]
Subject: RE: [PATCH net-next v2 0/4] net: stmmac: Enable Per DMA Channel interrupt


> -----Original Message-----
> From: Serge Semin <[email protected]>
> Sent: Monday, January 8, 2024 5:24 AM
> To: Krzysztof Kozlowski <[email protected]>; Swee, Leong
> Ching <[email protected]>; David S. Miller
> <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH net-next v2 0/4] net: stmmac: Enable Per DMA Channel
> interrupt
>
> On Sun, Jan 07, 2024 at 08:06:55PM +0100, Krzysztof Kozlowski wrote:
> > On 07/01/2024 17:40, [email protected] wrote:
> > > Hello:
> > >
> > > This series was applied to netdev/net-next.git (main) by David S.
> > > Miller <[email protected]>:
> > >
> > > On Fri, 5 Jan 2024 15:09:21 +0800 you wrote:
> > >> From: Swee Leong Ching <[email protected]>
> > >>
> > >> Hi,
> > >> Add Per DMA Channel interrupt feature for DWXGMAC IP.
> > >>
> > >> Patchset (link below) contains per DMA channel interrupt, But it
> > >> was achieved.
> > >> https://lore.kernel.org/lkml/20230821203328.GA2197059-
> > >> [email protected]/t/#m849b529a642e1bff89c05a07efc25d6a94c8bfb4
> > >>
> > >> [...]
> > >
> > > Here is the summary with links:
> > > - [net-next,v2,1/4] dt-bindings: net: snps,dwmac: per channel irq
> > > https://git.kernel.org/netdev/net-next/c/67d47c8ada0f
> >
> > Please wait for DT bindings review a bit more than one working day (I
> > don't count Saturday and Sunday, because we all have some life...).
>
> +1. Would be very nice to have some more time to review the rest of
> the bits too. This would be specifically important for the STMMAC driver
> which doesn't currently have active maintainer. What about 5-10 work days
> to make sure that no comment would be submitted? Besides I thought that
> no features were supposed to be submitted during the merge window. Are
> we over the merge window already? (I might have lost track of time on the
> holidays.)
>
> Leong, next time before re-submitting your patchsets please wait for some
> more time than just two days. I waited for your response for almost two
> weeks.
>
> -Serge(y)
>
Sorry for that, will wait 5-10 days before resubmitting v3 patches.
> >
> > Best regards,
> > Krzysztof
> >
> >

2024-01-09 09:20:12

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH net-next v2 1/4] dt-bindings: net: snps,dwmac: per channel irq

On 07/01/2024 21:10, Serge Semin wrote:
> On Fri, Jan 05, 2024 at 03:09:22PM +0800, Leong Ching Swee wrote:
>> From: Swee Leong Ching <[email protected]>
>>
>> Add dt-bindings for per channel irq.
>>
>> Signed-off-by: Rohan G Thomas <[email protected]>
>> Signed-off-by: Swee Leong Ching <[email protected]>
>> ---
>> .../devicetree/bindings/net/snps,dwmac.yaml | 24 +++++++++++++------
>> 1 file changed, 17 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> index 5c2769dc689a..e72dded824f4 100644
>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> @@ -103,17 +103,27 @@ properties:
>>
>> interrupts:
>> minItems: 1
>> - items:
>> - - description: Combined signal for various interrupt events
>> - - description: The interrupt to manage the remote wake-up packet detection
>> - - description: The interrupt that occurs when Rx exits the LPI state
>> + maxItems: 19
>>
>> interrupt-names:
>> minItems: 1
>> + maxItems: 19
>> items:
>> - - const: macirq
>> - - enum: [eth_wake_irq, eth_lpi]
>> - - const: eth_lpi
>> + oneOf:
>> + - description: Combined signal for various interrupt events
>> + const: macirq
>> + - description: The interrupt to manage the remote wake-up packet detection
>> + const: eth_wake_irq
>> + - description: The interrupt that occurs when Rx exits the LPI state
>> + const: eth_lpi
>> + - description: DMA Tx per-channel interrupt
>> + pattern: '^dma_tx[0-7]?$'
>> + - description: DMA Rx per-channel interrupt
>> + pattern: '^dma_rx[0-7]?$'
>> +
>> + allOf:
>> + - contains:
>> + const: macirq
>
> In order to restore the v1 discussion around this change, here is my
> comment copied from there:
>
>> As Rob correctly noted it's also better to make sure that 'macirq' is placed first
>> in the array. So instead of the constraint above I guess the next one would
>> make sure both the array has 'macirq' name and it's the first item:
>>
>> allOf:
>> - maxItems: 34
>> items:
>> - const: macirq
>
> Leong said it didn't work:
> https://lore.kernel.org/netdev/CH0PR11MB54904615B45E521DE6B1A7B3CF61A@CH0PR11MB5490.namprd11.prod.outlook.com/
>
> Rob, Krzysztof, Conor could you please clarify whether this change is ok the
> way it is or it would be better to preserve the stricter constraint
> and fix the DT-schema validation tool somehow?

First of all this change is not good, because commit msg explains
absolutely nothing why this is done and what exactly you want to achieve
here. The "what" part often is obvious from the code, but not in this
case. Are the per-channel IRQs conflicting with macirq or others? Are
they complementary (maxItems: 19 suggests that, though, but could be
mistake as well)? Do they affect all snps,dwmac derivatives or only some?

So many questions and zero answers in one liner commit msg!

Now about the problem, I think we should preserve the order, assuming
that these are complementary so first three must be defined. This
however could be done in the device schema referencing snps,dwmac. I
think I will repeat myself: I dislike this schema, because it mixes two
purposes: defining shared part and defining final device part. The code
in this patch is fine for a schema defining the shared part.

Therefore before we start growing this monstrosity into bigger one, I
think we should go back to the plans of reworking and cleaning it.

Best regards,
Krzysztof


2024-01-09 22:21:40

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH net-next v2 1/4] dt-bindings: net: snps,dwmac: per channel irq

On Tue, Jan 09, 2024 at 10:10:37AM +0100, Krzysztof Kozlowski wrote:
> On 07/01/2024 21:10, Serge Semin wrote:
> > On Fri, Jan 05, 2024 at 03:09:22PM +0800, Leong Ching Swee wrote:
> >> From: Swee Leong Ching <[email protected]>
> >>
> >> Add dt-bindings for per channel irq.
> >>
> >> Signed-off-by: Rohan G Thomas <[email protected]>
> >> Signed-off-by: Swee Leong Ching <[email protected]>
> >> ---
> >> .../devicetree/bindings/net/snps,dwmac.yaml | 24 +++++++++++++------
> >> 1 file changed, 17 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> >> index 5c2769dc689a..e72dded824f4 100644
> >> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> >> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> >> @@ -103,17 +103,27 @@ properties:
> >>
> >> interrupts:
> >> minItems: 1
> >> - items:
> >> - - description: Combined signal for various interrupt events
> >> - - description: The interrupt to manage the remote wake-up packet detection
> >> - - description: The interrupt that occurs when Rx exits the LPI state
> >> + maxItems: 19
> >>
> >> interrupt-names:
> >> minItems: 1
> >> + maxItems: 19
> >> items:
> >> - - const: macirq
> >> - - enum: [eth_wake_irq, eth_lpi]
> >> - - const: eth_lpi
> >> + oneOf:
> >> + - description: Combined signal for various interrupt events
> >> + const: macirq
> >> + - description: The interrupt to manage the remote wake-up packet detection
> >> + const: eth_wake_irq
> >> + - description: The interrupt that occurs when Rx exits the LPI state
> >> + const: eth_lpi
> >> + - description: DMA Tx per-channel interrupt
> >> + pattern: '^dma_tx[0-7]?$'
> >> + - description: DMA Rx per-channel interrupt
> >> + pattern: '^dma_rx[0-7]?$'
> >> +
> >> + allOf:
> >> + - contains:
> >> + const: macirq
> >
> > In order to restore the v1 discussion around this change, here is my
> > comment copied from there:
> >
> >> As Rob correctly noted it's also better to make sure that 'macirq' is placed first
> >> in the array. So instead of the constraint above I guess the next one would
> >> make sure both the array has 'macirq' name and it's the first item:
> >>
> >> allOf:
> >> - maxItems: 34
> >> items:
> >> - const: macirq
> >
> > Leong said it didn't work:
> > https://lore.kernel.org/netdev/CH0PR11MB54904615B45E521DE6B1A7B3CF61A@CH0PR11MB5490.namprd11.prod.outlook.com/
> >
> > Rob, Krzysztof, Conor could you please clarify whether this change is ok the
> > way it is or it would be better to preserve the stricter constraint
> > and fix the DT-schema validation tool somehow?
>

> First of all this change is not good, because commit msg explains
> absolutely nothing why this is done and what exactly you want to achieve
> here. The "what" part often is obvious from the code, but not in this
> case. Are the per-channel IRQs conflicting with macirq or others? Are
> they complementary (maxItems: 19 suggests that, though, but could be
> mistake as well)? Do they affect all snps,dwmac derivatives or only some?
>
> So many questions and zero answers in one liner commit msg!

Right. The commit message is way too modest =) Leong?

>
> Now about the problem, I think we should preserve the order, assuming
> that these are complementary so first three must be defined.

Ok. But please note that "Wake" and "LPI" IRQs are optional. It's
possible to have a device with the "MAC" and "DMA" IRQs and no
individual "Wake"/"LPI" IRQ lines. Thus the only mandatory IRQ is
"MAC" which order (being always first), I agree, should be preserved.

> This
> however could be done in the device schema referencing snps,dwmac. I
> think I will repeat myself: I dislike this schema, because it mixes two
> purposes: defining shared part and defining final device part. The code
> in this patch is fine for a schema defining the shared part.
>
> Therefore before we start growing this monstrosity into bigger one, I
> think we should go back to the plans of reworking and cleaning it.

If you are talking about the changes like introduced here (essentially
it's Patch 4):
https://www.spinics.net/lists/netdev/msg888079.html
I can resurrect it (rebase on the latest kernel, fix the notes, run
dt-validation, etc) and submit for review on the next week or so.
Then the Leong' patch in subject either won't be necessary or will
concern the shared schema only. Does it sound acceptable?

-Serge(y)

>
> Best regards,
> Krzysztof
>

2024-01-10 05:43:54

by Swee, Leong Ching

[permalink] [raw]
Subject: RE: [PATCH net-next v2 4/4] net: stmmac: Use interrupt mode INTM=1 for per channel irq

> -----Original Message-----
> From: Serge Semin <[email protected]>
> Sent: Monday, January 8, 2024 4:52 AM
> To: Swee, Leong Ching <[email protected]>
> Cc: Maxime Coquelin <[email protected]>; Alexandre Torgue
> <[email protected]>; Jose Abreu <[email protected]>;
> David S . Miller <[email protected]>; Eric Dumazet
> <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni
> <[email protected]>; Rob Herring <[email protected]>; Krzysztof
> Kozlowski <[email protected]>; Conor Dooley
> <[email protected]>; Giuseppe Cavallaro <[email protected]>;
> [email protected]; linux-arm-
> [email protected]; [email protected];
> [email protected]; [email protected]; Teoh Ji Sheng
> <[email protected]>
> Subject: Re: [PATCH net-next v2 4/4] net: stmmac: Use interrupt mode
> INTM=1 for per channel irq
>
> On Fri, Jan 05, 2024 at 03:09:25PM +0800, Leong Ching Swee wrote:
> > From: Swee Leong Ching <[email protected]>
> >
> > Enable per DMA channel interrupt that uses shared peripheral interrupt
> > (SPI), so only per channel TX and RX intr (TI/RI) are handled by TX/RX
> > ISR without calling common interrupt ISR.
> >
> > Signed-off-by: Teoh Ji Sheng <[email protected]>
> > Signed-off-by: Swee Leong Ching <[email protected]>
> > ---
> > .../net/ethernet/stmicro/stmmac/dwxgmac2.h | 3 ++
> > .../ethernet/stmicro/stmmac/dwxgmac2_dma.c | 32 +++++++++++-------
> -
> > 2 files changed, 22 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> > b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> > index 207ff1799f2c..04bf731cb7ea 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> > @@ -346,6 +346,9 @@
> > /* DMA Registers */
> > #define XGMAC_DMA_MODE 0x00003000
> > #define XGMAC_SWR BIT(0)
>
> > +#define XGMAC_DMA_MODE_INTM_MASK GENMASK(13, 12)
> > +#define XGMAC_DMA_MODE_INTM_SHIFT 12
> > +#define XGMAC_DMA_MODE_INTM_MODE1 0x1
>
> AFAICS the DW XGMAC module doesn't maintain a convention of having the
> CSR fields macro names prefixed with the CSR name. Let's drop the
> DMA_MODE suffix from the macro name then:
> +#define XGMAC_INTM_MASK GENMASK(13, 12)
> +#define XGMAC_INTM_SHIFT 12
> +#define XGMAC_INTM_MODE1 0x1
> to have it unified with the rest of the macros in dwxgmac2.h.
>
> Other than that the change looks good. Thanks.
>
> -Serge(y)
>
Thanks. Will rename the macros in v3.
> > #define XGMAC_DMA_SYSBUS_MODE 0x00003004
> > #define XGMAC_WR_OSR_LMT GENMASK(29, 24)
> > #define XGMAC_WR_OSR_LMT_SHIFT 24
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
> > b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
> > index 3cde695fec91..dcb9f094415d 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 &= ~XGMAC_DMA_MODE_INTM_MASK;
> > + value |= (XGMAC_DMA_MODE_INTM_MODE1 <<
> XGMAC_DMA_MODE_INTM_SHIFT);
> > + writel(value, ioaddr + XGMAC_DMA_MODE);
> > + }
> > }
> >
> > static void dwxgmac2_dma_init_chan(struct stmmac_priv *priv, @@
> > -365,19 +372,18 @@ static int dwxgmac2_dma_interrupt(struct
> stmmac_priv *priv,
> > }
> >
> > /* TX/RX NORMAL interrupts */
> > - if (likely(intr_status & XGMAC_NIS)) {
> > - if (likely(intr_status & XGMAC_RI)) {
> > - u64_stats_update_begin(&rxq_stats->syncp);
> > - rxq_stats->rx_normal_irq_n++;
> > - u64_stats_update_end(&rxq_stats->syncp);
> > - ret |= handle_rx;
> > - }
> > - if (likely(intr_status & (XGMAC_TI | XGMAC_TBU))) {
> > - u64_stats_update_begin(&txq_stats->syncp);
> > - txq_stats->tx_normal_irq_n++;
> > - u64_stats_update_end(&txq_stats->syncp);
> > - ret |= handle_tx;
> > - }
> > + if (likely(intr_status & XGMAC_RI)) {
> > + u64_stats_update_begin(&rxq_stats->syncp);
> > + rxq_stats->rx_normal_irq_n++;
> > + u64_stats_update_end(&rxq_stats->syncp);
> > + ret |= handle_rx;
> > + }
> > +
> > + if (likely(intr_status & (XGMAC_TI | XGMAC_TBU))) {
> > + u64_stats_update_begin(&txq_stats->syncp);
> > + txq_stats->tx_normal_irq_n++;
> > + u64_stats_update_end(&txq_stats->syncp);
> > + ret |= handle_tx;
> > }
> >
> > /* Clear interrupts */
> > --
> > 2.34.1
> >
> >

2024-01-10 05:45:41

by Swee, Leong Ching

[permalink] [raw]
Subject: RE: [PATCH net-next v2 3/4] net: stmmac: Add support for TX/RX channel interrupt

> -----Original Message-----
> From: Serge Semin <[email protected]>
> Sent: Monday, January 8, 2024 4:39 AM
> To: Swee, Leong Ching <[email protected]>
> Cc: Maxime Coquelin <[email protected]>; Alexandre Torgue
> <[email protected]>; Jose Abreu <[email protected]>;
> David S . Miller <[email protected]>; Eric Dumazet
> <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni
> <[email protected]>; Rob Herring <[email protected]>; Krzysztof
> Kozlowski <[email protected]>; Conor Dooley
> <[email protected]>; Giuseppe Cavallaro <[email protected]>;
> [email protected]; linux-arm-
> [email protected]; [email protected];
> [email protected]; [email protected]; Teoh Ji Sheng
> <[email protected]>
> Subject: Re: [PATCH net-next v2 3/4] net: stmmac: Add support for TX/RX
> channel interrupt
>
> On Fri, Jan 05, 2024 at 03:09:24PM +0800, Leong Ching Swee wrote:
> > From: Swee Leong Ching <[email protected]>
> >
> > Enable TX/RX channel interrupt registration for MAC that interrupts
> > CPU through shared peripheral interrupt (SPI).
> >
> > Per channel interrupts and interrupt-names are registered through,
> > Eg: 4 tx and 4 rx channels:
> > interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
> > <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>,
> > <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
> > <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
> > <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
> > <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
> > <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
> > <GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>; interrupt-names =
> > "dma_tx0",
> > "dma_tx1",
> > "dma_tx2",
> > "dma_tx3",
> > "dma_rx0",
> > "dma_rx1",
> > "dma_rx2",
> > "dma_rx3";
> >
> > Signed-off-by: Teoh Ji Sheng <[email protected]>
> > Signed-off-by: Swee Leong Ching <[email protected]>
> > ---
> > .../ethernet/stmicro/stmmac/stmmac_platform.c | 28
> > +++++++++++++++++++
> > 1 file changed, 28 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > index 70eadc83ca68..ae6859153e98 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > @@ -710,6 +710,10 @@
> EXPORT_SYMBOL_GPL(devm_stmmac_probe_config_dt);
> > int stmmac_get_platform_resources(struct platform_device *pdev,
> > struct stmmac_resources *stmmac_res) {
>
> > + char irq_name[9];
> > + int i;
> > + int irq;
> > +
>
> Reverse xmas tree please. Also what the point in having "i" and "irq"
> defined separately? Wouldn't it be better to merge them into a single
> statement:
> + char irq_name[9];
> + int i, irq;
>
Will rework this in v3.
> > memset(stmmac_res, 0, sizeof(*stmmac_res));
> >
> > /* Get IRQ information early to have an ability to ask for deferred
> > @@ -743,6 +747,30 @@ int stmmac_get_platform_resources(struct
> platform_device *pdev,
> > dev_info(&pdev->dev, "IRQ eth_lpi not found\n");
> > }
> >
>
> > + /* For RX Channel */
>
> Why haven't you added a more descriptive comment as I suggested on v1:
>
> + /* Get optional Tx/Rx DMA per-channel IRQs, which otherwise
> + * are supposed to be delivered via the common MAC IRQ line
> + */
>
> ?
>
Sorry I missed this, will rework this on v3.
> > + for (i = 0; i < MTL_MAX_RX_QUEUES; i++) {
> > + snprintf(irq_name, sizeof(irq_name), "dma_rx%i", i);
> > + irq = platform_get_irq_byname_optional(pdev, irq_name);
> > + if (irq == -EPROBE_DEFER)
> > + return irq;
> > + else if (irq < 0)
> > + break;
> > +
> > + stmmac_res->rx_irq[i] = irq;
> > + }
> > +
>
> > + /* For TX Channel */
>
> * see the comment above
>
> -Serge(y)
>
> > + for (i = 0; i < MTL_MAX_TX_QUEUES; i++) {
> > + snprintf(irq_name, sizeof(irq_name), "dma_tx%i", i);
> > + irq = platform_get_irq_byname_optional(pdev, irq_name);
> > + if (irq == -EPROBE_DEFER)
> > + return irq;
> > + else if (irq < 0)
> > + break;
> > +
> > + stmmac_res->tx_irq[i] = irq;
> > + }
> > +
> > stmmac_res->addr = devm_platform_ioremap_resource(pdev, 0);
> >
> > return PTR_ERR_OR_ZERO(stmmac_res->addr);
> > --
> > 2.34.1
> >
> >

2024-01-10 05:52:05

by Swee, Leong Ching

[permalink] [raw]
Subject: RE: [PATCH net-next v2 2/4] net: stmmac: Make MSI interrupt routine generic

> -----Original Message-----
> From: Serge Semin <[email protected]>
> Sent: Monday, January 8, 2024 4:28 AM
> To: Swee, Leong Ching <[email protected]>
> Cc: Maxime Coquelin <[email protected]>; Alexandre Torgue
> <[email protected]>; Jose Abreu <[email protected]>;
> David S . Miller <[email protected]>; Eric Dumazet
> <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni
> <[email protected]>; Rob Herring <[email protected]>; Krzysztof
> Kozlowski <[email protected]>; Conor Dooley
> <[email protected]>; Giuseppe Cavallaro <[email protected]>;
> [email protected]; linux-arm-
> [email protected]; [email protected];
> [email protected]; [email protected]; Teoh Ji Sheng
> <[email protected]>
> Subject: Re: [PATCH net-next v2 2/4] net: stmmac: Make MSI interrupt
> routine generic
>
> On Fri, Jan 05, 2024 at 03:09:23PM +0800, Leong Ching Swee wrote:
> > From: Swee Leong Ching <[email protected]>
> >
> > There is no support for per DMA channel interrupt for non-MSI
> > platform, where the MAC's per channel interrupt hooks up to interrupt
> > controller(GIC) through shared peripheral interrupt(SPI) to handle
> > interrupt from TX/RX transmit channel.
> >
> > This patch generalize the existing MSI ISR to also support non-MSI
> > platform.
>
> Basically this patch just fixes the individual IRQ handling code names.
>
Will change the commit log to below, please check if it sounds ok?
net: stmmac: Fixes individual IRQ handling code names

Individual IRQ can also be used for non-MSI platform,
today some of the code name for individual IRQ has
msi naming, so change msi naming to irq to make it common
for both platforms.

> >
> > Signed-off-by: Teoh Ji Sheng <[email protected]>
> > Signed-off-by: Swee Leong Ching <[email protected]>
> > ---
> > .../net/ethernet/stmicro/stmmac/dwmac-intel.c | 4 +--
> > .../ethernet/stmicro/stmmac/dwmac-socfpga.c | 3 ++
> > .../net/ethernet/stmicro/stmmac/dwmac4_dma.c | 2 +-
> > .../net/ethernet/stmicro/stmmac/stmmac_main.c | 30 +++++++++----------
> > include/linux/stmmac.h | 4 +--
> > 5 files changed, 23 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> > b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> > index 60283543ffc8..f0ec69af96c9 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> > @@ -952,7 +952,7 @@ static int stmmac_config_single_msi(struct pci_dev
> > *pdev,
> >
> > res->irq = pci_irq_vector(pdev, 0);
> > res->wol_irq = res->irq;
> > - plat->flags &= ~STMMAC_FLAG_MULTI_MSI_EN;
> > + plat->flags &= ~STMMAC_FLAG_MULTI_IRQ_EN;
> > dev_info(&pdev->dev, "%s: Single IRQ enablement successful\n",
> > __func__);
> >
> > @@ -1004,7 +1004,7 @@ static int stmmac_config_multi_msi(struct
> pci_dev *pdev,
> > if (plat->msi_sfty_ue_vec < STMMAC_MSI_VEC_MAX)
> > res->sfty_ue_irq = pci_irq_vector(pdev, plat-
> >msi_sfty_ue_vec);
> >
> > - plat->flags |= STMMAC_FLAG_MULTI_MSI_EN;
> > + plat->flags |= STMMAC_FLAG_MULTI_IRQ_EN;
> > dev_info(&pdev->dev, "%s: multi MSI enablement successful\n",
> > __func__);
> >
> > return 0;
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> > b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> > index ba2ce776bd4d..cf43fb3c6cc5 100644
>
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> > @@ -427,6 +427,9 @@ static int socfpga_dwmac_probe(struct
> platform_device *pdev)
> > plat_dat->bsp_priv = dwmac;
> > plat_dat->fix_mac_speed = socfpga_dwmac_fix_mac_speed;
> >
> > + if (stmmac_res.rx_irq[0] > 0 && stmmac_res.tx_irq[0] > 0)
> > + plat_dat->flags |= STMMAC_FLAG_MULTI_IRQ_EN;
> > +
> > ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
> > if (ret)
> > return ret;
>
> This is unrelated change. It adds the individual DMA IRQs support for the SoC
> FPGA platform, which AFAICS doesn't have it supported at the moment.
> Please move this into a separate patch with the commit log describing the
> change.
>
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> > b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> > index 84d3a8551b03..5f649106ffcd 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> > @@ -175,7 +175,7 @@ static void dwmac4_dma_init(void __iomem
> *ioaddr,
> >
> > value = readl(ioaddr + DMA_BUS_MODE);
> >
> > - if (dma_cfg->multi_msi_en) {
> > + if (dma_cfg->multi_irq_en) {
> > value &= ~DMA_BUS_MODE_INTM_MASK;
> > value |= (DMA_BUS_MODE_INTM_MODE1 <<
> DMA_BUS_MODE_INTM_SHIFT);
> > }
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index 47de466e432c..57873b879b33 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -129,8 +129,8 @@ static irqreturn_t stmmac_interrupt(int irq, void
> > *dev_id);
> > /* For MSI interrupts handling */
> > static irqreturn_t stmmac_mac_interrupt(int irq, void *dev_id);
> > static irqreturn_t stmmac_safety_interrupt(int irq, void *dev_id);
> > -static irqreturn_t stmmac_msi_intr_tx(int irq, void *data); -static
> > irqreturn_t stmmac_msi_intr_rx(int irq, void *data);
> > +static irqreturn_t stmmac_dma_tx_interrupt(int irq, void *data);
> > +static irqreturn_t stmmac_dma_rx_interrupt(int irq, void *data);
> > static void stmmac_reset_rx_queue(struct stmmac_priv *priv, u32
> > queue); static void stmmac_reset_tx_queue(struct stmmac_priv *priv,
> > u32 queue); static void stmmac_reset_queues_param(struct stmmac_priv
> > *priv); @@ -3602,7 +3602,7 @@ static void stmmac_free_irq(struct
> net_device *dev,
> > }
> > }
> >
> > -static int stmmac_request_irq_multi_msi(struct net_device *dev)
> > +static int stmmac_request_irq_multi(struct net_device *dev)
> > {
> > struct stmmac_priv *priv = netdev_priv(dev);
> > enum request_irq_err irq_err;
> > @@ -3697,7 +3697,7 @@ static int stmmac_request_irq_multi_msi(struct
> net_device *dev)
> > }
> > }
> >
> > - /* Request Rx MSI irq */
>
> > + /* Request Rx irq */
>
> s/irq/IRQ
> (capitalize)
Sure, rework on v3.
>
> > for (i = 0; i < priv->plat->rx_queues_to_use; i++) {
> > if (i >= MTL_MAX_RX_QUEUES)
> > break;
> > @@ -3707,11 +3707,11 @@ static int stmmac_request_irq_multi_msi(struct
> net_device *dev)
> > int_name = priv->int_name_rx_irq[i];
> > sprintf(int_name, "%s:%s-%d", dev->name, "rx", i);
> > ret = request_irq(priv->rx_irq[i],
> > - stmmac_msi_intr_rx,
> > + stmmac_dma_rx_interrupt,
> > 0, int_name, &priv-
> >dma_conf.rx_queue[i]);
> > if (unlikely(ret < 0)) {
> > netdev_err(priv->dev,
> > - "%s: alloc rx-%d MSI %d (error: %d)\n",
>
> > + "%s: alloc rx-%d dma rx_irq %d (error:
> %d)\n",
>
> s/ dma/DMA
> (capitalize and drop extra space)
>
Thanks, rework on v3.
> > __func__, i, priv->rx_irq[i], ret);
> > irq_err = REQ_IRQ_ERR_RX;
> > irq_idx = i;
> > @@ -3722,7 +3722,7 @@ static int stmmac_request_irq_multi_msi(struct
> net_device *dev)
> > irq_set_affinity_hint(priv->rx_irq[i], &cpu_mask);
> > }
> >
> > - /* Request Tx MSI irq */
>
> > + /* Request Tx irq */
>
> s/irq/IRQ
>
rework on v3.
> > for (i = 0; i < priv->plat->tx_queues_to_use; i++) {
> > if (i >= MTL_MAX_TX_QUEUES)
> > break;
> > @@ -3732,11 +3732,11 @@ static int stmmac_request_irq_multi_msi(struct
> net_device *dev)
> > int_name = priv->int_name_tx_irq[i];
> > sprintf(int_name, "%s:%s-%d", dev->name, "tx", i);
> > ret = request_irq(priv->tx_irq[i],
> > - stmmac_msi_intr_tx,
> > + stmmac_dma_tx_interrupt,
> > 0, int_name, &priv-
> >dma_conf.tx_queue[i]);
> > if (unlikely(ret < 0)) {
> > netdev_err(priv->dev,
> > - "%s: alloc tx-%d MSI %d (error: %d)\n",
>
> > + "%s: alloc tx-%d dma tx_irq %d (error:
> %d)\n",
>
> s/ dma/DMA
>
> -Serge(y)
>
rework on v3.
> > __func__, i, priv->tx_irq[i], ret);
> > irq_err = REQ_IRQ_ERR_TX;
> > irq_idx = i;
> > @@ -3811,8 +3811,8 @@ static int stmmac_request_irq(struct net_device
> *dev)
> > int ret;
> >
> > /* Request the IRQ lines */
> > - if (priv->plat->flags & STMMAC_FLAG_MULTI_MSI_EN)
> > - ret = stmmac_request_irq_multi_msi(dev);
> > + if (priv->plat->flags & STMMAC_FLAG_MULTI_IRQ_EN)
> > + ret = stmmac_request_irq_multi(dev);
> > else
> > ret = stmmac_request_irq_single(dev);
> >
> > @@ -6075,7 +6075,7 @@ static irqreturn_t stmmac_safety_interrupt(int
> irq, void *dev_id)
> > return IRQ_HANDLED;
> > }
> >
> > -static irqreturn_t stmmac_msi_intr_tx(int irq, void *data)
> > +static irqreturn_t stmmac_dma_tx_interrupt(int irq, void *data)
> > {
> > struct stmmac_tx_queue *tx_q = (struct stmmac_tx_queue *)data;
> > struct stmmac_dma_conf *dma_conf;
> > @@ -6107,7 +6107,7 @@ static irqreturn_t stmmac_msi_intr_tx(int irq,
> void *data)
> > return IRQ_HANDLED;
> > }
> >
> > -static irqreturn_t stmmac_msi_intr_rx(int irq, void *data)
> > +static irqreturn_t stmmac_dma_rx_interrupt(int irq, void *data)
> > {
> > struct stmmac_rx_queue *rx_q = (struct stmmac_rx_queue *)data;
> > struct stmmac_dma_conf *dma_conf;
> > @@ -7456,8 +7456,8 @@ int stmmac_dvr_probe(struct device *device,
> > priv->plat = plat_dat;
> > priv->ioaddr = res->addr;
> > priv->dev->base_addr = (unsigned long)res->addr;
> > - priv->plat->dma_cfg->multi_msi_en =
> > - (priv->plat->flags & STMMAC_FLAG_MULTI_MSI_EN);
> > + priv->plat->dma_cfg->multi_irq_en =
> > + (priv->plat->flags & STMMAC_FLAG_MULTI_IRQ_EN);
> >
> > priv->dev->irq = res->irq;
> > priv->wol_irq = res->wol_irq;
> > diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h index
> > dee5ad6e48c5..b950e6f9761d 100644
> > --- a/include/linux/stmmac.h
> > +++ b/include/linux/stmmac.h
> > @@ -98,7 +98,7 @@ struct stmmac_dma_cfg {
> > int mixed_burst;
> > bool aal;
> > bool eame;
> > - bool multi_msi_en;
> > + bool multi_irq_en;
> > bool dche;
> > };
> >
> > @@ -215,7 +215,7 @@ struct dwmac4_addrs {
> > #define STMMAC_FLAG_TSO_EN BIT(4)
> > #define STMMAC_FLAG_SERDES_UP_AFTER_PHY_LINKUP BIT(5)
> > #define STMMAC_FLAG_VLAN_FAIL_Q_EN BIT(6)
> > -#define STMMAC_FLAG_MULTI_MSI_EN BIT(7)
> > +#define STMMAC_FLAG_MULTI_IRQ_EN BIT(7)
> > #define STMMAC_FLAG_EXT_SNAPSHOT_EN BIT(8)
> > #define STMMAC_FLAG_INT_SNAPSHOT_EN BIT(9)
> > #define STMMAC_FLAG_RX_CLK_RUNS_IN_LPI BIT(10)
> > --
> > 2.34.1
> >
> >

2024-01-24 15:36:56

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH net-next v2 2/4] net: stmmac: Make MSI interrupt routine generic

On Wed, Jan 10, 2024 at 05:51:37AM +0000, Swee, Leong Ching wrote:
> > -----Original Message-----
> > From: Serge Semin <[email protected]>
> > Sent: Monday, January 8, 2024 4:28 AM
> > To: Swee, Leong Ching <[email protected]>
> > Cc: Maxime Coquelin <[email protected]>; Alexandre Torgue
> > <[email protected]>; Jose Abreu <[email protected]>;
> > David S . Miller <[email protected]>; Eric Dumazet
> > <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni
> > <[email protected]>; Rob Herring <[email protected]>; Krzysztof
> > Kozlowski <[email protected]>; Conor Dooley
> > <[email protected]>; Giuseppe Cavallaro <[email protected]>;
> > [email protected]; linux-arm-
> > [email protected]; [email protected];
> > [email protected]; [email protected]; Teoh Ji Sheng
> > <[email protected]>
> > Subject: Re: [PATCH net-next v2 2/4] net: stmmac: Make MSI interrupt
> > routine generic
> >
> > On Fri, Jan 05, 2024 at 03:09:23PM +0800, Leong Ching Swee wrote:
> > > From: Swee Leong Ching <[email protected]>
> > >
> > > There is no support for per DMA channel interrupt for non-MSI
> > > platform, where the MAC's per channel interrupt hooks up to interrupt
> > > controller(GIC) through shared peripheral interrupt(SPI) to handle
> > > interrupt from TX/RX transmit channel.
> > >
> > > This patch generalize the existing MSI ISR to also support non-MSI
> > > platform.
> >
> > Basically this patch just fixes the individual IRQ handling code names.
> >

> Will change the commit log to below, please check if it sounds ok?
>
> net: stmmac: Fixes individual IRQ handling code names
>
> Individual IRQ can also be used for non-MSI platform,
> today some of the code name for individual IRQ has
> msi naming, so change msi naming to irq to make it common
> for both platforms.

Much better but IMO the next wording would be a bit more descriptive:

net: stmmac: Generalize individual IRQs handling code names

The individual IRQs can be also available on the non-MSI platforms.
The respective code has been developed with the MSI-based platform in
mind thus having the "MSI" word in implementation entities. Drop such
wording or replace it with just "IRQ" where it's relevant in order to
generalize the individual IRQs handling code.

-Serge(y)

>
> > >
> > > Signed-off-by: Teoh Ji Sheng <[email protected]>
> > > Signed-off-by: Swee Leong Ching <[email protected]>
> > > ---
> > > .../net/ethernet/stmicro/stmmac/dwmac-intel.c | 4 +--
> > > .../ethernet/stmicro/stmmac/dwmac-socfpga.c | 3 ++
> > > .../net/ethernet/stmicro/stmmac/dwmac4_dma.c | 2 +-
> > > .../net/ethernet/stmicro/stmmac/stmmac_main.c | 30 +++++++++----------
> > > include/linux/stmmac.h | 4 +--
> > > 5 files changed, 23 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> > > b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> > > index 60283543ffc8..f0ec69af96c9 100644
> > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> > > @@ -952,7 +952,7 @@ static int stmmac_config_single_msi(struct pci_dev
> > > *pdev,
> > >
> > > res->irq = pci_irq_vector(pdev, 0);
> > > res->wol_irq = res->irq;
> > > - plat->flags &= ~STMMAC_FLAG_MULTI_MSI_EN;
> > > + plat->flags &= ~STMMAC_FLAG_MULTI_IRQ_EN;
> > > dev_info(&pdev->dev, "%s: Single IRQ enablement successful\n",
> > > __func__);
> > >
> > > @@ -1004,7 +1004,7 @@ static int stmmac_config_multi_msi(struct
> > pci_dev *pdev,
> > > if (plat->msi_sfty_ue_vec < STMMAC_MSI_VEC_MAX)
> > > res->sfty_ue_irq = pci_irq_vector(pdev, plat-
> > >msi_sfty_ue_vec);
> > >
> > > - plat->flags |= STMMAC_FLAG_MULTI_MSI_EN;
> > > + plat->flags |= STMMAC_FLAG_MULTI_IRQ_EN;
> > > dev_info(&pdev->dev, "%s: multi MSI enablement successful\n",
> > > __func__);
> > >
> > > return 0;
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> > > b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> > > index ba2ce776bd4d..cf43fb3c6cc5 100644
> >
> > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> > > @@ -427,6 +427,9 @@ static int socfpga_dwmac_probe(struct
> > platform_device *pdev)
> > > plat_dat->bsp_priv = dwmac;
> > > plat_dat->fix_mac_speed = socfpga_dwmac_fix_mac_speed;
> > >
> > > + if (stmmac_res.rx_irq[0] > 0 && stmmac_res.tx_irq[0] > 0)
> > > + plat_dat->flags |= STMMAC_FLAG_MULTI_IRQ_EN;
> > > +
> > > ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
> > > if (ret)
> > > return ret;
> >
> > This is unrelated change. It adds the individual DMA IRQs support for the SoC
> > FPGA platform, which AFAICS doesn't have it supported at the moment.
> > Please move this into a separate patch with the commit log describing the
> > change.
> >
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> > > b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> > > index 84d3a8551b03..5f649106ffcd 100644
> > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> > > @@ -175,7 +175,7 @@ static void dwmac4_dma_init(void __iomem
> > *ioaddr,
> > >
> > > value = readl(ioaddr + DMA_BUS_MODE);
> > >
> > > - if (dma_cfg->multi_msi_en) {
> > > + if (dma_cfg->multi_irq_en) {
> > > value &= ~DMA_BUS_MODE_INTM_MASK;
> > > value |= (DMA_BUS_MODE_INTM_MODE1 <<
> > DMA_BUS_MODE_INTM_SHIFT);
> > > }
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > index 47de466e432c..57873b879b33 100644
> > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > @@ -129,8 +129,8 @@ static irqreturn_t stmmac_interrupt(int irq, void
> > > *dev_id);
> > > /* For MSI interrupts handling */
> > > static irqreturn_t stmmac_mac_interrupt(int irq, void *dev_id);
> > > static irqreturn_t stmmac_safety_interrupt(int irq, void *dev_id);
> > > -static irqreturn_t stmmac_msi_intr_tx(int irq, void *data); -static
> > > irqreturn_t stmmac_msi_intr_rx(int irq, void *data);
> > > +static irqreturn_t stmmac_dma_tx_interrupt(int irq, void *data);
> > > +static irqreturn_t stmmac_dma_rx_interrupt(int irq, void *data);
> > > static void stmmac_reset_rx_queue(struct stmmac_priv *priv, u32
> > > queue); static void stmmac_reset_tx_queue(struct stmmac_priv *priv,
> > > u32 queue); static void stmmac_reset_queues_param(struct stmmac_priv
> > > *priv); @@ -3602,7 +3602,7 @@ static void stmmac_free_irq(struct
> > net_device *dev,
> > > }
> > > }
> > >
> > > -static int stmmac_request_irq_multi_msi(struct net_device *dev)
> > > +static int stmmac_request_irq_multi(struct net_device *dev)
> > > {
> > > struct stmmac_priv *priv = netdev_priv(dev);
> > > enum request_irq_err irq_err;
> > > @@ -3697,7 +3697,7 @@ static int stmmac_request_irq_multi_msi(struct
> > net_device *dev)
> > > }
> > > }
> > >
> > > - /* Request Rx MSI irq */
> >
> > > + /* Request Rx irq */
> >
> > s/irq/IRQ
> > (capitalize)
> Sure, rework on v3.
> >
> > > for (i = 0; i < priv->plat->rx_queues_to_use; i++) {
> > > if (i >= MTL_MAX_RX_QUEUES)
> > > break;
> > > @@ -3707,11 +3707,11 @@ static int stmmac_request_irq_multi_msi(struct
> > net_device *dev)
> > > int_name = priv->int_name_rx_irq[i];
> > > sprintf(int_name, "%s:%s-%d", dev->name, "rx", i);
> > > ret = request_irq(priv->rx_irq[i],
> > > - stmmac_msi_intr_rx,
> > > + stmmac_dma_rx_interrupt,
> > > 0, int_name, &priv-
> > >dma_conf.rx_queue[i]);
> > > if (unlikely(ret < 0)) {
> > > netdev_err(priv->dev,
> > > - "%s: alloc rx-%d MSI %d (error: %d)\n",
> >
> > > + "%s: alloc rx-%d dma rx_irq %d (error:
> > %d)\n",
> >
> > s/ dma/DMA
> > (capitalize and drop extra space)
> >
> Thanks, rework on v3.
> > > __func__, i, priv->rx_irq[i], ret);
> > > irq_err = REQ_IRQ_ERR_RX;
> > > irq_idx = i;
> > > @@ -3722,7 +3722,7 @@ static int stmmac_request_irq_multi_msi(struct
> > net_device *dev)
> > > irq_set_affinity_hint(priv->rx_irq[i], &cpu_mask);
> > > }
> > >
> > > - /* Request Tx MSI irq */
> >
> > > + /* Request Tx irq */
> >
> > s/irq/IRQ
> >
> rework on v3.
> > > for (i = 0; i < priv->plat->tx_queues_to_use; i++) {
> > > if (i >= MTL_MAX_TX_QUEUES)
> > > break;
> > > @@ -3732,11 +3732,11 @@ static int stmmac_request_irq_multi_msi(struct
> > net_device *dev)
> > > int_name = priv->int_name_tx_irq[i];
> > > sprintf(int_name, "%s:%s-%d", dev->name, "tx", i);
> > > ret = request_irq(priv->tx_irq[i],
> > > - stmmac_msi_intr_tx,
> > > + stmmac_dma_tx_interrupt,
> > > 0, int_name, &priv-
> > >dma_conf.tx_queue[i]);
> > > if (unlikely(ret < 0)) {
> > > netdev_err(priv->dev,
> > > - "%s: alloc tx-%d MSI %d (error: %d)\n",
> >
> > > + "%s: alloc tx-%d dma tx_irq %d (error:
> > %d)\n",
> >
> > s/ dma/DMA
> >
> > -Serge(y)
> >
> rework on v3.
> > > __func__, i, priv->tx_irq[i], ret);
> > > irq_err = REQ_IRQ_ERR_TX;
> > > irq_idx = i;
> > > @@ -3811,8 +3811,8 @@ static int stmmac_request_irq(struct net_device
> > *dev)
> > > int ret;
> > >
> > > /* Request the IRQ lines */
> > > - if (priv->plat->flags & STMMAC_FLAG_MULTI_MSI_EN)
> > > - ret = stmmac_request_irq_multi_msi(dev);
> > > + if (priv->plat->flags & STMMAC_FLAG_MULTI_IRQ_EN)
> > > + ret = stmmac_request_irq_multi(dev);
> > > else
> > > ret = stmmac_request_irq_single(dev);
> > >
> > > @@ -6075,7 +6075,7 @@ static irqreturn_t stmmac_safety_interrupt(int
> > irq, void *dev_id)
> > > return IRQ_HANDLED;
> > > }
> > >
> > > -static irqreturn_t stmmac_msi_intr_tx(int irq, void *data)
> > > +static irqreturn_t stmmac_dma_tx_interrupt(int irq, void *data)
> > > {
> > > struct stmmac_tx_queue *tx_q = (struct stmmac_tx_queue *)data;
> > > struct stmmac_dma_conf *dma_conf;
> > > @@ -6107,7 +6107,7 @@ static irqreturn_t stmmac_msi_intr_tx(int irq,
> > void *data)
> > > return IRQ_HANDLED;
> > > }
> > >
> > > -static irqreturn_t stmmac_msi_intr_rx(int irq, void *data)
> > > +static irqreturn_t stmmac_dma_rx_interrupt(int irq, void *data)
> > > {
> > > struct stmmac_rx_queue *rx_q = (struct stmmac_rx_queue *)data;
> > > struct stmmac_dma_conf *dma_conf;
> > > @@ -7456,8 +7456,8 @@ int stmmac_dvr_probe(struct device *device,
> > > priv->plat = plat_dat;
> > > priv->ioaddr = res->addr;
> > > priv->dev->base_addr = (unsigned long)res->addr;
> > > - priv->plat->dma_cfg->multi_msi_en =
> > > - (priv->plat->flags & STMMAC_FLAG_MULTI_MSI_EN);
> > > + priv->plat->dma_cfg->multi_irq_en =
> > > + (priv->plat->flags & STMMAC_FLAG_MULTI_IRQ_EN);
> > >
> > > priv->dev->irq = res->irq;
> > > priv->wol_irq = res->wol_irq;
> > > diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h index
> > > dee5ad6e48c5..b950e6f9761d 100644
> > > --- a/include/linux/stmmac.h
> > > +++ b/include/linux/stmmac.h
> > > @@ -98,7 +98,7 @@ struct stmmac_dma_cfg {
> > > int mixed_burst;
> > > bool aal;
> > > bool eame;
> > > - bool multi_msi_en;
> > > + bool multi_irq_en;
> > > bool dche;
> > > };
> > >
> > > @@ -215,7 +215,7 @@ struct dwmac4_addrs {
> > > #define STMMAC_FLAG_TSO_EN BIT(4)
> > > #define STMMAC_FLAG_SERDES_UP_AFTER_PHY_LINKUP BIT(5)
> > > #define STMMAC_FLAG_VLAN_FAIL_Q_EN BIT(6)
> > > -#define STMMAC_FLAG_MULTI_MSI_EN BIT(7)
> > > +#define STMMAC_FLAG_MULTI_IRQ_EN BIT(7)
> > > #define STMMAC_FLAG_EXT_SNAPSHOT_EN BIT(8)
> > > #define STMMAC_FLAG_INT_SNAPSHOT_EN BIT(9)
> > > #define STMMAC_FLAG_RX_CLK_RUNS_IN_LPI BIT(10)
> > > --
> > > 2.34.1
> > >
> > >