[PATCH 1/3] net: stmmac: Always enable MAC RX queues

From: Thierry Reding <redacted>

The MAC RX queues always need to be enabled in order to receive network
packets. Remove the condition that this only needs to be done for multi-
queue configurations.

Signed-off-by: Thierry Reding <redacted>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index d3a21519e4c0..298956032098 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c

@@ -1943,7 +1943,7 @@ static void stmmac_mtl_configuration(struct stmmac_priv *priv)
 		stmmac_rx_queue_dma_chan_map(priv);
 
 	/* Enable MAC RX Queues */
-	if (rx_queues_count > 1 && priv->hw->mac->rx_queue_enable)
+	if (priv->hw->mac->rx_queue_enable)
 		stmmac_mac_enable_rx_queues(priv);
 
 	/* Set the HW DMA mode and the COE */

-- 
2.12.0

From: Thierry Reding <redacted>

Prior to the recent multi-queue changes the driver would configure the
queues to use the AVB mode, but the mode then got switched to DCB. The
hardware still works fine in DCB mode, but my testing capabilities are
limited, so it's safer to revert to the prior setting anyway.

Signed-off-by: Thierry Reding <redacted>
---
 include/linux/stmmac.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index be47b859e954..8349a5c1537b 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h

@@ -56,8 +56,8 @@
 #define MTL_RX_ALGORITHM_WSP	0x5
 
 /* RX/TX Queue Mode */
-#define MTL_QUEUE_DCB		0x0
-#define MTL_QUEUE_AVB		0x1
+#define MTL_QUEUE_AVB		0x0
+#define MTL_QUEUE_DCB		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

-- 
2.12.0

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

This text is from the Databook:

"In multiple Rx queues configuration, all the queues are disabled by default.
Enable the Rx queue by programming the corresponding field in this register."

So by theory, only multiple queue configured cores needs the enable operation.

quoted

quoted

But came to my attention a setup that has 1 RX queue and 2 TX queues, which

enables multiple queues mechanism inside the core (even with 1 RX) and so RX
needs to be enabled. Because of that I agree with this patch.

Acked-By: Joao Pinto <redacted>

From: Thierry Reding <redacted>

Recent changes to support multiple queues in the device tree bindings
resulted in the number of RX and TX queues to be initialized to zero for
device trees not adhering to the new bindings.

Restore backwards-compatibility with those device trees by falling back
to a single RX and TX queues each.

Signed-off-by: Thierry Reding <redacted>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 37f550ae76a5..74b0aff79b25 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c

@@ -143,6 +143,13 @@ static void stmmac_mtl_setup(struct platform_device *pdev,
 	struct device_node *tx_node;
 	u8 queue = 0;
 
+	/* For backwards-compatibility with device trees that don't have any
+	 * snps,mtl-rx-config or snps,mtl-tx-config properties, we fall back
+	 * to one RX and TX queues each.
+	 */
+	plat->rx_queues_to_use = 1;
+	plat->tx_queues_to_use = 1;
+
 	rx_node = of_parse_phandle(pdev->dev.of_node, "snps,mtl-rx-config", 0);
 	if (!rx_node)
 		return;

-- 
2.12.0

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

Acked-By: Joao Pinto <redacted>

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

Thierry, I don't understand this patch. It will have 0 impact.

In stmmac_platform configuration, 0 impact:

		if (of_property_read_bool(q_node, "snps,dcb-algorithm"))
			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_QUEUE_AVB;
		else
**			plat->rx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;

In dwmac4_core, 0 impact:

	value &= GMAC_RX_QUEUE_CLEAR(queue);
	if (mode == MTL_QUEUE_AVB)
		value |= GMAC_RX_AV_QUEUE_ENABLE(queue);
	else if (mode == MTL_QUEUE_DCB)
		value |= GMAC_RX_DCB_QUEUE_ENABLE(queue);

I think you should set the default mode in (**).

Thanks.

On Tue, Mar 21, 2017 at 03:18:20PM +0000, Joao Pinto wrote:

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

quoted

From: Thierry Reding <redacted>

The MAC RX queues always need to be enabled in order to receive network
packets. Remove the condition that this only needs to be done for multi-
queue configurations.

Signed-off-by: Thierry Reding <redacted>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index d3a21519e4c0..298956032098 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c

@@ -1943,7 +1943,7 @@ static void stmmac_mtl_configuration(struct stmmac_priv *priv)
 		stmmac_rx_queue_dma_chan_map(priv);
 
 	/* Enable MAC RX Queues */
-	if (rx_queues_count > 1 && priv->hw->mac->rx_queue_enable)
+	if (priv->hw->mac->rx_queue_enable)
 		stmmac_mac_enable_rx_queues(priv);
 
 	/* Set the HW DMA mode and the COE */

This text is from the Databook:

"In multiple Rx queues configuration, all the queues are disabled by default.
Enable the Rx queue by programming the corresponding field in this register."

So by theory, only multiple queue configured cores needs the enable operation.

But that's related to multiple queues configured in the core when it was
instantiated (i.e. the capabilities). rx_queues_count reflects the
number of queues enabled in the driver, so it can be 1 even if the core
itself supports more than one queue.

In that case, we still want to enable the MAC RX queue because it will
otherwise remain disabled.

quoted

quoted

quoted

But came to my attention a setup that has 1 RX queue and 2 TX queues, which

enables multiple queues mechanism inside the core (even with 1 RX) and so RX
needs to be enabled. Because of that I agree with this patch.

Acked-By: Joao Pinto <redacted>

Yeah, that case would also require this patch.

Thierry

On Tue, Mar 21, 2017 at 03:23:00PM +0000, Joao Pinto wrote:

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

quoted

From: Thierry Reding <redacted>

Prior to the recent multi-queue changes the driver would configure the
queues to use the AVB mode, but the mode then got switched to DCB. The
hardware still works fine in DCB mode, but my testing capabilities are
limited, so it's safer to revert to the prior setting anyway.

Signed-off-by: Thierry Reding <redacted>
---
 include/linux/stmmac.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index be47b859e954..8349a5c1537b 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h

@@ -56,8 +56,8 @@
 #define MTL_RX_ALGORITHM_WSP	0x5
 
 /* RX/TX Queue Mode */
-#define MTL_QUEUE_DCB		0x0
-#define MTL_QUEUE_AVB		0x1
+#define MTL_QUEUE_AVB		0x0
+#define MTL_QUEUE_DCB		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

Thierry, I don't understand this patch. It will have 0 impact.

In stmmac_platform configuration, 0 impact:

		if (of_property_read_bool(q_node, "snps,dcb-algorithm"))
			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_QUEUE_AVB;
		else
**			plat->rx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;

In dwmac4_core, 0 impact:

	value &= GMAC_RX_QUEUE_CLEAR(queue);
	if (mode == MTL_QUEUE_AVB)
		value |= GMAC_RX_AV_QUEUE_ENABLE(queue);
	else if (mode == MTL_QUEUE_DCB)
		value |= GMAC_RX_DCB_QUEUE_ENABLE(queue);

I think you should set the default mode in (**).

That was my initial attempt, but then I realized that for old DTBs,
stmmac_mtl_setup() will already exit prematurely because of the missing
snps,mtl-{rx,tx}-config properties. It's pretty much for the same reason
as the separate assignment of the default {rx,tx}_queues_to_use. In this
case it's somewhat more obfuscated, though. Changing AVB to be mode 0
means that plat->rx_queues_cfg[].mode_to_use will contain AVB as default
because plat is devm_kzalloc()'ed.

Effectively this change makes all queues use AVB by default unless they
are configured using the new device tree bindings.

Thierry

Às 4:42 PM de 3/21/2017, Thierry Reding escreveu:

On Tue, Mar 21, 2017 at 03:23:00PM +0000, Joao Pinto wrote:

quoted

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

quoted

From: Thierry Reding <redacted>

Prior to the recent multi-queue changes the driver would configure the
queues to use the AVB mode, but the mode then got switched to DCB. The
hardware still works fine in DCB mode, but my testing capabilities are
limited, so it's safer to revert to the prior setting anyway.

Signed-off-by: Thierry Reding <redacted>
---
 include/linux/stmmac.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index be47b859e954..8349a5c1537b 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h

@@ -56,8 +56,8 @@
 #define MTL_RX_ALGORITHM_WSP	0x5
 
 /* RX/TX Queue Mode */
-#define MTL_QUEUE_DCB		0x0
-#define MTL_QUEUE_AVB		0x1
+#define MTL_QUEUE_AVB		0x0
+#define MTL_QUEUE_DCB		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

Thierry, I don't understand this patch. It will have 0 impact.

In stmmac_platform configuration, 0 impact:

		if (of_property_read_bool(q_node, "snps,dcb-algorithm"))
			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_QUEUE_AVB;
		else
**			plat->rx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;

In dwmac4_core, 0 impact:

	value &= GMAC_RX_QUEUE_CLEAR(queue);
	if (mode == MTL_QUEUE_AVB)
		value |= GMAC_RX_AV_QUEUE_ENABLE(queue);
	else if (mode == MTL_QUEUE_DCB)
		value |= GMAC_RX_DCB_QUEUE_ENABLE(queue);

I think you should set the default mode in (**).

That was my initial attempt, but then I realized that for old DTBs,
stmmac_mtl_setup() will already exit prematurely because of the missing
snps,mtl-{rx,tx}-config properties. It's pretty much for the same reason
as the separate assignment of the default {rx,tx}_queues_to_use. In this
case it's somewhat more obfuscated, though. Changing AVB to be mode 0
means that plat->rx_queues_cfg[].mode_to_use will contain AVB as default
because plat is devm_kzalloc()'ed.

Effectively this change makes all queues use AVB by default unless they
are configured using the new device tree bindings.

Yes I keep forgeting that :), but you are assuming that
plat->rx_queues_cfg[queue].mode_to_use is 0 by default, which might not be the
case, but I agree with you that this is the simpler approach. Let's see what
others have to say.

Thierry

Às 4:50 PM de 3/21/2017, Joao Pinto escreveu:

Às 4:42 PM de 3/21/2017, Thierry Reding escreveu:

quoted

On Tue, Mar 21, 2017 at 03:23:00PM +0000, Joao Pinto wrote:

quoted

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

quoted

From: Thierry Reding <redacted>

Prior to the recent multi-queue changes the driver would configure the
queues to use the AVB mode, but the mode then got switched to DCB. The
hardware still works fine in DCB mode, but my testing capabilities are
limited, so it's safer to revert to the prior setting anyway.

Signed-off-by: Thierry Reding <redacted>
---
 include/linux/stmmac.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index be47b859e954..8349a5c1537b 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h

@@ -56,8 +56,8 @@
 #define MTL_RX_ALGORITHM_WSP	0x5
 
 /* RX/TX Queue Mode */
-#define MTL_QUEUE_DCB		0x0
-#define MTL_QUEUE_AVB		0x1
+#define MTL_QUEUE_AVB		0x0
+#define MTL_QUEUE_DCB		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

Thierry, I don't understand this patch. It will have 0 impact.

In stmmac_platform configuration, 0 impact:

		if (of_property_read_bool(q_node, "snps,dcb-algorithm"))
			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_QUEUE_AVB;
		else
**			plat->rx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;

In dwmac4_core, 0 impact:

	value &= GMAC_RX_QUEUE_CLEAR(queue);
	if (mode == MTL_QUEUE_AVB)
		value |= GMAC_RX_AV_QUEUE_ENABLE(queue);
	else if (mode == MTL_QUEUE_DCB)
		value |= GMAC_RX_DCB_QUEUE_ENABLE(queue);

I think you should set the default mode in (**).

That was my initial attempt, but then I realized that for old DTBs,
stmmac_mtl_setup() will already exit prematurely because of the missing
snps,mtl-{rx,tx}-config properties. It's pretty much for the same reason
as the separate assignment of the default {rx,tx}_queues_to_use. In this
case it's somewhat more obfuscated, though. Changing AVB to be mode 0
means that plat->rx_queues_cfg[].mode_to_use will contain AVB as default
because plat is devm_kzalloc()'ed.

Effectively this change makes all queues use AVB by default unless they
are configured using the new device tree bindings.

Yes I keep forgeting that :), but you are assuming that
plat->rx_queues_cfg[queue].mode_to_use is 0 by default, which might not be the
case, but I agree with you that this is the simpler approach. Let's see what
others have to say.

Forget what I said, yes devm_kzalloc() in plat guarantees this. I need a cup of
coffee :).

Acked-By: Joao Pinto <redacted>

quoted

Thierry

On Tue, Mar 21, 2017 at 04:12:10PM +0100, Thierry Reding wrote:

Tested-by: Corentin Labbe <clabbe.montjoie@gmail.com>

Fix one of the reported problem on dwmac-sunxi and dwmac-sun8i
Regards

From: Thierry Reding <redacted>
Date: Tue, 21 Mar 2017 16:12:09 +0100

From: Thierry Reding <redacted>

The MAC RX queues always need to be enabled in order to receive network
packets. Remove the condition that this only needs to be done for multi-
queue configurations.

Signed-off-by: Thierry Reding <redacted>

Applied to net-next

From: Thierry Reding <redacted>
Date: Tue, 21 Mar 2017 16:12:10 +0100

From: Thierry Reding <redacted>

Recent changes to support multiple queues in the device tree bindings
resulted in the number of RX and TX queues to be initialized to zero for
device trees not adhering to the new bindings.

Restore backwards-compatibility with those device trees by falling back
to a single RX and TX queues each.

Signed-off-by: Thierry Reding <redacted>

Applied to net-next

From: Thierry Reding <redacted>
Date: Tue, 21 Mar 2017 16:12:11 +0100

From: Thierry Reding <redacted>

Prior to the recent multi-queue changes the driver would configure the
queues to use the AVB mode, but the mode then got switched to DCB. The
hardware still works fine in DCB mode, but my testing capabilities are
limited, so it's safer to revert to the prior setting anyway.

Signed-off-by: Thierry Reding <redacted>

Applied to net-next