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

[Thierry Reding <thierry.reding@gmail.com>]

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

On Tue, Mar 21, 2017 at 03:23:00PM +0000, Joao Pinto wrote:
> Às 3:12 PM de 3/21/2017, Thierry Reding escreveu:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > 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 <treding@nvidia.com>
> > ---
> >  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

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