All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/4] net: stmmac: Enable Per DMA Channel interrupt
@ 2024-01-05  7:09 ` Leong Ching Swee
  0 siblings, 0 replies; 40+ messages in thread
From: Leong Ching Swee @ 2024-01-05  7:09 UTC (permalink / raw)
  To: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro
  Cc: linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Swee Leong Ching

From: Swee Leong Ching <leong.ching.swee@intel.com>

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-
robh@kernel.org/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


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH net-next v2 0/4] net: stmmac: Enable Per DMA Channel interrupt
@ 2024-01-05  7:09 ` Leong Ching Swee
  0 siblings, 0 replies; 40+ messages in thread
From: Leong Ching Swee @ 2024-01-05  7:09 UTC (permalink / raw)
  To: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro
  Cc: linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Swee Leong Ching

From: Swee Leong Ching <leong.ching.swee@intel.com>

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-
robh@kernel.org/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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH net-next v2 1/4] dt-bindings: net: snps,dwmac: per channel irq
  2024-01-05  7:09 ` Leong Ching Swee
@ 2024-01-05  7:09   ` Leong Ching Swee
  -1 siblings, 0 replies; 40+ messages in thread
From: Leong Ching Swee @ 2024-01-05  7:09 UTC (permalink / raw)
  To: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro
  Cc: linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Swee Leong Ching, Rohan G Thomas

From: Swee Leong Ching <leong.ching.swee@intel.com>

Add dt-bindings for per channel irq.

Signed-off-by: Rohan G Thomas <rohan.g.thomas@intel.com>
Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
---
 .../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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH net-next v2 1/4] dt-bindings: net: snps,dwmac: per channel irq
@ 2024-01-05  7:09   ` Leong Ching Swee
  0 siblings, 0 replies; 40+ messages in thread
From: Leong Ching Swee @ 2024-01-05  7:09 UTC (permalink / raw)
  To: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro
  Cc: linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Swee Leong Ching, Rohan G Thomas

From: Swee Leong Ching <leong.ching.swee@intel.com>

Add dt-bindings for per channel irq.

Signed-off-by: Rohan G Thomas <rohan.g.thomas@intel.com>
Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
---
 .../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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH net-next v2 2/4] net: stmmac: Make MSI interrupt routine generic
  2024-01-05  7:09 ` Leong Ching Swee
@ 2024-01-05  7:09   ` Leong Ching Swee
  -1 siblings, 0 replies; 40+ messages in thread
From: Leong Ching Swee @ 2024-01-05  7:09 UTC (permalink / raw)
  To: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro
  Cc: linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Swee Leong Ching, Teoh Ji Sheng

From: Swee Leong Ching <leong.ching.swee@intel.com>

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 <ji.sheng.teoh@intel.com>
Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
---
 .../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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH net-next v2 2/4] net: stmmac: Make MSI interrupt routine generic
@ 2024-01-05  7:09   ` Leong Ching Swee
  0 siblings, 0 replies; 40+ messages in thread
From: Leong Ching Swee @ 2024-01-05  7:09 UTC (permalink / raw)
  To: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro
  Cc: linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Swee Leong Ching, Teoh Ji Sheng

From: Swee Leong Ching <leong.ching.swee@intel.com>

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 <ji.sheng.teoh@intel.com>
Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
---
 .../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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH net-next v2 3/4] net: stmmac: Add support for TX/RX channel interrupt
  2024-01-05  7:09 ` Leong Ching Swee
@ 2024-01-05  7:09   ` Leong Ching Swee
  -1 siblings, 0 replies; 40+ messages in thread
From: Leong Ching Swee @ 2024-01-05  7:09 UTC (permalink / raw)
  To: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro
  Cc: linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Swee Leong Ching, Teoh Ji Sheng

From: Swee Leong Ching <leong.ching.swee@intel.com>

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 <ji.sheng.teoh@intel.com>
Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
---
 .../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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH net-next v2 3/4] net: stmmac: Add support for TX/RX channel interrupt
@ 2024-01-05  7:09   ` Leong Ching Swee
  0 siblings, 0 replies; 40+ messages in thread
From: Leong Ching Swee @ 2024-01-05  7:09 UTC (permalink / raw)
  To: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro
  Cc: linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Swee Leong Ching, Teoh Ji Sheng

From: Swee Leong Ching <leong.ching.swee@intel.com>

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 <ji.sheng.teoh@intel.com>
Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
---
 .../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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH net-next v2 4/4] net: stmmac: Use interrupt mode INTM=1 for per channel irq
  2024-01-05  7:09 ` Leong Ching Swee
@ 2024-01-05  7:09   ` Leong Ching Swee
  -1 siblings, 0 replies; 40+ messages in thread
From: Leong Ching Swee @ 2024-01-05  7:09 UTC (permalink / raw)
  To: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro
  Cc: linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Swee Leong Ching, Teoh Ji Sheng

From: Swee Leong Ching <leong.ching.swee@intel.com>

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 <ji.sheng.teoh@intel.com>
Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
---
 .../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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH net-next v2 4/4] net: stmmac: Use interrupt mode INTM=1 for per channel irq
@ 2024-01-05  7:09   ` Leong Ching Swee
  0 siblings, 0 replies; 40+ messages in thread
From: Leong Ching Swee @ 2024-01-05  7:09 UTC (permalink / raw)
  To: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro
  Cc: linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Swee Leong Ching, Teoh Ji Sheng

From: Swee Leong Ching <leong.ching.swee@intel.com>

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 <ji.sheng.teoh@intel.com>
Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
---
 .../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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [PATCH net-next v2 0/4] net: stmmac: Enable Per DMA Channel interrupt
  2024-01-05  7:09 ` Leong Ching Swee
@ 2024-01-07 16:40   ` patchwork-bot+netdevbpf
  -1 siblings, 0 replies; 40+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-01-07 16:40 UTC (permalink / raw)
  To: Leong Ching Swee
  Cc: mcoquelin.stm32, alexandre.torgue, joabreu, davem, edumazet,
	kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	peppe.cavallaro, linux-stm32, linux-arm-kernel, linux-kernel,
	netdev, devicetree

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Fri,  5 Jan 2024 15:09:21 +0800 you wrote:
> From: Swee Leong Ching <leong.ching.swee@intel.com>
> 
> 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-
> robh@kernel.org/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



^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH net-next v2 0/4] net: stmmac: Enable Per DMA Channel interrupt
@ 2024-01-07 16:40   ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 40+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-01-07 16:40 UTC (permalink / raw)
  To: Leong Ching Swee
  Cc: mcoquelin.stm32, alexandre.torgue, joabreu, davem, edumazet,
	kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	peppe.cavallaro, linux-stm32, linux-arm-kernel, linux-kernel,
	netdev, devicetree

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Fri,  5 Jan 2024 15:09:21 +0800 you wrote:
> From: Swee Leong Ching <leong.ching.swee@intel.com>
> 
> 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-
> robh@kernel.org/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



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH net-next v2 0/4] net: stmmac: Enable Per DMA Channel interrupt
  2024-01-07 16:40   ` patchwork-bot+netdevbpf
@ 2024-01-07 19:06     ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 40+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-07 19:06 UTC (permalink / raw)
  To: patchwork-bot+netdevbpf, Leong Ching Swee
  Cc: mcoquelin.stm32, alexandre.torgue, joabreu, davem, edumazet,
	kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	peppe.cavallaro, linux-stm32, linux-arm-kernel, linux-kernel,
	netdev, devicetree

On 07/01/2024 17:40, patchwork-bot+netdevbpf@kernel.org wrote:
> Hello:
> 
> This series was applied to netdev/net-next.git (main)
> by David S. Miller <davem@davemloft.net>:
> 
> On Fri,  5 Jan 2024 15:09:21 +0800 you wrote:
>> From: Swee Leong Ching <leong.ching.swee@intel.com>
>>
>> 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-
>> robh@kernel.org/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


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH net-next v2 0/4] net: stmmac: Enable Per DMA Channel interrupt
@ 2024-01-07 19:06     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 40+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-07 19:06 UTC (permalink / raw)
  To: patchwork-bot+netdevbpf, Leong Ching Swee
  Cc: mcoquelin.stm32, alexandre.torgue, joabreu, davem, edumazet,
	kuba, pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	peppe.cavallaro, linux-stm32, linux-arm-kernel, linux-kernel,
	netdev, devicetree

On 07/01/2024 17:40, patchwork-bot+netdevbpf@kernel.org wrote:
> Hello:
> 
> This series was applied to netdev/net-next.git (main)
> by David S. Miller <davem@davemloft.net>:
> 
> On Fri,  5 Jan 2024 15:09:21 +0800 you wrote:
>> From: Swee Leong Ching <leong.ching.swee@intel.com>
>>
>> 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-
>> robh@kernel.org/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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH net-next v2 1/4] dt-bindings: net: snps,dwmac: per channel irq
  2024-01-05  7:09   ` Leong Ching Swee
@ 2024-01-07 20:10     ` Serge Semin
  -1 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2024-01-07 20:10 UTC (permalink / raw)
  To: Leong Ching Swee, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Giuseppe Cavallaro,
	linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Rohan G Thomas

On Fri, Jan 05, 2024 at 03:09:22PM +0800, Leong Ching Swee wrote:
> From: Swee Leong Ching <leong.ching.swee@intel.com>
> 
> Add dt-bindings for per channel irq.
> 
> Signed-off-by: Rohan G Thomas <rohan.g.thomas@intel.com>
> Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
> ---
>  .../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
> 
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH net-next v2 1/4] dt-bindings: net: snps,dwmac: per channel irq
@ 2024-01-07 20:10     ` Serge Semin
  0 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2024-01-07 20:10 UTC (permalink / raw)
  To: Leong Ching Swee, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Giuseppe Cavallaro,
	linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Rohan G Thomas

On Fri, Jan 05, 2024 at 03:09:22PM +0800, Leong Ching Swee wrote:
> From: Swee Leong Ching <leong.ching.swee@intel.com>
> 
> Add dt-bindings for per channel irq.
> 
> Signed-off-by: Rohan G Thomas <rohan.g.thomas@intel.com>
> Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
> ---
>  .../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
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH net-next v2 2/4] net: stmmac: Make MSI interrupt routine generic
  2024-01-05  7:09   ` Leong Ching Swee
@ 2024-01-07 20:27     ` Serge Semin
  -1 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2024-01-07 20:27 UTC (permalink / raw)
  To: Leong Ching Swee
  Cc: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro,
	linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Teoh Ji Sheng

On Fri, Jan 05, 2024 at 03:09:23PM +0800, Leong Ching Swee wrote:
> From: Swee Leong Ching <leong.ching.swee@intel.com>
> 
> 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 <ji.sheng.teoh@intel.com>
> Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
> ---
>  .../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
> 
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH net-next v2 2/4] net: stmmac: Make MSI interrupt routine generic
@ 2024-01-07 20:27     ` Serge Semin
  0 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2024-01-07 20:27 UTC (permalink / raw)
  To: Leong Ching Swee
  Cc: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro,
	linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Teoh Ji Sheng

On Fri, Jan 05, 2024 at 03:09:23PM +0800, Leong Ching Swee wrote:
> From: Swee Leong Ching <leong.ching.swee@intel.com>
> 
> 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 <ji.sheng.teoh@intel.com>
> Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
> ---
>  .../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
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH net-next v2 3/4] net: stmmac: Add support for TX/RX channel interrupt
  2024-01-05  7:09   ` Leong Ching Swee
@ 2024-01-07 20:38     ` Serge Semin
  -1 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2024-01-07 20:38 UTC (permalink / raw)
  To: Leong Ching Swee
  Cc: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro,
	linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Teoh Ji Sheng

On Fri, Jan 05, 2024 at 03:09:24PM +0800, Leong Ching Swee wrote:
> From: Swee Leong Ching <leong.ching.swee@intel.com>
> 
> 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 <ji.sheng.teoh@intel.com>
> Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
> ---
>  .../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
> 
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH net-next v2 3/4] net: stmmac: Add support for TX/RX channel interrupt
@ 2024-01-07 20:38     ` Serge Semin
  0 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2024-01-07 20:38 UTC (permalink / raw)
  To: Leong Ching Swee
  Cc: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro,
	linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Teoh Ji Sheng

On Fri, Jan 05, 2024 at 03:09:24PM +0800, Leong Ching Swee wrote:
> From: Swee Leong Ching <leong.ching.swee@intel.com>
> 
> 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 <ji.sheng.teoh@intel.com>
> Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
> ---
>  .../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
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH net-next v2 4/4] net: stmmac: Use interrupt mode INTM=1 for per channel irq
  2024-01-05  7:09   ` Leong Ching Swee
@ 2024-01-07 20:52     ` Serge Semin
  -1 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2024-01-07 20:52 UTC (permalink / raw)
  To: Leong Ching Swee
  Cc: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro,
	linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Teoh Ji Sheng

On Fri, Jan 05, 2024 at 03:09:25PM +0800, Leong Ching Swee wrote:
> From: Swee Leong Ching <leong.ching.swee@intel.com>
> 
> 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 <ji.sheng.teoh@intel.com>
> Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
> ---
>  .../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
> 
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH net-next v2 4/4] net: stmmac: Use interrupt mode INTM=1 for per channel irq
@ 2024-01-07 20:52     ` Serge Semin
  0 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2024-01-07 20:52 UTC (permalink / raw)
  To: Leong Ching Swee
  Cc: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro,
	linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Teoh Ji Sheng

On Fri, Jan 05, 2024 at 03:09:25PM +0800, Leong Ching Swee wrote:
> From: Swee Leong Ching <leong.ching.swee@intel.com>
> 
> 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 <ji.sheng.teoh@intel.com>
> Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
> ---
>  .../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
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH net-next v2 0/4] net: stmmac: Enable Per DMA Channel interrupt
  2024-01-07 19:06     ` Krzysztof Kozlowski
@ 2024-01-07 21:24       ` Serge Semin
  -1 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2024-01-07 21:24 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Leong Ching Swee, David S. Miller
  Cc: patchwork-bot+netdevbpf, mcoquelin.stm32, alexandre.torgue,
	joabreu, davem, edumazet, kuba, pabeni, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, peppe.cavallaro, linux-stm32,
	linux-arm-kernel, linux-kernel, netdev, devicetree

On Sun, Jan 07, 2024 at 08:06:55PM +0100, Krzysztof Kozlowski wrote:
> On 07/01/2024 17:40, patchwork-bot+netdevbpf@kernel.org wrote:
> > Hello:
> > 
> > This series was applied to netdev/net-next.git (main)
> > by David S. Miller <davem@davemloft.net>:
> > 
> > On Fri,  5 Jan 2024 15:09:21 +0800 you wrote:
> >> From: Swee Leong Ching <leong.ching.swee@intel.com>
> >>
> >> 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-
> >> robh@kernel.org/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
> 
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH net-next v2 0/4] net: stmmac: Enable Per DMA Channel interrupt
@ 2024-01-07 21:24       ` Serge Semin
  0 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2024-01-07 21:24 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Leong Ching Swee, David S. Miller
  Cc: patchwork-bot+netdevbpf, mcoquelin.stm32, alexandre.torgue,
	joabreu, davem, edumazet, kuba, pabeni, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, peppe.cavallaro, linux-stm32,
	linux-arm-kernel, linux-kernel, netdev, devicetree

On Sun, Jan 07, 2024 at 08:06:55PM +0100, Krzysztof Kozlowski wrote:
> On 07/01/2024 17:40, patchwork-bot+netdevbpf@kernel.org wrote:
> > Hello:
> > 
> > This series was applied to netdev/net-next.git (main)
> > by David S. Miller <davem@davemloft.net>:
> > 
> > On Fri,  5 Jan 2024 15:09:21 +0800 you wrote:
> >> From: Swee Leong Ching <leong.ching.swee@intel.com>
> >>
> >> 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-
> >> robh@kernel.org/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
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH net-next v2 0/4] net: stmmac: Enable Per DMA Channel interrupt
  2024-01-07 21:24       ` Serge Semin
@ 2024-01-08  1:02         ` Jakub Kicinski
  -1 siblings, 0 replies; 40+ messages in thread
From: Jakub Kicinski @ 2024-01-08  1:02 UTC (permalink / raw)
  To: Serge Semin, Krzysztof Kozlowski
  Cc: Leong Ching Swee, David S. Miller, patchwork-bot+netdevbpf,
	mcoquelin.stm32, alexandre.torgue, joabreu, edumazet, pabeni,
	robh+dt, krzysztof.kozlowski+dt, conor+dt, peppe.cavallaro,
	linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree

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.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH net-next v2 0/4] net: stmmac: Enable Per DMA Channel interrupt
@ 2024-01-08  1:02         ` Jakub Kicinski
  0 siblings, 0 replies; 40+ messages in thread
From: Jakub Kicinski @ 2024-01-08  1:02 UTC (permalink / raw)
  To: Serge Semin, Krzysztof Kozlowski
  Cc: Leong Ching Swee, David S. Miller, patchwork-bot+netdevbpf,
	mcoquelin.stm32, alexandre.torgue, joabreu, edumazet, pabeni,
	robh+dt, krzysztof.kozlowski+dt, conor+dt, peppe.cavallaro,
	linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree

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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* RE: [PATCH net-next v2 0/4] net: stmmac: Enable Per DMA Channel interrupt
  2024-01-07 21:24       ` Serge Semin
@ 2024-01-09  8:05         ` Swee, Leong Ching
  -1 siblings, 0 replies; 40+ messages in thread
From: Swee, Leong Ching @ 2024-01-09  8:05 UTC (permalink / raw)
  To: Serge Semin, Krzysztof Kozlowski, David S. Miller
  Cc: patchwork-bot+netdevbpf, mcoquelin.stm32, alexandre.torgue,
	joabreu, davem, edumazet, kuba, pabeni, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, peppe.cavallaro, linux-stm32,
	linux-arm-kernel, linux-kernel, netdev, devicetree


> -----Original Message-----
> From: Serge Semin <fancer.lancer@gmail.com>
> Sent: Monday, January 8, 2024 5:24 AM
> To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>; Swee, Leong
> Ching <leong.ching.swee@intel.com>; David S. Miller
> <davem@davemloft.net>
> Cc: patchwork-bot+netdevbpf@kernel.org; mcoquelin.stm32@gmail.com;
> alexandre.torgue@foss.st.com; joabreu@synopsys.com;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org;
> peppe.cavallaro@st.com; linux-stm32@st-md-mailman.stormreply.com;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> netdev@vger.kernel.org; devicetree@vger.kernel.org
> 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, patchwork-bot+netdevbpf@kernel.org wrote:
> > > Hello:
> > >
> > > This series was applied to netdev/net-next.git (main) by David S.
> > > Miller <davem@davemloft.net>:
> > >
> > > On Fri,  5 Jan 2024 15:09:21 +0800 you wrote:
> > >> From: Swee Leong Ching <leong.ching.swee@intel.com>
> > >>
> > >> 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-
> > >> robh@kernel.org/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
> >
> >

^ permalink raw reply	[flat|nested] 40+ messages in thread

* RE: [PATCH net-next v2 0/4] net: stmmac: Enable Per DMA Channel interrupt
@ 2024-01-09  8:05         ` Swee, Leong Ching
  0 siblings, 0 replies; 40+ messages in thread
From: Swee, Leong Ching @ 2024-01-09  8:05 UTC (permalink / raw)
  To: Serge Semin, Krzysztof Kozlowski, David S. Miller
  Cc: patchwork-bot+netdevbpf, mcoquelin.stm32, alexandre.torgue,
	joabreu, davem, edumazet, kuba, pabeni, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, peppe.cavallaro, linux-stm32,
	linux-arm-kernel, linux-kernel, netdev, devicetree


> -----Original Message-----
> From: Serge Semin <fancer.lancer@gmail.com>
> Sent: Monday, January 8, 2024 5:24 AM
> To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>; Swee, Leong
> Ching <leong.ching.swee@intel.com>; David S. Miller
> <davem@davemloft.net>
> Cc: patchwork-bot+netdevbpf@kernel.org; mcoquelin.stm32@gmail.com;
> alexandre.torgue@foss.st.com; joabreu@synopsys.com;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org;
> peppe.cavallaro@st.com; linux-stm32@st-md-mailman.stormreply.com;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> netdev@vger.kernel.org; devicetree@vger.kernel.org
> 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, patchwork-bot+netdevbpf@kernel.org wrote:
> > > Hello:
> > >
> > > This series was applied to netdev/net-next.git (main) by David S.
> > > Miller <davem@davemloft.net>:
> > >
> > > On Fri,  5 Jan 2024 15:09:21 +0800 you wrote:
> > >> From: Swee Leong Ching <leong.ching.swee@intel.com>
> > >>
> > >> 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-
> > >> robh@kernel.org/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
> >
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH net-next v2 1/4] dt-bindings: net: snps,dwmac: per channel irq
  2024-01-07 20:10     ` Serge Semin
@ 2024-01-09  9:10       ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 40+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-09  9:10 UTC (permalink / raw)
  To: Serge Semin, Leong Ching Swee, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Giuseppe Cavallaro,
	linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Rohan G Thomas

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 <leong.ching.swee@intel.com>
>>
>> Add dt-bindings for per channel irq.
>>
>> Signed-off-by: Rohan G Thomas <rohan.g.thomas@intel.com>
>> Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
>> ---
>>  .../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


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH net-next v2 1/4] dt-bindings: net: snps,dwmac: per channel irq
@ 2024-01-09  9:10       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 40+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-09  9:10 UTC (permalink / raw)
  To: Serge Semin, Leong Ching Swee, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Giuseppe Cavallaro,
	linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Rohan G Thomas

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 <leong.ching.swee@intel.com>
>>
>> Add dt-bindings for per channel irq.
>>
>> Signed-off-by: Rohan G Thomas <rohan.g.thomas@intel.com>
>> Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
>> ---
>>  .../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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH net-next v2 1/4] dt-bindings: net: snps,dwmac: per channel irq
  2024-01-09  9:10       ` Krzysztof Kozlowski
@ 2024-01-09 22:20         ` Serge Semin
  -1 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2024-01-09 22:20 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Leong Ching Swee
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin,
	Alexandre Torgue, Jose Abreu, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Giuseppe Cavallaro, linux-stm32,
	linux-arm-kernel, linux-kernel, netdev, devicetree,
	Rohan G Thomas

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 <leong.ching.swee@intel.com>
> >>
> >> Add dt-bindings for per channel irq.
> >>
> >> Signed-off-by: Rohan G Thomas <rohan.g.thomas@intel.com>
> >> Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
> >> ---
> >>  .../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
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH net-next v2 1/4] dt-bindings: net: snps,dwmac: per channel irq
@ 2024-01-09 22:20         ` Serge Semin
  0 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2024-01-09 22:20 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Leong Ching Swee
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin,
	Alexandre Torgue, Jose Abreu, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Giuseppe Cavallaro, linux-stm32,
	linux-arm-kernel, linux-kernel, netdev, devicetree,
	Rohan G Thomas

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 <leong.ching.swee@intel.com>
> >>
> >> Add dt-bindings for per channel irq.
> >>
> >> Signed-off-by: Rohan G Thomas <rohan.g.thomas@intel.com>
> >> Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
> >> ---
> >>  .../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
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* RE: [PATCH net-next v2 4/4] net: stmmac: Use interrupt mode INTM=1 for per channel irq
  2024-01-07 20:52     ` Serge Semin
@ 2024-01-10  5:43       ` Swee, Leong Ching
  -1 siblings, 0 replies; 40+ messages in thread
From: Swee, Leong Ching @ 2024-01-10  5:43 UTC (permalink / raw)
  To: Serge Semin
  Cc: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro,
	linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Teoh Ji Sheng

> -----Original Message-----
> From: Serge Semin <fancer.lancer@gmail.com>
> Sent: Monday, January 8, 2024 4:52 AM
> To: Swee, Leong Ching <leong.ching.swee@intel.com>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>; Alexandre Torgue
> <alexandre.torgue@foss.st.com>; Jose Abreu <joabreu@synopsys.com>;
> David S . Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Rob Herring <robh+dt@kernel.org>; Krzysztof
> Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> <conor+dt@kernel.org>; Giuseppe Cavallaro <peppe.cavallaro@st.com>;
> linux-stm32@st-md-mailman.stormreply.com; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> netdev@vger.kernel.org; devicetree@vger.kernel.org; Teoh Ji Sheng
> <ji.sheng.teoh@intel.com>
> 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 <leong.ching.swee@intel.com>
> >
> > 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 <ji.sheng.teoh@intel.com>
> > Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
> > ---
> >  .../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
> >
> >

^ permalink raw reply	[flat|nested] 40+ messages in thread

* RE: [PATCH net-next v2 4/4] net: stmmac: Use interrupt mode INTM=1 for per channel irq
@ 2024-01-10  5:43       ` Swee, Leong Ching
  0 siblings, 0 replies; 40+ messages in thread
From: Swee, Leong Ching @ 2024-01-10  5:43 UTC (permalink / raw)
  To: Serge Semin
  Cc: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro,
	linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Teoh Ji Sheng

> -----Original Message-----
> From: Serge Semin <fancer.lancer@gmail.com>
> Sent: Monday, January 8, 2024 4:52 AM
> To: Swee, Leong Ching <leong.ching.swee@intel.com>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>; Alexandre Torgue
> <alexandre.torgue@foss.st.com>; Jose Abreu <joabreu@synopsys.com>;
> David S . Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Rob Herring <robh+dt@kernel.org>; Krzysztof
> Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> <conor+dt@kernel.org>; Giuseppe Cavallaro <peppe.cavallaro@st.com>;
> linux-stm32@st-md-mailman.stormreply.com; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> netdev@vger.kernel.org; devicetree@vger.kernel.org; Teoh Ji Sheng
> <ji.sheng.teoh@intel.com>
> 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 <leong.ching.swee@intel.com>
> >
> > 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 <ji.sheng.teoh@intel.com>
> > Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
> > ---
> >  .../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
> >
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* RE: [PATCH net-next v2 3/4] net: stmmac: Add support for TX/RX channel interrupt
  2024-01-07 20:38     ` Serge Semin
@ 2024-01-10  5:45       ` Swee, Leong Ching
  -1 siblings, 0 replies; 40+ messages in thread
From: Swee, Leong Ching @ 2024-01-10  5:45 UTC (permalink / raw)
  To: Serge Semin
  Cc: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro,
	linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Teoh Ji Sheng

> -----Original Message-----
> From: Serge Semin <fancer.lancer@gmail.com>
> Sent: Monday, January 8, 2024 4:39 AM
> To: Swee, Leong Ching <leong.ching.swee@intel.com>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>; Alexandre Torgue
> <alexandre.torgue@foss.st.com>; Jose Abreu <joabreu@synopsys.com>;
> David S . Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Rob Herring <robh+dt@kernel.org>; Krzysztof
> Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> <conor+dt@kernel.org>; Giuseppe Cavallaro <peppe.cavallaro@st.com>;
> linux-stm32@st-md-mailman.stormreply.com; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> netdev@vger.kernel.org; devicetree@vger.kernel.org; Teoh Ji Sheng
> <ji.sheng.teoh@intel.com>
> 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 <leong.ching.swee@intel.com>
> >
> > 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 <ji.sheng.teoh@intel.com>
> > Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
> > ---
> >  .../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
> >
> >

^ permalink raw reply	[flat|nested] 40+ messages in thread

* RE: [PATCH net-next v2 3/4] net: stmmac: Add support for TX/RX channel interrupt
@ 2024-01-10  5:45       ` Swee, Leong Ching
  0 siblings, 0 replies; 40+ messages in thread
From: Swee, Leong Ching @ 2024-01-10  5:45 UTC (permalink / raw)
  To: Serge Semin
  Cc: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro,
	linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Teoh Ji Sheng

> -----Original Message-----
> From: Serge Semin <fancer.lancer@gmail.com>
> Sent: Monday, January 8, 2024 4:39 AM
> To: Swee, Leong Ching <leong.ching.swee@intel.com>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>; Alexandre Torgue
> <alexandre.torgue@foss.st.com>; Jose Abreu <joabreu@synopsys.com>;
> David S . Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Rob Herring <robh+dt@kernel.org>; Krzysztof
> Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> <conor+dt@kernel.org>; Giuseppe Cavallaro <peppe.cavallaro@st.com>;
> linux-stm32@st-md-mailman.stormreply.com; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> netdev@vger.kernel.org; devicetree@vger.kernel.org; Teoh Ji Sheng
> <ji.sheng.teoh@intel.com>
> 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 <leong.ching.swee@intel.com>
> >
> > 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 <ji.sheng.teoh@intel.com>
> > Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
> > ---
> >  .../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
> >
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* RE: [PATCH net-next v2 2/4] net: stmmac: Make MSI interrupt routine generic
  2024-01-07 20:27     ` Serge Semin
@ 2024-01-10  5:51       ` Swee, Leong Ching
  -1 siblings, 0 replies; 40+ messages in thread
From: Swee, Leong Ching @ 2024-01-10  5:51 UTC (permalink / raw)
  To: Serge Semin
  Cc: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro,
	linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Teoh Ji Sheng

> -----Original Message-----
> From: Serge Semin <fancer.lancer@gmail.com>
> Sent: Monday, January 8, 2024 4:28 AM
> To: Swee, Leong Ching <leong.ching.swee@intel.com>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>; Alexandre Torgue
> <alexandre.torgue@foss.st.com>; Jose Abreu <joabreu@synopsys.com>;
> David S . Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Rob Herring <robh+dt@kernel.org>; Krzysztof
> Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> <conor+dt@kernel.org>; Giuseppe Cavallaro <peppe.cavallaro@st.com>;
> linux-stm32@st-md-mailman.stormreply.com; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> netdev@vger.kernel.org; devicetree@vger.kernel.org; Teoh Ji Sheng
> <ji.sheng.teoh@intel.com>
> 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 <leong.ching.swee@intel.com>
> >
> > 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 <ji.sheng.teoh@intel.com>
> > Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
> > ---
> >  .../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
> >
> >

^ permalink raw reply	[flat|nested] 40+ messages in thread

* RE: [PATCH net-next v2 2/4] net: stmmac: Make MSI interrupt routine generic
@ 2024-01-10  5:51       ` Swee, Leong Ching
  0 siblings, 0 replies; 40+ messages in thread
From: Swee, Leong Ching @ 2024-01-10  5:51 UTC (permalink / raw)
  To: Serge Semin
  Cc: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro,
	linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Teoh Ji Sheng

> -----Original Message-----
> From: Serge Semin <fancer.lancer@gmail.com>
> Sent: Monday, January 8, 2024 4:28 AM
> To: Swee, Leong Ching <leong.ching.swee@intel.com>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>; Alexandre Torgue
> <alexandre.torgue@foss.st.com>; Jose Abreu <joabreu@synopsys.com>;
> David S . Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Rob Herring <robh+dt@kernel.org>; Krzysztof
> Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> <conor+dt@kernel.org>; Giuseppe Cavallaro <peppe.cavallaro@st.com>;
> linux-stm32@st-md-mailman.stormreply.com; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> netdev@vger.kernel.org; devicetree@vger.kernel.org; Teoh Ji Sheng
> <ji.sheng.teoh@intel.com>
> 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 <leong.ching.swee@intel.com>
> >
> > 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 <ji.sheng.teoh@intel.com>
> > Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
> > ---
> >  .../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
> >
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH net-next v2 2/4] net: stmmac: Make MSI interrupt routine generic
  2024-01-10  5:51       ` Swee, Leong Ching
@ 2024-01-24 14:47         ` Serge Semin
  -1 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2024-01-24 14:47 UTC (permalink / raw)
  To: Swee, Leong Ching
  Cc: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro,
	linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Teoh Ji Sheng

On Wed, Jan 10, 2024 at 05:51:37AM +0000, Swee, Leong Ching wrote:
> > -----Original Message-----
> > From: Serge Semin <fancer.lancer@gmail.com>
> > Sent: Monday, January 8, 2024 4:28 AM
> > To: Swee, Leong Ching <leong.ching.swee@intel.com>
> > Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>; Alexandre Torgue
> > <alexandre.torgue@foss.st.com>; Jose Abreu <joabreu@synopsys.com>;
> > David S . Miller <davem@davemloft.net>; Eric Dumazet
> > <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> > <pabeni@redhat.com>; Rob Herring <robh+dt@kernel.org>; Krzysztof
> > Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> > <conor+dt@kernel.org>; Giuseppe Cavallaro <peppe.cavallaro@st.com>;
> > linux-stm32@st-md-mailman.stormreply.com; linux-arm-
> > kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> > netdev@vger.kernel.org; devicetree@vger.kernel.org; Teoh Ji Sheng
> > <ji.sheng.teoh@intel.com>
> > 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 <leong.ching.swee@intel.com>
> > >
> > > 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 <ji.sheng.teoh@intel.com>
> > > Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
> > > ---
> > >  .../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
> > >
> > >

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH net-next v2 2/4] net: stmmac: Make MSI interrupt routine generic
@ 2024-01-24 14:47         ` Serge Semin
  0 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2024-01-24 14:47 UTC (permalink / raw)
  To: Swee, Leong Ching
  Cc: Maxime Coquelin, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro,
	linux-stm32, linux-arm-kernel, linux-kernel, netdev, devicetree,
	Teoh Ji Sheng

On Wed, Jan 10, 2024 at 05:51:37AM +0000, Swee, Leong Ching wrote:
> > -----Original Message-----
> > From: Serge Semin <fancer.lancer@gmail.com>
> > Sent: Monday, January 8, 2024 4:28 AM
> > To: Swee, Leong Ching <leong.ching.swee@intel.com>
> > Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>; Alexandre Torgue
> > <alexandre.torgue@foss.st.com>; Jose Abreu <joabreu@synopsys.com>;
> > David S . Miller <davem@davemloft.net>; Eric Dumazet
> > <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> > <pabeni@redhat.com>; Rob Herring <robh+dt@kernel.org>; Krzysztof
> > Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> > <conor+dt@kernel.org>; Giuseppe Cavallaro <peppe.cavallaro@st.com>;
> > linux-stm32@st-md-mailman.stormreply.com; linux-arm-
> > kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> > netdev@vger.kernel.org; devicetree@vger.kernel.org; Teoh Ji Sheng
> > <ji.sheng.teoh@intel.com>
> > 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 <leong.ching.swee@intel.com>
> > >
> > > 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 <ji.sheng.teoh@intel.com>
> > > Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com>
> > > ---
> > >  .../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
> > >
> > >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2024-01-24 14:48 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-05  7:09 [PATCH net-next v2 0/4] net: stmmac: Enable Per DMA Channel interrupt Leong Ching Swee
2024-01-05  7:09 ` Leong Ching Swee
2024-01-05  7:09 ` [PATCH net-next v2 1/4] dt-bindings: net: snps,dwmac: per channel irq Leong Ching Swee
2024-01-05  7:09   ` Leong Ching Swee
2024-01-07 20:10   ` Serge Semin
2024-01-07 20:10     ` Serge Semin
2024-01-09  9:10     ` Krzysztof Kozlowski
2024-01-09  9:10       ` Krzysztof Kozlowski
2024-01-09 22:20       ` Serge Semin
2024-01-09 22:20         ` Serge Semin
2024-01-05  7:09 ` [PATCH net-next v2 2/4] net: stmmac: Make MSI interrupt routine generic Leong Ching Swee
2024-01-05  7:09   ` Leong Ching Swee
2024-01-07 20:27   ` Serge Semin
2024-01-07 20:27     ` Serge Semin
2024-01-10  5:51     ` Swee, Leong Ching
2024-01-10  5:51       ` Swee, Leong Ching
2024-01-24 14:47       ` Serge Semin
2024-01-24 14:47         ` Serge Semin
2024-01-05  7:09 ` [PATCH net-next v2 3/4] net: stmmac: Add support for TX/RX channel interrupt Leong Ching Swee
2024-01-05  7:09   ` Leong Ching Swee
2024-01-07 20:38   ` Serge Semin
2024-01-07 20:38     ` Serge Semin
2024-01-10  5:45     ` Swee, Leong Ching
2024-01-10  5:45       ` Swee, Leong Ching
2024-01-05  7:09 ` [PATCH net-next v2 4/4] net: stmmac: Use interrupt mode INTM=1 for per channel irq Leong Ching Swee
2024-01-05  7:09   ` Leong Ching Swee
2024-01-07 20:52   ` Serge Semin
2024-01-07 20:52     ` Serge Semin
2024-01-10  5:43     ` Swee, Leong Ching
2024-01-10  5:43       ` Swee, Leong Ching
2024-01-07 16:40 ` [PATCH net-next v2 0/4] net: stmmac: Enable Per DMA Channel interrupt patchwork-bot+netdevbpf
2024-01-07 16:40   ` patchwork-bot+netdevbpf
2024-01-07 19:06   ` Krzysztof Kozlowski
2024-01-07 19:06     ` Krzysztof Kozlowski
2024-01-07 21:24     ` Serge Semin
2024-01-07 21:24       ` Serge Semin
2024-01-08  1:02       ` Jakub Kicinski
2024-01-08  1:02         ` Jakub Kicinski
2024-01-09  8:05       ` Swee, Leong Ching
2024-01-09  8:05         ` Swee, Leong Ching

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.