Re: [PATCH 3/3] net: stmmac: Use AVB mode by default

À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