netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 net-next 0/9] prepare mac operations for multiple queues
@ 2017-03-10 18:24 Joao Pinto
  2017-03-10 18:24 ` [PATCH v5 net-next 1/9] net: stmmac: multiple queues dt configuration Joao Pinto
                   ` (9 more replies)
  0 siblings, 10 replies; 26+ messages in thread
From: Joao Pinto @ 2017-03-10 18:24 UTC (permalink / raw)
  To: davem
  Cc: peppe.cavallaro, alexandre.torgue, niklas.cassel, netdev, Joao Pinto

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).

Joao Pinto (9):
  net: stmmac: multiple queues dt configuration
  net: stmmac: configure mtl rx and tx algorithms
  net: stmmac: configure tx queue weight
  net: stmmac: mtl rx queue enabled as dcb or avb
  net: stmmac: mapping mtl rx to dma channel
  net: stmmac: flow_ctrl functions adapted to mtl
  net: stmmac: prepare irq_status for mtl
  net: stmmac: mac debug prepared for multiple queues
  net: stmmac: configuration of CBS in case of a TX AVB queue

 Documentation/devicetree/bindings/net/stmmac.txt   |  58 +++-
 drivers/net/ethernet/stmicro/stmmac/common.h       |  22 +-
 .../net/ethernet/stmicro/stmmac/dwmac1000_core.c   |   6 +-
 .../net/ethernet/stmicro/stmmac/dwmac100_core.c    |   3 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h       |  59 +++-
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c  | 302 ++++++++++++++++-----
 .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c   |   8 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 142 ++++++++--
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  | 114 ++++++++
 include/linux/stmmac.h                             |  36 +++
 10 files changed, 647 insertions(+), 103 deletions(-)

-- 
2.9.3

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

* [PATCH v5 net-next 1/9] net: stmmac: multiple queues dt configuration
  2017-03-10 18:24 [PATCH v5 net-next 0/9] prepare mac operations for multiple queues Joao Pinto
@ 2017-03-10 18:24 ` Joao Pinto
       [not found]   ` <c315778b650a846db3c04816fd1d1041bb1e4043.1489169213.git.jpinto-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
  2017-03-10 18:24 ` [PATCH v5 net-next 2/9] net: stmmac: configure mtl rx and tx algorithms Joao Pinto
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 26+ messages in thread
From: Joao Pinto @ 2017-03-10 18:24 UTC (permalink / raw)
  To: davem
  Cc: peppe.cavallaro, alexandre.torgue, niklas.cassel, netdev, Joao Pinto

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@synopsys.com>
---
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(+)

diff --git a/Documentation/devicetree/bindings/net/stmmac.txt b/Documentation/devicetree/bindings/net/stmmac.txt
index d3bfc2b..4107e67 100644
--- a/Documentation/devicetree/bindings/net/stmmac.txt
+++ b/Documentation/devicetree/bindings/net/stmmac.txt
@@ -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>;
@@ -104,4 +142,6 @@ Examples:
 			phy1: ethernet-phy@0 {
 			};
 		};
+		snps,mtl-rx-config = <&mtl_rx_setup>;
+		snps,mtl-tx-config = <&mtl_tx_setup>;
 	};
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 433a842..ff6af8d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -132,6 +132,95 @@ static struct stmmac_axi *stmmac_axi_setup(struct platform_device *pdev)
 }
 
 /**
+ * stmmac_mtl_setup - parse DT parameters for multiple queues configuration
+ * @pdev: platform device
+ */
+static void stmmac_mtl_setup(struct platform_device *pdev,
+			     struct plat_stmmacenet_data *plat)
+{
+	struct device_node *q_node;
+	struct device_node *rx_node;
+	struct device_node *tx_node;
+	u8 queue = 0;
+
+	rx_node = of_parse_phandle(pdev->dev.of_node, "snps,mtl-rx-config", 0);
+	if (!rx_node)
+		return;
+
+	tx_node = of_parse_phandle(pdev->dev.of_node, "snps,mtl-tx-config", 0);
+	if (!tx_node) {
+		of_node_put(rx_node);
+		return;
+	}
+
+	/* Processing RX queues common config */
+	if (of_property_read_u8(rx_node, "snps,rx-queues-to-use",
+				&plat->rx_queues_to_use))
+		plat->rx_queues_to_use = 1;
+
+	if (of_property_read_bool(rx_node, "snps,rx-sched-sp"))
+		plat->rx_sched_algorithm = MTL_RX_ALGORITHM_SP;
+	else if (of_property_read_bool(rx_node, "snps,rx-sched-wsp"))
+		plat->rx_sched_algorithm = MTL_RX_ALGORITHM_WSP;
+	else
+		plat->rx_sched_algorithm = MTL_RX_ALGORITHM_SP;
+
+	/* Processing individual RX queue config */
+	for_each_child_of_node(rx_node, q_node) {
+		if (queue >= plat->rx_queues_to_use)
+			break;
+
+		if (of_property_read_bool(q_node, "snps,dcb-algorithm"))
+			plat->rx_queues_cfg[queue].mode_to_use = MTL_RX_DCB;
+		else if (of_property_read_bool(q_node, "snps,avb-algorithm"))
+			plat->rx_queues_cfg[queue].mode_to_use = MTL_RX_AVB;
+		else
+			plat->rx_queues_cfg[queue].mode_to_use = MTL_RX_DCB;
+
+		if (of_property_read_u8(q_node, "snps,map-to-dma-channel",
+					&plat->rx_queues_cfg[queue].chan))
+			plat->rx_queues_cfg[queue].chan = queue;
+		/* TODO: Dynamic mapping to be included in the future */
+
+		queue++;
+	}
+
+	/* Processing TX queues common config */
+	if (of_property_read_u8(tx_node, "snps,tx-queues-to-use",
+				&plat->tx_queues_to_use))
+		plat->tx_queues_to_use = 1;
+
+	if (of_property_read_bool(tx_node, "snps,tx-sched-wrr"))
+		plat->tx_sched_algorithm = MTL_TX_ALGORITHM_WRR;
+	else if (of_property_read_bool(tx_node, "snps,tx-sched-wfq"))
+		plat->tx_sched_algorithm = MTL_TX_ALGORITHM_WFQ;
+	else if (of_property_read_bool(tx_node, "snps,tx-sched-dwrr"))
+		plat->tx_sched_algorithm = MTL_TX_ALGORITHM_DWRR;
+	else if (of_property_read_bool(tx_node, "snps,tx-sched-sp"))
+		plat->tx_sched_algorithm = MTL_TX_ALGORITHM_SP;
+	else
+		plat->tx_sched_algorithm = MTL_TX_ALGORITHM_SP;
+
+	queue = 0;
+
+	/* Processing individual TX queue config */
+	for_each_child_of_node(tx_node, q_node) {
+		if (queue >= plat->tx_queues_to_use)
+			break;
+
+		if (of_property_read_u8(q_node, "snps,weight",
+					&plat->tx_queues_cfg[queue].weight))
+			plat->tx_queues_cfg[queue].weight = 0x10 + queue;
+
+		queue++;
+	}
+
+	of_node_put(rx_node);
+	of_node_put(tx_node);
+	of_node_put(q_node);
+}
+
+/**
  * stmmac_dt_phy - parse device-tree driver parameters to allocate PHY resources
  * @plat: driver data platform structure
  * @np: device tree node
@@ -340,6 +429,8 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 
 	plat->axi = stmmac_axi_setup(pdev);
 
+	stmmac_mtl_setup(pdev, plat);
+
 	/* clock setup */
 	plat->stmmac_clk = devm_clk_get(&pdev->dev,
 					STMMAC_RESOURCE_NAME);
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index fc273e9..266ff2a 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -28,6 +28,9 @@
 
 #include <linux/platform_device.h>
 
+#define MTL_MAX_RX_QUEUES	8
+#define MTL_MAX_TX_QUEUES	8
+
 #define STMMAC_RX_COE_NONE	0
 #define STMMAC_RX_COE_TYPE1	1
 #define STMMAC_RX_COE_TYPE2	2
@@ -44,6 +47,18 @@
 #define	STMMAC_CSR_150_250M	0x4	/* MDC = clk_scr_i/102 */
 #define	STMMAC_CSR_250_300M	0x5	/* MDC = clk_scr_i/122 */
 
+/* MTL algorithms identifiers */
+#define MTL_TX_ALGORITHM_WRR	0x0
+#define MTL_TX_ALGORITHM_WFQ	0x1
+#define MTL_TX_ALGORITHM_DWRR	0x2
+#define MTL_TX_ALGORITHM_SP	0x3
+#define MTL_RX_ALGORITHM_SP	0x4
+#define MTL_RX_ALGORITHM_WSP	0x5
+
+/* RX Queue Mode */
+#define MTL_RX_DCB		0x0
+#define MTL_RX_AVB		0x1
+
 /* The MDC clock could be set higher than the IEEE 802.3
  * specified frequency limit 0f 2.5 MHz, by programming a clock divider
  * of value different than the above defined values. The resultant MDIO
@@ -109,6 +124,15 @@ struct stmmac_axi {
 	bool axi_rb;
 };
 
+struct stmmac_rxq_cfg {
+	u8 mode_to_use;
+	u8 chan;
+};
+
+struct stmmac_txq_cfg {
+	u8 weight;
+};
+
 struct plat_stmmacenet_data {
 	int bus_id;
 	int phy_addr;
@@ -133,6 +157,12 @@ struct plat_stmmacenet_data {
 	int unicast_filter_entries;
 	int tx_fifo_size;
 	int rx_fifo_size;
+	u8 rx_queues_to_use;
+	u8 tx_queues_to_use;
+	u8 rx_sched_algorithm;
+	u8 tx_sched_algorithm;
+	struct stmmac_rxq_cfg rx_queues_cfg[MTL_MAX_RX_QUEUES];
+	struct stmmac_txq_cfg tx_queues_cfg[MTL_MAX_TX_QUEUES];
 	void (*fix_mac_speed)(void *priv, unsigned int speed);
 	int (*init)(struct platform_device *pdev, void *priv);
 	void (*exit)(struct platform_device *pdev, void *priv);
-- 
2.9.3

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

* [PATCH v5 net-next 2/9] net: stmmac: configure mtl rx and tx algorithms
  2017-03-10 18:24 [PATCH v5 net-next 0/9] prepare mac operations for multiple queues Joao Pinto
  2017-03-10 18:24 ` [PATCH v5 net-next 1/9] net: stmmac: multiple queues dt configuration Joao Pinto
@ 2017-03-10 18:24 ` Joao Pinto
  2017-03-21 11:58   ` [v5,net-next,2/9] " Thierry Reding
  2017-03-10 18:24 ` [PATCH v5 net-next 3/9] net: stmmac: configure tx queue weight Joao Pinto
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 26+ messages in thread
From: Joao Pinto @ 2017-03-10 18:24 UTC (permalink / raw)
  To: davem
  Cc: peppe.cavallaro, alexandre.torgue, niklas.cassel, netdev, Joao Pinto

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 <jpinto@synopsys.com>
---
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(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 04d9245..5a0a781 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -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 */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
index db45134..748ab6f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
@@ -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)
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 1e79e65..f966755 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -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,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 4498a38..af57f8d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -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,
+						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);
+}
+
+/**
  * stmmac_hw_setup - setup mac in a usable state.
  *  @dev : pointer to the device structure.
  *  Description:
@@ -1688,9 +1713,9 @@ static int stmmac_hw_setup(struct net_device *dev, bool init_ptp)
 	/* Initialize the MAC Core */
 	priv->hw->mac->core_init(priv->hw, dev->mtu);
 
-	/* Initialize MAC RX Queues */
-	if (priv->hw->mac->rx_queue_enable)
-		stmmac_mac_enable_rx_queues(priv);
+	/* Initialize MTL*/
+	if (priv->synopsys_id >= DWMAC_CORE_4_00)
+		stmmac_mtl_configuration(priv);
 
 	ret = priv->hw->mac->rx_ipc(priv->hw);
 	if (!ret) {
-- 
2.9.3

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

* [PATCH v5 net-next 3/9] net: stmmac: configure tx queue weight
  2017-03-10 18:24 [PATCH v5 net-next 0/9] prepare mac operations for multiple queues Joao Pinto
  2017-03-10 18:24 ` [PATCH v5 net-next 1/9] net: stmmac: multiple queues dt configuration Joao Pinto
  2017-03-10 18:24 ` [PATCH v5 net-next 2/9] net: stmmac: configure mtl rx and tx algorithms Joao Pinto
@ 2017-03-10 18:24 ` Joao Pinto
  2017-03-10 18:24 ` [PATCH v5 net-next 4/9] net: stmmac: mtl rx queue enabled as dcb or avb Joao Pinto
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 26+ messages in thread
From: Joao Pinto @ 2017-03-10 18:24 UTC (permalink / raw)
  To: davem
  Cc: peppe.cavallaro, alexandre.torgue, niklas.cassel, netdev, Joao Pinto

This patch adds TX queues weight programming.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
Changes v4->v5:
- patch title update (stmicro replaced by stmmac)
Changes v3->v4:
- variable init was simplified in some functions
Changes v2->v3:
- local variable declarations from longest to shortest line
Changes v1->v2:
- Just to keep up with patch-set version

 drivers/net/ethernet/stmicro/stmmac/common.h      |  3 +++
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h      |  7 +++++++
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 12 ++++++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 20 ++++++++++++++++++++
 4 files changed, 42 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 5a0a781..a25b2f8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -459,6 +459,9 @@ struct stmmac_ops {
 	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);
+	/* Set MTL TX queues weight */
+	void (*set_mtl_tx_queue_weight)(struct mac_device_info *hw,
+					u32 weight, u32 queue);
 	/* Dump MAC registers */
 	void (*dump_regs)(struct mac_device_info *hw, u32 *reg_space);
 	/* Handle extra events on specific interrupts hw dependent */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
index 748ab6f..7d77e78 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
@@ -211,6 +211,13 @@ enum power_event {
 #define MTL_OP_MODE_RTC_96		(2 << MTL_OP_MODE_RTC_SHIFT)
 #define MTL_OP_MODE_RTC_128		(3 << MTL_OP_MODE_RTC_SHIFT)
 
+/* MTL Queue Quantum Weight */
+#define MTL_TXQ_WEIGHT_BASE_ADDR	0x00000d18
+#define MTL_TXQ_WEIGHT_BASE_OFFSET	0x40
+#define MTL_TXQX_WEIGHT_BASE_ADDR(x)	(MTL_TXQ_WEIGHT_BASE_ADDR + \
+					((x) * MTL_TXQ_WEIGHT_BASE_OFFSET))
+#define MTL_TXQ_WEIGHT_ISCQW_MASK	GENMASK(20, 0)
+
 /*  MTL debug */
 #define MTL_DEBUG_TXSTSFSTS		BIT(5)
 #define MTL_DEBUG_TXFSTS		BIT(4)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index f966755..fda6cfa 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -116,6 +116,17 @@ static void dwmac4_prog_mtl_tx_algorithms(struct mac_device_info *hw,
 	}
 }
 
+static void dwmac4_set_mtl_tx_queue_weight(struct mac_device_info *hw,
+					   u32 weight, u32 queue)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value = readl(ioaddr + MTL_TXQX_WEIGHT_BASE_ADDR(queue));
+
+	value &= ~MTL_TXQ_WEIGHT_ISCQW_MASK;
+	value |= weight & MTL_TXQ_WEIGHT_ISCQW_MASK;
+	writel(value, ioaddr + MTL_TXQX_WEIGHT_BASE_ADDR(queue));
+}
+
 static void dwmac4_dump_regs(struct mac_device_info *hw, u32 *reg_space)
 {
 	void __iomem *ioaddr = hw->pcsr;
@@ -505,6 +516,7 @@ static const struct stmmac_ops dwmac4_ops = {
 	.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,
+	.set_mtl_tx_queue_weight = dwmac4_set_mtl_tx_queue_weight,
 	.dump_regs = dwmac4_dump_regs,
 	.host_irq_status = dwmac4_irq_status,
 	.flow_ctrl = dwmac4_flow_ctrl,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index af57f8d..735540f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1645,6 +1645,23 @@ static void stmmac_init_tx_coalesce(struct stmmac_priv *priv)
 }
 
 /**
+ *  stmmac_set_tx_queue_weight - Set TX queue weight
+ *  @priv: driver private structure
+ *  Description: It is used for setting TX queues weight
+ */
+static void stmmac_set_tx_queue_weight(struct stmmac_priv *priv)
+{
+	u32 tx_queues_count = priv->plat->tx_queues_to_use;
+	u32 weight;
+	u32 queue;
+
+	for (queue = 0; queue < tx_queues_count; queue++) {
+		weight = priv->plat->tx_queues_cfg[queue].weight;
+		priv->hw->mac->set_mtl_tx_queue_weight(priv->hw, weight, queue);
+	}
+}
+
+/**
  *  stmmac_mtl_configuration - Configure MTL
  *  @priv: driver private structure
  *  Description: It is used for configurring MTL
@@ -1654,6 +1671,9 @@ 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;
 
+	if (tx_queues_count > 1 && priv->hw->mac->set_mtl_tx_queue_weight)
+		stmmac_set_tx_queue_weight(priv);
+
 	/* 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,
-- 
2.9.3

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

* [PATCH v5 net-next 4/9] net: stmmac: mtl rx queue enabled as dcb or avb
  2017-03-10 18:24 [PATCH v5 net-next 0/9] prepare mac operations for multiple queues Joao Pinto
                   ` (2 preceding siblings ...)
  2017-03-10 18:24 ` [PATCH v5 net-next 3/9] net: stmmac: configure tx queue weight Joao Pinto
@ 2017-03-10 18:24 ` Joao Pinto
  2017-03-10 18:24 ` [PATCH v5 net-next 5/9] net: stmmac: mapping mtl rx to dma channel Joao Pinto
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 26+ messages in thread
From: Joao Pinto @ 2017-03-10 18:24 UTC (permalink / raw)
  To: davem
  Cc: peppe.cavallaro, alexandre.torgue, niklas.cassel, netdev, Joao Pinto

This patch introduces the enabling of RX queues as DCB or as AVB based
on configuration.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
Changes v4->v5:
- patch title update (stmicro replaced by stmmac)
Changes v3->v4:
- variable init was simplified in some functions
Changes v1->v3:
- Just to keep up with patch-set version

 drivers/net/ethernet/stmicro/stmmac/common.h      |  2 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c |  8 ++++++--
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 19 +++++++------------
 3 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index a25b2f8..ad89c47 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -454,7 +454,7 @@ struct stmmac_ops {
 	/* Enable and verify that the IPC module is supported */
 	int (*rx_ipc)(struct mac_device_info *hw);
 	/* Enable RX Queues */
-	void (*rx_queue_enable)(struct mac_device_info *hw, u32 queue);
+	void (*rx_queue_enable)(struct mac_device_info *hw, u8 mode, u32 queue);
 	/* Program RX Algorithms */
 	void (*prog_mtl_rx_algorithms)(struct mac_device_info *hw, u32 rx_alg);
 	/* Program TX Algorithms */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index fda6cfa..21a696e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -59,13 +59,17 @@ static void dwmac4_core_init(struct mac_device_info *hw, int mtu)
 	writel(value, ioaddr + GMAC_INT_EN);
 }
 
-static void dwmac4_rx_queue_enable(struct mac_device_info *hw, u32 queue)
+static void dwmac4_rx_queue_enable(struct mac_device_info *hw,
+				   u8 mode, u32 queue)
 {
 	void __iomem *ioaddr = hw->pcsr;
 	u32 value = readl(ioaddr + GMAC_RXQ_CTRL0);
 
 	value &= GMAC_RX_QUEUE_CLEAR(queue);
-	value |= GMAC_RX_AV_QUEUE_ENABLE(queue);
+	if (mode == MTL_RX_AVB)
+		value |= GMAC_RX_AV_QUEUE_ENABLE(queue);
+	else if (mode == MTL_RX_DCB)
+		value |= GMAC_RX_DCB_QUEUE_ENABLE(queue);
 
 	writel(value, ioaddr + GMAC_RXQ_CTRL0);
 }
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 735540f..c3e2dbf 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1256,19 +1256,14 @@ static void free_dma_desc_resources(struct stmmac_priv *priv)
  */
 static void stmmac_mac_enable_rx_queues(struct stmmac_priv *priv)
 {
-	int rx_count = priv->dma_cap.number_rx_queues;
-	int queue = 0;
-
-	/* If GMAC does not have multiple queues, then this is not necessary*/
-	if (rx_count == 1)
-		return;
+	u32 rx_queues_count = priv->plat->rx_queues_to_use;
+	int queue;
+	u8 mode;
 
-	/**
-	 *  If the core is synthesized with multiple rx queues / multiple
-	 *  dma channels, then rx queues will be disabled by default.
-	 *  For now only rx queue 0 is enabled.
-	 */
-	priv->hw->mac->rx_queue_enable(priv->hw, queue);
+	for (queue = 0; queue < rx_queues_count; queue++) {
+		mode = priv->plat->rx_queues_cfg[queue].mode_to_use;
+		priv->hw->mac->rx_queue_enable(priv->hw, mode, queue);
+	}
 }
 
 /**
-- 
2.9.3

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

* [PATCH v5 net-next 5/9] net: stmmac: mapping mtl rx to dma channel
  2017-03-10 18:24 [PATCH v5 net-next 0/9] prepare mac operations for multiple queues Joao Pinto
                   ` (3 preceding siblings ...)
  2017-03-10 18:24 ` [PATCH v5 net-next 4/9] net: stmmac: mtl rx queue enabled as dcb or avb Joao Pinto
@ 2017-03-10 18:24 ` Joao Pinto
  2017-03-10 18:24 ` [PATCH v5 net-next 6/9] net: stmmac: flow_ctrl functions adapted to mtl Joao Pinto
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 26+ messages in thread
From: Joao Pinto @ 2017-03-10 18:24 UTC (permalink / raw)
  To: davem
  Cc: peppe.cavallaro, alexandre.torgue, niklas.cassel, netdev, Joao Pinto

This patch adds the functionality of RX queue to dma channel mapping
based on configuration.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
Changes v4->v5:
- patch title update (stmicro replaced by stmmac)
Changes v3->v4:
- variable init was simplified in some functions
Changes v1->v3:
- Just to keep up with patch-set version

 drivers/net/ethernet/stmicro/stmmac/common.h      |  2 ++
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h      |  7 +++++++
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 25 +++++++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 21 +++++++++++++++++++
 4 files changed, 55 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index ad89c47..32f5f25 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -462,6 +462,8 @@ struct stmmac_ops {
 	/* Set MTL TX queues weight */
 	void (*set_mtl_tx_queue_weight)(struct mac_device_info *hw,
 					u32 weight, u32 queue);
+	/* RX MTL queue to RX dma mapping */
+	void (*map_mtl_to_dma)(struct mac_device_info *hw, u32 queue, u32 chan);
 	/* Dump MAC registers */
 	void (*dump_regs)(struct mac_device_info *hw, u32 *reg_space);
 	/* Handle extra events on specific interrupts hw dependent */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
index 7d77e78..9dd8ac1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
@@ -174,6 +174,13 @@ enum power_event {
 #define MTL_INT_STATUS			0x00000c20
 #define MTL_INT_Q0			BIT(0)
 
+#define MTL_RXQ_DMA_MAP0		0x00000c30 /* queue 0 to 3 */
+#define MTL_RXQ_DMA_MAP1		0x00000c34 /* queue 4 to 7 */
+#define MTL_RXQ_DMA_Q04MDMACH_MASK	GENMASK(3, 0)
+#define MTL_RXQ_DMA_Q04MDMACH(x)	((x) << 0)
+#define MTL_RXQ_DMA_QXMDMACH_MASK(x)	GENMASK(11 + (8 * ((x) - 1)), 8 * (x))
+#define MTL_RXQ_DMA_QXMDMACH(chan, q)	((chan) << (8 * (q)))
+
 #define MTL_CHAN_BASE_ADDR		0x00000d00
 #define MTL_CHAN_BASE_OFFSET		0x40
 #define MTL_CHANX_BASE_ADDR(x)		(MTL_CHAN_BASE_ADDR + \
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 21a696e..e9b153f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -131,6 +131,30 @@ static void dwmac4_set_mtl_tx_queue_weight(struct mac_device_info *hw,
 	writel(value, ioaddr + MTL_TXQX_WEIGHT_BASE_ADDR(queue));
 }
 
+static void dwmac4_map_mtl_dma(struct mac_device_info *hw, u32 queue, u32 chan)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value;
+
+	if (queue < 4)
+		value = readl(ioaddr + MTL_RXQ_DMA_MAP0);
+	else
+		value = readl(ioaddr + MTL_RXQ_DMA_MAP1);
+
+	if (queue == 0 || queue == 4) {
+		value &= ~MTL_RXQ_DMA_Q04MDMACH_MASK;
+		value |= MTL_RXQ_DMA_Q04MDMACH(chan);
+	} else {
+		value &= ~MTL_RXQ_DMA_QXMDMACH_MASK(queue);
+		value |= MTL_RXQ_DMA_QXMDMACH(chan, queue);
+	}
+
+	if (queue < 4)
+		writel(value, ioaddr + MTL_RXQ_DMA_MAP0);
+	else
+		writel(value, ioaddr + MTL_RXQ_DMA_MAP1);
+}
+
 static void dwmac4_dump_regs(struct mac_device_info *hw, u32 *reg_space)
 {
 	void __iomem *ioaddr = hw->pcsr;
@@ -521,6 +545,7 @@ static const struct stmmac_ops dwmac4_ops = {
 	.prog_mtl_rx_algorithms = dwmac4_prog_mtl_rx_algorithms,
 	.prog_mtl_tx_algorithms = dwmac4_prog_mtl_tx_algorithms,
 	.set_mtl_tx_queue_weight = dwmac4_set_mtl_tx_queue_weight,
+	.map_mtl_to_dma = dwmac4_map_mtl_dma,
 	.dump_regs = dwmac4_dump_regs,
 	.host_irq_status = dwmac4_irq_status,
 	.flow_ctrl = dwmac4_flow_ctrl,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index c3e2dbf..619bcc6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1657,6 +1657,23 @@ static void stmmac_set_tx_queue_weight(struct stmmac_priv *priv)
 }
 
 /**
+ *  stmmac_rx_queue_dma_chan_map - Map RX queue to RX dma channel
+ *  @priv: driver private structure
+ *  Description: It is used for mapping RX queues to RX dma channels
+ */
+static void stmmac_rx_queue_dma_chan_map(struct stmmac_priv *priv)
+{
+	u32 rx_queues_count = priv->plat->rx_queues_to_use;
+	u32 queue;
+	u32 chan;
+
+	for (queue = 0; queue < rx_queues_count; queue++) {
+		chan = priv->plat->rx_queues_cfg[queue].chan;
+		priv->hw->mac->map_mtl_to_dma(priv->hw, queue, chan);
+	}
+}
+
+/**
  *  stmmac_mtl_configuration - Configure MTL
  *  @priv: driver private structure
  *  Description: It is used for configurring MTL
@@ -1679,6 +1696,10 @@ static void stmmac_mtl_configuration(struct stmmac_priv *priv)
 		priv->hw->mac->prog_mtl_tx_algorithms(priv->hw,
 						priv->plat->tx_sched_algorithm);
 
+	/* Map RX MTL to DMA channels */
+	if (rx_queues_count > 1 && priv->hw->mac->map_mtl_to_dma)
+		stmmac_rx_queue_dma_chan_map(priv);
+
 	/* Enable MAC RX Queues */
 	if (rx_queues_count > 1 && priv->hw->mac->rx_queue_enable)
 		stmmac_mac_enable_rx_queues(priv);
-- 
2.9.3

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

* [PATCH v5 net-next 6/9] net: stmmac: flow_ctrl functions adapted to mtl
  2017-03-10 18:24 [PATCH v5 net-next 0/9] prepare mac operations for multiple queues Joao Pinto
                   ` (4 preceding siblings ...)
  2017-03-10 18:24 ` [PATCH v5 net-next 5/9] net: stmmac: mapping mtl rx to dma channel Joao Pinto
@ 2017-03-10 18:24 ` Joao Pinto
  2017-03-10 18:24 ` [PATCH v5 net-next 7/9] net: stmmac: prepare irq_status for mtl Joao Pinto
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 26+ messages in thread
From: Joao Pinto @ 2017-03-10 18:24 UTC (permalink / raw)
  To: davem
  Cc: peppe.cavallaro, alexandre.torgue, niklas.cassel, netdev, Joao Pinto

This patch adapts flow_ctrl function to prepare it for multiple queues.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
Changes v4->v5:
- patch title update (stmicro replaced by stmmac)
Changes v3->v4:
- variable init simplified in some functions
Changes v1->v3:
- Just to keep up with patch-set version

 drivers/net/ethernet/stmicro/stmmac/common.h         |  2 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c |  3 ++-
 drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c  |  3 ++-
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c    | 20 +++++++++++++-------
 drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c |  3 ++-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c    | 17 ++++++++++++++---
 6 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 32f5f25..5532633 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -473,7 +473,7 @@ struct stmmac_ops {
 	void (*set_filter)(struct mac_device_info *hw, struct net_device *dev);
 	/* Flow control setting */
 	void (*flow_ctrl)(struct mac_device_info *hw, unsigned int duplex,
-			  unsigned int fc, unsigned int pause_time);
+			  unsigned int fc, unsigned int pause_time, u32 tx_cnt);
 	/* Set power management mode (e.g. magic frame) */
 	void (*pmt)(struct mac_device_info *hw, unsigned long mode);
 	/* Set/Get Unicast MAC addresses */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
index 19b9b30..3a95ad9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
@@ -216,7 +216,8 @@ static void dwmac1000_set_filter(struct mac_device_info *hw,
 
 
 static void dwmac1000_flow_ctrl(struct mac_device_info *hw, unsigned int duplex,
-				unsigned int fc, unsigned int pause_time)
+				unsigned int fc, unsigned int pause_time,
+				u32 tx_cnt)
 {
 	void __iomem *ioaddr = hw->pcsr;
 	/* Set flow such that DZPQ in Mac Register 6 is 0,
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
index e370cce..524135e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
@@ -131,7 +131,8 @@ static void dwmac100_set_filter(struct mac_device_info *hw,
 }
 
 static void dwmac100_flow_ctrl(struct mac_device_info *hw, unsigned int duplex,
-			       unsigned int fc, unsigned int pause_time)
+			       unsigned int fc, unsigned int pause_time,
+			       u32 tx_cnt)
 {
 	void __iomem *ioaddr = hw->pcsr;
 	unsigned int flow = MAC_FLOW_CTRL_ENABLE;
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index e9b153f..3069def 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -336,11 +336,12 @@ static void dwmac4_set_filter(struct mac_device_info *hw,
 }
 
 static void dwmac4_flow_ctrl(struct mac_device_info *hw, unsigned int duplex,
-			     unsigned int fc, unsigned int pause_time)
+			     unsigned int fc, unsigned int pause_time,
+			     u32 tx_cnt)
 {
 	void __iomem *ioaddr = hw->pcsr;
-	u32 channel = STMMAC_CHAN0;	/* FIXME */
 	unsigned int flow = 0;
+	u32 queue = 0;
 
 	pr_debug("GMAC Flow-Control:\n");
 	if (fc & FLOW_RX) {
@@ -350,13 +351,18 @@ static void dwmac4_flow_ctrl(struct mac_device_info *hw, unsigned int duplex,
 	}
 	if (fc & FLOW_TX) {
 		pr_debug("\tTransmit Flow-Control ON\n");
-		flow |= GMAC_TX_FLOW_CTRL_TFE;
-		writel(flow, ioaddr + GMAC_QX_TX_FLOW_CTRL(channel));
 
-		if (duplex) {
+		if (duplex)
 			pr_debug("\tduplex mode: PAUSE %d\n", pause_time);
-			flow |= (pause_time << GMAC_TX_FLOW_CTRL_PT_SHIFT);
-			writel(flow, ioaddr + GMAC_QX_TX_FLOW_CTRL(channel));
+
+		for (queue = 0; queue < tx_cnt; queue++) {
+			flow |= GMAC_TX_FLOW_CTRL_TFE;
+
+			if (duplex)
+				flow |=
+				(pause_time << GMAC_TX_FLOW_CTRL_PT_SHIFT);
+
+			writel(flow, ioaddr + GMAC_QX_TX_FLOW_CTRL(queue));
 		}
 	}
 }
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index 85d6411..4a5dc89 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -481,6 +481,7 @@ stmmac_set_pauseparam(struct net_device *netdev,
 		      struct ethtool_pauseparam *pause)
 {
 	struct stmmac_priv *priv = netdev_priv(netdev);
+	u32 tx_cnt = priv->plat->tx_queues_to_use;
 	struct phy_device *phy = netdev->phydev;
 	int new_pause = FLOW_OFF;
 
@@ -511,7 +512,7 @@ stmmac_set_pauseparam(struct net_device *netdev,
 	}
 
 	priv->hw->mac->flow_ctrl(priv->hw, phy->duplex, priv->flow_ctrl,
-				 priv->pause);
+				 priv->pause, tx_cnt);
 	return 0;
 }
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 619bcc6..ed57409 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -673,6 +673,19 @@ static void stmmac_release_ptp(struct stmmac_priv *priv)
 }
 
 /**
+ *  stmmac_mac_flow_ctrl - Configure flow control in all queues
+ *  @priv: driver private structure
+ *  Description: It is used for configuring the flow control in all queues
+ */
+static void stmmac_mac_flow_ctrl(struct stmmac_priv *priv, u32 duplex)
+{
+	u32 tx_cnt = priv->plat->tx_queues_to_use;
+
+	priv->hw->mac->flow_ctrl(priv->hw, duplex, priv->flow_ctrl,
+				 priv->pause, tx_cnt);
+}
+
+/**
  * stmmac_adjust_link - adjusts the link parameters
  * @dev: net device structure
  * Description: this is the helper called by the physical abstraction layer
@@ -687,7 +700,6 @@ static void stmmac_adjust_link(struct net_device *dev)
 	struct phy_device *phydev = dev->phydev;
 	unsigned long flags;
 	int new_state = 0;
-	unsigned int fc = priv->flow_ctrl, pause_time = priv->pause;
 
 	if (!phydev)
 		return;
@@ -709,8 +721,7 @@ static void stmmac_adjust_link(struct net_device *dev)
 		}
 		/* Flow Control operation */
 		if (phydev->pause)
-			priv->hw->mac->flow_ctrl(priv->hw, phydev->duplex,
-						 fc, pause_time);
+			stmmac_mac_flow_ctrl(priv, phydev->duplex);
 
 		if (phydev->speed != priv->speed) {
 			new_state = 1;
-- 
2.9.3

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

* [PATCH v5 net-next 7/9] net: stmmac: prepare irq_status for mtl
  2017-03-10 18:24 [PATCH v5 net-next 0/9] prepare mac operations for multiple queues Joao Pinto
                   ` (5 preceding siblings ...)
  2017-03-10 18:24 ` [PATCH v5 net-next 6/9] net: stmmac: flow_ctrl functions adapted to mtl Joao Pinto
@ 2017-03-10 18:24 ` Joao Pinto
  2017-03-10 18:24 ` [PATCH v5 net-next 8/9] net: stmmac: mac debug prepared for multiple queues Joao Pinto
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 26+ messages in thread
From: Joao Pinto @ 2017-03-10 18:24 UTC (permalink / raw)
  To: davem
  Cc: peppe.cavallaro, alexandre.torgue, niklas.cassel, netdev, Joao Pinto

This patch prepares mac irq status treatment for multiple queues.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
Changes v4->v5:
- patch title update (stmicro replaced by stmmac)
Changes v3->v4:
- Build error in dwmac4_irq_mtl_status()
- variable init simplified in some functions
Changes v2->v3:
- local variable declarations from longest to shortest line
Changes v1->v2:
- Just to keep up with patch-set version

 drivers/net/ethernet/stmicro/stmmac/common.h      |  2 ++
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h      |  2 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 40 ++++++++++++++---------
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |  5 +++
 4 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 5532633..6a348d3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -469,6 +469,8 @@ struct stmmac_ops {
 	/* Handle extra events on specific interrupts hw dependent */
 	int (*host_irq_status)(struct mac_device_info *hw,
 			       struct stmmac_extra_stats *x);
+	/* Handle MTL interrupts */
+	int (*host_mtl_irq_status)(struct mac_device_info *hw, u32 chan);
 	/* Multicast filter setting */
 	void (*set_filter)(struct mac_device_info *hw, struct net_device *dev);
 	/* Flow control setting */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
index 9dd8ac1..5ca4d64 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
@@ -172,7 +172,7 @@ enum power_event {
 #define MTL_OPERATION_RAA_WSP		(0x1 << 2)
 
 #define MTL_INT_STATUS			0x00000c20
-#define MTL_INT_Q0			BIT(0)
+#define MTL_INT_QX(x)			BIT(x)
 
 #define MTL_RXQ_DMA_MAP0		0x00000c30 /* queue 0 to 3 */
 #define MTL_RXQ_DMA_MAP1		0x00000c34 /* queue 4 to 7 */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 3069def..f0f2dce 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -416,11 +416,34 @@ static void dwmac4_phystatus(void __iomem *ioaddr, struct stmmac_extra_stats *x)
 	}
 }
 
+static int dwmac4_irq_mtl_status(struct mac_device_info *hw, u32 chan)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 mtl_int_qx_status;
+	int ret = 0;
+
+	mtl_int_qx_status = readl(ioaddr + MTL_INT_STATUS);
+
+	/* Check MTL Interrupt */
+	if (mtl_int_qx_status & MTL_INT_QX(chan)) {
+		/* read Queue x Interrupt status */
+		u32 status = readl(ioaddr + MTL_CHAN_INT_CTRL(chan));
+
+		if (status & MTL_RX_OVERFLOW_INT) {
+			/*  clear Interrupt */
+			writel(status | MTL_RX_OVERFLOW_INT,
+			       ioaddr + MTL_CHAN_INT_CTRL(chan));
+			ret = CORE_IRQ_MTL_RX_OVERFLOW;
+		}
+	}
+
+	return ret;
+}
+
 static int dwmac4_irq_status(struct mac_device_info *hw,
 			     struct stmmac_extra_stats *x)
 {
 	void __iomem *ioaddr = hw->pcsr;
-	u32 mtl_int_qx_status;
 	u32 intr_status;
 	int ret = 0;
 
@@ -439,20 +462,6 @@ static int dwmac4_irq_status(struct mac_device_info *hw,
 		x->irq_receive_pmt_irq_n++;
 	}
 
-	mtl_int_qx_status = readl(ioaddr + MTL_INT_STATUS);
-	/* Check MTL Interrupt: Currently only one queue is used: Q0. */
-	if (mtl_int_qx_status & MTL_INT_Q0) {
-		/* read Queue 0 Interrupt status */
-		u32 status = readl(ioaddr + MTL_CHAN_INT_CTRL(STMMAC_CHAN0));
-
-		if (status & MTL_RX_OVERFLOW_INT) {
-			/*  clear Interrupt */
-			writel(status | MTL_RX_OVERFLOW_INT,
-			       ioaddr + MTL_CHAN_INT_CTRL(STMMAC_CHAN0));
-			ret = CORE_IRQ_MTL_RX_OVERFLOW;
-		}
-	}
-
 	dwmac_pcs_isr(ioaddr, GMAC_PCS_BASE, intr_status, x);
 	if (intr_status & PCS_RGSMIIIS_IRQ)
 		dwmac4_phystatus(ioaddr, x);
@@ -554,6 +563,7 @@ static const struct stmmac_ops dwmac4_ops = {
 	.map_mtl_to_dma = dwmac4_map_mtl_dma,
 	.dump_regs = dwmac4_dump_regs,
 	.host_irq_status = dwmac4_irq_status,
+	.host_mtl_irq_status = dwmac4_irq_mtl_status,
 	.flow_ctrl = dwmac4_flow_ctrl,
 	.pmt = dwmac4_pmt,
 	.set_umac_addr = dwmac4_set_umac_addr,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index ed57409..3be77d4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2880,6 +2880,11 @@ static irqreturn_t stmmac_interrupt(int irq, void *dev_id)
 	if ((priv->plat->has_gmac) || (priv->plat->has_gmac4)) {
 		int status = priv->hw->mac->host_irq_status(priv->hw,
 							    &priv->xstats);
+
+		if (priv->synopsys_id >= DWMAC_CORE_4_00)
+			status |= priv->hw->mac->host_mtl_irq_status(priv->hw,
+								STMMAC_CHAN0);
+
 		if (unlikely(status)) {
 			/* For LPI we need to save the tx status */
 			if (status & CORE_IRQ_TX_PATH_IN_LPI_MODE)
-- 
2.9.3

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

* [PATCH v5 net-next 8/9] net: stmmac: mac debug prepared for multiple queues
  2017-03-10 18:24 [PATCH v5 net-next 0/9] prepare mac operations for multiple queues Joao Pinto
                   ` (6 preceding siblings ...)
  2017-03-10 18:24 ` [PATCH v5 net-next 7/9] net: stmmac: prepare irq_status for mtl Joao Pinto
@ 2017-03-10 18:24 ` Joao Pinto
  2017-03-10 18:24 ` [PATCH v5 net-next 9/9] net: stmmac: configuration of CBS in case of a TX AVB queue Joao Pinto
  2017-03-13  6:42 ` [PATCH v5 net-next 0/9] prepare mac operations for multiple queues David Miller
  9 siblings, 0 replies; 26+ messages in thread
From: Joao Pinto @ 2017-03-10 18:24 UTC (permalink / raw)
  To: davem
  Cc: peppe.cavallaro, alexandre.torgue, niklas.cassel, netdev, Joao Pinto

This patch prepares mac debug dump for multiple queues.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
Changes v4->v5:
- patch title update (stmicro replaced by stmmac)
Changes v3->v4:
- Debug function now gets all rx / tx queues data
Changes v1->v3:
- Just to keep up with patch-set version

 drivers/net/ethernet/stmicro/stmmac/common.h       |   3 +-
 .../net/ethernet/stmicro/stmmac/dwmac1000_core.c   |   3 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c  | 107 +++++++++++----------
 .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c   |   5 +-
 4 files changed, 64 insertions(+), 54 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 6a348d3..86f43ac 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -488,7 +488,8 @@ struct stmmac_ops {
 	void (*reset_eee_mode)(struct mac_device_info *hw);
 	void (*set_eee_timer)(struct mac_device_info *hw, int ls, int tw);
 	void (*set_eee_pls)(struct mac_device_info *hw, int link);
-	void (*debug)(void __iomem *ioaddr, struct stmmac_extra_stats *x);
+	void (*debug)(void __iomem *ioaddr, struct stmmac_extra_stats *x,
+		      u32 rx_queues, u32 tx_queues);
 	/* PCS calls */
 	void (*pcs_ctrl_ane)(void __iomem *ioaddr, bool ane, bool srgmi_ral,
 			     bool loopback);
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
index 3a95ad9..7f78f77 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
@@ -413,7 +413,8 @@ static void dwmac1000_get_adv_lp(void __iomem *ioaddr, struct rgmii_adv *adv)
 	dwmac_get_adv_lp(ioaddr, GMAC_PCS_BASE, adv);
 }
 
-static void dwmac1000_debug(void __iomem *ioaddr, struct stmmac_extra_stats *x)
+static void dwmac1000_debug(void __iomem *ioaddr, struct stmmac_extra_stats *x,
+			    u32 rx_queues, u32 tx_queues)
 {
 	u32 value = readl(ioaddr + GMAC_DEBUG);
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index f0f2dce..670cfee 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -469,64 +469,69 @@ static int dwmac4_irq_status(struct mac_device_info *hw,
 	return ret;
 }
 
-static void dwmac4_debug(void __iomem *ioaddr, struct stmmac_extra_stats *x)
+static void dwmac4_debug(void __iomem *ioaddr, struct stmmac_extra_stats *x,
+			 u32 rx_queues, u32 tx_queues)
 {
 	u32 value;
-
-	/*  Currently only channel 0 is supported */
-	value = readl(ioaddr + MTL_CHAN_TX_DEBUG(STMMAC_CHAN0));
-
-	if (value & MTL_DEBUG_TXSTSFSTS)
-		x->mtl_tx_status_fifo_full++;
-	if (value & MTL_DEBUG_TXFSTS)
-		x->mtl_tx_fifo_not_empty++;
-	if (value & MTL_DEBUG_TWCSTS)
-		x->mmtl_fifo_ctrl++;
-	if (value & MTL_DEBUG_TRCSTS_MASK) {
-		u32 trcsts = (value & MTL_DEBUG_TRCSTS_MASK)
-			     >> MTL_DEBUG_TRCSTS_SHIFT;
-		if (trcsts == MTL_DEBUG_TRCSTS_WRITE)
-			x->mtl_tx_fifo_read_ctrl_write++;
-		else if (trcsts == MTL_DEBUG_TRCSTS_TXW)
-			x->mtl_tx_fifo_read_ctrl_wait++;
-		else if (trcsts == MTL_DEBUG_TRCSTS_READ)
-			x->mtl_tx_fifo_read_ctrl_read++;
-		else
-			x->mtl_tx_fifo_read_ctrl_idle++;
+	u32 queue;
+
+	for (queue = 0; queue < tx_queues; queue++) {
+		value = readl(ioaddr + MTL_CHAN_TX_DEBUG(queue));
+
+		if (value & MTL_DEBUG_TXSTSFSTS)
+			x->mtl_tx_status_fifo_full++;
+		if (value & MTL_DEBUG_TXFSTS)
+			x->mtl_tx_fifo_not_empty++;
+		if (value & MTL_DEBUG_TWCSTS)
+			x->mmtl_fifo_ctrl++;
+		if (value & MTL_DEBUG_TRCSTS_MASK) {
+			u32 trcsts = (value & MTL_DEBUG_TRCSTS_MASK)
+				     >> MTL_DEBUG_TRCSTS_SHIFT;
+			if (trcsts == MTL_DEBUG_TRCSTS_WRITE)
+				x->mtl_tx_fifo_read_ctrl_write++;
+			else if (trcsts == MTL_DEBUG_TRCSTS_TXW)
+				x->mtl_tx_fifo_read_ctrl_wait++;
+			else if (trcsts == MTL_DEBUG_TRCSTS_READ)
+				x->mtl_tx_fifo_read_ctrl_read++;
+			else
+				x->mtl_tx_fifo_read_ctrl_idle++;
+		}
+		if (value & MTL_DEBUG_TXPAUSED)
+			x->mac_tx_in_pause++;
 	}
-	if (value & MTL_DEBUG_TXPAUSED)
-		x->mac_tx_in_pause++;
 
-	value = readl(ioaddr + MTL_CHAN_RX_DEBUG(STMMAC_CHAN0));
+	for (queue = 0; queue < rx_queues; queue++) {
+		value = readl(ioaddr + MTL_CHAN_RX_DEBUG(queue));
 
-	if (value & MTL_DEBUG_RXFSTS_MASK) {
-		u32 rxfsts = (value & MTL_DEBUG_RXFSTS_MASK)
-			     >> MTL_DEBUG_RRCSTS_SHIFT;
+		if (value & MTL_DEBUG_RXFSTS_MASK) {
+			u32 rxfsts = (value & MTL_DEBUG_RXFSTS_MASK)
+				     >> MTL_DEBUG_RRCSTS_SHIFT;
 
-		if (rxfsts == MTL_DEBUG_RXFSTS_FULL)
-			x->mtl_rx_fifo_fill_level_full++;
-		else if (rxfsts == MTL_DEBUG_RXFSTS_AT)
-			x->mtl_rx_fifo_fill_above_thresh++;
-		else if (rxfsts == MTL_DEBUG_RXFSTS_BT)
-			x->mtl_rx_fifo_fill_below_thresh++;
-		else
-			x->mtl_rx_fifo_fill_level_empty++;
-	}
-	if (value & MTL_DEBUG_RRCSTS_MASK) {
-		u32 rrcsts = (value & MTL_DEBUG_RRCSTS_MASK) >>
-			     MTL_DEBUG_RRCSTS_SHIFT;
-
-		if (rrcsts == MTL_DEBUG_RRCSTS_FLUSH)
-			x->mtl_rx_fifo_read_ctrl_flush++;
-		else if (rrcsts == MTL_DEBUG_RRCSTS_RSTAT)
-			x->mtl_rx_fifo_read_ctrl_read_data++;
-		else if (rrcsts == MTL_DEBUG_RRCSTS_RDATA)
-			x->mtl_rx_fifo_read_ctrl_status++;
-		else
-			x->mtl_rx_fifo_read_ctrl_idle++;
+			if (rxfsts == MTL_DEBUG_RXFSTS_FULL)
+				x->mtl_rx_fifo_fill_level_full++;
+			else if (rxfsts == MTL_DEBUG_RXFSTS_AT)
+				x->mtl_rx_fifo_fill_above_thresh++;
+			else if (rxfsts == MTL_DEBUG_RXFSTS_BT)
+				x->mtl_rx_fifo_fill_below_thresh++;
+			else
+				x->mtl_rx_fifo_fill_level_empty++;
+		}
+		if (value & MTL_DEBUG_RRCSTS_MASK) {
+			u32 rrcsts = (value & MTL_DEBUG_RRCSTS_MASK) >>
+				     MTL_DEBUG_RRCSTS_SHIFT;
+
+			if (rrcsts == MTL_DEBUG_RRCSTS_FLUSH)
+				x->mtl_rx_fifo_read_ctrl_flush++;
+			else if (rrcsts == MTL_DEBUG_RRCSTS_RSTAT)
+				x->mtl_rx_fifo_read_ctrl_read_data++;
+			else if (rrcsts == MTL_DEBUG_RRCSTS_RDATA)
+				x->mtl_rx_fifo_read_ctrl_status++;
+			else
+				x->mtl_rx_fifo_read_ctrl_idle++;
+		}
+		if (value & MTL_DEBUG_RWCSTS)
+			x->mtl_rx_fifo_ctrl_active++;
 	}
-	if (value & MTL_DEBUG_RWCSTS)
-		x->mtl_rx_fifo_ctrl_active++;
 
 	/* GMAC debug */
 	value = readl(ioaddr + GMAC_DEBUG);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index 4a5dc89..61b9369 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -520,6 +520,8 @@ static void stmmac_get_ethtool_stats(struct net_device *dev,
 				 struct ethtool_stats *dummy, u64 *data)
 {
 	struct stmmac_priv *priv = netdev_priv(dev);
+	u32 rx_queues_count = priv->plat->rx_queues_to_use;
+	u32 tx_queues_count = priv->plat->tx_queues_to_use;
 	int i, j = 0;
 
 	/* Update the DMA HW counters for dwmac10/100 */
@@ -550,7 +552,8 @@ static void stmmac_get_ethtool_stats(struct net_device *dev,
 		if ((priv->hw->mac->debug) &&
 		    (priv->synopsys_id >= DWMAC_CORE_3_50))
 			priv->hw->mac->debug(priv->ioaddr,
-					     (void *)&priv->xstats);
+					     (void *)&priv->xstats,
+					     rx_queues_count, tx_queues_count);
 	}
 	for (i = 0; i < STMMAC_STATS_LEN; i++) {
 		char *p = (char *)priv + stmmac_gstrings_stats[i].stat_offset;
-- 
2.9.3

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

* [PATCH v5 net-next 9/9] net: stmmac: configuration of CBS in case of a TX AVB queue
  2017-03-10 18:24 [PATCH v5 net-next 0/9] prepare mac operations for multiple queues Joao Pinto
                   ` (7 preceding siblings ...)
  2017-03-10 18:24 ` [PATCH v5 net-next 8/9] net: stmmac: mac debug prepared for multiple queues Joao Pinto
@ 2017-03-10 18:24 ` Joao Pinto
  2017-03-13  6:42 ` [PATCH v5 net-next 0/9] prepare mac operations for multiple queues David Miller
  9 siblings, 0 replies; 26+ messages in thread
From: Joao Pinto @ 2017-03-10 18:24 UTC (permalink / raw)
  To: davem
  Cc: peppe.cavallaro, alexandre.torgue, niklas.cassel, netdev, Joao Pinto

This patch adds the configuration of the AVB Credit-Based Shaper.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
Changes v4->v5:
- patch title update (stmicro replaced by stmmac)
Changes v3->v4:
- variable init was simplified in some functions
Changes v1->v3:
- Added in v3

 Documentation/devicetree/bindings/net/stmmac.txt   | 22 +++++++++--
 drivers/net/ethernet/stmicro/stmmac/common.h       |  4 ++
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h       | 33 ++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c  | 46 +++++++++++++++++++++-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 29 ++++++++++++++
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  | 29 ++++++++++++--
 include/linux/stmmac.h                             | 12 ++++--
 7 files changed, 164 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/stmmac.txt b/Documentation/devicetree/bindings/net/stmmac.txt
index 4107e67..9b4d5dd 100644
--- a/Documentation/devicetree/bindings/net/stmmac.txt
+++ b/Documentation/devicetree/bindings/net/stmmac.txt
@@ -92,8 +92,15 @@ Optional properties:
 		- 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)
-
+		- snps,weight: TX queue weight (if using a DCB weight algorithm)
+		- Choose one of these modes:
+			- snps,dcb-algorithm: TX queue will be working in DCB
+			- snps,avb-algorithm: TX queue will be working in AVB
+		- Configure Credit Base Shaper (if AVB Mode selected):
+			- snps,send_slope: enable Low Power Interface
+			- snps,idle_slope: unlock on WoL
+			- snps,high_credit: max write outstanding req. limit
+			- snps,low_credit: max read outstanding req. limit
 Examples:
 
 	stmmac_axi_setup: stmmac-axi-config {
@@ -112,10 +119,19 @@ Examples:
 	};
 
 	mtl_tx_setup: tx-queues-config {
-		snps,tx-queues-to-use = <1>;
+		snps,tx-queues-to-use = <2>;
 		snps,tx-sched-wrr;
 		queue0 {
 			snps,weight = <0x10>;
+			snps,dcb-algorithm;
+		};
+
+		queue1 {
+			snps,avb-algorithm;
+			snps,send_slope = <0x1000>;
+			snps,idle_slope = <0x1000>;
+			snps,high_credit = <0x3E800>;
+			snps,low_credit = <0xFFC18000>;
 		};
 	};
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 86f43ac..e3ced07 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -464,6 +464,10 @@ struct stmmac_ops {
 					u32 weight, u32 queue);
 	/* RX MTL queue to RX dma mapping */
 	void (*map_mtl_to_dma)(struct mac_device_info *hw, u32 queue, u32 chan);
+	/* Configure AV Algorithm */
+	void (*config_cbs)(struct mac_device_info *hw, u32 send_slope,
+			   u32 idle_slope, u32 high_credit, u32 low_credit,
+			   u32 queue);
 	/* Dump MAC registers */
 	void (*dump_regs)(struct mac_device_info *hw, u32 *reg_space);
 	/* Handle extra events on specific interrupts hw dependent */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
index 5ca4d64..cf0e602 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
@@ -218,6 +218,15 @@ enum power_event {
 #define MTL_OP_MODE_RTC_96		(2 << MTL_OP_MODE_RTC_SHIFT)
 #define MTL_OP_MODE_RTC_128		(3 << MTL_OP_MODE_RTC_SHIFT)
 
+/* MTL ETS Control register */
+#define MTL_ETS_CTRL_BASE_ADDR		0x00000d10
+#define MTL_ETS_CTRL_BASE_OFFSET	0x40
+#define MTL_ETSX_CTRL_BASE_ADDR(x)	(MTL_ETS_CTRL_BASE_ADDR + \
+					((x) * MTL_ETS_CTRL_BASE_OFFSET))
+
+#define MTL_ETS_CTRL_CC			BIT(3)
+#define MTL_ETS_CTRL_AVALG		BIT(2)
+
 /* MTL Queue Quantum Weight */
 #define MTL_TXQ_WEIGHT_BASE_ADDR	0x00000d18
 #define MTL_TXQ_WEIGHT_BASE_OFFSET	0x40
@@ -225,6 +234,30 @@ enum power_event {
 					((x) * MTL_TXQ_WEIGHT_BASE_OFFSET))
 #define MTL_TXQ_WEIGHT_ISCQW_MASK	GENMASK(20, 0)
 
+/* MTL sendSlopeCredit register */
+#define MTL_SEND_SLP_CRED_BASE_ADDR	0x00000d1c
+#define MTL_SEND_SLP_CRED_OFFSET	0x40
+#define MTL_SEND_SLP_CREDX_BASE_ADDR(x)	(MTL_SEND_SLP_CRED_BASE_ADDR + \
+					((x) * MTL_SEND_SLP_CRED_OFFSET))
+
+#define MTL_SEND_SLP_CRED_SSC_MASK	GENMASK(13, 0)
+
+/* MTL hiCredit register */
+#define MTL_HIGH_CRED_BASE_ADDR		0x00000d20
+#define MTL_HIGH_CRED_OFFSET		0x40
+#define MTL_HIGH_CREDX_BASE_ADDR(x)	(MTL_HIGH_CRED_BASE_ADDR + \
+					((x) * MTL_HIGH_CRED_OFFSET))
+
+#define MTL_HIGH_CRED_HC_MASK		GENMASK(28, 0)
+
+/* MTL loCredit register */
+#define MTL_LOW_CRED_BASE_ADDR		0x00000d24
+#define MTL_LOW_CRED_OFFSET		0x40
+#define MTL_LOW_CREDX_BASE_ADDR(x)	(MTL_LOW_CRED_BASE_ADDR + \
+					((x) * MTL_LOW_CRED_OFFSET))
+
+#define MTL_HIGH_CRED_LC_MASK		GENMASK(28, 0)
+
 /*  MTL debug */
 #define MTL_DEBUG_TXSTSFSTS		BIT(5)
 #define MTL_DEBUG_TXFSTS		BIT(4)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 670cfee..10599db 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -66,9 +66,9 @@ static void dwmac4_rx_queue_enable(struct mac_device_info *hw,
 	u32 value = readl(ioaddr + GMAC_RXQ_CTRL0);
 
 	value &= GMAC_RX_QUEUE_CLEAR(queue);
-	if (mode == MTL_RX_AVB)
+	if (mode == MTL_QUEUE_AVB)
 		value |= GMAC_RX_AV_QUEUE_ENABLE(queue);
-	else if (mode == MTL_RX_DCB)
+	else if (mode == MTL_QUEUE_DCB)
 		value |= GMAC_RX_DCB_QUEUE_ENABLE(queue);
 
 	writel(value, ioaddr + GMAC_RXQ_CTRL0);
@@ -155,6 +155,47 @@ static void dwmac4_map_mtl_dma(struct mac_device_info *hw, u32 queue, u32 chan)
 		writel(value, ioaddr + MTL_RXQ_DMA_MAP1);
 }
 
+static void dwmac4_config_cbs(struct mac_device_info *hw,
+			      u32 send_slope, u32 idle_slope,
+			      u32 high_credit, u32 low_credit, u32 queue)
+{
+	void __iomem *ioaddr = hw->pcsr;
+	u32 value;
+
+	pr_debug("Queue %d configured as AVB. Parameters:\n", queue);
+	pr_debug("\tsend_slope: 0x%08x\n", send_slope);
+	pr_debug("\tidle_slope: 0x%08x\n", idle_slope);
+	pr_debug("\thigh_credit: 0x%08x\n", high_credit);
+	pr_debug("\tlow_credit: 0x%08x\n", low_credit);
+
+	/* enable AV algorithm */
+	value = readl(ioaddr + MTL_ETSX_CTRL_BASE_ADDR(queue));
+	value |= MTL_ETS_CTRL_AVALG;
+	value |= MTL_ETS_CTRL_CC;
+	writel(value, ioaddr + MTL_ETSX_CTRL_BASE_ADDR(queue));
+
+	/* configure send slope */
+	value = readl(ioaddr + MTL_SEND_SLP_CREDX_BASE_ADDR(queue));
+	value &= ~MTL_SEND_SLP_CRED_SSC_MASK;
+	value |= send_slope & MTL_SEND_SLP_CRED_SSC_MASK;
+	writel(value, ioaddr + MTL_SEND_SLP_CREDX_BASE_ADDR(queue));
+
+	/* configure idle slope (same register as tx weight) */
+	dwmac4_set_mtl_tx_queue_weight(hw, idle_slope, queue);
+
+	/* configure high credit */
+	value = readl(ioaddr + MTL_HIGH_CREDX_BASE_ADDR(queue));
+	value &= ~MTL_HIGH_CRED_HC_MASK;
+	value |= high_credit & MTL_HIGH_CRED_HC_MASK;
+	writel(value, ioaddr + MTL_HIGH_CREDX_BASE_ADDR(queue));
+
+	/* configure high credit */
+	value = readl(ioaddr + MTL_LOW_CREDX_BASE_ADDR(queue));
+	value &= ~MTL_HIGH_CRED_LC_MASK;
+	value |= low_credit & MTL_HIGH_CRED_LC_MASK;
+	writel(value, ioaddr + MTL_LOW_CREDX_BASE_ADDR(queue));
+}
+
 static void dwmac4_dump_regs(struct mac_device_info *hw, u32 *reg_space)
 {
 	void __iomem *ioaddr = hw->pcsr;
@@ -566,6 +607,7 @@ static const struct stmmac_ops dwmac4_ops = {
 	.prog_mtl_tx_algorithms = dwmac4_prog_mtl_tx_algorithms,
 	.set_mtl_tx_queue_weight = dwmac4_set_mtl_tx_queue_weight,
 	.map_mtl_to_dma = dwmac4_map_mtl_dma,
+	.config_cbs = dwmac4_config_cbs,
 	.dump_regs = dwmac4_dump_regs,
 	.host_irq_status = dwmac4_irq_status,
 	.host_mtl_irq_status = dwmac4_irq_mtl_status,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 3be77d4..ee5cb16 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1668,6 +1668,31 @@ static void stmmac_set_tx_queue_weight(struct stmmac_priv *priv)
 }
 
 /**
+ *  stmmac_configure_cbs - Configure CBS in TX queue
+ *  @priv: driver private structure
+ *  Description: It is used for configuring CBS in AVB TX queues
+ */
+static void stmmac_configure_cbs(struct stmmac_priv *priv)
+{
+	u32 tx_queues_count = priv->plat->tx_queues_to_use;
+	u32 mode_to_use;
+	u32 queue;
+
+	for (queue = 0; queue < tx_queues_count; queue++) {
+		mode_to_use = priv->plat->tx_queues_cfg[queue].mode_to_use;
+		if (mode_to_use == MTL_QUEUE_DCB)
+			continue;
+
+		priv->hw->mac->config_cbs(priv->hw,
+				priv->plat->tx_queues_cfg[queue].send_slope,
+				priv->plat->tx_queues_cfg[queue].idle_slope,
+				priv->plat->tx_queues_cfg[queue].high_credit,
+				priv->plat->tx_queues_cfg[queue].low_credit,
+				queue);
+	}
+}
+
+/**
  *  stmmac_rx_queue_dma_chan_map - Map RX queue to RX dma channel
  *  @priv: driver private structure
  *  Description: It is used for mapping RX queues to RX dma channels
@@ -1707,6 +1732,10 @@ static void stmmac_mtl_configuration(struct stmmac_priv *priv)
 		priv->hw->mac->prog_mtl_tx_algorithms(priv->hw,
 						priv->plat->tx_sched_algorithm);
 
+	/* Configure CBS in AVB TX queues */
+	if (tx_queues_count > 1 && priv->hw->mac->config_cbs)
+		stmmac_configure_cbs(priv);
+
 	/* Map RX MTL to DMA channels */
 	if (rx_queues_count > 1 && priv->hw->mac->map_mtl_to_dma)
 		stmmac_rx_queue_dma_chan_map(priv);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index ff6af8d..fc339bf 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -171,11 +171,11 @@ static void stmmac_mtl_setup(struct platform_device *pdev,
 			break;
 
 		if (of_property_read_bool(q_node, "snps,dcb-algorithm"))
-			plat->rx_queues_cfg[queue].mode_to_use = MTL_RX_DCB;
+			plat->rx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;
 		else if (of_property_read_bool(q_node, "snps,avb-algorithm"))
-			plat->rx_queues_cfg[queue].mode_to_use = MTL_RX_AVB;
+			plat->rx_queues_cfg[queue].mode_to_use = MTL_QUEUE_AVB;
 		else
-			plat->rx_queues_cfg[queue].mode_to_use = MTL_RX_DCB;
+			plat->rx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;
 
 		if (of_property_read_u8(q_node, "snps,map-to-dma-channel",
 					&plat->rx_queues_cfg[queue].chan))
@@ -212,6 +212,29 @@ static void stmmac_mtl_setup(struct platform_device *pdev,
 					&plat->tx_queues_cfg[queue].weight))
 			plat->tx_queues_cfg[queue].weight = 0x10 + queue;
 
+		if (of_property_read_bool(q_node, "snps,dcb-algorithm")) {
+			plat->tx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;
+		} else if (of_property_read_bool(q_node,
+						 "snps,avb-algorithm")) {
+			plat->tx_queues_cfg[queue].mode_to_use = MTL_QUEUE_AVB;
+
+			/* Credit Base Shaper parameters used by AVB */
+			if (of_property_read_u32(q_node, "snps,send_slope",
+				&plat->tx_queues_cfg[queue].send_slope))
+				plat->tx_queues_cfg[queue].send_slope = 0x0;
+			if (of_property_read_u32(q_node, "snps,idle_slope",
+				&plat->tx_queues_cfg[queue].idle_slope))
+				plat->tx_queues_cfg[queue].idle_slope = 0x0;
+			if (of_property_read_u32(q_node, "snps,high_credit",
+				&plat->tx_queues_cfg[queue].high_credit))
+				plat->tx_queues_cfg[queue].high_credit = 0x0;
+			if (of_property_read_u32(q_node, "snps,low_credit",
+				&plat->tx_queues_cfg[queue].low_credit))
+				plat->tx_queues_cfg[queue].low_credit = 0x0;
+		} else {
+			plat->tx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;
+		}
+
 		queue++;
 	}
 
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 266ff2a..be47b85 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -55,9 +55,9 @@
 #define MTL_RX_ALGORITHM_SP	0x4
 #define MTL_RX_ALGORITHM_WSP	0x5
 
-/* RX Queue Mode */
-#define MTL_RX_DCB		0x0
-#define MTL_RX_AVB		0x1
+/* RX/TX Queue Mode */
+#define MTL_QUEUE_DCB		0x0
+#define MTL_QUEUE_AVB		0x1
 
 /* The MDC clock could be set higher than the IEEE 802.3
  * specified frequency limit 0f 2.5 MHz, by programming a clock divider
@@ -131,6 +131,12 @@ struct stmmac_rxq_cfg {
 
 struct stmmac_txq_cfg {
 	u8 weight;
+	u8 mode_to_use;
+	/* Credit Base Shaper parameters */
+	u32 send_slope;
+	u32 idle_slope;
+	u32 high_credit;
+	u32 low_credit;
 };
 
 struct plat_stmmacenet_data {
-- 
2.9.3

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

* Re: [PATCH v5 net-next 0/9] prepare mac operations for multiple queues
  2017-03-10 18:24 [PATCH v5 net-next 0/9] prepare mac operations for multiple queues Joao Pinto
                   ` (8 preceding siblings ...)
  2017-03-10 18:24 ` [PATCH v5 net-next 9/9] net: stmmac: configuration of CBS in case of a TX AVB queue Joao Pinto
@ 2017-03-13  6:42 ` David Miller
  9 siblings, 0 replies; 26+ messages in thread
From: David Miller @ 2017-03-13  6:42 UTC (permalink / raw)
  To: Joao.Pinto; +Cc: peppe.cavallaro, alexandre.torgue, niklas.cassel, netdev

From: Joao Pinto <Joao.Pinto@synopsys.com>
Date: Fri, 10 Mar 2017 18:24:50 +0000

> 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).

Series applied, thanks.

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

* Re: [v5,net-next,1/9] net: stmmac: multiple queues dt configuration
       [not found]   ` <c315778b650a846db3c04816fd1d1041bb1e4043.1489169213.git.jpinto-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
@ 2017-03-21 11:32     ` Thierry Reding
       [not found]       ` <20170321113245.GA10526-EkSeR96xj6Pcmrwk2tT4+A@public.gmane.org>
  0 siblings, 1 reply; 26+ messages in thread
From: Thierry Reding @ 2017-03-21 11:32 UTC (permalink / raw)
  To: Joao Pinto
  Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, peppe.cavallaro-qxv4g6HH51o,
	alexandre.torgue-qxv4g6HH51o, niklas.cassel-VrBV9hrLPhE,
	netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 3373 bytes --]

On Fri, Mar 10, 2017 at 06:24:51PM +0000, Joao Pinto wrote:
> 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/w@public.gmane.org>
> ---
> 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(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/stmmac.txt b/Documentation/devicetree/bindings/net/stmmac.txt
> index d3bfc2b..4107e67 100644
> --- a/Documentation/devicetree/bindings/net/stmmac.txt
> +++ b/Documentation/devicetree/bindings/net/stmmac.txt
> @@ -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>;
> @@ -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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [v5,net-next,1/9] net: stmmac: multiple queues dt configuration
       [not found]       ` <20170321113245.GA10526-EkSeR96xj6Pcmrwk2tT4+A@public.gmane.org>
@ 2017-03-21 11:39         ` Joao Pinto
       [not found]           ` <340aba35-de12-14a8-749c-5507edab449e-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
  0 siblings, 1 reply; 26+ messages in thread
From: Joao Pinto @ 2017-03-21 11:39 UTC (permalink / raw)
  To: Thierry Reding, Joao Pinto
  Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, peppe.cavallaro-qxv4g6HH51o,
	alexandre.torgue-qxv4g6HH51o, niklas.cassel-VrBV9hrLPhE,
	netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA


Hi Thierry,

Às 11:32 AM de 3/21/2017, Thierry Reding escreveu:
> On Fri, Mar 10, 2017 at 06:24:51PM +0000, Joao Pinto wrote:
>> 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/w@public.gmane.org>
>> ---
>> 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(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/stmmac.txt b/Documentation/devicetree/bindings/net/stmmac.txt
>> index d3bfc2b..4107e67 100644
>> --- a/Documentation/devicetree/bindings/net/stmmac.txt
>> +++ b/Documentation/devicetree/bindings/net/stmmac.txt
>> @@ -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>;
>> @@ -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
> 

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.

Thanks,
Joao


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [v5,net-next,2/9] net: stmmac: configure mtl rx and tx algorithms
  2017-03-10 18:24 ` [PATCH v5 net-next 2/9] net: stmmac: configure mtl rx and tx algorithms Joao Pinto
@ 2017-03-21 11:58   ` Thierry Reding
  2017-03-21 12:02     ` Joao Pinto
  0 siblings, 1 reply; 26+ messages in thread
From: Thierry Reding @ 2017-03-21 11:58 UTC (permalink / raw)
  To: Joao Pinto
  Cc: davem, peppe.cavallaro, alexandre.torgue, niklas.cassel, netdev

[-- Attachment #1: Type: text/plain, Size: 7204 bytes --]

On Fri, Mar 10, 2017 at 06:24:52PM +0000, Joao Pinto wrote:
> 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 <jpinto@synopsys.com>
> ---
> 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(-)

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

See below...

> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
> index 04d9245..5a0a781 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> @@ -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 */
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> index db45134..748ab6f 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> @@ -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)
>  
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> index 1e79e65..f966755 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> @@ -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,
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 4498a38..af57f8d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -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,
> +						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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [v5,net-next,2/9] net: stmmac: configure mtl rx and tx algorithms
  2017-03-21 11:58   ` [v5,net-next,2/9] " Thierry Reding
@ 2017-03-21 12:02     ` Joao Pinto
  2017-03-21 12:24       ` Thierry Reding
  0 siblings, 1 reply; 26+ messages in thread
From: Joao Pinto @ 2017-03-21 12:02 UTC (permalink / raw)
  To: Thierry Reding, Joao Pinto
  Cc: davem, peppe.cavallaro, alexandre.torgue, niklas.cassel, netdev

Às 11:58 AM de 3/21/2017, Thierry Reding escreveu:
> On Fri, Mar 10, 2017 at 06:24:52PM +0000, Joao Pinto wrote:
>> 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 <jpinto@synopsys.com>
>> ---
>> 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(-)
> 
> This patch breaks backwards-compatibility with DTBs that don't have an
> of the multiple queue properties.
> 
> See below...
> 
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
>> index 04d9245..5a0a781 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
>> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
>> @@ -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 */
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>> index db45134..748ab6f 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>> @@ -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)
>>  
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
>> index 1e79e65..f966755 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
>> @@ -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,
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index 4498a38..af57f8d 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -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,
>> +						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.

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

Thanks for reviewing.

> 
> Thierry
> 

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

* Re: [v5,net-next,1/9] net: stmmac: multiple queues dt configuration
       [not found]           ` <340aba35-de12-14a8-749c-5507edab449e-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
@ 2017-03-21 12:10             ` Thierry Reding
  0 siblings, 0 replies; 26+ messages in thread
From: Thierry Reding @ 2017-03-21 12:10 UTC (permalink / raw)
  To: Joao Pinto
  Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, peppe.cavallaro-qxv4g6HH51o,
	alexandre.torgue-qxv4g6HH51o, niklas.cassel-VrBV9hrLPhE,
	netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 4339 bytes --]

On Tue, Mar 21, 2017 at 11:39:24AM +0000, Joao Pinto wrote:
> 
> Hi Thierry,
> 
> Às 11:32 AM de 3/21/2017, Thierry Reding escreveu:
> > On Fri, Mar 10, 2017 at 06:24:51PM +0000, Joao Pinto wrote:
> >> 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/w@public.gmane.org>
> >> ---
> >> 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(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/net/stmmac.txt b/Documentation/devicetree/bindings/net/stmmac.txt
> >> index d3bfc2b..4107e67 100644
> >> --- a/Documentation/devicetree/bindings/net/stmmac.txt
> >> +++ b/Documentation/devicetree/bindings/net/stmmac.txt
> >> @@ -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>;
> >> @@ -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
> > 
> 
> 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.

Okay, I see. I've never really seen this done, and I suspect the device
tree maintainers would've objected to the AXI bindings, but looks like
it fell through the cracks. Given that this has been upstream for a very
long time now, it is what it is, and I suppose this is at least
consistent.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [v5,net-next,2/9] net: stmmac: configure mtl rx and tx algorithms
  2017-03-21 12:02     ` Joao Pinto
@ 2017-03-21 12:24       ` Thierry Reding
  2017-03-21 13:58         ` Joao Pinto
  0 siblings, 1 reply; 26+ messages in thread
From: Thierry Reding @ 2017-03-21 12:24 UTC (permalink / raw)
  To: Joao Pinto
  Cc: davem, peppe.cavallaro, alexandre.torgue, niklas.cassel, netdev

[-- Attachment #1: Type: text/plain, Size: 8945 bytes --]

On Tue, Mar 21, 2017 at 12:02:03PM +0000, Joao Pinto wrote:
> Às 11:58 AM de 3/21/2017, Thierry Reding escreveu:
> > On Fri, Mar 10, 2017 at 06:24:52PM +0000, Joao Pinto wrote:
> >> 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 <jpinto@synopsys.com>
> >> ---
> >> 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(-)
> > 
> > This patch breaks backwards-compatibility with DTBs that don't have an
> > of the multiple queue properties.
> > 
> > See below...
> > 
> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
> >> index 04d9245..5a0a781 100644
> >> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> >> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> >> @@ -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 */
> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> >> index db45134..748ab6f 100644
> >> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> >> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> >> @@ -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)
> >>  
> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> >> index 1e79e65..f966755 100644
> >> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> >> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> >> @@ -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,
> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >> index 4498a38..af57f8d 100644
> >> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >> @@ -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,
> >> +						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.
> 
> 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

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [v5,net-next,2/9] net: stmmac: configure mtl rx and tx algorithms
  2017-03-21 12:24       ` Thierry Reding
@ 2017-03-21 13:58         ` Joao Pinto
  2017-03-21 14:08           ` Thierry Reding
  0 siblings, 1 reply; 26+ messages in thread
From: Joao Pinto @ 2017-03-21 13:58 UTC (permalink / raw)
  To: Thierry Reding, Joao Pinto
  Cc: davem, peppe.cavallaro, alexandre.torgue, niklas.cassel, netdev

Às 12:24 PM de 3/21/2017, Thierry Reding escreveu:
> On Tue, Mar 21, 2017 at 12:02:03PM +0000, Joao Pinto wrote:
>> Às 11:58 AM de 3/21/2017, Thierry Reding escreveu:
>>> On Fri, Mar 10, 2017 at 06:24:52PM +0000, Joao Pinto wrote:
>>>> 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 <jpinto@synopsys.com>
>>>> ---
>>>> 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(-)
>>>
>>> This patch breaks backwards-compatibility with DTBs that don't have an
>>> of the multiple queue properties.
>>>
>>> See below...
>>>
>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
>>>> index 04d9245..5a0a781 100644
>>>> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
>>>> @@ -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 */
>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>>> index db45134..748ab6f 100644
>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>>> @@ -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)
>>>>  
>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
>>>> index 1e79e65..f966755 100644
>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
>>>> @@ -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,
>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>> index 4498a38..af57f8d 100644
>>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>> @@ -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,
>>>> +						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.
>>
>> 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= 
> 
> 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.

> 
> 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
> 

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

* Re: [v5,net-next,2/9] net: stmmac: configure mtl rx and tx algorithms
  2017-03-21 13:58         ` Joao Pinto
@ 2017-03-21 14:08           ` Thierry Reding
  2017-03-21 14:10             ` Joao Pinto
  2017-03-21 14:18             ` Joao Pinto
  0 siblings, 2 replies; 26+ messages in thread
From: Thierry Reding @ 2017-03-21 14:08 UTC (permalink / raw)
  To: Joao Pinto
  Cc: davem, peppe.cavallaro, alexandre.torgue, niklas.cassel, netdev

[-- Attachment #1: Type: text/plain, Size: 10271 bytes --]

On Tue, Mar 21, 2017 at 01:58:36PM +0000, Joao Pinto wrote:
> Às 12:24 PM de 3/21/2017, Thierry Reding escreveu:
> > On Tue, Mar 21, 2017 at 12:02:03PM +0000, Joao Pinto wrote:
> >> Às 11:58 AM de 3/21/2017, Thierry Reding escreveu:
> >>> On Fri, Mar 10, 2017 at 06:24:52PM +0000, Joao Pinto wrote:
> >>>> 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 <jpinto@synopsys.com>
> >>>> ---
> >>>> 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(-)
> >>>
> >>> This patch breaks backwards-compatibility with DTBs that don't have an
> >>> of the multiple queue properties.
> >>>
> >>> See below...
> >>>
> >>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
> >>>> index 04d9245..5a0a781 100644
> >>>> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> >>>> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> >>>> @@ -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 */
> >>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> >>>> index db45134..748ab6f 100644
> >>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> >>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> >>>> @@ -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)
> >>>>  
> >>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> >>>> index 1e79e65..f966755 100644
> >>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> >>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> >>>> @@ -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,
> >>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >>>> index 4498a38..af57f8d 100644
> >>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >>>> @@ -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,
> >>>> +						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.
> >>
> >> 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= 
> > 
> > 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [v5,net-next,2/9] net: stmmac: configure mtl rx and tx algorithms
  2017-03-21 14:08           ` Thierry Reding
@ 2017-03-21 14:10             ` Joao Pinto
  2017-03-21 14:23               ` Corentin Labbe
  2017-03-21 14:18             ` Joao Pinto
  1 sibling, 1 reply; 26+ messages in thread
From: Joao Pinto @ 2017-03-21 14:10 UTC (permalink / raw)
  To: Thierry Reding, Joao Pinto
  Cc: davem, peppe.cavallaro, alexandre.torgue, niklas.cassel, netdev,
	Corentin Labbe

++Adding Corentin

Às 2:08 PM de 3/21/2017, Thierry Reding escreveu:
> On Tue, Mar 21, 2017 at 01:58:36PM +0000, Joao Pinto wrote:
>> Às 12:24 PM de 3/21/2017, Thierry Reding escreveu:
>>> On Tue, Mar 21, 2017 at 12:02:03PM +0000, Joao Pinto wrote:
>>>> Às 11:58 AM de 3/21/2017, Thierry Reding escreveu:
>>>>> On Fri, Mar 10, 2017 at 06:24:52PM +0000, Joao Pinto wrote:
>>>>>> 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 <jpinto@synopsys.com>
>>>>>> ---
>>>>>> 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(-)
>>>>>
>>>>> This patch breaks backwards-compatibility with DTBs that don't have an
>>>>> of the multiple queue properties.
>>>>>
>>>>> See below...
>>>>>
>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
>>>>>> index 04d9245..5a0a781 100644
>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
>>>>>> @@ -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 */
>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>>>>> index db45134..748ab6f 100644
>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>>>>> @@ -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)
>>>>>>  
>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
>>>>>> index 1e79e65..f966755 100644
>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
>>>>>> @@ -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,
>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>>> index 4498a38..af57f8d 100644
>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>>> @@ -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,
>>>>>> +						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.
>>>>
>>>> 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= 
>>>
>>> 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.

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?

> 
> Thierry
> 

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

* Re: [v5,net-next,2/9] net: stmmac: configure mtl rx and tx algorithms
  2017-03-21 14:08           ` Thierry Reding
  2017-03-21 14:10             ` Joao Pinto
@ 2017-03-21 14:18             ` Joao Pinto
  2017-03-21 14:35               ` Thierry Reding
  1 sibling, 1 reply; 26+ messages in thread
From: Joao Pinto @ 2017-03-21 14:18 UTC (permalink / raw)
  To: Thierry Reding, Joao Pinto
  Cc: davem, peppe.cavallaro, alexandre.torgue, niklas.cassel, netdev

Às 2:08 PM de 3/21/2017, Thierry Reding escreveu:
> On Tue, Mar 21, 2017 at 01:58:36PM +0000, Joao Pinto wrote:
>> Às 12:24 PM de 3/21/2017, Thierry Reding escreveu:
>>> On Tue, Mar 21, 2017 at 12:02:03PM +0000, Joao Pinto wrote:
>>>> Às 11:58 AM de 3/21/2017, Thierry Reding escreveu:
>>>>> On Fri, Mar 10, 2017 at 06:24:52PM +0000, Joao Pinto wrote:
>>>>>> 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 <jpinto@synopsys.com>
>>>>>> ---
>>>>>> 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(-)
>>>>>
>>>>> This patch breaks backwards-compatibility with DTBs that don't have an
>>>>> of the multiple queue properties.
>>>>>
>>>>> See below...
>>>>>
>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
>>>>>> index 04d9245..5a0a781 100644
>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
>>>>>> @@ -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 */
>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>>>>> index db45134..748ab6f 100644
>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>>>>> @@ -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)
>>>>>>  
>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
>>>>>> index 1e79e65..f966755 100644
>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
>>>>>> @@ -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,
>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>>> index 4498a38..af57f8d 100644
>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>>> @@ -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,
>>>>>> +						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.
>>>>
>>>> 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= 
>>>
>>> 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.

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.

Joao

> 
> Thierry
> 

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

* Re: [v5,net-next,2/9] net: stmmac: configure mtl rx and tx algorithms
  2017-03-21 14:10             ` Joao Pinto
@ 2017-03-21 14:23               ` Corentin Labbe
  2017-03-21 14:25                 ` Joao Pinto
  0 siblings, 1 reply; 26+ messages in thread
From: Corentin Labbe @ 2017-03-21 14:23 UTC (permalink / raw)
  To: Joao Pinto
  Cc: Thierry Reding, davem, peppe.cavallaro, alexandre.torgue,
	niklas.cassel, netdev

On Tue, Mar 21, 2017 at 02:10:47PM +0000, Joao Pinto wrote:
> ++Adding Corentin
> 
> Às 2:08 PM de 3/21/2017, Thierry Reding escreveu:
> > On Tue, Mar 21, 2017 at 01:58:36PM +0000, Joao Pinto wrote:
> >> Às 12:24 PM de 3/21/2017, Thierry Reding escreveu:
> >>> On Tue, Mar 21, 2017 at 12:02:03PM +0000, Joao Pinto wrote:
> >>>> Às 11:58 AM de 3/21/2017, Thierry Reding escreveu:
> >>>>> On Fri, Mar 10, 2017 at 06:24:52PM +0000, Joao Pinto wrote:
> >>>>>> 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 <jpinto@synopsys.com>
> >>>>>> ---
> >>>>>> 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(-)
> >>>>>
> >>>>> This patch breaks backwards-compatibility with DTBs that don't have an
> >>>>> of the multiple queue properties.
> >>>>>
> >>>>> See below...
> >>>>>
> >>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
> >>>>>> index 04d9245..5a0a781 100644
> >>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> >>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> >>>>>> @@ -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 */
> >>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> >>>>>> index db45134..748ab6f 100644
> >>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> >>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> >>>>>> @@ -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)
> >>>>>>  
> >>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> >>>>>> index 1e79e65..f966755 100644
> >>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> >>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> >>>>>> @@ -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,
> >>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >>>>>> index 4498a38..af57f8d 100644
> >>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >>>>>> @@ -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,
> >>>>>> +						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.
> >>>>
> >>>> 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= 
> >>>
> >>> 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.
> 
> 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.

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

* Re: [v5,net-next,2/9] net: stmmac: configure mtl rx and tx algorithms
  2017-03-21 14:23               ` Corentin Labbe
@ 2017-03-21 14:25                 ` Joao Pinto
  2017-03-21 14:33                   ` Thierry Reding
  0 siblings, 1 reply; 26+ messages in thread
From: Joao Pinto @ 2017-03-21 14:25 UTC (permalink / raw)
  To: Corentin Labbe, Joao Pinto
  Cc: Thierry Reding, davem, peppe.cavallaro, alexandre.torgue,
	niklas.cassel, netdev

Às 2:23 PM de 3/21/2017, Corentin Labbe escreveu:
> On Tue, Mar 21, 2017 at 02:10:47PM +0000, Joao Pinto wrote:
>> ++Adding Corentin
>>
>> Às 2:08 PM de 3/21/2017, Thierry Reding escreveu:
>>> On Tue, Mar 21, 2017 at 01:58:36PM +0000, Joao Pinto wrote:
>>>> Às 12:24 PM de 3/21/2017, Thierry Reding escreveu:
>>>>> On Tue, Mar 21, 2017 at 12:02:03PM +0000, Joao Pinto wrote:
>>>>>> Às 11:58 AM de 3/21/2017, Thierry Reding escreveu:
>>>>>>> On Fri, Mar 10, 2017 at 06:24:52PM +0000, Joao Pinto wrote:
>>>>>>>> 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 <jpinto@synopsys.com>
>>>>>>>> ---
>>>>>>>> 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(-)
>>>>>>>
>>>>>>> This patch breaks backwards-compatibility with DTBs that don't have an
>>>>>>> of the multiple queue properties.
>>>>>>>
>>>>>>> See below...
>>>>>>>
>>>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
>>>>>>>> index 04d9245..5a0a781 100644
>>>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
>>>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
>>>>>>>> @@ -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 */
>>>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>>>>>>> index db45134..748ab6f 100644
>>>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>>>>>>> @@ -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)
>>>>>>>>  
>>>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
>>>>>>>> index 1e79e65..f966755 100644
>>>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
>>>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
>>>>>>>> @@ -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,
>>>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>>>>> index 4498a38..af57f8d 100644
>>>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>>>>> @@ -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,
>>>>>>>> +						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.
>>>>>>
>>>>>> 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= 
>>>>>
>>>>> 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.
>>
>> 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.
> 

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!

Joao

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

* Re: [v5,net-next,2/9] net: stmmac: configure mtl rx and tx algorithms
  2017-03-21 14:25                 ` Joao Pinto
@ 2017-03-21 14:33                   ` Thierry Reding
  2017-03-21 14:35                     ` Joao Pinto
  0 siblings, 1 reply; 26+ messages in thread
From: Thierry Reding @ 2017-03-21 14:33 UTC (permalink / raw)
  To: Joao Pinto
  Cc: Corentin Labbe, davem, peppe.cavallaro, alexandre.torgue,
	niklas.cassel, netdev

[-- Attachment #1: Type: text/plain, Size: 12331 bytes --]

On Tue, Mar 21, 2017 at 02:25:15PM +0000, Joao Pinto wrote:
> Às 2:23 PM de 3/21/2017, Corentin Labbe escreveu:
> > On Tue, Mar 21, 2017 at 02:10:47PM +0000, Joao Pinto wrote:
> >> ++Adding Corentin
> >>
> >> Às 2:08 PM de 3/21/2017, Thierry Reding escreveu:
> >>> On Tue, Mar 21, 2017 at 01:58:36PM +0000, Joao Pinto wrote:
> >>>> Às 12:24 PM de 3/21/2017, Thierry Reding escreveu:
> >>>>> On Tue, Mar 21, 2017 at 12:02:03PM +0000, Joao Pinto wrote:
> >>>>>> Às 11:58 AM de 3/21/2017, Thierry Reding escreveu:
> >>>>>>> On Fri, Mar 10, 2017 at 06:24:52PM +0000, Joao Pinto wrote:
> >>>>>>>> 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 <jpinto@synopsys.com>
> >>>>>>>> ---
> >>>>>>>> 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(-)
> >>>>>>>
> >>>>>>> This patch breaks backwards-compatibility with DTBs that don't have an
> >>>>>>> of the multiple queue properties.
> >>>>>>>
> >>>>>>> See below...
> >>>>>>>
> >>>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
> >>>>>>>> index 04d9245..5a0a781 100644
> >>>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> >>>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> >>>>>>>> @@ -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 */
> >>>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> >>>>>>>> index db45134..748ab6f 100644
> >>>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> >>>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> >>>>>>>> @@ -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)
> >>>>>>>>  
> >>>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> >>>>>>>> index 1e79e65..f966755 100644
> >>>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> >>>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> >>>>>>>> @@ -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,
> >>>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >>>>>>>> index 4498a38..af57f8d 100644
> >>>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >>>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >>>>>>>> @@ -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,
> >>>>>>>> +						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.
> >>>>>>
> >>>>>> 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= 
> >>>>>
> >>>>> 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.
> >>
> >> 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.
> > 
> 
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [v5,net-next,2/9] net: stmmac: configure mtl rx and tx algorithms
  2017-03-21 14:33                   ` Thierry Reding
@ 2017-03-21 14:35                     ` Joao Pinto
  0 siblings, 0 replies; 26+ messages in thread
From: Joao Pinto @ 2017-03-21 14:35 UTC (permalink / raw)
  To: Thierry Reding, Joao Pinto
  Cc: Corentin Labbe, davem, peppe.cavallaro, alexandre.torgue,
	niklas.cassel, netdev

Às 2:33 PM de 3/21/2017, Thierry Reding escreveu:
> On Tue, Mar 21, 2017 at 02:25:15PM +0000, Joao Pinto wrote:
>> Às 2:23 PM de 3/21/2017, Corentin Labbe escreveu:
>>> On Tue, Mar 21, 2017 at 02:10:47PM +0000, Joao Pinto wrote:
>>>> ++Adding Corentin
>>>>
>>>> Às 2:08 PM de 3/21/2017, Thierry Reding escreveu:
>>>>> On Tue, Mar 21, 2017 at 01:58:36PM +0000, Joao Pinto wrote:
>>>>>> Às 12:24 PM de 3/21/2017, Thierry Reding escreveu:
>>>>>>> On Tue, Mar 21, 2017 at 12:02:03PM +0000, Joao Pinto wrote:
>>>>>>>> Às 11:58 AM de 3/21/2017, Thierry Reding escreveu:
>>>>>>>>> On Fri, Mar 10, 2017 at 06:24:52PM +0000, Joao Pinto wrote:
>>>>>>>>>> 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 <jpinto@synopsys.com>
>>>>>>>>>> ---
>>>>>>>>>> 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(-)
>>>>>>>>>
>>>>>>>>> This patch breaks backwards-compatibility with DTBs that don't have an
>>>>>>>>> of the multiple queue properties.
>>>>>>>>>
>>>>>>>>> See below...
>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
>>>>>>>>>> index 04d9245..5a0a781 100644
>>>>>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
>>>>>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
>>>>>>>>>> @@ -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 */
>>>>>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>>>>>>>>> index db45134..748ab6f 100644
>>>>>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>>>>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>>>>>>>>> @@ -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)
>>>>>>>>>>  
>>>>>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
>>>>>>>>>> index 1e79e65..f966755 100644
>>>>>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
>>>>>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
>>>>>>>>>> @@ -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,
>>>>>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>>>>>>> index 4498a38..af57f8d 100644
>>>>>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>>>>>>> @@ -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,
>>>>>>>>>> +						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.
>>>>>>>>
>>>>>>>> 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= 
>>>>>>>
>>>>>>> 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.
>>>>
>>>> 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.
>>>
>>
>> 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
> 

Great thanks!

Joao

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

* Re: [v5,net-next,2/9] net: stmmac: configure mtl rx and tx algorithms
  2017-03-21 14:18             ` Joao Pinto
@ 2017-03-21 14:35               ` Thierry Reding
  0 siblings, 0 replies; 26+ messages in thread
From: Thierry Reding @ 2017-03-21 14:35 UTC (permalink / raw)
  To: Joao Pinto
  Cc: davem, peppe.cavallaro, alexandre.torgue, niklas.cassel, netdev

[-- Attachment #1: Type: text/plain, Size: 11533 bytes --]

On Tue, Mar 21, 2017 at 02:18:59PM +0000, Joao Pinto wrote:
> Às 2:08 PM de 3/21/2017, Thierry Reding escreveu:
> > On Tue, Mar 21, 2017 at 01:58:36PM +0000, Joao Pinto wrote:
> >> Às 12:24 PM de 3/21/2017, Thierry Reding escreveu:
> >>> On Tue, Mar 21, 2017 at 12:02:03PM +0000, Joao Pinto wrote:
> >>>> Às 11:58 AM de 3/21/2017, Thierry Reding escreveu:
> >>>>> On Fri, Mar 10, 2017 at 06:24:52PM +0000, Joao Pinto wrote:
> >>>>>> 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 <jpinto@synopsys.com>
> >>>>>> ---
> >>>>>> 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(-)
> >>>>>
> >>>>> This patch breaks backwards-compatibility with DTBs that don't have an
> >>>>> of the multiple queue properties.
> >>>>>
> >>>>> See below...
> >>>>>
> >>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
> >>>>>> index 04d9245..5a0a781 100644
> >>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> >>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> >>>>>> @@ -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 */
> >>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> >>>>>> index db45134..748ab6f 100644
> >>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> >>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> >>>>>> @@ -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)
> >>>>>>  
> >>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> >>>>>> index 1e79e65..f966755 100644
> >>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> >>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> >>>>>> @@ -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,
> >>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >>>>>> index 4498a38..af57f8d 100644
> >>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >>>>>> @@ -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,
> >>>>>> +						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.
> >>>>
> >>>> 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= 
> >>>
> >>> 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.
> 
> 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.

I fully agree. The point I was arguing is that this is not how it
currently works. However, I've got a set of patches to fix these issues
and restore things on a Tegra186 (and likely also on other devices that
are probably also broken at the moment).

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

end of thread, other threads:[~2017-03-21 14:35 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-10 18:24 [PATCH v5 net-next 0/9] prepare mac operations for multiple queues Joao Pinto
2017-03-10 18:24 ` [PATCH v5 net-next 1/9] net: stmmac: multiple queues dt configuration Joao Pinto
     [not found]   ` <c315778b650a846db3c04816fd1d1041bb1e4043.1489169213.git.jpinto-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
2017-03-21 11:32     ` [v5,net-next,1/9] " Thierry Reding
     [not found]       ` <20170321113245.GA10526-EkSeR96xj6Pcmrwk2tT4+A@public.gmane.org>
2017-03-21 11:39         ` Joao Pinto
     [not found]           ` <340aba35-de12-14a8-749c-5507edab449e-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
2017-03-21 12:10             ` Thierry Reding
2017-03-10 18:24 ` [PATCH v5 net-next 2/9] net: stmmac: configure mtl rx and tx algorithms Joao Pinto
2017-03-21 11:58   ` [v5,net-next,2/9] " Thierry Reding
2017-03-21 12:02     ` Joao Pinto
2017-03-21 12:24       ` Thierry Reding
2017-03-21 13:58         ` Joao Pinto
2017-03-21 14:08           ` Thierry Reding
2017-03-21 14:10             ` Joao Pinto
2017-03-21 14:23               ` Corentin Labbe
2017-03-21 14:25                 ` Joao Pinto
2017-03-21 14:33                   ` Thierry Reding
2017-03-21 14:35                     ` Joao Pinto
2017-03-21 14:18             ` Joao Pinto
2017-03-21 14:35               ` Thierry Reding
2017-03-10 18:24 ` [PATCH v5 net-next 3/9] net: stmmac: configure tx queue weight Joao Pinto
2017-03-10 18:24 ` [PATCH v5 net-next 4/9] net: stmmac: mtl rx queue enabled as dcb or avb Joao Pinto
2017-03-10 18:24 ` [PATCH v5 net-next 5/9] net: stmmac: mapping mtl rx to dma channel Joao Pinto
2017-03-10 18:24 ` [PATCH v5 net-next 6/9] net: stmmac: flow_ctrl functions adapted to mtl Joao Pinto
2017-03-10 18:24 ` [PATCH v5 net-next 7/9] net: stmmac: prepare irq_status for mtl Joao Pinto
2017-03-10 18:24 ` [PATCH v5 net-next 8/9] net: stmmac: mac debug prepared for multiple queues Joao Pinto
2017-03-10 18:24 ` [PATCH v5 net-next 9/9] net: stmmac: configuration of CBS in case of a TX AVB queue Joao Pinto
2017-03-13  6:42 ` [PATCH v5 net-next 0/9] prepare mac operations for multiple queues David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).