prepare mac operations for multiple queues

This patch adds the multiple queues configuration in the Device Tree. It was also created a set of structures to keep the RX and TX queues configurations to be used in the driver.

Signed-off-by Joao Pinto <jpinto-HKixBCOQz3hWk0Htik3J/[email protected]>

---
Changes v4->v5:
- patch title update (stmicro replaced by stmmac)
Changes v2->v4:
- Just to keep up with patch-set version
Changes v1->v2:
- RX and TX queues child nodes had bad handle

 Documentation/devicetree/bindings/net/stmmac.txt   | 40 ++++++++++
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  | 91 ++++++++++++++++++++++
 include/linux/stmmac.h                             | 30 +++++++
 3 files changed, 161 insertions(+)

Documentation/devicetree/bindings/net/stmmac.txt +38 -0

index d3bfc2b..4107e67 100644
@@ -72,6 +72,27 @@ Optional properties:
 	- snps,mb: mixed-burst
 	- snps,rb: rebuild INCRx Burst
 - mdio: with compatible = "snps,dwmac-mdio", create and register mdio bus.
+- Multiple RX Queues parameters: below the list of all the parameters to
+				 configure the multiple RX queues:
+	- snps,rx-queues-to-use: number of RX queues to be used in the driver
+	- Choose one of these RX scheduling algorithms:
+		- snps,rx-sched-sp: Strict priority
+		- snps,rx-sched-wsp: Weighted Strict priority
+	- For each RX queue
+		- Choose one of these modes:
+			- snps,dcb-algorithm: Queue to be enabled as DCB
+			- snps,avb-algorithm: Queue to be enabled as AVB
+		- snps,map-to-dma-channel: Channel to map
+- Multiple TX Queues parameters: below the list of all the parameters to
+				 configure the multiple TX queues:
+	- snps,tx-queues-to-use: number of TX queues to be used in the driver
+	- Choose one of these TX scheduling algorithms:
+		- snps,tx-sched-wrr: Weighted Round Robin
+		- snps,tx-sched-wfq: Weighted Fair Queuing
+		- snps,tx-sched-dwrr: Deficit Weighted Round Robin
+		- snps,tx-sched-sp: Strict priority
+	- For each TX queue
+		- snps,weight: TX queue weight (if using a weighted algorithm)
 
 Examples:
 

@@ -81,6 +102,23 @@ Examples:
 		snps,blen = <256 128 64 32 0 0 0>;
 	};
 
+	mtl_rx_setup: rx-queues-config {
+		snps,rx-queues-to-use = <1>;
+		snps,rx-sched-sp;
+		queue0 {
+			snps,dcb-algorithm;
+			snps,map-to-dma-channel = <0x0>;
+		};
+	};
+
+	mtl_tx_setup: tx-queues-config {
+		snps,tx-queues-to-use = <1>;
+		snps,tx-sched-wrr;
+		queue0 {
+			snps,weight = <0x10>;
+		};
+	};
+
 	gmac0: ethernet@e0800000 {
 		compatible = "st,spear600-gmac";
 		reg = <0xe0800000 0x8000>;

Quoted 75 lines

This patch adds the multiple queues configuration in the Device Tree. It was also created a set of structures to keep the RX and TX queues configurations to be used in the driver.

Signed-off-by Joao Pinto <jpinto-HKixBCOQz3hWk0Htik3J/[email protected]>

---
Changes v4->v5:
- patch title update (stmicro replaced by stmmac)
Changes v2->v4:
- Just to keep up with patch-set version
Changes v1->v2:
- RX and TX queues child nodes had bad handle

 Documentation/devicetree/bindings/net/stmmac.txt   | 40 ++++++++++
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  | 91 ++++++++++++++++++++++
 include/linux/stmmac.h                             | 30 +++++++
 3 files changed, 161 insertions(+)

Documentation/devicetree/bindings/net/stmmac.txt +38 -0

index d3bfc2b..4107e67 100644
@@ -72,6 +72,27 @@ Optional properties:
 	- snps,mb: mixed-burst
 	- snps,rb: rebuild INCRx Burst
 - mdio: with compatible = "snps,dwmac-mdio", create and register mdio bus.
+- Multiple RX Queues parameters: below the list of all the parameters to
+				 configure the multiple RX queues:
+	- snps,rx-queues-to-use: number of RX queues to be used in the driver
+	- Choose one of these RX scheduling algorithms:
+		- snps,rx-sched-sp: Strict priority
+		- snps,rx-sched-wsp: Weighted Strict priority
+	- For each RX queue
+		- Choose one of these modes:
+			- snps,dcb-algorithm: Queue to be enabled as DCB
+			- snps,avb-algorithm: Queue to be enabled as AVB
+		- snps,map-to-dma-channel: Channel to map
+- Multiple TX Queues parameters: below the list of all the parameters to
+				 configure the multiple TX queues:
+	- snps,tx-queues-to-use: number of TX queues to be used in the driver
+	- Choose one of these TX scheduling algorithms:
+		- snps,tx-sched-wrr: Weighted Round Robin
+		- snps,tx-sched-wfq: Weighted Fair Queuing
+		- snps,tx-sched-dwrr: Deficit Weighted Round Robin
+		- snps,tx-sched-sp: Strict priority
+	- For each TX queue
+		- snps,weight: TX queue weight (if using a weighted algorithm)
 
 Examples:
 

@@ -81,6 +102,23 @@ Examples:
 		snps,blen = <256 128 64 32 0 0 0>;
 	};
 
+	mtl_rx_setup: rx-queues-config {
+		snps,rx-queues-to-use = <1>;
+		snps,rx-sched-sp;
+		queue0 {
+			snps,dcb-algorithm;
+			snps,map-to-dma-channel = <0x0>;
+		};
+	};
+
+	mtl_tx_setup: tx-queues-config {
+		snps,tx-queues-to-use = <1>;
+		snps,tx-sched-wrr;
+		queue0 {
+			snps,weight = <0x10>;
+		};
+	};
+
 	gmac0: ethernet@e0800000 {
 		compatible = "st,spear600-gmac";
 		reg = <0xe0800000 0x8000>;

Documentation/devicetree/bindings/net/stmmac.txt Examples: · line 147

Quoted 6 lines

Documentation/devicetree/bindings/net/stmmac.txt +2 -0

@@ -104,4 +142,6 @@ Examples:
 			phy1: ethernet-phy@0 {
 			};
 		};
+		snps,mtl-rx-config = <&mtl_rx_setup>;
+		snps,mtl-tx-config = <&mtl_tx_setup>;
 	};

That's kind of an odd placement for these nodes. Wouldn't they be better located within the controller's device tree node, rather than referenced by phandle? Making them children of the root node leaves them completely without context.

Thierry

Hi Thierry,

Às 11:32 AM de 3/21/2017, Thierry Reding escreveu:

Quoted 88 lines · Joao Pinto

Quoted 75 lines

This patch adds the multiple queues configuration in the Device Tree. It was also created a set of structures to keep the RX and TX queues configurations to be used in the driver.

Signed-off-by Joao Pinto <jpinto-HKixBCOQz3hWk0Htik3J/[email protected]>

---
Changes v4->v5:
- patch title update (stmicro replaced by stmmac)
Changes v2->v4:
- Just to keep up with patch-set version
Changes v1->v2:
- RX and TX queues child nodes had bad handle

 Documentation/devicetree/bindings/net/stmmac.txt   | 40 ++++++++++
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  | 91 ++++++++++++++++++++++
 include/linux/stmmac.h                             | 30 +++++++
 3 files changed, 161 insertions(+)

Documentation/devicetree/bindings/net/stmmac.txt +38 -0

index d3bfc2b..4107e67 100644
@@ -72,6 +72,27 @@ Optional properties:
 	- snps,mb: mixed-burst
 	- snps,rb: rebuild INCRx Burst
 - mdio: with compatible = "snps,dwmac-mdio", create and register mdio bus.
+- Multiple RX Queues parameters: below the list of all the parameters to
+				 configure the multiple RX queues:
+	- snps,rx-queues-to-use: number of RX queues to be used in the driver
+	- Choose one of these RX scheduling algorithms:
+		- snps,rx-sched-sp: Strict priority
+		- snps,rx-sched-wsp: Weighted Strict priority
+	- For each RX queue
+		- Choose one of these modes:
+			- snps,dcb-algorithm: Queue to be enabled as DCB
+			- snps,avb-algorithm: Queue to be enabled as AVB
+		- snps,map-to-dma-channel: Channel to map
+- Multiple TX Queues parameters: below the list of all the parameters to
+				 configure the multiple TX queues:
+	- snps,tx-queues-to-use: number of TX queues to be used in the driver
+	- Choose one of these TX scheduling algorithms:
+		- snps,tx-sched-wrr: Weighted Round Robin
+		- snps,tx-sched-wfq: Weighted Fair Queuing
+		- snps,tx-sched-dwrr: Deficit Weighted Round Robin
+		- snps,tx-sched-sp: Strict priority
+	- For each TX queue
+		- snps,weight: TX queue weight (if using a weighted algorithm)
 
 Examples:
 

@@ -81,6 +102,23 @@ Examples:
 		snps,blen = <256 128 64 32 0 0 0>;
 	};
 
+	mtl_rx_setup: rx-queues-config {
+		snps,rx-queues-to-use = <1>;
+		snps,rx-sched-sp;
+		queue0 {
+			snps,dcb-algorithm;
+			snps,map-to-dma-channel = <0x0>;
+		};
+	};
+
+	mtl_tx_setup: tx-queues-config {
+		snps,tx-queues-to-use = <1>;
+		snps,tx-sched-wrr;
+		queue0 {
+			snps,weight = <0x10>;
+		};
+	};
+
 	gmac0: ethernet@e0800000 {
 		compatible = "st,spear600-gmac";
 		reg = <0xe0800000 0x8000>;

Documentation/devicetree/bindings/net/stmmac.txt Examples: · line 147

Quoted 6 lines

Documentation/devicetree/bindings/net/stmmac.txt +2 -0

@@ -104,4 +142,6 @@ Examples:
 			phy1: ethernet-phy@0 {
 			};
 		};
+		snps,mtl-rx-config = <&mtl_rx_setup>;
+		snps,mtl-tx-config = <&mtl_tx_setup>;
 	};

That's kind of an odd placement for these nodes. Wouldn't they be better located within the controller's device tree node, rather than referenced by phandle? Making them children of the root node leaves them completely without context.

Quoted 2 lines

Thierry

I followed the same approach as "stmmac_axi_setup":
https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/Documentation/devicetree/bindings/net/stmmac.txt

I personally find it cleaner and easier to read.

This patch adds the RX and TX scheduling algorithms programming. It introduces the multiple queues configuration function (stmmac_mtl_configuration) in stmmac_main.

Signed-off-by Joao Pinto <[email protected]>

---
Changes v4->v5:
- patch title update (stmicro replaced by stmmac)
Changes v3->v4:
- Just to keep up with patch-set version
Changes v2->v3:
- Switch statements with a tab
Changes v1->v2:
- Just to keep up with patch-set version

 drivers/net/ethernet/stmicro/stmmac/common.h      |  4 ++
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h      | 10 +++++
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 48 +++++++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 31 +++++++++++++--
 4 files changed, 90 insertions(+), 3 deletions(-)

drivers/net/ethernet/stmicro/stmmac/common.h +4 -0

index 04d9245..5a0a781 100644
@@ -455,6 +455,10 @@ struct stmmac_ops {
 	int (*rx_ipc)(struct mac_device_info *hw);
 	/* Enable RX Queues */
 	void (*rx_queue_enable)(struct mac_device_info *hw, u32 queue);
+	/* Program RX Algorithms */
+	void (*prog_mtl_rx_algorithms)(struct mac_device_info *hw, u32 rx_alg);
+	/* Program TX Algorithms */
+	void (*prog_mtl_tx_algorithms)(struct mac_device_info *hw, u32 tx_alg);
 	/* Dump MAC registers */
 	void (*dump_regs)(struct mac_device_info *hw, u32 *reg_space);
 	/* Handle extra events on specific interrupts hw dependent */

drivers/net/ethernet/stmicro/stmmac/dwmac4.h +10 -0

index db45134..748ab6f 100644
@@ -161,6 +161,16 @@ enum power_event {
 #define GMAC_HI_REG_AE			BIT(31)
 
 /*  MTL registers */
+#define MTL_OPERATION_MODE		0x00000c00
+#define MTL_OPERATION_SCHALG_MASK	GENMASK(6, 5)
+#define MTL_OPERATION_SCHALG_WRR	(0x0 << 5)
+#define MTL_OPERATION_SCHALG_WFQ	(0x1 << 5)
+#define MTL_OPERATION_SCHALG_DWRR	(0x2 << 5)
+#define MTL_OPERATION_SCHALG_SP		(0x3 << 5)
+#define MTL_OPERATION_RAA		BIT(2)
+#define MTL_OPERATION_RAA_SP		(0x0 << 2)
+#define MTL_OPERATION_RAA_WSP		(0x1 << 2)
+
 #define MTL_INT_STATUS			0x00000c20
 #define MTL_INT_Q0			BIT(0)
 

drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +48 -0

index 1e79e65..f966755 100644
@@ -70,6 +70,52 @@ static void dwmac4_rx_queue_enable(struct mac_device_info *hw, u32 queue)
 	writel(value, ioaddr + GMAC_RXQ_CTRL0);
 }
 
+static void dwmac4_prog_mtl_rx_algorithms(struct mac_device_info *hw,
+					  u32 rx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_RAA;
+	switch (rx_alg) {
+	case MTL_RX_ALGORITHM_SP:
+		value |= MTL_OPERATION_RAA_SP;
+		break;
+	case MTL_RX_ALGORITHM_WSP:
+		value |= MTL_OPERATION_RAA_WSP;
+		break;
+	default:
+		break;
+	}
+
+	writel(value, ioaddr + MTL_OPERATION_MODE);
+}
+
+static void dwmac4_prog_mtl_tx_algorithms(struct mac_device_info *hw,
+					  u32 tx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_SCHALG_MASK;
+	switch (tx_alg) {
+	case MTL_TX_ALGORITHM_WRR:
+		value |= MTL_OPERATION_SCHALG_WRR;
+		break;
+	case MTL_TX_ALGORITHM_WFQ:
+		value |= MTL_OPERATION_SCHALG_WFQ;
+		break;
+	case MTL_TX_ALGORITHM_DWRR:
+		value |= MTL_OPERATION_SCHALG_DWRR;
+		break;
+	case MTL_TX_ALGORITHM_SP:
+		value |= MTL_OPERATION_SCHALG_SP;
+		break;
+	default:
+		break;
+	}
+}
+
 static void dwmac4_dump_regs(struct mac_device_info *hw, u32 *reg_space)
 {
 	void __iomem *ioaddr = hw->pcsr;

@@ -457,6 +503,8 @@ static const struct stmmac_ops dwmac4_ops = {
 	.core_init = dwmac4_core_init,
 	.rx_ipc = dwmac4_rx_ipc_enable,
 	.rx_queue_enable = dwmac4_rx_queue_enable,
+	.prog_mtl_rx_algorithms = dwmac4_prog_mtl_rx_algorithms,
+	.prog_mtl_tx_algorithms = dwmac4_prog_mtl_tx_algorithms,
 	.dump_regs = dwmac4_dump_regs,
 	.host_irq_status = dwmac4_irq_status,
 	.flow_ctrl = dwmac4_flow_ctrl,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +12 -0

index 4498a38..af57f8d 100644
@@ -1645,6 +1645,31 @@ static void stmmac_init_tx_coalesce(struct stmmac_priv *priv)
 }
 
 /**
+ *  stmmac_mtl_configuration - Configure MTL
+ *  @priv: driver private structure
+ *  Description: It is used for configurring MTL
+ */
+static void stmmac_mtl_configuration(struct stmmac_priv *priv)
+{
+	u32 rx_queues_count = priv->plat->rx_queues_to_use;
+	u32 tx_queues_count = priv->plat->tx_queues_to_use;
+
+	/* Configure MTL RX algorithms */
+	if (rx_queues_count > 1 && priv->hw->mac->prog_mtl_rx_algorithms)
+		priv->hw->mac->prog_mtl_rx_algorithms(priv->hw,

Quoted 15 lines

This patch adds the RX and TX scheduling algorithms programming. It introduces the multiple queues configuration function (stmmac_mtl_configuration) in stmmac_main.

Signed-off-by Joao Pinto <[email protected]>

---
Changes v4->v5:
- patch title update (stmicro replaced by stmmac)
Changes v3->v4:
- Just to keep up with patch-set version
Changes v2->v3:
- Switch statements with a tab
Changes v1->v2:
- Just to keep up with patch-set version

Quoted 5 lines

 drivers/net/ethernet/stmicro/stmmac/common.h      |  4 ++
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h      | 10 +++++
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 48 +++++++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 31 +++++++++++++--
 4 files changed, 90 insertions(+), 3 deletions(-)

This patch breaks backwards-compatibility with DTBs that don't have an of the multiple queue properties.

See below...

Quoted 122 lines

drivers/net/ethernet/stmicro/stmmac/common.h +4 -0

index 04d9245..5a0a781 100644
@@ -455,6 +455,10 @@ struct stmmac_ops {
 	int (*rx_ipc)(struct mac_device_info *hw);
 	/* Enable RX Queues */
 	void (*rx_queue_enable)(struct mac_device_info *hw, u32 queue);
+	/* Program RX Algorithms */
+	void (*prog_mtl_rx_algorithms)(struct mac_device_info *hw, u32 rx_alg);
+	/* Program TX Algorithms */
+	void (*prog_mtl_tx_algorithms)(struct mac_device_info *hw, u32 tx_alg);
 	/* Dump MAC registers */
 	void (*dump_regs)(struct mac_device_info *hw, u32 *reg_space);
 	/* Handle extra events on specific interrupts hw dependent */

drivers/net/ethernet/stmicro/stmmac/dwmac4.h +10 -0

index db45134..748ab6f 100644
@@ -161,6 +161,16 @@ enum power_event {
 #define GMAC_HI_REG_AE			BIT(31)
 
 /*  MTL registers */
+#define MTL_OPERATION_MODE		0x00000c00
+#define MTL_OPERATION_SCHALG_MASK	GENMASK(6, 5)
+#define MTL_OPERATION_SCHALG_WRR	(0x0 << 5)
+#define MTL_OPERATION_SCHALG_WFQ	(0x1 << 5)
+#define MTL_OPERATION_SCHALG_DWRR	(0x2 << 5)
+#define MTL_OPERATION_SCHALG_SP		(0x3 << 5)
+#define MTL_OPERATION_RAA		BIT(2)
+#define MTL_OPERATION_RAA_SP		(0x0 << 2)
+#define MTL_OPERATION_RAA_WSP		(0x1 << 2)
+
 #define MTL_INT_STATUS			0x00000c20
 #define MTL_INT_Q0			BIT(0)
 

drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +48 -0

index 1e79e65..f966755 100644
@@ -70,6 +70,52 @@ static void dwmac4_rx_queue_enable(struct mac_device_info *hw, u32 queue)
 	writel(value, ioaddr + GMAC_RXQ_CTRL0);
 }
 
+static void dwmac4_prog_mtl_rx_algorithms(struct mac_device_info *hw,
+					  u32 rx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_RAA;
+	switch (rx_alg) {
+	case MTL_RX_ALGORITHM_SP:
+		value |= MTL_OPERATION_RAA_SP;
+		break;
+	case MTL_RX_ALGORITHM_WSP:
+		value |= MTL_OPERATION_RAA_WSP;
+		break;
+	default:
+		break;
+	}
+
+	writel(value, ioaddr + MTL_OPERATION_MODE);
+}
+
+static void dwmac4_prog_mtl_tx_algorithms(struct mac_device_info *hw,
+					  u32 tx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_SCHALG_MASK;
+	switch (tx_alg) {
+	case MTL_TX_ALGORITHM_WRR:
+		value |= MTL_OPERATION_SCHALG_WRR;
+		break;
+	case MTL_TX_ALGORITHM_WFQ:
+		value |= MTL_OPERATION_SCHALG_WFQ;
+		break;
+	case MTL_TX_ALGORITHM_DWRR:
+		value |= MTL_OPERATION_SCHALG_DWRR;
+		break;
+	case MTL_TX_ALGORITHM_SP:
+		value |= MTL_OPERATION_SCHALG_SP;
+		break;
+	default:
+		break;
+	}
+}
+
 static void dwmac4_dump_regs(struct mac_device_info *hw, u32 *reg_space)
 {
 	void __iomem *ioaddr = hw->pcsr;

@@ -457,6 +503,8 @@ static const struct stmmac_ops dwmac4_ops = {
 	.core_init = dwmac4_core_init,
 	.rx_ipc = dwmac4_rx_ipc_enable,
 	.rx_queue_enable = dwmac4_rx_queue_enable,
+	.prog_mtl_rx_algorithms = dwmac4_prog_mtl_rx_algorithms,
+	.prog_mtl_tx_algorithms = dwmac4_prog_mtl_tx_algorithms,
 	.dump_regs = dwmac4_dump_regs,
 	.host_irq_status = dwmac4_irq_status,
 	.flow_ctrl = dwmac4_flow_ctrl,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +12 -0

index 4498a38..af57f8d 100644
@@ -1645,6 +1645,31 @@ static void stmmac_init_tx_coalesce(struct stmmac_priv *priv)
 }
 
 /**
+ *  stmmac_mtl_configuration - Configure MTL
+ *  @priv: driver private structure
+ *  Description: It is used for configurring MTL
+ */
+static void stmmac_mtl_configuration(struct stmmac_priv *priv)
+{
+	u32 rx_queues_count = priv->plat->rx_queues_to_use;
+	u32 tx_queues_count = priv->plat->tx_queues_to_use;
+
+	/* Configure MTL RX algorithms */
+	if (rx_queues_count > 1 && priv->hw->mac->prog_mtl_rx_algorithms)
+		priv->hw->mac->prog_mtl_rx_algorithms(priv->hw,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c static void stmmac_init_tx_coalesce(struct stmmac_priv *priv) · line 1669

Quoted 10 lines

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +10 -0

+						priv->plat->rx_sched_algorithm);
+
+	/* Configure MTL TX algorithms */
+	if (tx_queues_count > 1 && priv->hw->mac->prog_mtl_tx_algorithms)
+		priv->hw->mac->prog_mtl_tx_algorithms(priv->hw,
+						priv->plat->tx_sched_algorithm);
+
+	/* Enable MAC RX Queues */
+	if (rx_queues_count > 1 && priv->hw->mac->rx_queue_enable)
+		stmmac_mac_enable_rx_queues(priv);

This is almost equivalent to the code removed from stmmac_hw_setup() which happens to be the key for this driver to work for me. However, the code above adds an additional check for rx_queues_count > 1 which is going to be false for any existing DTB, because it is derived from the values retrieved from new device tree properties.

So I think for backwards compatibility we'd need something like this:

	if ((rx_queue_count == 0 || rx_queue_count > 1) &&
	    priv->hw->mac->rx_queue_enable)

But then I'm beginning to think maybe we don't need a check here at all because it would only prevent RX queue setup for rx_queue_count == 1 and I think it would still be legitimate to set it up even then.

stmmac_mac_enable_rx_queues() already checks for rx_count == 1, but that is derived from the number of RX queues derived from the feature registers and therefore refers to the number of queues that the hardware supports as opposed to the number of queues configured in device tree.

I can follow up with a patch to restore backwards-compatibility.

Thierry

Às 11:58 AM de 3/21/2017, Thierry Reding escreveu:

Quoted 180 lines · Joao Pinto

Quoted 15 lines

This patch adds the RX and TX scheduling algorithms programming. It introduces the multiple queues configuration function (stmmac_mtl_configuration) in stmmac_main.

Signed-off-by Joao Pinto <[email protected]>

---
Changes v4->v5:
- patch title update (stmicro replaced by stmmac)
Changes v3->v4:
- Just to keep up with patch-set version
Changes v2->v3:
- Switch statements with a tab
Changes v1->v2:
- Just to keep up with patch-set version

Quoted 5 lines

 drivers/net/ethernet/stmicro/stmmac/common.h      |  4 ++
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h      | 10 +++++
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 48 +++++++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 31 +++++++++++++--
 4 files changed, 90 insertions(+), 3 deletions(-)

This patch breaks backwards-compatibility with DTBs that don't have an of the multiple queue properties.

See below...

Quoted 122 lines

drivers/net/ethernet/stmicro/stmmac/common.h +4 -0

index 04d9245..5a0a781 100644
@@ -455,6 +455,10 @@ struct stmmac_ops {
 	int (*rx_ipc)(struct mac_device_info *hw);
 	/* Enable RX Queues */
 	void (*rx_queue_enable)(struct mac_device_info *hw, u32 queue);
+	/* Program RX Algorithms */
+	void (*prog_mtl_rx_algorithms)(struct mac_device_info *hw, u32 rx_alg);
+	/* Program TX Algorithms */
+	void (*prog_mtl_tx_algorithms)(struct mac_device_info *hw, u32 tx_alg);
 	/* Dump MAC registers */
 	void (*dump_regs)(struct mac_device_info *hw, u32 *reg_space);
 	/* Handle extra events on specific interrupts hw dependent */

drivers/net/ethernet/stmicro/stmmac/dwmac4.h +10 -0

index db45134..748ab6f 100644
@@ -161,6 +161,16 @@ enum power_event {
 #define GMAC_HI_REG_AE			BIT(31)
 
 /*  MTL registers */
+#define MTL_OPERATION_MODE		0x00000c00
+#define MTL_OPERATION_SCHALG_MASK	GENMASK(6, 5)
+#define MTL_OPERATION_SCHALG_WRR	(0x0 << 5)
+#define MTL_OPERATION_SCHALG_WFQ	(0x1 << 5)
+#define MTL_OPERATION_SCHALG_DWRR	(0x2 << 5)
+#define MTL_OPERATION_SCHALG_SP		(0x3 << 5)
+#define MTL_OPERATION_RAA		BIT(2)
+#define MTL_OPERATION_RAA_SP		(0x0 << 2)
+#define MTL_OPERATION_RAA_WSP		(0x1 << 2)
+
 #define MTL_INT_STATUS			0x00000c20
 #define MTL_INT_Q0			BIT(0)
 

drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +48 -0

index 1e79e65..f966755 100644
@@ -70,6 +70,52 @@ static void dwmac4_rx_queue_enable(struct mac_device_info *hw, u32 queue)
 	writel(value, ioaddr + GMAC_RXQ_CTRL0);
 }
 
+static void dwmac4_prog_mtl_rx_algorithms(struct mac_device_info *hw,
+					  u32 rx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_RAA;
+	switch (rx_alg) {
+	case MTL_RX_ALGORITHM_SP:
+		value |= MTL_OPERATION_RAA_SP;
+		break;
+	case MTL_RX_ALGORITHM_WSP:
+		value |= MTL_OPERATION_RAA_WSP;
+		break;
+	default:
+		break;
+	}
+
+	writel(value, ioaddr + MTL_OPERATION_MODE);
+}
+
+static void dwmac4_prog_mtl_tx_algorithms(struct mac_device_info *hw,
+					  u32 tx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_SCHALG_MASK;
+	switch (tx_alg) {
+	case MTL_TX_ALGORITHM_WRR:
+		value |= MTL_OPERATION_SCHALG_WRR;
+		break;
+	case MTL_TX_ALGORITHM_WFQ:
+		value |= MTL_OPERATION_SCHALG_WFQ;
+		break;
+	case MTL_TX_ALGORITHM_DWRR:
+		value |= MTL_OPERATION_SCHALG_DWRR;
+		break;
+	case MTL_TX_ALGORITHM_SP:
+		value |= MTL_OPERATION_SCHALG_SP;
+		break;
+	default:
+		break;
+	}
+}
+
 static void dwmac4_dump_regs(struct mac_device_info *hw, u32 *reg_space)
 {
 	void __iomem *ioaddr = hw->pcsr;

@@ -457,6 +503,8 @@ static const struct stmmac_ops dwmac4_ops = {
 	.core_init = dwmac4_core_init,
 	.rx_ipc = dwmac4_rx_ipc_enable,
 	.rx_queue_enable = dwmac4_rx_queue_enable,
+	.prog_mtl_rx_algorithms = dwmac4_prog_mtl_rx_algorithms,
+	.prog_mtl_tx_algorithms = dwmac4_prog_mtl_tx_algorithms,
 	.dump_regs = dwmac4_dump_regs,
 	.host_irq_status = dwmac4_irq_status,
 	.flow_ctrl = dwmac4_flow_ctrl,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +12 -0

index 4498a38..af57f8d 100644
@@ -1645,6 +1645,31 @@ static void stmmac_init_tx_coalesce(struct stmmac_priv *priv)
 }
 
 /**
+ *  stmmac_mtl_configuration - Configure MTL
+ *  @priv: driver private structure
+ *  Description: It is used for configurring MTL
+ */
+static void stmmac_mtl_configuration(struct stmmac_priv *priv)
+{
+	u32 rx_queues_count = priv->plat->rx_queues_to_use;
+	u32 tx_queues_count = priv->plat->tx_queues_to_use;
+
+	/* Configure MTL RX algorithms */
+	if (rx_queues_count > 1 && priv->hw->mac->prog_mtl_rx_algorithms)
+		priv->hw->mac->prog_mtl_rx_algorithms(priv->hw,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c static void stmmac_init_tx_coalesce(struct stmmac_priv *priv) · line 1669

Quoted 10 lines

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +10 -0

+						priv->plat->rx_sched_algorithm);
+
+	/* Configure MTL TX algorithms */
+	if (tx_queues_count > 1 && priv->hw->mac->prog_mtl_tx_algorithms)
+		priv->hw->mac->prog_mtl_tx_algorithms(priv->hw,
+						priv->plat->tx_sched_algorithm);
+
+	/* Enable MAC RX Queues */
+	if (rx_queues_count > 1 && priv->hw->mac->rx_queue_enable)
+		stmmac_mac_enable_rx_queues(priv);

This is almost equivalent to the code removed from stmmac_hw_setup() which happens to be the key for this driver to work for me. However, the code above adds an additional check for rx_queues_count > 1 which is going to be false for any existing DTB, because it is derived from the values retrieved from new device tree properties.

So I think for backwards compatibility we'd need something like this:

	if ((rx_queue_count == 0 || rx_queue_count > 1) &&
	    priv->hw->mac->rx_queue_enable)

But then I'm beginning to think maybe we don't need a check here at all because it would only prevent RX queue setup for rx_queue_count == 1 and I think it would still be legitimate to set it up even then.

stmmac_mac_enable_rx_queues() already checks for rx_count == 1, but that is derived from the number of RX queues derived from the feature registers and therefore refers to the number of queues that the hardware supports as opposed to the number of queues configured in device tree.

Quoted 1 line

I can follow up with a patch to restore backwards-compatibility.

Forhw configured as single queue you don't need to enable the rx queue, since they are enable by default. if you check in stmmac_platform.c I assure backward compatibility by setting the number of rx and tx queues = 1 if nothing is declared. Please check here:

https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c#n156

Quoted 188 lines

Às 11:58 AM de 3/21/2017, Thierry Reding escreveu:

Quoted 175 lines · Joao Pinto

Quoted 15 lines

This patch adds the RX and TX scheduling algorithms programming. It introduces the multiple queues configuration function (stmmac_mtl_configuration) in stmmac_main.

Signed-off-by Joao Pinto <[email protected]>

---
Changes v4->v5:
- patch title update (stmicro replaced by stmmac)
Changes v3->v4:
- Just to keep up with patch-set version
Changes v2->v3:
- Switch statements with a tab
Changes v1->v2:
- Just to keep up with patch-set version

Quoted 5 lines

 drivers/net/ethernet/stmicro/stmmac/common.h      |  4 ++
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h      | 10 +++++
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 48 +++++++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 31 +++++++++++++--
 4 files changed, 90 insertions(+), 3 deletions(-)

This patch breaks backwards-compatibility with DTBs that don't have an of the multiple queue properties.

See below...

Quoted 122 lines

drivers/net/ethernet/stmicro/stmmac/common.h +4 -0

index 04d9245..5a0a781 100644
@@ -455,6 +455,10 @@ struct stmmac_ops {
 	int (*rx_ipc)(struct mac_device_info *hw);
 	/* Enable RX Queues */
 	void (*rx_queue_enable)(struct mac_device_info *hw, u32 queue);
+	/* Program RX Algorithms */
+	void (*prog_mtl_rx_algorithms)(struct mac_device_info *hw, u32 rx_alg);
+	/* Program TX Algorithms */
+	void (*prog_mtl_tx_algorithms)(struct mac_device_info *hw, u32 tx_alg);
 	/* Dump MAC registers */
 	void (*dump_regs)(struct mac_device_info *hw, u32 *reg_space);
 	/* Handle extra events on specific interrupts hw dependent */

drivers/net/ethernet/stmicro/stmmac/dwmac4.h +10 -0

index db45134..748ab6f 100644
@@ -161,6 +161,16 @@ enum power_event {
 #define GMAC_HI_REG_AE			BIT(31)
 
 /*  MTL registers */
+#define MTL_OPERATION_MODE		0x00000c00
+#define MTL_OPERATION_SCHALG_MASK	GENMASK(6, 5)
+#define MTL_OPERATION_SCHALG_WRR	(0x0 << 5)
+#define MTL_OPERATION_SCHALG_WFQ	(0x1 << 5)
+#define MTL_OPERATION_SCHALG_DWRR	(0x2 << 5)
+#define MTL_OPERATION_SCHALG_SP		(0x3 << 5)
+#define MTL_OPERATION_RAA		BIT(2)
+#define MTL_OPERATION_RAA_SP		(0x0 << 2)
+#define MTL_OPERATION_RAA_WSP		(0x1 << 2)
+
 #define MTL_INT_STATUS			0x00000c20
 #define MTL_INT_Q0			BIT(0)
 

drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +48 -0

index 1e79e65..f966755 100644
@@ -70,6 +70,52 @@ static void dwmac4_rx_queue_enable(struct mac_device_info *hw, u32 queue)
 	writel(value, ioaddr + GMAC_RXQ_CTRL0);
 }
 
+static void dwmac4_prog_mtl_rx_algorithms(struct mac_device_info *hw,
+					  u32 rx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_RAA;
+	switch (rx_alg) {
+	case MTL_RX_ALGORITHM_SP:
+		value |= MTL_OPERATION_RAA_SP;
+		break;
+	case MTL_RX_ALGORITHM_WSP:
+		value |= MTL_OPERATION_RAA_WSP;
+		break;
+	default:
+		break;
+	}
+
+	writel(value, ioaddr + MTL_OPERATION_MODE);
+}
+
+static void dwmac4_prog_mtl_tx_algorithms(struct mac_device_info *hw,
+					  u32 tx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_SCHALG_MASK;
+	switch (tx_alg) {
+	case MTL_TX_ALGORITHM_WRR:
+		value |= MTL_OPERATION_SCHALG_WRR;
+		break;
+	case MTL_TX_ALGORITHM_WFQ:
+		value |= MTL_OPERATION_SCHALG_WFQ;
+		break;
+	case MTL_TX_ALGORITHM_DWRR:
+		value |= MTL_OPERATION_SCHALG_DWRR;
+		break;
+	case MTL_TX_ALGORITHM_SP:
+		value |= MTL_OPERATION_SCHALG_SP;
+		break;
+	default:
+		break;
+	}
+}
+
 static void dwmac4_dump_regs(struct mac_device_info *hw, u32 *reg_space)
 {
 	void __iomem *ioaddr = hw->pcsr;

@@ -457,6 +503,8 @@ static const struct stmmac_ops dwmac4_ops = {
 	.core_init = dwmac4_core_init,
 	.rx_ipc = dwmac4_rx_ipc_enable,
 	.rx_queue_enable = dwmac4_rx_queue_enable,
+	.prog_mtl_rx_algorithms = dwmac4_prog_mtl_rx_algorithms,
+	.prog_mtl_tx_algorithms = dwmac4_prog_mtl_tx_algorithms,
 	.dump_regs = dwmac4_dump_regs,
 	.host_irq_status = dwmac4_irq_status,
 	.flow_ctrl = dwmac4_flow_ctrl,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +12 -0

index 4498a38..af57f8d 100644
@@ -1645,6 +1645,31 @@ static void stmmac_init_tx_coalesce(struct stmmac_priv *priv)
 }
 
 /**
+ *  stmmac_mtl_configuration - Configure MTL
+ *  @priv: driver private structure
+ *  Description: It is used for configurring MTL
+ */
+static void stmmac_mtl_configuration(struct stmmac_priv *priv)
+{
+	u32 rx_queues_count = priv->plat->rx_queues_to_use;
+	u32 tx_queues_count = priv->plat->tx_queues_to_use;
+
+	/* Configure MTL RX algorithms */
+	if (rx_queues_count > 1 && priv->hw->mac->prog_mtl_rx_algorithms)
+		priv->hw->mac->prog_mtl_rx_algorithms(priv->hw,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c static void stmmac_init_tx_coalesce(struct stmmac_priv *priv) · line 1669

Quoted 10 lines

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +10 -0

+						priv->plat->rx_sched_algorithm);
+
+	/* Configure MTL TX algorithms */
+	if (tx_queues_count > 1 && priv->hw->mac->prog_mtl_tx_algorithms)
+		priv->hw->mac->prog_mtl_tx_algorithms(priv->hw,
+						priv->plat->tx_sched_algorithm);
+
+	/* Enable MAC RX Queues */
+	if (rx_queues_count > 1 && priv->hw->mac->rx_queue_enable)
+		stmmac_mac_enable_rx_queues(priv);

This is almost equivalent to the code removed from stmmac_hw_setup() which happens to be the key for this driver to work for me. However, the code above adds an additional check for rx_queues_count > 1 which is going to be false for any existing DTB, because it is derived from the values retrieved from new device tree properties.

So I think for backwards compatibility we'd need something like this:

	if ((rx_queue_count == 0 || rx_queue_count > 1) &&
	    priv->hw->mac->rx_queue_enable)

But then I'm beginning to think maybe we don't need a check here at all because it would only prevent RX queue setup for rx_queue_count == 1 and I think it would still be legitimate to set it up even then.

Quoted 6 lines

stmmac_mac_enable_rx_queues() already checks for rx_count == 1, but that is derived from the number of RX queues derived from the feature registers and therefore refers to the number of queues that the hardware supports as opposed to the number of queues configured in device tree.

I can follow up with a patch to restore backwards-compatibility.

Forhw configured as single queue you don't need to enable the rx queue, since they are enable by default. if you check in stmmac_platform.c I assure backward compatibility by setting the number of rx and tx queues = 1 if nothing is declared. Please check here:

Quoted 1 line

https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_davem_net-2Dnext.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fplatform.c-23n156&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=tvYpf8cZr-Y6Zl-d7NlT2FEW7Gv5Bzs86GMIhZs4OSI&s=jA0P7-MXCVOtu262fF75EhUq5dfcDutzsYg8CAE6XQc&e=

Ah yes, I just ran across that trying to debug why a subsequent patch broke things again. I think the rx_queue_count > 1 condition is still wrong in the above because it will still fail for the backwards- compatibility case.

I still think any check other than priv->hw->mac->rx_queue_enable should be dropped when calling stmmac_mac_enable_rx_queues(). A later patch is going to move the check into that function anyway (via the for loop). I need to step through the series some more since there are a couple of other things I noticed that make the driver fail for Tegra186 now. Let me report back once I'm through all of it.

Thierry

Às 12:24 PM de 3/21/2017, Thierry Reding escreveu:

Quoted 191 lines · Joao Pinto

Quoted 189 lines

Às 11:58 AM de 3/21/2017, Thierry Reding escreveu:

Quoted 175 lines · Joao Pinto

Quoted 15 lines

This patch adds the RX and TX scheduling algorithms programming. It introduces the multiple queues configuration function (stmmac_mtl_configuration) in stmmac_main.

Signed-off-by Joao Pinto <[email protected]>

---
Changes v4->v5:
- patch title update (stmicro replaced by stmmac)
Changes v3->v4:
- Just to keep up with patch-set version
Changes v2->v3:
- Switch statements with a tab
Changes v1->v2:
- Just to keep up with patch-set version

Quoted 5 lines

 drivers/net/ethernet/stmicro/stmmac/common.h      |  4 ++
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h      | 10 +++++
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 48 +++++++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 31 +++++++++++++--
 4 files changed, 90 insertions(+), 3 deletions(-)

This patch breaks backwards-compatibility with DTBs that don't have an of the multiple queue properties.

See below...

Quoted 122 lines

drivers/net/ethernet/stmicro/stmmac/common.h +4 -0

index 04d9245..5a0a781 100644
@@ -455,6 +455,10 @@ struct stmmac_ops {
 	int (*rx_ipc)(struct mac_device_info *hw);
 	/* Enable RX Queues */
 	void (*rx_queue_enable)(struct mac_device_info *hw, u32 queue);
+	/* Program RX Algorithms */
+	void (*prog_mtl_rx_algorithms)(struct mac_device_info *hw, u32 rx_alg);
+	/* Program TX Algorithms */
+	void (*prog_mtl_tx_algorithms)(struct mac_device_info *hw, u32 tx_alg);
 	/* Dump MAC registers */
 	void (*dump_regs)(struct mac_device_info *hw, u32 *reg_space);
 	/* Handle extra events on specific interrupts hw dependent */

drivers/net/ethernet/stmicro/stmmac/dwmac4.h +10 -0

index db45134..748ab6f 100644
@@ -161,6 +161,16 @@ enum power_event {
 #define GMAC_HI_REG_AE			BIT(31)
 
 /*  MTL registers */
+#define MTL_OPERATION_MODE		0x00000c00
+#define MTL_OPERATION_SCHALG_MASK	GENMASK(6, 5)
+#define MTL_OPERATION_SCHALG_WRR	(0x0 << 5)
+#define MTL_OPERATION_SCHALG_WFQ	(0x1 << 5)
+#define MTL_OPERATION_SCHALG_DWRR	(0x2 << 5)
+#define MTL_OPERATION_SCHALG_SP		(0x3 << 5)
+#define MTL_OPERATION_RAA		BIT(2)
+#define MTL_OPERATION_RAA_SP		(0x0 << 2)
+#define MTL_OPERATION_RAA_WSP		(0x1 << 2)
+
 #define MTL_INT_STATUS			0x00000c20
 #define MTL_INT_Q0			BIT(0)
 

drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +48 -0

index 1e79e65..f966755 100644
@@ -70,6 +70,52 @@ static void dwmac4_rx_queue_enable(struct mac_device_info *hw, u32 queue)
 	writel(value, ioaddr + GMAC_RXQ_CTRL0);
 }
 
+static void dwmac4_prog_mtl_rx_algorithms(struct mac_device_info *hw,
+					  u32 rx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_RAA;
+	switch (rx_alg) {
+	case MTL_RX_ALGORITHM_SP:
+		value |= MTL_OPERATION_RAA_SP;
+		break;
+	case MTL_RX_ALGORITHM_WSP:
+		value |= MTL_OPERATION_RAA_WSP;
+		break;
+	default:
+		break;
+	}
+
+	writel(value, ioaddr + MTL_OPERATION_MODE);
+}
+
+static void dwmac4_prog_mtl_tx_algorithms(struct mac_device_info *hw,
+					  u32 tx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_SCHALG_MASK;
+	switch (tx_alg) {
+	case MTL_TX_ALGORITHM_WRR:
+		value |= MTL_OPERATION_SCHALG_WRR;
+		break;
+	case MTL_TX_ALGORITHM_WFQ:
+		value |= MTL_OPERATION_SCHALG_WFQ;
+		break;
+	case MTL_TX_ALGORITHM_DWRR:
+		value |= MTL_OPERATION_SCHALG_DWRR;
+		break;
+	case MTL_TX_ALGORITHM_SP:
+		value |= MTL_OPERATION_SCHALG_SP;
+		break;
+	default:
+		break;
+	}
+}
+
 static void dwmac4_dump_regs(struct mac_device_info *hw, u32 *reg_space)
 {
 	void __iomem *ioaddr = hw->pcsr;

@@ -457,6 +503,8 @@ static const struct stmmac_ops dwmac4_ops = {
 	.core_init = dwmac4_core_init,
 	.rx_ipc = dwmac4_rx_ipc_enable,
 	.rx_queue_enable = dwmac4_rx_queue_enable,
+	.prog_mtl_rx_algorithms = dwmac4_prog_mtl_rx_algorithms,
+	.prog_mtl_tx_algorithms = dwmac4_prog_mtl_tx_algorithms,
 	.dump_regs = dwmac4_dump_regs,
 	.host_irq_status = dwmac4_irq_status,
 	.flow_ctrl = dwmac4_flow_ctrl,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +12 -0

index 4498a38..af57f8d 100644
@@ -1645,6 +1645,31 @@ static void stmmac_init_tx_coalesce(struct stmmac_priv *priv)
 }
 
 /**
+ *  stmmac_mtl_configuration - Configure MTL
+ *  @priv: driver private structure
+ *  Description: It is used for configurring MTL
+ */
+static void stmmac_mtl_configuration(struct stmmac_priv *priv)
+{
+	u32 rx_queues_count = priv->plat->rx_queues_to_use;
+	u32 tx_queues_count = priv->plat->tx_queues_to_use;
+
+	/* Configure MTL RX algorithms */
+	if (rx_queues_count > 1 && priv->hw->mac->prog_mtl_rx_algorithms)
+		priv->hw->mac->prog_mtl_rx_algorithms(priv->hw,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c static void stmmac_init_tx_coalesce(struct stmmac_priv *priv) · line 1669

Quoted 10 lines

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +10 -0

+						priv->plat->rx_sched_algorithm);
+
+	/* Configure MTL TX algorithms */
+	if (tx_queues_count > 1 && priv->hw->mac->prog_mtl_tx_algorithms)
+		priv->hw->mac->prog_mtl_tx_algorithms(priv->hw,
+						priv->plat->tx_sched_algorithm);
+
+	/* Enable MAC RX Queues */
+	if (rx_queues_count > 1 && priv->hw->mac->rx_queue_enable)
+		stmmac_mac_enable_rx_queues(priv);

This is almost equivalent to the code removed from stmmac_hw_setup() which happens to be the key for this driver to work for me. However, the code above adds an additional check for rx_queues_count > 1 which is going to be false for any existing DTB, because it is derived from the values retrieved from new device tree properties.

So I think for backwards compatibility we'd need something like this:

	if ((rx_queue_count == 0 || rx_queue_count > 1) &&
	    priv->hw->mac->rx_queue_enable)

But then I'm beginning to think maybe we don't need a check here at all because it would only prevent RX queue setup for rx_queue_count == 1 and I think it would still be legitimate to set it up even then.

Quoted 6 lines

stmmac_mac_enable_rx_queues() already checks for rx_count == 1, but that is derived from the number of RX queues derived from the feature registers and therefore refers to the number of queues that the hardware supports as opposed to the number of queues configured in device tree.

I can follow up with a patch to restore backwards-compatibility.

Forhw configured as single queue you don't need to enable the rx queue, since they are enable by default. if you check in stmmac_platform.c I assure backward compatibility by setting the number of rx and tx queues = 1 if nothing is declared. Please check here:

https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_davem_net-2Dnext.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fplatform.c-23n156&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=tvYpf8cZr-Y6Zl-d7NlT2FEW7Gv5Bzs86GMIhZs4OSI&s=jA0P7-MXCVOtu262fF75EhUq5dfcDutzsYg8CAE6XQc&e=

Quoted 4 lines

Ah yes, I just ran across that trying to debug why a subsequent patch broke things again. I think the rx_queue_count > 1 condition is still wrong in the above because it will still fail for the backwards- compatibility case.

Enabling RX Queue is needed only if the hw is configured as having more than 1 RX queue. If your hardware is configured as being multiple-queue, rx_queue_to_use should be > 1. True single queue hardware will work fine.

Quoted 192 lines

Às 12:24 PM de 3/21/2017, Thierry Reding escreveu:

Quoted 191 lines · Joao Pinto

Quoted 189 lines

Às 11:58 AM de 3/21/2017, Thierry Reding escreveu:

Quoted 175 lines · Joao Pinto

Quoted 15 lines

This patch adds the RX and TX scheduling algorithms programming. It introduces the multiple queues configuration function (stmmac_mtl_configuration) in stmmac_main.

Signed-off-by Joao Pinto <[email protected]>

---
Changes v4->v5:
- patch title update (stmicro replaced by stmmac)
Changes v3->v4:
- Just to keep up with patch-set version
Changes v2->v3:
- Switch statements with a tab
Changes v1->v2:
- Just to keep up with patch-set version

Quoted 5 lines

 drivers/net/ethernet/stmicro/stmmac/common.h      |  4 ++
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h      | 10 +++++
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 48 +++++++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 31 +++++++++++++--
 4 files changed, 90 insertions(+), 3 deletions(-)

This patch breaks backwards-compatibility with DTBs that don't have an of the multiple queue properties.

See below...

Quoted 122 lines

drivers/net/ethernet/stmicro/stmmac/common.h +4 -0

index 04d9245..5a0a781 100644
@@ -455,6 +455,10 @@ struct stmmac_ops {
 	int (*rx_ipc)(struct mac_device_info *hw);
 	/* Enable RX Queues */
 	void (*rx_queue_enable)(struct mac_device_info *hw, u32 queue);
+	/* Program RX Algorithms */
+	void (*prog_mtl_rx_algorithms)(struct mac_device_info *hw, u32 rx_alg);
+	/* Program TX Algorithms */
+	void (*prog_mtl_tx_algorithms)(struct mac_device_info *hw, u32 tx_alg);
 	/* Dump MAC registers */
 	void (*dump_regs)(struct mac_device_info *hw, u32 *reg_space);
 	/* Handle extra events on specific interrupts hw dependent */

drivers/net/ethernet/stmicro/stmmac/dwmac4.h +10 -0

index db45134..748ab6f 100644
@@ -161,6 +161,16 @@ enum power_event {
 #define GMAC_HI_REG_AE			BIT(31)
 
 /*  MTL registers */
+#define MTL_OPERATION_MODE		0x00000c00
+#define MTL_OPERATION_SCHALG_MASK	GENMASK(6, 5)
+#define MTL_OPERATION_SCHALG_WRR	(0x0 << 5)
+#define MTL_OPERATION_SCHALG_WFQ	(0x1 << 5)
+#define MTL_OPERATION_SCHALG_DWRR	(0x2 << 5)
+#define MTL_OPERATION_SCHALG_SP		(0x3 << 5)
+#define MTL_OPERATION_RAA		BIT(2)
+#define MTL_OPERATION_RAA_SP		(0x0 << 2)
+#define MTL_OPERATION_RAA_WSP		(0x1 << 2)
+
 #define MTL_INT_STATUS			0x00000c20
 #define MTL_INT_Q0			BIT(0)
 

drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +48 -0

index 1e79e65..f966755 100644
@@ -70,6 +70,52 @@ static void dwmac4_rx_queue_enable(struct mac_device_info *hw, u32 queue)
 	writel(value, ioaddr + GMAC_RXQ_CTRL0);
 }
 
+static void dwmac4_prog_mtl_rx_algorithms(struct mac_device_info *hw,
+					  u32 rx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_RAA;
+	switch (rx_alg) {
+	case MTL_RX_ALGORITHM_SP:
+		value |= MTL_OPERATION_RAA_SP;
+		break;
+	case MTL_RX_ALGORITHM_WSP:
+		value |= MTL_OPERATION_RAA_WSP;
+		break;
+	default:
+		break;
+	}
+
+	writel(value, ioaddr + MTL_OPERATION_MODE);
+}
+
+static void dwmac4_prog_mtl_tx_algorithms(struct mac_device_info *hw,
+					  u32 tx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_SCHALG_MASK;
+	switch (tx_alg) {
+	case MTL_TX_ALGORITHM_WRR:
+		value |= MTL_OPERATION_SCHALG_WRR;
+		break;
+	case MTL_TX_ALGORITHM_WFQ:
+		value |= MTL_OPERATION_SCHALG_WFQ;
+		break;
+	case MTL_TX_ALGORITHM_DWRR:
+		value |= MTL_OPERATION_SCHALG_DWRR;
+		break;
+	case MTL_TX_ALGORITHM_SP:
+		value |= MTL_OPERATION_SCHALG_SP;
+		break;
+	default:
+		break;
+	}
+}
+
 static void dwmac4_dump_regs(struct mac_device_info *hw, u32 *reg_space)
 {
 	void __iomem *ioaddr = hw->pcsr;

@@ -457,6 +503,8 @@ static const struct stmmac_ops dwmac4_ops = {
 	.core_init = dwmac4_core_init,
 	.rx_ipc = dwmac4_rx_ipc_enable,
 	.rx_queue_enable = dwmac4_rx_queue_enable,
+	.prog_mtl_rx_algorithms = dwmac4_prog_mtl_rx_algorithms,
+	.prog_mtl_tx_algorithms = dwmac4_prog_mtl_tx_algorithms,
 	.dump_regs = dwmac4_dump_regs,
 	.host_irq_status = dwmac4_irq_status,
 	.flow_ctrl = dwmac4_flow_ctrl,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +12 -0

index 4498a38..af57f8d 100644
@@ -1645,6 +1645,31 @@ static void stmmac_init_tx_coalesce(struct stmmac_priv *priv)
 }
 
 /**
+ *  stmmac_mtl_configuration - Configure MTL
+ *  @priv: driver private structure
+ *  Description: It is used for configurring MTL
+ */
+static void stmmac_mtl_configuration(struct stmmac_priv *priv)
+{
+	u32 rx_queues_count = priv->plat->rx_queues_to_use;
+	u32 tx_queues_count = priv->plat->tx_queues_to_use;
+
+	/* Configure MTL RX algorithms */
+	if (rx_queues_count > 1 && priv->hw->mac->prog_mtl_rx_algorithms)
+		priv->hw->mac->prog_mtl_rx_algorithms(priv->hw,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c static void stmmac_init_tx_coalesce(struct stmmac_priv *priv) · line 1669

Quoted 10 lines

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +10 -0

+						priv->plat->rx_sched_algorithm);
+
+	/* Configure MTL TX algorithms */
+	if (tx_queues_count > 1 && priv->hw->mac->prog_mtl_tx_algorithms)
+		priv->hw->mac->prog_mtl_tx_algorithms(priv->hw,
+						priv->plat->tx_sched_algorithm);
+
+	/* Enable MAC RX Queues */
+	if (rx_queues_count > 1 && priv->hw->mac->rx_queue_enable)
+		stmmac_mac_enable_rx_queues(priv);

This is almost equivalent to the code removed from stmmac_hw_setup() which happens to be the key for this driver to work for me. However, the code above adds an additional check for rx_queues_count > 1 which is going to be false for any existing DTB, because it is derived from the values retrieved from new device tree properties.

So I think for backwards compatibility we'd need something like this:

	if ((rx_queue_count == 0 || rx_queue_count > 1) &&
	    priv->hw->mac->rx_queue_enable)

But then I'm beginning to think maybe we don't need a check here at all because it would only prevent RX queue setup for rx_queue_count == 1 and I think it would still be legitimate to set it up even then.

Quoted 6 lines

stmmac_mac_enable_rx_queues() already checks for rx_count == 1, but that is derived from the number of RX queues derived from the feature registers and therefore refers to the number of queues that the hardware supports as opposed to the number of queues configured in device tree.

I can follow up with a patch to restore backwards-compatibility.

Forhw configured as single queue you don't need to enable the rx queue, since they are enable by default. if you check in stmmac_platform.c I assure backward compatibility by setting the number of rx and tx queues = 1 if nothing is declared. Please check here:

https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_davem_net-2Dnext.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fplatform.c-23n156&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=tvYpf8cZr-Y6Zl-d7NlT2FEW7Gv5Bzs86GMIhZs4OSI&s=jA0P7-MXCVOtu262fF75EhUq5dfcDutzsYg8CAE6XQc&e=

Quoted 8 lines

Quoted 4 lines

Ah yes, I just ran across that trying to debug why a subsequent patch broke things again. I think the rx_queue_count > 1 condition is still wrong in the above because it will still fail for the backwards- compatibility case.

Enabling RX Queue is needed only if the hw is configured as having more than 1 RX queue. If your hardware is configured as being multiple-queue, rx_queue_to_use should be > 1. True single queue hardware will work fine.

But that's certainly not the case right now. The rx_queues_to_use field is initialized based on the new snps,rx-queues-to-use device tree property, or initialized to 1 in the absence of that property (and for PCI). Effectively that means all users of this driver will have their rx_queues_to_use field set to 1 until the DTS files are updated to set some other value. But that's not a requirement that we can impose. We still need to be able to deal with old device trees without regression.

Even without the backwards-compatibility argument, wouldn't it make sense to allow users to restrict usage to a single RX queue even on hardware that supports multiple ones? It certainly works fine in my case, I don't see any advantage in forbidding it.

Thierry

++Adding Corentin

Às 2:08 PM de 3/21/2017, Thierry Reding escreveu:

Quoted 210 lines · Joao Pinto

Quoted 192 lines

Às 12:24 PM de 3/21/2017, Thierry Reding escreveu:

Quoted 191 lines · Joao Pinto

Quoted 189 lines

Às 11:58 AM de 3/21/2017, Thierry Reding escreveu:

Quoted 175 lines · Joao Pinto

Quoted 15 lines

This patch adds the RX and TX scheduling algorithms programming. It introduces the multiple queues configuration function (stmmac_mtl_configuration) in stmmac_main.

Signed-off-by Joao Pinto <[email protected]>

---
Changes v4->v5:
- patch title update (stmicro replaced by stmmac)
Changes v3->v4:
- Just to keep up with patch-set version
Changes v2->v3:
- Switch statements with a tab
Changes v1->v2:
- Just to keep up with patch-set version

Quoted 5 lines

 drivers/net/ethernet/stmicro/stmmac/common.h      |  4 ++
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h      | 10 +++++
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 48 +++++++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 31 +++++++++++++--
 4 files changed, 90 insertions(+), 3 deletions(-)

This patch breaks backwards-compatibility with DTBs that don't have an of the multiple queue properties.

See below...

Quoted 122 lines

drivers/net/ethernet/stmicro/stmmac/common.h +4 -0

index 04d9245..5a0a781 100644
@@ -455,6 +455,10 @@ struct stmmac_ops {
 	int (*rx_ipc)(struct mac_device_info *hw);
 	/* Enable RX Queues */
 	void (*rx_queue_enable)(struct mac_device_info *hw, u32 queue);
+	/* Program RX Algorithms */
+	void (*prog_mtl_rx_algorithms)(struct mac_device_info *hw, u32 rx_alg);
+	/* Program TX Algorithms */
+	void (*prog_mtl_tx_algorithms)(struct mac_device_info *hw, u32 tx_alg);
 	/* Dump MAC registers */
 	void (*dump_regs)(struct mac_device_info *hw, u32 *reg_space);
 	/* Handle extra events on specific interrupts hw dependent */

drivers/net/ethernet/stmicro/stmmac/dwmac4.h +10 -0

index db45134..748ab6f 100644
@@ -161,6 +161,16 @@ enum power_event {
 #define GMAC_HI_REG_AE			BIT(31)
 
 /*  MTL registers */
+#define MTL_OPERATION_MODE		0x00000c00
+#define MTL_OPERATION_SCHALG_MASK	GENMASK(6, 5)
+#define MTL_OPERATION_SCHALG_WRR	(0x0 << 5)
+#define MTL_OPERATION_SCHALG_WFQ	(0x1 << 5)
+#define MTL_OPERATION_SCHALG_DWRR	(0x2 << 5)
+#define MTL_OPERATION_SCHALG_SP		(0x3 << 5)
+#define MTL_OPERATION_RAA		BIT(2)
+#define MTL_OPERATION_RAA_SP		(0x0 << 2)
+#define MTL_OPERATION_RAA_WSP		(0x1 << 2)
+
 #define MTL_INT_STATUS			0x00000c20
 #define MTL_INT_Q0			BIT(0)
 

drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +48 -0

index 1e79e65..f966755 100644
@@ -70,6 +70,52 @@ static void dwmac4_rx_queue_enable(struct mac_device_info *hw, u32 queue)
 	writel(value, ioaddr + GMAC_RXQ_CTRL0);
 }
 
+static void dwmac4_prog_mtl_rx_algorithms(struct mac_device_info *hw,
+					  u32 rx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_RAA;
+	switch (rx_alg) {
+	case MTL_RX_ALGORITHM_SP:
+		value |= MTL_OPERATION_RAA_SP;
+		break;
+	case MTL_RX_ALGORITHM_WSP:
+		value |= MTL_OPERATION_RAA_WSP;
+		break;
+	default:
+		break;
+	}
+
+	writel(value, ioaddr + MTL_OPERATION_MODE);
+}
+
+static void dwmac4_prog_mtl_tx_algorithms(struct mac_device_info *hw,
+					  u32 tx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_SCHALG_MASK;
+	switch (tx_alg) {
+	case MTL_TX_ALGORITHM_WRR:
+		value |= MTL_OPERATION_SCHALG_WRR;
+		break;
+	case MTL_TX_ALGORITHM_WFQ:
+		value |= MTL_OPERATION_SCHALG_WFQ;
+		break;
+	case MTL_TX_ALGORITHM_DWRR:
+		value |= MTL_OPERATION_SCHALG_DWRR;
+		break;
+	case MTL_TX_ALGORITHM_SP:
+		value |= MTL_OPERATION_SCHALG_SP;
+		break;
+	default:
+		break;
+	}
+}
+
 static void dwmac4_dump_regs(struct mac_device_info *hw, u32 *reg_space)
 {
 	void __iomem *ioaddr = hw->pcsr;

@@ -457,6 +503,8 @@ static const struct stmmac_ops dwmac4_ops = {
 	.core_init = dwmac4_core_init,
 	.rx_ipc = dwmac4_rx_ipc_enable,
 	.rx_queue_enable = dwmac4_rx_queue_enable,
+	.prog_mtl_rx_algorithms = dwmac4_prog_mtl_rx_algorithms,
+	.prog_mtl_tx_algorithms = dwmac4_prog_mtl_tx_algorithms,
 	.dump_regs = dwmac4_dump_regs,
 	.host_irq_status = dwmac4_irq_status,
 	.flow_ctrl = dwmac4_flow_ctrl,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +12 -0

index 4498a38..af57f8d 100644
@@ -1645,6 +1645,31 @@ static void stmmac_init_tx_coalesce(struct stmmac_priv *priv)
 }
 
 /**
+ *  stmmac_mtl_configuration - Configure MTL
+ *  @priv: driver private structure
+ *  Description: It is used for configurring MTL
+ */
+static void stmmac_mtl_configuration(struct stmmac_priv *priv)
+{
+	u32 rx_queues_count = priv->plat->rx_queues_to_use;
+	u32 tx_queues_count = priv->plat->tx_queues_to_use;
+
+	/* Configure MTL RX algorithms */
+	if (rx_queues_count > 1 && priv->hw->mac->prog_mtl_rx_algorithms)
+		priv->hw->mac->prog_mtl_rx_algorithms(priv->hw,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c static void stmmac_init_tx_coalesce(struct stmmac_priv *priv) · line 1669

Quoted 10 lines

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +10 -0

+						priv->plat->rx_sched_algorithm);
+
+	/* Configure MTL TX algorithms */
+	if (tx_queues_count > 1 && priv->hw->mac->prog_mtl_tx_algorithms)
+		priv->hw->mac->prog_mtl_tx_algorithms(priv->hw,
+						priv->plat->tx_sched_algorithm);
+
+	/* Enable MAC RX Queues */
+	if (rx_queues_count > 1 && priv->hw->mac->rx_queue_enable)
+		stmmac_mac_enable_rx_queues(priv);

This is almost equivalent to the code removed from stmmac_hw_setup() which happens to be the key for this driver to work for me. However, the code above adds an additional check for rx_queues_count > 1 which is going to be false for any existing DTB, because it is derived from the values retrieved from new device tree properties.

So I think for backwards compatibility we'd need something like this:

	if ((rx_queue_count == 0 || rx_queue_count > 1) &&
	    priv->hw->mac->rx_queue_enable)

But then I'm beginning to think maybe we don't need a check here at all because it would only prevent RX queue setup for rx_queue_count == 1 and I think it would still be legitimate to set it up even then.

Quoted 6 lines

stmmac_mac_enable_rx_queues() already checks for rx_count == 1, but that is derived from the number of RX queues derived from the feature registers and therefore refers to the number of queues that the hardware supports as opposed to the number of queues configured in device tree.

I can follow up with a patch to restore backwards-compatibility.

Forhw configured as single queue you don't need to enable the rx queue, since they are enable by default. if you check in stmmac_platform.c I assure backward compatibility by setting the number of rx and tx queues = 1 if nothing is declared. Please check here:

https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_davem_net-2Dnext.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fplatform.c-23n156&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=tvYpf8cZr-Y6Zl-d7NlT2FEW7Gv5Bzs86GMIhZs4OSI&s=jA0P7-MXCVOtu262fF75EhUq5dfcDutzsYg8CAE6XQc&e=

Quoted 8 lines

Quoted 4 lines

Ah yes, I just ran across that trying to debug why a subsequent patch broke things again. I think the rx_queue_count > 1 condition is still wrong in the above because it will still fail for the backwards- compatibility case.

Enabling RX Queue is needed only if the hw is configured as having more than 1 RX queue. If your hardware is configured as being multiple-queue, rx_queue_to_use should be > 1. True single queue hardware will work fine.

But that's certainly not the case right now. The rx_queues_to_use field is initialized based on the new snps,rx-queues-to-use device tree property, or initialized to 1 in the absence of that property (and for PCI). Effectively that means all users of this driver will have their rx_queues_to_use field set to 1 until the DTS files are updated to set some other value. But that's not a requirement that we can impose. We still need to be able to deal with old device trees without regression.

Quoted 4 lines

Even without the backwards-compatibility argument, wouldn't it make sense to allow users to restrict usage to a single RX queue even on hardware that supports multiple ones? It certainly works fine in my case, I don't see any advantage in forbidding it.

Corentin spotted the real problem! By mistake I restricted dma_op_mode configuration to IP Cores Version >= 4.00. Thank you Corentin for spotting that one! Do you want to make the fix or do you want me to do it and put the reported-by tag?

Quoted 218 lines

++Adding Corentin

Às 2:08 PM de 3/21/2017, Thierry Reding escreveu:

Quoted 210 lines · Joao Pinto

Quoted 192 lines

Às 12:24 PM de 3/21/2017, Thierry Reding escreveu:

Quoted 191 lines · Joao Pinto

Quoted 189 lines

Às 11:58 AM de 3/21/2017, Thierry Reding escreveu:

Quoted 175 lines · Joao Pinto

Quoted 15 lines

This patch adds the RX and TX scheduling algorithms programming. It introduces the multiple queues configuration function (stmmac_mtl_configuration) in stmmac_main.

Signed-off-by Joao Pinto <[email protected]>

---
Changes v4->v5:
- patch title update (stmicro replaced by stmmac)
Changes v3->v4:
- Just to keep up with patch-set version
Changes v2->v3:
- Switch statements with a tab
Changes v1->v2:
- Just to keep up with patch-set version

Quoted 5 lines

 drivers/net/ethernet/stmicro/stmmac/common.h      |  4 ++
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h      | 10 +++++
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 48 +++++++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 31 +++++++++++++--
 4 files changed, 90 insertions(+), 3 deletions(-)

This patch breaks backwards-compatibility with DTBs that don't have an of the multiple queue properties.

See below...

Quoted 122 lines

drivers/net/ethernet/stmicro/stmmac/common.h +4 -0

index 04d9245..5a0a781 100644
@@ -455,6 +455,10 @@ struct stmmac_ops {
 	int (*rx_ipc)(struct mac_device_info *hw);
 	/* Enable RX Queues */
 	void (*rx_queue_enable)(struct mac_device_info *hw, u32 queue);
+	/* Program RX Algorithms */
+	void (*prog_mtl_rx_algorithms)(struct mac_device_info *hw, u32 rx_alg);
+	/* Program TX Algorithms */
+	void (*prog_mtl_tx_algorithms)(struct mac_device_info *hw, u32 tx_alg);
 	/* Dump MAC registers */
 	void (*dump_regs)(struct mac_device_info *hw, u32 *reg_space);
 	/* Handle extra events on specific interrupts hw dependent */

drivers/net/ethernet/stmicro/stmmac/dwmac4.h +10 -0

index db45134..748ab6f 100644
@@ -161,6 +161,16 @@ enum power_event {
 #define GMAC_HI_REG_AE			BIT(31)
 
 /*  MTL registers */
+#define MTL_OPERATION_MODE		0x00000c00
+#define MTL_OPERATION_SCHALG_MASK	GENMASK(6, 5)
+#define MTL_OPERATION_SCHALG_WRR	(0x0 << 5)
+#define MTL_OPERATION_SCHALG_WFQ	(0x1 << 5)
+#define MTL_OPERATION_SCHALG_DWRR	(0x2 << 5)
+#define MTL_OPERATION_SCHALG_SP		(0x3 << 5)
+#define MTL_OPERATION_RAA		BIT(2)
+#define MTL_OPERATION_RAA_SP		(0x0 << 2)
+#define MTL_OPERATION_RAA_WSP		(0x1 << 2)
+
 #define MTL_INT_STATUS			0x00000c20
 #define MTL_INT_Q0			BIT(0)
 

drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +48 -0

index 1e79e65..f966755 100644
@@ -70,6 +70,52 @@ static void dwmac4_rx_queue_enable(struct mac_device_info *hw, u32 queue)
 	writel(value, ioaddr + GMAC_RXQ_CTRL0);
 }
 
+static void dwmac4_prog_mtl_rx_algorithms(struct mac_device_info *hw,
+					  u32 rx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_RAA;
+	switch (rx_alg) {
+	case MTL_RX_ALGORITHM_SP:
+		value |= MTL_OPERATION_RAA_SP;
+		break;
+	case MTL_RX_ALGORITHM_WSP:
+		value |= MTL_OPERATION_RAA_WSP;
+		break;
+	default:
+		break;
+	}
+
+	writel(value, ioaddr + MTL_OPERATION_MODE);
+}
+
+static void dwmac4_prog_mtl_tx_algorithms(struct mac_device_info *hw,
+					  u32 tx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_SCHALG_MASK;
+	switch (tx_alg) {
+	case MTL_TX_ALGORITHM_WRR:
+		value |= MTL_OPERATION_SCHALG_WRR;
+		break;
+	case MTL_TX_ALGORITHM_WFQ:
+		value |= MTL_OPERATION_SCHALG_WFQ;
+		break;
+	case MTL_TX_ALGORITHM_DWRR:
+		value |= MTL_OPERATION_SCHALG_DWRR;
+		break;
+	case MTL_TX_ALGORITHM_SP:
+		value |= MTL_OPERATION_SCHALG_SP;
+		break;
+	default:
+		break;
+	}
+}
+
 static void dwmac4_dump_regs(struct mac_device_info *hw, u32 *reg_space)
 {
 	void __iomem *ioaddr = hw->pcsr;

@@ -457,6 +503,8 @@ static const struct stmmac_ops dwmac4_ops = {
 	.core_init = dwmac4_core_init,
 	.rx_ipc = dwmac4_rx_ipc_enable,
 	.rx_queue_enable = dwmac4_rx_queue_enable,
+	.prog_mtl_rx_algorithms = dwmac4_prog_mtl_rx_algorithms,
+	.prog_mtl_tx_algorithms = dwmac4_prog_mtl_tx_algorithms,
 	.dump_regs = dwmac4_dump_regs,
 	.host_irq_status = dwmac4_irq_status,
 	.flow_ctrl = dwmac4_flow_ctrl,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +12 -0

index 4498a38..af57f8d 100644
@@ -1645,6 +1645,31 @@ static void stmmac_init_tx_coalesce(struct stmmac_priv *priv)
 }
 
 /**
+ *  stmmac_mtl_configuration - Configure MTL
+ *  @priv: driver private structure
+ *  Description: It is used for configurring MTL
+ */
+static void stmmac_mtl_configuration(struct stmmac_priv *priv)
+{
+	u32 rx_queues_count = priv->plat->rx_queues_to_use;
+	u32 tx_queues_count = priv->plat->tx_queues_to_use;
+
+	/* Configure MTL RX algorithms */
+	if (rx_queues_count > 1 && priv->hw->mac->prog_mtl_rx_algorithms)
+		priv->hw->mac->prog_mtl_rx_algorithms(priv->hw,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c static void stmmac_init_tx_coalesce(struct stmmac_priv *priv) · line 1669

Quoted 10 lines

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +10 -0

+						priv->plat->rx_sched_algorithm);
+
+	/* Configure MTL TX algorithms */
+	if (tx_queues_count > 1 && priv->hw->mac->prog_mtl_tx_algorithms)
+		priv->hw->mac->prog_mtl_tx_algorithms(priv->hw,
+						priv->plat->tx_sched_algorithm);
+
+	/* Enable MAC RX Queues */
+	if (rx_queues_count > 1 && priv->hw->mac->rx_queue_enable)
+		stmmac_mac_enable_rx_queues(priv);

This is almost equivalent to the code removed from stmmac_hw_setup() which happens to be the key for this driver to work for me. However, the code above adds an additional check for rx_queues_count > 1 which is going to be false for any existing DTB, because it is derived from the values retrieved from new device tree properties.

So I think for backwards compatibility we'd need something like this:

	if ((rx_queue_count == 0 || rx_queue_count > 1) &&
	    priv->hw->mac->rx_queue_enable)

But then I'm beginning to think maybe we don't need a check here at all because it would only prevent RX queue setup for rx_queue_count == 1 and I think it would still be legitimate to set it up even then.

Quoted 6 lines

stmmac_mac_enable_rx_queues() already checks for rx_count == 1, but that is derived from the number of RX queues derived from the feature registers and therefore refers to the number of queues that the hardware supports as opposed to the number of queues configured in device tree.

I can follow up with a patch to restore backwards-compatibility.

Forhw configured as single queue you don't need to enable the rx queue, since they are enable by default. if you check in stmmac_platform.c I assure backward compatibility by setting the number of rx and tx queues = 1 if nothing is declared. Please check here:

https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_davem_net-2Dnext.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fplatform.c-23n156&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=tvYpf8cZr-Y6Zl-d7NlT2FEW7Gv5Bzs86GMIhZs4OSI&s=jA0P7-MXCVOtu262fF75EhUq5dfcDutzsYg8CAE6XQc&e=

Quoted 8 lines

Quoted 4 lines

Ah yes, I just ran across that trying to debug why a subsequent patch broke things again. I think the rx_queue_count > 1 condition is still wrong in the above because it will still fail for the backwards- compatibility case.

Enabling RX Queue is needed only if the hw is configured as having more than 1 RX queue. If your hardware is configured as being multiple-queue, rx_queue_to_use should be > 1. True single queue hardware will work fine.

But that's certainly not the case right now. The rx_queues_to_use field is initialized based on the new snps,rx-queues-to-use device tree property, or initialized to 1 in the absence of that property (and for PCI). Effectively that means all users of this driver will have their rx_queues_to_use field set to 1 until the DTS files are updated to set some other value. But that's not a requirement that we can impose. We still need to be able to deal with old device trees without regression.

Quoted 4 lines

Even without the backwards-compatibility argument, wouldn't it make sense to allow users to restrict usage to a single RX queue even on hardware that supports multiple ones? It certainly works fine in my case, I don't see any advantage in forbidding it.

Quoted 4 lines

Corentin spotted the real problem! By mistake I restricted dma_op_mode configuration to IP Cores Version >= 4.00. Thank you Corentin for spotting that one! Do you want to make the fix or do you want me to do it and put the reported-by tag?

reported-by would be enougth.

But I found also that rx_queues_to_use/tx_queues_to_use are not 1 by default.

The following code exit stmmac_mtl_setup() before rx_queues_to_use/tx_queues_to_use are set. rx_node = of_parse_phandle(pdev->dev.of_node, "snps,mtl-rx-config", 0);

The default value must be set elsewhere.

Às 2:23 PM de 3/21/2017, Corentin Labbe escreveu:

Quoted 228 lines · Joao Pinto

Quoted 218 lines

++Adding Corentin

Às 2:08 PM de 3/21/2017, Thierry Reding escreveu:

Quoted 210 lines · Joao Pinto

Quoted 192 lines

Às 12:24 PM de 3/21/2017, Thierry Reding escreveu:

Quoted 191 lines · Joao Pinto

Quoted 189 lines

Às 11:58 AM de 3/21/2017, Thierry Reding escreveu:

Quoted 175 lines · Joao Pinto

Quoted 15 lines

This patch adds the RX and TX scheduling algorithms programming. It introduces the multiple queues configuration function (stmmac_mtl_configuration) in stmmac_main.

Signed-off-by Joao Pinto <[email protected]>

---
Changes v4->v5:
- patch title update (stmicro replaced by stmmac)
Changes v3->v4:
- Just to keep up with patch-set version
Changes v2->v3:
- Switch statements with a tab
Changes v1->v2:
- Just to keep up with patch-set version

Quoted 5 lines

 drivers/net/ethernet/stmicro/stmmac/common.h      |  4 ++
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h      | 10 +++++
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 48 +++++++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 31 +++++++++++++--
 4 files changed, 90 insertions(+), 3 deletions(-)

This patch breaks backwards-compatibility with DTBs that don't have an of the multiple queue properties.

See below...

Quoted 122 lines

drivers/net/ethernet/stmicro/stmmac/common.h +4 -0

index 04d9245..5a0a781 100644
@@ -455,6 +455,10 @@ struct stmmac_ops {
 	int (*rx_ipc)(struct mac_device_info *hw);
 	/* Enable RX Queues */
 	void (*rx_queue_enable)(struct mac_device_info *hw, u32 queue);
+	/* Program RX Algorithms */
+	void (*prog_mtl_rx_algorithms)(struct mac_device_info *hw, u32 rx_alg);
+	/* Program TX Algorithms */
+	void (*prog_mtl_tx_algorithms)(struct mac_device_info *hw, u32 tx_alg);
 	/* Dump MAC registers */
 	void (*dump_regs)(struct mac_device_info *hw, u32 *reg_space);
 	/* Handle extra events on specific interrupts hw dependent */

drivers/net/ethernet/stmicro/stmmac/dwmac4.h +10 -0

index db45134..748ab6f 100644
@@ -161,6 +161,16 @@ enum power_event {
 #define GMAC_HI_REG_AE			BIT(31)
 
 /*  MTL registers */
+#define MTL_OPERATION_MODE		0x00000c00
+#define MTL_OPERATION_SCHALG_MASK	GENMASK(6, 5)
+#define MTL_OPERATION_SCHALG_WRR	(0x0 << 5)
+#define MTL_OPERATION_SCHALG_WFQ	(0x1 << 5)
+#define MTL_OPERATION_SCHALG_DWRR	(0x2 << 5)
+#define MTL_OPERATION_SCHALG_SP		(0x3 << 5)
+#define MTL_OPERATION_RAA		BIT(2)
+#define MTL_OPERATION_RAA_SP		(0x0 << 2)
+#define MTL_OPERATION_RAA_WSP		(0x1 << 2)
+
 #define MTL_INT_STATUS			0x00000c20
 #define MTL_INT_Q0			BIT(0)
 

drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +48 -0

index 1e79e65..f966755 100644
@@ -70,6 +70,52 @@ static void dwmac4_rx_queue_enable(struct mac_device_info *hw, u32 queue)
 	writel(value, ioaddr + GMAC_RXQ_CTRL0);
 }
 
+static void dwmac4_prog_mtl_rx_algorithms(struct mac_device_info *hw,
+					  u32 rx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_RAA;
+	switch (rx_alg) {
+	case MTL_RX_ALGORITHM_SP:
+		value |= MTL_OPERATION_RAA_SP;
+		break;
+	case MTL_RX_ALGORITHM_WSP:
+		value |= MTL_OPERATION_RAA_WSP;
+		break;
+	default:
+		break;
+	}
+
+	writel(value, ioaddr + MTL_OPERATION_MODE);
+}
+
+static void dwmac4_prog_mtl_tx_algorithms(struct mac_device_info *hw,
+					  u32 tx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_SCHALG_MASK;
+	switch (tx_alg) {
+	case MTL_TX_ALGORITHM_WRR:
+		value |= MTL_OPERATION_SCHALG_WRR;
+		break;
+	case MTL_TX_ALGORITHM_WFQ:
+		value |= MTL_OPERATION_SCHALG_WFQ;
+		break;
+	case MTL_TX_ALGORITHM_DWRR:
+		value |= MTL_OPERATION_SCHALG_DWRR;
+		break;
+	case MTL_TX_ALGORITHM_SP:
+		value |= MTL_OPERATION_SCHALG_SP;
+		break;
+	default:
+		break;
+	}
+}
+
 static void dwmac4_dump_regs(struct mac_device_info *hw, u32 *reg_space)
 {
 	void __iomem *ioaddr = hw->pcsr;

@@ -457,6 +503,8 @@ static const struct stmmac_ops dwmac4_ops = {
 	.core_init = dwmac4_core_init,
 	.rx_ipc = dwmac4_rx_ipc_enable,
 	.rx_queue_enable = dwmac4_rx_queue_enable,
+	.prog_mtl_rx_algorithms = dwmac4_prog_mtl_rx_algorithms,
+	.prog_mtl_tx_algorithms = dwmac4_prog_mtl_tx_algorithms,
 	.dump_regs = dwmac4_dump_regs,
 	.host_irq_status = dwmac4_irq_status,
 	.flow_ctrl = dwmac4_flow_ctrl,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +12 -0

index 4498a38..af57f8d 100644
@@ -1645,6 +1645,31 @@ static void stmmac_init_tx_coalesce(struct stmmac_priv *priv)
 }
 
 /**
+ *  stmmac_mtl_configuration - Configure MTL
+ *  @priv: driver private structure
+ *  Description: It is used for configurring MTL
+ */
+static void stmmac_mtl_configuration(struct stmmac_priv *priv)
+{
+	u32 rx_queues_count = priv->plat->rx_queues_to_use;
+	u32 tx_queues_count = priv->plat->tx_queues_to_use;
+
+	/* Configure MTL RX algorithms */
+	if (rx_queues_count > 1 && priv->hw->mac->prog_mtl_rx_algorithms)
+		priv->hw->mac->prog_mtl_rx_algorithms(priv->hw,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c static void stmmac_init_tx_coalesce(struct stmmac_priv *priv) · line 1669

Quoted 10 lines

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +10 -0

+						priv->plat->rx_sched_algorithm);
+
+	/* Configure MTL TX algorithms */
+	if (tx_queues_count > 1 && priv->hw->mac->prog_mtl_tx_algorithms)
+		priv->hw->mac->prog_mtl_tx_algorithms(priv->hw,
+						priv->plat->tx_sched_algorithm);
+
+	/* Enable MAC RX Queues */
+	if (rx_queues_count > 1 && priv->hw->mac->rx_queue_enable)
+		stmmac_mac_enable_rx_queues(priv);

This is almost equivalent to the code removed from stmmac_hw_setup() which happens to be the key for this driver to work for me. However, the code above adds an additional check for rx_queues_count > 1 which is going to be false for any existing DTB, because it is derived from the values retrieved from new device tree properties.

So I think for backwards compatibility we'd need something like this:

	if ((rx_queue_count == 0 || rx_queue_count > 1) &&
	    priv->hw->mac->rx_queue_enable)

But then I'm beginning to think maybe we don't need a check here at all because it would only prevent RX queue setup for rx_queue_count == 1 and I think it would still be legitimate to set it up even then.

Quoted 6 lines

stmmac_mac_enable_rx_queues() already checks for rx_count == 1, but that is derived from the number of RX queues derived from the feature registers and therefore refers to the number of queues that the hardware supports as opposed to the number of queues configured in device tree.

I can follow up with a patch to restore backwards-compatibility.

Forhw configured as single queue you don't need to enable the rx queue, since they are enable by default. if you check in stmmac_platform.c I assure backward compatibility by setting the number of rx and tx queues = 1 if nothing is declared. Please check here:

https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_davem_net-2Dnext.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fplatform.c-23n156&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=tvYpf8cZr-Y6Zl-d7NlT2FEW7Gv5Bzs86GMIhZs4OSI&s=jA0P7-MXCVOtu262fF75EhUq5dfcDutzsYg8CAE6XQc&e=

Quoted 8 lines

Quoted 4 lines

Ah yes, I just ran across that trying to debug why a subsequent patch broke things again. I think the rx_queue_count > 1 condition is still wrong in the above because it will still fail for the backwards- compatibility case.

Enabling RX Queue is needed only if the hw is configured as having more than 1 RX queue. If your hardware is configured as being multiple-queue, rx_queue_to_use should be > 1. True single queue hardware will work fine.

But that's certainly not the case right now. The rx_queues_to_use field is initialized based on the new snps,rx-queues-to-use device tree property, or initialized to 1 in the absence of that property (and for PCI). Effectively that means all users of this driver will have their rx_queues_to_use field set to 1 until the DTS files are updated to set some other value. But that's not a requirement that we can impose. We still need to be able to deal with old device trees without regression.

Quoted 4 lines

Even without the backwards-compatibility argument, wouldn't it make sense to allow users to restrict usage to a single RX queue even on hardware that supports multiple ones? It certainly works fine in my case, I don't see any advantage in forbidding it.

Quoted 4 lines

Corentin spotted the real problem! By mistake I restricted dma_op_mode configuration to IP Cores Version >= 4.00. Thank you Corentin for spotting that one! Do you want to make the fix or do you want me to do it and put the reported-by tag?

reported-by would be enougth.

But I found also that rx_queues_to_use/tx_queues_to_use are not 1 by default.

Quoted 5 lines

The following code exit stmmac_mtl_setup() before rx_queues_to_use/tx_queues_to_use are set. rx_node = of_parse_phandle(pdev->dev.of_node, "snps,mtl-rx-config", 0);

The default value must be set elsewhere.

Yes you are right, I just have a pci setup, so I was not able to test it sorry. I will fix that and add you to the thread. Thanks for checking that!

Quoted 235 lines

Às 2:23 PM de 3/21/2017, Corentin Labbe escreveu:

Quoted 233 lines · Joao Pinto

Quoted 218 lines

++Adding Corentin

Às 2:08 PM de 3/21/2017, Thierry Reding escreveu:

Quoted 210 lines · Joao Pinto

Quoted 192 lines

Às 12:24 PM de 3/21/2017, Thierry Reding escreveu:

Quoted 191 lines · Joao Pinto

Quoted 189 lines

Às 11:58 AM de 3/21/2017, Thierry Reding escreveu:

Quoted 175 lines · Joao Pinto

Quoted 15 lines

This patch adds the RX and TX scheduling algorithms programming. It introduces the multiple queues configuration function (stmmac_mtl_configuration) in stmmac_main.

Signed-off-by Joao Pinto <[email protected]>

---
Changes v4->v5:
- patch title update (stmicro replaced by stmmac)
Changes v3->v4:
- Just to keep up with patch-set version
Changes v2->v3:
- Switch statements with a tab
Changes v1->v2:
- Just to keep up with patch-set version

Quoted 5 lines

 drivers/net/ethernet/stmicro/stmmac/common.h      |  4 ++
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h      | 10 +++++
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 48 +++++++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 31 +++++++++++++--
 4 files changed, 90 insertions(+), 3 deletions(-)

This patch breaks backwards-compatibility with DTBs that don't have an of the multiple queue properties.

See below...

Quoted 122 lines

drivers/net/ethernet/stmicro/stmmac/common.h +4 -0

index 04d9245..5a0a781 100644
@@ -455,6 +455,10 @@ struct stmmac_ops {
 	int (*rx_ipc)(struct mac_device_info *hw);
 	/* Enable RX Queues */
 	void (*rx_queue_enable)(struct mac_device_info *hw, u32 queue);
+	/* Program RX Algorithms */
+	void (*prog_mtl_rx_algorithms)(struct mac_device_info *hw, u32 rx_alg);
+	/* Program TX Algorithms */
+	void (*prog_mtl_tx_algorithms)(struct mac_device_info *hw, u32 tx_alg);
 	/* Dump MAC registers */
 	void (*dump_regs)(struct mac_device_info *hw, u32 *reg_space);
 	/* Handle extra events on specific interrupts hw dependent */

drivers/net/ethernet/stmicro/stmmac/dwmac4.h +10 -0

index db45134..748ab6f 100644
@@ -161,6 +161,16 @@ enum power_event {
 #define GMAC_HI_REG_AE			BIT(31)
 
 /*  MTL registers */
+#define MTL_OPERATION_MODE		0x00000c00
+#define MTL_OPERATION_SCHALG_MASK	GENMASK(6, 5)
+#define MTL_OPERATION_SCHALG_WRR	(0x0 << 5)
+#define MTL_OPERATION_SCHALG_WFQ	(0x1 << 5)
+#define MTL_OPERATION_SCHALG_DWRR	(0x2 << 5)
+#define MTL_OPERATION_SCHALG_SP		(0x3 << 5)
+#define MTL_OPERATION_RAA		BIT(2)
+#define MTL_OPERATION_RAA_SP		(0x0 << 2)
+#define MTL_OPERATION_RAA_WSP		(0x1 << 2)
+
 #define MTL_INT_STATUS			0x00000c20
 #define MTL_INT_Q0			BIT(0)
 

drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +48 -0

index 1e79e65..f966755 100644
@@ -70,6 +70,52 @@ static void dwmac4_rx_queue_enable(struct mac_device_info *hw, u32 queue)
 	writel(value, ioaddr + GMAC_RXQ_CTRL0);
 }
 
+static void dwmac4_prog_mtl_rx_algorithms(struct mac_device_info *hw,
+					  u32 rx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_RAA;
+	switch (rx_alg) {
+	case MTL_RX_ALGORITHM_SP:
+		value |= MTL_OPERATION_RAA_SP;
+		break;
+	case MTL_RX_ALGORITHM_WSP:
+		value |= MTL_OPERATION_RAA_WSP;
+		break;
+	default:
+		break;
+	}
+
+	writel(value, ioaddr + MTL_OPERATION_MODE);
+}
+
+static void dwmac4_prog_mtl_tx_algorithms(struct mac_device_info *hw,
+					  u32 tx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_SCHALG_MASK;
+	switch (tx_alg) {
+	case MTL_TX_ALGORITHM_WRR:
+		value |= MTL_OPERATION_SCHALG_WRR;
+		break;
+	case MTL_TX_ALGORITHM_WFQ:
+		value |= MTL_OPERATION_SCHALG_WFQ;
+		break;
+	case MTL_TX_ALGORITHM_DWRR:
+		value |= MTL_OPERATION_SCHALG_DWRR;
+		break;
+	case MTL_TX_ALGORITHM_SP:
+		value |= MTL_OPERATION_SCHALG_SP;
+		break;
+	default:
+		break;
+	}
+}
+
 static void dwmac4_dump_regs(struct mac_device_info *hw, u32 *reg_space)
 {
 	void __iomem *ioaddr = hw->pcsr;

@@ -457,6 +503,8 @@ static const struct stmmac_ops dwmac4_ops = {
 	.core_init = dwmac4_core_init,
 	.rx_ipc = dwmac4_rx_ipc_enable,
 	.rx_queue_enable = dwmac4_rx_queue_enable,
+	.prog_mtl_rx_algorithms = dwmac4_prog_mtl_rx_algorithms,
+	.prog_mtl_tx_algorithms = dwmac4_prog_mtl_tx_algorithms,
 	.dump_regs = dwmac4_dump_regs,
 	.host_irq_status = dwmac4_irq_status,
 	.flow_ctrl = dwmac4_flow_ctrl,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +12 -0

index 4498a38..af57f8d 100644
@@ -1645,6 +1645,31 @@ static void stmmac_init_tx_coalesce(struct stmmac_priv *priv)
 }
 
 /**
+ *  stmmac_mtl_configuration - Configure MTL
+ *  @priv: driver private structure
+ *  Description: It is used for configurring MTL
+ */
+static void stmmac_mtl_configuration(struct stmmac_priv *priv)
+{
+	u32 rx_queues_count = priv->plat->rx_queues_to_use;
+	u32 tx_queues_count = priv->plat->tx_queues_to_use;
+
+	/* Configure MTL RX algorithms */
+	if (rx_queues_count > 1 && priv->hw->mac->prog_mtl_rx_algorithms)
+		priv->hw->mac->prog_mtl_rx_algorithms(priv->hw,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c static void stmmac_init_tx_coalesce(struct stmmac_priv *priv) · line 1669

Quoted 10 lines

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +10 -0

+						priv->plat->rx_sched_algorithm);
+
+	/* Configure MTL TX algorithms */
+	if (tx_queues_count > 1 && priv->hw->mac->prog_mtl_tx_algorithms)
+		priv->hw->mac->prog_mtl_tx_algorithms(priv->hw,
+						priv->plat->tx_sched_algorithm);
+
+	/* Enable MAC RX Queues */
+	if (rx_queues_count > 1 && priv->hw->mac->rx_queue_enable)
+		stmmac_mac_enable_rx_queues(priv);

This is almost equivalent to the code removed from stmmac_hw_setup() which happens to be the key for this driver to work for me. However, the code above adds an additional check for rx_queues_count > 1 which is going to be false for any existing DTB, because it is derived from the values retrieved from new device tree properties.

So I think for backwards compatibility we'd need something like this:

	if ((rx_queue_count == 0 || rx_queue_count > 1) &&
	    priv->hw->mac->rx_queue_enable)

But then I'm beginning to think maybe we don't need a check here at all because it would only prevent RX queue setup for rx_queue_count == 1 and I think it would still be legitimate to set it up even then.

Quoted 6 lines

stmmac_mac_enable_rx_queues() already checks for rx_count == 1, but that is derived from the number of RX queues derived from the feature registers and therefore refers to the number of queues that the hardware supports as opposed to the number of queues configured in device tree.

I can follow up with a patch to restore backwards-compatibility.

Forhw configured as single queue you don't need to enable the rx queue, since they are enable by default. if you check in stmmac_platform.c I assure backward compatibility by setting the number of rx and tx queues = 1 if nothing is declared. Please check here:

https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_davem_net-2Dnext.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fplatform.c-23n156&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=tvYpf8cZr-Y6Zl-d7NlT2FEW7Gv5Bzs86GMIhZs4OSI&s=jA0P7-MXCVOtu262fF75EhUq5dfcDutzsYg8CAE6XQc&e=

Quoted 8 lines

Quoted 4 lines

Ah yes, I just ran across that trying to debug why a subsequent patch broke things again. I think the rx_queue_count > 1 condition is still wrong in the above because it will still fail for the backwards- compatibility case.

Enabling RX Queue is needed only if the hw is configured as having more than 1 RX queue. If your hardware is configured as being multiple-queue, rx_queue_to_use should be > 1. True single queue hardware will work fine.

But that's certainly not the case right now. The rx_queues_to_use field is initialized based on the new snps,rx-queues-to-use device tree property, or initialized to 1 in the absence of that property (and for PCI). Effectively that means all users of this driver will have their rx_queues_to_use field set to 1 until the DTS files are updated to set some other value. But that's not a requirement that we can impose. We still need to be able to deal with old device trees without regression.

Quoted 4 lines

Even without the backwards-compatibility argument, wouldn't it make sense to allow users to restrict usage to a single RX queue even on hardware that supports multiple ones? It certainly works fine in my case, I don't see any advantage in forbidding it.

Quoted 4 lines

Corentin spotted the real problem! By mistake I restricted dma_op_mode configuration to IP Cores Version >= 4.00. Thank you Corentin for spotting that one! Do you want to make the fix or do you want me to do it and put the reported-by tag?

reported-by would be enougth.

But I found also that rx_queues_to_use/tx_queues_to_use are not 1 by default.

The following code exit stmmac_mtl_setup() before rx_queues_to_use/tx_queues_to_use are set. rx_node = of_parse_phandle(pdev->dev.of_node, "snps,mtl-rx-config", 0);

The default value must be set elsewhere.

Quoted 2 lines

Yes you are right, I just have a pci setup, so I was not able to test it sorry. I will fix that and add you to the thread. Thanks for checking that!

I've got a set of patches that fix all these issues and that I've tested on Tegra186. Will send out soon.

Thierry

Quoted 192 lines

Às 12:24 PM de 3/21/2017, Thierry Reding escreveu:

Quoted 191 lines · Joao Pinto

Quoted 189 lines

Às 11:58 AM de 3/21/2017, Thierry Reding escreveu:

Quoted 175 lines · Joao Pinto

Quoted 15 lines

This patch adds the RX and TX scheduling algorithms programming. It introduces the multiple queues configuration function (stmmac_mtl_configuration) in stmmac_main.

Signed-off-by Joao Pinto <[email protected]>

---
Changes v4->v5:
- patch title update (stmicro replaced by stmmac)
Changes v3->v4:
- Just to keep up with patch-set version
Changes v2->v3:
- Switch statements with a tab
Changes v1->v2:
- Just to keep up with patch-set version

Quoted 5 lines

 drivers/net/ethernet/stmicro/stmmac/common.h      |  4 ++
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h      | 10 +++++
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 48 +++++++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 31 +++++++++++++--
 4 files changed, 90 insertions(+), 3 deletions(-)

This patch breaks backwards-compatibility with DTBs that don't have an of the multiple queue properties.

See below...

Quoted 122 lines

drivers/net/ethernet/stmicro/stmmac/common.h +4 -0

index 04d9245..5a0a781 100644
@@ -455,6 +455,10 @@ struct stmmac_ops {
 	int (*rx_ipc)(struct mac_device_info *hw);
 	/* Enable RX Queues */
 	void (*rx_queue_enable)(struct mac_device_info *hw, u32 queue);
+	/* Program RX Algorithms */
+	void (*prog_mtl_rx_algorithms)(struct mac_device_info *hw, u32 rx_alg);
+	/* Program TX Algorithms */
+	void (*prog_mtl_tx_algorithms)(struct mac_device_info *hw, u32 tx_alg);
 	/* Dump MAC registers */
 	void (*dump_regs)(struct mac_device_info *hw, u32 *reg_space);
 	/* Handle extra events on specific interrupts hw dependent */

drivers/net/ethernet/stmicro/stmmac/dwmac4.h +10 -0

index db45134..748ab6f 100644
@@ -161,6 +161,16 @@ enum power_event {
 #define GMAC_HI_REG_AE			BIT(31)
 
 /*  MTL registers */
+#define MTL_OPERATION_MODE		0x00000c00
+#define MTL_OPERATION_SCHALG_MASK	GENMASK(6, 5)
+#define MTL_OPERATION_SCHALG_WRR	(0x0 << 5)
+#define MTL_OPERATION_SCHALG_WFQ	(0x1 << 5)
+#define MTL_OPERATION_SCHALG_DWRR	(0x2 << 5)
+#define MTL_OPERATION_SCHALG_SP		(0x3 << 5)
+#define MTL_OPERATION_RAA		BIT(2)
+#define MTL_OPERATION_RAA_SP		(0x0 << 2)
+#define MTL_OPERATION_RAA_WSP		(0x1 << 2)
+
 #define MTL_INT_STATUS			0x00000c20
 #define MTL_INT_Q0			BIT(0)
 

drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +48 -0

index 1e79e65..f966755 100644
@@ -70,6 +70,52 @@ static void dwmac4_rx_queue_enable(struct mac_device_info *hw, u32 queue)
 	writel(value, ioaddr + GMAC_RXQ_CTRL0);
 }
 
+static void dwmac4_prog_mtl_rx_algorithms(struct mac_device_info *hw,
+					  u32 rx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_RAA;
+	switch (rx_alg) {
+	case MTL_RX_ALGORITHM_SP:
+		value |= MTL_OPERATION_RAA_SP;
+		break;
+	case MTL_RX_ALGORITHM_WSP:
+		value |= MTL_OPERATION_RAA_WSP;
+		break;
+	default:
+		break;
+	}
+
+	writel(value, ioaddr + MTL_OPERATION_MODE);
+}
+
+static void dwmac4_prog_mtl_tx_algorithms(struct mac_device_info *hw,
+					  u32 tx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_SCHALG_MASK;
+	switch (tx_alg) {
+	case MTL_TX_ALGORITHM_WRR:
+		value |= MTL_OPERATION_SCHALG_WRR;
+		break;
+	case MTL_TX_ALGORITHM_WFQ:
+		value |= MTL_OPERATION_SCHALG_WFQ;
+		break;
+	case MTL_TX_ALGORITHM_DWRR:
+		value |= MTL_OPERATION_SCHALG_DWRR;
+		break;
+	case MTL_TX_ALGORITHM_SP:
+		value |= MTL_OPERATION_SCHALG_SP;
+		break;
+	default:
+		break;
+	}
+}
+
 static void dwmac4_dump_regs(struct mac_device_info *hw, u32 *reg_space)
 {
 	void __iomem *ioaddr = hw->pcsr;

@@ -457,6 +503,8 @@ static const struct stmmac_ops dwmac4_ops = {
 	.core_init = dwmac4_core_init,
 	.rx_ipc = dwmac4_rx_ipc_enable,
 	.rx_queue_enable = dwmac4_rx_queue_enable,
+	.prog_mtl_rx_algorithms = dwmac4_prog_mtl_rx_algorithms,
+	.prog_mtl_tx_algorithms = dwmac4_prog_mtl_tx_algorithms,
 	.dump_regs = dwmac4_dump_regs,
 	.host_irq_status = dwmac4_irq_status,
 	.flow_ctrl = dwmac4_flow_ctrl,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +12 -0

index 4498a38..af57f8d 100644
@@ -1645,6 +1645,31 @@ static void stmmac_init_tx_coalesce(struct stmmac_priv *priv)
 }
 
 /**
+ *  stmmac_mtl_configuration - Configure MTL
+ *  @priv: driver private structure
+ *  Description: It is used for configurring MTL
+ */
+static void stmmac_mtl_configuration(struct stmmac_priv *priv)
+{
+	u32 rx_queues_count = priv->plat->rx_queues_to_use;
+	u32 tx_queues_count = priv->plat->tx_queues_to_use;
+
+	/* Configure MTL RX algorithms */
+	if (rx_queues_count > 1 && priv->hw->mac->prog_mtl_rx_algorithms)
+		priv->hw->mac->prog_mtl_rx_algorithms(priv->hw,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c static void stmmac_init_tx_coalesce(struct stmmac_priv *priv) · line 1669

Quoted 10 lines

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +10 -0

+						priv->plat->rx_sched_algorithm);
+
+	/* Configure MTL TX algorithms */
+	if (tx_queues_count > 1 && priv->hw->mac->prog_mtl_tx_algorithms)
+		priv->hw->mac->prog_mtl_tx_algorithms(priv->hw,
+						priv->plat->tx_sched_algorithm);
+
+	/* Enable MAC RX Queues */
+	if (rx_queues_count > 1 && priv->hw->mac->rx_queue_enable)
+		stmmac_mac_enable_rx_queues(priv);

This is almost equivalent to the code removed from stmmac_hw_setup() which happens to be the key for this driver to work for me. However, the code above adds an additional check for rx_queues_count > 1 which is going to be false for any existing DTB, because it is derived from the values retrieved from new device tree properties.

So I think for backwards compatibility we'd need something like this:

	if ((rx_queue_count == 0 || rx_queue_count > 1) &&
	    priv->hw->mac->rx_queue_enable)

But then I'm beginning to think maybe we don't need a check here at all because it would only prevent RX queue setup for rx_queue_count == 1 and I think it would still be legitimate to set it up even then.

Quoted 6 lines

stmmac_mac_enable_rx_queues() already checks for rx_count == 1, but that is derived from the number of RX queues derived from the feature registers and therefore refers to the number of queues that the hardware supports as opposed to the number of queues configured in device tree.

I can follow up with a patch to restore backwards-compatibility.

Forhw configured as single queue you don't need to enable the rx queue, since they are enable by default. if you check in stmmac_platform.c I assure backward compatibility by setting the number of rx and tx queues = 1 if nothing is declared. Please check here:

https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_davem_net-2Dnext.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fplatform.c-23n156&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=tvYpf8cZr-Y6Zl-d7NlT2FEW7Gv5Bzs86GMIhZs4OSI&s=jA0P7-MXCVOtu262fF75EhUq5dfcDutzsYg8CAE6XQc&e=

Quoted 8 lines

Quoted 4 lines

Ah yes, I just ran across that trying to debug why a subsequent patch broke things again. I think the rx_queue_count > 1 condition is still wrong in the above because it will still fail for the backwards- compatibility case.

Enabling RX Queue is needed only if the hw is configured as having more than 1 RX queue. If your hardware is configured as being multiple-queue, rx_queue_to_use should be > 1. True single queue hardware will work fine.

But that's certainly not the case right now. The rx_queues_to_use field is initialized based on the new snps,rx-queues-to-use device tree property, or initialized to 1 in the absence of that property (and for PCI). Effectively that means all users of this driver will have their rx_queues_to_use field set to 1 until the DTS files are updated to set some other value. But that's not a requirement that we can impose. We still need to be able to deal with old device trees without regression.

Even without the backwards-compatibility argument, wouldn't it make sense to allow users to restrict usage to a single RX queue even on hardware that supports multiple ones? It certainly works fine in my case, I don't see any advantage in forbidding it.

Thierry

Às 2:08 PM de 3/21/2017, Thierry Reding escreveu:

Quoted 210 lines · Joao Pinto

Quoted 192 lines

Às 12:24 PM de 3/21/2017, Thierry Reding escreveu:

Quoted 191 lines · Joao Pinto

Quoted 189 lines

Às 11:58 AM de 3/21/2017, Thierry Reding escreveu:

Quoted 175 lines · Joao Pinto

Quoted 15 lines

This patch adds the RX and TX scheduling algorithms programming. It introduces the multiple queues configuration function (stmmac_mtl_configuration) in stmmac_main.

Signed-off-by Joao Pinto <[email protected]>

---
Changes v4->v5:
- patch title update (stmicro replaced by stmmac)
Changes v3->v4:
- Just to keep up with patch-set version
Changes v2->v3:
- Switch statements with a tab
Changes v1->v2:
- Just to keep up with patch-set version

Quoted 5 lines

 drivers/net/ethernet/stmicro/stmmac/common.h      |  4 ++
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h      | 10 +++++
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 48 +++++++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 31 +++++++++++++--
 4 files changed, 90 insertions(+), 3 deletions(-)

This patch breaks backwards-compatibility with DTBs that don't have an of the multiple queue properties.

See below...

Quoted 122 lines

drivers/net/ethernet/stmicro/stmmac/common.h +4 -0

index 04d9245..5a0a781 100644
@@ -455,6 +455,10 @@ struct stmmac_ops {
 	int (*rx_ipc)(struct mac_device_info *hw);
 	/* Enable RX Queues */
 	void (*rx_queue_enable)(struct mac_device_info *hw, u32 queue);
+	/* Program RX Algorithms */
+	void (*prog_mtl_rx_algorithms)(struct mac_device_info *hw, u32 rx_alg);
+	/* Program TX Algorithms */
+	void (*prog_mtl_tx_algorithms)(struct mac_device_info *hw, u32 tx_alg);
 	/* Dump MAC registers */
 	void (*dump_regs)(struct mac_device_info *hw, u32 *reg_space);
 	/* Handle extra events on specific interrupts hw dependent */

drivers/net/ethernet/stmicro/stmmac/dwmac4.h +10 -0

index db45134..748ab6f 100644
@@ -161,6 +161,16 @@ enum power_event {
 #define GMAC_HI_REG_AE			BIT(31)
 
 /*  MTL registers */
+#define MTL_OPERATION_MODE		0x00000c00
+#define MTL_OPERATION_SCHALG_MASK	GENMASK(6, 5)
+#define MTL_OPERATION_SCHALG_WRR	(0x0 << 5)
+#define MTL_OPERATION_SCHALG_WFQ	(0x1 << 5)
+#define MTL_OPERATION_SCHALG_DWRR	(0x2 << 5)
+#define MTL_OPERATION_SCHALG_SP		(0x3 << 5)
+#define MTL_OPERATION_RAA		BIT(2)
+#define MTL_OPERATION_RAA_SP		(0x0 << 2)
+#define MTL_OPERATION_RAA_WSP		(0x1 << 2)
+
 #define MTL_INT_STATUS			0x00000c20
 #define MTL_INT_Q0			BIT(0)
 

drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +48 -0

index 1e79e65..f966755 100644
@@ -70,6 +70,52 @@ static void dwmac4_rx_queue_enable(struct mac_device_info *hw, u32 queue)
 	writel(value, ioaddr + GMAC_RXQ_CTRL0);
 }
 
+static void dwmac4_prog_mtl_rx_algorithms(struct mac_device_info *hw,
+					  u32 rx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_RAA;
+	switch (rx_alg) {
+	case MTL_RX_ALGORITHM_SP:
+		value |= MTL_OPERATION_RAA_SP;
+		break;
+	case MTL_RX_ALGORITHM_WSP:
+		value |= MTL_OPERATION_RAA_WSP;
+		break;
+	default:
+		break;
+	}
+
+	writel(value, ioaddr + MTL_OPERATION_MODE);
+}
+
+static void dwmac4_prog_mtl_tx_algorithms(struct mac_device_info *hw,
+					  u32 tx_alg)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_OPERATION_MODE);
+
+	value &= ~MTL_OPERATION_SCHALG_MASK;
+	switch (tx_alg) {
+	case MTL_TX_ALGORITHM_WRR:
+		value |= MTL_OPERATION_SCHALG_WRR;
+		break;
+	case MTL_TX_ALGORITHM_WFQ:
+		value |= MTL_OPERATION_SCHALG_WFQ;
+		break;
+	case MTL_TX_ALGORITHM_DWRR:
+		value |= MTL_OPERATION_SCHALG_DWRR;
+		break;
+	case MTL_TX_ALGORITHM_SP:
+		value |= MTL_OPERATION_SCHALG_SP;
+		break;
+	default:
+		break;
+	}
+}
+
 static void dwmac4_dump_regs(struct mac_device_info *hw, u32 *reg_space)
 {
 	void __iomem *ioaddr = hw->pcsr;

@@ -457,6 +503,8 @@ static const struct stmmac_ops dwmac4_ops = {
 	.core_init = dwmac4_core_init,
 	.rx_ipc = dwmac4_rx_ipc_enable,
 	.rx_queue_enable = dwmac4_rx_queue_enable,
+	.prog_mtl_rx_algorithms = dwmac4_prog_mtl_rx_algorithms,
+	.prog_mtl_tx_algorithms = dwmac4_prog_mtl_tx_algorithms,
 	.dump_regs = dwmac4_dump_regs,
 	.host_irq_status = dwmac4_irq_status,
 	.flow_ctrl = dwmac4_flow_ctrl,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +12 -0

index 4498a38..af57f8d 100644
@@ -1645,6 +1645,31 @@ static void stmmac_init_tx_coalesce(struct stmmac_priv *priv)
 }
 
 /**
+ *  stmmac_mtl_configuration - Configure MTL
+ *  @priv: driver private structure
+ *  Description: It is used for configurring MTL
+ */
+static void stmmac_mtl_configuration(struct stmmac_priv *priv)
+{
+	u32 rx_queues_count = priv->plat->rx_queues_to_use;
+	u32 tx_queues_count = priv->plat->tx_queues_to_use;
+
+	/* Configure MTL RX algorithms */
+	if (rx_queues_count > 1 && priv->hw->mac->prog_mtl_rx_algorithms)
+		priv->hw->mac->prog_mtl_rx_algorithms(priv->hw,

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c static void stmmac_init_tx_coalesce(struct stmmac_priv *priv) · line 1669

Quoted 10 lines

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +10 -0

+						priv->plat->rx_sched_algorithm);
+
+	/* Configure MTL TX algorithms */
+	if (tx_queues_count > 1 && priv->hw->mac->prog_mtl_tx_algorithms)
+		priv->hw->mac->prog_mtl_tx_algorithms(priv->hw,
+						priv->plat->tx_sched_algorithm);
+
+	/* Enable MAC RX Queues */
+	if (rx_queues_count > 1 && priv->hw->mac->rx_queue_enable)
+		stmmac_mac_enable_rx_queues(priv);

This is almost equivalent to the code removed from stmmac_hw_setup() which happens to be the key for this driver to work for me. However, the code above adds an additional check for rx_queues_count > 1 which is going to be false for any existing DTB, because it is derived from the values retrieved from new device tree properties.

So I think for backwards compatibility we'd need something like this:

	if ((rx_queue_count == 0 || rx_queue_count > 1) &&
	    priv->hw->mac->rx_queue_enable)

But then I'm beginning to think maybe we don't need a check here at all because it would only prevent RX queue setup for rx_queue_count == 1 and I think it would still be legitimate to set it up even then.

Quoted 6 lines

stmmac_mac_enable_rx_queues() already checks for rx_count == 1, but that is derived from the number of RX queues derived from the feature registers and therefore refers to the number of queues that the hardware supports as opposed to the number of queues configured in device tree.

I can follow up with a patch to restore backwards-compatibility.

Forhw configured as single queue you don't need to enable the rx queue, since they are enable by default. if you check in stmmac_platform.c I assure backward compatibility by setting the number of rx and tx queues = 1 if nothing is declared. Please check here:

https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_davem_net-2Dnext.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fplatform.c-23n156&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=tvYpf8cZr-Y6Zl-d7NlT2FEW7Gv5Bzs86GMIhZs4OSI&s=jA0P7-MXCVOtu262fF75EhUq5dfcDutzsYg8CAE6XQc&e=

Quoted 8 lines

Quoted 4 lines

Ah yes, I just ran across that trying to debug why a subsequent patch broke things again. I think the rx_queue_count > 1 condition is still wrong in the above because it will still fail for the backwards- compatibility case.

Enabling RX Queue is needed only if the hw is configured as having more than 1 RX queue. If your hardware is configured as being multiple-queue, rx_queue_to_use should be > 1. True single queue hardware will work fine.

But that's certainly not the case right now. The rx_queues_to_use field is initialized based on the new snps,rx-queues-to-use device tree property, or initialized to 1 in the absence of that property (and for PCI). Effectively that means all users of this driver will have their rx_queues_to_use field set to 1 until the DTS files are updated to set some other value. But that's not a requirement that we can impose. We still need to be able to deal with old device trees without regression.

Quoted 4 lines

Even without the backwards-compatibility argument, wouldn't it make sense to allow users to restrict usage to a single RX queue even on hardware that supports multiple ones? It certainly works fine in my case, I don't see any advantage in forbidding it.

Well lets see, stmmac works nowadays with a single queue and in the future it will remain workign like that for people that don't update the DT, so I see stability, not forbiding.

Of course we could always assume that RX queues = 8 and TX queues = 8 which is the max for the IP, it would be much easy, but in my opinion multiple queues should be used for now by choice since it is a recent added feature.

As agreed with David Miller, this patch-set is the first of 3 to enable multiple queues in stmmac.

This first one concentrates on mac operations adding functionalities as:
a) Configuration through DT
b) RX and TX scheduling algorithms programming
b) TX queues weight programming (essential in weightes algorithms)
c) RX enable as DCB or AVB (preparing for future AVB support)
d) Mapping RX queue to DMA channel
e) IRQ treatment prepared for multiple queues
f) Debug dump prepared for multiple queues
g) CBS configuration

In v3 patch-set version I included a new patch to enable CBS configuration (Patch 9).